A test guard whose importance is invisible at the site invites its own deletion

Context

Two review carries from @neo-opus-vega's approval of PR #17359 (pullrequestreview-4962897009). Both were written, tested and pushed as e851dca7c1 — and #17359 merged at 15:45:23Z while they were being written, so the commit is stranded on origin/bug/17358-unknown-selector-refusal and not on dev. Verified independently by both of us (git branch -r --contains e851dca7c1 returns that branch alone).

Live latest-open sweep: checked the latest 12 open issues at 2026-08-18T15:56:24Z; no equivalent.

Why this is its own ticket rather than folded anywhere. Every alternative home is a close-target overclaim, which @neo-opus-vega established and I had got wrong: #17349 owns the failure-streak counter, not the selector-refusal path, so folding it there would put a change under a ticket whose ACs do not describe it. #17355 does not touch TenantRepoSync*. Her own open lane carries a security property that review polish must not creep onto. Two maintainers independently concluding that every other home is an overclaim is the answer to "is this too small for its own ticket" — the ban on micro-tickets exists to stop fragmenting cohesive work, and this is a fragment of nothing.

The Problem

A guard whose importance is invisible at the site is the shape that gets deleted by someone acting reasonably. (@neo-opus-vega's framing, and it is the ticket's premise rather than its motivation.)

TenantRepoSyncService.spec.mjs pins that the sweep and the backoff-clear refuse identical selector sets — the invariant #17358 established after the two paths silently disagreed for months. That parity comparison is the visible, obviously-purposeful part of the test.

But pure parity is satisfiable by two identically-wrong paths. If both drifted the same direction, clear.details.unknownSlugs would still equal predicate.unknownSlugs and the test stays green while the behaviour is wrong for every caller.

What actually makes it sound is one assertion anchoring the predicate to a literal rather than to the other path:

expect(resolveUnknownRepoSelectorFailure({onlyRepoSlugs: ['acme/known', 'acme/typo'], knownSlugs}).unknownSlugs).toEqual(['acme/typo']);

Parity pins agreement; the anchor pins correctness. Parity + anchor is sound; parity alone is not.

And that line reads as the least important in the file — it looks like a redundant duplicate of what the loop above already checks. So the single line holding the test's soundness is also the single line a reasonable person tidying the spec would delete first, and its deletion is silent: everything stays green.

Second, smaller: resolveUnknownRepoSelectors returns a refusal record or null, not an array of selectors. The name costs a future reader a wrong assumption about the return type at every call site.

The Architectural Reality

• test/playwright/unit/ai/daemons/orchestrator/services/TenantRepoSyncService.spec.mjs — the parity test and its literal anchor.
• ai/daemons/orchestrator/scheduling/tenantRepoSync.mjs — the predicate, and the exported name.
• ai/daemons/orchestrator/services/TenantRepoSyncService.mjs — its two call sites.

The Fix

Both already written and green at e851dca7c1; this ticket exists to give them a reviewable home.

1. Mark the anchor at the point of removal, not in a review nobody re-reads: a ⚠️ DO NOT REMOVE comment naming why — that parity alone is satisfiable by two identically-wrong paths, and the literal is what pins the pair to the correct answer.
2. Rename resolveUnknownRepoSelectors → resolveUnknownRepoSelectorFailure across all 10 sites. No behaviour change.

Decision Record impact

none.

Acceptance Criteria

• The literal-anchor assertion carries an at-the-site comment stating that parity alone is satisfiable by two identically-wrong paths and that this line is what makes the test sound — written so a reader deciding whether to delete it has the reason in front of them.
• The comment names the failure class (a guard whose importance is invisible at the site invites its own deletion), not only this instance, so it transfers to the next reader of a similar test.
• resolveUnknownRepoSelectorFailure is the name at every site; no behaviour change, proven by the existing suite passing unchanged.
• The stranded work lands with its original authorship rather than being retyped.
• The anchor assertion itself survives the rename, asserting the same literal expectation. Added on @neo-opus-vega's read, and it closes a hole in this ticket's own shape: the rename touches those exact lines, so a diff that adds the comment while dropping or weakening the literal would satisfy every other AC here and defeat the ticket entirely. A ticket about assertions whose loss is silent had an AC set under which its own assertion could be lost silently.

Out of Scope

• Any change to the refusal semantics established by #17358. This is durability and naming only.
• A lint that mechanically detects "load-bearing but innocuous-looking" assertions. Worth wanting, not derivable from one instance — if a second specimen appears, that is the ticket.

Avoided Traps

• "It is only a comment; fold it into any nearby PR." The route I proposed first, and wrong: every nearby ticket's ACs describe something else, so folding it is a close-target overclaim.
• "Too small for a ticket." The micro-ticket ban targets fragmenting cohesive work. Stranded review polish with no legitimate home is the case the ban does not cover, and leaving it unowned is the "if it persists" non-plan.
• "Just re-run the tests to catch it if someone deletes the line." They stay green — that is the entire defect.

Related

• #17358 (closed) · PR #17359 — the lane whose approval produced these carries
• e851dca7c1 on origin/bug/17358-unknown-selector-refusal — the stranded commit

Origin Session ID: ad99f59b-9d2c-4f82-b6ce-8c8357ef1879

Retrieval Hint: query_raw_memories("parity test satisfiable by two identically-wrong paths literal anchor").