Repository navigation
test(runtime): isolate per-repo worktree scan expiry from the shared timer queue - #22575
Conversation
…timer queue The TTL case was advancing every fake timer, so a scan an earlier test had left scheduled was counted as this repo's rescan.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe scan-expiration test now stubs Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The updated test checks repository scan expiry without a demonstrated merge-blocking issue. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the problem, change, rationale, testing, and UI impact. However, it does not provide the required linked issue. It states "No ticket" even though the template requires an issue link.
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — one test-only file, the worktree scan cache: expires scans per repo without coupling sibling repos case in lineage-and-scan-cache-part-03.spec.ts.
- Clock-driven expiry instead of timer flush — swaps
vi.advanceTimersByTimeAsyncforvi.setSystemTime, so the case no longer flushes the shared fake-timer queue that earlierorca-runtime-testsparts share throughorca-runtime.test.ts. Detects the reported/tmp/repoextra scan without running unrelated timers. - Fingerprint stub —
vi.spyOn(readRepoWorktreeAdminFingerprint).mockResolvedValue(null)(restored infinally) so a real git dir under/tmp/repo-a//tmp/repo-bcannot prove "unchanged" and skip the rescan at the boundary. - Stricter assertion — replaces
toHaveBeenCalledTimes(3)+nth(3, '/tmp/repo-a')with the exact scan order['/tmp/repo-a', '/tmp/repo-b', '/tmp/repo-a'].
The change is sound: the cache checks expiresAt > Date.now() at read time (orca-runtime-list-known-resolved-worktrees-for-explicit-target.ts:159), so setting the clock expires the entry without a scheduled timer; the strict order would fail if the per-repo TTL regressed or the TTL moved off 30s in either direction; and vi.useRealTimers() in finally discards any stragglers rather than leaking them. Verified locally via pnpm exec vitest run src/main/runtime/orca-runtime.test.ts -t "expires scans per repo without coupling sibling repos" (passed).
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
ELI5
A unit test that checks each repo's worktree list expires on its own was sometimes failing in CI because it also counted a scan from a different test.
What Changed
The test still checks that a 30 second cache expires for one repo while its sibling stays cached. It no longer does that by running every pending timer in the process.
Before, moving the clock forward also ran timers left behind by earlier tests in the same file. One of those timers listed a different repo (
/tmp/repo) and the assertion saw four scans instead of three. On a machine where that path is a real git repo, the same test could also skip the rescan, because an unchanged admin fingerprint is allowed to extend the cache.After, the clock is set directly, the fingerprint check is stubbed so it cannot prove the repo unchanged, and the assertion is the exact scan order: repo A, repo B, repo A again.
Why
The cache already stores an expiry per repo. The flake was the test driving that check by flushing the shared timer queue, which is full of unrelated work by the time this case runs. Setting the clock, and stubbing the fingerprint, measures the expiry rule without those side paths. Counting only this case's paths would still pass if the stray scan happened; not running the queue means the stray scan does not happen inside the test.
Linked Issue
No ticket. This is the shard 8 failure on #22564 (
expected "vi.fn()" to be called 3 times, but got 4 times).Visual Proof
N/A. Test-only change; nothing in the app UI changes.
Testing
pnpm exec vitest run src/main/runtime/orca-runtime.test.ts— 1298 passed, 1 skipped. Before the fix, that same file failed this case with an extra/tmp/reposcan at fake time 14s while the clock was being advanced 20s.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)