test: pin selector-port behavior ahead of the P5 extraction (#1478)

[thymikee]

Summary

Step 2 of the approved P5 sequence for #1478 (issue comment 5156017698): pins, at existing root seams, the eight behavior cells the future packages/ad-replay selector port (readSelectorExpression / resolveRecordedTarget / buildSelectorCandidates) must preserve. Test-only — no production code changes (git diff main touches only *.test.ts files).

Discipline followed (docs/agents/testing.md): every test below was proven load-bearing by temporarily inverting the exact production condition it pins, watching it fail, and restoring the original code. Each cell's counterfactual — the mutation, the failure observed, and confirmation of the restore — is recorded below.

Update after review: cells 5 and 7 were consolidated to remove duplicated coverage against pre-existing tests (see PR comment). Their entries below reflect the final locations and were re-verified against their counterfactuals post-move.

The 8 cells

Cell 1 — invalid selector vs valid-but-no-match

Lives in: src/selectors/__tests__/selector-port-contract.test.ts
Pins that tryParseSelectorChain (parse failure → null, before any resolution) and resolveSelectorChain (valid chain, zero matches → null) are two distinct, non-interchangeable failure shapes reached through different functions.
Counterfactual: In src/selectors/parse.ts, changed if (!ALL_KEYS.has(key)) to if (false) so an unknown selector key like foo=bar parses instead of throwing. tryParseSelectorChain('foo=bar') returned a real SelectorChain object instead of null — test failed on assert.equal(invalid, null). Restored; test green again.

Cell 2 — fallback-alternative selection

Lives in: src/selectors/__tests__/selector-port-contract.test.ts
Pins that id="missing" || label="Save" resolves via the second alternative (selectorIndex: 1) when the first has zero matches.
Counterfactual: In src/selectors/resolve.ts, changed the alternative loop to chain.selectors.slice(0, 1).entries() so only the first alternative is ever tried. The resolution returned null instead of the second alternative's winner — test failed. Restored; test green again (cells 1/3/4 in the same file were unaffected by this change, confirming isolation).

Cell 3 — ambiguity with and without disambiguation

Lives in: src/selectors/__tests__/selector-port-contract.test.ts
Pins that the SAME two-way-tied match resolves to null with disambiguateAmbiguous unset, and to a disclosed winner (tiebreak: 'deepest', alternatives: ['e1']) with it enabled.
Counterfactual: In resolve.ts, changed if (!options.disambiguateAmbiguous || !summary.disambiguated || !summary.tiebreak) continue; to drop the !options.disambiguateAmbiguous clause, forcing disambiguation on unconditionally. The "without a tiebreak" half of the test failed (withoutTiebreak was no longer null). Restored; test green again.

Cell 4 — requireRect

Lives in: src/selectors/__tests__/selector-port-contract.test.ts
Pins that a matching, rect-less node is excluded when requireRect: true and included when false, consistently between resolveSelectorChain and listSelectorChainMatches.
Counterfactual: In resolve.ts's analyzeSelectorMatches, removed the if (requireRect && !node.rect) continue; guard. The rect-less node resolved even with requireRect: true — test failed. Restored; test green again.

Cell 5 — winner and matched-node domain from the same alternative

Lives in: src/daemon/handlers/__tests__/session-replay-target-classification-port.test.ts (2 tests, sharpened per review to stop overlapping an existing regression)
Pins that classifyReplayTarget's matchCount/domain always comes from the SAME chain alternative resolveSelectorChain picked as the winner — never a domain leaked from an alternative resolution skipped over. session-replay-target-classification.test.ts already has a same-invariant regression, but its ambiguous first alternative is an EXACT tie (unresolvable regardless of allowDisambiguation), so it only exercises the !summary.disambiguated branch of resolveSelectorChain's skip condition. The two tests here instead hold ONE fixture fixed — a first alternative that IS resolvable, proven by the second test succeeding through it — and flip only allowDisambiguation, isolating the OTHER branch (!options.disambiguateAmbiguous): off, classification must use the second alternative's own domain (1), never leak the first's larger unused domain (3); on, the reverse (3, not the untried second alternative's 1).
Counterfactual (re-verified post-sharpening): In src/daemon/handlers/session-replay-target-classification.ts's resolveSelectorTargetMatches, changed the domain filter from matchesSelector(node, resolved.selector, platform) to matchesSelector(node, chain.selectors[0]!, platform) (always the first alternative). The allowDisambiguation: false case failed red (matchCount 3 instead of the expected 1), as did the pre-existing repo regression classifyReplayTarget uses the later chain alternative that resolution selected after an earlier tie. The allowDisambiguation: true contrast case correctly stayed green (its own resolved alternative IS chain.selectors[0], so the mutation is a no-op for it — confirming it's genuinely the contrast case, not a second copy of the mechanism proof). Restored; all green again.

Cell 6 — shared-ID demotion

Lives in: src/replay/__tests__/target-identity-node.test.ts (new file — idMatchCountInTree/demoteNonUniqueLocalIdentity had no direct unit test before, only indirect coverage through src/selectors/build.test.ts's chain-building consumer)
Pins the shared uniqueness predicate itself: idMatchCountInTree counts every node sharing a canonical (NFC, byte-capped) id; demoteNonUniqueLocalIdentity drops ONLY the id tier, and only when it's non-unique, leaving role/label untouched.
Counterfactual: Changed idMatchCountInTree to return 1; unconditionally. Both my new tests and the pre-existing src/selectors/build.test.ts (a second, independent consumer of the same predicate) failed red. Restored; both green again — confirming the cross-invariant that record-time evidence and the selector chain demote in lockstep through this ONE shared function.

Cell 7 — wait-landmark identity mismatch (#1349)

Lives in: src/commands/interaction/runtime/selector-wait.test.ts (relocated per review from selector-read.test.ts, which is the 1:1 topology location for selector-wait.ts; 4 tests — runtime wait keeps polling past a same-selector impostor and succeeds on the recorded landmark, runtime wait fails closed at the deadline when only impostors matched the selector, runtime wait with a recorded landmark keeps the plain timeout when the selector never matched, runtime wait without a recorded landmark returns the satisfying match for record-time evidence)
Pins that a landmark wait only succeeds once a poll's match carries the recorded identity (not on the first same-selector "impostor" match, and not before an initial no-match poll), refuses at the deadline with WAIT_LANDMARK_MISMATCH_REASON plus the correct matchCount/observed/observedAncestry evidence when every poll is an impostor, and keeps an undifferentiated plain timeout when the selector never matches at all (so the two failure modes stay distinguishable).
Counterfactual (re-verified post-move): In src/commands/interaction/runtime/selector-wait.ts's resolveLandmarkMatch, changed if (!recorded) return { kind: 'satisfied', node: firstMatch }; to if (true) return ... so any match satisfies the wait regardless of identity. At the new location, both landmark tests failed red (runtime wait keeps polling past a same-selector impostor... failed on waitedMs >= 600; runtime wait fails closed at the deadline... failed on error instanceof AppError), and the pre-existing daemon-level regression a replayed wait with a landmark guard refuses at the deadline when only impostors matched (wait-landmark-recording.test.ts) failed red too. Restored; all green again.

Cell 8 — repair-suggestion ordering

Lives in: src/daemon/handlers/__tests__/session-replay-divergence-suggestion-port.test.ts (new file, targeting the previously-untested exported buildReplayDivergenceSuggestionForNode)
Pins that a divergence/repair suggestion's selector string preserves buildSelectorChainForNode's priority order (id, then role+label, then label, then value) verbatim, and that a non-unique id (per cell 6's demotion) never appears in the suggestion at all.
Counterfactual: In src/selectors/build.ts, moved the value push block to run before the role+label block. The exact-order string assertion failed (value="Draft" appeared second instead of fourth) — a case build.test.ts's existing fixtures don't exercise since none of them carry all four tiers at once. Restored; test green again.

Verification

• git diff main --stat — only *.test.ts files changed, zero production code.
• pnpm typecheck && pnpm lint && pnpm format:check && npx vitest run <the 9 touched files> — all green.
• Full npx vitest run (foreground, synchronous): 637/637 test files, 5300/5300 tests passed, zero failures.

Test plan

• All touched test files pass in isolation
• Full unit suite passes (637 files / 5300 tests)
• typecheck / lint / format:check all green
• Every new/sharpened assertion counterfactually proven against the exact production line it pins, re-verified after the cell 5/7 consolidation

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.93 MB	1.93 MB	0 B
JS gzip	618.1 kB	618.1 kB	0 B
npm tarball	736.4 kB	736.4 kB	0 B
npm unpacked	2.58 MB	2.58 MB	0 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	25.9 ms	26.9 ms	+1.1 ms
CLI --help	63.5 ms	63.8 ms	+0.3 ms

Top changed chunks: no changes in the largest emitted chunks.

[thymikee]

P2 — consolidate the P5 cell-7 wait-landmark pins instead of duplicating them. The new cases in selector-wait.test.ts exercise the same device.selectors.wait entry point, recorded-landmark ancestry mismatch, eventual success, deadline refusal reason, and matchCount: 1 already covered by selector-read.test.ts (runtime wait keeps polling past a same-selector impostor… / fails closed at the deadline…). The existing tests are stronger: they derive real evidence, include an initial no-match poll, assert observed ancestry, and distinguish plain timeout. The stated counterfactual already fails those tests. Relocate/annotate the existing cases into the 1:1 selector-wait.test.ts topology and delete the originals, or reuse them in place; do not keep both 122-line copies. Cell 5’s first case also overlaps the existing later-alternative regression and should be tightened in the same pass. The other behavior pins are valid and all exact-head CI is green; no device evidence is needed for this test-only PR. Withhold ready-for-human until the duplicate coverage is consolidated and focused tests pass.

[thymikee]

Consolidated per review.

Cell 7 (wait-landmark): dropped my two duplicate cases in selector-wait.test.ts and relocated the four existing #1349 landmark tests from selector-read.test.ts into selector-wait.test.ts — the 1:1 topology location for selector-wait.ts (where resolveLandmarkMatch/waitForSelector actually live). Deleted the originals; kept their stronger assertions verbatim (real computeTargetEvidence-derived evidence, an initial no-match poll before the impostor, observed/observedAncestry checks, and the plain-timeout-vs-landmark-mismatch distinction). selector-read.test.ts keeps a local copy of the small landmarkScreen snapshot builder for its one remaining, unrelated "rides out an unreadable capture" test, which isn't a landmark test at all — everything else in that block moved.

Cell 5 (winner/domain same alternative): the first case did overlap session-replay-target-classification.test.ts's existing "uses the later chain alternative..." regression on the surface, but it's the ONLY case in the file sensitive to the counterfactual (the second case resolves through the first alternative already, so forcing the domain onto chain.selectors[0] is a no-op for it). Dropping it would have left cell 5 with zero mechanism-level proof. Instead I sharpened it: the existing regression's ambiguous alternative is an exact tie, unresolvable regardless of allowDisambiguation, so it only exercises resolveSelectorChain's !summary.disambiguated skip branch. My case now holds the same fixture as the second case (a first alternative that IS resolvable, proven by the second case succeeding through it) and isolates the OTHER skip branch, !options.disambiguateAmbiguous. The two tests are now an explicit same-fixture, flag-flipped pair rather than two independent scenarios.

Re-ran both counterfactuals after the move:

• Cell 7: broke resolveLandmarkMatch (if (!recorded) return {...} → if (true) return {...}) — both relocated tests failed red, plus the pre-existing wait-landmark-recording.test.ts regression. Restored, all green.
• Cell 5: broke the domain filter in resolveSelectorTargetMatches (resolved.selector → chain.selectors[0]!) — the sharpened first case failed red (matchCount 3 instead of 1), plus the pre-existing classification regression; the second (contrast) case correctly stayed green, confirming it isn't redundant with the first. Restored, all green.

Gates (typecheck && lint && format:check && vitest run <touched files>) and a full foreground vitest run (637/637 files, 5300/5300 tests) both pass on the consolidated branch. PR body's per-cell table updated to point at the final locations.

Generated by Claude Code

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-02 10:26 UTC