Skip to content

fix(scripts): check-engine-double-contract discovers doubles whose write verb is vi.fn(...) - #8650

Merged
qq9340100 merged 4 commits into
mainfrom
claude/issue-8639-implof-call-expression
Aug 14, 2026
Merged

fix(scripts): check-engine-double-contract discovers doubles whose write verb is vi.fn(...)#8650
qq9340100 merged 4 commits into
mainfrom
claude/issue-8639-implof-call-expression

Conversation

@qq9340100

Copy link
Copy Markdown
Collaborator

Fixes#8639

check-engine-double-contract's discovery could not see an engine double whose write verb is
spelled vi.fn(async …). implOf returned null for a call-expression initializer, so
consider() returned before the sibling and shape tests ever ran and the double reached
neither the pinned listing nor the baseline — absent from both sides of a ledger that
reconciles in both directions. DISCOVERED != 0 catches a scan that breaks entirely; it cannot
catch a scan that quietly skips one spelling.

The change

implOf now unwraps a call carrying exactly one argument which is a function, via a shared
fnInitializer helper used by both initializer branches — the object-literal
(PropertyAssignment) and the class-field (PropertyDeclaration) spellings. Sharing one helper
is deliberate: the two branches previously carried the same three lines twice, which is how one
spelling got fixed and the other did not.

How wide to unwrap — measured, not assumed

Full census of the scanned corpus (every delete/update member whose initializer is a call,
310 of them, untruncated):

countshapeholds an implementation?
163vi.fn()no — no argument
89vi.fn(fn)yes
39vi.fn().mockResolvedValue(value)no — a value
9rec('DELETE')no — string argument
4vi.fn().mockImplementation(fn)yes
3record('DELETE')no
2on('DELETE')no
1vi.fn().mockRejectedValue(ERR())no

So the criterion is structural and callee-agnostic: it admits the 93 that hold an implementation
(including .mockImplementation(fn) for free and correctly) and rejects all 217 that do not,
with no allowlist of callee names that would go blind the day someone writes vitest.fn.

Two widenings were declined, both measured at zero occurrences: a function among several
arguments, and a function in a chained receiver (vi.fn(fn).mockResolvedValue(v)). A bare
vi.fn() stays undiscovered on purpose — there is no implementation to judge, so nothing could
be looser than the producer.

Discovery: 379 to 435 doubles (+56, 0 lost)

Measured by building two harnesses from the real gate source differing in implOf only.
Of the 93 verb members the fix newly makes readable:

  • 56 are engine-shaped and are now discovered (delete 176 to 202, update 203 to 233)
  • 37 are correctly vetoed — 24 with fewer than two engine siblings, 8 by the driver veto,
    5 low-arity with no engine-only sibling

The card's 168-file grep was explicitly an upper bound; 93 / 56 / 37 is the classification.

Both named control files are now discovered — and their delete was already routed through
assertEngineDeleteDispatch the whole time; the gate simply could not see it.

21 pinned, 29 entries ledgered as FIRST MEASUREMENTS

Per the "fix rather than ledger anything cheap to pin" rule, every double whose pin was a
single call was pinned: 21 across metadata-protocol, objectql, runtime and plugin-security.
All four suites run green (8,392 tests).

The remaining 29 (file, verb) pairs enter the baseline, and each entry states individually why a
one-line pin does not reach it: an implementation declaring too few parameters to hand the
predicate anything, a deliberate tripwire double whose body opens by throwing, a concise arrow
body, or @objectstack/mcp which depends on neither home of the predicate.

These are first measurements, not a raised ratchet — stated in every entry and in the
baseline's $comment. The doubles were always there; the scan could not reach them. Same act as
#5629 and #5480, both already recorded in the script's comments.

Gate: OK — 238 pinned, 162 in the DEBT ledger, 2 exempt (was 213 / 133 / 2).

Self-test fixtures, mutation-verified

Eight fixtures were added, and each was proven to actually bite by mutation — this file's own
comments record that an unfixtured arm stayed green when neutered:

mutationpredictedobserved
neuter unwrapCallImpl4 positive fixtures failexactly those 4
revert only the PropertyDeclaration branch1 fail (class field)exactly that 1
revert only the PropertyAssignment branch3 fail (object literal)exactly those 3

The class-field spelling has zero occurrences in the corpus, so that fixture is the only
possible evidence the sibling branch works — which is precisely why it is there.

Verification

  • check:engine-double-contract OK; --self-test OK
  • check:type-check-coverage --re-measureEXIT=0, nothing above its ceiling
  • typecheck green for all four affected packages
  • also run: check:nul-bytes, check:cross-package-test-inputs, check:durability-log-level,
    check:test-source-alias, check:type-source-resolution, check:query-options-erasure,
    check:i18n, check-engine-split-ratio — all green

Tests and root scripts/ tooling only; nothing publishable changes, so this carries
skip-changeset.


Generated by Claude Code

@vercel

vercelBot commented Aug 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 14, 2026 11:54am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

No hand-written docs reference the 0 changed package(s). ✅

@qq9340100Claude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPTed on substance — and ⛔ deliberately HELD AS DRAFT. Do not flip, do not enqueue, do not arm auto-merge.

Merge freeze in force. The maintainer instructed this seat directly at ~07:2xZ: 「changeset 在发版本,现在不要合并任何代码」. This PR is finished and would otherwise be flipped now. It stays draft until the maintainer lifts the freeze. ⚠️"Enough time has probably passed" is a guess, not a reading.

Four things make this an unusually strong round.

① The fix addresses why the bug existed, not just the bug

implOf's two branches "previously carried the same three lines twice, which is how one spelling got fixed and the other did not." Extracting one shared fnInitializer used by both the object-literal and class-field branches removes the duplication that produced the asymmetry. A fix that patched only the reported spelling would have left the identical trap armed one branch over — which is exactly how this card came to exist.

② The unwrap width was measured, and the criterion refuses to go blind later

A full, untruncated census of all 310 call-initializer members, classified into eight shapes. The resulting criterion is structural and callee-agnostic — one argument which is a function — admitting the 93 that hold an implementation (picking up .mockImplementation(fn) for free and correctly) and rejecting all 217 that do not.

⭐ The reasoning for rejecting a callee allowlist is the point: it "would go blind the day someone writes vitest.fn." That is this card's own defect — a matcher keyed on a spelling — refused in the fix for it. Two candidate widenings were declined with zero measured occurrences each, and a bare vi.fn() stays undiscovered on purpose because there is no implementation to judge.

③ The classification, not the grep count

93 newly readable → 56 engine-shaped and discovered, 37 correctly vetoed (24 with fewer than two engine siblings, 8 by the driver veto, 5 low-arity without an engine-only sibling). Discovery 379 → 435, 0 lost.

The card's 168-file grep was an upper bound and is explicitly named as such. Reporting it as the answer would have been the truncated-census error in a new costume; this is the measurement the card actually asked for.

⚠️ And the honest result underneath: both control files' delete was already routed through assertEngineDeleteDispatch the whole time — the gate simply could not see it. No contract violation was hiding. Saying so plainly, when a larger claim was available, is worth more than the fix.

④ First measurements, labelled where a future reader will look

21 pinned by fixing (four suites green, 8,392 tests) per "fix rather than ledger anything cheap to pin"; 29 baselined, each entry stating individually why a one-line pin cannot reach it. Stated as first measurements in every entry and in the baseline's $comment — so the claim sits where someone auditing the ledger will read it, not only in a PR body that scrolls away. Gate moves 213/133/2 → 238/162/2.

⭐ On the fixtures: the class-field spelling has zero occurrences in the corpus, so its fixture "is the only possible evidence the sibling branch works — which is precisely why it is there." Knowing that a fixture is load-bearing because the real corpus cannot exercise it is the difference between a test and a decoration. All eight fixtures were mutation-proven to bite, with predicted and observed matching exactly on all three mutations.

--re-measure EXIT=0 with nothing above ceiling — the ratchet trap that bit this lane twice today did not bite a third time.


Generated by Claude Code

@qq9340100Claude

Copy link
Copy Markdown
CollaboratorAuthor

⚠️CI red on ESLint (head bad297d) — diagnosed, and it is NOT this PR's defect. ⛔ No fix is being dispatched.

The failing step is check:changeset-gate-self-testscheck-changeset-no-major --self-test:

✗ check-changeset-no-major --self-test — 3 failure(s)
• control (#7005): the real .changeset stock must actually contain major-declaring
changesets, or the assertion below is green for the wrong reason
• reader: the real .changeset directory is non-empty — got 0 entries
• reader: the real .changeset/pre.json is readable and parses

Mechanism: that self-test carries a positive control — it refuses to pass unless the repo's real .changeset stock actually contains major-declaring changesets, precisely so the assertion beneath it cannot be green for the wrong reason (#7005). A release consumes .changeset/*.md when it versions packages, so during the release window the directory is legitimately empty (0 entries) and pre.json is absent. The control is therefore false by design of the release, not by anything in this diff.

⭐ Note the irony worth recording rather than "fixing": this is a positive control doing exactly its job. The gate is refusing to report green while its own corpus is empty — which is the discipline this lane has been enforcing all night (a measurement over an empty corpus proves nothing). It is correct behaviour that happens to be inconvenient at this moment.

Why nothing is being changed here:

Standing: the PR remains ACCEPTed on substance and held. When the freeze lifts, CI is re-read from a fresh run — ⛔ this red is not carried forward as a verdict on the change, and equally ⛔ it is not waved through: the re-run must be green on its own terms once .changeset is repopulated post-release.

Reported to the maintainer, since it is their release window and the condition is repo-wide rather than local to this branch.


Generated by Claude Code

@qq9340100
qq9340100 added this pull request to the merge queueAug 14, 2026
Merged via the queue into main with commit b803635Aug 14, 2026
27 checks passed
@qq9340100
qq9340100 deleted the claude/issue-8639-implof-call-expression branch August 14, 2026 12:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

check-engine-double-contract never discovers a double whose write verb is vi.fn(…)implOf drops a call-expression initializer

2 participants

@qq9340100@claude