Skip to content

fix(runtime): consume the caller-scope load verdict at the flow and script action doors - #16854

Merged
os-project-manager merged 4 commits into
mainfrom
claude/issue-16370-flow-door-record-load-denied
Sep 8, 2026
Merged

fix(runtime): consume the caller-scope load verdict at the flow and script action doors#16854
os-project-manager merged 4 commits into
mainfrom
claude/issue-16370-flow-door-record-load-denied

Conversation

@claude

@claude claude Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #16370

Clause-②: no

The defect

packages/runtime/src/action-execution.ts computes one caller-scope load verdict — recordLoadDenied, from the single producer loadActionSubjectRecord — for every action door. Exactly one door consumed it as a refusal: the declarative update (#15079, contract point 3). The flow door and the script/body door spread the same verdict into the context as a field and proceeded.

So MCP run_action on a type: 'flow' action answered ok: true and started a persisted run for a recordId the caller cannot read — and, identically, for an id that names nothing at all — while get_record answered "not found" and update_record answered "no access" for that same id in the same session. Nothing in the response told the calling agent the row had not been delivered.

#15168 carried the verdict into the flow context and said in as many words that "whether the automation engine acts on it (a flow-level refusal, a step condition) is a separate reading". This PR is that reading.

What changed

One shared refusal, refuseDeniedSubjectLoad(objectName, recordId, subject), in action-execution.ts beside the producer that computes the verdict. Three doors call it, on both surfaces:

door MCP run_action bridge REST /actions route
declarative update executeDeclarativeUpdateAction — converged onto the shared function, behaviour unchanged to the byte same executor, same call
flow new contract point 3 block in invokeBusinessAction new contract point 3 block in domains/actions.ts
script/body same block, ahead of the trusted-mode plumbing same block, ahead of the trusted-mode plumbing

Placement is load-bearing in both directions. It sits after the declarative branch, whose own points 7 and 4 answer a located 400 before point 3 ever runs — hoisting the refusal over them would re-label those prescriptions as a 404. And it sits before the identity resolution and everything below it, so no persisted automation run exists and no trusted, RLS/FLS-bypassing body has been entered when the refusal lands.

验收备注

1. ⭐ Mirrors the shape of the existing contract point 3 block, comment convention included

Both new blocks open with the file's own marker, // ── contract point 3: the caller-scope load's VERDICT, consumed ──, and the existing declarative block keeps it while delegating to the shared function. Three doors, one rule, one implementation, three visible contract points — which is the part that makes the next reader able to see there are three doors at all.

2. ⚠️ The script/body door is fixed in the same PR as the flow door — and so is the REST surface

The card names two doors; the file's own comment at the shared producer's MCP call site names two surfaces ("this door and the REST /actions door emit the same recordLoadDenied signal to handlers"). Both readings are honoured: the refusal is called at the flow door and the script/body door, on the MCP bridge and on the REST route. packages/runtime/src/domains/actions.ts is therefore edited too — one file outside the claim's declared surface. Re-scanned before editing: 22 open PRs, full pagination (6 pages for the Version Packages PR), 687 file rows, 673 distinct filespackages/runtime/src/domains/actions.ts 0 holders, packages/runtime/src/action-execution.ts 0 holders. Positive control fired on the same scan: packages/runtime/src/dispatcher-error-vocabulary.ts 2 holders (#16827, #16730), packages/runtime/src/domains/automation.ts 1 holder (#16755).

Leaving the REST script door open would have re-created the exact failure class #14143 and #15168 each already paid for on this seam.

3. Record-less and new-record actions are unchanged — by construction, not by a second test

The predicate is subject.recordLoadDenied, which the producer sets only when a caller-scope load was attempted: a recordId was supplied and the action key is not object-less. A record-less (object-less key) action and a new-record (no recordId) invocation therefore never reach the refusal, and their recordId stamp on ctx.record is byte-for-byte what it was.

⛔ Deliberately not summarizeAction's requiresRecord, the other spelling the card offers: that value is derived from locations, optional metadata an author may omit entirely — which is exactly why contract point 7 one function up refuses to be left to it. An authorization refusal keyed on optional metadata is absent for precisely the authors who declared the least, and schedule_followup-shaped actions that declare no locations would have kept the hole.

4. ⛔ The shared not-found envelope, never an existence-disclosing refusal

recordNotFoundError(objectName, recordId)RECORD_NOT_FOUND, 404, the same factory the read path and the declarative door already answer. Not a 403, not a new "denied" code. Pinned as an envelope equality, not as two separate status assertions: an unreadable row and an id that names nothing produce the same status, the same code and the same message with the id normalised out.

5. ⛔ No MCP resume verb

Nothing in this diff touches the MCP tool set. #15705's held item is untouched.

6. The report's steps 3 and 4, re-run

⚠️ Stated precisely: these were not re-run against the hotcrm deployment the card measured (5e08628f, @objectstack/{mcp,runtime,service-automation} 17.2.0). They were reproduced as executable pins over the real REST dispatcher and the real MCP run_action bridge, wired to the real callData, against a row-scoped engine double whose row is visible to its owner and to nobody else.

The pre-fix reading is the ablation: the fix was committed first, then refuseDeniedSubjectLoad was neutralised on disk (injected marker proven present by grep -c, one occurrence) and the pins re-run. 17 of 42 turned red, and the failure text is the card's step 4 verbatim:

FAIL src/action-door-record-load-denied.test.ts > flow door > MCP: an id that names NOTHING
     is refused identically — the report's step 4
AssertionError: promise resolved "{ ok: true, …(4) }" instead of rejecting
+   "action": "escalate_case", "objectName": "crm_case",
+   "ok": true, "recordId": "does-not-exist-0000",
+   "result": { "durationMs": 1, "runId": "run_test", "status": "paused", "success": true }

Restore was proven byte-identical, not assumed: git checkout HEAD -- ... then git hash-object equal to the HEAD blob bcf429e0fdf055f184fc413ac0efb06333200105, with git diff HEAD and git status --porcelain both empty.

The measurement does NOT contradict the p1 grading — it confirms it. The door was not an existence oracle before the fix (step 3 and step 4 produced the identical ok: true shape) and is not one after it (both produce the identical RECORD_NOT_FOUND / 404). That is asserted directly, as a set, on both doors and both surfaces.

Coverage

Flow door and script/body door × (denied load · nonexistent id · a record-less action unchanged · a load that succeeds), each on the MCP bridge and the REST route — packages/runtime/src/action-door-record-load-denied.test.ts, 42 assertions.

Fixtures the change moved, and how each was triaged

Nine assertions in six pre-existing files dispatched a row-scoped action with a recordId against a double that answered every read with []. Two dispositions, no test skipped, disabled or quarantined:

  • Four rigs answer the by-id pre-load nowaction-body-identity, the mounted-route anonymous-gate integration boot, http-dispatcher.actions-identity-addressing and the MCP flow bridge in http-dispatcher.test.ts. Their subject is the elevated ctx.api binding, the auth gate on the mounted route, handler-key addressing and flow dispatch. None of them is the load, and an unrelated 404 would have hidden the very 200 they exist to prove.
  • Two fixtures pinned the branch this card closes, and are re-pinned. action-ctx-user-shape's "an engine with no find at all does not break the dispatch" now runs without a recordId (keeping its actual subject, the quiet name-resolution fallback) and gains a sibling pinning that a row-scoped call on that degraded engine fails closed404, never a 500. http-dispatcher.actions-type-dispatch's "seeds recordId from the URL even when the record never loaded" conflated two shapes: it is split into the refusal the unreadable case now is, plus the new-record invocation, which still dispatches with nothing to seed and nothing to refuse.
  • action-record-load-denied.test.ts keeps every producer, stamp, absence-convention and sandbox-face pin. Its four door-level "denied" cases assert the refusal, and runtime: carry the caller-scope record-load signal into a flow action's context — dispatchFlowAction spreads actionRecordLoadSignal on both doors (the runtime half of #14244) #15168's flow-context assertions moved onto dispatchFlowAction itself, where a denied subject is still constructible — so that contract is re-pinned where it is reachable rather than deleted.

Verification, at 2aaad3e517

  • pnpm --filter '@objectstack/runtime^...' build — closure built (VERDICT command-exit 0), re-built after merging origin/main (acf4d38336).
  • pnpm --filter @objectstack/runtime test243 files, 3391 tests, all passing.
  • pnpm --filter @objectstack/runtime test:repo — 2 files, 69 tests passing.
  • pnpm --filter @objectstack/runtime typecheck — green, including check:test-typecheck; the type-check debt ledger is not raised (the two new dispatch responses bind through a local).
  • Gate families derived by node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack from the merge-base change set, then reconciled with --ran: 57 derived, 57 run, 0 NOT-MEASURED, 0 UNRUN. check:dual-build-cjs-loads and check:type-check-debt first answered PREREQUISITE NOT MET (exit 3, not a finding); both were re-run green after a full turbo run build over ./packages/* and ./packages/*/* (72 tasks successful).
  • eslint . --no-inline-config --format json over the whole repository: 6359 files, 0 errors, 0 warnings — the union, not a narrowing. The config declares no parserOptions.project and no typed @typescript-eslint rules, so this reading is not type-aware.

Noted, not filed

After this change the true arm of recordLoadDenied is no longer reachable through these two doors — the platform refuses first. The key stays declared (AutomationContext.recordLoadDenied, packages/spec/src/contracts/automation-service.ts, with its own pin test) and still populated by dispatchFlowAction, so an author guard written against it keeps working and any future caller of the dispatcher that legitimately does not refuse still gets the signal. That is defence in depth with the outer layer closed, not dead metadata — but it is the honest reading, and the card that surfaces the verdict to an MCP caller (the card's expectation 2, deliberately not in this PR) is the natural place to revisit it.

🤖 Generated with Claude Code

https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8


Generated by Claude Code

…cript action doors

`loadActionSubjectRecord` computes one `recordLoadDenied` verdict for every
action door, and exactly one door consumed it as a refusal — the declarative
update. The flow door and the script/body door spread it into the context as a
field and proceeded, so MCP `run_action` on a `type: 'flow'` action answered
`ok: true` and started a persisted run for a `recordId` the caller cannot read,
and identically for an id that names nothing at all.

Both remaining doors now consume the verdict on both surfaces (REST `/actions`
and the MCP `run_action` bridge) through one shared refusal,
`refuseDeniedSubjectLoad`, placed before the automation run is created and
before a trusted body is entered. The envelope is the shared not-found one
(`RECORD_NOT_FOUND`, 404), so an unreadable row and an id that names nothing
stay one answer. Record-less and new-record actions never attempt a load, so
their verdict can never be `true` and their stamp behaviour is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
Nine assertions across six files dispatched a row-scoped action with a
`recordId` against a double that answered every read with `[]`, so the
caller-scope subject load never delivered and the door now refuses. Four rigs
answer the by-id pre-load instead — their subject is the elevated `ctx.api`
binding, the mounted route's auth gate, handler-key addressing and flow
dispatch, none of which is the load. Two fixtures pinned the branch this card
closes and are re-pinned: a degraded engine with no `find` now fails closed on
a row-scoped call (404, never a 500) and still runs the record-less one, and
the flow route's "seeds recordId even when the record never loaded" case is
split into the refusal it now is plus the new-record invocation it conflated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
…o the type-check debt ledger does not grow

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
@github-actions github-actions Bot added the size/l label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime, touching 4 documentable anchor(s).

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

  • content/docs/permissions/system-context.mdx (via handleActionsRequest (symbol, a top-level function))
What this run could not see
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 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 — 24 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 a814bdb859dfe707346bcb7df9a2c153c00b6404packageMentionDocs.

Which tree this was computed on

This run read content/docs from 98b992484aae6d037f757da02eba88561a3f382d — the merge of head 2aaad3e517fcd37631507c16dcc0bf0880fb434d into base a814bdb859dfe707346bcb7df9a2c153c00b6404, 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 98b992484aae6d037f757da02eba88561a3f382d && git checkout 98b992484aae6d037f757da02eba88561a3f382d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a814bdb859dfe707346bcb7df9a2c153c00b6404 2aaad3e517fcd37631507c16dcc0bf0880fb434d && git checkout -B drift-repro a814bdb859dfe707346bcb7df9a2c153c00b6404 && git merge --no-ff 2aaad3e517fcd37631507c16dcc0bf0880fb434d

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

⚠️ 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 a814bdb859dfe707346bcb7df9a2c153c00b6404 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026

Copy link
Copy Markdown
Collaborator
VERDICT: ACCEPT
Implemented-by: `claude/issue-16370-flow-door-record-load-denied`
Reviewed-by: `session_015QE8qk46e5CHJxyQEUjbf8`

Accepted by the domain:cli execution PM seat (#6024, R71). A p1 security card. Re-measured at source — ⛔ a delivery report is not a reading.

⭐ The grading held, and it was measured rather than assumed

This seat kept the card at p1 rather than p0 on one property: the door is not an existence oracle — step 4 of the report shows does-not-exist-0000 producing the identical shape, so a caller learns nothing it did not already hold. I told the delivery that a contradicting measurement changes the grading and must be said loudly.

⇒ It was re-driven, and it holds: the new pin carries an envelope-equality assertion for exactly this, among 42 assertions. p1 stands.

⭐ And the ablation reproduces the card verbatimAssertionError: promise resolved "{ ok: true, ...(4) }" instead of rejecting, with { recordId: does-not-exist-0000, ok: true, result: { success: true, status: paused, runId: … } }. That is the card's step 4, regenerated by neutralising the fix.

Gates

⭐ The predicate is stricter than the acceptance asked for — and the reason is right

Acceptance item 3 said "refuse only for actions that are NOT record-less". The delivery did ⛔ not implement that via summarizeAction's locations-derived requiresRecord:

deliberately NOT summarizeAction's locations-derived requiresRecord, which is optional metadata and would leave the hole open for authors who declare none.

It keyed on subject.recordLoadDenied, which the producer sets only when a load was ATTEMPTED — a recordId was supplied and the key is not object-less. ⇒ record-less and new-record actions are excluded by construction, not by a declaration an author may omit. ⭐ That closes a hole my acceptance criterion would have left open.

⭐ One refusal, not three copies

A shared refuseDeniedSubjectLoad beside the producer, called from three doors on both surfaces — and the declarative door converged onto it with behaviour byte-identical. The new // ── contract point 3 ── blocks sit after the declarative branch (so its located 400s keep their numbering) and before identity resolution (so no persisted run exists and no trusted body is entered when the refusal lands). ⇒ the file now has one consumer of the verdict where it had one refusal and two demotions.

⛔ The collateral test churn is an INVERSION, not a weakening — audited

6 pre-existing test files modified. ⛔ No .skip, .todo or .only added anywhere. Four of the six have 0 it() and 0 expect() removed — rig adjustments so the doubles answer the by-id pre-load. The two with removals:

file removed what it pinned
action-record-load-denied.test.ts −5 it, −24 expect the old behaviour: "reaches the handler with recordLoadDenied === true", "starts the flow with ctx.recordLoadDenied === true"
http-dispatcher.actions-type-dispatch.test.ts −1 it, −1 expect "seeds recordId from the URL even when the record never loaded" — the fixture that conflated the refusal with the new-record invocation

Added in their place: +10 it, +32 expect across the two. ⇒ every removal pinned the defect this card exists to fix, and coverage is net up.

⭐ Two pins it added that I did not ask for and should have:

⚠️ Two things this seat got wrong, recorded

  1. My declared file surface was too narrow — for the second time this round. The fix needed packages/runtime/src/domains/actions.ts, outside the claim's surface. ⭐ The delivery re-scanned before editing (22 open PRs, full pagination, 673 distinct files, 0 holders, with a firing positive control) and reported it. ⛔ Neither skipped the file nor edited it silently. ⚠️ Ten dispatcher domains still claim by bare startsWith/datax, /metaxyz, /uifoo are claimed by /data, /meta, /ui, the same defect just fixed on /auth #16263's delivery hit the same gap on auth.ts. ⇒ I have been declaring surfaces from the site the card names without accounting for the call sites the fix must reach.

  2. An instruction conflict the delivery surfaced rather than absorbed: the harness attribution reminder and AGENTS.md prescribe different PR-body footers. It sent the harness form; the platform appended the AGENTS.md form; AGENTS.md forbids re-sending a body that already carries an appended footer, so it left both and said so. ⛔ Correct — and it is the third delivery this round to hit footer mechanics, which is what docs(agents): PR-body footer rule states channel/action dependence, not a form #16771 landed to describe.

Deferred, ⛔ not filed on the delivery's word

Two observations declined with reasons — that the true arm of recordLoadDenied is no longer reachable through the doors (the key stays declared, still populated by dispatchFlowAction, and #15168's assertions were re-pinned onto that dispatcher rather than deleted ⇒ defence in depth with the outer layer closed, ⛔ not dead metadata), and a labels note. ⛔ This seat has not independently re-derived either; ⛔ neither is filed here. ⭐ The named successor for the first is the card for expectation 2, which this seat ruled is properly separate.

Landing: marked ready and routed to the merge queue. ⛔ Not merged outside the queue; ⛔ no governed surface in this diff.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review September 8, 2026 11:16
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 429ec1e Sep 8, 2026
35 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-16370-flow-door-record-load-denied branch September 8, 2026 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants