Skip to content

fix(ui): unwatch projects after UI delete#830

Closed
SyntaxSawdust wants to merge 2 commits into
DeusData:mainfrom
SyntaxSawdust:codex/issue-803-ui-delete-unwatch
Closed

fix(ui): unwatch projects after UI delete#830
SyntaxSawdust wants to merge 2 commits into
DeusData:mainfrom
SyntaxSawdust:codex/issue-803-ui-delete-unwatch

Conversation

@SyntaxSawdust

Copy link
Copy Markdown
Contributor

Summary

  • Wire the UI HTTP server to the existing watcher so project lifecycle actions can update subscriptions.
  • Unwatch projects when DELETE /api/project successfully deletes the DB or finds a valid project DB already missing.
  • Add focused HTTP regressions for success, already-missing DB, no-watcher behavior, missing name, invalid name, and unlink failure.
  • Harden watcher unwatch bookkeeping so pending-free allocation failure leaves the watch entry intact.

Why

Fixes #803.

The UI delete path removed project database files but left the watcher entry alive, so a later watch cycle could reindex the deleted project and recreate it. The MCP delete path already handled watcher cleanup; this applies the same lifecycle boundary to the UI HTTP route.

Tests

  • make -f Makefile.cbm build/c/test-runner
  • build/c/test-runner httpd watcher (92 passed)
  • git diff --check

Scope

  • Keeps 400 for missing names and no watcher cleanup.
  • Keeps invalid project names as 404 without watcher cleanup.
  • Keeps primary DB unlink failures as 500 without watcher cleanup.
  • Does not change project discovery, indexing, or MCP delete behavior.

Caveats

  • The watcher out-of-memory path is review-covered rather than fault-injection tested to avoid adding public test-only hooks.
  • Full make -f Makefile.cbm test and make -f Makefile.cbm lint-ci were not run locally; focused affected C suites were rebuilt and passed.

@SyntaxSawdust SyntaxSawdust marked this pull request as ready for review July 3, 2026 23:29
@SyntaxSawdust SyntaxSawdust requested a review from DeusData as a code owner July 3, 2026 23:29
@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 4, 2026
@DeusData

DeusData commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Thanks for the UI delete/watch cleanup for #803. Triage: UX/project-lifecycle bug.

This PR is currently conflicting, so the first step is a rebase on current main. After that, review will check that UI delete mirrors the MCP delete lifecycle boundary, and that watcher unwatch bookkeeping stays correct on success, already-missing DBs, invalid names, and unlink failures.

@DeusData

DeusData commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Thank you — this was exactly what #803 needed, and your red-first test (main's UI delete handler provably had zero watcher access) made the review easy. We carried it over the line as e8d2832 (PR #843) with you credited as co-author, with one structural improvement your branch predated: the OOM hardening moved into the shared defer_state_free helper (which now returns success and only removes the watch entry when the defer actually succeeded), so both the unwatch path AND the root-prune path from the watcher-prune work get the same UAF-free discipline. Closes #803. Closing this in favor of the distill — thanks again, and the libgit2 gate in #829 was merged as-is earlier today!

@DeusData DeusData closed this Jul 4, 2026
aimasteracc pushed a commit to aimasteracc/codebase-memory-mcp that referenced this pull request Jul 4, 2026
DELETE /api/project removed the project's .db file but never told the
watcher; the zombie watch kept polling the repo and the next change
silently resurrected the deleted DB (DeusData#803). Wire the watcher into the
UI HTTP server (non-owned ref; created before the server, freed after
it) and unwatch after a successful unlink and on ENOENT (clears an
already-orphaned watch, still 404). Invalid name stays a plain 404
with no unwatch; a non-ENOENT unlink failure stays 500 and keeps the
watch. The cbm_file_exists pre-check is replaced by unlink-errno
handling, removing the TOCTOU window.

Distilled from DeusData#830, rebased over the defer_state_free refactor
(DeusData#817): the OOM hardening of the deferred-free path moves into
defer_state_free itself so BOTH unwatch and stale-root prune get it.
On pending-free realloc failure the helper no longer frees the state
immediately (a potential use-after-free against an in-flight poll_once
snapshot) — it logs watcher.unwatch.oom and returns failure, and the
callers keep the entry registered; the prune path retries on the next
poll cycle.

Carries six httpd regression tests from DeusData#830 driving the live UI
server with a real watcher wired in.

Closes DeusData#803.

Co-authored-by: Dustin Persek <dustin.persek@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.

ui: DELETE /api/project leaves a zombie watch (deleted project resurrects)

2 participants