refactor(types): one isNativeErrorName reader, so three doors cannot disagree about what a crash is - #17842
Conversation
…pies The crash-vs-refusal predicate — does a thrown thing name a native JS error constructor — had three byte-identical copies, in `@objectstack/rest` (`isScriptFaultMessage`), `@objectstack/objectql` (`isScriptCrash`) and `@objectstack/runtime` (`sandboxRefusalMessage`). Every stated reason for a copy was a statement about reaching `@objectstack/rest`, which publishes one export subpath and re-exports nothing from `error-response`. `@objectstack/types` is the home: all three packages already depend on it and it depends on none of them, so the fold adds zero dependency edges and cannot cycle. `isNativeErrorName` owns the name list, the `^` anchor and the deliberate absence of a bare `Error:`; each door keeps its own wrapper, because the three wrappers are not the same shape and folding them would change per-door behaviour. Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com>
… a real build Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com>
…s siblings do Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check5 anchor(s) derived from 4 changed package(s); no hand-written page names any of them. What this run could not see
Coarse fallback — 41 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 ce6a09e40d0956a9fbddd25d2978bcdb99f63d28 && git checkout ce6a09e40d0956a9fbddd25d2978bcdb99f63d28
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6059b29c03e470dbaa513cc3f0992a8e610855be 17f0909a62ea9a963d8b12f7f66498716d854896 && git checkout -B drift-repro 6059b29c03e470dbaa513cc3f0992a8e610855be && git merge --no-ff 17f0909a62ea9a963d8b12f7f66498716d854896
node scripts/docs-audit/affected-docs.mjs --json 6059b29c03e470dbaa513cc3f0992a8e610855be |
Contract reviewHead reviewed: Verdict: PASS. ⭐ First — the dev's
|
| judgment | reading | verdict |
|---|---|---|
| the published surface gains exactly one symbol | export function isNativeErrorName(text: string | undefined | null): boolean, re-exported through packages/types/src/index.ts on the "." subpath |
✅ and narrower than it could have been — NATIVE_ERROR_NAME_RE stays module-private, so the pattern is an implementation detail and the predicate is the contract |
| no dependency edge is added | all three consumers already depend on @objectstack/types; packages/types depends on neither sibling ⇒ no cycle is possible |
✅ triage's Route 1 reasoning holds on the delivered diff |
the helper deliberately does not trim and says so as a contract, so each door keeps its own trimming: objectql .name untrimmed / .innerMessage.trim(), rest message.trim(), runtime inner.trim() — all four preserved, and both typeof … === 'string' guards intact |
✅ the one axis where absorbing a guard could silently have moved a door | |
the absorbed !text arm |
the regex is anchored and requires a name, so '' answered false before and answers false now; undefined/null cannot reach any site through its existing guards |
✅ no reachable delta |
| ⛔ the two predicates stay separate | index.ts's new comment fences it from error-leak.js: "⛔ Never merge the two predicates — a driver dump and a runtime crash are withheld for different reasons and at different statuses" |
✅ the right fence, written at the export site where the next reader meets it |
⭐ The narrow predicate moved and all three wrappers stayed. This seat published that shape as a reading, explicitly marked «⛔ not a route, the claimant's to choose»; the dev arrived at it and its report gives its own reason (the trim contract). ⛔ Recorded as agreement, ⛔ not as an instruction followed.
② Semver
'@objectstack/types': minor ← a new export
'@objectstack/rest': patch
'@objectstack/objectql': patch
'@objectstack/runtime': patch ← internal substitution, no surface change
✅ Correct on all four, and the split is the informative part: only the package whose surface grows is minor.
③ Boundary flags — each answered
- The
open_questionsconflict → resolved above; the write was the PM's and is done. rest-hook-script-fault-envelope.test.ts:116namesNATIVE_ERROR_NAME_RE, a constant that no longer exists under that name inpackages/rest. ⭐ Left intact deliberately: the table is a dated record of measurements on a past tree, and rewriting it would falsify a record. ✅ Correct, and the right instinct — a historical measurement is not a stale comment.detect-free-identifiers.ts:69(SANDBOX_GLOBALS) carries the same eight constructor names for a different question (which globals the sandbox provides). ✅ Named as the near-miss a future token-grep census will hit. ⛔ Neither is in the three filing classes; both correctly carry «Carrier: none».
The falsification, which the order put first — and which answered the one thing this seat could not
The order's open item was: «Is there a copy neither the token grep nor the declaration grep would catch? ⛔ This seat did not search for that and says so.»
It was searched, with five independent nets: a rare-token sweep (URIError/EvalError/AggregateError) repo-wide; startsWith/===/includes against a native name; any regex literal naming two or more of the eight; functions named for this question; and packages/types scanned with the same matcher lit at 1 in packages/rest on the same pass. All negative. Census confirmed three, byte-identical — one distinct 74-character literal, sort -u | wc -l = 1 — and after the fold 3 → 1, with the same matcher answering 3 on origin/main as the lit control.
⭐ The ablation, and the leg that makes this a real fold
Dropping Aggregate from the one reader's alternation, with the mutation proved on disk ('Internal|Aggregate)Error' 1→0, 'Internal)Error' 0→1, blob 2f84513b→e1d909da) and proved to have reached the artifact (ablation-dist-preflight.mjs … --absent exit 0), reddened both predicted places:
- the new module's own test — 2 failed, naming
AggregateErrorin both slots; - ⭐
packages/rest/src/rest-hook-script-fault-envelope.test.ts— "AggregateError: All promises were rejected: expected 400 to be 500".
⇒ leg 2 is the one that matters twice over: it reproduces the card's stated hazard (one door calling a crash a refusal and relaying the raw crash text at 400 while the others still withhold at 500), and it proves the fold is real rather than cosmetic — rest resolves @objectstack/types through dist/ per KNOWN_UNALIASED_TEST_IMPORTS, so that test could only move if rest now genuinely reads the one helper. Restore proved by blob equality with HEAD, an empty git diff HEAD, an empty whole-tree git status --porcelain, and both suites back green 31/31 and 18/18.
And the order's section 8 was honoured: guards were opened before writing, including error-response-sandbox-arm-message.test.ts, which asserts on error-response.ts's source text — keeping rest's wrapper name and call site intact is exactly what keeps that guard meaningful. ⛔ No gate weakened; no gate's assertion conflicted with the fix.
Independence pair
Implemented-by: claude/issue-17681-native-error-name-one-reader
Reviewed-by: session_01TSf4DV7ziu4V5j73e46b7c
⛔ Stated plainly: the implementer ran as a mode:subagent dev of this session — default-tier seat review plus gates, the model contract-review.md prescribes for a non-spec seat, ⛔ not an arm's-length second opinion. The checkable independent artefact is the ① baseline, published at 5645530604 before this report arrived.
What this review does NOT authorise
It verifies the widening is the one triage ruled (Route 1), that it is declared correctly, that no door's per-case behaviour moved, and that the fold is observable by a test that can fail. ⛔ It does not authorise the widening — triage's ruling 5643034660 did.
Generated by Claude Code
Fixes #17681
Clause-②: yes— inherited from the dispatch claim (5645195799). Widening@objectstack/types's published surface with one new export; the contract review of record rides with this PR.1. Falsification FIRST — the premise held, and here is the evidence
The order asked for three readings before a line was written. All three were taken on this branch's own tree (base
0cd841a16), each with a lit control on the same pass.① Is the census still three? Yes. The predicate that means something is the pattern itself, not the token
InternalError:Three, no fourth. The declaration grep (
const NATIVE_ERROR_NAME_RE) agrees at three, and a wider net — any regex literal naming two or more native error constructors — also returns exactly these three and nothing else.② Is there a copy neither grep would catch? ⭐ This is the one the seat explicitly did not search, and it is the reading this PR owes. Five independent nets, all negative:
URIError|EvalError|AggregateError, whole repostartsWith/===/includesagainst a native error name*ScriptCrash,*ScriptFault,*NativeError*,*RefusalMessage)packages/typesscanned for this predicate, with the same matcher lit at 1 inpackages/reston the same passpackages/cli/src/utils/detect-free-identifiers.ts:69carries'EvalError', 'URIError', 'AggregateError'insideSANDBOX_GLOBALS— the set of globals the QuickJS sandbox provides. Same eight names, an entirely different question (what can authored code reference), pinned by its own probe. ⛔ Not a copy.③ Do the three still agree today? Yes, byte-for-byte — measured, not asserted:
One distinct literal, 74 bytes including its
/delimiters, at all three sites. The card's own restraint («⛔ This seat is not asserting the copies currently disagree») survives re-derivation. ⇒premise_still_valid: true, and the hazard stays potential, exactly aspriority:p2was graded on.2. What landed — Route 1, the fold's shape, and what deliberately did NOT move
Triage's ruling is Route 1 and this PR takes it: the one reader lives in
@objectstack/types. Re-measured here —objectqlhas no@objectstack/restedge (so Route 2 cannot reach copy 2), all three consumers already have the@objectstack/typesedge, andpackages/typesdepends on none of them. Zero dependency edges added, no cycle possible.⭐ The narrow predicate moved; the three wrappers stayed. The seat's wrapper reading was re-derived and acted on: the three do not share a shape, only a pattern. Folding the wrappers would move a door's answer, which acceptance item 3 forbids.
@objectstack/restisScriptFaultMessageisNativeErrorName(message.trim())@objectstack/objectqlisScriptCrasherr.nameORerr.innerMessage.trim()isNativeErrorName(e.name)/isNativeErrorName(e.innerMessage.trim())@objectstack/runtimesandboxRefusalMessageisNativeErrorName(inner.trim())isNativeErrorNamedeliberately does not trim for its callers — the three disagree about trimming and each keeps its own choice, which is what makes the substitution per-case identical rather than merely equivalent-looking. That no-trim contract is pinned in §4 of the new test.3. The full file face — all four sites, the new home and its test
Declared per the cross-domain exception path; this is exactly the face the dispatch named, no growth.
packages/types/src/native-error-name.tsdomain:clipackages/types/src/native-error-name.test.tsdomain:clipackages/types/src/index.tsdomain:clipackages/rest/src/error-response.tsdomain:clipackages/runtime/src/sandbox/quickjs-runner.tsdomain:clipackages/objectql/src/hook-withheld-readonly-fault.tsdomain:engine.changeset/17681-native-error-name-one-reader.mdminor, the three consumerspatchAcceptance item 1 — the census after the fold, with the lit control the card asked for:
4. Acceptance item 3 — the per-case behaviour-identity ablation
⭐ Predictions written before the run: (a) the new reader's test goes red on the
AggregateErrorcases; (b) the REST door — which resolves@objectstack/typesthroughdist/, perKNOWN_UNALIASED_TEST_IMPORTS— goes red on its native-name row.Mutate leg.
Aggregatedropped from the alternation in the one reader.1 -> 0and0 -> 1; blob2f84513b… -> e1d909da…ablation-dist-preflight.mjs @objectstack/types 'Aggregate)Error' --absentexit 0packages/typesAggregateError — the bare name slotand— the flattened formpackages/restAggregateError: All promises were rejected: expected 400 to be 500Restore leg.
git checkout HEAD -- packages/types/src/native-error-name.ts, then:git hash-object== HEAD blob2f84513baea259a2feb58a6ff1e2cd80ec1a34f8;git diff HEADemptygit status --porcelainempty (not a per-path diff)marker present in 2 built files⛔ No ablation residue is committed: the ablation was a one-shot proof and the tree it ran against is the tree in this PR.
5. Verification
Every exit code captured before any pipe; every verdict read from the gate's own printed line. Run at the final head
17f0909a6, on a fully built tree.dispatch-gates.mjs --commands --repo objectstack-ai/objectstack)--ran, exit codes recorded)63 derived famil(ies) accounted for — 63 run, 0 NOT-MEASURED (a DERIVED zero)pnpm lint(this lane's standing addition;dispatch-gatesdoes not name it)VERDICT command-exit 0, no narrowing — the fulleslint . --no-inline-configran@objectstack/typestypecheckclean@objectstack/objectqltypecheckclean@objectstack/resttypecheckclean@objectstack/runtimetypecheckcleanturbo run build --filter=!@objectstack/docs)Two gates first answered exit 3 — PREREQUISITE NOT MET (
check:dual-build-cjs-loads,check:type-check-debt) on a partially built tree. ⛔ That is not a pass and not a finding; both were re-run after a full build and both print their own green verdict line. The table above is the post-build reading.Named, rather than left to read as green: the 50 artifact-roster families, the 10 wide-population families, the 5 path-scheduled CI jobs and the 5 workflow-valued commands
dispatch-gateslists as outside the derived total were NOT MEASURED here — they are CI's, by that tool's own accounting.6. Guards opened before writing, per §8 of the order
The order's standing-fault note was taken literally: the guards policing the current shape of each site were read first, not only the sites.
packages/rest/src/error-response-sandbox-arm-message.test.tsasserts on source text oferror-response.ts—aboveTheUnwrapDoor()must not containisScriptFaultMessage(, and the crash terminal must be asked once above the arms. Keeping rest's wrapper name and call site intact is what keeps that guard meaningful; it is green.packages/rest/src/rest-hook-script-fault-envelope.test.tscarries the mutation ledger for this predicate (cases A–F, including "the regex anchor^dropped") and the all-eight-names row that mutation leg (b) moved.scripts/check-lean-entry-closure.mjsdeclarespackages/types/**in its population and asserts a package set;@objectstack/typesis already admitted, so a module added inside it moves nothing. Green.scripts/check-test-source-alias.mjs'sKNOWN_UNALIASED_TEST_IMPORTSis what says rest and objectql read@objectstack/typesthroughdist/while runtime aliases it to source — the fact the ablation was designed around. Green.scripts/check-published-readme-exports.mjsreads README-to-exports in one direction only, so a new export cannot redden it.⛔ No gate was weakened, and no gate's assertion conflicted with the fix.
Acceptance notes
Neither of these is a reproducible defect, a contract violation, or a metadata-authoring trap, so neither is filed.
packages/rest/src/rest-hook-script-fault-envelope.test.ts:116namesNATIVE_ERROR_NAME_REin its historical mutation table ("E · The regex anchor dropped from …"). That constant no longer exists under that name inpackages/rest. The table is a dated record of measurements taken on a past tree, and rewriting it would falsify a record, so it is left intact — deliberately, not by oversight. Carrier: none.packages/cli/src/utils/detect-free-identifiers.ts:69(SANDBOX_GLOBALS) lists the same eight constructor names for a different question. A future census on the token will hit it; it is not a copy. Carrier: none.Generated by Claude Code