Skip to content

fix(objectql): engine refusals stamp httpStatus beside status so a locally caught refusal carries its HTTP status - #16154

Draft
claude[bot] wants to merge 5 commits into
mainfrom
claude/issue-15779-engine-httpstatus
Draft

fix(objectql): engine refusals stamp httpStatus beside status so a locally caught refusal carries its HTTP status#16154
claude[bot] wants to merge 5 commits into
mainfrom
claude/issue-15779-engine-httpstatus

Conversation

@claude

@claude claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #15779

Executes the maintainer ruling of decision batch #49 item 3 (comment 5551139388): shape (b) — the producers converge on httpStatus. packages/cli/src/utils/format.ts is 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 for INVALID_SORT in packages/objectql. A direct grep -c on engine.ts returns 4. Without :(glob) magic, a **/ pathspec matches only files at least one directory deep, so every file sitting directly in src/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/objectql

Triage 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:

form sites
inline err.status = N 12 — engine.ts (8), filter-comparand-shape.ts (3), summary-backfill.ts (1)
class field readonly status = X 8 — DuplicateRecordError, HookUnscopedDataAccessError, MultiUpdateHookKeyDivergenceError, EmptyCredentialWriteError, SystemWriteOrganizationRequiredError, and three in registry.ts

The class half is the one a .status grep misses entirely: readonly status = 409 has 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 each httpStatus carrying the same value as its status.

Also swept and found absent: statusCode has zero occurrences in packages/objectql/src (control: the same sweep over packages/types finds it, which is why resolveThrownHttpError reads both).

(ii) Is there a status → httpStatus conversion layer on the local path? — No. One exists, but not on this path

There is a single normaliser: resolveThrownHttpError in @objectstack/types — "the ONE rule for what HTTP answer does a THROWN error declare". And there is exactly one existing status → httpStatus conversion in the repo: metadata-protocol's toRowApiError (protocol.ts:1885-1889), which calls that resolver and stamps httpStatus: declaredStatus — the #8570 mirror case the card cites.

But both sit on the wire path. The local path is: engine throws → the CLI's catcherrorCodeFields(error). Nothing sits between them. packages/cli does not import resolveThrownHttpError anywhere (control: the same grep across the repo returns real hits in metadata-protocol, objectql/registry.ts, adapters/hono — the pattern fires, the CLI just has none; the only match under packages/cli is a CHANGELOG line). And routing the local path through that resolver would mean editing errorCodeFields, which the ruling forbids.

The census selected the second limb: each producer stamps httpStatus at the site where it stamps code today.

(iii) Readers of error.status on a locally thrown engine error — not zero, so status is KEPT

36 read sites survive filtering out the assignments. The load-bearing ones are not comments:

  • packages/types/src/thrown-http-error.tsresolveThrownHttpError reads e.status then e.statusCode; every HTTP door in this repo goes through it.
  • packages/rest/src/error-response.ts:2110if (error.status >= 500), a live branch.
  • packages/plugins/plugin-sharing/src/share-link-routes.ts — five err?.status ?? 500 sites.
  • packages/runtime/src/domains/actions.ts:766, packages/runtime/src/sandbox/quickjs-runner.ts:287-288 (the sandbox boundary serialises status across the VM edge).

⇒ Ruling item 4 is satisfied by measurement, not assumption: status is 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 :20 repeats it as "error.httpStatus only". 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 is error.code's vocabulary — so this is a destination, not a present-tense mandate, and it is cited here as the reason the convergence points at httpStatus rather than as an existing rule anyone violated.

The p3 promotion condition — the census answers it: no promotion, stays p3

The condition is "any machine consumer that branches on that status code". Measured: nothing under scripts/ or .github/ consumes the CLI --json envelope's status (control: the same sweep over scripts/ finds objectql and httpStatus in other roles, so it fires). check-error-status-conformance.mjs mentions httpStatus, but as docs-reconciliation prose about the wire envelope, not as a consumer of a CLI run. The error.status readers in (iii) are in-repo HTTP doors — they branch on status, which is unchanged. No machine consumer of the affected envelope was found. ⇒ p3 holds.

What landed

httpStatus stamped beside status, same number, at all 20 producers. status unchanged everywhere. No consumer was widened, and errorCodeFields is byte-identical to main.

Anchors re-located by text (the card measured at 1914df81f; this branch cut from 1f2a02ba0) — errorCodeFields is still at packages/cli/src/utils/format.ts:246, exactly as the card said. engine.ts's INVALID_SORT producer was at 1001 and INVALID_FIELD at 1102/1246; summary-backfill.ts's resolveRecomputeScope refusal at 297-298. All found.

Tests

  • summary-backfill.test.ts — the card's own producer, exercised for real through backfillSummaryNulls (not mocked), now asserts httpStatus beside status.
  • 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-Error control so an all-undefined read cannot pass as agreement. Nothing else enforces the pairing: both keys are plain data on a thrown value, and check:error-status-conformance's deriver reads status/statusCode and does not know this spelling.
  • summary-nulls.test.ts — the pin the ruling names, widened from code only to code + 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, trap on absolute paths):

ablation result
drop readonly httpStatus from DuplicateRecordError red — 2 failures, expected undefined to be 409
drop err.httpStatus = 400 from summary-backfill.ts (the card's producer) redproject.nope: expected undefined to be 400

Green at 77bcc4150: 36 affected objectql suites (937 tests), 6 rest consumer suites (153), metadata-protocol batch-row (18), driver-turso (60), service-automation (6), the CLI pin + errorCodeFields suites (19). typecheck green for both packages, test layers included. Full-repo pnpm lint green — the whole-tree run, not a narrowed one.

Gates: the family was re-derived from the real change set with scripts/pm/dispatch-gates.mjs rather 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 were PREREQUISITE NOT MET (exit 3, a non-reading) until the CLI was built, after which check:i18n, check:i18n-coverage and check:i18n-walk-parity all ran green; check:dual-build-cjs-loads and check:type-check-debt need a whole-workspace build and remain unmeasured locally — CI runs both. check:error-status-conformance is green and, as predicted from its source, unmoved: its deriver reads status/statusCode only.

The one file outside packages/

content/docs/permissions/system-context.mdx — stamping httpStatus inserted 8 lines into engine.ts, which rotted 15 line-number citations on that page and reddened check: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 not content/docs/releases/, which is untouched.

Clause ② — yes, re-derived by measurement rather than inherited

The 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.

httpStatus count in the published declarations: 0 at base, 5 at head. Five real declaration lines, on five exported error classes:

  • dist/index.d.tsreadonly httpStatus: 409 (DuplicateRecordError), : 400 (EmptyCredentialWriteError), = 500 (SystemWriteOrganizationRequiredError)
  • dist/util-DrIPeSRh.d.tsreadonly 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.ts alone would have found 3 and reported the other 2 as absent. The barrels also differ in the chunk filename (util-mkEsVIjjutil-DrIPeSRh) — classified as not a surface change, correctly, since the export lists either side are otherwise identical. The three registry.ts classes 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-review is set on both carriers, this PR and the issue, each with a comparative read-back.

Changeset

@objectstack/objectql minor — additive key on thrown errors, matching both the ruling and the measured widening. No @objectstack/cli changeset: the ruling makes it conditional on the pin touching shipped code, and it does not — @objectstack/cli publishes files: ["dist", "README.md", "CHANGELOG.md"], and the only CLI file in this diff is a .test.ts.

Not done here, deliberately


Generated by Claude Code

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

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 18 documentable anchor(s).

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

  • content/docs/api/client-sdk.mdx (via httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))
  • content/docs/api/error-catalog.mdx (via httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))
  • content/docs/api/error-handling-client.mdx (via httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))
  • content/docs/api/error-handling-server.mdx (via httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))
  • content/docs/api/index.mdx (via httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))
  • content/docs/api/wire-format.mdx (via httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))
  • content/docs/automation/flows.mdx (via httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))
  • content/docs/plugins/development.mdx (via httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))
  • content/docs/protocol/kernel/error-handling.mdx (via DuplicateRecordError (symbol, a top-level class))
  • content/docs/protocol/kernel/http-protocol.mdx (via httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))

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

  • content/docs/releases/v17.mdx (via cascadeDeleteRelations (symbol, a method of class ObjectQL), httpStatus (symbol, a field of class ArtifactObjectNameConflictError; a field of class DuplicateRecordError; a field of class EmptyCredentialWriteError; a field of class HookUnscopedDataAccessError; a field of class MultiUpdateHookKeyDivergenceError; a field of class ObjectOwnershipConflictError; a field of class SystemWriteOrganizationRequiredError))

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 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 16 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 658262e462bee93a4babcd93a1aeddd468097320packageMentionDocs.

Which tree this was computed on

This run read content/docs from f6486727e4c6bb4b965ceea798a3b90d62d24f37 — the merge of head 011dfc57f9f92c5020cc579773a4504a4e2eaef8 into base 658262e462bee93a4babcd93a1aeddd468097320, 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 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

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

Copy link
Copy Markdown
Contributor

Contract review (clause ②) — PASS — PR #16154 at head 011dfc57 (Fixes #15779)

Director seat, summon #15, session_01TezFG8ZMrNH6n5VTNpPpdH (os-zhuang), 2026-09-06T03:41Z, batch review under the maintainer's 「按批次执行完所有的契约复审」. Tier fuse: get_session this session reads session_context.model = last_served_model = CONTRACT_REVIEW_TIER. Readings from the PR diff (14 files), card #15779 + triage facets 5550529014 + the maintainer ruling 5551139388 (batch #49 item 3, 「其他同意」 → shape (b), census first); dev report 5556636415 and PM record 5556644843 read afterwards as cross-check.

Implemented-by: session_01ARYe3yQTQCUFm5qPYNgKaJ os-dev round (branch claude/issue-15779-engine-httpstatus)
Reviewed-by: session_01TezFG8ZMrNH6n5VTNpPpdH

Clause ② standing — yes (limb 1, additive), correctly re-derived

Five exported @objectstack/objectql error classes gain readonly httpStatus in their published declarations (three on the root barrel, two only in the content-hashed shared chunk — the instrument point holds again); the twelve inline producers add a key on thrown Error values. Nothing removed; status kept at all 20 sites. Limb 2: no — no request is newly accepted or refused; the HTTP doors read status and are unchanged.

① Derived judgments

# 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

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

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