Repository navigation
fix(metadata-protocol): a batch row answers a sandboxed hook's refusal in the hook's words - #24631
Conversation
…w sink (red before the fix) Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude <noreply@anthropic.com>
…l in the hook's words The batch row sink withheld a sandboxed hook body's bare throw as a server fault (INTERNAL_ERROR, "the reason is in the server log"), while /data, /createMany and /import answer the body's sentence as a 400 refusal. The row now reads the sentence through sandboxBusinessMessage first, takes the 400 floor code the doors serve, and never quotes a sandboxed error's .message (the runner's debug wrapper). Crashes, the sandbox's own faults, declared-5xx refusals and non-sandbox faults stay withheld, with the warning. Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude <noreply@anthropic.com>
…rigin only The runner's own faults never declare a status, so the isSandboxOwnFault half of the guard could not be reached. Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude <noreply@anthropic.com>
…ed refusal sentence Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude <noreply@anthropic.com>
… doubles in the pinned ledger Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin caee1bd14e74aeb6b3fc92ba82d2c42741a7d1a5 && git checkout caee1bd14e74aeb6b3fc92ba82d2c42741a7d1a5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 973917c5795f7a440d46a15bc5cf8961af9d8703 0c8dab6db9b7204219f7ed66973446d6fb7bfcd7 && git checkout -B drift-repro 973917c5795f7a440d46a15bc5cf8961af9d8703 && git merge --no-ff 0c8dab6db9b7204219f7ed66973446d6fb7bfcd7
node scripts/docs-audit/affected-docs.mjs --json 973917c5795f7a440d46a15bc5cf8961af9d8703
|
席位核对 · ACCEPT ·
|
Fixes #24561
Clause-②: no
What was wrong
A sandboxed
beforeInserthook body can refuse a row withthrow new Error('QA-B: locked codes cannot be created.'). OnPOST /api/v1/data/:object,/createManyand/importthe caller gets that sentence. OnPOST /api/v1/data/:object/batchthe row said{ code: 'INTERNAL_ERROR', message: 'The create of this record failed. The reason is in the server log.' }, and the server logged the withhold.The batch row sink (
toRowApiErrorandclientFacingRowFailureTextinpackages/metadata-protocol/src/protocol.ts) disclosed a row's text only when the thrown error resolved to a 4xx. A sandboxed body's barethrowdeclares no status, so the row withheld it as a fault. The QuickJS runner puts the body's sentence onSandboxError.innerMessagefor the caller and keeps thehook 'NAME' threw: ...debug wrapper on.messagefor the log. Every sibling door reads the sentence throughsandboxBusinessMessage(@objectstack/types).What changed
The change is in one place: the batch row sink in
protocol.ts. The per-row catches ofbatchData,updateManyDataanddeleteManyDataall go through it.sandboxRefusalSentence(err). It issandboxBusinessMessage(err), the read the import runner makes, and it declines a refusal that declared a 5xx. That band test is the doors' own:declaredHttpStatus, with the declared status below 500, as inclassifyDataError.clientFacingRowFailureTextasks it first. After that, a sandbox origin's.messageis never quoted.toRowApiError's code limb uses the400the doors serve when the refusal declared no status. The code isVALIDATION_ERROR, the code/createManyanswers. A declared code or status still wins.httpStatusis unchanged: it carries only a status the producer declared, so none is stamped here.The row for the card's request (
{"operation":"create","records":[{"data":{"code":"LOCK-9"}}]}, refused by the hook):These stay withheld, with the
console.warn, exactly as the doors withhold them:An in-process hook's bare
Errorstays withheld too, even with the same sentence. The read keys on the producer, not on the text.Bounded in-place fix, same function
Two more rows were measured red before the fix, at the same per-row catch:
{ code: 'RECORD_LOCKED', status: 409 }). Its row quoted.message, which is the wrapper:hook 'qa_lock_codes' threw: Error: QA-B: .... It now quotes the sentence, with the declared code and status.innerMessage: 'TypeError: boom',status: 400). Its row quotedhook 'qa_lock_codes' threw: TypeError: boom, where every data door answers500and withholds the text. The text is now withheld. ItscodeandhttpStatusare unchanged; see Acceptance notes.Both are the same defect class: the row sink's text disagrees with the doors on a sandboxed error. Both sit in the claimed function, are pinned in the same new file, and add no gate.
Tests
The new
packages/metadata-protocol/src/protocol.batch-row-sandbox-refusal.test.tsdrives the real per-row catches with the runner'sSandboxErrorshape:{ code: 'VALIDATION_ERROR', message: SENTENCE }and nohttpStatus. The row parses againstBatchOperationResultSchema, no wrapper appears anywhere, and no withhold warning is written. It equalsmapDataError's sentence, with the code the door's status names.updateManyDataanddeleteManyDatagive the same answer.RECORD_LOCKED,VALIDATION_FAILEDwith fields, and an unregistered code, which takes the 400 floor.mapDataErrorclassify each double as the producer it stands for.Red before the fix. The test was committed first (41c657b) and run against the unfixed
protocol.ts:Tests 13 failed | 7 passed (20). The §1 row was exactly the card's{ code: 'INTERNAL_ERROR', message: 'The create of this record failed. ...' }, and the log printed the withhold line.Ablation (one-shot, through
scripts/ablation-replace.mjs, with the restore proven). Removing the 5xx band fromsandboxRefusalSentenceturned exactly §4's declared-5xx row red:Tests 1 failed | 19 passed (20), with"message": "The ledger service is down."where the fallback was expected. After the restore, the blob equals HEAD andgit diff HEADis empty. This was the one pinned behaviour the red-before run could not prove, because that row was withheld before the fix too.At the final head 0c8dab6, after merging
origin/main65a8eeb:pnpm --filter @objectstack/metadata-protocol exec vitest run: Test Files 249 passed, 3 skipped (252); Tests 20751 passed, 19 skipped.pnpm --filter @objectstack/metadata-protocol typecheck: exit 0.--listFilesshows the new file in the tsc program.pnpm --filter @objectstack/spec check:generated: all 14 artifacts are current.Gates
Derived with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackat 0c8dab6: 4 paths against merge base 65a8eeb, 76 commands. All 76 ran at that head and exited 0. The--ranreconciliation reads76 derived, 76 run, 0 NOT-MEASURED, 0 UNRUN.check:engine-double-contractwas red once before this head. The new pin's three fake-engine doubles (delete,findOne,update) were missing fromscripts/engine-double-contract.pinned.json.--writeadded exactly those three rows and lost none, and the gate is green at this head.check:dual-build-cjs-loadsandcheck:lean-entry-closurefirst answered PREREQUISITE NOT MET (exit 3) in a partly built tree. Both measured green at this head: 111 require entry points across 69 packages load, and the objectql lean entry closure holds at 15 packages. Fordual-build-cjs-loads, this package'sdist/was rebuilt after the merge, but the packages main's merge moved were read from a pre-merge build. CI's fresh build is the reading of record for those.Lint & Repo Gates,Test Core,TypeScript Type CheckandBuild Core.Acceptance notes
packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.tslistsPOST /api/v1/data/:object/batchunder ANSWERS_THE_SENTENCE. Its double makesprotocol.batchDatareject as a whole, so the per-row catch never runs, and the row passes there on the whole-request door's answer. A real-path pin at that layer would need the realObjectStackProtocolImplementation(already a@objectstack/restdependency) over an engine double whoseinsertthrows the sandbox refusal. It would then assertresults[0].errors[0]on the 200 body, not the response status. That file belongs to another lane and is not edited here. The pin in this PR drives the per-row catch directly, one layer closer to the code.httpStatusis not stamped for an undeclared sandboxed refusal. The row carriescode: 'VALIDATION_ERROR'with nohttpStatus, while the doors answer HTTP 400. The limb carries only a status the producer declared, and the dispatch scoped this card to the sentence and the code. Whether a sandbox origin counts as a declared 400 for that limb, the way the validation shape does, is a separate decision.codeandhttpStatus. Its text is now withheld, but the row still saysVALIDATION_ERROR/httpStatus: 400, taken from the declared status, while every door answers500 INTERNAL_ERROR. Withdrawing a declared status from the row is the different decision thattoRowApiError's doc names. The only producer is a body that sets.statuson a native error before throwing it.updateManyanddeleteManychange the same way as/batch.Generated by Claude Code