Repository navigation
deleteManyData's row errors carry raw driver text in errors[].message — the data-batch producer #8136/#8333 never reached #8502
Description
Found by the dev on #8441 while filtering the metadata batch verbs' code limb. Unassigned; nobody is on it. Not a defect in #8441's PR — a different producer cluster, deliberately not ridden in.
The measurement
Driven in process against ObjectStackProtocolImplementation with an engine whose delete fails the way a missing table does (Object.assign(new Error('SQLITE_ERROR: no such table: leave_request'), { code: 'SQLITE_ERROR', errno: 1 })), one row through deleteManyData:
{
"success": false, "operation": "delete", "total": 1, "succeeded": 0, "failed": 1,
"results": [{
"id": "r1", "success": false, "index": 0,
"errors": [{ "code": "INTERNAL_ERROR", "message": "SQLITE_ERROR: no such table: leave_request" }]
}]
}The code half is already correct — toRowApiError gates it on StandardErrorCode ∪ ERROR_CODE_LEDGER, so the driver's SQLITE_ERROR became INTERNAL_ERROR. The message half is ungated:
message: typeof err?.message === 'string' && err.message.length > 0 ? err.message : String(err),packages/metadata-protocol/src/protocol.ts, in toRowApiError. Three catches feed it — the bulk batchData loop, updateManyData and deleteManyData — each pushing errors: [toRowApiError(err)] onto a per-row result.
Why this is a producer neither prior card covered
Same class as #8136 (the uninstall/overlay cluster) and #8333 (the metadata batch verbs P6–P13): the text rides response DATA, not a thrown message, so no HTTP boundary's 5xx withhold reaches it. But the sink is different — BatchOperationResult.errors[] on a data batch write, not failed[] on a metadata verb — and it is in neither card's enumeration. #8333's table stops at P13 and every one of its entries is a metadata verb.
The producer-side rule those cards installed applies unchanged: clientFacingFailureText(err, fallback) already exists in this file and asks the bounded question — did this error declare itself a client-facing refusal (a 4xx status)?
What a taker must check first, because it is the whole risk
updateManyData on a missing row never reaches the catch at all — a pre-check answers RECORD_NOT_FOUND / 404 directly — but the catch also receives rowRequiredIdError (VALIDATION_FAILED / 400), recordNotFoundError, and every validation refusal a hook or the schema raises. Whether each of those declares a 4xx status is the measurement that decides whether the rule can be applied here or whether some producer needs declaring first, exactly as #8333 had to do for the seed request. Do not sweep it blind.
Note also that httpStatus is absent from the measured envelope above — toRowApiError only sets it when the caught error carried a status, which a bare driver throw does not. That is the same signal the rule keys on, so the two facts are consistent.
Scope note
code is already correct here and needs nothing. This is the message half only.
Related
#8136 (option C, the measured cluster) · #8333 (the metadata batch verbs, PR #8436) · #8441 (the code limb, where this was found) · #8442 (seed-loader's own errors[].message, a different file) · #8086 (the ruling recording option C)
Generated by Claude Code
Activity
hotlong commentedon Aug 13, 2026
Triage: lands in⚠️ Dispatch note for the lane seat: the card's own warning is the whole risk — this catch carries a large authored 4xx population; the dev must measure which producers declare a 4xx
packages/metadata-protocol/src/protocol.ts(toRowApiError, message half) ⇒domain:metadata; queued as Bug + security (same info-disclosure class as #8441/#8333, producer not covered by either enumeration). The ruling context (#8086 option C, producer-sideclientFacingFailureTextrule) already exists — no decision needed.statusbefore applying the rule, not sweep blind.Size/model suggestion: M, judgment in the measurement step.
本评论来自分诊座位 Routine。
Generated by Claude Code
os-zhuang commentedon Aug 13, 2026
Claiming this card.
session_012WMpuAfA2KSdDjGF6tm1bHclaude/issue-8502-batch-row-error-messageDeclared region (3-concurrent-editor cap on
packages/metadata-protocol/src/protocol.ts):toRowApiErrorand its call sites — the per-rowerrors: [toRowApiError(err)]catches. Explicitly not thefailed[]collectors (publishPackageDrafts/revertCommit, #8333 + #8441, both landed), notcanonicalMetaType, not thefieldwrite door.Scope: the
messagehalf only.codeis already gated on catalog membership and is out of scope per the card.Plan, in order, before any edit:
toRowApiErrorfrom the built value / a real search, not from the card's count.status— the table that decides the card. This is step 1, not a preamble.seed-loaderinterpolates driver text intoerrors[].message, which rides onseedAppliedof a 200 publish response #8442'svalidationFailureDetailsdisjunct or only [finding]metadata-protocol's batch verbs still put caught error text on client-facing payloads — the 8 producers option C did not reach #8333's 4xx test. Two sibling sinks needed different answers to the same question, so this is measured here rather than inherited.seed-loaderinterpolates driver text intoerrors[].message, which rides onseedAppliedof a 200 publish response #8442 found the real text carries far more than the tidy sentence the card quotes.Generated by Claude Code
Generated by Claude Code
os-zhuang commentedon Aug 13, 2026
{ "issue": 8502, "status": "done", "branch": "claude/issue-8502-batch-row-error-message", "pr": "https://github.com/objectstack-ai/objectstack/pull/8569", "premise_still_valid": true, "summary": "Premise reproduced verbatim on main. The card's central risk was disarmed by measurement, and the measurement changed the answer: this sink receives TWO no-status client-refusal populations, not one, so neither sibling's rule transfers whole. #8333's status-only test blanks the objectql ValidationError AND plugin-approvals' RECORD_LOCKED; #8442's disjunct (4xx status OR VALIDATION_FAILED shape) reaches the first and STILL blanks the second, because plugin-approvals binds a GLOBAL beforeUpdate hook whose lockedError spells its refusal `statusCode` — a spelling the seed channel never saw. So the rule asks the one question covering all three declarations: would the boundary serving this throw call it a client refusal? That is resolveThrownHttpError (@objectstack/types), IMPORTED not re-spelled — the same resolver /api/v1/data uses, and reading only one status spelling is exactly how that door answered 500 to a deliberate 409 RECORD_LOCKED until #7525. declaresClientRefusal is NOT widened, so #8333's and #8441's collectors are untouched. Undeclared text (driver faults, bare hook Errors) gets a stable operation-named sentence; the original goes to a console.warn marked as withheld, written at the single decision point so no call site can withhold without logging. The old String(err) fallback — itself a leak, the same second path #8333 found at P13 — is closed too.", "tests": "POPULATION (driven on the real stack, real ObjectQL over real SqlDriver/better-sqlite3, error captured at the engine seam, through ALL THREE loops): rowRequiredIdError VALIDATION_FAILED status=400 -> QUOTED; recordNotFoundError RECORD_NOT_FOUND status=404 -> QUOTED; objectql ValidationError VALIDATION_FAILED status=undefined validationShape=true own=[stack,message,code,name,fields] -> QUOTED via the shape; plugin-approvals lockedError RECORD_LOCKED status=undefined statusCode=409 own=[stack,message,code,statusCode] -> QUOTED via statusCode; app hook bare Error -> WITHHELD; SqliteError (SQLITE_ERROR / SQLITE_CONSTRAINT_UNIQUE / SQLITE_CONSTRAINT_FOREIGNKEY) status=undefined -> WITHHELD. The approvals row was driven through the REAL bindApprovalLockHook against a real pending sys_approval_request, not inferred from source. REAL-DRIVER DELETE TEXT: `delete from `bd_parent` where `id` = 'p1' - FOREIGN KEY constraint failed` — the WHERE clause and its bound record id; the no-such-table case surfaces as the engine's pre-read SELECT carrying the same id; create-side carries the full INSERT with values (dup@example.com). Row data, not schema shape, as the PM predicted. NEW PINS: packages/metadata-protocol/src/protocol.batch-row-driver-text.test.ts (16 cases, 5 sections), packages/objectql/src/batch-row-authoring-feedback.test.ts (3, REAL validator), packages/runtime/src/batch-row-driver-text-real-driver.integration.test.ts (3, REAL SqlDriver). POSITIVE CONTROL, real not synthesized: a genuinely malformed record through the REAL validateRecord still answers 'Reason must be ≤ 8 characters (got 27)' on both batchData and updateManyData, and the same file measures that refusal carries neither status nor statusCode — so it is quotable ONLY because the rule reads more than status. ANTI-VACUITY: section 5 asserts each double's own-key list IN ORDER and runs the PRODUCTION recogniser (resolveThrownHttpError) over it — it caught a real drift in my own ValidationError double on first run (name assigned before code) and went red until the construction order matched the real class; it also asserts the withheld sentences interpolate nothing, so they cannot carry a leak by construction. Every leak assertion is over JSON.stringify(res), not the failing row, because reconcileStoppedBatch/buildRolledBackBatchResponse copy the causal message onto NOT_ATTEMPTED/ROLLED_BACK siblings. ABLATIONS, both predicted BEFORE running: (a) revert protocol.ts — predicted 20 red / 12 green (new file 7 red, 10 re-spelled fixtures red, objectql control 3 GREEN deliberately, runtime 3 red), MEASURED exactly 20 red / 12 green, no miss; (b) blank the tail always — predicted new file 5 red, 10 re-spelled green, objectql 2 red / 1 green, runtime 3 green, other declared-refusal pins >= 6 red; MEASURED all exact, with the 'other pins' count 12 (my prediction was a floor, not a point estimate — stated as such). (a) alone would NOT have been sufficient: the two no-status populations are green in BOTH arms of (a), because pre-fix quoted everything and so quoted them for the wrong reason. FIXTURE TRIAGE: 11 pins asserted an undeclared bare Error's message verbatim — 10 in metadata-protocol, 1 in packages/objectql that a package-scoped sweep would have missed (caught by the downstream sweep). 9 re-spelled with propagation claims re-anchored to the causal row's OWN message; 1 (upsert-existence 'a real update failure surfaces ITSELF') REPLACED rather than re-spelled, because 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 structural (update attempted, no insert followed). SUITES on the merged tree: metadata-protocol 85 files/1261, objectql 201/3553, rest 114/1881, runtime 155/2369, all green. CONSUMER SWEEP in the PREFIX direction (--filter '...@objectstack/metadata-protocol' = the 44 packages that DEPEND ON it): all green, run in foreground chunks (the 10-min tool cap, not a background watcher). One intermediate red was the unbuilt-closure signature (plugin-email: 4 FILES failed to import @objectstack/service-queue, 0 tests failed) and cleared after a full workspace build. GATES re-derived with dispatch-gates.mjs against actual changed paths and re-run AFTER the final commit AND after merging origin/main (per #8550): 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 (--self-test AND --re-measure, per #8545) PASS, 33 entries re-measured, 1969 raw errors, none above its recorded number, NO ledger raised — it caught two of my own defects mid-run (2 untyped engine-option sites tripping check:query-options-erasure, and a dead helper adding metadata-protocol DEBT 63->64), both fixed at the source. check:objectui-pin-fresh RED and pre-existing: .objectui-sha untouched, fires identically on main. Control-byte self-scan over all changed files: clean. No engine-double-contract entry raised — the new fake engine's delete() opens with assertEngineDeleteDispatch from @objectstack/metadata-core (the in-package route; objectql would close a cycle). Head SHA 91718d4eeaebcdc0893206349a96b2cc64b5ebb9.", "open_questions": [], "out_of_scope_findings": [ "filed as #8570: a batch row's `httpStatus` limb still reads only `.status`, so the two 4xx populations that declare themselves otherwise ship a row with no status at all — measured `{code: VALIDATION_FAILED, message: ...}` and `{code: RECORD_LOCKED, message: ...}`, both with httpStatus absent, beside sibling rows in the same response that do carry it. Adding it is an ADDITION to the wire (the reasoning #8441 recorded for declining to mint a `code`), so it is filed rather than ridden in. Searched first, keyword + semantic over open issues; nothing existing. Filed unassigned and unlabeled for triage." ] }Three corrections to the dispatch's PM mechanism assumptions.
A1 — partially falsified.
clientFacingFailureTextis the right family and is not superseded: it is #8136's, extended by #8333 to thefailed[]collectors, and it lives where the card said. But it is not the right helper for this sink, because its predicatedeclaresClientRefusalreads.statusalone and this sink's population declares itself two other ways. Using it here would have shipped a measurable usability regression. It is left untouched, and the row sink got its own helper delegating toresolveThrownHttpError. Related:origin/mainexportedclientFacingFailureTextmid-run (#8443,b30963dbc9) for the runtime publish door — no conflict, merged cleanly.A2 — confirmed and refined. Derived from the TypeScript AST, not a grep:
toRowApiErroris module-private, declared once, with exactly three call sites and zero other references of any kind (no export, no alias, no passing it as a value). But the enclosing functions are the private loopsrunBatchDataLoop/runUpdateManyLoop/runDeleteManyLoop, not the public verbs the card names — and those three serve five entry points, sincebatchData,updateManyDataanddeleteManyDataeach route both an atomic and a non-atomic arm through the same loop. So "three catches" is right and "three verbs" undercounts the surface.A3 — confirmed mechanically, and the two facts DO diverge.
httpStatusis set iff.statusis a number, so the card is right that the absence in its measured envelope shares a signal with the old message rule. But the inference does not survive the fix: after it,messagekeys on the boundary resolver whilehttpStatusstill keys on.statusalone, and theVALIDATION_FAILED/RECORD_LOCKEDrows are the specimens — sentence quoted,httpStatusabsent. Filed as #8570 rather than fixed here, since adding the field is an addition to the wire.One thing nobody asked about. The
DELETE_RESTRICTED/ 409 refusal inengine.tslooked at first like a bulk-path divergence — mydeleteBehavior: 'restrict'fixture produced a raw FK violation instead. I drove the single-recorddeleteDataover the same data as a control and it behaved identically, so it is my fixture's lookup declaration, not a bulk/single gap. Recorded as checked-and-not-a-finding rather than filed, so the next card in this lane does not re-derive it.Housekeeping: the sibling worktree
objectstack-issue-8468was removed by its own agent during my run — it is gone fromgit worktree listand from disk. Mygit worktree removenamed only my own path and exited 0; nothing of mine touchedplugin-securityordriver-sql.Generated by Claude Code
Generated by Claude Code
os-zhuang commentedon Aug 13, 2026
Landed —
ec746464onorigin/main(PR #8569, squash-merged, 22:40:15Z). Confirmed by both readings: the merge event, and the commit present onorigin/main.Fourth sink in the family, after #8136's overlay delete, #8333's
failed[].errorand #8442's seederrors[].message. Thecodehalf was already correct (#8441 gates it on catalog membership) and is untouched.The card understated the leak — the same way #8442's did
The card quotes a tidy
SQLITE_ERROR: no such table: leave_request, which reads like a schema-shape disclosure. Driven against a realSqlDriveron better-sqlite3, a delete's raw text is the whole failing statement:The
WHEREclause and its bound record id — which row, by id, in which table. On the create side of the same surface it is the fullINSERTwith every submitted value. Row data, not schema shape.And it multiplies:
reconcileStoppedBatchandbuildRolledBackBatchResponsecopy the causal row's message onto everyNOT_ATTEMPTED/ROLLED_BACKsibling, so one leaked sentence repeats across the batch. Every assertion is therefore taken overJSON.stringify(res)rather than the failing row — a scan of the failing row alone can be green while the payload still carries the text.This sink has TWO no-
statusrefusal populations, not one — so neither sibling's answer transfersThe complete call-site set was derived from the TypeScript AST, not a grep:
toRowApiErroris module-private with exactly three call sites, serving five entry points. Every producer reaching them was then driven on the real stack:statusstatusCoderowRequiredIdErrorrecordNotFoundErrorValidationErrorlockedErrorErrorSqliteError(unique / FK / no-table)#8333's
status-only test blanks rows 3 and 4. #8442's disjunct (4xxstatusor theVALIDATION_FAILEDshape) reaches row 3 and still blanks row 4, becauseplugin-approvalsbinds a globalbeforeUpdatehook spelling its refusalstatusCode— a spelling this sink receives and the seed channel never did. That row was measured by driving the realbindApprovalLockHookagainst a real pendingsys_approval_request, not inferred from source.So the rule asks the one question covering all three declarations — would the boundary serving this throw call it a client refusal? — and that question has exactly one implementation:
resolveThrownHttpErrorfrom@objectstack/types, the same resolver/api/v1/dataanswers with, imported and never re-spelled. Reading only one status spelling is how that door answered 500 to a deliberate409 RECORD_LOCKEDuntil #7525; a fourth local spelling here would have rebuilt that divergence one layer down, so a batch row and the single-recordPATCHof the same object would disagree about the same error.declaresClientRefusalis not widened — #8333's limb is left exactly as it landed.What it costs, stated rather than discovered later
A hook that refuses by throwing an undeclared
Errorno longer has its sentence echoed on the row. That is the measured cost of a positive list: 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 4xxstatus, a 4xxstatusCode, orvalidationFailure(message, fields). The case is pinned rather than regretted.Also closed: the old
String(err)fallback for an empty message, which rendersError: SQLITE_ERROR: …— the same second leak path #8333 found at its P13.Two things from the verification worth keeping
Ablation (a) alone would not have been sufficient. Reverting
protocol.tsleaves the two no-statuspopulations green in both arms — pre-fix quoted everything, so it quoted them too, for the wrong reason. Only the over-broad "blank the tail always" direction separates "quotes what it should" from "quotes everything". Two ablation directions here test different properties; they are not redundant confirmation.A fixture triage caught a test this fix would have made vacuous. Of 11 pins asserting an undeclared bare
Error's message verbatim (10 in metadata-protocol, one inpackages/objectqlthat a package-scoped sweep would have missed), nine were re-spelled — butupsert-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. Its.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.Residue
httpStatusreads only.status, so two genuine 4xx populations ship a row with no status at all #8570 — a batch row'shttpStatuslimb still reads only.status, so the two 4xx populations that declare themselves otherwise ship a row with no status at all, beside sibling rows in the same response that carry one. Adding it is an addition to the wire, which is the reasoning A raw drivercode(SQLITE_ERROR) still reaches client-facing payloads through the batch verbs'codelimb — the sibling field #8333 did not cover #8441 recorded for declining to mint acode, so it is filed rather than ridden in.Generated by Claude Code
fix(metadata-protocol): gate a batch row's errors[].message on a decl…