Files
brain-of-reese/.agents/phases/complete/69_source_removal_cleanup/00_phase.md
T
ducoterra dbf2af26c6 refactor(agents): migrate .agent/ planning tree to .agents/
Standardize on the .agents/ directory (shared with project skills):
phases/, user_stories/, reports/, screenshots/, validate.sh, and
phase-sessions/ + pipeline.log all move to .agents/ (git mv preserves
history; runtime artifacts move alongside).

Updates every reference in AGENTS.md, README.md, .gitignore, app
docstrings, and test story headers. Historical KB content in data/
and the runtime pipeline.log transcript are left untouched.
2026-09-05 10:57:07 -04:00

9.7 KiB
Raw Blame History

Phase 69 — Full Source Removal: Files, Index, and a Confirmation Modal

Source: owner request (chat, 2026-09-02) — "Removing sources doesn't remove the data from the filesystem or the rag… I need files cleaned up and the rag index automatically synced. When I remove a source it should be totally removed. For that reason, there should be a confirmation modal that pops up asking for confirmation if the user clicks delete on a source." Story: n/a (owner request from chat — full source removal, 2026-09-02) Context:

  • app/api/git_sources.py — delete_git_source is row-only today: its docstring says "Removing does not touch the clones or the index — the next Sync (prune=True) prunes the dropped repo (phase scope boundary)". The module's "Scope boundary" paragraph repeats it. 404 (unknown id) / 422 (bad uuid) / 204 pins.
  • app/rag/importer.py — _prune deletes a source's Document rows whose files are no longer walked; Document.chunks carries cascade="all, delete-orphan" (app/models.py:91), so deleting a document drops every chunk row including its pgvector embedding. Sync source naming: git rows index under repo_name(row.url) (scripts/import_docs.py), local rows under Path(row.path or row.url).expanduser().name — the exact expressions removal must reuse to find the right documents.
  • app/api/sync.py::_run_sync — the pipeline whose pieces removal reuses (not re-implements): the bump_sources_version short-lived-session pattern (app/rag/sources_meta.py, phase 53 saved-chat invalidation — any KB change, including a pure prune, bumps exactly once) and the change-gated regenerate_overview (app/rag/overview.py, phase 31 — best-effort by design: an LLMError returns False with the previous row intact).
  • On-disk layout: git checkouts live under settings.sources_dir (~/bor-sources/<repo>/), unpacked uploads under settings.upload_dir (~/bor-sources/uploads/<name>/) — both app-managed. A kind='local' row may also point at any owner directory; those are not app-managed and must never be deleted (the page no longer offers the local-dir form — phase 49 — but the API still accepts it, and such rows can exist).
  • frontend/assets/git-sources.js — the remove flow calls window.confirm (the only window.confirm in the frontend); module docstring bullets "remove" (L69–76) and "Scope boundary" (L82–87) carry the stale "prunes on the next sync" copy. frontend/git-sources.html — #git-sources-hint (L237–243) carries the same stale copy.
  • tests/e2e/test_git_sources_admin.py — test 4 pins the remove lifecycle through a Playwright page.on("dialog") handler (accept → one DELETE; dismiss → none).
  • tests/integration/test_git_sources_api.py — the existing DELETE pins (test_delete_removes_row_and_falls_back_to_env, test_delete_unknown_id_returns_404, test_delete_invalid_id_returns_422). tests/integration/test_git_sources_upload.py is the house pattern for pointing the router at tmp dirs (the _point_at monkeypatch) and faking the LLM (FakeEmbedder, spied import_sources).
  • README.md — git-sources section (~L129–142: "Adding/removing does not clone…") and local-sources section (~L404–410: removal semantics) carry the stale contract.

Objective

Removing a source (admin page or API) is a total removal: the stored row, every indexed document of that source (chunks + embeddings), and — for app-managed sources — the files on disk (the git checkout or the unpacked upload folder), all in one action. The page confirms the removal first through an accessible, in-app modal (replacing window.confirm) that spells out exactly what will be deleted.

Dependencies

  • 68_search_tool (complete; ordering by number — no functional dependency).
  • Functional foundations, all complete: 28_git_based_sources / phase 35 (sources registry + CRUD), 38_local_directory_sources (local rows), phase 49/64_sync_upload_progress (archive uploads + upload dir), phase 32 (sync + prune=True), 53_stale_saved_chats (sources version), phase 31 (KB overview).

Tasks

  1. 01_full_removal_backend.md — the app/rag/source_removal.py helper (source-name resolver, managed-dir mapping, sibling guard, disk removal) and the rewired DELETE /api/git-sources/{id} (row + index + managed files + version bump + best-effort overview).
  2. 02_confirmation_modal.md — the accessible confirmation modal on /git-sources.html (replaces window.confirm), the updated hint-box + docstring copy, frontend unit pins.
  3. 03_e2e_and_commit.md — the dedicated E2E suite (modal → API → disk + DB), test_git_sources_admin.py updated to the modal, README copy, full gates, commit.

Testing & Quality

  • Unit: tests/unit/test_source_removal.py — the resolver (git URL shapes incl. .git suffix + scp-style git@, ~ expansion), the managed-dir mapping (git / upload-under-root / foreign-local → None; the containment check so a sibling named uploads-foo never counts as under upload_dir), disk removal (absent dir no-op, present dir removed, OSError logged not fatal), the sibling-guard decision.
  • Unit (frontend): tests/unit/test_remove_confirm_modal.py — window.confirm gone from git-sources.js; the dialog ids + role="alertdialog" + aria wiring + Esc/Cancel/focus-return wiring present; the stale "stays indexed until the next sync" copy gone from git-sources.html/.js; the new hint copy present.
  • Integration: tests/integration/test_source_removal_api.py — the full-removal matrix (task 01 step 3), tmp dirs + faked/spied LLM per the test_git_sources_upload.py patterns; existing 404/422/204 pins stay green.
  • E2E (mandatory, house rule): tests/e2e/test_source_removal_cleanup.py, run in isolation — the modal flow end-to-end for uploaded, git, and local-directory sources, incl. disk assertions (same-host pathlib against the settings-resolved dirs) and the cancel/Esc paths.
  • Coverage: >90% on app/ (validate.sh gate).

Completion Criteria

  • DELETE /api/git-sources/{id} removes the row and prunes the source's documents (+chunks/embeddings) in one commit, deletes the app-managed on-disk dir when present (git checkout / upload folder), bumps sources_version when docs were pruned, and best-effort-regenerates the overview when pruned > 0. 404/422/204 pins unchanged. Foreign local directories are never touched. Sibling rows sharing a source name keep their documents and files.
  • /git-sources.html removal is a page-local role="alertdialog" modal (no window.confirm anywhere in frontend/): names the source, states the removal policy, Cancel/Esc/backdrop cancel, "Removing…" lifecycle, in-modal role="alert" error, focus return to the trigger, WCAG 2.1 AA basics (labelled, focus-visible, contrast ≥4.5:1, ≥44px targets).
  • uv run pytest green; uv run pytest --cov=app TOTAL >90%; uv run ruff check . && uv run pyright clean.
  • uv run pytest tests/e2e/test_source_removal_cleanup.py -v --no-cov green in isolation (DB up); regression suites green in isolation: test_git_sources_admin.py, test_archive_upload_sources.py.
  • README removal-semantics copy updated (git-sources + local-sources sections).
  • One --no-gpg-sign commit (message in the Commit block); phase dir moved to .agents/phases/complete/.

Locked decisions

  • Owner (chat, 2026-09-02): removal is a total removal — row + RAG index + app-managed files, immediately (not deferred to the next sync); and a real confirmation modal (not window.confirm) pops up when Remove is clicked, stating what will be deleted before it happens.
  • DB first, disk second. The row + document prune commit atomically first (the RAG is always consistent with the registry — this is the owner's core ask); the disk removal runs after the commit, and an OSError is logged (logger.exception) but does not fail the 204. A leftover dir is inert (no row → never imported) and self-heals on re-add (git re-clones — clone_or_pull clones when .git is absent; a re-upload recreates the folder). The reverse order is forbidden: a disk failure must never leave a row pointing at deleted files.
  • App-managed files only. Removal deletes sources_dir/<repo>/ for git rows and the stored dir for local rows when it is upload_dir itself or nested under it (containment via resolved paths + parents, so …/uploads-foo never counts). Any other kind='local' path — the owner's own directory — is never touched on disk; only its row + index entries are removed.
  • Sibling guard. If another stored row resolves to the same source name (e.g. https://e.com/r and https://e.com/r.git → both r), only the row is deleted — the shared documents and files still belong to the sibling. Logged loudly.
  • Version bump + overview gates. sources_version bumps exactly once when pruned > 0 (the phase-53 saved-chat invalidation gate, same as sync). Overview regeneration runs when pruned > 0 — deliberately broader than sync's added+updated gate, because a whole-source removal changes the KB's face — and is best-effort: an LLM failure logs and never fails the delete (the next added/updated change refreshes it, as today).
  • Contract preserved. DELETE keeps 204 with no body (the UI success path and the 404/422 pins are unchanged); the per-operation INFO log line (PLAN §9 / AGENTS.md rule 10) carries the counts.

Commit

git add -A .agents/ app/ tests/ frontend/ README.md && git commit --no-gpg-sign -m "feat(sources): removing a source deletes its files and index entries behind a confirmation modal"