Skip to content

fix(rest): hand-built write routes withhold the sandbox's own fault as the data door does (#22885) - #22896

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-22885-rest-sandbox-own-fault-withheld
Oct 11, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-22885-rest-sandbox-own-fault-withheld

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #22885
Clause-②: no

What this does

The QuickJS runner raises some SandboxErrors itself, with no innerMessage: a denied capability, a missing or unavailable ctx.api method, a CPU budget or wall-clock ceiling, an install or marshal failure, a nested body's fault relayed one VM hop up. isSandboxOwnFault (@objectstack/types) recognises them off the producer, and the /data door answers them 500 INTERNAL_ERROR / Internal server error. Twenty-three hand-built REST write rows relayed the runner's diagnostic text instead.

  • packages/rest/src/error-response.ts: sandboxFaultAnswer answers isSandboxOwnFault with resolveErrorResponse, the same classification door and the same withheld 500 it gives a crash. One predicate, one terminal, no own-fault branch anywhere else.
  • packages/rest/src/rest-server.ts, record-share arms only: sharingFaultMessage asks sandboxFaultAnswer for its sentence instead of keeping its own sandbox read. The family keeps its own 500 SHARE_*_FAILED status and code, as it does for a crash (see "The record-share rows" below).
  • packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts §6: nine own-fault shapes, one per kind the runner raises, over every row the ledger already drives (ANSWERS_THE_SENTENCE, held to REST_ROUTE_LEDGER by §1, not hand-listed). The crash and declared-5xx shapes stay as the CONTROL, unchanged. Each own-fault case asserts the ADR-0112 status + code, the doors' withheld sentence, no trace of the runner's text in the body, and that the whole SandboxError reaches console.error (the file now declares OS_REST_LOG: 'info' inside §6, as check:rest-log-spy-declared requires).
  • .changeset/22885-sandbox-own-fault-rest-write-routes.md: @objectstack/rest patch, Clause-②: no.

Measured at RED (the card's "Not measured"), BASE 2e11d9006c

The new §6 cases, run before the fix: 207 failed / 370 passed, which is exactly 23 rows x 9 shapes. Every named downstream was called, and no upstream arm (a prefix map, a 403/404 arm, classifiedRefusalAnswer) intercepted any shape. Every row reached its own terminal with the runner's text:

Rows Status and code at RED
POST /analytics/dataset/query 500 ANALYTICS_QUERY_FAILED
POST /security/explain 500 EXPLAIN_FAILED
POST / DELETE /data/:object/:id/shares 500 SHARE_GRANT_FAILED / SHARE_REVOKE_FAILED
the three sharing-rule writes 500 RULE_*_FAILED
suggested-binding confirm / dismiss 500 SUGGESTION_*_FAILED
the nine POST /approvals/requests/:id/* writes 500 APPROVAL_*_FAILED
POST /packages/publish 500 INTERNAL_ERROR (text relayed)
the four POST /datasources/:name/external/* routes 400 EXTERNAL_DATASOURCE_ERROR / EXTERNAL_IMPORT_ERROR

The other 20 driven rows already answered every shape 500 INTERNAL_ERROR (the import rows per row inside 200 / 201), and all 86 control readings already withheld. Before any edit, sendThrownError (the classification door) answered all eight runner kinds 500 INTERNAL_ERROR / Internal server error, while sandboxFaultAnswer and classifiedRefusalAnswer both answered undefined. So the change is one line at the function every family already asks.

After: 21 rows answer 500 INTERNAL_ERROR / Internal server error. The two record-share write rows answer 500 SHARE_*_FAILED / Internal server error. GET /data/:object/:id/shares, which the write ledger does not drive, was measured by a throwaway probe (deleted): 500 SHARES_LIST_FAILED / Internal server error; the same words on a plain Error keep the relayed text there.

The record-share rows: one reading of the triage text

Triage says the record-share rows "go through the same answer" and "Do not patch their own terminal separately", and also that "The crash rows stay as the CONTROL". The dispatch's reading was that these arms ask sandboxFaultAnswer as the other families do. Measured, that literal shape moves the CONTROL: on the two share write rows a crash would go from 500 SHARE_*_FAILED to 500 INTERNAL_ERROR, and a sandboxed declared 5xx from 500 SHARE_*_FAILED to 503 SERVICE_UNAVAILABLE, four existing assertions, and it would delete the ledger's OWN_FAULT_TERMINAL exception that #22764 recorded as "a decision this card does not take".

So the share arms' sentence is sandboxFaultAnswer's (one predicate, no own-fault check of their own), and their status and code stay their own, exactly as for a crash. OWN_FAULT_TERMINAL keeps its two rows; its reason now names the shared read. If the family's status and code should move too, that is a four-line change plus deleting OWN_FAULT_TERMINAL, and it moves the crash and declared-5xx answers on these routes with it.

Files outside the claim's surface, declared

packages/rest/src/package-routes.ts and packages/rest/src/external-datasource-routes.ts, comment-only. Both docblocks said sentences this change makes false ("sandboxFaultAnswer is the doors' answer for exactly those two", "Every other throw keeps this family's own 400"). No code there changes; both callers already ask sandboxFaultAnswer first.

Reverse verification (one-time; all from committed 24f3dc6f46, via scripts/ablation-replace.mjs, each restore proven by blob == HEAD and an empty git diff HEAD)

The tested subjects resolve by relative src import, not through a dependency's exports, so no dist rebuild leg applies.

  • A, delete the own-fault line in sandboxFaultAnswer: anchor 1 to 0, blob 1c91226dfd37 to aa8e4dc71f19. Ledger 207 failed / 370 passed, the RED population exactly (the share rows go red too, since their sentence reads the same answer).
  • B, delete the share arm's sandboxFaultAnswer read: blob 8e11bff9ffa9 to e31a3df9f2c5. 18 failed, only the two record-share write rows (9 shapes each).
  • C, delete the approvals family's fault-arm logError: blob 8e11bff9ffa9 to 96834aaee48f. 81 failed (9 approval rows x 9 shapes), each on "the runner's error reaches the server log". The log assertion can fail.

Tests, at 24f3dc6f46

  • pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2 src/rest-write-route-hook-refusal-sentence.ledger.test.ts: 577 passed (189 before this change).
  • pnpm --filter @objectstack/rest exec vitest run --project local --maxWorkers=2: 273 files, 5721 passed, 326 skipped. This includes sharing-envelope.test.ts.
  • pnpm --filter @objectstack/rest exec vitest run --project repo --maxWorkers=2: 5 files, 209 passed, 1 skipped. This includes the three error-response-* suites.
  • pnpm --filter @objectstack/rest typecheck: exit 0, check:test-typecheck: OK. --listFiles puts the ledger in the tsconfig.test.json program (1 hit).
  • Gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack with no paths, on this tree. Reconciled with --ran: 65 derived, 64 run at exit 0, 1 NOT-MEASURED, 0 unrun.
  • NOT MEASURED: pnpm check:dual-build-cjs-loads, reason: exit 3, PREREQUISITE NOT MET. 42 packages this diff does not touch have no dist/ in this container. Narrower reading: @objectstack/rest is built and require('./dist/index.cjs') loads, with 34 exports. CI runs the full gate.
  • Lint, a proven narrowing rather than pnpm lint: eslint --no-inline-config --format json over the five touched .ts files. ① Population, read from eslint's own config: --print-config resolves a config for each of the five (6/6/6/6/5 rules), and the changeset .md matches none. ② The JSON result holds 5 files, 0 errors, 0 warnings. ③ Invariance: eslint.config.mjs sets no parserOptions.project and no projectService (its own comment: it "never enables type-aware linting"), and this diff touches no config or data file a rule reads, so it cannot move any untouched file's verdict.

Acceptance notes

  • The other sandboxFaultAnswer callers in rest-server.ts (analytics, explain, sharing rules, suggested bindings, approvals) still describe the answer as "(a crash, or a refusal declaring a 5xx)". That is now incomplete but not false, and it was left alone to keep rest-server.ts to the record-share arms. Carrier: none.
  • resolveErrorResponse asks isSandboxCrash at its top but not isSandboxOwnFault. An own fault reaches classifyDataError's own-fault terminal only because it carries no status or code. That holds for every raise site in quickjs-runner.ts today (no own-fault construction passes SandboxErrorInfo), so it is dormant. Carrier: none.
  • The import rows answer an own fault per row with the withheld sentence. This PR adds no log assertion on them, because their contract is a per-row outcome handed to the caller.

Generated by Claude Code

…s the data door does

sandboxFaultAnswer now answers isSandboxOwnFault (a SandboxError the
runner raised itself: a denied capability, a missing or unavailable
ctx.api method, a budget, an install or marshal failure) with the
classification door's own answer, the withheld 500 INTERNAL_ERROR it
already gives a crash. One predicate, one terminal.

The record-share arms' sentence goes through the same answer:
sharingFaultMessage asks sandboxFaultAnswer, and the family keeps its
own 500 SHARE_*_FAILED code, the write-route ledger's recorded exception.

The ledger's section 6 drives every write row with one own-fault shape
per kind the runner raises, the crash and declared-5xx shapes kept as
the control, and holds the full error to the server log.

Claude-Session: https://claude.ai/code/session_01VoSxBQujKLZKPwK2u5ehQ6
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 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 8 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/rest/src/package-routes.ts, packages/rest/src/rest-server.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/permissions/permission-sets.mdx (via /security/suggested-bindings/:id/dismiss (route, bridged from symbol sandboxFaultAnswer — its route source's handler names it))
  • content/docs/permissions/sharing-rules.mdx (via /sharing/rules/:idOrName/evaluate (route, bridged from symbol sandboxFaultAnswer — its route source's handler names it))
  • content/docs/permissions/system-context.mdx (via registerExternalDatasourceRoutes (symbol, a top-level function))
What this run could not see
  • 2 changed file(s) yielded no anchor (packages/rest/src/package-routes.ts, packages/rest/src/rest-server.ts) — pages documenting those are invisible to this run
  • 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 — 17 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 45e206e4987494dcb3f762ae527caf28f8ca3491 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 45e206e4987494dcb3f762ae527caf28f8ca3491

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

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