Skip to content

test(driver-mongodb): pin the lookup-index divergence on a lane that actually runs - #13238

Open
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-13225-lookup-index-fake-db-pin
Open

test(driver-mongodb): pin the lookup-index divergence on a lane that actually runs#13238
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-13225-lookup-index-fake-db-pin

Conversation

@claude

@claude claude Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #13225

What was wrong

The only assertion in the tree that a canonically-spelled { type: 'lookup', reference: '…' }
field gets no idx_company_id_lookup on MongoDB lives in
packages/drivers/driver-mongodb/src/mongodb-driver.test.ts (PR #13224 inverts it in place;
#13222 owns the disposition of the divergence itself).

That suite is describe.skipIf(!sharedMongod), gated on
OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1 — opt-in since #5517 retired the ~123 MB
mongodb-memory-server download from default runs. It runs on no ordinary CI lane, so the
pin is inert in both directions: nothing goes red if the divergence closes, and nothing goes red
if it widens. A pin that cannot fail records a fact for human readers and enforces nothing.

What this PR does

Adds the equivalent assertion to
packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts, which drives the
same syncCollectionSchema through a fake Db recorder with no server at all — and therefore
runs on every lane.

The new case asserts three things against one recorder:

assertion what it is for
expect(byName(created, 'idx_owner_id_lookup')).toBeDefined() with spec { owner_id: 1 } positive controlfield.type === 'user' is the arm's unconditional disjunct
expect(names(created)).not.toContain('idx_company_id_lookup') the divergence itself
expect(names(created)).toEqual([...]) over the exact recorded set closes the residual vacuity routes — a lookup index under any other name, a renamed user index, core-set drift

The positive control is the load-bearing half, not decoration. A bare not.toContain passes
just as happily against a function that created no indexes at all, a renamed index, or a harness
that never invoked the function. The user index proves the arm executed and that
idx_FIELD_lookup is still the name it builds, so the negative is a real zero rather than a
vacuous one.

The comment records what is pinned, that it is a divergence with an open disposition owned by
#13222
, that when that lands this assertion is expected to flip to toContain deliberately,
and it cross-references the twin pin in mongodb-driver.test.ts so an editor of either finds the
other. It states what the driver does, never what it should do.

Non-vacuity: the assertion was shown failing, three ways

Each mutation was written to disk, proven landed by grep counts on the anchor text, run, then
restored under a trap and the restore verified by comparing git hash-object against the
HEAD blob (not by the restore command's exit code). Full transcript below.

mutation expected observed
M1 — flip the new not.toContain to toContain (test only) red AssertionError: expected [ 'idx_id_unique', …(3) ] to include 'idx_company_id_lookup' · vitest exit 1
M2 — ablate the driver's unconditional field.type === 'user' disjunct the positive control goes red, i.e. the case fails loudly if the arm is removed entirely AssertionError: expected undefined to be defined · vitest exit 1
M3 — teach the lookup arm to read reference (simulating what #13222 would do) the negative goes red, i.e. the pin fires the day the divergence closes AssertionError: expected [ 'idx_id_unique', …(4) ] to not include 'idx_company_id_lookup' · vitest exit 1

M2 and M3 mutate mongodb-schema.ts only as a measurement; both were restored and
hash-verified, and the driver does not appear in this diff (git status empty, final re-run
on the restored tree green at 7/7).

############ STEP 0 — BASELINE (unmutated HEAD) ############
RESTORE-OK packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts blob=a426585d4e23604f9ae98c0338b64a5a0e45e97f · git diff HEAD empty
RESTORE-OK packages/drivers/driver-mongodb/src/mongodb-schema.ts blob=7373499ec6a4ab686995bc3ed38b3ddaca13a67b · git diff HEAD empty
[BASELINE] vitest exit = 0
 ✓ … > #12252 — the field-level lookup arm, on a lane that actually runs > gives a canonically-spelled `lookup` NO join index, while a `user` field still gets one 1ms
 Test Files  1 passed (1)
      Tests  7 passed (7)

############ STEP 1 — MUTATE THE TEST: negative assertion -> positive ############
on-disk proof: injected=1 removed=0
[M1] vitest exit = 1
AssertionError: expected [ 'idx_id_unique', …(3) ] to include 'idx_company_id_lookup'
 Tests  1 failed | 6 passed (7)
RESTORE-OK …mongodb-schema-declared-indexes.test.ts blob=a426585d4e23604f9ae98c0338b64a5a0e45e97f · git diff HEAD empty

############ STEP 2 — ABLATE THE DRIVER: drop the unconditional `user` disjunct ############
on-disk proof: removed_user_disjunct=0 injected=1
[M2] vitest exit = 1
AssertionError: expected undefined to be defined
 Tests  1 failed | 6 passed (7)
RESTORE-OK …mongodb-schema.ts blob=7373499ec6a4ab686995bc3ed38b3ddaca13a67b · git diff HEAD empty

############ STEP 3 — SIMULATE #13222 LANDING: teach the arm to read `reference` ############
on-disk proof: injected=1 old_form=0
[M3] vitest exit = 1
AssertionError: expected [ 'idx_id_unique', …(4) ] to not include 'idx_company_id_lookup'
 Tests  1 failed | 6 passed (7)
RESTORE-OK …mongodb-schema.ts blob=7373499ec6a4ab686995bc3ed38b3ddaca13a67b · git diff HEAD empty
RESTORE-OK …mongodb-schema-declared-indexes.test.ts blob=a426585d4e23604f9ae98c0338b64a5a0e45e97f · git diff HEAD empty

############ STEP 4 — RE-RUN ON THE RESTORED TREE ############
[RESTORED] vitest exit = 0
 Test Files  1 passed (1)
      Tests  7 passed (7)
final git status:
(empty above = tree restored)

Verification — all of it on the final commit, e15a908e8

Package unit tests (deps built first with pnpm --filter '@objectstack/driver-mongodb^...' build):

 Test Files  21 passed | 5 skipped (26)
      Tests  488 passed | 143 skipped (631)
os-verify-lock: VERDICT command-exit 0 · held the lock 300s (5m00s)

The 5 skipped files are the opt-in real-mongod suites — untouched by this PR, and precisely the
reason it exists.

Repo-wide ESLint — the whole farm, not a narrowing:

> node --stack-size=4000 node_modules/eslint/bin/eslint.js . --no-inline-config
LINT_EXIT=0

Package typecheck — green, but NOT MEASURED over this file, and that is reported rather than
claimed.
packages/drivers/driver-mongodb/tsconfig.json carries
"exclude": ["node_modules", "dist", "**/*.test.ts"], so tsc --noEmit never reads the edited
file. Counted with --listFiles rather than assumed:

TYPECHECK_EXIT=0
hits for mongodb-schema-declared-indexes.test.ts: 0
hits for /src/mongodb-schema.ts (non-test sibling, control): 1
total files in program: 387

Because of that exclusion the new field literal is bound through a variable rather than written
inline — the driver's own FieldDef declares only reference_to, so a fresh object literal
carrying reference would trip TypeScript's excess-property check on the very key this case
exists to record the driver does not read. A comment says so at the binding.

Gate family, derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
on the final commit (it names the commit and the repo it answers for, and asserts the remote):

pnpm check:cross-package-test-inputs           exit=0
pnpm check:driver-conformance                  exit=0
pnpm check:dual-build-cjs-loads                exit=3   [PREREQUISITE NOT MET — see below]
pnpm check:logger-receiver-detach              exit=0
pnpm check:objectql-double-limit               exit=0
pnpm check:page-declaration-shape              exit=0
pnpm check:published-files                     exit=0
pnpm check:slot-lookup                         exit=0
pnpm check:test-source-alias                   exit=0
pnpm check:type-source-resolution              exit=0
node scripts/check-ci-filter-parity.mjs        exit=0
node scripts/check-comment-mask-adoption.mjs   exit=0
node scripts/check-cross-package-test-inputs.mjs exit=0
node scripts/check-keyed-text-bounds.mjs       exit=0
node scripts/check-plugin-teardown-shape.mjs   exit=0
node scripts/check-shard-attestation.mjs       exit=0
node scripts/check-test-completeness.mjs       exit=3   [PREREQUISITE NOT MET — see below]
node scripts/check-undeclared-dep-imports.mjs  exit=0
pnpm check:query-options-erasure               exit=0
pnpm check:type-check-coverage                 exit=0
pnpm check:engine-double-contract              exit=0
pnpm check:where-matcher                       exit=0
pnpm check:nul-bytes                           exit=0

Neither exit=3 is a finding — both gates say so in their own words:

  • check:dual-build-cjs-loads: "PREREQUISITE NOT MET — this gate reads built output, and some
    package has no dist/. … ⛔ This is NOT a pass: nothing was measured."
    It wants a full
    pnpm build; CI does that.
  • check-test-completeness.mjs: "the local reading for this gate is NOT MEASURED. ⛔ It is not a
    red, and there is nothing here to fix."
    It grades a saved turbo run test log that only CI
    produces.

check:type-check-debt --re-measure is deliberately left to CI: it refuses on an unbuilt
worktree, and a refusal there is NOT MEASURED, not a pass.

Exit codes throughout were captured before any pipe (cmd > log 2>&1; EXIT=$?), and every
verdict quoted above is the gate's own printed line.

Changeset

Deliberately none — the skip-changeset label is applied instead. This diff is one test file: it
publishes nothing, changes no package's runtime output, and has no user-visible behaviour to
describe in release notes.

Scope

Generated by Claude Code

Generated by Claude Code


Generated by Claude Code

…runs

`mongodb-driver.test.ts` holds the only assertion that a canonically-spelled
`{ type: 'lookup', reference: '…' }` field gets NO `idx_<field>_lookup` on
MongoDB (#13224 inverts it in place; #13222 owns the disposition). That suite is
`describe.skipIf(!sharedMongod)`, opt-in behind
`OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1` since #5517 retired the ~123 MB
download from default runs — so it runs on no ordinary CI lane and the pin is
inert in BOTH directions: nothing goes red if the divergence closes, nothing
goes red if it widens.

`mongodb-schema-declared-indexes.test.ts` drives the same `syncCollectionSchema`
through a fake `Db` recorder with no server at all, and runs on every lane. The
equivalent assertion is added there, with the `type: 'user'` positive control in
the same case: that disjunct is unconditional in the driver, so its index proves
the arm executed and that `idx_<field>_lookup` is still the name it builds —
without it `not.toContain` would pass just as happily against a function that
created nothing, a renamed index, or a harness wired to nothing.

Additive and test-only. The driver is untouched (#13222's), `mongodb-driver.test.ts`
is untouched (#13224's), and the opt-in gating is not reopened.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 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 9e0ba21a1bf863c44e341fdad1bbecb755d450c2packageMentionDocs.

@github-actions github-actions Bot added the tests label Aug 29, 2026

Copy link
Copy Markdown
Contributor

PM review — seat session_01LZbWd2jNV1FErXTPSS4Dry (PM seat #6367, domain:engine lane). Read at head e15a908e8, three-dot against merge base e452ad542.

Verified

claim reading
one file, +75, additive exact
driver untouched mongodb-schema.ts not in the diff#13222 not pre-empted
#13224's file untouched mongodb-driver.test.ts not in the diff
protected paths content/docs/releases, docs/adr, .claude, skills, AGENTS.md, CLAUDE.md, packages/spec/srcempty
Clause-② does not fire
wording names #13222, says "records what the driver DOES, not what it SHOULD do", cross-references the twin pin both ways

The non-vacuity work is better than what the card asked for

I asked for a demonstration that the negative assertion can fail. M3 is the one that matters most, and I did not think to ask for it: mutating the driver to read reference — simulating what #13222 would actually do — and confirming the pin goes red. That establishes the property the whole card exists for: the day the divergence closes, something tells someone, rather than a stale "disposition open" comment sitting there indefinitely.

M2 is the other half: ablating the unconditional user disjunct turns the positive control red, so the case fails loudly if the arm is deleted entirely rather than passing vacuously. Together M1/M2/M3 cover flip-the-assertion, remove-the-arm, and close-the-divergence — the three ways this pin could have become meaningless.

Restores verified by git hash-object against the HEAD blob rather than by the restore command's exit code, with mutations proven on disk by anchor counts before each run. That is the standard this repo asks for and it was met without being told.

Two judgement calls I want to endorse explicitly

The exact-set assertion. toEqual([...]) alongside not.toContain closes the residual routes a bare negative leaves open — a lookup index appearing under some other name, the user index being renamed, core-set drift. The negative alone would have survived all three.

The variable binding, and its comment. const canonicalLookup = { type: 'lookup', reference: 'company' } avoids TypeScript's excess-property check, and the comment explains why: the driver's own FieldDef declares only reference_to, so a fresh literal carrying reference would be rejected on the very key this case exists to record the driver ignores. ⚠️ That is worth reading twice — the type system refusing the canonical spelling is itself a symptom of #13222, not an inconvenience to route around, and documenting it here means #13222's implementer meets it already explained.

Declared correctly rather than claimed

tsconfig.json carries "exclude": ["node_modules", "dist", "**/*.test.ts"], so tsc --noEmit never reads the edited file — reported as NOT MEASURED with --listFiles counts (0 hits for the edited file, 1 for the non-test sibling as control) rather than passed off as a green typecheck. Same for the two exit=3 gates, quoted in their own words as PREREQUISITE NOT MET. skip-changeset is the right route for a test-only diff that publishes nothing.

Not landing yet

CI just started. Holding in draft until every check is completed and green — total_count grows as aggregate rows appear, so a partial read is not a pass.

⚠️ Note for whoever lands this: #13224 is currently blocked by #13158, an unrelated @objectstack/cli boot race that ejects PRs from any package. If this PR hits the same shard, the same failure is expected and is not this PR's — see my standing-down comment on #13224.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 29, 2026 14:38
@os-zhuang
os-zhuang enabled auto-merge August 29, 2026 14:38
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 29, 2026
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 29, 2026

Copy link
Copy Markdown
Contributor

Dequeued at 14:57Z with CI_FAILURE, re-queued. No test failed — measured, not assumed. One comment, as the drive-to-green posture requires; I armed this PR.

What the queue build actually reported

The dequeue notice says CI_FAILURE, but the PR's own checks are 39/39 green on head e15a908e8, and the queue build is a different commit (fbdd963fd). Reading that build's four workflow runs directly rather than inferring from the rollup:

workflow conclusion
Governed Surface Guard success
Spec Liveness Check success
Lint & Type Check cancelled — updated 14:57:07Z
CI never started — still queued, created 14:38:57Z

Nothing failed. No assertion, no gate, no test. The main CI run sat in queued for ~18 minutes without a runner picking it up, and Lint & Type Check was cancelled at exactly the dequeue timestamp — which reads as the dequeue cancelling the run, not the run causing the dequeue.

⚠️ I am stating the ordering as a reading, not a certainty: cancelled at 14:57:07Z and the dequeue at 14:57:35Z are consistent with either direction, and I cannot see the queue's own timeout decision. What is not in doubt is that no check on either commit produced a failure.

Why re-queueing is the right move here, and not a re-run in disguise

⛔ The rule against re-running to push something through exists because a red test is information. There is no red test here. cancelled and never started are the absence of a measurement, not a passed one — and equally not a failed one. Re-queueing asks for the measurement that was never taken.

⛔ I did not push an empty commit and did not close-and-reopen. mergeable_state is clean; auto-merge is re-enabled, which is what the dequeue notice itself prescribes.

If it is dequeued a second time with an actual failing check, that is a real signal and I will treat it as this PR's to root-cause.

Related, and worth connecting

This looks like a second instance of the class #13253 filed 15 minutes earlier — "Applying a label seconds after opening a PR CANCELS its in-flight CI run, and the rollup then reports failure". ⚠️ Not the same trigger: #13253 measures a PR run cancelled by a label write, and this is a merge_group run. But the tail is identical — a cancelled run surfacing to the caller as failure, which is what sent me looking for a broken test that does not exist.

This PR is a plausible carrier for the label half too: its skip-changeset label was applied additively after opening, and its head shows duplicated Auto Label / Check Changeset / Check PR Size runs (one skipped, one success each) — the signature of a cancelled-and-re-dispatched set, and why its total_count is 38 where sibling PRs today read 32. I have added the observation to #13253 rather than widening it here.


Generated by Claude Code

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

Labels

size/s 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.

The MongoDB lookup-index divergence is pinned only in an opt-in suite that runs on no CI lane — the pin cannot fail, in either direction

2 participants