Skip to content

fix(scan): rerun a scan that was requested while one was running - #20

Merged
luwill merged 1 commit into
luwill:mainfrom
amenjason936:fix/scan-rerun-dropped
Oct 20, 2026
Merged

luwill merged 1 commit into
luwill:mainfrom
amenjason936:fix/scan-rerun-dropped

Conversation

@amenjason936

Copy link
Copy Markdown
Contributor

Problem

scanAll() returns { skippedConcurrent: true } when a scan is already running, and the request is then lost. The watcher's debounced scan can fire while the previous scan is still in progress; at 5000 Codex files, an idle scan takes ~0.5–1 s. If the running scan has already passed the file that just changed, that change is only picked up by the 60 s interval scan. This typically happens at the end of a burst of agent activity: the last write lands mid-scan and nothing follows it. The dashboard and the menubar then show stale numbers for up to a minute.

Fix

  • A request that arrives during a scan sets _rescan. When the running scan finishes, it schedules one follow-up scan through the normal debounce (_scheduleScan). Going through the debounce keeps continuous writes at the current scan rate instead of turning them into back-to-back scans.
  • stop() sets _stopped, so a scan that finishes after shutdown does not schedule anything. startWatching() clears the flag.
  • scanAll() keeps the same return values. CLI callers (scan, today, doctor) never call it concurrently, so they are unaffected.

Tests

test/scan-rerun.mjs:

  • Starts a scan, writes a new rollout, and calls scanAll() again, which is skipped. It then waits for a follow-up scan, at most 5 s; in practice ~0.8 s, the debounce time. Finally it asserts that every rollout was ingested. This case fails on main with "no follow-up scan within 5 s".
  • With no mid-scan request, no follow-up is scheduled.
  • stop() during a scan cancels the follow-up.

Full npm test is green. This PR is independent of #16–#19; the only overlap is the one-line registration in test/run.mjs.

🤖 Generated with Claude Code

scanAll() returned { skippedConcurrent: true } and forgot the request.
When a watch event's debounced scan landed during a running scan, and
that scan had already passed the changed file, the change waited for
the 60 s interval scan. The dashboard and menubar lagged by up to a
minute after the last write of a burst.

Now a mid-scan request sets a flag, and the scan that finishes
schedules one debounced follow-up. Going through the debounce keeps
continuous writes from turning into back-to-back scans. stop() cancels
the follow-up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@luwill

luwill commented Oct 20, 2026

Copy link
Copy Markdown
Owner

Reviewed and verified in a clean worktree — LGTM.

  • The lost-update scenario is real (last write of a burst lands mid-scan after its directory was walked → waits for the 60 s interval), and the fix converges correctly: requests arriving mid-scan coalesce into a single _rescan, the follow-up goes through the normal debounce so continuous writes don't turn into back-to-back scans, and a stop() during a scan leaves no timer behind (_scheduleScan is gated on _stopped, startWatching() clears it). The return contract is unchanged, so CLI callers are unaffected.
  • The regression test reproduces the failure mode honestly — it fails on main with "no follow-up scan within 5 s" — and the no-request and stop-during-scan cases pin the behavior from the other side.
  • Full npm test green locally; CI green.

Same note as on #19: the two PRs conflict in src/scanner.js (constructor fields, post-finally region) in addition to the test/run.mjs line, so whichever merges second needs a small scanner.js rebase.

@luwill
luwill merged commit 5cab18b into luwill:main Oct 20, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants