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

Closed

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. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 14, 2026
  4. self-assigned this
    on Aug 14, 2026
  5. 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

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

  7. added a commit that references this issue on Aug 14, 2026
  8. 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

bugSomething isn't workingpm:dispatchedDispatched to a dev agent by /pm-dispatch

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions