Skip to content

A batch row's httpStatus reads only .status, so two genuine 4xx populations ship a row with no status at all #8570

Description

@os-zhuang

Found while measuring #8502's population on the real stack. Unassigned; nobody is on it. Not a defect in #8502's PR — that card is the message limb only, and this is the httpStatus limb beside it, deliberately not ridden in.

The measurement

toRowApiError (packages/metadata-protocol/src/protocol.ts) sets httpStatus from one field:

const status = typeof err?.status === 'number' ? err.status : undefined;
...
...(status !== undefined ? { httpStatus: status } : {}),

Driven on the real stack — a real ObjectQL over a real SqlDriver, through all three bulk-write loops — two producers that reach these catches declare a client refusal without .status:

producer code .status .statusCode validation shape row's httpStatus
objectql ValidationError VALIDATION_FAILED — — yes absent
plugin-approvals record lock RECORD_LOCKED — 409 no absent

Measured rows, verbatim:

{ "code": "VALIDATION_FAILED", "message": "name must be ≤ 4 characters (got 15)" }
{ "code": "RECORD_LOCKED", "message": "RECORD_LOCKED: record 'ok1' of 'm8502_task' is locked while an approval is in progress" }

Both are well-defined client refusals — 400 and 409 respectively — and a caller branching on httpStatus to tell "fix your input" from "the server broke" gets nothing for either. The sibling rows in the same response do carry it (rowRequiredIdError → 400, recordNotFoundError → 404), so within one batch the field is present for some failure rows and absent for others with no signal saying which.

resolveThrownHttpError (@objectstack/types) already answers this exact question and reads all three declarations — it is what the HTTP doors use, and what #8502 routed the message limb through. The httpStatus limb is the only one of the row's three fields still reading a single spelling:

Why it was left out of #8502

Adding httpStatus where the wire did not previously carry one is an addition to the response, not a withhold — the same reasoning #8441 recorded for declining to mint a code where none existed. It wants its own decision about whether BatchOperationResult consumers can absorb a newly-present field, and possibly a note in the ApiError contract about what absence means. So it is filed rather than ridden in.

Not a disclosure issue, unlike its two siblings — this is a usability/contract gap.

Related

#8502 (the message limb, PR #8569) · #8441 (the code limb) · #7525 (the same single-spelling defect at the HTTP door, fixed there) · ADR-0112


Generated by Claude Code

Activity

  1. added theissue type on Aug 13, 2026
  2. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    Triage: lands in packages/metadata-protocol (toRowApiError) → domain:metadata, pm:queue, type Bug.

    Rationale: httpStatus is an already-declared optional field of the row error contract, and sibling rows in the same batch response already carry it (rowRequiredIdError → 400, recordNotFoundError → 404). Reading only the .status spelling where resolveThrownHttpError reads all three is the same single-spelling defect #7525 fixed at the HTTP door — this is declared→enforced restoration on an existing field, not a new response surface, so it does not need the decision inbox. Mechanical boundary test: the acceptance set is unchanged; the field's population is pulled up to what the contract already declares.

    Note for the executing seat: the filer's open point — whether the ApiError contract should document what absence of httpStatus means — is a fair one-line docs rider on the same PR, not a separate decision. If implementation turns up a consumer that actually breaks on the newly-present field, that is a fork report (premise_still_valid: false), not a silent scope change.

    Triage seat Routine, session session_01KKqo1q7JddqjVbabF1ujMj.


    Generated by Claude Code

  3. self-assigned this
    on Aug 14, 2026
  4. os-zhuang commented on Aug 14, 2026

    @os-zhuang
    ContributorAuthor

    Claim — session session_012WMpuAfA2KSdDjGF6tm1bH, branch claude/issue-8570-row-http-status-declarations.

    protocol.ts region declaration (per the 3-concurrent-editor cap on this file):

    Concurrency check before starting: the only other live claim on this file is #8361's D1-gate region (distinct, and no open PR carries packages/metadata-protocol/src/protocol.ts in its diff yet — checked #8611, #8614, #8610, #8600, #8602, #8597, #8507, #8456). Cap therefore 2/3 with this claim.


    Generated by Claude Code


    Generated by Claude Code

  5. os-zhuang commented on Aug 14, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 8570,
      "status": "done",
      "branch": "claude/issue-8570-row-http-status-declarations",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8633",
      "premise_still_valid": true,
      "summary": "toRowApiError's httpStatus limb now reads resolveThrownHttpError (imported, the resolver the message limb and the HTTP doors already answer with) instead of err.status, so objectql's ValidationError (400 by shape) and plugin-approvals' record lock (409 spelled statusCode) reach the row with the status they always declared. It reads the resolver's declaredStatus, NOT status: for an undeclared throw the resolver answers 500 as the caller's fallback, and stamping that would newly put httpStatus 500 on driver faults and bare hook Errors — the over-broad direction the card explicitly does not take. The code limb reads the same resolution so a row cannot answer {code: INTERNAL_ERROR, httpStatus: 409}; the gate is declared-ness and deliberately not the 4xx band the message limb uses, because a row already ships httpStatus 503 today when the same refusal spells .status and narrowing would withdraw it. To make declared-ness askable without a third derivation, @objectstack/types' ThrownHttpError gains declaredStatus (the resolution minus the fallback, absent when nothing was declared) — the same question packages/rest's publish-classification suite currently asks with a sentinel fallback of 0. Docs rider included: ApiErrorSchema.httpStatus now documents that absence means 'no claim made' (an undeclared server-side fault on a batch row), never a status of its own and never 200. Note for the PM: two bots' labels landed on the PR after creation (documentation, size/xl, dependencies, tests, tooling) — not mine, left as they are.",
      "tests": "Real stack, card row 1 (packages/runtime/src/batch-row-http-status-real-driver.integration.test.ts, real ObjectQL + real SqlDriver/better-sqlite3 + real protocol loops): row = {code: VALIDATION_FAILED, message: 'name must be ≤ 4 characters (got 15)', httpStatus: 400}, with the thrown ValidationError asserted to declare no .status and no .statusCode — 4 passed. Real stack, card row 2 (packages/plugins/plugin-approvals/src/record-lock-batch-row-status.integration.test.ts, REAL bindApprovalLockHook over a real pending sys_approval_request row): row = {code: RECORD_LOCKED, message: \"RECORD_LOCKED: record '<id>' of 'opportunity' is locked while an approval is in progress\", httpStatus: 409}, hook throw measured in place (own props [stack, message, code, statusCode], status undefined) — 4 passed. New pin file protocol.batch-row-http-status.test.ts — 13 passed (6 sections: newly-populated rows; unchanged .status rows; the OVER-BROAD direction; declared-5xx boundary; code/httpStatus coherence; anti-vacuity on the doubles and the production recogniser). ABLATION (a) fix reverted, rebuilt, re-run: 8 pins red across the two metadata-protocol files, each 'expected undefined to be 400 / 409 / 503'; the undeclared-population pins stayed green as designed. ABLATION (b) over-broad (const {status} = resolveThrownHttpError(err); httpStatus: status unconditionally), rebuilt, re-run: 'a driver fault carries no httpStatus' and 'an undeclared app-hook refusal gains nothing either' both red with '+ \"httpStatus\": 500', and the real-driver SqliteError pin in runtime red too. Fix restored from the commit (git checkout <branch> -- protocol.ts), rebuilt, all green. Suites (pnpm test, per package): metadata-protocol 88 files/1286 tests, spec 397/10527, objectql 203/3579, rest 115/1895, runtime 156/2373, types 11/280, plugin-approvals 22/462 — all passed. Typecheck: types, spec, runtime, plugin-approvals — Done (metadata-protocol declares no typecheck script). Gates run locally (union of lint.yml + scripts/pm/dispatch-gates.mjs over the actual changed paths): check:test-source-alias OK (the new value import is aliased to source in a new plugin-approvals vitest.config.ts — the gate's prescribed fix; its shrink-only registry was NOT grown), check:nul-bytes OK, check:cross-package-test-inputs OK, check:filter-alias-parity OK, check:durability-log-level OK, check:spec-parsed-alias OK, check:type-source-resolution OK, check:merge-driver OK, check:changeset-gate-self-tests OK, check:objectui-changeset OK, check:type-check-coverage OK, check:query-options-erasure OK (test-surface ceiling unchanged, no baseline file added), and post-commit check-adr-0087-registration / check-changeset-no-major / check-empty-changeset all OK. Two gates could not run locally for environment reasons, both stated by the gates themselves: check:i18n needs the built CLI (and cannot move on this diff — plugin-approvals' i18n-extract.config.ts reads only the three sys_approval_* object files, none touched), and check:type-check-debt --re-measure refuses without a full workspace build, which is lint.yml's job.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8634: packages/rest's publish-classification suite asks 'did this throw declare a status?' with a sentinel fallback of 0 (resolveThrownHttpError(e, 0).status !== 0); now that ThrownHttpError.declaredStatus states the fact, the two call sites can delegate instead of inferring it from a magic number. `finding` label, no pm:queue, unassigned."
      ]
    }

    Generated by Claude Code

  6. added a commit that references this issue on Aug 14, 2026
  7. os-zhuang commented on Aug 14, 2026

    @os-zhuang
    ContributorAuthor

    Patch round — PR #8633 is now GREEN (all 28 check runs: 27 success, 1 skipped; TypeScript Type Check success at head 048b21ee).

    {
      "issue": 8570,
      "status": "done",
      "branch": "claude/issue-8570-row-http-status-declarations",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8633",
      "premise_still_valid": true,
      "summary": "Patch round on the CI red. The failure was one raw tsc error added by the new plugin-approvals integration test: registry.registerObject takes (schema, packageId, …) and the test passed only the schema (TS2554 'Expected 2-5 arguments, but got 1' at line 91), which the package's own typecheck script cannot see because its tsconfig excludes **/*.test.ts, while the TEST_DEBT ratchet measures tsc WITH the test layer in the program. Fixed at the call site by passing the package id ('com.objectstack.test.8570', the same spelling the runtime integration test uses) — the TEST_DEBT entry was NOT touched. Also merged origin/main (which had moved 15 commits, including #8594 on packages/metadata-protocol/src/protocol.ts): the merge is clean and touches a different region (promoteDraftForPublish's denial-audit hand-off) than toRowApiError, and metadata-protocol's suite is green post-merge. The fix and the docs rider from the first round are unchanged.",
      "tests": "Reproduced the ratchet's own measurement before fixing, using remeasureProject's generated shape (extends the package tsconfig, test exclusion dropped, absolute typeRoots): 349 diagnostics with exactly one in the new file — packages/plugins/plugin-approvals/src/record-lock-batch-row-status.integration.test.ts(91,69): error TS2554: Expected 2-5 arguments, but got 1. After the fix: 348, zero in that file, matching the recorded TEST_DEBT number exactly. Then, on a full workspace closure (turbo run build over ./packages/* and ./packages/*/* — 70/70 tasks successful, 6m19s), the real gate in all three modes: pnpm check:type-check-debt EXIT=0 — self-test 23 semantic + 24 observation + 25 re-measure + 28 built-closure + 9 auto-lowering cases hold; plain OK (64/77 packages type-checked, 13 in DEBT, 1 exempt); --re-measure 'OK — 33 ledger entr(ies) re-measured in 245.0s, 1968 raw tsc error(s) total, none above its recorded number'. The @objectstack/lint -1 informational line is present and untouched, as instructed. check:engine-double-contract EXIT=0 (my double already routes delete through assertEngineDeleteDispatch; the baseline was not grown). Full gate union re-run post-merge on the built closure, all EXIT=0: test-source-alias, nul-bytes, cross-package-test-inputs, filter-alias-parity, durability-log-level, spec-parsed-alias, type-source-resolution, type-check-coverage, query-options-erasure, engine-double-contract, changeset-gate-self-tests, objectui-changeset, merge-driver, and check:i18n (which ran for real this round — the built CLI is now present — rather than being argued from its config). Post-commit changeset trio (adr-0087-registration / changeset-no-major / empty-changeset) all EXIT=0. Suites post-merge: metadata-protocol 88 files/1291 tests, runtime 156/2373, plugin-approvals 22/464 — all passed. CI on head 048b21ee: 28 check runs, 27 success + 1 skipped (Console Pin Gate), including TypeScript Type Check, ESLint, Build Core, Test Core (1/3, 2/3, 3/3), Dogfood Regression Gate (1-3/3), Temporal Conformance, Validate Package Dependencies and Check Changeset.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8634 (first round): packages/rest's publish-classification suite asks 'did this throw declare a status?' with a sentinel fallback of 0; ThrownHttpError.declaredStatus now states the fact, so the two call sites can delegate. `finding`, no pm:queue, unassigned.",
        "filed as #8639: check-engine-double-contract never discovers a double whose write verb is `vi.fn(…)` — implOf (scripts/check-engine-double-contract.mjs:353) returns null for a call-expression initializer, so such doubles are neither pinned nor ledgered. Verified on two real metadata-protocol files (#8502's and this PR's), which are absent from both the pinned listing and the baseline while the gate reports OK; upper-bound population 168 files match `delete: vi.fn|update: vi.fn`, unclassified. `finding`, no pm:queue, unassigned. Not a defect in this PR — its double calls assertEngineDeleteDispatch explicitly."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions