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.
This commit is contained in:
@@ -0,0 +1,149 @@
|
||||
# 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
|
||||
```bash
|
||||
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"
|
||||
```
|
||||
Reference in New Issue
Block a user