fix(rest): serve a sandboxed hook's own refusal sentence on the bulk write and analytics routes, not the QuickJS debug wrapper

[os-zhuang]

Fixes#11588

What the client used to get

A hook's throw new Error('删除被阻断…') is a deliberate business rule, and classifyDataError's sandbox unwrap door exists precisely so the end user sees only that sentence — the <kind> '<name>' threw: <msg> prefix "belongs in server logs", in the door's own words. Seven routes never reached the door. Reproduced against the real route handlers in process, on main:

route	before	after
PATCH /api/v1/data/:obj/:id	Opportunity is closed.	unchanged
POST …/batch · …/createMany · …/updateMany · …/deleteMany · …/:id/clone	hook 'guard' threw: Error: Opportunity is closed.	Opportunity is closed.
POST /api/v1/analytics/dataset/query	hook 'guard' threw: Error: locked	locked

⛔ The fork — measured, and the answer is NOT a reorder

The card asked whether moving the unwrap above resolveErrorResponse's declared-status passthrough is the repair, and flagged that passthrough as load-bearing for #5437/#5582. I read the docblock first and did not reorder anything. Its own argument rules it out, verbatim:

Sanitising HERE rather than by falling through to mapDataError is the point: mapDataError derives a status from the message TEXT, so handing it a declared 5xx re-labels the fault as something else entirely — the overlay-delete 500 comes back as 404 OBJECT_NOT_FOUND … both of which then read as expected statuses and stop being logged at all.

So the passthrough stays exactly where it is and keeps deciding the status. Only the sentence its 4xx arm reads for the caller changes. The 5xx arm is untouched: a sandbox refusal declaring a 5xx still answers INTERNAL_ERROR_MESSAGE, pinned on both spellings.

What this restores is an invariant the same docblock already asserts. Its #7525 paragraph says an error declaring statusCode instead falls to mapDataError, "So the two doors already agree on the wire answer." For a sandbox refusal that sentence was false — statusCode fell through and was unwrapped, status was answered from the wrapper — which is exactly the two-spellings asymmetry the card named as the real defect. The doors agree again, pinned door-to-door across the whole 4xx band (§4) rather than restated in a comment.

That also settles the PM's option (2): normalising the spellings at the gate would have widened the passthrough (routing statusCode errors into an arm that adds no object and truncates differently), and #7525 argues the status-only read there is deliberate. Fixing the message closes the asymmetry the card named without touching that gate.

The change

• packages/rest/src/error-response.ts — new sandboxBusinessMessage(error): the unwrap door's own two conditions in the door's own order (a non-empty string .innerMessage; not an isScriptFaultMessage crash), named once. The 4xx passthrough arm reads it. It is a read of a field the sandbox populated, never a pattern-strip of .message — see the controls below.
• packages/rest/src/rest-server.ts — /analytics/dataset/query builds its { code, message } envelope by hand and touches neither door, so it needed the same read at its own boundary. It imports the predicate rather than re-deriving it. Both of its client emissions are covered. Deliberately unchanged: logError still receives the whole error, and looksLikeInternalErrorLeak still reads the raw text (feeding it the unwrapped text could only make it withhold less).

POST …/import and GET …/export exit through handleRouteError like the bulk routes, so they are repaired by the same change — measured, not assumed. The card said these were never exercised; they share the branch.

Anti-vacuity

Predictions recorded before running, in the test file header, left as written.

• §1–§7 predicted 11 red, measured 12 — Tests 12 failed | 11 passed (23). The miss is recorded rather than fitted: "a declared userMessage still rides" was drafted as a §5 positive control and predicted green, but its fixture is a sandboxed refusal, so its error assertion reads the defect like §1 does. It is not a control and never was. Moved to §6 and the claim corrected; the prediction stands as written.
• §8 predicted 2 red of 4, measured exactly 2 — Tests 14 failed | 13 passed (27) for the full file against the merged base.
• Mutation proven on disk each leg by grepping the injected and the removed text in both files (injected 1/1 → 0/0, removed 0/0 → 1/2), never an editor's exit code. Carried trap … EXIT INT TERM, and the restore was verified by re-reading both files from the repository root plus an empty git status --porcelain — not by trusting the trap.
• No rebuild between legs, and the reason is load-bearing: every symbol under test is reached by a relative import inside this package, so vitest transforms it from source and no dist/ sits between the mutation and the assertion.

⭐ Positive controls (§5, §8d — green on both sides, and the reason the fix is a read):

• a plain 4xx refusal comes through verbatim;
• a plain error whose own text is "hook 'guard' threw: Error: locked" is NOT rewritten — no .innerMessage, so nothing about it came from the sandbox;
• innerMessage of undefined / null / '' / 42 / {} all fall back to .message.

A fix that stripped the wrapper by pattern would satisfy every other assertion in the file and redden exactly here.

Also pinned green both sides: the declared-5xx withhold (both spellings), #5423 truncation at 500 chars, the 'Request failed' degrade, the empty-string message served as itself, OBJECT_NOT_FOUND still bypassing the passthrough, and the dogfood-pinned undeclared-status 400.

Measured and deliberately NOT repaired

Filed as their own issues rather than widened into this PR:

• rest: the record-share routes classify refusals by message.startsWith(CODE) and interpolate error.message into a hand-built 500 — a declared status/code is ignored and the sandbox wrapper reaches the client #11683 — the record-share routes are a third branch again: respondSharingError classifies by message.startsWith(CODE) and its fallback interpolates error.message into a hand-built 500, ignoring a declared status/code entirely.
• rest: an UNDECLARED hook refusal answers 500 on /analytics/dataset/query where the same refusal answers 400 on /data — the route's fallback arm treats a business refusal as a server fault #11684 — on the analytics route an undeclared hook refusal answers 500 where /data answers 400. Only the sentence was corrected; the status disagreement is a contract call. Pinned as measured in §8b so it is not rediscovered as new.
• A sandbox crash carrying a declared 4xx status still answers that status and the wrapper, where mapDataError sanitises it to a 500. sandboxBusinessMessage declines a script fault (A raw TypeError: not a function leaks in the 400 envelope for {"title": 12345} — no code, no fields[] #7543), so this arm's answer is byte-identical to before. Closing it means moving the status this arm decided — the contract question this card was fenced from. Pinned as measured in §7.

Clause-② — measured, not inherited

No. The message text on a served error envelope is user-visible, and that is why the changeset argues its level explicitly. But the accept set does not move: no request is newly accepted or newly refused, the status is unchanged on every arm, the code is unchanged, no field is added or removed, and the public TypeScript surface gains only an internal predicate export consumed inside this package. needs:contract-review is not applied.

Changeset — patch, argued rather than defaulted

Nothing about the envelope's contract moves. The delta is that one string loses a debug prefix this boundary already declares must never be on the wire, and that the single-row routes never emitted — so no client could have been reading it uniformly. Keying on it would mean substring-matching prose that is localised and deliberately reworded, the practice ADR-0112's code vocabulary exists to remove. Full argument in .changeset/bulk-write-refusal-message-parity.md.

Verification — all at f93aa39e, the final commit

• pnpm --filter @objectstack/rest test → RUN v4.1.10 …/packages/rest, Test Files 143 passed (143), Tests 2301 passed (2301)
• pnpm --filter @objectstack/rest typecheck → tsc --noEmit, clean
• new file individually → RUN v4.1.10 …/packages/rest, Test Files 1 passed (1), Tests 27 passed (27)
• Gate union derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack and re-derived on the final diff (4 paths; the re-derivation added check:authz-resolver, which the first derivation could not name because rest-server.ts was not yet in the change set). All 17 path-matched families plus the 6 convention-triggered ones run green, each quoted from its own verdict line:
  • check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 294.8s, 1898 raw tsc error(s) total, none above its recorded number.
  • check-nul-bytes: OK (scanned 6523 text file(s) … no raw ASCII control bytes).
  • ✓ check:authz-resolver: single shared authorization resolver intact; both entry points delegate.
  • check:route-envelope → ✓ Express-style response modules — 4 module(s) discovered and audited …, 2 conformant, 2 ratcheted, 0 exempt, 0 vendor-wire (the two ratcheted are query-allowlist.ts / query-multiplicity.ts, neither touched here)
  • check:dispatcher-error-vocabulary → door typing (#9098): packages/rest/src/error-response.ts checked for the typed author-side responder …
  • closure built as lint.yml does before the ratchet: Tasks: 70 successful, 70 total

One gate went red and was fixed, not waved through.check:type-check-debt reported @objectstack/rest: TEST_DEBT records 155 raw tsc error(s), tsc --noEmit now reports 156 (+1) — my analytics harness passed a hand-built request literal that is not assignable to IHttpRequest. Repaired at the source (typed any, matching the sibling route helpers in the same file) rather than by raising a shrink-only ratchet; re-measured back to exactly 155.

main was merged in mid-task after PR #11641 released the hold on rest-server.ts; every reading above was re-run against the merged base rather than carried across it.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see

• 1 changed file(s) yielded no anchor (packages/rest/src/rest-server.ts) — pages documenting those are invisible to this run
• the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 13 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 88b9d749a1eb76b53966b0081006d2c5bccb341d → packageMentionDocs.

Which tree this was computed on

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

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

[os-zhuang]

ACCEPT — PM seat review (domain:cli, session 019siH5jDmk5hrayvfyojUqR, round R33)

CI at f93aa39e: 20 of 28 complete, nothing red; Build Core, Test Core (1/6),
Temporal Conformance, Dogfood Verify CLI, Dogfood Regression Gate (3/3), Lint & Repo Gates
and two Type Check jobs still running. Arming when they land — ⛔ not on a partial reading.

The fork — resolved by the source, and I read the source rather than your account of it

The dispatch carried a hard stop-and-report: if reordering resolveErrorResponse's passthrough is
the only correct fix, that is a contract question and it goes to the maintainer. It did not need to
fire, and the reason is stronger than "the docblock says so". packages/rest/src/error-response.ts:1451:

Sanitising HERE rather than by falling through to mapDataError is the point: mapDataError
derives a status from the message TEXT, so handing it a declared 5xx re-labels the fault as
something else entirely — the overlay-delete 500 comes back as 404 OBJECT_NOT_FOUND ("no such
table" trips the unknown-object heuristic) and the atomic batch's 501 NOT_IMPLEMENTED as
404 Object '<name>' is not registered …

So a reorder does not merely violate a stated preference; it is measurably wrong — it would
re-label declared faults through a text heuristic. Leaving the arm in place and changing only the
sentence it reads is the fix, which is PM shape (1).

⭐ And the framing you found is the one that makes this a repair rather than a copy edit

error-response.ts:1408, verified verbatim:

So the two doors already agree on the wire answer, and this one is not duplicating the
two-spelling read to say so.

That comment declines to do something on the strength of an invariant it asserts — and the card
measured the invariant false for exactly this producer: the status spelling ships the wrapper,
the statusCode spelling falls through and gets unwrapped. So this is declared ≠ enforced inside a
justification, and the fix makes the comment true instead of making it a lie with a test around it.

It is also why shape (2) is the wrong instrument, and your rejection of it is right for a reason
worth restating: normalising the spellings would route statusCode errors into an arm that adds no
object and truncates differently — it would falsify the comment's premise in a different
direction rather than satisfying it.

The fence lift — handled the way a base change has to be

The analytics half was correctly reported blocked while rest-server.ts was held by #11641, and
repaired after that merged, importing the same sandboxBusinessMessage rather than re-deriving the
unwrap. ⭐ The part that matters: "Every reading was re-run against the merged base rather than
carried across it." Carrying greens across a base change is how a measurement becomes a claim about
a tree nobody ran — and the gate union was re-derived, which is what surfaced
check:authz-resolver (unnameable before rest-server.ts entered the change set).

The wrong prediction, and the red that was fixed at the source

Predicted 11 red, measured 12 — reported, not fitted, with the cause named: the userMessage
case was drafted as an S5 positive control but its fixture is a sandboxed refusal, so it reads the
defect like S1. It was never a control; it was re-shelved with the wrong prediction left in the file
header. That is the standard.

And check:type-check-debt went 155 → 156 (+1) from a hand-built request literal in the new
analytics harness. Repaired at the source — typed to match the sibling helpers — and re-measured
back to exactly 155. ⛔ Not by raising a shrink-only ratchet, which is the tempting move and is on the
manual floor.

Scope — wider than the card's table, and correctly so

POST …/import and GET …/export exit through handleRouteError like the bulk routes, so they
shared the defect and are repaired by the same change. The card recorded them as never exercised;
measured, they share the branch. ⛔ This is not scope creep: a fix that repaired six of eight exits on
one branch would have been the incomplete one, and the card's "not established" was an absence of
measurement, not a boundary.

Findings — filed, and one deliberately left

• rest: the record-share routes classify refusals by message.startsWith(CODE) and interpolate error.message into a hand-built 500 — a declared status/code is ignored and the sandbox wrapper reaches the client #11683 — the record-share routes are a third branch (respondSharingError classifies by
  message.startsWith(CODE) and its fallback hand-builds a 500), so a declared status/code is ignored
  entirely. Same family, different mechanism, correctly not folded.
• rest: an UNDECLARED hook refusal answers 500 on /analytics/dataset/query where the same refusal answers 400 on /data — the route's fallback arm treats a business refusal as a server fault #11684 — on /analytics/dataset/query an undeclared refusal answers 500 where the same throw
  answers 400 on /data. Only the sentence was corrected here; the status disagreement is a contract
  call. ⭐ And it is pinned as measured in S8b, so the next reader finds it recorded rather than
  rediscovering it as new — that is the right way to leave a known divergence behind.
• S7, deliberately not filed: a sandbox crash carrying a declared 4xx still answers that status
  and the wrapper. Closing it means moving the status the passthrough decided — the exact contract
  question this card was fenced from. Pinned as measured and left.

---

Generated by Claude Code

[os-zhuang]

Armed at 12:21Z. CI at f93aa39e: 31 of 31 complete, no failure (two skipped). Fresh label read
before flipping: documentation, size/l, tests, tooling — no needs:contract-review, and no
skip-changeset, so a real changeset ships.

The one thing your report left unstated — checked, and patch is right

The dispatch asked you to argue the bump level explicitly rather than default it, and the report
didn't. .changeset/bulk-write-refusal-message-parity.md says patch, and I am accepting it on
a precedent set in this repo today rather than on my own preference: the contract review on #11580
ruled, four hours ago, that "patch bump fits a fix that makes declared configuration take effect."

This is the same shape one level over — the door's own docblock already says the user must see only
the business sentence and that the <kind> '<name>' threw: prefix "belongs in server logs". Making
that true on six more routes is declared = enforced, not a contract move.

⚠️ For the record, since it is the one reading that could have gone the other way: a consumer
string-matching on hook 'x' threw:does break. That is not a supported contract — it is
matching on text the code explicitly designates as log-only — but it is the fact a release note
should carry, and the changeset's prose does carry it.

⛔ Next time, say the level and why. Not because this one was wrong, but because a level nobody argued
is a level nobody checked.

---

Generated by Claude Code