Uh oh!
There was an error while loading. Please reload this page.
fix(metadata-protocol): withhold caught driver text from the seed loader's errors[].message (#8442) - #8533
Conversation
…der's errors[].message (#8442) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
…e real-driver withhold (#8442) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
… drift this card introduced (#8442) All four new tsc errors are inside test files this PR adds, so they cannot have existed at the merge base. Fixed rather than ledgered: - metadata-protocol (+2, TS2352): section 6's fixture inspection cast an `Error & {...}` straight to `Record<string, unknown>`. Now cast through `unknown`, which is what the diagnostic itself prescribes. - objectql / runtime (+1 each, TS2554): `registry.registerObject` takes `(schema, packageId, ...)`. The one-argument form copied from neighbouring fixtures is itself part of those packages' frozen TEST_DEBT, so it was not replicated; both calls now pass a package id. No DEBT / TEST_DEBT entry was raised. `check:type-check-debt` re-measures 33 entries with none above its recorded number, surplus none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
Uh oh!
There was an error while loading. Please reload this page.
Fixes#8442
The third field in the family, after #8333's
errorstring and #8441'scode.The premise, reproduced on current
mainDriven in process through the real
SeedLoaderServicewith an engine failing the way a driver really does —Object.assign(new Error('SQLITE_ERROR: no such table: sys_metadata'), { code: 'SQLITE_ERROR', errno: 1 }):Verbatim the issue's shape.
seedAppliedrides a 200 publish response (applySeedBodiespassesr.errorsstraight through), so no HTTP boundary's message withhold reaches it.Which populations reach the catch — the measurement that decides the card
status?VALIDATION_FAILED?SQLITE_ERROR,errno)ValidationErrorErrorSqliteError,SQLITE_CONSTRAINT_UNIQUE)The last row was added under review — see "Q2" below. It is measured on a real driver, not constructed.
The rule, and why it is neither sibling's
errors[].messageis free text, so no catalog bounds it: #8441's membership rule governscodebecause that field writes a closed union (ADR-0112 D4), and it does not transfer. This is #8333's question — did the producer AUTHOR this sentence for a caller?But #8333's answer — a numeric 4xx
status— is measurably insufficient here, because this sink receives a populationprotocol.ts's collectors never see: the data engine's validation layer. Measured from the real class,@objectstack/objectql'sValidationErrorhas own properties[stack, message, code, name, fields]—code: 'VALIDATION_FAILED'and deliberately nostatus, because (@objectstack/types,validation-failure.ts) "deciding it means 400 is the job of whichever boundary serves it". For the seed channel this loader is that boundary, andVALIDATION_FAILED_STATUS = 400is the repo already stating such a throw is a 4xx client refusal missing only the property.So a sentence is quoted when the error declared itself a refusal by either shape: a 4xx
status, or theVALIDATION_FAILEDshape thatvalidationFailureDetailsalready recognises — imported, not re-spelled, so the seed channel and the HTTP boundaries cannot drift about what counts as one.On this producer the structured keys do not carry the offending field:
fieldis the literal'(write)', andtargetField/attemptedValuename the record's EXTERNAL key — i.e. which ROW. "Which key was rejected and why" (plan,max_length) exists only inside the validation sentence. Applying the 4xx test alone would have blanked exactly the per-record authoring feedbackerrors[]exists for, trading an authoring surface for a disclosure — the trade #8441 refused. Filtered, never deleted.After the fix
Driver text withheld at both payload producers (pass-1 write, pass-2 deferred back-fill); authoring feedback intact. Every structured key untouched — built from the seed declaration and the record, never from the caught error — and the authored prefix unchanged byte for byte (two runtime pins read it). The withheld line still reaches
logger.error, markedCause (withheld from the seed response), so the operator half is intact on both passes (see Q1).Scope: exactly the two payload producers the issue named. The loader's other
errors[]messages (unresolved / dropped references, dynamic-value failures) are authored and never quoted a driver; theCause:interpolations are LOG lines and keep the raw text by design.The positive control, green — and real
packages/objectql/src/seed-loader-authoring-feedback.test.ts: a realObjectQLengine, a real object declaringmaxLength: 4, a genuinely malformed seed record (plan: 'enterprise'). No error constructed by hand:Which record, which key, why — and the clean row still inserted. Blank the tail unconditionally and this goes red.
Anti-vacuity
ValidationErrordouble's shape was measured from the real class, not guessed (objectql cannot be imported into metadata-protocol — it depends on that package; the import would close a cycle), so the real own-property set was printed from inside objectql first.status/statusCodeundefined, and thatvalidationFailureDetails— the production recogniser, imported into the test — accepts the validation double and rejects the driver fault.Reverse verification — both directions predicted BEFORE running
(a)
seed-loader.tsreverted to pre-#8442. Predicted 6 red / 4 green; measured 6 red / 4 green, red set exactly as predicted. Section 5 predicted RED deliberately — it asserts the payload as well as the log, the miss both #8333 and #8441 recorded for their own operator-half case.(b) The over-broad "just blank the tail" variant. Predicted 3 red / 7 green; measured 3 red / 7 green, exactly section 2's three authoring-feedback cases. The positive control was predicted RED and measured RED.
No prediction missed in either direction. The fix was committed before the reverse runs, so each restore came out of a real commit, and the restored tree was confirmed byte-identical to the committed state.
Review round — three questions from the PM
Q1 — the pass-2 operator half was neither marked nor pinned. Fixed.
Measured: the pass-2
logger.errorand the pass-2 withhold are sequential statements in the same catch block, so the log does fire on exactly the path that withholds, and it did already carry the raw cause. The PM's two objections were both correct anyway:Cause (withheld from the seed response); pass-2 said plainCause:. An operator reading a pass-2 line had no signal the reporter received a different sentence. Both passes now share one vocabulary viaseedCauseLabel(err), which marks the label exactly whenquotableSeedFailureDetailwithheld.logger.errorand that it carries the withheld marker.Q2 — can driver text arrive wrapped as a validation failure? Measured: no.
Answered empirically rather than by reading, on the real stack — real
SqlDriver(better-sqlite3, on disk), realObjectQL, realSeedLoaderService, a duplicate on auniquecolumn. The driver raises:It does not wear the validation shape, so the quoting limb never opens for it. Nothing converts it on the way up: between the driver and this catch there is only ObjectQL, whose own
ValidationErrorthrows are authored —reference_not_foundbuilt from the message catalog, and a re-wrap of already-authored fields. The engine's unique-violation branch is an autonumber resync that retries and rethrows the original untouched. The conversions that do exist (mapDataError,resolveThrownHttpError) live at HTTP boundaries that consume this loader's output; they are downstream of this producer and can never wrap the engine's throw on its way into it. Every otherVALIDATION_FAILED-shaped producer in the repo was read and each carries a hardcoded authored sentence.packages/runtime/src/seed-loader-driver-text-real-driver.integration.test.tswith the assertion taken over the whole payload (noinsert into, nodup@example.com), plus non-vacuity assertions that the driver really rejected the duplicate and that the error really is not validation-shaped.The predicate was not widened, so the authoring-feedback trade this card exists to protect is unchanged.
Q3 — the type-check ratchet drift. Attributed to me, and fixed — no ledger raised.
Prediction, recorded before measuring: mine — the drifted packages were exactly the ones I added test files to.
Measured, and stronger than a base comparison: every drifted error sits inside test files this PR creates, which cannot have existed at the merge base, and the per-file counts match the deltas exactly.
seed-loader-driver-text.test.tsseed-loader-authoring-feedback.test.ts(runtime's +1 appeared after the CI run — it is the Q2 pin added in this round.)
Fixed rather than ledgered:
Error & {...}straight toRecord<string, unknown>. Now cast throughunknown, which is what the diagnostic itself prescribes.registry.registerObjecttakes(schema, packageId, …). The one-argument form copied from neighbouring fixtures is itself part of those packages' frozen TEST_DEBT, so it was not replicated; both calls now pass a package id.pnpm check:type-check-debtnow: 33 ledger entries re-measured, 1969 raw errors total, none above its recorded number, surplus none.The gate-derivation gap, stated rather than patched over
My earlier report listed
check:type-check-coverageas union-green. That is the wrong invocation: CI's ratchet step ispnpm check:type-check-debt(lint.yml:1357), which is the same script with--re-measure. The union-of-two-lists rule could not catch this — both lists name a script, and what differed was the flag. Compounding it,--re-measurerefuses to run without a fully built workspace closure, so it is the gate least likely to be run incidentally.Filed as #8545:
dispatch-gates.mjs:396emitscheck:type-check-coveragewhile giving the ratchet as its rationale, so the command it names cannot reproduce the failure it warns about.Verification
@objectstack/metadata-protocol1245 tests green ·@objectstack/objectql3544 green ·@objectstack/runtime2320 green · downstream consumer@objectstack/rest1870 green (consumer sweep used the prefix form...@objectstack/metadata-protocol).Gates re-derived with
dispatch-gates.mjsagainst the actual changed paths, union green:check:nul-bytes,check:error-code-casing,check:durability-log-level,check:engine-double-contract,check:cross-package-test-inputs,check:filter-alias-parity,check:changeset-gate-self-tests,check:objectui-changeset,check:query-options-erasure,check:type-check-debt(the ratchet, this round), pluscheck-adr-0087-registration,check-changeset-no-major,check-empty-changeset,check-engine-split-ratio. Control-byte self-scan over all changed files: clean.check:objectui-pin-freshis red, pre-existing and unrelated:.objectui-shais untouched here and the gate fires identically onmain.Notes for the PM
protocol.tswas NOT edited — both payload producers live inseed-loader.ts.code(SQLITE_ERROR) still reaches client-facing payloads through the batch verbs'codelimb — the sibling field #8333 did not cover #8441's PR fix(metadata-protocol): filter the batch verbs'failed[].codethrough the error catalog (#8441) #8505 was still OPEN when this card started —maincarried 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, so the premise was measured against that tree. fix(metadata-protocol): filter the batch verbs'failed[].codethrough the error catalog (#8441) #8505 landed mid-run (c2c67bf551) and is merged in here.packages/objectql's andpackages/runtime's tests read metadata-protocol's dist, so both pins are only meaningful after building it. An unbuilt closure also makes@objectstack/runtimereport ~124 failed files with tests passing — the resolution-failure signature, not a regression.Generated by Claude Code