Skip to content

Audited Action Layer — Design Spec (#1848)

Milestone: audited-action-layer Manual: TBD — contributor-facing only; this is a backend architecture rule, not a user-visible surface (the user-visible consequence — undo, audit history — belongs to the specs that surface it, e.g. kg/kg-readable-representation.md’s kg.read.audit-history).

Design-led (Testing Constitution). Creative director owns intent; tests enforce it; code makes them pass. Status: DRAFT — awaiting approval. Written because a cross-cutting HARD rule (referenced in prose by kg/kg-tables.md, ui/panes-workspaces.md, ui/modes-to-panes.md, ui/automation.md, ui/research.md) had no spec of its own to pin a tag or a test against — this is that anchor. Tags: [OK] built and tested · [PARTIAL] built, partly proven or partly wired · [GAP] intended, never built (needs an issue) · [BROKEN] regression, code contradicts the rule (needs an issue).

Intent (the design)

Every mutating capability in Fichero — create, edit, delete, merge, split, link, curate, anything that changes stored state — is one named, typed action in a single backend registry, reached the same way regardless of WHO is asking: a person clicking a UI button, the in-app agent calling a chat tool, an external MCP client, App Intents/Siri, or a test. The registry is the ONE write choke point: it validates typed params, checks authorization, executes the mutation, writes an audit row that is NOT best-effort (the action fails if the audit can’t be written), and broadcasts a change event that IS best-effort (broadcast failure never fails the action). An action optionally declares itself undoable with an inverse, so the SAME generic undo endpoint reverses (or redoes) any action without per-action undo code.

This converges four historically separate asks into one mechanism: a UI element that silently does the wrong thing (no assertion), an agent that needs the app’s own capabilities as tools (no audited path to hand it), “who changed this and when” (no actor record), and duplicate hand-rolled mutation paths for the same capability (two merge endpoints, two merge UIs). Fixing the write path once fixes all four, and removes the duplication that caused the original entity-merge bug this rule was converged from.

The model is a user. When the in-app agent (chat) invokes an action, it does so as a real account with a role (viewer/editor/owner), through the same ActionContext.actor every human click populates — there is no separate “AI audit,” no privileged bypass. A model’s authority is scoped by that role today; a settings pane for finer per-tool grants does not exist yet (research.md’s research.per-model-tool-grants, [GAP]).

One object model, one write path. Every knowledge object (entity, claim, interpretation, note, annotation) shares this same choke point rather than a hand-rolled service per type — the historical alternative (AnnotationService/NoteService hand-rolled URLSession, hardcoded hosts, missing library-path headers) is the defect class this architecture retires, not a parallel pattern to keep.

What already exists (the mechanism itself — grounded, read 2026-09-18)

  • fichero_server.actions.registry.ActionRegistry (actions/registry.py) — the process-global <domain>.<verb> → ActionRegistration map. @action(name, params_model, domains=…, undoable=…, invert=…, read_only=…) decorates an execute(db, params, ctx) -> (result, ChangeSpec) function and registers it.
  • ActionRegistry.invoke(db, name, raw_params, ctx) is the single mutation path: resolve the action (unknown name → ActionNotFoundError) → validate raw_params into the action’s Pydantic params_model → authorize (authz.assert_can_write, per-target-id when the params name specific ids) → execute() inside db.transaction() when the action is atomic → write an ActionAudit row (action name, actor, client, target ids, params, before/after snapshot, run id) — not best-effort, the action fails if this write fails — → broadcast a ChangeEvent to the observable layer — best-effort, broadcast failure never fails the action → return ActionResult(ok, result, audit_id, changed_domains).
  • ActionContext carries actor, client (attribution only, e.g. fichero-mcp, never an authorization input, #4469), origin_window, run_id (ties an action to an AI run, #1832), library_path, and is_bootstrap. The actor cannot be forged: POST /api/actions/invoke’s request body explicitly REJECTS an actor/origin_window field if a caller sets one (InvokeActionRequest.reject_deprecated_fields, #3285) — the real actor comes exclusively from action_context()’s actor_from_request(request), populated by authentication middleware, never from anything a request body or header could claim.
  • POST /api/actions/invoke — run any registered action by name; the same endpoint chat tools, App Intents, and UI-action tests all drive.
  • GET /api/actions/registry — lists every registered action + its param JSON schema; this is what chat tools and App Intents read to build their tool/intent definitions FROM the registry, not a hand-maintained parallel list.
  • GET /api/actions/audit + POST /api/actions/audit/{audit_id}/undo — the audit log and the ONE generic undo/redo endpoint. Undo: look up the action’s invert(before, after, ctx) -> (inverse_name, inverse_params), then invoke() that inverse tagged inverse_of=this audit. Redo: when the target audit IS an inverse (inverse_of set), replay the ORIGINAL forward action’s recorded name+params — this works for ANY action regardless of how its inverse was implemented, no per-action redo code and no requirement that an inverse action itself be independently undoable.
  • Not built: a fine-grained per-tool grant/deny UI for a model’s role (beyond viewer/editor/ owner) — research.md’s research.per-model-tool-grants.

Behaviors

A. The registry mechanism itself

  • audit.registry-is-the-one-write-choke-point — [OK] every registered action’s mutation runs inside ActionRegistry.invoke; execute() never writes state outside the transaction invoke opens for an atomic action. Verified 2026-09-19 (test-audit follow-up: the citations below were plumbing-only, a throwaway test.dummy action, not a real production action driven end to end): TestActionsRegistryRoute::test_invoke_via_route_writes_audit drives the real entity.merge action through the real POST /api/actions/invoke route with real KnowledgeEntity rows, and TestEntityMergeAction::test_merge_via_registry_effect_and_audit drives the same action directly through registry.invoke, asserting both the merge effect (merged_into_id) and the written ActionAudit row — the choke point proven on real production behavior, not only the dummy. Pinned (plumbing + real-action, both needed): test_action_registry.py::TestRegistryInvoke::test_invoke_returns_result_writes_audit_and_emits, ::test_invoke_validates_params, ::test_invoke_unknown_action_raises, ::TestActionsRegistryRoute::test_invoke_via_route_writes_audit, ::TestEntityMergeAction::test_merge_via_registry_effect_and_audit.
  • audit.actor-cannot-be-forged — [OK] (#3129, #2980, #4844, fixed for #4843’s specific finding by 8aa6c8e12) POST /api/actions/invoke rejects a request body that sets actor/ origin_window directly (InvokeActionRequest.reject_deprecated_fields, → #3285); the real actor is derived exclusively from authenticated request state (action_context() → actor_from_request(request)). A client header (e.g. X-Fichero-Client: fichero-mcp) is attribution metadata only, never an authorization input. The concrete forgeable path this behavior found is fixed: kg/inclusion.py::upsert_inclusion’s InclusionUpsertRequest .updated_by field stays on the request model for compatibility, but its value is now IGNORED — the route stores updated_by=actor (inclusion.py:63,71), never request.updated_by; a test sends a forged name and reads the real actor back from the stored row. Verified at HEAD: no code path reads request.updated_by for the write. The POST /api/actions/invoke guard is now driven through the route too (#4844): a body naming actor or origin_window gets a 422, the action never runs and no audit row is written; the same call without the field runs under the request’s actor. Pinned: test_routes_kg_inclusion.py::test_forged_updated_by_is_ignored, security/test_invoke_actor_cannot_be_forged.py.
  • audit.undo-redo-is-generic — [OK] one endpoint (POST /api/actions/audit/{audit_id}/undo) reverses any undoable action via its own declared invert(), and redoes an inverse by replaying the ORIGINAL forward action’s recorded name+params — no per-action redo code exists or is needed. Verified 2026-09-19 (test-audit follow-up): both cited tests genuinely drive a real round trip — entity.merge through registry.invoke, registry.get(...).invert(...) for the real inverse, then registry.invoke again on the inverse — not a mock or a plumbing-only assertion; this citation was already correct, re-confirmed by opening the test bodies rather than trusted. Pinned: test_action_registry.py::TestEntityMergeAction::test_undo_reverses_merge, ::test_unmerge_undo_remerges_same_entities.
  • audit.one-operation-has-one-undo — [OK] (#4864) one operation has ONE undo, and every undo of it gives the SAME result. Before #4864 an entity delete wrote two trails. delete_entity_impl is reached through the registered, undoable entity.delete action, whose inverse entity.restore brings back the entity AND its claims’ snapshot; the same function also writes a plain MutationLog row on every call, which feeds the older POST /api/kg/mutations/{id}/undo (mutations.py::undo_mutation, one of the nine routes still outside the registry, see audit.every-mutating-route-uses-the-registry). That older undo restores ONLY the entity row: the claims keep their cleared entity links and stripped entity_ids, which is exactly the drift the merge and delete link-integrity fixes removed from the forward path (kg-tables, kg.merge.repoints-subject and its delete sibling). Expected, in two steps. First, the narrowing, which needs no design decision: the older undo never performs a PARTIAL restore. For a mutation whose operation the action layer owns and can invert, it refuses with an error that names the action-layer undo, rather than restoring half the state; a refusal is loud and loses nothing, a partial restore is silent and corrupts curated links. Every other MutationLog writer is audited for the same duplication before the narrowing is called complete. Second (ruled 2026-10-04, see Open questions): the second trail stops being written for those operations. Built: the three registered, undoable actions that wrote a duplicate row — entity.delete, claim.delete, document.batch_exclude — now write their ActionAudit and NO MutationLog row (delete_entity_impl/delete_claim_impl take write_mutation_log=False from the action; batch_exclude_documents_impl no longer writes one). The older route stays only for rows with no undoable action behind them (the document-delete cascade, the NLP-draft purge, the two non-undoable curation batch actions, migrations) and still refuses an owned row an older build already wrote. Pinned: test_mutations.py::TestOneOperationHasOneUndo (incl. ::test_an_owned_operation_writes_one_trail_not_two, one case per action).
  • audit.action-record-not-best-effort — [PARTIAL] (#4845) the ActionAudit write happens inside the same transaction as the mutation and is NOT best-effort — if the audit write fails, the whole action fails (registry.py:220-233). The change-stream broadcast that follows IS best-effort and never fails the action (registry.py:259-290) — an intentional asymmetry: silently losing an audit row is unacceptable, silently missing a live-UI nudge is recoverable on next reload. The happy path and the no-library-path case are pinned (test_action_registry.py::TestRegistryInvoke ::test_invoke_returns_result_writes_audit_and_emits, ::test_emit_skipped_without_library_path) but nothing forces an audit-write failure or a broadcast failure to prove the asymmetry itself — filed as #4845.

B. Every route reaches the registry (the rule this spec exists to test)

  • audit.every-mutating-route-uses-the-registry — [BROKEN] (#2888, #4831) every mutating @router.post/put/patch/delete under api/routes/kg/ and api/routes/entity/ must call registry.invoke( in its own body (directly — no tracing through helper indirection; a route that calls a private helper which itself calls registry.invoke is still flagged, deliberately, per the guardrail’s own docstring). The guardrail (scripts/check_routes_use_action_layer.py, an AST scan) ran for real (re-run 2026-09-18, after tonight’s three landings) and now finds 9 violations + 5 by-design routes across the same 42 decorated mutating routes — down from the 20 violations + 4 by-design this behavior originally found. Three more landings since this spec’s first pass: 8aa6c8e12 (inclusion.upsert, review.accept, review.reject, review.queue, triangulation.recompute — five routes; review.accept also stopped carrying its own copy of the entity-merge algorithm, calling merge_entities_impl instead so its undo is the already-tested entity.unmerge); 8e06dc31d (claim.embed, entity.embed, the external-authority setting, and creating/deciding a prediction review — five more routes, and generate_heuristic_predictions reclassified BY-DESIGN, not fixed — its body only reads and returns, POST only because its parameters travel in the body).

The 7 remaining violations (re-run 2026-10-08; enrich_import moved onto entity.enrich_import, reset_kg no longer exists). None is a mechanical wrap, so each waits on a decision, and the allowlist records why: - rebuild.py::rebuild_kg, pykeen.py::train, predictions.py::apply_prediction_run are long synchronous work; inside registry.invoke they would hold the library write transaction. They need a job-shaped action. - pykeen.py’s delete_trained_model and verify_prediction write the PyKEEN model store on disk, not the library, and have no library database for the audit row. - entity_curation.py::refresh_external_authority is a network fetch plus an upsert of the authority-snapshot cache. Either audit the cache write or call it a cache (by-design). - mutations.py::undo_mutation is ruled to retire (2026-10-04). It now serves only rows with no undoable action behind them (audit.one-operation-has-one-undo).

The 5 by-design routes: render.py::render_paragraph, sparql.py’s sparql_query/sparql_query_legacy, entity_curation.py::enrich_preview, predictions.py::generate_heuristic_predictions.

This behavior is the guardrail’s own home. It stays BROKEN until the guardrail reports zero unallowlisted violations. The count has shrunk 20 → 15 → 9 → 7. Pinned: test_check_routes_use_action_layer.py (unchanged suite, all 13 tests), plus the new landings’ own tests — test_review_actions.py, test_routes_kg_inclusion.py, test_action_layer_batch3.py (TestClaimEmbedAction, TestEntityEmbedAction, TestSetExternalAuthorityEnabledAction, TestPykeenCreateReviewAction, TestPykeenDecideReviewAction). - audit.wrapping-a-route-preserves-its-openapi-surface — [PARTIAL] (#4846) wrapping a bare route to call registry.invoke internally must not change the OpenAPI document — three concrete traps found and avoided while doing this (2026-09-18): (1) adding Depends(action_context) as a NEW parameter adds header parameters (X-Fichero-Origin-Window, X-Fichero-Client) to the schema that weren’t there before — building ActionContext directly from the route’s EXISTING declared params (already-present actor/x_fichero_library_path/x_fichero_origin_window dependencies) avoids the diff; (2) a docstring added to a previously-undocumented function becomes its OpenAPI operation description — an unintended schema change; (3) the inverse mistake — REMOVING a route’s original one-line docstring DELETES that operation’s description, a diff in the other direction (a lesson named in that same landing’s own commit message). Verified via build_openapi_schema() byte-identical checks in the fixing commits (e06b12551, 7523bcf95) — by hand, commit by commit. No standing regression test exists that would catch a FUTURE route-wrapping change breaking either direction — filed as #4846.

C. Attribution — the actor recorded is the real one

  • audit.actor-attribution-is-real-not-hardcoded — [OK] (8e06dc31d; #4843 closed 2026-09-26 after re-verification — test_action_registry.py:204 undo-of-merge, test_routes_entity_curation.py:490 undo-of-split and :857 authority-link, 165 passing) all three sibling spots this behavior found hardcoding created_by="human" regardless of who or what triggered the operation now record the real actor. Verified at HEAD: EntityMergeAudit rows for undo-of-merge, undo-of-split, and the authority-link audit (entity_curation.py, created_by=actor at every one of the sites this behavior previously named) — a grep for the literal created_by="human" in the file finds only the fix’s own explanatory comment, no live hardcode. The authority-link argument that “a person confirmed this match, so the hardcode is a true domain statement” no longer holds — once entity.link_authority became reachable by agents through POST /api/actions/invoke, an agent-confirmed match was a real case the hardcode misrepresented, which is exactly why it was fixed rather than kept. Tests use different actors on each side of an operation, so a coincidence cannot hide a hardcoded value returning. Pinned: test_routes_entity_curation.py::TestUndoEntityOperation, ::TestLinkAuthorityAction. kg/inclusion.py::upsert_inclusion’s related but distinct failure (a caller-CHOSEN actor, not a hardcoded one) is audit.actor-cannot-be-forged above’s finding, not this one’s — also fixed, per that behavior’s own line.

D. Undo/redo is honestly declared

  • audit.undoable-actions-declare-a-real-inverse — [OK] entity.split is registered undoable=True and its inverse genuinely round-trips through the SAME generic undo endpoint as every other undoable action — verified against the code, not assumed from the undoable=True flag alone (undo-of-split restores the aliases the split moved away; it does NOT re-merge the split-off entities, which is existing, deliberate behavior, not a bug). Pinned: test_routes_entity_curation.py::TestSplitEntityAction::test_split_is_undoable_via_the_existing_undo_endpoint.
  • audit.non-undoable-actions-say-so — [OK] entity.link_authority is registered undoable=False — the generic undo endpoint 409s on anything but merge/split, and the action registration honestly does not claim an undo capability the endpoint cannot deliver. Pinned: test_routes_entity_curation.py::TestLinkAuthorityAction::test_not_undoable_no_regression_from_bare_route.
  • audit.every-mutating-op-has-an-undo-surface-or-says-why-not — [PARTIAL] (#4907) every mutating operation should either be reachable through the generic undo endpoint or have a recorded reason it deliberately is not — an unlabeled gap is indistinguishable from an oversight. A guardrail enforces this, scripts/check_undo_coverage.py, against the baseline scripts/check_undo_coverage_known_gaps.json; it is green. The five operations the 2026-09-19 review found (#4907) now each record the true reason: the HPC cluster test and the Wikidata enrich preview write nothing (dry-run probe, read-only fetch); the Kraken install is a no-op since Kraken is bundled; the SPARQL-endpoint list is an app preference, undone by setting it again like the other settings routes; enrich import is audited through entity.enrich_import but registered not undoable (no batch claim-delete inverse yet; each imported claim is removed by its own undoable claim.delete). Still PARTIAL, not OK: many older baseline entries still carry a generic “no undo registration” reason rather than a decided one. Distinct from audit.non-undoable-actions-say-so above, which is about ONE verified-correct example. Test: the guardrail itself, plus scripts/test_check_undo_coverage.py::test_each_reviewed_gap_records_why_it_is_not_undoable and ::test_enrich_import_is_honestly_not_undoable_in_the_registry.

E. Only the action surface reaches a capability

  • audit.chat-tools-share-the-registry — [PARTIAL] (#1847, legacy milestone fold, 2026-09-19) the chat agent should reach the app’s own capabilities as tools — the same registry a UI button and App Intents already reach, not a second hand-built tool list. Verified at HEAD: actions/chat_tools.py — action_tools() generates one LiteLLM/OpenAI function-tool definition per registered action, and dispatch_tool_call() runs the matching one through the SAME audited registry.invoke choke point (actor="chat") every other caller uses — genuinely “one definition, reached five ways,” not a parallel mechanism. Wired into POST /api/chat (api/routes/system/chat.py) behind a default-OFF FICHERO_CHAT_TOOLS flag: with it on, a READ-only tool call dispatches and surfaces in the response as a real tool_calls[] entry with a real audit_id; a MUTATING tool call is explicitly DENIED (recorded, never invoked, no audit row) — a deliberate reads-only slice, not a bug. PARTIAL, not OK: the issue’s own ask (make workflows, run actions — mutating capabilities) is exactly the half still switched off; the mechanism it would route through already exists and is tested. Pinned: test_chat_tools_loop.py (flag-off unchanged behavior, flag-on read dispatch, flag-on mutating denial), test_action_chat_tools.py.

  • audit.only-the-action-surface-reaches-capabilities — [PARTIAL] (#4847, MCP half fixed 93b2e97e5/54a13ffdf; #4866) agents, the CLI, and MCP should reach every mutating capability ONLY through the registered action surface — never a route or code path the action surface doesn’t also expose. The MCP tools half is now built and pinned: all five MCP entity/claim write and delete tools (mcp/tools.py) are thin callers of entity.create/ entity.update/entity.delete/claim.create/claim.delete via registry.invoke — verified at HEAD (registry.invoke(db, "entity.create"/"claim.create"/"entity.delete"/ "claim.delete", …, ctx) at each of the five call sites) — where they previously built rows inline with no ActionAudit, no guarded undo, and none of the action side’s validation. A source-scan test now enforces this as an INVARIANT for the whole file, not just these five: it fails if any function in mcp/tools.py writes to the database outside registry.invoke unless explicitly allowlisted, and the allowlist (the “bypass list”) is now EMPTY — a synthetic case proves the scan still catches a real bypass, so an empty list reads as “nothing bypasses,” not as a broken detector. The CLI half is now pinned too (#4847): a parsed scan of fichero-cli/src fails on any in-process route to a capability: an import of the database, the registry or a route function, or a db.save/db.delete. The CLI may take only types, *_via_http importers and a short, reasoned module allowlist (transport, formatting, local manifest and file work), so it changes a library only over HTTP. Still PARTIAL, not OK: over HTTP, the CLI and MCP can still reach the seven routes audit.every-mutating-route-uses-the-registry lists as bypassing the registry, and the capability-list diff against the registry (Open question 2) is not built. Pinned: fichero-cli/tests/test_cli_reaches_capabilities_only_over_http.py (incl. a synthetic bypass case), test_mcp_tools_write_through_registry.py::test_no_function_writes_to_the_db_outside_registry_invoke_unless_allowlisted, ::test_the_five_reconciled_tools_are_not_in_the_bypass_list, ::test_the_scan_itself_would_catch_a_real_bypass, test_mcp_kg_write_attribution.py::TestEntityCreateIsAccountable, ::TestEntityDeleteIsAccountable, ::TestClaimDeleteIsAccountable.

  • audit.app-intents-use-typed-entities-not-raw-ids — [PARTIAL] (#3304, legacy milestone “UX - Mac - Menus, Commands & Shortcuts” fold, 2026-09-19) every App Intent already routes through invokeAuditedAction → the registry (FicheroActionIntents.swift, verified at HEAD) — the one-audited-action-layer contract this behavior’s siblings above establish holds for Shortcuts/Siri too, not only MCP. But a Siri/Shortcuts user can’t discover a UUID: this issue’s own two findings are partly fixed, partly still open, verified at HEAD 2026-09-19. DeleteDocumentIntent now takes a real DocumentAppEntity parameter (@Parameter(title: "Document") var document: DocumentAppEntity) — fixed, a picker with EntityQuery resolves it by name, not a pasted id — but still has no confirmation step before deleting (perform() invokes document.delete directly; no requestConfirmation), the destructive- action-without-confirmation half of this issue, unfixed. MergeEntitiesIntent, RunWorkflowIntent, and CreateAnnotationIntent still take raw String/[String] id parameters (absorbingEntityId, workflowId, selectedDocumentIds, documentId) despite EntityAppEntity/WorkflowAppEntity-shaped queries existing elsewhere in the codebase for at least some of these — the majority of this issue’s own ask is not yet built. No dedicated pinning test found this pass for either the fixed or unfixed half.

F. Undo scope beyond a single action — not this spec’s mechanism, but its natural extension

The mechanism above (registry, generic undo/redo, actor attribution) is built and largely proven for a SINGLE action. These three legacy-milestone issues (2026-09-19 fold, “UX - Mac - Menus, Commands & Shortcuts” #131) all ask for something structurally bigger — undoing a GROUP of actions, or exposing the audit trail as its own view — none of which exists yet. Grouped here as one family rather than three unrelated gaps, since all three read from the SAME ActionAudit table this spec’s mechanism already writes.

  • audit.run-scoped-undo — [GAP] (#2074, #1831 — the workflow-run half of this issue; its curation-undo half is already covered by the entity.merge/entity.split OK behaviors above, not a second gap) no capability exists to reverse every action of one agent/workflow run (ActionContext.run_id, already stamped on every ActionAudit row) as a single “Undo this run” — verified at HEAD: no POST /api/actions/run/{run_id}/undo-shaped route exists anywhere under fichero-server/src/fichero_server/. The data this would walk (per-run audit rows, newest-first) already exists; only the grouped-reversal endpoint and its UI affordance are missing. ui/activity-and-automatic-work.md (activity.window.what-it-made) is the natural home for the UI half of this once it’s built (a run’s own undo control) — cross-referenced, not restated, since that spec’s own territory note already excludes execution mechanics.
  • audit.blame-and-rollback-view — [GAP] (#1691) no “who changed what, when” view exists spanning documents/pages/entities/claims/notes with author+timestamp, wired to undo/rollback — verified at HEAD: no blame-named route, view, or model was found anywhere in fichero-server/ or fichero/fichero/. The ActionAudit table this spec’s mechanism already writes (actor, target ids, before/after snapshot, timestamp) is the exact data this behavior would read — the gap is a query/view surface over data that already exists, not a new audit mechanism. Explicitly needs-design per the issue’s own label — scope the event model and retention before building, not decided here.

G. Curation protection

  • audit.curation-actions-protect-nlp-drafts — [OK] an entity or claim touched through the action layer (update, merge, link-authority, a claim link) is recognized as “touched” by the NLP draft purge action (entity.purge_nlp_draft, importer.md’s importer.nlp-never-overwrites-curated-rows) via the ActionAudit table itself, not only through domain-specific fallbacks (EntityMergeAudit, a hand-set metadata flag) — proof that routing a capability through the registry has a real downstream payoff beyond the audit log itself. Pinned: test_nlp_draft_purge_action.py::TestF2IndependentTouchProtection (the whole class), test_routes_entity_curation.py::TestLinkAuthorityAction::test_a_linked_entity_survives_the_nlp_draft_purge.

H. Durability and thread-safety of the write path beneath the registry

The registry mechanism above (section A) assumes the write path it sits on is durable and serialized. These two legacy-milestone issues (2026-09-19 fold, “Engine - AI” #90) are about that FOUNDATION, not the registry’s own contract — grouped here because a race or a reset-on-restart beneath the choke point undermines the audit/undo guarantees this spec makes, even when registry.invoke itself is implemented correctly.

  • audit.write-policy-durable — [BROKEN] (#4161) the agent-write policy engine (llm/orchestration_policy.py::PolicyEngine) must persist its rules and its audit trail durably, not reset to factory defaults on every engine restart. Verified at HEAD: PolicyEngine.__init__ still initializes self._records: dict[str, AgentWriteRecord] = {} — a plain in-process dict, exactly as filed. /api/agents/write* and /api/policies/orchestration* remain unusable as a real policy/audit surface across a restart. Not built. The maintainer’s own call on this issue (2026-07-27): fix the 7 in-memory endpoints properly (DB-backed rules + durable audit) rather than delete them — the evaluation logic itself is fine and stays.
  • audit.write-path-is-thread-safe — [PARTIAL] (#2508) every write should serialize through one locked connection, not race across a mixed direct/DBWriter model. Verified at HEAD, from the issue’s own most recent finding (2026-07-30): Phases 0–4 of the fix are REACHABLE on origin/integration — one shared Database connection with a lock per package, locked execute helpers, direct-SQL stores sealed onto that seam, and a permanent guardrail (test_single_connection_guardrail.py) that fails on a bare connection-attribute execute outside the locked seam. Not fixed, and structurally excluded from the guardrail’s own scope, not merely unverified: file_watcher.py, scheduler.py, activity_store, and tasks still open a fresh per-operation duckdb.connect each time — the guardrail’s own docstring says this shape “is NOT matched — that is a connection-per-op handle, not a shared connection touched past the seam,” meaning these four sites can race with the locked seam’s writers and the guardrail would never catch it. Whether a per-operation connection is actually safe here or is the same systemic race the issue names is the issue’s own unresolved question, not decided here. Pinned (the built four-fifths): test_single_connection_guardrail.py::test_db_manager_pool_not_keyed_by_thread_ident, ::test_get_database_is_one_shared_instance_across_threads, ::test_no_raw_conn_attr_execute_outside_seam.

Test matrix

Leg This surface? Pins File
Pure rule (py) y invoke()’s validate→execute→audit→emit contract actions/registry.py (mechanism; see audit.registry-is-the-one-write-choke-point)
Guardrail (Swift-style Python AST scan) y every mutating KG/entity route calls registry.invoke scripts/check_routes_use_action_layer.py + fichero-server/tests/unit/scripts/test_check_routes_use_action_layer.py
Backend (pytest) y undo/redo generic contract, actor cannot be forged, non-undoable says so test_routes_entity_curation.py
Backend (pytest) y an action-layer touch protects an NLP draft row from purge test_nlp_draft_purge_action.py::TestF2IndependentTouchProtection
MCP / CLI y MCP write/delete tools reach capabilities only via registry.invoke; the CLI only over HTTP; both source-scan enforced test_mcp_tools_write_through_registry.py, test_mcp_kg_write_attribution.py, fichero-cli/tests/test_cli_reaches_capabilities_only_over_http.py (audit.only-the-action-surface-reaches-capabilities, [PARTIAL] — the seven bypassing routes and the capability-list diff remain)
OpenAPI diff n no dedicated regression test for the three wrapping traps — (Open Questions)

Hard-gate: audit.every-mutating-route-uses-the-registry (the guardrail exists and is wired into verify_all.sh’s check_*.py sweep by filename, same as every other check_*.py guardrail) once its violation count starts shrinking rather than only being tracked.

Open questions

  1. Answered (design lead 2026-10-04, applying the spec’s lean): Yes, a before/after schema diff test, once the route sweep is closer to done. Should audit.wrapping-a-route-preserves-its-openapi-surface’s three traps get their own dedicated regression test (a build_openapi_schema() before/after diff assertion run as part of the guardrail or a sibling check), rather than relying on each fixing commit having manually verified byte-identity? Recommend: yes, as a follow-up once the route sweep (audit.every-mutating-route-uses-the-registry) is closer to done — not blocking this pass.
  2. Answered (design lead 2026-10-04, applying the spec’s lean): Yes, a guardrail diffs the CLI and MCP capability list against the action registry. Should audit.only-the-action-surface-reaches-capabilities get its own guardrail (e.g. diffing the CLI’s/MCP’s exposed capability list against GET /api/actions/registry), or is this adequately enforced by convention plus the route guardrail above? Not decided here — flagged as a real gap, not assumed solved by the route sweep.
  3. Answered (design lead 2026-10-04, applying the spec’s lean): Yes, understand the self-referencing reversal ids before extending that pattern. EntityMergeAudit’s two self-referencing reversal_id = audit.id writes (noted on #4831 as “meaning unknown; parity kept”) — worth understanding before extending that table’s pattern to more operations, or safe to leave as an established quirk? Not investigated in this pass.
  4. Answered (design lead 2026-10-04, applying the spec’s lean): It retires in favour of the registry’s undo, and MutationLog stops being written for those operations (audit.one-operation-has-one-undo). For the maintainer: once the older mutation undo refuses operations the action layer owns (audit.one-operation-has-one-undo, #4864), does that route RETIRE in favour of the registry’s undo, or stay as a thin caller of it? And when does MutationLog stop being written for operations that already have an action audit?

Sources folded in

Design content carried into this spec; files kept, not moved:

  • ~/.claude/.../memory/one-audited-action-layer.md — origin of the “one named typed action, five ways in” architecture (EPIC #1848), paraphrased throughout the Intent section.
  • ~/.claude/.../memory/agent-chat-model-is-a-user.md — origin of “the model is a user,” a real account with a role, folded into this spec’s “The model is a user” paragraph.
  • ~/.claude/.../memory/knowledge-consistency-mandate.md — origin of “one write path for every knowledge object,” folded into this spec’s “One object model, one write path” paragraph.

Triaged from the backlog (2026-10-04)

  • audit.research-source-actions — [GAP] (#4435) the research source-create and browser-save mutations are audited actions with an inverse (_invert_source_to_delete), so they are undoable, not just logged.
  • actions.no-residual-direct-mutations — [GAP] (#4084) a generated audit lists every handler that mutates outside a registered action with ChangeSpec and inverse (or a stated non-invertible reason).
  • audit.cli-writes-go-over-http — [GAP] (#2884) CLI corpus importers and writes run through the audited registry over HTTP, not in-process.
  • audit.one-action-layer-all-surfaces — [GAP] (#4166) CLI, AppleScript, MCP, Shortcuts, Spotlight and Siri are thin adapters over the one action registry, never parallel implementations (AppleScript run verbs exist, Shortcuts and Spotlight partial).
  • audit.one-action-layer — [PARTIAL] (#1848) every capability is one typed audited action reached by UI, chat tools, App Intents and tests; the spec’s audit.* behaviours track the remaining gaps.
  • actions.drop-resolves-to-typed-action — [GAP] (#3707) every drop (associate, merge, move/copy, export) resolves to a typed, audited, undoable action; artifacts, annotations and pages have no move/reparent action yet.
  • audit.params-carry-a-digest-not-content — [OK] (#5057) every create action that takes content records a digest through the audit_params hook, not the content: all of the issue’s Group A creates (artifact.create/bulk_create, document.create, annotation.create, note.create, research.note.create, interpretation.create, canvas.item.create, bibliography.bulk_import; representation.create before them), and each one’s redo restores the same row through its own inverse. Left alone on purpose: the updates (research.note.update, interpretation.update, canvas.item.update) and bibliography.attach, whose redo replays the new text; workflow.import; the restores; the short texts that are themselves the decision (claim.create, citation.create). Pinned (each reads the persisted ActionAudit row, not the ChangeSpec): test_routes_artifacts.py::…::test_create_audits_digests_not_the_body, test_document_actions.py::…::test_create_audits_a_digest_not_the_page_text, test_routes_annotations_actions.py::…::test_create_audits_digests_not_the_marks_content, test_routes_notes_actions.py::…::test_create_audits_a_digest_not_the_body, test_research_notes_actions.py::test_a_research_note_audits_a_digest_and_its_redo_restores_the_same_note, test_hermeneutics_actions.py::…::test_create_audits_digests_not_the_interpretation, test_canvas_item.py::test_create_audits_digests_and_redo_restores_the_same_card, test_routes_bibliography_bulk_persist.py::test_the_audit_row_holds_the_files_digest_not_the_file.
  • audit.applescript-is-a-door-onto-the-action-layer — [GAP] (#5262) AppleScript engine verbs are generated from the registry/OpenAPI and run through POST /api/actions/invoke gated and audited like CLI and MCP, with front-end hooks only for app-only verbs.