Skip to content

fix(runtime,mcp): enforce ai.requiresConfirmation at the AI-facing action door, and grow run_action the member that satisfies it - #17486

Merged
os-justin merged 5 commits into
mainfrom
claude/issue-15942-run-action-confirmation-gate
Sep 10, 2026
Merged

fix(runtime,mcp): enforce ai.requiresConfirmation at the AI-facing action door, and grow run_action the member that satisfies it#17486
os-justin merged 5 commits into
mainfrom
claude/issue-15942-run-action-confirmation-gate

Conversation

@os-justin

Copy link
Copy Markdown
Collaborator

Fixes #15942

action.ai.requiresConfirmation was a safety-shaped flag with no execution path: read once, projected once into the list_actions summary, never consulted by run_action. This lands the runtime half of decision batch #54 (option A, maintainer verbatim 「同意」) — the pre-dispatch refusal — together with the MCP door member without which the gate would be unsatisfiable.

The contract half is @objectstack/spec #16293 / PR #16531, merged 2026-09-07. packages/spec is untouched here: it is the contract this implements against.

What changed

file change
packages/runtime/src/action-execution.ts actionConfirmationRefusal() — the gate producer — plus its call site in invokeBusinessAction, after the param contract and before the subject-record load. The door's request shape mixes in AIActionConfirmation from the contract.
packages/runtime/src/domains/mcp.ts the bridge's runAction input type carries the member (it already forwarded the whole request object).
packages/mcp/src/mcp-http-tools.ts the run_action inputSchema grows the member, the handler forwards it, and errorResultFromThrown keeps an ADR-0112 envelope's code / status / details instead of flattening the throw to a sentence.
packages/runtime/src/http-dispatcher.ts the runtime's own "HITL hint" comment, updated to the enforced truth.
packages/mcp/src/skill-md.ts the agent-facing run_action signature and the sentence that makes the member usable.

list_actions is unchanged. There is no queue, no parking, no stored pending-approval record and no resume path.

The member's spelling is never hand-written: both packages import AI_ACTION_CONFIRMATION_MEMBER and use it as the schema key, the forward key and the echoed details.confirmationMember.

The trap this delivery had to clear

The domain:spec seat recorded on the card that enforcing only in invokeBusinessAction would make every gated action un-invokable, because the MCP door strips the member twice over. Both legs were re-measured here rather than taken on trust.

Leg 2 — the handler forward. Confirmed by symbol on the merge base. The coordinates had moved: :761 / :769 when the block was recorded at 11:00Z, :816 / :824 in the claim comment at 15:43Z. PR #17332 edited this file. Both readings were snapshots; the destructure and the forward are what was located, not the line numbers.

Leg 1 — the SDK's unknown-key drop. This is the leg the dispatching seat did not re-measure, so it was measured here, on the unmodified tree, through a real JSON-RPC tools/call:

PROBE rpc-result:   { "content": [ { "type": "text", "text": "{\n  \"ok\": true\n}" } ] }
PROBE bridge-calls: [ [ "archive_account", { "recordId": "r1" } ] ]

The client sent { actionName, recordId: 'r1', confirm: true }. The call was not rejected — it succeeded, and the bridge received recordId and no confirm. recordId surviving the identical round trip is the lit control: the transport works and only the undeclared member was lost. So the strip is a silent drop, not an error, and the two-layer claim holds.

Acceptance notes

The end-to-end drive, both directions

examples/app-todo/test/mcp-actions.e2e.ts gains Step 7. It is the one drive that spans the whole path: a real ObjectKernel, the real ObjectQL engine on a real SQLite driver, app-todo's real registered handlers, the real principal-bound bridge from HttpDispatcher.buildMcpBridge, and the real MCPServerRuntime answering JSON-RPC — both strip layers included. Run at e19fe3a7:

🔒 Step 7 — ai.requiresConfirmation is ENFORCED, and satisfiable
   ✓ list_actions still reports requiresConfirmation:true (unchanged)
   ✓ run_action WITHOUT confirm is refused
   ✓ refusal carries code ACTION_CONFIRMATION_REQUIRED (got ACTION_CONFIRMATION_REQUIRED)
   ✓ refusal carries status 428 (got 428)
   ✓ refusal names the action
   ✓ refusal names the member to set
   ✓ nothing ran — status is still 'not_started'
   ✓ run_action WITH confirm:true succeeds
   ✓ the handler ran — status is now 'completed'
   ✓ a destructive-LOOKING action with no declared flag is NOT gated
✅ MCP action E2E PASSED

The record itself is the evidence in both directions: not_started after the refusal, completed after the confirmed retry. Every refusal case asserts the code and the status; none of them asserts only that something threw.

Reverse verification — the door half is load-bearing

The read and the forward were ablated from mcp-http-tools.ts, @objectstack/mcp rebuilt, the marker confirmed on disk and in dist/index.js, and the drive re-run. It reproduces the seat's warning exactly:

ON-DISK PROOF: forward-line before=1 after=0 injected=1
DIST PROOF: ABLATED-FORWARD in dist/index.js = 1
   ✓ run_action WITHOUT confirm is refused          ← the gate still fires
   ✗ run_action WITH confirm:true succeeds          ← and cannot be satisfied
   ✗ the handler ran — status is now 'not_started'
❌ MCP action E2E FAILED — 2 check(s) failed

The door-level pin goes red on the same ablation (2 failed / 4 passed). Restored with git checkout HEAD -- packages/mcp/src/mcp-http-tools.ts; git diff HEAD empty and the blob hash equal to HEAD's (948ad578f436b489f585ed4f191d8edf94805d97), and dist rebuilt clean (marker count 0).

An earlier ablation of the forward alone failed the DTS build with TS6133: 'confirm' is declared but its value is never read — a second, compile-time tie between the two halves.

CI-visible coverage

  • packages/runtime/src/action-confirmation-gate.test.ts — 9 tests. The refusal asserts code + status + details and that executeAction was never called and that callData (the subject-record read) was never called. Flow-typed actions gated ahead of the type branch, with a lit control that the same action does dispatch once confirmed. 'true', 1, {}, false, null are each refused. Both heuristic cases pinned: a mode: 'delete' action whose author declared nothing is NOT gated (with actionLooksDestructive asserted true on the same fixture, so the case cannot pass vacuously), and an explicit false on a variant: 'danger' action is honoured.
  • packages/mcp/src/mcp-action-confirmation-member.test.ts — 6 tests through the real JSON-RPC door: the member is advertised on the tool schema as a boolean, a gated call is refused with the full envelope, the retry carrying the member reaches the bridge as true, recordId and params ride unchanged beside it, a truthy string never reaches the bridge at all, and an uncoded throw still flattens to its message.

Measurements

run result
pnpm --filter @objectstack/runtime test 254 files / 3558 tests passed
pnpm --filter @objectstack/mcp test 28 files / 303 tests passed
pnpm --filter @objectstack/mcp --filter @objectstack/runtime typecheck exit 0
pnpm lint (whole repo, eslint . --no-inline-config) exit 0
pnpm --filter @objectstack/spec check:generated 15/15 up to date
gate families derived by scripts/pm/dispatch-gates.mjs 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN

check:dual-build-cjs-loads and check:type-check-debt first answered PREREQUISITE NOT MET (exit 3 — explicitly not a pass and not a finding); both were re-run to exit 0 after the full workspace build.

packages/spec's two pins that read the edited runtime file — action-requires-confirmation-docblock.pin.test.ts and action-confirmation-contract.pin.test.ts — pass (12 tests). actionLooksDestructive is untouched, which is what the first of those holds.

One pre-existing pin moved with the surface: skill-md.test.ts pinned the run_action signature string, so it now pins the new one plus the two sentences that make the member actionable.

Notes for review

confirm: true is an unverifiable caller claim. An agent that always sends it bypasses the gate entirely; the ruling accepted that model. The gate makes forgetting loud — it does not prove a human, and the code says so where someone would otherwise assume otherwise.

The enforced set is bounded to the doors that enforce ai.exposed — today invokeBusinessAction via MCP run_action. REST /actions is not ai.exposed-gated (no actionAiExposureError in domains/actions.ts), so an API-key agent there sits outside this gate rather than being silently assumed inside it. Widening that set is its own decision.

Clause-②: yes

This narrows the accept set on a published door: a run_action call on an action declaring ai.requiresConfirmation: true that succeeds today starts answering 428 ACTION_CONFIRMATION_REQUIRED. The diff carries two opposite directions and both belong in the declaration — adding the member to the door's inputSchema widens what run_action accepts (a request that was rejected-by-silent-drop is now a first-class member), while the refusal narrows what succeeds. The narrowing is deliberately as small as the author's own declaration: only ai.requiresConfirmation === true, only the boolean true as an attestation, only the AI-facing doors. Review of record is this seat's default-tier review plus the gates (PR #17294); build stayed default tier.

The changeset is minor on both packages and spells the break with its FROM → TO.


🤖 Generated with Claude Code

https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt


Generated by Claude Code

…tion door

The runtime half of the confirmation gate, plus the MCP door member the
gate is unsatisfiable without.

Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/mcp, @objectstack/runtime, touching 13 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/mcp/src/skill-md.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/environment-routing.mdx (via HttpDispatcher (symbol, a top-level class))
  • content/docs/automation/webhooks.mdx (via HttpDispatcher (symbol, a top-level class))
  • content/docs/kernel/cluster.mdx (via HttpDispatcher (symbol, a top-level class))
  • content/docs/permissions/authorization.mdx (via buildMcpBridge (symbol, a top-level function))
  • content/docs/plugins/packages.mdx (via HttpDispatcher (symbol, a top-level class))
  • content/docs/ui/actions.mdx (via runAction (symbol, a method of interface McpActionBridge))
  • content/docs/ui/apps.mdx (via runAction (symbol, a method of interface McpActionBridge))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-1.mdx (via runAction (symbol, a method of interface McpActionBridge))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/mcp/src/skill-md.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 31 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fa23d69875d1d75b55afc485790fb3df3c9fb712packageMentionDocs.

Which tree this was computed on

This run read content/docs from 16550e30ca8eb622db13db2dc5201497cfb83dab — the merge of head e19fe3a78dcb5a6ab5e70e50175f3fb010cb37d6 into base fa23d69875d1d75b55afc485790fb3df3c9fb712, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 16550e30ca8eb622db13db2dc5201497cfb83dab && git checkout 16550e30ca8eb622db13db2dc5201497cfb83dab
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fa23d69875d1d75b55afc485790fb3df3c9fb712 e19fe3a78dcb5a6ab5e70e50175f3fb010cb37d6 && git checkout -B drift-repro fa23d69875d1d75b55afc485790fb3df3c9fb712 && git merge --no-ff e19fe3a78dcb5a6ab5e70e50175f3fb010cb37d6

node scripts/docs-audit/affected-docs.mjs --json fa23d69875d1d75b55afc485790fb3df3c9fb712

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fa23d69875d1d75b55afc485790fb3df3c9fb712 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants