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

Merged
TH
2mo ago
+642-135

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