Repository navigation
Add the system initiator for the check engine and maintenance - #2558
Merged
Merged
Conversation
This was referenced Oct 19, 2026
pournasserian
added a commit
that referenced
this pull request
Oct 19, 2026
Closes #2573 ## What The check run slice of F-CHK (#2420), CHK-03 and PERF-02: every submitted change set is checked by the check engine. The results are stored with the set, and the engine, not the agent, moves the set to checks failed or awaiting review. It runs as `SystemInitiator.Checks` (#2558, ADR-0012) with the runner from #2556 and the job queue from #2536. - **Core** (`FluentCMS.Core.Checks`): - `IChangeSetChecks.SubmitAsync` moves a set to checks running and enqueues a `checks.run` job, both in one unit of work, so a set in checks running always has a run coming. - While every active check is deterministic, a set of up to 50 operations is checked on the submitting request, in a scope acting as the engine. Its job waits two minutes and does nothing if that run finished. Any other set is checked by the job, which reports progress at most once a second. - `RunAsync` checks each entry and entity the set creates or updates at its proposed version, against the content type version it was written to. An update's changed fields come from its base version. The active checks are read once, so every item is judged by the same checks. - An LLM-judged check's model call is charged to the agent that opened the set (`CheckInput.ChargedTo`, SEC-04), not to the engine. - The run, its results, `change_set.checked`, one `check.judged` per LLM-judged result (model and prompt hash, CHK-05) and the move all commit together. - **Runs belong to submissions.** A run stores its submission's job id. It does nothing unless the set is in checks running and that job is the set's latest `checks.run` job. This is checked again after the move, inside the unit of work, so a run of an earlier submission, or one another run beat, rolls back and changes nothing. The job payload names only the set. - **Sets do not stay in checks running.** An item that cannot be read, such as an entry deleted after it was proposed, is a problem that fails the run, and the other items are still checked. A conflict is retried. A job that still throws on its last attempt (6) fails the set with a problem. A set whose job ended without moving it, as when the host stops during the last attempt, is failed when the agent submits it again, so it can be reopened. - **A failed set is fixed in place.** A set holds one operation per item. Until now, an agent that reopened a failed set could not change an entry already in it, because the update was refused as a duplicate. Now a later draft of an entry or entity the set already proposes moves that operation on to the new draft. It must be the same change from the same published version. The item keeps one operation, one diff and one version to restore, and `change_set.operation_updated` records the move. Other duplicates, including content type operations, are still refused. The tools' size pre-check no longer counts such a fix as a new operation. - **Reverts are not held.** A revert takes no new operations, so it could never pass a check it fails, such as one asking for a source. Its run is recorded and it goes to review with the results (CHG-06). - **Infrastructure**: tables `CheckRuns` (numbered per set, unique on set and number, with problems as JSON) and `CheckResults` (one row per item and check, with findings and the model call). Migrations for SQLite and PostgreSQL. - **Engine scopes** (`EngineScopes.AddEngineScopes`): a host wraps its `ICurrentInitiator` so Core can open a scope acting as an engine for the inline run. The scope is opened before the set moves, so a host that cannot run the engine fails the submit rather than leaving every set to its job. Opening one is internal to Core and takes only a `SystemInitiator`, so no other code can act as someone it is not. The job worker keeps its own wrapper. - **Tools**: - `submit_change_set` returns the run when it ran inline, or the job to follow otherwise. - New read-only `get_change_set_checks` returns the run of the latest submission (none while a resubmitted set is still running) and its job's state and progress, without the raw error a worker recorded. - MCP-01 coverage row "Read the check results of a change set". - `ICheckRunner` gains an overload that takes the active checks. - CONTRIBUTING.md: the check run. ## Why CHK-03 requires checks on every change set before routing, with results, evidence and cost stored with the set. PERF-02 requires deterministic checks within ten seconds for 200 items, with LLM-judged checks running asynchronously with progress. Choices worth a look: - **Inline for small sets while every check is deterministic, a job otherwise.** The agent gets its findings in the answer to `submit_change_set` for the common case. The job enqueued with every submission makes the run durable even if the inline run never finishes. While any LLM-judged check is active, every set runs as a job, since a model call never holds up a request. Deciding by the checks whose scope applies would need the set's items read before the submit; that can come later if jobs prove slow. - **The inline run ignores the request's cancellation.** The set is already submitted, so an agent that disconnects still gets its set checked, and an inline failure or fault is left to the job instead of being reported as a failed submit. - **Problems as a run-level list**, not fake check results: they say what kept the engine from checking an item and what to do, and they fail the run. - **Fixing in place, in this PR.** It is a change to the change set model (F-CHG), made here because this PR is the first to leave sets in checks failed. The F-CHG lane knows. - **Core logs nothing**, so an inline fault is not logged; the job checks the set two minutes later and records the error if the fault recurs. - **Plugin check types run in the engine's scope.** They are compiled-in extensions today. Per-plugin permissions come with the plugin SDK (EXT-03). - **Passing sets wait for review.** Routing (F-RTE #2421) decides more once it lands. - **Not in this PR**: checks that read entities (F-ENT #2417), and a scheduled retention purge as `SystemInitiator.Retention`. ## Tests Unit: - `CheckRunJob`: - it runs as the engine for its own submission and reports progress - an invalid payload fails the job - a conflict is retried - a fault before the last attempt is retried, and on the last attempt it fails the set - a lost lease ends the attempt without failing the set - `CheckRunner`: an LLM-judged check charges its model call to the agent named. - `ChangeSet`: a later draft of an entry, as a create or an update from the same published version, moves its operation on. A draft that is not later, or a content type operation, is still a duplicate. - MCP-01 coverage row. Integration over MCP, on SQLite and PostgreSQL unless noted: - S2: a set with "best in class", an image without alt text and unsourced copy is checked inline and lands in checks failed, with three failing results whose findings point at the field. `get_change_set_checks` reads the run back, along with its waiting backup job. The agent reopens the set, updates the entry from the draft the set proposes with a source, and submits it again. The same set passes to awaiting review as run 2, holding one create operation at the new draft. The engine's four events name the check engine, with no credential, and `change_set.operation_updated` records the fix. - A set no check applies to passes to awaiting review. - The job of an earlier submission changes nothing, both while the set is reopened and after it is submitted again, even when a person runs it. - An item that cannot be read fails the set with a problem naming it, and the other item is still checked. - While an LLM-judged plugin check is active, a set is checked as a job with progress. The check runs as `SystemInitiator.Checks` and is charged to the agent's credential. Its `check.judged` event names the model, the prompt hash and the set, as the engine, and the run's cost is the judgement's. - A revert whose restored version fails the sources check goes to awaiting review with the failing result. - A set whose check job died while it was in checks running is failed when its agent submits it again, with a problem naming the job, and can be reopened. A person submitting a set whose job failed fails it the same way. - PERF-02: deterministic checks on a 200-item set run as a job and finish in under ten seconds. - The existing write tool test now expects a submitted schema-only set to pass to awaiting review. - A flaky activation race test (`CheckDirectoryTests`) accepts either refusal the second activation can legitimately get. The full suite ran locally against a `postgres:17` container as well as SQLite, and markdownlint passes. No new package. This PR adds no `IContentTypeDirectory.CreateAsync` or `ReviseAsync` call. ## Checklist - [x] Linked to an issue a maintainer has confirmed - [x] Targets `v2` - [x] Tests or docs updated where behaviour changed
3 tasks done
pournasserian
added a commit
that referenced
this pull request
Oct 19, 2026
…fusal tests (#2617) Closes #2600 ## What Hardening from the review of #2558 (`SystemInitiator`, ADR-0012): low-severity gaps, none exploitable today, that future engine or plugin code could have used to act beyond what ADR-0012 allows. - **Reset links** (`IdentityPasswordReset.IssueAsync`): an agent or an engine is refused with `user_people_only`, as managing users already is. No token is issued and nothing is logged. - **Only Core's engines exist**: - `SystemInitiator`'s constructor is internal to Core. Code gets an engine from the named ones (`Checks`, `Retention`). - `InitiatorKinds.RestoreSystem` reads a stored row back only as one of the product's engines, with its own id; any other component name is refused as a fault. - **Engines are an allow-list for jobs**: - `EngineJobs` in Core lists each job handler that may run as an engine, with that engine. Today the check run is listed as `checks`. - The job worker asks it before it runs a handler, and fails the job with `job_engine_not_allowed`, without running it, when: - its handler names an engine it is not listed for, or - an engine enqueued the job and its handler names no engine. Code running inside an engine's scope, such as a check type, would otherwise pass the engine on through a job. - A listed handler runs as its own engine whoever enqueued its job. A handler that names no engine runs as whoever enqueued its job when that is a person or an agent, as before. - **Docs**: ADR-0012 states the allow-list, that a plugin's job handler never runs as an engine (EXT-03), and that check types, a plugin's included, run inside the check engine's scope, bounded by the engine's limits. It also corrects the list of what only a person does. CONTRIBUTING.md says the same for job handlers. - **Tests for refusals that had none**, with an engine or an agent as initiator: - grouping an operation into a session's set, at the directory level - disabling and enabling a user - revoking an agent credential - deactivating a check - issuing a reset link - an audit row and a job row stored with another engine's id, and a row naming no engine of the product, read back ## Why The review of #2558 found three gaps: - `IssueAsync` did not check its caller. - The worker ran a handler as any `SystemInitiator` its `RunAs` returned, and anyone could make one. - Several refusals had no test. Its first review here found two more: code running as an engine could pass the engine on through a job it enqueued, and reading a row could make up an engine. Nothing exploits these today, since only Core code runs, but ADR-0012's "no caller can borrow an engine's permissions" should hold for the code that comes later, plugins included. Choices worth a look: - **Keyed by the handler's type.** A plugin cannot make an object of Core's internal `CheckRunJob`, so it cannot pass as it. Code in the same process could still get around this through reflection; the allow-list stops a handler from claiming an engine, not code set on misuse. Per-plugin permissions come with the plugin SDK (EXT-03, M3). - **Fail, don't fall back.** A job refused this way fails rather than running as its enqueuer or as no one, so the mistake shows on the job instead of running with different permissions than its author expected. - **Check types still run in the engine's scope.** Running them as the agent whose set they judge would record the agent for what the engine does, which ADR-0012 rejects. The engine's own limits bound them. - **Later**: when routing publishes as an engine (F-RTE), jobs that publishing enqueues for handlers outside Core, such as webhook deliveries, would be refused by this rule; #2632 tracks how they should run. - **Merge after #2586.** ADR-0012's model-spend line describes `OnBehalfOf` from #2586. ## Tests Unit: - `EngineJobs`: - the check run runs as `checks` - a handler that names no engine runs as a person's or an agent's job - an unlisted handler that claims an engine is refused - a job an engine enqueued for a handler that names no engine is refused - `SystemInitiator`: one stable id per component, read back only for the product's engines; no public constructor. Integration, on SQLite and PostgreSQL: - A plugin handler that claims `checks` fails its job with `job_engine_not_allowed` and never runs. - A probe check type running as the check engine during a submission is refused a reset link. The job it enqueues for an unlisted handler fails unrun, and an agent is refused a reset link too. Nothing is logged as issued. - The refusals and mismatched rows listed above. - The check run still runs as the engine (`ChangeSetChecksTests`). The full suite ran locally against a `postgres:17` container as well as SQLite, and markdownlint passes. No new package and no migration. ## Checklist - [x] Linked to an issue a maintainer has confirmed - [x] Targets `v2` - [x] Tests or docs updated where behaviour changed
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 #2557
What
A third initiator kind for what the product does on its own, recorded as ADR-0012. It lands ahead of the check run on every change set (F-CHK #2420), which needs it, and routing's auto-publish (F-RTE) can reuse it.
FluentCMS.Core.Identity):SystemInitiator, named for its component:SystemInitiator.ChecksandSystemInitiator.Retention. Its id is derived from the component, so one engine is named one way in every row.InitiatorKinds.System(system). Rows store the component and its id, with no agent credential; the existing constraint (only agent rows carry a credential) holds.initiatorKindsystem, and the audit screen badges it as System.ChangeSet.Openrefuses it (CHG-01), and so do session grouping and revert, which open sets through it.AgentPermissionstreats it as not an agent. A table lists the moves each component owns:checksmoves a set from checks running to checks failed or awaiting review, and routing adds auto-publish when it lands.system_not_permitted).retentionas well as a person.IJobHandler.RunAs, a default interface member. A handler that names an engine has its jobs run as that engine, whoever enqueued them, and the job still records who enqueued it. Job rows map thesystemkind both ways.docs/adr/README.md. CONTRIBUTING.md covers the initiator kinds andRunAs.Why
The check engine has to move a submitted set from checks running to checks failed or awaiting review. Agents may not make those moves (MCP-06), and a job runs as whoever enqueued it, which here is the submitting agent. Recording the move as that agent would be false provenance. A service user would be a fake person in the user list and in the log. The ADR records both alternatives.
For the maintainer: AUD-01 and CHG-01 read "agent/user". The system kind covers only engine-driven moves and maintenance, never a set's initiator and never content writes. The decision record's wording can follow; this PR leaves the requirement documents as they are.
Choices worth a look:
RunAssits on the handler, so no code that enqueues a job can borrow an engine's permissions, and the job row keeps who asked for the work.IJobHandler, and the worker resolves the handler before it sets the run's initiator.Tests
Unit:
system.AgentPermissionsaschecks: checks running → checks failed and → awaiting review are allowed, and nine other moves are refused.retentionowns none of them.Integration, each on SQLite and PostgreSQL unless noted:
system, with its component and id and no credential.retention, notchecks, purges stored prompts, and each purge event namesretention.The full suite ran locally against a
postgres:17container as well as SQLite, and markdownlint passes. No migration and no new package.Checklist
v2