fix(objectql): engine refusals stamp httpStatus beside status so a locally caught refusal carries its HTTP status - #16154
fix(objectql): engine refusals stamp httpStatus beside status so a locally caught refusal carries its HTTP status#16154claude[bot] wants to merge 5 commits into
httpStatus beside status so a locally caught refusal carries its HTTP status#16154Conversation
Every producer in `packages/objectql` that stamps a numeric HTTP status on a thrown error now stamps it under both spellings. `status` is unchanged and kept: it is what every HTTP door in this repo reads. `httpStatus` is the ADR-0112 D5 spelling, and it is what a consumer holding the THROWN error reads — the CLI's `--json` error envelope was emitting `code` with no status at all for a locally thrown engine refusal. 20 producer sites: 12 inline `err.status = N` stamps (engine.ts, filter-comparand-shape.ts, summary-backfill.ts) and 8 error classes declaring `readonly status`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…the CLI envelope - `summary-backfill.test.ts`: the real-producer refusal test now asserts `httpStatus` beside `status` — this is the card's own producer, exercised through `backfillSummaryNulls` rather than mocked. - `error-http-status-spelling.test.ts` (new): constructs every engine error class that declares a status and asserts the two spellings agree. Nothing else enforces the pairing — the two keys are plain data on a thrown value, and `check:error-status-conformance`'s deriver reads `status`/`statusCode` only. Carries a bare-`Error` control so an all-undefined read cannot pass as agreement. - `summary-nulls.test.ts`: the pin the ruling names widens from `code` only to `code` + `httpStatus: 400`, and its fixture now mirrors what the producer really stamps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
… new line numbers Mechanical, by `node scripts/check-system-context-census.mjs --fix`. Stamping `httpStatus` inserted 8 lines into `engine.ts`, so every `isSystem` anchor below line 1001 shifted by the cumulative insertion count at its position (+9, +12, +13, +14 — each verified against the insert points). No prose changed; only the cited line numbers move. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…ed tree The merge brought in #16087, which inserted its own lines into `engine.ts` and re-anchored this page for them. Both sides had edited this `merge=os-regen` artifact, so the driver merged it with exit 0 while silently keeping one side; `scripts/pm/os-regen-merge.sh` took main's bytes and this commit re-derives the page from the merged tree with `pnpm gen:system-context-census`. Blast radius measured, not assumed: 105 rows before and 105 after, row SET identical once integers are normalised, 12 changed lines and all 12 identical apart from line numbers — no row dropped, none added, no prose moved. The deltas (+9/+12/+13/+14) are this branch's own cumulative insertion offsets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
📓 Docs Drift CheckThis PR changes 1 package(s): 10 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 16 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 f6486727e4c6bb4b965ceea798a3b90d62d24f37 && git checkout f6486727e4c6bb4b965ceea798a3b90d62d24f37
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 658262e462bee93a4babcd93a1aeddd468097320 011dfc57f9f92c5020cc579773a4504a4e2eaef8 && git checkout -B drift-repro 658262e462bee93a4babcd93a1aeddd468097320 && git merge --no-ff 011dfc57f9f92c5020cc579773a4504a4e2eaef8
node scripts/docs-audit/affected-docs.mjs --json 658262e462bee93a4babcd93a1aeddd468097320
|
Contract review (clause ②) — PASS — PR #16154 at head
|
| # | claim | reading | verdict |
|---|---|---|---|
| 1 | Ruling (b) executed: producers converge on httpStatus; packages/cli/src/utils/format.ts untouched |
File list confirms the reader is absent from the diff; every producer stamps the same number under both spellings. | correct |
| 2 | Census (the ruling's precondition): 20 producers (12 inline + 8 class fields, the latter invisible to a .status grep); the only status→httpStatus conversion (toRowApiError) and the one normaliser (resolveThrownHttpError) both sit on the wire path, nothing between the engine throw and errorCodeFields ⇒ per-producer limb; error.status readers are not zero ⇒ status kept |
Each figure with a firing control; the git grep '**/*.ts' pathspec trap (silently excluding src/*.ts) caught before the census was trusted — the same trap #15853 met. |
correct |
| 3 | ADR-0112 D5 cited accurately: error.httpStatus is the named destination, "(optionally)", target end-state — the card's "status is the ADR spelling" framing was wrong; triage's correction confirmed with bounds |
Read. | correct |
| 4 | Pins: the ruled summary-nulls.test.ts widening (code + httpStatus: 400, fixture corrected), a new class-agreement suite with a bare-Error control, the real producer exercised through backfillSummaryNulls; two ablations red |
Discriminating direction. | correct |
| 5 | p3 promotion condition (a machine consumer branching on the CLI envelope's status) measured absent with a firing control |
Stays p3. |
correct |
| 6 | Census page re-anchored by --fix (15 citations, offsets verified against insert points) |
Tool output on an os-regen path. | accepted |
| 7 | No @objectstack/cli changeset — the pin change touches only a .test.ts |
Per the ruling's conditional. | correct |
② semver
@objectstack/objectql minor — additive key on thrown errors and on five published class declarations. As ruled.
③ Boundary flags
- Retiring
statusis a follow-up card carrying the census (ruling item 4) — not a rider; correctly left. os lint --eval --json's generator-load failure emits a bare{error}— nocode, nohttpStatus— while the same command's project-lint catch-all emits the full ADR-0112 envelope #15549 (a site spreading no envelope) stays separate — it landed via fix(cli): carry the ADR-0112 error carriers onos lint --eval --json's generator-load exit #16044 tonight; read together, not merged.
Evidence and landing
Checks on 011dfc57: 11 success / 2 skipped / 18 in progress at review time; check-governed-merges --test on the 14 paths: 0 hits. Clearing, same stroke: needs:contract-review off #15779 and PR #16154 with provenance. Landing (ready + auto-merge SQUASH) from this seat once every check is green — at its next check-in if the dispatching seat has not.
Generated by Claude Code
Fixes #15779
Executes the maintainer ruling of decision batch #49 item 3 (comment
5551139388): shape (b) — the producers converge onhttpStatus.packages/cli/src/utils/format.tsis not touched; the reader stays exactly as it is.The census, first — it is what decided the landing
The ruling made the landing conditional on three measurements. Each is reported with a firing positive control, because a zero-hit grep without one is not a reading.
⚠ A measuring instrument that lied, found before the census was trusted
The first pass used
git grep -- 'packages/objectql/src/**/*.ts', which returned zero hits forINVALID_SORTinpackages/objectql. A directgrep -conengine.tsreturns 4. Without:(glob)magic, a**/pathspec matches only files at least one directory deep, so every file sitting directly insrc/—engine.ts,summary-backfill.ts,filter-comparand-shape.ts, i.e. all three producers the card names — was silently excluded. Every census figure below is measured with plain directory pathspecs instead. Had that first reading stood, the census would have reported "the card's premise is false, no producers exist".(i) Engine producers that stamp a numeric HTTP status — 20, all in
packages/objectqlTriage explicitly did not count these, so no number was inherited. Two spellings had to be swept, because the second is invisible to a
.status =grep:err.status = Nengine.ts(8),filter-comparand-shape.ts(3),summary-backfill.ts(1)readonly status = XDuplicateRecordError,HookUnscopedDataAccessError,MultiUpdateHookKeyDivergenceError,EmptyCredentialWriteError,SystemWriteOrganizationRequiredError, and three inregistry.tsThe class half is the one a
.statusgrep misses entirely:readonly status = 409has no dot. Control that the sweep fires: the same pattern returns 12 inline sites and 20 total, and every one of the 20 is now paired — a mechanical re-read confirms 20 matched pairs, 0 problems, with eachhttpStatuscarrying the same value as itsstatus.Also swept and found absent:
statusCodehas zero occurrences inpackages/objectql/src(control: the same sweep overpackages/typesfinds it, which is whyresolveThrownHttpErrorreads both).(ii) Is there a
status → httpStatusconversion layer on the local path? — No. One exists, but not on this pathThere is a single normaliser:
resolveThrownHttpErrorin@objectstack/types— "the ONE rule for what HTTP answer does a THROWN error declare". And there is exactly one existingstatus → httpStatusconversion in the repo:metadata-protocol'stoRowApiError(protocol.ts:1885-1889), which calls that resolver and stampshttpStatus: declaredStatus— the #8570 mirror case the card cites.But both sit on the wire path. The local path is: engine throws → the CLI's
catch→errorCodeFields(error). Nothing sits between them.packages/clidoes not importresolveThrownHttpErroranywhere (control: the same grep across the repo returns real hits inmetadata-protocol,objectql/registry.ts,adapters/hono— the pattern fires, the CLI just has none; the only match underpackages/cliis a CHANGELOG line). And routing the local path through that resolver would mean editingerrorCodeFields, which the ruling forbids.⇒ The census selected the second limb: each producer stamps
httpStatusat the site where it stampscodetoday.(iii) Readers of
error.statuson a locally thrown engine error — not zero, sostatusis KEPT36 read sites survive filtering out the assignments. The load-bearing ones are not comments:
packages/types/src/thrown-http-error.ts—resolveThrownHttpErrorreadse.statusthene.statusCode; every HTTP door in this repo goes through it.packages/rest/src/error-response.ts:2110—if (error.status >= 500), a live branch.packages/plugins/plugin-sharing/src/share-link-routes.ts— fiveerr?.status ?? 500sites.packages/runtime/src/domains/actions.ts:766,packages/runtime/src/sandbox/quickjs-runner.ts:287-288(the sandbox boundary serialisesstatusacross the VM edge).⇒ Ruling item 4 is satisfied by measurement, not assumption:
statusis kept alongside at all 20 sites, unchanged. Retiring it is not attempted here and would need its own card.The ADR, cited accurately
The card calls
status"the ADR-0112 engine spelling". Read directly, the ADR says the opposite: D5 (docs/adr/0112-error-code-vocabulary-and-ledger.md:154) records "the HTTP status lives on the transport and (optionally)error.httpStatus", and the summary at:20repeats it as "error.httpStatusonly". Triage's correction is confirmed and the card's framing is wrong. Its bounds are kept too: D5 is a target end-state, it carries "(optionally)", and ADR-0112's actual subject iserror.code's vocabulary — so this is a destination, not a present-tense mandate, and it is cited here as the reason the convergence points athttpStatusrather than as an existing rule anyone violated.The
p3promotion condition — the census answers it: no promotion, staysp3The condition is "any machine consumer that branches on that status code". Measured: nothing under
scripts/or.github/consumes the CLI--jsonenvelope's status (control: the same sweep overscripts/findsobjectqlandhttpStatusin other roles, so it fires).check-error-status-conformance.mjsmentionshttpStatus, but as docs-reconciliation prose about the wire envelope, not as a consumer of a CLI run. Theerror.statusreaders in (iii) are in-repo HTTP doors — they branch onstatus, which is unchanged. No machine consumer of the affected envelope was found. ⇒p3holds.What landed
httpStatusstamped besidestatus, same number, at all 20 producers.statusunchanged everywhere. No consumer was widened, anderrorCodeFieldsis byte-identical tomain.Anchors re-located by text (the card measured at
1914df81f; this branch cut from1f2a02ba0) —errorCodeFieldsis still atpackages/cli/src/utils/format.ts:246, exactly as the card said.engine.ts'sINVALID_SORTproducer was at1001andINVALID_FIELDat1102/1246;summary-backfill.ts'sresolveRecomputeScoperefusal at297-298. All found.Tests
summary-backfill.test.ts— the card's own producer, exercised for real throughbackfillSummaryNulls(not mocked), now assertshttpStatusbesidestatus.error-http-status-spelling.test.ts(new) — constructs every engine error class that declares a status and asserts the two spellings agree, with a bare-Errorcontrol so an all-undefinedread cannot pass as agreement. Nothing else enforces the pairing: both keys are plain data on a thrown value, andcheck:error-status-conformance's deriver readsstatus/statusCodeand does not know this spelling.summary-nulls.test.ts— the pin the ruling names, widened fromcodeonly tocode+httpStatus: 400, with its fixture corrected to mirror what the producer really stamps.Reverse verification (both ablations: mutation proved on disk by blob hash before measuring, restore proved byte-exact by blob-equals-HEAD after,
trapon absolute paths):readonly httpStatusfromDuplicateRecordErrorexpected undefined to be 409err.httpStatus = 400fromsummary-backfill.ts(the card's producer)project.nope: expected undefined to be 400Green at
77bcc4150: 36 affectedobjectqlsuites (937 tests), 6restconsumer suites (153),metadata-protocolbatch-row (18),driver-turso(60),service-automation(6), the CLI pin +errorCodeFieldssuites (19).typecheckgreen for both packages, test layers included. Full-repopnpm lintgreen — the whole-tree run, not a narrowed one.Gates: the family was re-derived from the real change set with
scripts/pm/dispatch-gates.mjsrather than a hand-built diff, twice — the second time after the docs file below entered the change set, which pulled in 29 further families the first derivation could not have named. All green. Four werePREREQUISITE NOT MET(exit 3, a non-reading) until the CLI was built, after whichcheck:i18n,check:i18n-coverageandcheck:i18n-walk-parityall ran green;check:dual-build-cjs-loadsandcheck:type-check-debtneed a whole-workspace build and remain unmeasured locally — CI runs both.check:error-status-conformanceis green and, as predicted from its source, unmoved: its deriver readsstatus/statusCodeonly.The one file outside
packages/content/docs/permissions/system-context.mdx— stampinghttpStatusinserted 8 lines intoengine.ts, which rotted 15 line-number citations on that page and reddenedcheck:system-context-census. Repaired mechanically by the gate's own--fix. Every offset was verified against the insert points (+9, +12, +13, +14 by cumulative position) before accepting it as rot rather than a real finding; the diff moves line numbers only, no prose. This is notcontent/docs/releases/, which is untouched.Clause ② —
yes, re-derived by measurement rather than inheritedThe ruling's starting answer was
yes. Rather than adopt it, it was measured with an ablation: build at head → revert the 9 changed sources to the merge base → rebuild → diff every declaration file the package publishes (files: ["dist", …]), then restore and prove byte-exactness.httpStatuscount in the published declarations: 0 at base, 5 at head. Five real declaration lines, on five exported error classes:dist/index.d.ts—readonly httpStatus: 409(DuplicateRecordError),: 400(EmptyCredentialWriteError),= 500(SystemWriteOrganizationRequiredError)dist/util-DrIPeSRh.d.ts—readonly httpStatus: 403(HookUnscopedDataAccessError),= 400(MultiUpdateHookKeyDivergenceError)⭐ The shared-chunk trap fired live here. Two of the five declarations exist ONLY in the content-hashed chunk, which neither barrel names by symbol; reading
dist/index.d.tsalone would have found 3 and reported the other 2 as absent. The barrels also differ in the chunk filename (util-mkEsVIjj→util-DrIPeSRh) — classified as not a surface change, correctly, since the export lists either side are otherwise identical. The threeregistry.tsclasses are not on the barrel, which is exactly why 8 producers yield 5 published declarations.⇒ An additive widening of a published type surface.
needs:contract-reviewis set on both carriers, this PR and the issue, each with a comparative read-back.Changeset
@objectstack/objectqlminor— additive key on thrown errors, matching both the ruling and the measured widening. No@objectstack/clichangeset: the ruling makes it conditional on the pin touching shipped code, and it does not —@objectstack/clipublishesfiles: ["dist", "README.md", "CHANGELOG.md"], and the only CLI file in this diff is a.test.ts.Not done here, deliberately
status— the census found readers, so ruling item 4 keeps it. A follow-up card would carry the census.os lint --eval --json's generator-load failure emits a bare{error}— nocode, nohttpStatus— while the same command's project-lint catch-all emits the full ADR-0112 envelope #15549 is not addressed here and stays its own card — read together, never merged; it is a site that spreads no envelope at all, a different defect.Generated by Claude Code