Skip to content

fix(metadata-protocol): gate a batch row's errors[].message on a declared client refusal - #8569

Merged
os-zhuang merged 5 commits into
mainfrom
claude/issue-8502-batch-row-error-message
Aug 13, 2026
Merged

fix(metadata-protocol): gate a batch row's errors[].message on a declared client refusal#8569
os-zhuang merged 5 commits into
mainfrom
claude/issue-8502-batch-row-error-message

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #8502

toRowApiError interpolated whatever it caught into a bulk write's per-row errors[].message, so a driver fault under deleteManyData answered { "code": "INTERNAL_ERROR", "message": "SQLITE_ERROR: no such table: leave_request" } on response data riding a 200 — where no HTTP boundary's 5xx withhold can reach it. Fourth sink in the family, after #8136's overlay delete, #8333's failed[].error and #8442's seed errors[].message. The code half was already correct (#8441 gates it on catalog membership) and is untouched.

The disclosure is row data, not schema shape

The card quotes a tidy no such table. Driven against a real SqlDriver on better-sqlite3, the raw text is the whole failing statement:

SqliteError  code: SQLITE_CONSTRAINT_FOREIGNKEY  status: undefined
delete from `bd_parent` where `id` = 'p1' - FOREIGN KEY constraint failed

A delete leaks the WHERE clause and its bound record id; a create on the same surface leaks the full INSERT with every submitted value (dup@example.com in the pin). And it multiplies: reconcileStoppedBatch and buildRolledBackBatchResponse copy the causal row's message onto every NOT_ATTEMPTED / ROLLED_BACK sibling, so one leaked sentence is repeated across the batch. Every assertion here is therefore taken over the whole payload, not the failing row.

The measurement that decided the card

The complete set of catches was derived from the TypeScript AST, not a source grep: toRowApiError is module-private with exactly three call sites — runBatchDataLoop, runUpdateManyLoop, runDeleteManyLoop — and no other reference of any kind. Those three serve five entry points (batchData + its atomic arm, and both arms of updateManyData / deleteManyData).

Every producer reaching them was then driven on the real stack — a real ObjectQL over a real SqlDriver, through all three loops, with the error captured at the engine seam:

producer code status statusCode validation shape verdict
rowRequiredIdError VALIDATION_FAILED 400 no quoted
recordNotFoundError RECORD_NOT_FOUND 404 no quoted
objectql ValidationError VALIDATION_FAILED yes quoted
plugin-approvals record lock RECORD_LOCKED 409 no quoted
app hook, bare Error no withheld
SqliteError (unique / FK / no-table) SQLITE_* no withheld

Neither sibling's answer transfers whole. #8333's status-only test blanks rows 3 and 4. #8442's disjunct (4xx status or the VALIDATION_FAILED shape) reaches row 3 and still blanks row 4 — because plugin-approvals binds a global beforeUpdate hook whose lockedError spells its refusal statusCode, a spelling this sink receives and the seed channel never did.

So the rule asks the one question that covers all three declarations: would the boundary serving this throw call it a client refusal? That question already has exactly one implementation — resolveThrownHttpError from @objectstack/types, the same resolver /api/v1/data answers with — and it is imported, never re-spelled. Reading only one status spelling is how that door answered 500 to a deliberate 409 RECORD_LOCKED until #7525; a fourth local spelling here would rebuild that divergence one layer down, so a batch row and the single-record PATCH of the same object would disagree about the same error.

declaresClientRefusal is deliberately not widened — #8333 measured its own population and that limb is left exactly as it landed.

What it costs, stated plainly

A hook that refuses by throwing an undeclared Error no longer has its sentence echoed on the row. That is the measured cost of a positive list, not an oversight: at this sink an undeclared hook throw is indistinguishable from an undeclared driver throw, which is the whole hole. The remedy is at the producer and now has three accepted spellings — a 4xx status, a 4xx statusCode, or validationFailure(message, fields) — so declaring is cheaper than working around it. Same reasoning #8333 applied when it fixed P9's undeclared ZodError at the producer rather than loosening the collector. The case is pinned rather than regretted.

Also closed: the old String(err) fallback for an empty message, which renders Error: SQLITE_ERROR: … — the same second leak path #8333 found at its P13.

Verification

Positive control, real not synthesizedpackages/objectql/src/batch-row-authoring-feedback.test.ts runs the real validateRecord over a genuinely malformed record through the real loops: the author still reads Reason must be ≤ 8 characters (got 27) on both batchData and updateManyData, and the same file measures that the refusal carries no status and no statusCode — so it is quotable only because the rule reads more than status.

Real driverpackages/runtime/src/batch-row-driver-text-real-driver.integration.test.ts asserts the raw text really does carry delete from / 'p1' / insert into / dup@example.com, that it is not validation-shaped and resolves to 500, and that none of it appears anywhere in the payload.

Doubles are audited — metadata-protocol cannot import objectql or driver-sql (objectql depends on it), so the in-package pins use doubles whose exact own-property sets were measured from the real classes. Section 5 asserts those key lists in order and runs the production recogniser over each; it caught a real drift in my own ValidationError double on the first run (name assigned before code).

Both ablations, predicted before running:

ablation predicted measured
(a) revert protocol.ts 20 red / 12 green 20 red / 12 green, no miss
(b) blank the tail always new file 5 red; the 10 re-spelled fixtures green; objectql control 2 red / 1 green; runtime 3 green all exact; other declared-refusal pins went red 12 (predicted as a floor of 6)

The two directions test different properties, and (a) alone would not have been enough: the two no-status populations are green in both arms of (a) — pre-fix quoted everything, so it quoted them too, for the wrong reason. Only (b) separates "quotes what it should" from "quotes everything".

Fixture triage — 11 existing pins asserted an undeclared bare Error's message verbatim; 10 in metadata-protocol and one in packages/objectql that a package-scoped sweep would have missed (found by the downstream sweep). Nine were re-spelled with the propagation claims re-anchored to the causal row's own message so the two cannot drift. One needed replacing rather than re-spelling: upsert-existence's "a real update failure surfaces ITSELF" discriminated by message, and after the fix both the update's and the fallback insert's text are withheld — so .not.toContain('duplicate key') would have passed even if the fallback had run. Its claim is now carried structurally (the update was attempted, no insert followed), which is stronger than the string ever was.

Suites — metadata-protocol 85 files / 1261 tests, objectql 201 / 3553, rest 114 / 1881, runtime 155 / 2369, all green on the merged tree. Downstream consumer sweep in the prefix direction (--filter '...@objectstack/metadata-protocol' = the 44 packages that depend on it): all green.

Gates — the union re-derived with dispatch-gates.mjs against the actual changed paths and re-run after the final commit and after merging origin/main: check:nul-bytes, check:error-code-casing, check:engine-double-contract, check:cross-package-test-inputs, check:durability-log-level, check:filter-alias-parity, check:changeset-gate-self-tests, check:objectui-changeset, check:query-options-erasure, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-engine-split-ratio — all pass. check:type-check-debt (the ratchet, --self-test and --re-measure, per #8545) passes with no ledger raised: it caught two of my own defects mid-run — 2 untyped engine-option sites and a dead helper — both fixed at the source. check:objectui-pin-fresh is red and pre-existing: .objectui-sha is untouched by this branch and it fires identically on main. Control-byte self-scan over all changed files: clean.


Generated by Claude Code

@vercel

vercel Bot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 13, 2026 10:07pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/metadata-protocol)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/v9.mdx (via @objectstack/metadata-protocol)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

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/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deleteManyData's row errors carry raw driver text in errors[].message — the data-batch producer #8136/#8333 never reached

2 participants