fix(test): pin PR-fix comment footer clock and drop brittle "99" guard (#1257)

[itsmiso-ai]

Closes #1257.

The "renders workerExecutions and enqueueRows separately, never as 'attempts' (#1218)" case in src/lib/pr-fix-surfacing.test.ts asserted the legacy totalAttempts: 99 value by name with not.toContain("99"). The neighbouring not.toContain("Total attempts") label guard already covers the leak; the value-based guard collided with the rendered footer timestamp whenever the wall-clock millisecond field contained two consecutive nines (~5% of runs), so the suite flaked whenever the rendered new Date().toISOString() ended in e.g. 2026-10-14T05:47:20.799Z. One full-suite run failed on this case on 2026-10-14 during PR #1247 validation, while three targeted reruns of the file passed without any code change.

Fix:

• Freeze the footer timestamp with vi.useFakeTimers + vi.setSystemTime in the affected case so the rendered body is byte-stable across wall-clock millisecond values (Assert on the rendered legacy field instead of a bare "99" so the PR-fix comment test stops failing on the footer timestamp #1257).
• Drop the brittle value-based guard; keep the label-level guard which is both correct and sufficient.

Scope:

• src/lib/pr-fix-surfacing.test.ts only.

Local validation:

• npx vitest run src/lib/pr-fix-surfacing.test.ts: 44/44 passing — repeated 15× to confirm determinism across varying wall-clock values.
• npx vitest run (full suite): 4486/4486 passing (16 skipped + 1 todo pre-existing) — 182 files.
• npm run typecheck: clean.
• npm run lint: clean for the change (one pre-existing no-location-assign-relative-destination warning at src/app/login/page.tsx:39, unrelated).

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3.1-Flash-Preview (anthropic) — primary route · pr-reviewer-action v3.5.0

Recommendation: approve — no merge-blocking findings. The change removes the timestamp-dependent bare-value assertion while freezing the clock and retaining a meaningful label-level regression guard.

Change-by-change assessment

• vi.useFakeTimers() plus vi.setSystemTime(...) makes the new Date().toISOString() footer deterministic at 2026-01-01T00:00:00.000Z for this test.
• Restoring real timers in finally prevents fake-timer leakage if construction or an assertion fails; this is consistent with existing timer usage elsewhere in the repository.
• Dropping not.toContain("99") removes the false positive against millisecond values while retaining not.toContain("Total attempts"), which detects the expected legacy metric label.
• Production tracing confirms formatMetrics renders workerExecutions and enqueueRows as separate fields and never reads context.totalAttempts; buildNeedsHumanComment is the consumer that appends the clock-derived footer.

Claims checked

• Claim 1: held. The two intended metrics render independently, and the brittle bare 99 assertion is gone.
• Claim 2: unverified. The historical one-failure/three-rerun account is not independently available in the supplied artifacts.
• Claim 3: held, 1/1 scope item. PR metadata and the diff confirm that only src/lib/pr-fix-surfacing.test.ts changed.
• Claim 4: partially corroborated but not fully verified. The corpus contains successful Tests, Lint, and Typecheck checks at the exact head, but not the author's exact 44/44, 4486/4486, or 15-run output.
• Claim 5: held. Current production code ignores the legacy field, and the surviving assertion checks its rendered label without depending on an arbitrary numeric value.

Overall: held/verified for 3/5 claims; 2/5 are unverifiable historical or exact-run reports. No verified claim was contradicted.

Linked Issue Fit

• The bare 99 guard is removed as requested.
• A fixed clock is installed before rendering, with cleanup guaranteed by finally.
• The test no longer depends on wall-clock milliseconds; every invocation uses the same frozen footer time, and the exact-head Tests check succeeds.
• The legacy-field regression remains guarded through Total attempts.
• Scope remains test-only, matching issue PR 1257; no production seam was added or needed.

No acceptance criterion or issue requirement is missing or misinterpreted.

Standards Compliance

• The relevant AGENTS.md gates are satisfied: exact-head CI reports both Lint and Typecheck successful.
• The changed test does not read, log, echo, or persist credentials, so the token-confidentiality requirement is unaffected.
• No manifest, dependency, database, API, or generated-output change is present.
• The timer setup and teardown are internally consistent and do not leak state to sibling tests.

Sources

• PR PR 1271 metadata and diff.
• Linked issue PR 1257.
• src/lib/pr-fix-surfacing.test.ts at head 577e27544fecb732d423244eee36b19b4444cb86.
• src/lib/pr-fix-surfacing.ts at the same revision, including formatMetrics, PrFixSurfaceContext.totalAttempts, and the footer construction.
• Exact-head CI results for Tests, Lint, Typecheck, and the other repository checks.
• Repository AGENTS.md standards.

Tool Harness Findings

• All 11 read-only harness calls completed successfully against the exact head.
• Direct reads confirmed the changed test, the fixed timestamp, finally-based timer restoration, the retained label assertion, and the production renderer's independent metric formatting.
• Targeted repository searches found no production rendering of totalAttempts or Total attempts; references are limited to the compatibility field declaration, comments, and tests.
• The timer-pattern search found the same useFakeTimers/useRealTimers convention in multiple existing tests, supporting consistency with repository practice.
• The numeric not.toContain search found no equivalent guard in the relevant PR-fix surfacing code.

Unknowns or Needs Verification

The author's exact historical run counts and repeated-run transcript are not present as independently verifiable artifacts. This does not create an observed head failure: the supplied exact-head Tests check succeeds, and the fake clock deterministically removes wall-clock variance.

[joryirving]

Approved on exact head 577e275. This is a correctly scoped test-only fix for #1257: it removes the brittle not.toContain("99") assertion, preserves the meaningful not.toContain("Total attempts") guard, pins the footer date in the affected case, and restores real timers in finally. Independently checked that production formatMetrics reads workerExecutions and enqueueRows, not the deprecated totalAttempts, and that the comment footer uses new Date().toISOString(). Exact-head CI, security audit, image build, PR Smoke, and AI Review are green. No blocking findings.