Skip to content

A raw driver code (SQLITE_ERROR) still reaches client-facing payloads through the batch verbs' code limb — the sibling field #8333 did not cover #8441

Closed

Description

@os-zhuang

Filed by the dev on #8333 while landing the P6–P13 producer sweep (PR #8436). Unassigned; nobody is on it. Not a defect in that PR — a different field, with a different rule, deliberately left out of its scope rather than ridden in.

The measurement

#8333 fixed the error string on the batch verbs' payloads: a caught sentence is quoted back only when the error declared a 4xx status. Several of those same collectors carry a sibling limb built straight off the caught object:

...(e?.code ? { code: e.code } : {})

publishPackageDrafts' failed[] and revertCommit' failed[] both have it. That limb is ungated, and drivers do carry code: better-sqlite3 raises SQLITE_ERROR, node:sqlite ERR_SQLITE_ERROR, postgres a five-character SQLSTATE.

Reproduced end to end on the #8333 branch (so, with the message fix already in place) — an insert on sys_metadata failing with Object.assign(new Error('SQLITE_ERROR: no such table: sys_metadata'), { code: 'SQLITE_ERROR', errno: 1 }), driven through publishPackageDrafts:

[
  {
    "type": "view",
    "name": "acct_view",
    "error": "publish failed",
    "code": "SQLITE_ERROR"
  }
]

The message is withheld exactly as #8333 intended, and the driver's own dialect still ships beside it.

Why this is a different rule, not an oversight in #8333

carryCatalogedErrorCode's docblock states the distinction, and it is load-bearing: a message is free text, so the catalog does not bound it and status-alone is the safe test; a code writes ApiErrorSchema.code, a closed union a driver's dialect must never enter. The two limbs therefore need different predicates, and #8333's positive list is the wrong tool for this one — the right test is membership in StandardErrorCode ∪ ERROR_CODE_LEDGER, which the file already has a helper for.

⇒ The likely fix is to route these limbs through the same catalog gate carryCatalogedErrorCode applies at the re-wrap exits, rather than widening declaresClientRefusal.

Scope note

Same sink as #8333 — failed[] rides on response data, so no HTTP boundary's message withhold reaches it. Consumers that switch on code are the reason it cannot simply be dropped: BATCH_ABORTED and the declared 4xx codes on those same arrays are read by callers, so the fix must filter, not delete.

Sites read from source (packages/metadata-protocol/src/protocol.ts, the failed.push collectors in publishPackageDrafts and revertCommit); only the publishPackageDrafts one was reproduced.

Related

#8333 (the error string, PR #8436) · #8136 (option C, the measured cluster) · #8086 (the ruling recording option C)


Generated by Claude Code

Activity

  1. self-assigned this
    on Aug 13, 2026
  2. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Claiming this.

    Plan: verify the premise on current main first, then route the code limb through the same catalog-membership gate carryCatalogedErrorCode applies at the re-wrap exits — catalogued codes pass through unchanged, non-catalogued driver dialect replaced. #8333's positive control (Studio flow CEL draft answering with code + structured issues) must stay green.


    Generated by Claude Code

  3. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 8441,
      "status": "done",
      "branch": "claude/issue-8441-batch-verb-driver-code",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8505",
      "premise_still_valid": true,
      "summary": "Premise reproduced on current main at both named sites, with the message fix already in place: publishPackageDrafts answered { error: 'publish failed', code: 'SQLITE_ERROR' } and revertCommit { error: 'revert failed', code: 'SQLITE_ERROR' }. Added clientFacingFailureCode(err) beside clientFacingFailureText and routed both failed[] limbs through it: the rule is CATALOG MEMBERSHIP (ErrorCode = StandardErrorCode union ERROR_CODE_LEDGER, the predicate carryCatalogedErrorCode and toRowApiError already share), not #8333's 4xx question. A catalogued code passes through byte for byte; an uncatalogued one becomes the code its declared status maps to, else INTERNAL_ERROR; an error with no code still yields no key. No error code minted. PM assumptions both held: the fix is confined to the two named sites plus the new helper (protocol.ts only), and seed/sys_capability paths were not touched. The two sibling collectors that build the identical limb, discardPackageDrafts and deletePackage, were MEASURED clean (no code at all under the same driver fault) because deleteMetaItem's re-wrap exits already gate code, so they got no filter and the measurement is pinned instead. head SHA 03db0654d2bb0530114cf166eeaac60370d9b770.",
      "tests": "New pin: packages/metadata-protocol/src/protocol.batch-verb-driver-code.test.ts, 12 cases in six sections — 12 passed. PER-SITE PREMISE on main: publishPackageDrafts failed[] = [{\"error\":\"publish failed\",\"code\":\"SQLITE_ERROR\"}]; revertCommit failed[] = [{\"error\":\"revert failed\",\"code\":\"SQLITE_ERROR\"}]; discardPackageDrafts and deletePackage carried NO code. AFTER: both named sites answer code INTERNAL_ERROR, nothing leaked. PASSES THROUGH: BATCH_ABORTED on collateral rows, NOT_OVERRIDABLE, VERSION_NOT_FOUND, INVALID_METADATA, and ERR_DATASOURCE_UNAVAILABLE on a 503 whose sentence is withheld (the discriminator that proves the two limbs answer different questions). REPLACED: SQLITE_ERROR -> INTERNAL_ERROR; SQLSTATE 42501 on a declared 403 -> PERMISSION_DENIED with the authored sentence kept whole. POSITIVE CONTROL GREEN: the broken-CEL flow draft still reports flows[0].nodes[1].config.approvers[0].value, code INVALID_METADATA, and issues[0].rule approval-expression-invalid. REVERSE VERIFICATION, both directions predicted first: (a) protocol.ts reverted to pre-fix — predicted 4 red / 8 green, MEASURED 4 red / 8 green, every prediction held including the operator-half case predicted red for its payload assertion; (b) the over-broad drop-the-limb variant — predicted 8 red / 4 green, MEASURED 9 red / 3 green, ONE MISS: the operator-half case went red because it asserts expectCataloged before the log — the same miss #8333 recorded for its own P7 log case — recorded in the file header rather than retro-fitted. ANTI-VACUITY: #8333's fake throws a bare Error with no code property, which is why its payload scan could not see this; the harness here attaches a real driver code, and section 1 fails against pre-fix code. SUITES: @objectstack/metadata-protocol 82 files / 1208 tests passed; downstream consumers (prefix filter, i.e. dependents) @objectstack/objectql 197 / 3539 and @objectstack/rest 112 / 1843 passed. GATES (lint.yml parse union re-derived with dispatch-gates.mjs against actual changed paths, all run post-merge): 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:type-check-coverage, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset — all PASS. check:error-code-casing was NOT in the prompt's list and caught a real defect mid-run (a lowercase pseudo-code literal in my fixture, ADR-0112 D1); the fixture now uses the postgres SQLSTATE 42501. check:objectui-pin-fresh is RED and pre-existing: .objectui-sha untouched here, main already carries 2018 changesets, so it fires identically on main. Changeset added. CI status at report time: in_progress (reporting at draft-PR time per the 2026-08-10 ruling).",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8502: deleteManyData's row errors carry raw driver text in errors[].message — measured {\"code\":\"INTERNAL_ERROR\",\"message\":\"SQLITE_ERROR: no such table: leave_request\"}; toRowApiError gates the code half but not the message half, and the sink is a data batch row rather than a metadata verb, so neither #8136 nor #8333 enumerated it. Searched first (keyword + file-path over open issues); #3566 is closed and a different producer, #8442 is seed-loader. Filed unassigned and unlabeled for PM triage."
      ]
    }

    Generated by Claude Code

  4. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Landed — c2c67bf5 on origin/main (PR #8505, squash-merged). Confirmed by both readings: the merge event, and the commit present on origin/main touching packages/metadata-protocol/src/protocol.batch-verb-driver-code.test.ts.

    What shipped

    clientFacingFailureCode in protocol.ts now gates the code field on catalog membership, not on authorship:

    if (typeof code === 'string' && ErrorCode.safeParse(code).success) return code;
    const status = (err as { status?: unknown } | null | undefined)?.status;
    return typeof status === 'number' ? standardErrorCodeForHttpStatus(status) : 'INTERNAL_ERROR';

    The rule distinction this card turned on

    error and code answer different questions, and #8333's rule for the first does not transfer to the second:

    • error — did the producer author this sentence? Gated on 4xx status, because a driver's own prose must not be replayed as if the platform wrote it.
    • code — is this token a member of the published catalog? Gated on ErrorCode.safeParse, a closed union per ADR-0112 D4. Authorship is irrelevant; membership is the whole test.

    The discriminator that proves the two limbs are not the same rule: ERR_DATASOURCE_UNAVAILABLE on a 503. Under the error rule (4xx only) it is suppressed; under the code rule it is a catalog member and passes through — one input, two different correct answers. That case is pinned.

    Placement is deliberate, not incidental

    • publishPackageDrafts — the filter is computed once, outside the .map(): one error drives the whole map, because the batch is all-or-nothing. Computing per-record would imply per-record outcomes the verb does not have.
    • revertCommit — computed per iteration, because each iteration genuinely can fail on its own.

    Scope decision (Prime Directive #12 — measured, not assumed)

    discardPackageDrafts and deletePackage are left unfiltered. That is a decision, not an omission: it was measured and is pinned, so a later change that quietly extends the filter to them will turn a test red rather than pass silently.

    Reverse verification

    Direction predicted before running: 8 red / 4 green. Measured 9 red / 3 green. The extra red was section 6, which asserted expectCataloged before the log line — the same miss #8333 recorded for its own P7 log case. Two cards, independently, have now hit the ordering assumption around the log site; that is a pattern worth carrying into #8442's review rather than a one-off.

    Anti-vacuity: the harness was checked against the failure mode that made #8333's pins blind here — its fixture threw a bare Error with no code property, so the scan was correct and the fixture simply never carried the field under test. This card's fixtures carry code (and a SCREAMING-free SQLSTATE variant), and the positive control confirms the pins go red when the filter is ablated.

    Residue

    #8502 — filed separately, not blocking. Nothing else outstanding on this card.


    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 workingsecurity

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions