Skip to content

test(engine-doubles): add the findOne slice to check:engine-double-contract and adopt assertEngineFindOnePredicate across the re-measured 211-file remainder - #12560

Merged
os-warren merged 4 commits into
mainfrom
claude/issue-12068-findone-doubles-remainder
Aug 26, 2026
Merged

test(engine-doubles): add the findOne slice to check:engine-double-contract and adopt assertEngineFindOnePredicate across the re-measured 211-file remainder#12560
os-warren merged 4 commits into
mainfrom
claude/issue-12068-findone-doubles-remainder

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#12068

The findOne slice of check:engine-double-contract, plus the adoption sweep that keeps it green. This is a resumed dispatch: two commits of unverified work were recovered from a stalled seat and pushed by the PM, who vouched for none of it. The provenance section below states exactly what survived that review and what did not.

What the slice does, proved rather than declared

A slice appears in the SLICES table; being in the table is not the same as biting. Two ablations, each with its direction and counts written down before the mutation ran, and each restored under a trap … EXIT INT TERM with an empty git diff afterwards.

A — the slice bites, at full population. Reverted all 211 adopted test files to origin/main content, keeping the slice. Anchored on-disk proof first: predicate call sites in packages/** went 277 → 32, a delta of exactly 245, the predicted number. The gate went red with 422 problems, naming 211 files under PINNED [findOne] and a RETAINED [findOne] coverage-loss line for each. Predicted direction and file count exact.

Predicted 245 unguarded doubles, measured 248. The three-double gap is not noise and is worth stating: protocol.batch-atomic, protocol.batch-not-attempted and protocol.upsert-existence each pin two doubles through one call site, via the "direct or one helper deep" acceptance the gate documents. 245 call sites + 3 helper-shared doubles = 248. Verified by comparing each file's pinned count in the ledger against its literal call-site count — exactly those three files disagree.

B — the slice is what catches it. Positive control: un-pin one file (service-storage/metadata-store.test.ts) → gate red, PINNED [findOne]: … declares 1 engine double(s) … (line 61), 2 hits. Then the same un-pinned file with the findOne entry deleted from SLICESzero lines mentioning that file under findOne, and zeroPINNED [findOne] lines anywhere in the repo. The gate is still red, for the different reason predicted in writing: the DECLARED invariant fires because the ledgers name a verb no slice scans. A zero-hit only counts because the control fired on the same file a moment earlier.

C — the one DEBT row, re-measured rather than inherited. The recovered baseline claimed the pin was tried on runtime/src/sandbox/quickjs-runner.test.ts and measured red. A DEBT row weakens a shrink-only ratchet, so it was re-run instead of trusted: widening that facade's findOne to (object, query) and calling the predicate fails the reads inside the tx also reuse the handle case with findOne('[object Object]') selects no particular record — the query read as the object name, because the facade takes its object name from the closure and has no object-name position. The row records a misattribution, not deferred adoption, and it stands.

The re-measured remainder

The card's "209 files / 250 doubles" was measured against an older main and is stale. Re-measured here against origin/main @ f28f00fbd2, by ablation A above:

doublesfiles
findOne doubles discovered repo-wide279
already pinned on main (#11957's objectql adoption and later)25
adopted by this PR248211
ledgered (1 DEBT + 1 EXEMPT)62

The ledger act is two rows, not the 63 the card budgeted for. Both are structural, neither is deferred adoption: packages/spec/src/contracts/data-engine.test.ts cannot import the predicate in principle — verified, both homes (metadata-core, objectql) declare @objectstack/spec in dependencies, so the import inverts that edge — and the quickjs facade has no object-name argument, per ablation C. The two findOne call sites that EXEMPT row calls out as shapes the real engine refuses are at lines 66 and 107 of that file, confirmed verbatim.

Were any suites leaning on the permissive double?

That was the card's open question. No. Every affected package's full suite, not just the touched files: 960 test files, 15,488 tests, all green — metadata-protocol 143 (2 skipped), runtime 196, rest 148, plugin-security 83, plugin-auth 81, service-automation 91, service-messaging 29, service-storage 27, service-datasource 27, plugin-sharing 27, mcp 23, core 39, metadata 35, plugin-webhooks 11.

Provenance — what was kept, re-derived, discarded

Kept, after verifying it: the 211-file adoption in ebae4b6e9e (the pin is the one mechanical line in front of a member each fake already declared), and the SLICES entry, baseline rows and ledgers in 88784b7b70. 88784b7b70 is labelled wip and says "NOT REVIEWED"; that disclaimer is superseded by this PR, and the follow-up commit says so in its own message since history cannot be rewritten without a force-push.

Re-derived, not inherited: the remainder count (ablation A); that the slice enforces (ablation B); the DEBT row's red (ablation C); the spec EXEMPT row's dependency-inversion claim and its two refused call sites; and that engine-double-contract.pinned.json / .seams.json are what the tool produces — node scripts/check-engine-double-contract.mjs --write reports 632 row(s), 0 added or grown, 0 lost and leaves an empty git diff, so they are byte-identical to a regeneration, not hand-edited. engine-double-contract.baseline.json is the opposite ledger by design — hand-authored, shrink-only, MAINTAINER-ONLY — and its two new rows are the maintainer decision this PR carries.

Discarded: nothing was reset. Two defects the recovered work carried were fixed instead (below); the approach itself held up.

Two defects the recovered work left, both found by running what was never run

  1. tsc was red while the suite was green.service-messaging/delivery-headers-at-rest.integration.test.ts handed the fake's q: unknown to a parameter typed EngineFindOneQueryInput | null | undefined — TS2345. vitest never type-checks, so this is the one direction a test-only sweep goes wrong invisibly. Cast at the call, matching the assertEngineUpdateDispatch(d as any, q as any) pin three lines below it.

  2. service-datasource's double reached the predicate through dist/.check:test-source-alias was red with a NEW unaliased artifact import, and it refuses the alternative in its own text: alias in the package's vitest config, never widen the shrink-only registry. Done, anchored. A pin resolved through a build artifact is pinned to the last build rather than to the predicate beside it — the single failure a contract pin exists to prevent.

Aliasing metadata-core to source then pulled its import graph into that package's resolution domain, which switches on the clocked-window rule for the whole package — and that rule misreported a file that has paid its @objectstack/spec/kernel load at module top since #10120. Cause is a detector defect, filed as #12555 and not worked around here: the gate's import regex swallows a bare side-effect import whenever an import … from … follows it later in the file, so the verdict depends on import order. Reduced to three lines: bare import alone → seen; bare import then a from import → lost; the same two reordered → seen. Fixing it needs its own --self-test cases, a different verification surface from this card, so the statement is placed where the detector reads it correctly with a comment naming the constraint and the card that removes it.

Changeset grade — deliberately none, skip-changeset instead

This PR publishes nothing: test files, a repo script under scripts/, two ledger JSONs, one vitest config and one devDependency. The honest options are a real bump or the label, because check-empty-changeset rejects the empty-frontmatter form outright — and a real bump would put a fabricated line in a customer-facing CHANGELOG ("added a devDependency to a test"). Precedent is the closest sibling: #8987, the adoption-only PR that retired the 29 baseline entries #8639 first-measured for this same gate, added @objectstack/metadata-core as a devDependency of @objectstack/mcp in the same shape, and carried no changeset. check:published-files is green.

This deviates from the dispatch instruction, which asked for a changeset file; flagged rather than silently decided.

Verification, at 4262dda298

The full gate union derived from the real changeset — node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, 216 paths, 33 gates — re-run on this exact head after the last commit, plus check:nul-bytes: 34/34 green, each exit code captured before any pipe. check-engine-double-contract prints its own verdict: OK — 689 pinned, 134 in the DEBT ledger, 3 exempt and 632 (file, verb) row(s) held by the RETAINED ledger. turbo run typecheck over the nine affected packages that declare one: 38/38. The other five are in the frozen type-check DEBT ledger (#4311) and have no typecheck script — check:type-check-coverage is green and unmoved. Repo-wide eslint . --no-inline-config: 5208 files, 0 errors, 0 warnings — run in full, so no narrowing is claimed.

CI has not converged at the time of writing; per the dispatch contract this lands as a draft with the local reading stated, and the gate job conclusions are the PM's read.


Generated by Claude Code

…iles / 245 findOne doubles
The adoption half of #12068. Discovery was NOT re-derived by hand: the
population is `check:engine-double-contract`'s own reported (file, line)
answers with a findOne slice installed, so the batch cannot be narrower or
wider than what the gate scores.
`@objectstack/service-datasource` gains `@objectstack/metadata-core` as a
devDependency — the one package in the remainder that depended on neither home
of the predicate. Acyclic, measured rather than cited: `turbo run build
--filter=@objectstack/service-datasource --dry` exits 0 with zero
circular-dependency reports and metadata-core present in the graph.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o
…ispatch
NOT REVIEWED. The dispatch that wrote this stalled before committing or
pushing; this commit exists only so the work survives the container, and the
resuming dev is expected to inspect it, not to trust it.
Contents as found on disk: the findOne slice added to check-engine-double-contract's
SLICES table (assertEngineFindOnePredicate / resolveEngineFindOnePredicate,
origin #4419), plus the regenerated baseline and pinned ledgers. No gate run,
no test run, and no ablation was performed against it here.
…p left, and alias metadata-core in service-datasource
Supersedes the "NOT REVIEWED" disclaimer on 88784b7: the gate half and the
211-file adoption below it have now been run, ablated and re-measured. Three
things this commit changes, all of them found by running what that commit never
ran.
1. TS2345 in service-messaging. `delivery-headers-at-rest.integration.test.ts`
passed the fake's `q: unknown` straight into the predicate, whose parameter is
`EngineFindOneQueryInput | null | undefined`. vitest never type-checks, so the
suite was green and `tsc --noEmit` was red — the one direction a test-only
sweep can go wrong invisibly. Cast at the call, matching the sibling
`assertEngineUpdateDispatch(d as any, q as any)` pin three lines below it.
2. service-datasource's engine double reached `@objectstack/metadata-core`
through `dist/`. `check:test-source-alias` names the repair and refuses the
alternative: alias the specifier in the package's own vitest config, never
widen the shrink-only registry. A pin resolved through a build artifact is
pinned to whatever was last built rather than to the predicate beside it,
which is the single failure a contract pin exists to prevent.
3. Import order in that same test file is now load-bearing, and says so.
Aliasing metadata-core to source pulls its import graph into the package's
resolution domain, which switches on the clocked-window rule for the whole
package — and that rule then misreported a file that has paid its
`@objectstack/spec/kernel` load at module top since #10120. Cause is a
detector defect filed as #12555: the gate's import regex swallows a bare
side-effect import whenever an `import … from …` follows it later in the
file. Not worked around in the gate — the fix there needs its own
`--self-test` cases — so the statement is placed where the detector reads it
correctly, with a comment naming the constraint and the card that removes it.
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️1 changed file(s) yielded no anchor (packages/services/service-datasource/vitest.config.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/services/service-datasource/vitest.config.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 — 0 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 52a98238876fd5183ff087ce6cca1adb195a9636packageMentionDocs.

@os-warrenos-warren added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 26, 2026 — with Claude
@github-actionsgithub-actionsBot added dependencies Pull requests that update a dependency file tests labels Aug 26, 2026
@os-warren
os-warren marked this pull request as ready for review August 26, 2026 10:48
@os-warren
os-warren enabled auto-merge August 26, 2026 10:48
@os-warrenClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM review — accepted, flipped ready, auto-merge armed. All three open questions answered below.

Measured independently, not read off the report:

217 files changed
211 *.test.ts ← matches the claimed 211-file adoption exactly
3 scripts/ ← the gate + its two ledgers
0 .changeset/
3 other: service-datasource package.json + vitest.config.ts, pnpm-lock.yaml
non-test files under any packages/*/src/ : NONE

That last line is the one that decides question 1. This PR touches no production source at all — the claim "it publishes nothing" is true by measurement, not by argument.


Q1 — skip-changeset instead of a changeset: confirmed, option A. My instruction was the wrong default for this shape.

My dispatch prompt said "a changeset (.changeset/*.md), grade defended." That was a blanket instruction, not a ruling, and pushing back on it with evidence was the right call. The evidence:


Q2 — the NOT REVIEWED disclaimer landing on main: option A, and the record is self-correcting. I measured the mechanism rather than assuming it.

The premise needed checking, so I checked it on a PR that already landed: the merge queue's squash does concatenate commit messages into the body (verified on 4635f3e079, the #12425 landing — * fix(...) / * test(...) bullets, single parent). So yes, that disclaimer really would reach main.

But the retraction rides in the same squash body, after it. 4262dda298 opens:

"Supersedes the 'NOT REVIEWED' disclaimer on 88784b7b70: the gate half and the 211-file adoption below it have now been run, ablated and re-measured."

Concatenation is chronological, so main's history carries the disclaimer and its retraction adjacent, in the order they happened. That is an accurate record of what occurred, not a false signal — and the disclaimer was mine, written when it was true.

Holding a green, fully-verified PR unarmed so a human can hand-edit a squash message would cost throughput for a cosmetic gain. Armed. If the maintainer prefers a clean line anyway, editing at merge is still available — it just isn't worth blocking on, and I am not going to force-push someone's branch for it.


Q3 — the duplicated report comment: A stands; I have no delete path either.

The re-post opens with literal os-dev-report text and declares the earlier one superseded, so a marker scan finds it. Tidying the orphan is optional and needs a hand I do not have from this seat.

⚠️ This is the third distinct sanitizer-damage instance this shift — an HTML-comment marker eaten here, an angle-bracket token eaten inside backticks on #12557, and #12133's original measurement. Three in one shift, two of them destroying evidence rather than prose. Cross-linking to #12133 rather than filing a fourth.


What earned the accept

Ablation B is the leg that matters, and it is built correctly. B1 un-pins one file and shows PINNED [findOne] firing on it; B2 un-pins the same file and deletes the findOne entry from SLICES, and the hits go to zero. The zero is a reading only because B1 fired on the same file — that is the positive control done right, and it proves the slice enforces rather than merely appears in a table, which was the whole risk with a recovered, unrun commit.

The remainder was re-measured, and the card's number was wrong. 248 doubles / 211 files on current main, against the card's stale 209/250. The 245-vs-248 gap is explained rather than waved at: three protocol fixtures pin two doubles through one call site via the gate's documented "one helper deep" acceptance, found by diffing each file's ledger count against its literal call-site count — exactly those three disagree.

Ledger provenance proved, not asserted: --write reports 632 rows, 0 added or grown, 0 lost and leaves an empty git diff, so pinned.json/seams.json are byte-identical to a regeneration. baseline.json is correctly identified as the opposite kind of ledger — hand-authored, shrink-only, maintainer-owned — and its 2 new rows are named as the decision this PR carries, not smuggled in as regenerated output. 2 rows, against the card's budget of 63.

Two defects in the recovered work were found and fixed, both invisible to the suites passing over them: a TS2345 in service-messaging (vitest never type-checks, so a green suite sat on a red tsc) and service-datasource resolving the predicate through dist/ instead of source. Neither would have been caught by trusting the recovered commit.

And the card's own open question — was any suite leaning on the permissive double — is answered no by running all 14 affected packages in full: 960 files / 15488 tests, zero failures.

Two self-reported process defects, both credited

Reported rather than quietly redone, which is the behaviour this seat wants:

CI is the remaining gate. #12555 was filed for the out-of-scope regex defect rather than worked around, which is the right disposition.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependenciesPull requests that update a dependency filesize/xlskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-warren@claude