Repository navigation
Refuse agents' review moves and enforce change set caps - #2503
Merged
Merged
Conversation
# Conflicts: # src/FluentCMS.Infrastructure/InfrastructureServiceCollectionExtensions.cs
pournasserian
added a commit
that referenced
this pull request
Oct 18, 2026
Closes #2478 - `IChangeSetDirectory.PublishAsync` publishes an approved set in one unit of work: the set moves to published over the revision it read, each content type's published version moves from the operation's base to its proposed version (a delete clears it), and one `content_type.published` (`content_type.unpublished` for a delete) per item and `change_set.published` are appended with the set's id and sources. `TransitionAsync` to published runs the same publish. - An item no longer published at its operation's base version fails the publish with `base_version_stale` before the unit of work begins, one error per item. A concurrent publish over the same item is caught by the conditional pointer update, which rolls back everything moved before it. - `PublishAsync` runs the agent move check from #2503 once the set is read and before anything is written, so an agent cannot publish through either call (MCP-06). Tests that walk a set to published publish a fresh type each time. - Content types get `PublishedVersion`, moved only by publish. Content type writes and reads are unchanged until SCH-05 (F-SCH). Migrations for SQLite and PostgreSQL. - Tests cover publish with a create and an update, a published delete, sets that are not approved, stale items, a failure injected at each audit append of a publish, both races and a caller's outer unit of work. Test plumbing: `TestServices.Build` takes a callback that runs after Infrastructure, used to wrap the audit log, and the race helper moved into `ChangeSetScenario`. Checked locally: `dotnet build -warnaserror` and `dotnet test` on SQLite and on PostgreSQL 17.
This was referenced Oct 18, 2026
Merged
pournasserian
added a commit
that referenced
this pull request
Oct 18, 2026
Closes #2510 ## What The fifth slice of F-CHG (#2418): one revert action (CHG-06, D22), built on atomic publish (#2482). Reverting a published set opens a new set that puts every item back at the version it had before the original. Publishing that set restores the live content and moves the original to reverted, in one unit of work. - **Core** (`FluentCMS.Core.ChangeSets`): - `ChangeOperation.Inverse()`: a create becomes a delete of the created version, an update becomes an update from its proposed version back to its base, and a delete becomes a create of the deleted version. An update back to an earlier version exists only this way; `ChangeOperation.Create` still asks for a later one. - `ChangeSet.Revert(rationale, initiator)` opens a set with the inverse of every operation in the original's order. It names the original in `RevertsChangeSetId` instead of citing sources. Only a published set can be reverted (`transition_not_allowed`), and a revert set takes no further operations (`revert_set_fixed`). - **Directory**: - `RevertAsync(changeSetId, rationale)` stores the revert set and its operations with `change_set.opened` and one `change_set.operation_added` each, whose previous version is the row of the version the original published. Before the unit of work it refuses an unknown set, a set that is not published, a blank rationale, and an item that moved on since (`base_version_stale`, as the revert's publish would). - `PublishAsync` of a revert set also moves the original from published to reverted over its revision, and appends `change_set.reverted` after `change_set.published`. If the original is no longer published, nothing is written. - `TransitionAsync` to reverted is refused with `transition_not_allowed`, after #2503's agent check, so an agent's attempt is still logged. - **Agents** cannot revert: `RevertAsync` runs the move check for reverted from #2503 (MCP-06). People are not capped, so the change set caps do not apply. - **Infrastructure**: `ChangeSets.RevertsChangeSetId`, nullable and indexed, with a foreign key to the reverted set. Migrations for SQLite and PostgreSQL. ## Why ADR-0005 says a revert restores the previous version pointers, so a revert proposes the earlier, stored version instead of writing a copy. Live content then reads exactly as it did before the original, and the diff, the stale check and publish all work on it unchanged. Choices worth a look: - **A revert is a normal set.** It goes through checks and review like any other (S3 talks about it publishing), and nothing moves until it is published. Fixing its operations keeps "restores exactly what the original changed" true while it waits. - **The stale check runs when the revert opens.** A revert of a set whose item has moved on could never publish, so it is refused at once with the same message publish would give. - **Two reverts of one set**: the second one's publish fails, because the items have moved on and the original is no longer published, so the original is reverted once. - **The original moves with the revert's publish, not when the revert opens.** Until then the original is still what is live. - **Not in this PR**: fact propagation (F-PRP, M3), the correction as rule feedback (RUL-08), the desk's revert action, the MCP tool, webhooks on revert, and other item kinds, which publish with their epics. ## Tests Unit (Core): - The inverse of a create, an update and a delete, and the inverse of the inverse. - An update back to an earlier version is refused by `Create`. - A revert holds the inverse operations in order, names the original, has no sources, and leaves the original as it was. - Only a published set can be reverted. A revert needs a rationale and takes no further operations. Integration, each on SQLite and PostgreSQL: - Reverting a published set with a create, an update and a delete stores the inverse operations, with each event naming the row it is based on. The diff shows the update going back. - Publishing the revert puts every content type back at its earlier published version and reverts the original. The events are one per item, then `change_set.published` and `change_set.reverted`. - Reverting the revert publishes the original's versions again. - An unknown set, an open set, a set whose item moved on and a blank rationale are refused without writing a set or an event. An operation added to a revert set and `TransitionAsync` to reverted are refused. - An agent's revert is refused and logged as `agent.transition_refused`. - Of two reverts of one set, only the first publishes. - A failure injected at each of the five audit appends of a revert's publish leaves the items, the revert and the original as they were. `dotnet build -warnaserror` and the full suite pass locally on SQLite and against a `postgres:17` container. No new package and no new project reference. ## Checklist - [x] Linked to an issue a maintainer has confirmed - [x] Targets `v2` - [x] Tests or docs updated where behaviour changed - [x] No tool-generated co-author trailers or footers
3 of 4 tasks
pournasserian
added a commit
that referenced
this pull request
Oct 18, 2026
Closes #2509 ## What The write tools of F-MCP (#2423) for content types and change sets, on the conventions from #2508. An agent proposes schema changes and submits them, and a person reviews and publishes them. - **Change sets**: `open_change_set` (rationale, sources), `submit_change_set` (open to checks running) and `reopen_change_set` (checks failed to open), for the agent's own sets only (#2503). - **Content types**: `create_content_type` (name, slug, definition) and `revise_content_type` (slug, base version, definition). - Each stores the new version, unpublished (#2482), and adds the matching schema operation to a change set in one unit of work, so either both are written or neither is. - The set is a `change_set_id` from `open_change_set`, or a `session_id` with a rationale and sources, which opens the session's set on its first call (CHG-02). - Each write result carries the content type, the change set and a `next` sentence that says what comes next. - **Core**: - `IChangeSetDirectory.GetSessionSetAsync` reads the caller's open set for a session id. - `IAgentPermissions.CheckChangeSetSizeAsync` takes the number of operations a write is about to add. - `ToolArgumentsReader` reads lists of strings, objects, optional ids and required whole numbers. - **CONTRIBUTING.md**: the read and write tools and what is live when. ## Why MCP-01 asks for tools for every capability, and the governance model needs everything an agent writes to wait in a change set for a person. With #2482, a stored content type version is not published until its set is. With this PR, every version an agent stores is also in one of its change sets, and no tool approves, rejects or publishes (MCP-06). Choices worth a look: - **One unit of work for both writes.** Each directory finds its expected failures before its own unit of work, so the tool's outer one undoes the version when the change set refuses the operation, for example because of the item's version, a cap or ownership. - **Caps are checked before the write too.** A refused cap inside the outer unit of work would roll back its own security event (SEC-08, the limit noted in #2503). The tool therefore first reads the target set and asks `IAgentPermissions` with the operation it is about to add, or asks for a new set when the session has none. That refusal commits its event, and nothing is stored. The set checks again on the add, which settles a race. - **`GetSessionSetAsync` is a small read on the change set directory**, the one place that knows a session's open set. The pre-check needs it. - **No operation scope yet**: an agent with any key can propose. Type, field and operation scopes are the credentials lane's. - **Create and revise of one type in one set** is refused by the set (one operation per item), with the set's own message. - **Submit and reopen are separate tools**, so each description says exactly when to call it. - **Not in this PR**: approve, reject, publish and revert (never for agents), bulk operations with progress and the MCP-01 coverage test (next slice), and entry and entity writes (their lanes). ## Tests Integration, over HTTP: - On SQLite and PostgreSQL, an agent opens a set, creates `category`, revises a person's `product` from version 1 and submits the set. The set holds both operations in order, is in checks running and names the agent, and the tool call log has all four calls, succeeded. - On SQLite and PostgreSQL, two creates with one `session_id` land in one set with the first call's rationale. - On SQLite and PostgreSQL, these store no version, while the agent's own valid create is stored: - a definition with an invalid slug, an unknown field type and a bad field name returns every error at once - a create past a cap of one item is refused, and `agent.items_per_change_set_exceeded` stays in the log - a create into a person's set and a submit of it are refused as not the agent's own - a create without a target names the missing argument - A revision from a stale base version returns `version_conflict` with a hint to read the current version, and the type stays at version 2. - The conformance tests now cover the write tools: names, described schemas with valid examples, and unknown arguments refused before any service is read. Unit tests cover the new reader methods. The full suite ran locally against a `postgres:17` container as well as SQLite, and markdownlint passes. No new package and no new project reference. ## Checklist - [ ] Linked to an issue a maintainer has confirmed - [x] Targets `v2` - [x] Tests or docs updated where behaviour changed - [x] No tool-generated co-author trailers or footers
3 of 4 tasks
pournasserian
added a commit
that referenced
this pull request
Oct 18, 2026
Closes #2516 ## What The MCP-01 coverage test of F-MCP (#2423). `McpCoverageTests` lists the capabilities MCP-01 names with the tools that cover each one. - Built rows: SCH (list, read, schema document, create, revise) and CHG (open, read, diff, submit, reopen). - Expected-missing rows, each naming the epic that adds it: CNT #2416, ENT #2417, RUL #2419, CHK #2420, DSK read-only review decisions #2422, MED #2427, search #2430, and MCP-08 bulk operations #2423. - The test resolves every `IToolProvider` the way the host does (`AddFluentCmsCore`) and fails when: - a built row's tool is not registered - a registered tool is in no row - a missing row names no epic - a tool name starts with approve, reject, publish or revert (MCP-06, SEC-02) - CONTRIBUTING.md: a new tool adds its row in the same pull request. ## Why The epic's acceptance asks for a coverage test against the MCP-01 list that passes at the end of M1. Listing unbuilt capabilities with their epics lets it pass today and say exactly what is left. Failing on an unlisted tool keeps the list current as lanes add tools. Choices worth a look: - **Bulk operations stay expected-missing.** MCP-08 needs the jobs table from ADR-0010, which does not exist yet: `JobQueueWorker` only keeps its polling rhythm. It also needs change set operations on entries and entities, which the change set directory still refuses with `item_kind_unavailable`. Its row names #2423 until both land. - **Review verbs in names**: tool names start with their verb, so the check looks at the first word, and a read tool such as `get_published_entry` passes. A name check is coarse, but it stops the obvious mistake. The Core check from #2503 is what actually refuses the move. ## Tests The four checks above pass on v2. The full suite ran locally against a `postgres:17` container as well as SQLite, and markdownlint passes. No new package and no new project reference. ## Checklist - [ ] Linked to an issue a maintainer has confirmed - [x] Targets `v2` - [x] Tests or docs updated where behaviour changed - [x] No tool-generated co-author trailers or footers
pournasserian
added a commit
that referenced
this pull request
Oct 18, 2026
Closes #2507 ## What The scope checks on content and the calls-per-period cap, both failing closed (MCP-05, SEC-04). They build on `IAgentPermissions` from #2503 and on the tool call log from #2490. - **Core** (`IAgentPermissions`, `AgentPermissions`): - `CheckContentAsync(type, operation, fields?)` checks the operation on the content type and, for a write, the fields it changes. - `CheckDefinitionAsync(type, operation)` checks a type's definition, which also needs a scope that does not narrow the type to some of its fields. - `ReadableTypesAsync` filters lists. - `CheckToolCallAsync` applies the calls cap. - A scope refusal fails with `agent_scope_denied` and names the part of the scope that refused: the operation, with what the scope allows; the type, with the types in scope; or the fields, with the fields allowed. - People pass everything, and a refused credential is refused as in #2503. - **Reads** need `read` on the type, including: - a content type and its schema document - an entry, its versions and its history - a resolved view - where an entry or an entity field is used - **Lists** leave out types the agent may not read: the content type list, the site's schema document, deleted entries, and where-used answers. - **Entry writes**: - Each write needs its operation on the type. Create sets fields, update changes them, and delete and restore both need delete. - Every field the write changes must be allowed. A create names the fields it sets. An update names those whose values differ from its base version, compared as JSON with `ValueDiff.Compare`. - **Definitions**: creating or revising a content type, and a change set's schema operation, need the operation on the type through `CheckDefinitionAsync`. - **Calls per period** (`ToolCallRecording`): - Before a tool runs, the filter asks `CheckToolCallAsync`, which counts the credential's calls in its period through `IAgentActivity.ToolCallsSinceAsync`. - Past the cap, the tool does not run. The agent gets an error result that names the cap, its value and the period. `agent.calls_per_period_exceeded` is appended (SEC-08), and the call is recorded with the new outcome `Refused`. - **No migration**: outcomes are stored as names, so `Refused` needs no schema change. - **CONTRIBUTING.md**: how scopes and the calls cap apply. ## Why MCP-05 gives each credential a scope of types, fields, operations and sites. SEC-04 says a call past a cap fails closed. ADR-0007 puts both in Core, so tools and forms cannot diverge. The epic's acceptance needs an agent that creates types, entries and entities within its scope and nothing outside it. Choices worth a look: - **Reads take whole entries**: the field list narrows writes only. Narrowing reads would mean redacting stored documents and resolved views, and the glossary's "may touch" reads as writing. Fact values a resolved view inlines from another type belong to that view, as CNT-04 intends. - **Writes need read too**: an entry write is checked against the definition it reads through `IContentTypeDirectory`, so `read` on the type is part of writing. The refusal says so. - **Definitions are whole**: a definition covers every field, so a credential narrowed to `product.price` cannot create, revise or propose the `product` definition, but it can for types its field list leaves whole. - **Restore takes delete**, since it undoes a delete. - **Lists filter rather than fail**, because a list has no single type to name, and an agent sees only what it may read. A direct read of a type outside the scope fails and names the type, so the agent learns why. - **Refused calls do not count**: otherwise an agent retrying at its cap would keep itself there. "Per period" is a sliding window, like the change set cap in #2503. The count is made before the call and records are written after it, so concurrent calls can exceed the cap by at most the number in flight. - **A refused tool call is a tool error result**, not a protocol error, so the model reads the message, as UX-03 asks. - **Not in this PR**: LLM spend per period (needs the model gateway, F-CHK #2420), which change sets an agent may read, and usage views (ADM-03). ## Tests Integration, each on SQLite and PostgreSQL: - An agent scoped to `category` is refused 11 reads and writes of `product` and its entries, each with `agent_scope_denied` naming the type and the types in scope. - The refused calls are: the type, its schema document, entry list, get, version, history, resolved view, where-used, create, update and delete. - Nothing is written, `category` works, and the type list, the site schema document and deleted entries leave `product` out. - An agent with `read` alone is refused create, update, delete, restore, type create and type revise, each naming the operation, and nothing is written. An agent with `create` alone sees no types and is told a write needs `read`. With every operation all of these succeed. - An agent limited to `product.price`: - changes the price while sending the name back unchanged - creates a product with only a price, writes `category` in full, and revises the `category` definition - is refused, with the allowed field named, a rename, a dropped name, and a create that sets the name - is refused revising the `product` definition and proposing it in a change set - The refused writes leave no entry, version or event behind. The only new event is the change set it opened. - Over HTTP with a cap of two calls an hour: - The third and fourth calls are refused with a message naming the cap. Each appends `agent.calls_per_period_exceeded` for the credential and is recorded as `Refused`. - An hour after the first call, one more call runs, because refused calls do not count, and the next is refused again. The #2503 permission tests, the change set and entry tests, and the tool call tests pass unchanged. `dotnet build -warnaserror`, the full suite against a `postgres:17` container as well as SQLite, and markdownlint pass locally. No migration, no new package and no new project reference. ## Checklist - [x] Linked to an issue a maintainer has confirmed - [x] Targets `v2` - [x] Tests or docs updated where behaviour changed - [x] No tool-generated co-author trailers or footers
This was referenced Oct 19, 2026
pournasserian
added a commit
that referenced
this pull request
Oct 19, 2026
) Closes #2616 ## What A follow-up of F-CHG (#2418), from the post-merge review of publish (#2482) and revert (#2511). - **Core** (`ChangeSetTransitions`): the table gains approved → rejected. - A rejection still needs a reason (CHG-08). - Only people make this move. Agents are refused it, as any move other than submit and reopen is since #2503 (MCP-06). No engine owns it in `AgentPermissions.SystemMoves` (ADR-0012). - **Entries**: a rejected set holds nothing in review (CNT-01), so the entries of an approved set that a person rejects leave review with no further change. - **Publish** moves its items sorted by item kind and id, then appends their events in the set's order. Two sets that publish the same items take their row locks in the same order: the later one waits, finds its base gone and fails with `base_version_stale`. - **Deadlocks**: a PostgreSQL deadlock (`40P01`) or serialization failure (`40001`) during a publish, or at the commit of any change set write, is answered with `change_set_conflict` instead of an exception, as a unique violation already is (`DatabaseErrors.IsConcurrencyFailure`). - **Desk**: the set view offers Reject on an approved set too, next to Publish. Its hint says to reject it with a reason if it can no longer publish. ## Why An approved set whose publish has gone stale could never leave approved. For example, two reverts of one set are both approved and the first publishes. The second then sat approved forever, and its entries stayed in review. Rebasing a stale set is out of scope: a person rejects it and the author opens a new set. Publish moved items in the order the set lists them. On PostgreSQL, two sets that list the same items in opposite orders deadlocked, and one call threw. Choices worth a look: - **Lock order instead of retry.** Sorting the pointer moves removes the cycle. The deadlock mapping stays as a safety net for writers outside publish. - **The mapping has no test of its own.** With the sort in place, two publishes no longer deadlock, and no other writer here takes two item rows. To check the race test, I removed the sort and the mapping on purpose: it failed on PostgreSQL with `40P01: deadlock detected`, and with both back it passes. - **Events keep the set's order**, so the audit log reads as the set was written, whatever order the rows were locked in. - **The desk change is two conditions and one hint.** Without it, a person had no way to reach the new move from the admin. ## Tests Unit: - The transition table lists approved → rejected, and every other transition stays as it was. - `Reject` from approved succeeds with a reason, and stays refused where the table has no rejection. - The check engine cannot reject an approved set. - The set view offers Publish and Reject for an approved set. Integration, each on SQLite and PostgreSQL: - Two reverts R1 and R2 of one published set are both approved, and R1 publishes. R2's publish then fails with `base_version_stale`, and its entry stays in review. A blank reason is refused. With a reason, R2 is rejected, the entry leaves review, and the original set stays reverted once. - Two approved sets update the same two content types from the same base, listed in opposite orders, and publish at the same time, five rounds in a row. Each round, one wins, the other fails with `base_version_stale`, and neither call throws. - The table-driven tests cover the new transition for agents (refused and logged) and for people (stored with its event). `dotnet build -warnaserror` and the full suite pass locally on SQLite and against a `postgres:17` container. No new package and no new project reference. ## Checklist - [x] Linked to an issue a maintainer has confirmed - [x] Targets `v2` - [x] Tests or docs updated where behaviour changed - [x] No tool-generated co-author trailers or footers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2496
What
The next F-MCP slice (#2423) after agent authentication (#2485). Core now refuses what an agent may not do to change sets: agents never approve or publish (MCP-06, SEC-02), and the change set caps on a credential fail closed (SEC-04). Each refusal is a security event in the audit log (SEC-08).
FluentCMS.Core.Agents):IAgentPermissionsdecides for the current caller, andAgentPermissionsimplements it.IAgentActivitycounts what a credential has done for its caps. Infrastructure implements it by counting the credential'schange_set.openedevents.agent_not_change_set_owner.agent_transition_not_permittedand a message that says who makes that move.ItemsPerChangeSetis refused withagent_cap_exceeded, naming the cap and its value. This applies to explicit sets and to a session's set.ChangeSetsPerDayin the last 24 hours is refused the same way, whether the set is opened explicitly or by a session's first operation. Joining an open set is not opening one.agent.transition_refusedandagent.items_per_change_set_exceededname the change set, andagent.change_sets_per_day_exceedednames the credential. The refused write itself leaves nothing behind.ChangeSetDirectorygains only the four calls intoIAgentPermissionsand the target state passed toMoveAsync.Why
The epic's acceptance needs an agent that tries to approve or publish to be refused whatever its scope, with the refusal logged, and a call past a cap to fail closed with a message that names the cap. ADR-0007 puts both in Core, so a tool and a form can never disagree.
Choices worth a look:
change_set_conflict.AuditReaderdoes, because EF Core does not compare aDateTimeOffseton SQLite.ChangeSetScenarioagent now has a stored credential with room to spare. The existing tests make the checks' and reviewer's moves as a person throughAsPersonAsync. Their assertions are unchanged.PublishAsync. Whichever of the two PRs merges second adds the same check there. I've told the session that owns Add atomic change set publish #2482.Tests
Integration, each on SQLite and PostgreSQL:
agent.transition_refusedwith the agent's credential.The full suite ran locally against a
postgres:17container as well as SQLite. No new package and no new project reference.Checklist
v2