Skip to content

test(runtime): pin the batch-row sink's disclose/withhold log coherence - #14684

Merged
os-musk merged 1 commit into
mainfrom
claude/issue-14403-seed-loader-cause-reach-through
Sep 2, 2026
Merged

test(runtime): pin the batch-row sink's disclose/withhold log coherence#14684
os-musk merged 1 commit into
mainfrom
claude/issue-14403-seed-loader-cause-reach-through

Conversation

@os-musk

@os-musk os-musk commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Part of #14403 — the residual half. Deliberately not a closing keyword: the triage seat asked for a measured verdict and reserved the grading, so the card stays open for it.

TL;DR — this is a measured zero-implementation result on the source, plus the pin the residual was waiting for

The card predicted a lost operator diagnostic in the batch-row sink. Measured on the real stack, there is none, for a reason that is itself worth pinning. So this PR changes no executable line: one comment block in protocol.ts recording the measurement, and one test file where a prose "KNOWN RESIDUAL" note becomes an assertion.

git diff is two files, and the protocol.ts half is comment-only — proven mechanically, every added line matches ^\+\s*\*:

$ git diff -U0 packages/metadata-protocol/src/protocol.ts | grep '^+' | grep -v '^+++' | grep -vE '^\+\s*\*'
(no output)

Step 1 — the hard gate, run before any edit

origin/main @ 2aa8456cf, worktree byte-pristine:

pnpm --filter @objectstack/runtime exec vitest run --maxWorkers=2 \
  src/batch-row-driver-text-real-driver.integration.test.ts \
  src/seed-loader-driver-text-real-driver.integration.test.ts

 Test Files  2 passed (2)
      Tests  4 passed (4)
os-verify-lock: VERDICT command-exit 0

GREENmain is not broken today, priority:p2 stands, and card item 3 (retriage the sentence pins) is not triggered — both files already carry their RETRIAGED, not re-baselined blocks from PR #14405. Posted on the card as comment 5513980544 before the first edit.

Item 2 — both named sites already reach through, by two different mechanisms

site reach-through? mechanism
seedFailureCause (seed-loader.ts:139-151) yes walks the cause chain, prefers the deepest sentence ([#14095] docblock)
seedCauseLabel (seed-loader.ts) yes already takes (err, printed) and compares the sentence about to be printed against the one the payload quoted
batch-row sink, withheld path yes passes err whole to console.warn; util.inspect renders the [cause] chain (Node v22.22.2)
batch-row sink, disclosed path n/a returns before the warn — and that is correct, see below

The triage read had seedCauseLabel as an open question; on this tree it is already the more advanced form, so nothing was owed there either.

⭐ The measured correction: the residual note in the pin file was half wrong

batch-row-driver-text-real-driver.integration.test.ts carried this on main:

with the row disclosed, the sink returns before its console.warn, so the warn fires ZERO times and the driver's own sentence — UNIQUE constraint failed: bd_note.email — reaches neither the response nor the console.

The zero-warn half is true. The "nor the console" half is false, and the counter-evidence comes out of that suite's own rig — captured during the step-1 run, with stack frames naming batch-row-driver-text-real-driver.integration.test.ts:136 and engine.ts:4572:

ERROR Insert operation failed {"object":"bd_note","error":{"message":
  "UNIQUE constraint failed: bd_note.email [statement and bound values redacted]", ...}}

That is the engine's insert door taking the envelope's cause on purpose — e instanceof DuplicateRecordError ? e.cause : e (engine.ts:10198), the #14095 / #14390 remedy, already pinned in packages/objectql/src/driver-fault-redaction.test.ts where it asserts the failing column survives in the line. The diagnostic moved one hop; it was never deleted.

Why adding the log would have been the wrong fix, not merely a redundant one

Two independent reasons, either sufficient:

  1. It restates what the engine already logs, once per duplicate row — a 500-row batch with 100 duplicates would emit 100 extra warns for a condition already in the log.
  2. AGENTS.md's degradation rule refuses this site outright. A batch row is the third answer it names: "a failure handed to the CALLER is not a degradation at all … a batch whose contract IS a per-item outcome report … Do not bolt a logger.error onto such a site." The row reports success: false, code: DUPLICATE_RECORD, httpStatus: 409 and the producer's own sentence. The caller was told.

The sink's own log exists to record a withhold. Nothing was withheld here, so there is nothing for it to record.

What actually changed

packages/metadata-protocol/src/protocol.ts — comment only. A [#14403] section on clientFacingRowFailureText recording the measurement and marking the disclosed branch ⛔ do-not-log, so the next reader does not re-open this from the same inference the card made.

packages/runtime/src/batch-row-driver-text-real-driver.integration.test.ts — the residual note becomes a pin. A console.warn recorder is installed in rig() (⛔ recording, never muting — every call is forwarded to the real console.warn, so shard-log output is unchanged), and the sink's decision/log coherence is asserted in both directions on one rig:

  • deleteManyData (undeclared FK fault, withheld) → expect(sinkWarnings).toHaveLength(1) plus the driver's own FOREIGN KEY constraint failed inside it. This is the live control: without it a green zero in the sibling test could mean the recorder was never wired.
  • batchData create (declared 409, disclosed) → expect(sinkWarnings).toEqual([]).

Ablation — the new pin is proven to bite the exact change this PR argues against

Mutated clientFacingRowFailureText to log on the disclosed branch (the fix I am declining), rebuilt, ran, restored. Predicted direction: RED, and that is what happened.

== BEFORE: worktree=c7ce3813f… head=c7ce3813f…      (worktree == HEAD before mutating)
== MUTATED: injected_hits=1 worktree=43f69cf48…     (marker on disk, hash moved)
✓ dist/: marker present in 2 built files -- the ablation is live in the artifact the suite consumes
== ABLATED TEST RUN exit=1
     × batchData create leaks neither the INSERT statement nor the values it carries
     AssertionError: expected [ Array(1) ] to deeply equal []
== RESTORED: worktree=c7ce3813f… still_injected=0   (bytes back to the HEAD blob)
✓ tree: working tree clean against HEAD
✓ dist/: marker absent from all 24 built files

Both legs rebuilt @objectstack/metadata-protocol and both were proven through scripts/ablation-dist-preflight.mjs (present, then --absent) — the runtime suite resolves this package through exports to dist/, so an unrebuilt ablation would have stayed green and certified a vacuous assertion. Mutation was confirmed on disk by a hit count on the injected text plus a blob-hash move, never by the editor's exit code; the restore names HEAD explicitly rather than a bare git checkout --, which would have restored from the mutated index.

Verification

All on the final tree; the union below was run at HEAD = the commit this PR ships.

  • The two pinsTest Files 2 passed (2), Tests 4 passed (4), VERDICT command-exit 0.
  • @objectstack/metadata-protocol full suiteTest Files 156 passed | 2 skipped (158), Tests 2141 passed | 10 skipped (2151), VERDICT command-exit 0.
  • typecheck@objectstack/metadata-protocol and @objectstack/runtime, both clean.
  • ⚠️ packages/runtime/tsconfig.json excludes its own test files, so that typecheck says nothing about the file I edited. Checked explicitly with a throwaway config (removed again; tree clean): tsc --noEmit exit 0, and --listFiles confirms the edited file was actually in the program (1 hit) rather than silently absent.
  • Gate family re-derived on the final tree with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (change set taken by the script from the merge base, not from a hand-rolled diff): 29 commands, 26 measured green.
  • The other 3 exited 3, which each gate's own verdict text calls NOT MEASURED rather than a findingcheck-test-completeness.mjs ("there is no local log to hand it … ⛔ It is not a red"), check:dual-build-cjs-loads and check:type-check-debt (both "PREREQUISITE NOT MET", needing a full-repo build; their self-tests passed, and check:type-check-coverage printed OK). A full turbo run build was run to clear the prerequisite and both re-run; result recorded in the dispatch report.

Exit codes were captured before any pipe (cmd > log 2>&1; EXIT=$?), and every verdict above is quoted from the gate's own verdict line rather than read off a bare $?.

No contract moved

No accept set widens or narrows, no public entry gains or loses an export, no declared refusal changes shape, and no door's answer changes — only what one of them logs, and that not at all in this diff. Clause-② remains no. The four leak assertions this suite exists for (insert into, dup@example.com, UNIQUE constraint failed, SQLITE_CONSTRAINT) are untouched, byte for byte, and passing.

Changeset

Carrying skip-changeset instead of a changeset file, which is this workflow's own prescription for a PR that "declares no release of its own": the diff is one comment block and one test file, so nothing is published and a version bump would ship a release that changes nothing. The dispatch's default of patch was explicitly conditional on the measurement, and the measurement moved it.


🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68


Generated by Claude Code

WIP checkpoint before the verification round.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions github-actions Bot added the size/m label Sep 2, 2026
@os-musk os-musk added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 2, 2026 — with Claude
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️ 1 changed file(s) yielded no anchor (packages/metadata-protocol/src/protocol.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/metadata-protocol/src/protocol.ts) — pages documenting those are invisible to this run
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json bd4aa4e49c6ac11f1833576d8b98fc41584b6a5dpackageMentionDocs.

os-musk commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Landing provenance (engine execution seat, session session_0112hMx9hjJ9BgB28X97DS68): flipped ready at 18:46Z and armed auto-merge (squash) at 18:46:43Z on head 99b509c80. The echo read method: MERGE; the queue build ref is the arming reading, not the echo.

  • Review: ACCEPT on the card, comment 5514600105. Clause-② no, self-read from the final diff and confirmed by this seat: no executable line changes anywhere in the PR, so no accept set, export or declared refusal can have moved. In-seat review is in order — the CONTRACT_REVIEW_TIER requirement binds Clause-② yes cards.
  • Governed-surface test re-run on the final two-path file list: 0 of 2 path(s) hit the register — ordinary queue landing. Re-derived rather than recalled.
  • Enqueue bar: all 37 check runs on 99b509c80 completed success or skipped, read with perPage: 50. Last standing was Test Core (1/6) (18:21:17Z → 18:39:44Z, 18 min) and Lint & Repo Gates (→ 18:38:33Z).
  • Verified by this seat against the tree rather than the report: the protocol.ts half is comment-only (every added line filtered against ^\+\s*\* leaves an empty set), and the four leak assertions the suite exists for survive — insert into 2, dup@example.com 4, UNIQUE constraint failed 2, SQLITE_CONSTRAINT 4.

Part of #14403, correctly — ⛔ not a closing keyword. The card stays open at MERGED: the dev shipped the measured half and reserved the grading for this seat, and Part-of PR must not also close its card is green on the head.

⚠️ Known exposure, stated in advance: the queue build runs the full suite, so this PR meets packages/cli/test/run-dev-unbuilt-workspace.e2e.test.ts — the flake tracked on #14648, which ejected #14629 at 17:45Z. If ejected, the disposition is fixed: ⛔ no re-queue while #14648 is open, ⛔ no weakening of that test, and the card goes to pm:blocked with an Unlock-action: line rather than being re-dispatched.

Owed by the seat at MERGED: verify by content on origin/main that protocol.ts carries the [#14403] do-not-log note and that the sink's coherence pin is present in both directions, then grade what remains on #14403 — the card is Part of, so pm:dispatched comes off only when the grading closes it.


Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queue Sep 2, 2026
Merged via the queue into main with commit 93d2d67 Sep 2, 2026
39 checks passed
@os-musk
os-musk deleted the claude/issue-14403-seed-loader-cause-reach-through branch September 2, 2026 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants