Repository navigation
Hold the agent's credential while counting its caps, and keep refusal events - #2580
Merged
Merged
Conversation
This was referenced Oct 19, 2026
pournasserian
added a commit
that referenced
this pull request
Oct 19, 2026
Closes #2591 ## What The sixth slice of F-DSK (#2422): read-only desk tools for agents (MCP-01, "DSK (read-only for review decisions)"), on the tool conventions from #2508. It fills the DSK row of `McpCoverageTests`. - **`DeskTools`** (`FluentCMS.Core.Builtin.Tools`), three read-only tools: - `get_desk_counts`: how many items wait in each queue. - `list_desk_queue`: one page of a queue, ordered as the desk orders it. - The queues are `awaiting_review`, `checks_failed`, `stale_facts`, `recently_published` and `reverted`. Awaiting me is a person's queue, so it is not offered. - `page` and `limit` are optional (1 to 200, 50 by default), and the result has the total. - A set comes with its id, state, rationale, initiator, item count, the set it reverts and its times. A stale fact is shown as `list_stale_facts` shows it. - `get_change_set_review`: the state and since when, the rejection reason, the set it reverts, and every move in order with who made it and when. - Each tool reads its arguments before any service. An unknown queue, a page below 1 and a limit above 200 are `argument_invalid`. An unknown set is `change_set_not_found` with a hint. - **Agents see their own sets only.** #2580 made an agent read only the change sets it opened (`CheckChangeSetReadAsync`). - `ChangeSetListQuery` gains `OpenedBy`, which `ChangeSetLists` filters on. - `DeskQueues` sets it when the caller is an agent, so an agent's queues and counts hold its own sets. A person's view is unchanged. - `get_change_set_review` reads through `GetAsync`, so another party's set is refused with `agent_not_change_set_owner`. - **`ChangeSetMoves`** (`FluentCMS.Core.ChangeSets`) names the open and move operations once. `ChangeSetLists`, the set view and the review tool used to keep their own copies. - `DeskToolsConformanceTests`, and the DSK rows of `McpCoverageTests` in place of the missing one. - CONTRIBUTING names the desk tools in the MCP paragraph. ## Why - **Scoping in `DeskQueues`, not in each tool.** The queues are defined once for the desk and the tools. Applying the read rule where the queries are built keeps the counts and lists consistent, and a later tool cannot forget it. The rule itself stays in `AgentPermissions`. The list only follows it, as `GetAsync` does. - **`get_change_set_review` next to `get_change_set`.** `get_change_set` returns the set's content. The review status is the decisions: who moved it, when, and why it was rejected. An agent needs that to follow up on a rejection. - **No awaiting me for agents.** An agent never reviews (MCP-06), so the queue would only repeat awaiting review. - **Not in this PR**: writing decisions (people only, MCP-06), and security events for agents (SEC-08 puts them on the desk for people). ## Tests Unit: - `DeskQueuesTests`: an agent's lists and counts carry `OpenedBy` as the agent, and a person's carry none. - `ToolProviderConformanceTests` for `DeskTools`, and `McpCoverageTests` with the new rows. Integration over MCP, each on SQLite and PostgreSQL: - An agent opens, writes and submits three sets through its tools, which are moved to awaiting review and checks failed. The counts and the queues list those sets in the desk's order with paging and totals. A person's set awaiting review is left out, and nothing is appended to the audit log. - The agent reads the review of its rejected set: the reason, its own moves and the person's rejection, with `stateSince` at the rejection. Another party's set is refused with `agent_not_change_set_owner`. - An unknown queue (`awaiting_me`), a limit of 500, a page of 0 and an unknown set are refused, the last with a hint. `DeskQueueTests` now reads the queues as a person, since the scenario's default caller is an agent. `McpAgents.BuiltInTools` lists the three new tools. `dotnet build -warnaserror`, the full suite with the browser tests, and markdownlint pass locally against a `postgres:17` container as well as SQLite. 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
pournasserian
added a commit
that referenced
this pull request
Oct 19, 2026
Closes #2513 ## What The SCH-05 slice of F-SCH (#2415), on top of change set publish (#2482) and revert (#2511). Every content type write is now a change set operation. A create or a revision stores its new version and records the operation in the change set it names, in one unit of work. Nothing a write does is live until that set publishes. - **Core** (`FluentCMS.Core.ChangeSets`): - `ChangeSetTarget`: the set a write joins, either an existing set by id or a session id with the rationale and sources of the set the session's first write opens (CHG-02). - `ChangeSet.Record(operation)` adds the operation, or extends the one the set already holds for the item. The write must be based on the version that operation proposes, and the operation then proposes the write's version. A create stays a create and an update keeps its base, so a set still has one operation per item (CHG-03) and its diff runs from the base to the newest version. - **Core** (`FluentCMS.Core.Schema`): - `IContentTypeDirectory.CreateAsync` and `ReviseAsync` take a `ChangeSetTarget`. A create records a create proposing version 1, and a revision records an update from its base to the new version. - A revision's base is the latest version or the published one. In a set that already holds the type it is the version that set proposes, so the set can keep revising its own draft after another set drafts the type. Basing it on the published version leaves an unpublished draft behind, such as a rejected one. The referenced-field rule (SCH-04) then also checks the fields of every draft an open set proposes, so such a revision cannot drop a field another draft references. - A revision of a type no set has published, or one a published delete took offline (a rejected or reverted create), records a create, since there is nothing live to update. `ChangeSet.Record` takes whether the item is published, and an agent needs the create scope for such a revision (MCP-05). - A draft is a base only while a set that can still publish proposes it. A revision of a rejected or reverted draft is refused at once with `version_conflict` and points to the published version, instead of failing later at publish. While an open set proposes a draft, the latest or an older one, a revision keeps its referenced fields and its entity flag (SCH-03). A draft whose set went stale still counts until a person rejects it; #2588 tracks that. - References between types are checked against what each type is heading to: every draft an open set proposes, or its published version while there is none. A rejected draft neither drops a live reference nor offers a field to reference, and a field a pending draft removes is no target. A field another type references stays while any of that type's live versions or open drafts references it. - `GetPublishedAsync(slug)` reads the live version, or fails with `content_type_not_published`. `ContentTypeSummary` carries `PublishedVersion`. - **Infrastructure**: - `ChangeSetDirectory` gains an internal join for writes owned by another directory. `PrepareAsync` finds the expected failures before the caller's unit of work begins: unknown set, set not open, blank rationale, agent ownership and the items cap. Inside it, `BeginWriteAsync` opens the set or moves its revision, and `RecordAsync` records the operation after the item's own rows and event. An agent's daily cap is counted in `BeginWriteAsync` with its credential held, as `OpenAsync` does since #2580. It is therefore the one refusal that rolls a caller's unit of work back, and #2513's acceptance now says so. - `content_type.created` and `content_type.revised` now carry the set's id, and come after `change_set.opened` when the write opens its session's set. An extended operation appends `change_set.operation_updated`, the name #2575 uses for the same move of an entry operation. - Both directories share one `ChangeSetDirectory` per scope. - **MCP write tools** (#2514): `create_content_type` and `revise_content_type` pass their target to the directory, which now records the operation itself. `ProposedWrite` no longer opens its own unit of work or checks the set ahead of the write; the directory's checks run before anything is written, so a refused cap still keeps its security event. The answer names the kind the set recorded, so a revision of a type with nothing live reads as a create. In `WriteToolsTests` the agent's revision of the person's unpublished product is now expected as `Create null→2` instead of `Update 1→2`, which is the behaviour change agents see. The entry tools (#2525) keep the tool-level pre-check and add, since entry writes do not join change sets in their directory yet. - No migration: both pointers already exist. ## Why The decision, as written in #2513: `Version` is the latest version, which authoring builds on, and `PublishedVersion` is what is live. The authoring reads stay on the latest version: `GetAsync`, `ListAsync`, the schema document and entry validation. A set can then build on its own drafts, and entries can be written against the schema change they need in the same set (CHG-09). Delivery reads the published version when F-API arrives. Choices worth a look: - **The operation is written by the change set directory, inside the content type directory's unit of work.** Version and operation commit together, and the existing "expected failures before the unit of work" rule still holds for both. - **Extending instead of refusing a second write.** An agent iterating on a type within a session would otherwise need a new set for every revision. - **A unique index at commit** means the slug or version race it always meant. When the write opened a session's set, it may also be a concurrent first write of that session, so that case answers with `change_set_conflict`. - **Two revisions racing into the same set**: the loser now fails with `change_set_conflict`, since the set's revision moved first, rather than `version_conflict`. The race tests give each writer its own set to keep checking the version race. - **Reference checks at write time are best effort.** Sets publish in any order, and a revert can take a field away again, so a reference can still dangle across sets. #2604 checks references at publish, which is where the order is known. It also covers dropping a field together with the last reference to it in one set, which the write-time check still refuses. - **Breaking signature**: every caller of `CreateAsync` and `ReviseAsync` passes a target. In this PR that is the MCP schema tools and tests: the schema, entry and MCP scenarios write their content model in a `content-model` session set, as a person, so agent caps and the change set reads in tests are not affected. - **Answering from the set the write joined**: the schema tools read the session's set again after the write, which can race with a submit; #2615 passes the set out of the write. - **Not in this PR**: entries as change set operations, and refusing at publish an entry whose content type version is not published (F-CNT). Also left out: deleting a content type, the MCP schema tools, the desk's schema review and delivery reading the published schema. ## Tests Unit (Core): - A later write extends the item's operation, keeping its kind and base, for a create and an update. - A later write not based on the proposed version, a delete over an update, and an update over a delete are refused and change nothing. - A write is recorded only while the set is open. Integration, each on SQLite and PostgreSQL: - A create is an operation of its set, with `content_type.created` naming the set. The type lists with no published version and `GetPublishedAsync` refuses it until the set publishes. - A revision in the same set extends the operation (create from nothing to version 2). Its events name the version it was based on, and the diff lists both fields as added. - A revision of a published type into a new set is an update from the published version. It is live only after that set publishes, and the previous version stays readable. - After a rejected draft, a revision based on the published version goes through, while one based on the rejected draft fails with `version_conflict`. - After a rejected create, and after a published create was reverted, the next revision is recorded as a create, and publishing it makes the type live again. - A revision from the published version cannot drop a field that another draft in an open set references. - Writes through a session share the set the first one opens. - An unknown set, a set that is not open and a blank session rationale are refused inside a caller's unit of work, which then commits. - Inside a caller's unit of work, an agent cannot write into another agent's set, and a write past its items cap is refused. Neither stores a content type or change set event, the cap refusal keeps its security event, and the caller's unit of work commits. - A failure injected at either audit append of a revision, after the pointer and the set's revision reached the database, leaves neither the version nor the operation. A caller's unit of work rolled back after the revision's rows were saved leaves neither either. - A rejected draft is refused as a base, while a draft an open set proposes can be built on and publishes after it. - An agent without the create scope cannot revise a type with nothing live. - A revision from the published version keeps an open draft's entity flag, and may drop it once that draft's set is rejected. - A rejected draft of a page does not let its brand drop a field the live page references, and a field only a rejected draft has cannot be referenced. - A field that a pending draft removes cannot be referenced, from the draft's own set or another one. - A field that an older open draft references cannot be removed, even when a later draft of that type is the latest. - A set keeps revising its own draft after another set drafted the type, and a revision of it from the latest version is refused with `version_conflict`. - A revision into a rejected set that holds the type is refused with `change_set_not_open`. - A revision keeps the entity flag of an older open draft when the latest draft was rejected. - A content type write that would open a set past an agent's daily cap is refused, stores no set or type, and keeps one security event. - A session's first write appends the set's opening before the content type's own 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 #2570
What
Fixes from the post-merge review of #2480, #2487, #2503 and #2512 (SEC-04, SEC-08, MCP-05).
runningbefore its tool runs, in the transaction that counts it, and its outcome is filled in when it ends.IAgentPermissions.ClaimToolCallAsync(call)replacesCheckToolCallAsync. It holds the credential's row, counts, and records the call as running, or as refused with itsagent.calls_per_period_exceededevent.IToolCallLog.FinishAsyncsets the outcome and duration, only while the call is still running.ToolCallOutcome.Runningis new. The column is a string, so there is no migration.OpenAsyncand a session's first operation now count inside the unit of work that stores the set, after the same hold on the credential.GetAsyncandDiffAsync, soget_change_setandget_change_set_diff, refuse an agent a set it did not open withagent_not_change_set_owner. People and the system read every set.IUnitOfWork.AfterRollback(step)runs a step once the outermost unit of work ends without committing, in a unit of work of its own. A cap refusal registers its event there, so the event outlives a caller's write that the refusal rolled back.Why
Concurrent calls from one agent all passed the calls-per-period count, because a call was recorded only after it ran. Concurrent opens all passed the daily cap, because the count ran before the unit of work. An agent could read any change set by id. A cap refused inside a write tool's unit of work lost its security event when that write rolled back.
Choices worth a look:
UPDATEof the credential's row, as the first write in the transaction. PostgreSQL keeps the row lock and SQLite the write lock until the transaction ends, so one agent's capped actions take turns and other agents are not held up on PostgreSQL. The count after it sees every call admitted before.running, and that call still counts until it falls outside the period.IToolCallLog.AppendAsyncnow records a call whose claim failed, so every call still appears in the log (MCP-07).Tests
Integration, each on SQLite and PostgreSQL:
runningand refuses a second call. It ends as succeeded.agent_not_change_set_ownerfor a person's set and for another agent's set. A person reads both.get_change_setandget_change_set_diffon a person's set returnagent_not_change_set_owner.With the hold turned off, the two concurrency tests fail on PostgreSQL. On SQLite the transaction already takes the write lock when it begins.
The full suite ran locally against a
postgres:17container as well as SQLite, and markdownlint passes. No new package and no new project reference.Checklist
v2