Skip to content

fix(watcher): prune sustained missing roots#738

Closed
pcristin wants to merge 1 commit into
DeusData:mainfrom
pcristin:fix/watcher-stale-root-cleanup
Closed

fix(watcher): prune sustained missing roots#738
pcristin wants to merge 1 commit into
DeusData:mainfrom
pcristin:fix/watcher-stale-root-cleanup

Conversation

@pcristin

@pcristin pcristin commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Implements the watcher stale-root cleanup discussed in #286:

  • Track consecutive missing project roots in the watcher.
  • Preserve projects through transient missing-root checks, then prune after sustained absence.
  • Delete cached project DB/WAL/SHM files when sustained absence is confirmed.
  • Cover the transient-miss and sustained-cleanup behavior with a regression test.

Fixes #286.
Supersedes #666 after refreshing the branch/title/commit metadata to match the contribution guide.

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects unsigned commits.
  • Tests pass locally (make -f Makefile.cbm test).
  • Lint passes (make -f Makefile.cbm lint-ci).
  • New behavior is covered by a test (reproduce-first for bug fixes).

Validation

  • bash scripts/check-no-test-skips.sh
  • git diff --check
  • make -f Makefile.cbm build/c/test-runner
  • build/c/test-runner was run after rebasing; the watcher regression passed. The full local runner still had unrelated local/environment failures.

@pcristin pcristin requested a review from DeusData as a code owner July 1, 2026 12:31
@DeusData DeusData added bug Something isn't working ux/behavior Display bugs, docs, adoption UX priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Jul 1, 2026
@DeusData

DeusData commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Thanks for reopening this as a clean replacement for #666. DCO and CI are green, and the PR now has a cleaner title/body/history. I’ve added the same triage labels and moved this into the maintainer review queue.

@pcristin pcristin force-pushed the fix/watcher-stale-root-cleanup branch from 7e42852 to 2bd8f7b Compare July 2, 2026 12:30
@DeusData

DeusData commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Thank you — this is the right mechanism for #286 and complements #537 (just merged) perfectly: event-driven unwatch for explicit deletion, your polling prune for silently vanished roots; neither covers the other's case. Two changes before merge, both about the delete being irreversible:

  1. Count only ENOENT/ENOTDIR as 'missing' — today any stat failure qualifies, so an EACCES/EIO blip (network mount, macOS TCC revocation, ejected volume) is indistinguishable from deletion.
  2. Lengthen the window: 3 consecutive polls can be ~15 seconds, and the .db contains the user-authored ADR — unrecoverable content. Feature Request: Automatic deletion of project when underlying folder is deleted #286's thread explicitly asked for tolerance of transient unmounts; a minutes-scale sustained-absence threshold would honor that.
    Also heads-up: you'll need a small rebase (test_watcher.c moved under fix: unwatch deleted projects to prevent zombie reindex #537). Optionally route the prune through fix: unwatch deleted projects to prevent zombie reindex #537's new deferred-free list for a single freeing discipline. Thanks again — the reset-on-reappearance handling is nicely done.

Update: to keep momentum on the bug backlog, we're going to carry the changes above over the line ourselves shortly — a distilled follow-up on current main implementing the notes in this thread, with you credited as Co-authored-by on the commit, and this PR closed referencing it. If you'd prefer to push the update yourself, just reply within the next couple of days and we'll gladly take yours instead. Thanks again for the contribution!

Signed-off-by: pcristin <xxxokzxxx@protonmail.com>
@pcristin pcristin force-pushed the fix/watcher-stale-root-cleanup branch from 2bd8f7b to 0d1f60d Compare July 3, 2026 19:12
@pcristin

pcristin commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review. I pushed the requested update in 0d1f60d:

Local validation is green for git diff --check, bash scripts/check-no-test-skips.sh, and make -f Makefile.cbm build/c/test-runner; the watcher section passes in the full runner.

@DeusData

DeusData commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Thank you — this was the right mechanism for #286 and the perfect complement to the unwatch-on-delete fix: polling prune is the only way to catch silently vanished roots. We carried it over the line as 836f90d (PR #817) with you credited as co-author, with the data-loss hardening from the review folded in: only ENOENT/ENOTDIR count as missing (an EACCES/EIO blip on a network mount resets the streak instead of deleting the DB), pruning additionally requires a 10-minute sustained-absence grace window (env-tunable), and the prune's entry removal now routes through the deferred-free discipline the unwatch fix introduced — one consistent freeing model. Your restore-reset behavior carried over as-is. Closes #286. Closing this in favor of the distill — thanks again!

@DeusData DeusData closed this Jul 4, 2026
pull Bot pushed a commit to MrDolphin/codebase-memory-mcp that referenced this pull request Jul 4, 2026
…grace window)

Distilled from DeusData#738: the watcher now prunes a watched project whose root
directory has genuinely disappeared - deleting the cached DB (+wal/shm,
validated name, cache-dir-only paths) and removing the watch entry - so
vanished worktrees stop being watched forever (DeusData#286).

Hardened beyond the original PR to remove a data-loss hazard (the cached
DB can hold user-authored data such as the ADR, unrecoverable once
deleted):

- Only ENOENT/ENOTDIR stat failures count as missing. Any other failure
  (EACCES, EIO, transient mounts, macOS TCC revocation) resets the
  streak and logs watcher.root_stat_error with the errno; Windows
  (mingw/UCRT) maps not-found to ENOENT so the check holds there.
- Pruning requires BOTH >=3 consecutive missing polls AND a sustained-
  absence grace window since the streak's first miss (default 600s,
  override via CBM_WATCHER_PRUNE_GRACE_S), tracked with monotonic
  cbm_now_ms.
- Root reappearance resets streak + timestamp (watcher.root_restored).
- The pruned entry is released via the deferred-free list poll_once
  drains (one freeing model shared with cbm_watcher_unwatch).

Limitation: only currently-watched projects are pruned; stale DBs left
by older sessions are out of scope.

Co-authored-by: pcristin <xxxokzxxx@protonmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority/normal Standard review queue; useful PR with ordinary maintainer urgency. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Automatic deletion of project when underlying folder is deleted

2 participants