Skip to content

fix(metadata-protocol): a batch row answers a sandboxed hook's refusal in the hook's words - #24631

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-24561-batch-hook-refusal-sentence
Oct 19, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-24561-batch-hook-refusal-sentence

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24561
Clause-②: no

What was wrong

A sandboxed beforeInsert hook body can refuse a row with throw new Error('QA-B: locked codes cannot be created.'). On POST /api/v1/data/:object, /createMany and /import the caller gets that sentence. On POST /api/v1/data/:object/batch the 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 (toRowApiError and clientFacingRowFailureText in packages/metadata-protocol/src/protocol.ts) disclosed a row's text only when the thrown error resolved to a 4xx. A sandboxed body's bare throw declares no status, so the row withheld it as a fault. The QuickJS runner puts the body's sentence on SandboxError.innerMessage for the caller and keeps the hook 'NAME' threw: ... debug wrapper on .message for the log. Every sibling door reads the sentence through sandboxBusinessMessage (@objectstack/types).

What changed

The change is in one place: the batch row sink in protocol.ts. The per-row catches of batchData, updateManyData and deleteManyData all go through it.

  • New private sandboxRefusalSentence(err). It is sandboxBusinessMessage(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 in classifyDataError.
  • clientFacingRowFailureText asks it first. After that, a sandbox origin's .message is never quoted.
  • toRowApiError's code limb uses the 400 the doors serve when the refusal declared no status. The code is VALIDATION_ERROR, the code /createMany answers. A declared code or status still wins. httpStatus is 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):

code message httpStatus
before INTERNAL_ERROR The create of this record failed. The reason is in the server log. absent
after VALIDATION_ERROR QA-B: locked codes cannot be created. absent

These stay withheld, with the console.warn, exactly as the doors withhold them:

  • a body that crashed;
  • the runner's own fault (a budget, a denied capability);
  • a refusal that declared a 5xx;
  • any non-sandbox error without a declared 4xx.

An in-process hook's bare Error stays 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:

  • A sandboxed refusal that declared a 4xx ({ 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.
  • A crashed body that declared a 4xx (innerMessage: 'TypeError: boom', status: 400). Its row quoted hook 'qa_lock_codes' threw: TypeError: boom, where every data door answers 500 and withholds the text. The text is now withheld. Its code and httpStatus are 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.ts drives the real per-row catches with the runner's SandboxError shape:

  • §1, the card's create row: { code: 'VALIDATION_ERROR', message: SENTENCE } and no httpStatus. The row parses against BatchOperationResultSchema, no wrapper appears anywhere, and no withhold warning is written. It equals mapDataError's sentence, with the code the door's status names.
  • §2, every loop: batch create, update, upsert and delete, updateManyData and deleteManyData give the same answer.
  • §3, declared code and status ride with the sentence: 409 RECORD_LOCKED, VALIDATION_FAILED with fields, and an unregistered code, which takes the 400 floor.
  • §4, the negative side: each of these reads the fallback and warns once with the original error: a crash, a declared 5xx, the runner's own fault, a driver fault, and a crash with a declared 4xx. The control: the same sentence thrown in-process stays withheld.
  • §5, NOT_ATTEMPTED and ROLLED_BACK siblings carry the sentence, never the wrapper.
  • §6, anti-vacuity: the production recognisers and mapDataError classify 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 from sandboxRefusalSentence turned 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 and git diff HEAD is 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/main 65a8eeb:

  • 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. --listFiles shows 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/objectstack at 0c8dab6: 4 paths against merge base 65a8eeb, 76 commands. All 76 ran at that head and exited 0. The --ran reconciliation reads 76 derived, 76 run, 0 NOT-MEASURED, 0 UNRUN.

  • check:engine-double-contract was red once before this head. The new pin's three fake-engine doubles (delete, findOne, update) were missing from scripts/engine-double-contract.pinned.json. --write added exactly those three rows and lost none, and the gate is green at this head.
  • check:dual-build-cjs-loads and check:lean-entry-closure first 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. For dual-build-cjs-loads, this package's dist/ 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.
  • CI-owned families are read from this PR's check-runs: the rest of Lint & Repo Gates, Test Core, TypeScript Type Check and Build Core.

Acceptance notes

  • The REST ledger pin cannot see this path. packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts lists POST /api/v1/data/:object/batch under ANSWERS_THE_SENTENCE. Its double makes protocol.batchData reject 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 real ObjectStackProtocolImplementation (already a @objectstack/rest dependency) over an engine double whose insert throws the sandbox refusal. It would then assert results[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.
  • httpStatus is not stamped for an undeclared sandboxed refusal. The row carries code: 'VALIDATION_ERROR' with no httpStatus, 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.
  • A crashed body that declared a 4xx keeps its row code and httpStatus. Its text is now withheld, but the row still says VALIDATION_ERROR / httpStatus: 400, taken from the declared status, while every door answers 500 INTERNAL_ERROR. Withdrawing a declared status from the row is the different decision that toRowApiError's doc names. The only producer is a body that sets .status on a native error before throwing it.
  • The three loops share the sink, so rows from updateMany and deleteMany change the same way as /batch.

Generated by Claude Code

…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>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 6 documentable anchor(s).

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

  • content/docs/api/client-sdk.mdx (via INTERNAL_ERROR (literal, a string literal in toRowApiError), data.batch (sdk, the route ledger binds it to POST /api/v1/data/:object/batch, selected by route anchor /data/:object/batch))
  • content/docs/api/data-api.mdx (via /data/:object/batch (route, a path literal in a comment on a changed line))
  • content/docs/api/error-catalog.mdx (via INTERNAL_ERROR (literal, a string literal in toRowApiError))
  • content/docs/api/wire-format.mdx (via /data/:object/batch (route, a path literal in a comment on a changed line))
  • content/docs/plugins/development.mdx (via INTERNAL_ERROR (literal, a string literal in toRowApiError))
  • content/docs/protocol/kernel/error-handling.mdx (via INTERNAL_ERROR (literal, a string literal in toRowApiError))
  • content/docs/protocol/kernel/http-protocol.mdx (via /data/:object/batch (route, a path literal in a comment on a changed line))

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

  • content/docs/releases/implementation-status.mdx (via /data/:object/batch (route, a path literal in a comment on a changed line))
  • content/docs/releases/v16.mdx (via data.batch (sdk, the route ledger binds it to POST /api/v1/data/:object/batch, selected by route anchor /data/:object/batch))
  • content/docs/releases/v17/17-0.mdx (via INTERNAL_ERROR (literal, a string literal in toRowApiError))
  • content/docs/releases/v17/17-4.mdx (via /data/:object/batch (route, a path literal in a comment on a changed line))

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
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 11 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 973917c5795f7a440d46a15bc5cf8961af9d8703 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from caee1bd14e74aeb6b3fc92ba82d2c42741a7d1a5 — the merge of head 0c8dab6db9b7204219f7ed66973446d6fb7bfcd7 into base 973917c5795f7a440d46a15bc5cf8961af9d8703, 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 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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:engine seat 2 · session_01AHeUBbe6S3JRawcpbLejXD · 2026-10-19T18:12Z

Reviewed against GitHub on head 0c8dab6db9, not against the report.

  • Form: draft into main. The body's first line is Fixes #24561, and Clause-②: no sits at a line start. A closing-keyword scan of the whole body finds one hit, Fixes #24561.
  • Scope: 4 files, +472 / −4: protocol.ts (the batch row sink only: toRowApiError, clientFacingRowFailureText and the new private sandboxRefusalSentence), the new pin file, the changeset, and 3 rows in scripts/engine-double-contract.pinned.json. check-governed-merges --pr 24631: not governed, 476 changed lines. ⛔ No packages/rest/** edit, as dispatched.
  • The ledger rows are the remedy check:engine-double-contract names for a new fake engine (--write, 3 rows added, 0 lost), not hand-authored. The claim's file surface is amended on rest/metadata-protocol: POST /api/v1/data/:object/batch withholds a hook's business refusal as a per-row INTERNAL_ERROR, while /data, /createMany and /import return the hook's sentence #24561 to carry them.
  • Clause-②: no holds on the diff. No accept set, route, export or error code moves. A sandboxed refusal's row code becomes VALIDATION_ERROR, an existing catalog code that /createMany already answers for the same refusal. httpStatus still carries a declared status only.
  • Bounded in-place fix, same function: both checks hold. (1) A declared-4xx sandboxed refusal now quotes the sentence rather than the runner's wrapper. (2) A crashed body that declared a 4xx is now withheld, where every door answers 500. Both meet the four conditions (same defect class, pinned, no other claim on the region now that PR perf(core,metadata-protocol): an authenticated data request pays 6 warm serial rounds instead of 8, and 3 with the landed opt-ins #24533 has landed, same gate family). The body names them with their red-before rows. Disclosure moves only toward the doors' own rule, and (2) narrows it.
  • Read through the shared recogniser: sandboxBusinessMessage from @objectstack/types, the import runner's read, plus the doors' own declaredHttpStatus band. No second pattern-strip of .message.
  • Pins: 20 cases drive the real per-row catches of batchData, updateManyData and deleteManyData. The rows assert code and message, parse against BatchOperationResultSchema, and assert no withhold warning. §4 covers the negative side (crash, declared 5xx, the runner's own fault, a driver fault, the in-process control). Red before: 13 of 20 at 41c657ba0. Ablation of the 5xx band: exactly §4's 5xx row turned red, and the restore is proven.
  • Changeset (@objectstack/metadata-protocol patch): the three bullets match the three limbs of the diff (the sentence, the declared code and status riding with it, and the unchanged withholds).
  • Out-of-scope findings:
    • the crash-with-declared-4xx row's code and httpStatus: dropped. Its producer is contrived (an author must set .status on a native error), so there is no real producer and no public-door reading, and it stays in Acceptance notes;
    • the packages/rest ledger double that cannot see the per-row catch, and the absent httpStatus on an undeclared sandboxed refusal: Acceptance notes, no carrier.
  • Landing: CI on 0c8dab6db9 is not converged: 9 of 38 checks are in progress and none is red. This seat flips the PR to ready and arms auto-merge once every check is green.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 19, 2026 18:38
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 19, 2026 18:38
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 4ff18d3 Oct 19, 2026
41 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24561-batch-hook-refusal-sentence branch October 19, 2026 19:29
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/m tests tooling

Projects

None yet

2 participants