fix(approvals): refuse loudly when a successful mutation's read-back is org-filtered out

[os-elon]

Fixes#12769

What

ApprovalService read the row a write path had just mutated back through the caller's organization narrowing and asserted the result non-null at ten result sites (request: fresh!) — decideNode (both legs), recall, sendBack (both legs), resubmit, reassign, remind, requestInfo, comment. For an org-less request row, an org-scoped caller's read-back matches nothing, so a call that SUCCEEDED shipped a success envelope whose declared-non-null request was null — HTTP 200 with a null request through the REST pass-through, and the card's measured client crash on reading request.status.

Org-less rows are live by construction: every schedule / time-relative / api trigger run produces one (#10131; #9132 deliberately pinned that behaviour rather than repairing it, and the write-side repair waits on the #8778 ruling). Re-confirmed on this branch's merge base: ten request: fresh! sites (reverse control: readBackRequest spelled 11 times), all sharing one unguarded shape, none pre-guarded.

The fix — the loud-refusal branch, per the dispatch ruling

readBackRequest — the single private helper behind all ten sites — now throws READ_BACK_FAILED: ... when the read-back returns nothing. The message states that the write was recorded and is NOT rolled back, names the request id, and points at the org-scope cause. All ten fresh! assertions are deleted; the helper's return type loses its null arm.

• Not taken — type widening: ApprovalRecallResult and siblings live in packages/spec (enumerated in the published surface manifest), which is out of this lane's file surface — and no spec edit turned out to be needed: the declared non-null request is now always true, because a result that cannot be built is never returned. The spec contract already documents this family's throw-instead-of-malformed-result pattern (the RESUME_FAILED / RESUME_TARGET_LOST precedent on these very methods); the new throw follows it.
• Not touched — the fenced branch: the caller-org narrowing in loadRequest (tenancy wall) is byte-identical.
• On the wire: the new prefix is deliberately NOT added to the REST handleApprovalError mapping (packages/rest is outside this card's declared surface). Unmapped, it reaches the client through each route's existing terminal 500 arm — APPROVAL_RECALL_FAILED and siblings, already ledger-registered codes — with the READ_BACK_FAILED: sentence in the body. Whether the prefix deserves its own mapped row (status + wire code + error-code-ledger entry) is flagged for the contract-review seat in the report.

Tests

New describe block in approval-service.test.ts (6 cases), reusing the file's existing fake engine — no new double, the engine-double ledger is untouched:

• recall / decide / comment against an org-less request from an org-scoped caller: the row IS mutated (pinned via a system-context read-back) AND the call rejects with the READ_BACK_FAILED: prefix. The decide case uses an ordinary org-scoped slot holder — no admin override involved, so the miss is not an override-only corner.
• message contract: names the request id, says the write "was recorded" and is "NOT rolled back".
• two positive controls: an org-less caller (the trigger-context shape) reads back fine; an org-matched caller on an org-stamped request reads back fine.

Reverse verification, from the committed state: with the pre-fix approval-service.ts restored from the merge base (tests kept), exactly the 4 refusal cases fail — the calls RESOLVE with a null request instead of throwing — while both controls and all 292 pre-existing cases stay green. The restore was then verified by hashing the work-tree file against the HEAD blob (match).

Verification

At head b096be1, clean tree:

• pnpm --filter @objectstack/plugin-approvals test — Test Files 34 passed, Tests 632 passed.
• Targeted downstream: REST approval endpoint suites (wire codes + unknown filter) — 2 files, 16 tests passed. The route-layer 500 arm and its registered codes are what an unmapped service throw rides; the radius sweep found no cross-package fixture pinning the old null-read-back behaviour.
• pnpm --filter @objectstack/plugin-approvals typecheck — clean. Measured with tsc --listFiles: the package's tsc programs include approval-service.ts and exclude the test layer, so for the test file this typecheck is NOT a reading; the test layer is covered by the TEST_DEBT re-measure below and at runtime by vitest.
• Gate families derived from the actual changed set (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, 3 paths vs merge base 1a540e8): all 27 matched families plus the convention-triggered ones run locally and green, with two NOT MEASURED locally, named: scripts/pm/check-half-states.mjs (exit 3, PREREQUISITE NOT MET — needs a real GitHub credential this container does not hold; CI's half-state patrol carries it) and scripts/check-test-completeness.mjs (consumes CI's turbo test-log artifact; usage refusal locally — the gate body never ran).
• pnpm check:type-check-debt was the one real local red on the first pass: the new test's .at(-1) raised the frozen TEST_DEBT pile 345 to 346 under its ES2021 re-measure (TS2550). Repaired by spelling the index read; re-measured green — "none above its recorded number".
• Ratchet-family union re-run at head b096be1 on a clean tree: objectql-double-limit, slot-lookup, query-options-erasure, where-matcher, engine-double-contract, i18n, i18n-stale-fill, nul-bytes — all green, "baseline key set verified against 1a540e8: no files added".
• Narrowed eslint measurement: this repo runs one eslint.config.mjs and enables type-aware linting for NO file (the config's own recorded measurement); pnpm exec eslint --no-inline-config --format json over the two changed TS files reports 2 files linted, 0 errors, 0 warnings. With no type-aware linting, this diff cannot move any untouched file's verdict; the repo-wide pnpm lint remains CI's run.
• check-adr-0087-registration green — "this PR adds no declared-breaking changeset". The loud-refusal route changes no published type, so no ADR-0087 registration is due.

Changeset: patch on @objectstack/plugin-approvals.

Landing posture

Clause-② card (accept/reject behaviour change on a published contract's method family): this PR parks as a DRAFT at the contract-review gate by design. The dispatching seat is below the review tier, so the review chain belongs to another seat — please do not flip it ready or arm auto-merge from this lane. If needs:contract-review is applied now that a reviewable diff exists, that is the intended flow.

Related cards, all deliberately left as they are: #12775 waits behind this card in the same file; #9132 (merged) pinned the org-less write side; the #8778 ruling is still pending elsewhere; #10131 is prior history. None of them is affected by this PR's landing.

Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi

🤖 Generated with Claude Code

Generated by Claude Code

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 9 documentable anchor(s).

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

• content/docs/api/client-sdk.mdx(via approvals.getRequest (sdk), getRequest (sdk))
• content/docs/api/plugin-endpoints.mdx(via /approvals/requests/:id (route))
• content/docs/automation/approvals.mdx(via getRequest (sdk), /approvals/requests/:id (route))
• content/docs/automation/flows.mdx(via ApprovalService (symbol), /approvals/requests/:id (route))
• content/docs/kernel/services-checklist.mdx(via /approvals/requests/:id (route))

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

• content/docs/releases/v16.mdx(via getRequest (sdk))
• content/docs/releases/v17.mdx(via getRequest (sdk))

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

• 5 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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; 102 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 — 5 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 596b20a6ac26c94e185f0fb36b7b5f30cb56e7d0 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 21d5a357e086e804de2c03e83de670929377ad6f — the merge of head b096be1ec0ea3ca70d972789bad65d431cb1df2f into base 596b20a6ac26c94e185f0fb36b7b5f30cb56e7d0, 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 21d5a357e086e804de2c03e83de670929377ad6f && git checkout 21d5a357e086e804de2c03e83de670929377ad6f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 596b20a6ac26c94e185f0fb36b7b5f30cb56e7d0 b096be1ec0ea3ca70d972789bad65d431cb1df2f && git checkout -B drift-repro 596b20a6ac26c94e185f0fb36b7b5f30cb56e7d0 && git merge --no-ff b096be1ec0ea3ca70d972789bad65d431cb1df2f
node scripts/docs-audit/affected-docs.mjs --json 596b20a6ac26c94e185f0fb36b7b5f30cb56e7d0

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