diff --git a/.agents/phases/todo/134_sync_cancel/00_phase.md b/.agents/phases/complete/134_sync_cancel/00_phase.md similarity index 100% rename from .agents/phases/todo/134_sync_cancel/00_phase.md rename to .agents/phases/complete/134_sync_cancel/00_phase.md diff --git a/.agents/phases/todo/134_sync_cancel/01_backend_cancel_state.md b/.agents/phases/complete/134_sync_cancel/01_backend_cancel_state.md similarity index 100% rename from .agents/phases/todo/134_sync_cancel/01_backend_cancel_state.md rename to .agents/phases/complete/134_sync_cancel/01_backend_cancel_state.md diff --git a/.agents/phases/todo/134_sync_cancel/02_frontend_cancel_button.md b/.agents/phases/complete/134_sync_cancel/02_frontend_cancel_button.md similarity index 100% rename from .agents/phases/todo/134_sync_cancel/02_frontend_cancel_button.md rename to .agents/phases/complete/134_sync_cancel/02_frontend_cancel_button.md diff --git a/.agents/phases/todo/134_sync_cancel/03_e2e_sync_cancel.md b/.agents/phases/complete/134_sync_cancel/03_e2e_sync_cancel.md similarity index 100% rename from .agents/phases/todo/134_sync_cancel/03_e2e_sync_cancel.md rename to .agents/phases/complete/134_sync_cancel/03_e2e_sync_cancel.md diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__00_phase.a1.err b/.agents/reports/134_sync_cancel/134_sync_cancel__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__00_phase.a1.md b/.agents/reports/134_sync_cancel/134_sync_cancel__00_phase.a1.md new file mode 100644 index 0000000..0f92c18 --- /dev/null +++ b/.agents/reports/134_sync_cancel/134_sync_cancel__00_phase.a1.md @@ -0,0 +1,13 @@ +**Phase 134 final verification pass — all green** (all 3 task files already in `complete/`; no implementation was missing) + +- Verified: 5-state `SyncStatus` + `cancelled` terminal (error None, final counts kept), `POST /api/sync/cancel` (202/409/403), cooperative `_check_cancelled` boundaries, shutdown `CancelledError` contract preserved, `#sync-cancel-btn` show/hide + neutral "Sync cancelled" settle, 8-branch poll tree, re-attach — code, unit, integration, and E2E layers all present +- `uv run pytest --cov=app --cov-report=term-missing` → **3150 passed**, `app/` coverage **99%** (gate: >90%) +- `uv run pytest tests/e2e/test_sync_cancel.py -v --no-cov` (isolated) → **3 passed** (47.6 s) +- `uv run ruff check .` → clean; `uv run pyright` → 0 errors, 0 warnings + +**Completion criteria:** ① cancel endpoint 202/409/403 ✅ (unit + E2E) ② `cancelled` terminal reported by status, error null, counts kept ✅ (integration `test_cancel_mid_run_lands_in_cancelled_then_resyncs_clean`) ③ unrequested-cancel pin ✅ (`test_unrequested_cancel_is_never_swallowed_and_never_converts`) ④ UI lifecycle + clean re-sync (sentinel row, full 40-file import) ✅ (E2E) ⑤ suite/coverage/E2E gates ✅ ⑥ uncancelled runs byte-identical ✅ (all 3150 pre-existing tests green untouched) + +- Docs: README.md reviewed in full against code (pages, auth endpoints, every config default vs `app/config.py`, palette 9+8, fail-loud, cancel section) — consistent, no changes needed. AGENTS.md: fixed one pre-existing inaccuracy (rule 2 "move its file" → "move its directory" — phases are directories); everything else verified accurate +- No live infrastructure touched; no deviations + +**Next pending phase:** `135_git_source_branch` diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__00_phase.a1.validate b/.agents/reports/134_sync_cancel/134_sync_cancel__00_phase.a1.validate new file mode 100644 index 0000000..e7bb5c6 --- /dev/null +++ b/.agents/reports/134_sync_cancel/134_sync_cancel__00_phase.a1.validate @@ -0,0 +1,115 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 48%] +........................................................................ [ 50%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 57%] +........................................................................ [ 59%] +........................................................................ [ 61%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 80%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 93%] +........................................................................ [ 96%] +........................................................................ [ 98%] +...................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 268 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 179 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 183 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 256 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 132 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 412 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 595 9 98% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 71 0 100% +app/rag/pdf.py 109 0 100% +app/rag/prompts.py 144 0 100% +app/rag/retriever.py 248 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 49 0 100% +app/schemas.py 410 0 100% +-------------------------------------------------- +TOTAL 5285 22 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__01_backend_cancel_state.a1.err b/.agents/reports/134_sync_cancel/134_sync_cancel__01_backend_cancel_state.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__01_backend_cancel_state.a1.md b/.agents/reports/134_sync_cancel/134_sync_cancel__01_backend_cancel_state.a1.md new file mode 100644 index 0000000..411558d --- /dev/null +++ b/.agents/reports/134_sync_cancel/134_sync_cancel__01_backend_cancel_state.a1.md @@ -0,0 +1,14 @@ +Task complete. Final report: + +**Implemented (task 01 — backend cancel state):** +- `app/api/sync.py`: 5-state `SyncStatus` (`cancelled` = terminal, `error=None`, `detail={}`, final counts kept); module `_cancel_event`; `start_sync` clears it (no stale bleed); `POST /api/sync/cancel` (admin router → 403 anonymous, 202 in-flight, 409 `no sync is running` otherwise — sets event + `_task.cancel()`); `_check_cancelled()` at the 3 pipeline boundaries (source-loop top, pre-import, pre-overview/summaries); `except asyncio.CancelledError` before `except Exception` — user-requested → `cancelled` + PLAN §9 INFO line + re-raise; unrequested (shutdown) → re-raise unchanged. Module/dataclass/endpoint docstrings updated incl. cooperative semantics (in-flight git subprocess runs to completion). +- Tests: 8 new unit (`tests/unit/test_sync_button.py` — 202/409 contract, `_check_cancelled`, endpoint cancel flow, shutdown pin, stale-event pin, all 3 boundaries, caplog line) + 2 new integration (`tests/integration/test_sync_api.py` — anonymous 403; mid-run cancel → `cancelled` → 409 → clean re-sync to `success`). Both fixtures now reset `_cancel_event`. + +**Results (all green):** +- `uv run pytest --cov=app --cov-report=term-missing` → **3139 passed**, `app/` **99%**, `app/api/sync.py` **100%** +- `uv run pytest tests/unit/test_sync_button.py tests/integration/test_sync_api.py -v --no-cov` → 73 passed +- `uv run ruff check .` clean; `uv run pyright` → 0 errors + +**Docs:** README "Sync from the UI" gains the Cancel bullet (new API endpoint); AGENTS.md unchanged (no convention change). +**Decisions:** cancelled terminal also clears the chunk position (phase-129 "every terminal" convention); unit cancel runs driven through real `start_sync` with the cancel issued from the loop thread (cross-thread `Task.cancel()` would race a parked loop's selector wake). +**Next task:** `134_sync_cancel/02_frontend_cancel_button.md`. diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__01_backend_cancel_state.a1.validate b/.agents/reports/134_sync_cancel/134_sync_cancel__01_backend_cancel_state.a1.validate new file mode 100644 index 0000000..97d2c17 --- /dev/null +++ b/.agents/reports/134_sync_cancel/134_sync_cancel__01_backend_cancel_state.a1.validate @@ -0,0 +1,115 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 48%] +........................................................................ [ 50%] +........................................................................ [ 52%] +........................................................................ [ 55%] +........................................................................ [ 57%] +........................................................................ [ 59%] +........................................................................ [ 61%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 71%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 80%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 94%] +........................................................................ [ 96%] +........................................................................ [ 98%] +........................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 268 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 179 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 183 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 256 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 132 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 412 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 595 9 98% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 71 0 100% +app/rag/pdf.py 109 0 100% +app/rag/prompts.py 144 0 100% +app/rag/retriever.py 248 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 49 0 100% +app/schemas.py 410 0 100% +-------------------------------------------------- +TOTAL 5285 22 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__02_frontend_cancel_button.a1.err b/.agents/reports/134_sync_cancel/134_sync_cancel__02_frontend_cancel_button.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__02_frontend_cancel_button.a1.md b/.agents/reports/134_sync_cancel/134_sync_cancel__02_frontend_cancel_button.a1.md new file mode 100644 index 0000000..ac15095 --- /dev/null +++ b/.agents/reports/134_sync_cancel/134_sync_cancel__02_frontend_cancel_button.a1.md @@ -0,0 +1,16 @@ +Task 02 is complete and the whole project is green. + +**Implemented** +- `frontend/index.html`: `#sync-cancel-btn` (secondary pill, `hidden` by default, visible "Cancel" label = accessible name) in the sync-control row right after `#sync-btn` +- `frontend/assets/styles.css`: `.sync-cancel-btn` — house secondary-pill language (44px floor, AA `--ink-soft`, global `:focus-visible` ring, guarded hover, `:disabled`, `[hidden]` beats display, full-width in the ≤640px wrap case) +- `frontend/assets/sources.js`: `showSyncCancelBtn`/`hideSyncCancelBtn` (show = unhide+enable+aria-label; hide = hide+disable); shown only for `kind === "sync"` in `enterSyncRunningState`, hidden by `settleSyncButton` on every settle; `requestCancel()` (disable-first, 403 backstop / silent 409 / 202 wait / phase-55 retry copy + re-enable); new poll-tree branch 5 `cancelled` (after failed, before upload branches — tree is now 8); neutral `applySyncCancelled` ("Sync cancelled", no banner/modal); `initSyncButton` re-attach branch; docstrings updated (module + section header with the 8-branch tree and show/hide contract) +- Fixed the defect my change surfaced: the phase-129 node harness in `tests/unit/test_frontend_sync_upload.py` now extracts the REAL cancel helpers + `settleSyncButton` and behaviorally pins the control (shown+enabled on every sync-running state; hidden+disabled for upload and settle) +- New source-level contract pins in `tests/unit/test_sync_button.py` (11 tests: markup, CSS, helpers, choke points, never-stale request, branch order, neutral applier, re-attach, docstring) + +**Tests**: `uv run pytest --cov=app` → 3150 passed, `app/` 99% (>90%); `uv run ruff check .` clean; `uv run pyright` 0 errors; sanity E2E in isolation: `tests/e2e/test_sync_button.py` 3/3 and `tests/e2e/test_sync_label_mobile.py` 4/4 (page boots clean, uncancelled run unchanged) + +**Docs**: README "Sync from the UI" — Cancel bullet now covers the button + neutral "Sync cancelled" settle; AGENTS.md unchanged (no command/config/convention impact) + +**Decisions**: cancel hover guarded `:not(:disabled)` so a disabled control under the cursor never reads as actionable; step 8 of the node harness now runs the real `settleSyncButton` (stricter, same labels) + +**Next pending task**: `.agents/phases/todo/134_sync_cancel/03_e2e_sync_cancel.md` diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__02_frontend_cancel_button.a1.validate b/.agents/reports/134_sync_cancel/134_sync_cancel__02_frontend_cancel_button.a1.validate new file mode 100644 index 0000000..e7bb5c6 --- /dev/null +++ b/.agents/reports/134_sync_cancel/134_sync_cancel__02_frontend_cancel_button.a1.validate @@ -0,0 +1,115 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 48%] +........................................................................ [ 50%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 57%] +........................................................................ [ 59%] +........................................................................ [ 61%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 80%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 93%] +........................................................................ [ 96%] +........................................................................ [ 98%] +...................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 268 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 179 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 183 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 256 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 132 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 412 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 595 9 98% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 71 0 100% +app/rag/pdf.py 109 0 100% +app/rag/prompts.py 144 0 100% +app/rag/retriever.py 248 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 49 0 100% +app/schemas.py 410 0 100% +-------------------------------------------------- +TOTAL 5285 22 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__03_e2e_sync_cancel.a1.err b/.agents/reports/134_sync_cancel/134_sync_cancel__03_e2e_sync_cancel.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__03_e2e_sync_cancel.a1.md b/.agents/reports/134_sync_cancel/134_sync_cancel__03_e2e_sync_cancel.a1.md new file mode 100644 index 0000000..2640a0b --- /dev/null +++ b/.agents/reports/134_sync_cancel/134_sync_cancel__03_e2e_sync_cancel.a1.md @@ -0,0 +1,19 @@ +All gates are green. Final report: + +**Implemented** +- `tests/e2e/test_sync_cancel.py` (new, dedicated Playwright suite, run in isolation): per-module app on port 8133 (`E2E_APP_PORT_SYCNCEL` per the task file), the `slow_llm` proxy (0.3 s delay) in front of the mock LLM, a real `file://` git fixture repo (40 unique `notes/*.md` files, one sentinel `Marker: RESE-CANCEL-SENTINEL-4f1e`), and the generalized `_wait_sync_state(page, app_url, states, timeout_s)` helper. +- 3 tests per the Playwright Mapping Rule: `test_anonymous_cancel_403` (403 + Cancel never revealed), `test_cancel_when_idle_409` (exact detail `no sync is running`, no cancel affordance at boot/idle), `test_admin_cancel_lifecycle` (Cancel visible+enabled on the 202 → click → disables immediately → `cancelled` terminal with `error: null`/`detail: {}`/positions cleared/`files_done < 40` → neutral "Sync cancelled" settle, no banner/modal → clean re-sync: `added + unchanged == 40`, sentinel row in the drill-down catalog, sentinel content intact in the DB). +- Plus: module-scoped 120 s wall-time guard fixture and a console-error collector (task 02's no-console-errors criterion). + +**Results (all green)** +- `uv run pytest tests/e2e/test_sync_cancel.py -v --no-cov` → 3 passed, twice (47.6 s / 47.6 s — deterministic, under the 120 s guard) +- `uv run pytest --cov=app --cov-report=term-missing` → 3150 passed, `app/` coverage **99%** (>90%; `app/api/sync.py` 100%) +- `uv run ruff check . && uv run pyright` → clean (0 errors) + +**Docs** — no changes needed: README already documents the Cancel button + `POST /api/sync/cancel` contract (task 01/02); AGENTS.md unaffected. + +**Notable decisions** +- Kept the task file's `E2E_APP_PORT_SYCNCEL` name (typo as specified) and default port 8133 (also test_sync_upload_progress's default — suites run in isolation per house convention, no clash). +- The importer's progress hook fires *before* indexing a file, so committed-after-cancel is `files_done` or `files_done−1`; the re-sync assertion is robust to both (`added + unchanged == 40` + DB count + sentinel integrity). + +**Next pending task** — none in phase 134 (task 03 is the last); next todo phase is `135_git_source_branch`. diff --git a/.agents/reports/134_sync_cancel/134_sync_cancel__03_e2e_sync_cancel.a1.validate b/.agents/reports/134_sync_cancel/134_sync_cancel__03_e2e_sync_cancel.a1.validate new file mode 100644 index 0000000..e7bb5c6 --- /dev/null +++ b/.agents/reports/134_sync_cancel/134_sync_cancel__03_e2e_sync_cancel.a1.validate @@ -0,0 +1,115 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 48%] +........................................................................ [ 50%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 57%] +........................................................................ [ 59%] +........................................................................ [ 61%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 80%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 93%] +........................................................................ [ 96%] +........................................................................ [ 98%] +...................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 268 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 179 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 183 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 256 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 132 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 412 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 595 9 98% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 71 0 100% +app/rag/pdf.py 109 0 100% +app/rag/prompts.py 144 0 100% +app/rag/retriever.py 248 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 49 0 100% +app/schemas.py 410 0 100% +-------------------------------------------------- +TOTAL 5285 22 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/AGENTS.md b/AGENTS.md index 9975fcb..2a215c1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,7 +5,7 @@ Do not design anything that contradicts it. 2. **Follow the phased execution protocol in `.agents/phases/`.** Work through `.agents/phases/todo/` in numeric order. When a phase is complete, move its - file to `.agents/phases/complete/`. + directory to `.agents/phases/complete/`. 3. **Strictly adhere to the LOCKED DECISIONS** (PLAN §2). If you believe a LOCKED decision is wrong, stop and flag it — do not silently deviate. 4. **"One Story, One Phase":** each user story in `.agents/user_stories/` diff --git a/README.md b/README.md index 3728294..019d108 100644 --- a/README.md +++ b/README.md @@ -306,6 +306,18 @@ whole git-source refresh in one click: timeout — a clone + embed can legitimately take minutes. - One sync at a time: a second trigger while a run is in flight gets a `409`. +- **Cancel**: while a run is in flight, a **Cancel** button appears + next to the sync button; clicking it posts `POST /api/sync/cancel` + (admin only) and registers a cooperative cancel (`202`) — the run + stops at the next pipeline boundary (a git clone/pull already in + flight runs to completion) and settles into a neutral `cancelled` + terminal state (`error: null` — intent, not failure: a "Sync + cancelled" result line, no error banner or modal, the button + retry-ready) that the status endpoint reports; with no run in + flight the request gets a `409` (the UI treats it as a harmless + race — the poll settles). A sync started after a cancel runs clean + (the importer's content-hash delta re-imports what the cancel + skipped). --- diff --git a/app/api/sync.py b/app/api/sync.py index 6b5153b..2164c82 100644 --- a/app/api/sync.py +++ b/app/api/sync.py @@ -10,7 +10,13 @@ untouched — no queue, no new services): one ``asyncio`` background task plus a module-level :class:`SyncStatus` that the UI polls every 2 s (task 02). One sync at a time — ``POST`` while a run is in flight is 409; the status object is authoritative, so the UI can never sit on a -stale button state (§7.4 adaptation, phase locked decisions). +stale button state (§7.4 adaptation, phase locked decisions). Phase +134 (task 01) adds the cancel: ``POST /api/sync/cancel`` (admin-only, +same router) registers a COOPERATIVE cancel while a run is in flight +(202, 409 ``no sync is running`` otherwise), and the run lands in the +new ``cancelled`` TERMINAL state — a cancel is intent, not failure +(``error = None``, ``detail = {}``, the final counts kept; the cancel +paragraph below). Pipeline (the canonical "mirror the sources" action — phase locked decisions): @@ -92,6 +98,53 @@ carries the phase-64 per-file progress — ``current_file`` (the (clone/pull reports no file yet) and in terminal states, which clear ``current_file`` but keep the run's final counts. +Phase 134 (task 01) makes the machine FIVE-state: ``cancelled`` joins +``success`` / ``failed`` as a TERMINAL state — it carries +``finished_at``, clears ``current_file`` / ``phase`` / +``current_summary`` and the chunk position, and keeps the run's final +``files_done`` / ``files_total`` + summary counts (the phase-64/98 +keep-final-counts convention) — but with ``error = None`` and +``detail = {}``: a cancel is intent, not failure, so the UI never +treats it as an error (no banner, no modal — a neutral result line). +``GET /api/sync/status`` gains nothing structurally — the ``state`` +key simply reports ``"cancelled"``. + +The cancel is COOPERATIVE, best-effort (phase 134 design): while a +run is in flight, ``POST /api/sync/cancel`` (the router's +``require_admin`` dependency applies — 403 anonymous) sets the +module-level ``_cancel_event`` AND ``_task.cancel()`` (unblocking +whatever ``await`` the pipeline is parked on — e.g. a per-file LLM +call inside the import), answering 202 the moment the request is +REGISTERED (the run may already be terminal by the next poll — the +status object, not the task, is authoritative). ``start_sync`` clears +the event when it starts a run, so a stale event from a previously +cancelled run can never bleed into the next one. ``_run_sync`` +checks the event at the pipeline boundaries via +:func:`_check_cancelled` (raises ``asyncio.CancelledError`` when +set): the top of the per-source loop (before each clone/pull or +local-dir check), immediately before ``import_sources``, and +immediately before the overview + summaries span (the +``added + updated > 0`` block). Consequence a future reader must +know: a git subprocess already running at the cancel moment runs to +completion (the blocking ``subprocess.run`` has no await inside) — +the cancel lands at the NEXT boundary (after that source's git ops, +between files, or before overview/summaries); during the import +span, the ``_task.cancel()`` from the endpoint interrupts at the +next per-file await. + +The ``CancelledError`` contract has TWO paths (the former +"deliberately not caught" rule refined, not broken): in +``_run_sync``'s ``except asyncio.CancelledError`` clause (placed +before the ``except Exception``), a USER-requested cancel +(``_cancel_event`` set) lands the run in the ``cancelled`` terminal ++ one INFO line (PLAN §9: ``sync: cancelled by user state=cancelled +files_done=… files_total=…``) and then RE-RAISES (the task ends +cancelled — ``_task.done()`` is True, the status object stays +authoritative); an UNREQUESTED cancel (the app-shutdown path — the +event unset) re-raises UNCHANGED (the shutdown contract holds: a +background task must die in state, never as an unobserved exception +— swallowing an unrequested stop would mask a real one). + Phase 98 (task 01) adds the run's PHASE machine — ``phase`` is the pipeline stage the run is in: the model-check + clone/pull prelude reports ``null`` (the bare "Syncing…" label stays), then ``"import"`` @@ -108,9 +161,10 @@ summarized — the bare source name for the source-root row) plus through the long summary span where the file count sits still (the hook's ``total`` is the run's key count — the AFFECTED count on a changed KB, the missing count on a gap-fill). -Terminal states (success AND failed) clear ``phase`` + -``current_summary`` but keep the run's final ``summaries_done`` / -``summaries_total`` (the phase-64 keep-final-counts convention). +Terminal states (success, failed, and — phase 134 — cancelled) clear +``phase`` + ``current_summary`` but keep the run's final +``summaries_done`` / ``summaries_total`` (the phase-64 +keep-final-counts convention). Phase 129 (task 03) adds the in-flight file's EMBED-chunk position — ``current_chunk`` (the 1-based first chunk of the in-flight @@ -120,7 +174,7 @@ chunks), batch granularity via the :meth:`app.rag.llm.LLMClient.embed` batch (the prelude), during the ``"overview"`` / ``"summaries"`` phases, and in terminal states — the import phase clearing them at EVERY point it ends (the ``phase = "overview"`` and -``phase = "summaries"`` transitions and both terminals, the same spots +``phase = "summaries"`` transitions and every terminal, the same spots ``current_file`` clears). Unlike the phase-64 FILE counts, the chunk position is a SUB-position of a file that's done — the keep-final- counts convention does NOT extend to it: the file counts survive the @@ -168,10 +222,15 @@ router = APIRouter( class SyncStatus: """In-memory state of the (at most one) in-flight sync run. - ``state`` is a four-state machine: ``idle`` (never run / reset), - ``running``, ``success``, ``failed``. Terminal states carry the run's - ``detail`` (success) or ``error`` (failure) so the UI can render the - last result after a page reload (task 02's re-attach behavior). + ``state`` is a five-state machine: ``idle`` (never run / reset), + ``running``, ``success``, ``failed``, and ``cancelled`` (phase 134, + task 01 — terminal like ``success`` / ``failed``: ``finished_at`` + set, ``current_file`` / ``phase`` / ``current_summary`` cleared, the + final file + summary counts kept, but ``error = None`` and + ``detail = {}`` — a cancel is intent, not failure). Terminal states + carry the run's ``detail`` (success) or ``error`` (failure) so the UI + can render the last result after a page reload (task 02's re-attach + behavior). Phase 64 (task 02) progress fields: ``current_file`` is the ``source/relative/path`` the import is processing right now (null @@ -201,14 +260,14 @@ class SyncStatus: contract). They are 0/0 before the first batch, during the ``"overview"`` / ``"summaries"`` phases, and in terminal states — the import phase clears them at every point it ends (the - overview/summaries transitions + both terminals, where + overview/summaries transitions + every terminal, where ``current_file`` also clears). The phase-64 keep-final-counts convention covers the FILE counts only — the chunk position is a sub-position of a file that's done, so it clears in the terminal while ``files_done`` / ``files_total`` keep their final values. """ - state: Literal["idle", "running", "success", "failed"] = "idle" + state: Literal["idle", "running", "success", "failed", "cancelled"] = "idle" started_at: datetime | None = None finished_at: datetime | None = None detail: dict[str, Any] = field(default_factory=dict) @@ -236,6 +295,12 @@ class SyncStatus: _status = SyncStatus() _task: asyncio.Task[None] | None = None +# Phase 134 (task 01): the cancel request — set by POST +# /api/sync/cancel while a run is in flight; _run_sync's boundaries +# + its ``except CancelledError`` clause read it (the module +# docstring's cancel paragraph). Cleared by start_sync when a run +# starts, so a stale event can never bleed into the next run. +_cancel_event = asyncio.Event() @router.get("/status") @@ -257,6 +322,11 @@ def sync_status() -> dict[str, Any]: terminal states; while in ``"summaries"``, ``current_summary`` + ``summaries_done`` / ``summaries_total`` carry the generator's position (the terminal keeps the final summary counts). + + Phase 134 (task 01): ``state`` also reports ``"cancelled"`` for a + user-cancelled run — a terminal state with ``error`` null and + ``detail`` empty; the run's final file + summary counts are kept, + the chunk position cleared. """ return { "state": _status.state, @@ -270,7 +340,7 @@ def sync_status() -> dict[str, Any]: # Phase 129 (task 03): the in-flight file's embed-chunk # position — the dataclass defaults make idle 0/0, and the # runner clears it at every point the import phase ends - # (overview/summaries transitions + both terminals). + # (overview/summaries transitions + every terminal). "current_chunk": _status.current_chunk, "chunks_total": _status.chunks_total, # Phase 98 (task 01): the phase machine — null/0/0 idle (the @@ -291,22 +361,77 @@ async def start_sync() -> dict[str, str]: loop. 409 when a run is already in flight (one sync at a time — the status endpoint is the single source of truth for the run, and the UI re-attaches to it rather than starting a second one). + + Phase 134 (task 01): the run's cancel request resets with the run — + ``_cancel_event.clear()`` before the task is created, so a stale + event from a previously cancelled run can never bleed into this one. """ global _task if _task is not None and not _task.done(): raise HTTPException(status_code=409, detail="a sync is already running") + _cancel_event.clear() _task = asyncio.create_task(_run_sync()) return {"detail": "sync started"} +@router.post("/cancel", status_code=202) +async def cancel_sync() -> dict[str, str]: + """Request the cancel of the in-flight run (phase 134, task 01). + + 202 + ``cancel requested`` is answered the moment the request is + REGISTERED: the module ``_cancel_event`` is set (the runner's + ``_check_cancelled`` boundaries + its ``except CancelledError`` + conversion see it) AND ``_task.cancel()`` unblocks whatever + ``await`` the pipeline is parked on (e.g. a per-file LLM call + inside the import). The run may already be terminal by the next + status poll — the status object, not the task, is authoritative, + so the UI's poll settles on the terminal (``cancelled``) state. + 409 ``no sync is running`` when there is no run in flight (the + ``_task`` is None or done — the harmless race the UI's poll + settles; no error copy, no state change). + """ + if _task is None or _task.done(): + raise HTTPException(status_code=409, detail="no sync is running") + _cancel_event.set() + _task.cancel() + return {"detail": "cancel requested"} + + +def _check_cancelled() -> None: + """Phase 134 (task 01): the cooperative cancel boundary. + + Raises ``asyncio.CancelledError`` when the module ``_cancel_event`` + is set (the user requested a stop — ``POST /api/sync/cancel``); + :func:`_run_sync`'s ``except asyncio.CancelledError`` clause + converts it into the ``cancelled`` terminal state. Called at the + pipeline boundaries (the module docstring's cancel paragraph) — + the top of the per-source loop, before ``import_sources``, and + before the overview + summaries span. A git subprocess in flight + at the cancel moment runs to completion (no await inside + ``subprocess.run``); the cancel lands at the NEXT boundary. + """ + if _cancel_event.is_set(): + raise asyncio.CancelledError() + + async def _run_sync() -> None: """The full sync pipeline, one in-process background task. Every failure mode (git, embeddings, anything else) lands in the ``failed`` state with a sanitized ``error`` string — a background task must die in state, never as an unobserved exception. - ``CancelledError`` is deliberately *not* caught: app shutdown - cancels the task, and swallowing that would mask a real stop. + + Cancellation (phase 134, task 01 — the module docstring's cancel + paragraph): the cooperative boundaries (:func:`_check_cancelled` + at the top of the per-source loop, before ``import_sources``, and + before the overview + summaries span) plus the ``except + asyncio.CancelledError`` clause convert a USER-requested cancel + (``_cancel_event`` set by ``POST /api/sync/cancel``) into the + ``cancelled`` terminal state + one INFO line, then re-raise (the + task ends cancelled — the status object stays authoritative). An + UNREQUESTED cancel (the app-shutdown path — the event unset) + re-raises unchanged: a background task is never swallowed, and + swallowing a real stop would mask it. """ _status.state = "running" _status.started_at = datetime.now(UTC) @@ -363,6 +488,11 @@ async def _run_sync() -> None: include_hidden_by_root: dict[str, bool] = {} doc_dates_by_root: dict[str, dict[str, datetime]] = {} for row in rows: + # Phase 134 (task 01): the per-source boundary — a cancel + # registered while the previous source's git ops run (a + # blocking subprocess runs to completion) lands here, + # before this source's clone/pull or local-dir check. + _check_cancelled() if row.kind == "git": # Phase 121: the token column is injected into the clone # URL ONLY here (clone_url_for — NULL token → the bare @@ -424,12 +554,22 @@ async def _run_sync() -> None: # starts NOW (the model-check + clone/pull prelude above # reported ``phase: null``). _status.phase = "import" + # Phase 134 (task 01): the import boundary — a cancel between + # the source walk and the import (or parked in a blocking git + # span that just finished) lands here, before any file is + # imported. + _check_cancelled() summary: ImportSummary = await import_sources( sources, llm, prune=True, progress=_hook, on_chunk_progress=_chunk_hook, ignore_by_root=ignore_by_root, include_hidden_by_root=include_hidden_by_root, doc_dates_by_root=doc_dates_by_root, ) + # Phase 134 (task 01): the overview + summaries boundary — a + # cancel during the import span (the endpoint's _task.cancel() + # interrupts at the next per-file await) or registered anywhere + # before this point lands here, before the lite spans start. + _check_cancelled() overview = False if summary.added + summary.updated > 0: _status.phase = "overview" # phase 98 (task 01) @@ -574,6 +714,34 @@ async def _run_sync() -> None: "sources_version": sources_version, } logger.info("sync: done detail=%s", _status.detail) + except asyncio.CancelledError: + # Phase 134 (task 01): the TWO conversion paths (the module + # docstring's cancel paragraph). A user-requested cancel — + # ``_cancel_event`` set by POST /api/sync/cancel, whether the + # endpoint's ``_task.cancel()`` interrupted a parked await or a + # ``_check_cancelled`` boundary raised — lands the run in the + # ``cancelled`` terminal and re-raises (the task ends + # cancelled — ``_task.done()`` is True, the status object stays + # authoritative). An unrequested cancel (the app-shutdown path + # — the event unset) re-raises unchanged: never swallow a real + # stop. + if _cancel_event.is_set(): + _status.state = "cancelled" + _status.finished_at = datetime.now(UTC) + _status.error = None # a cancel is intent, not failure + _status.current_file = None # keep the final counts + _status.phase = None # keep the final summary counts + _status.current_summary = None + # Phase 129 (task 03): the chunk position is a sub-position + # of a file that's done — it clears in EVERY terminal, like + # success and failed (the convention split, pinned). + _status.current_chunk = 0 + _status.chunks_total = 0 + logger.info( + "sync: cancelled by user state=cancelled files_done=%d files_total=%d", + _status.files_done, _status.files_total, + ) + raise except Exception as e: # noqa: BLE001 — a background task dies in state, see above logger.exception("sync: failed") _status.state = "failed" diff --git a/frontend/assets/sources.js b/frontend/assets/sources.js index ddc3104..c4f13e7 100644 --- a/frontend/assets/sources.js +++ b/frontend/assets/sources.js @@ -328,6 +328,39 @@ * "Syncing… (n/m)", "Importing", "Writing KB overview…", * "Summarizing folders… (n/m)"); textContent only — no HTML * strings (the house rule). + * + * Phase 134 (task 02) — the Cancel control + the cancelled terminal: + * the owner can stop a running sync. #sync-cancel-btn (the static + * markup next to #sync-btn — ship-hidden, the same convention) is + * visible + enabled ONLY while a SYNC run is in flight: + * enterSyncRunningState shows it for kind "sync" (the live click, the + * load-time re-attach, and every running poll tick) and hides it for + * kind "upload" (the upload job is NOT cancellable in this phase — + * its run keeps the control hidden); settleSyncButton hides it on + * EVERY settle (every terminal + both idles — one choke point). A + * click runs requestCancel() under the same never-stale contract as + * the main button: the control disables IMMEDIATELY (one request at + * a time — no double fire) BEFORE the POST /api/sync/cancel — + * 403 → the whoami-backstop hide-everything branch (the same depth as + * every other 403 in this module), 409 → SILENT (the run ended + * between the tick and the click — the poll settles on the next tick, + * which hides the control; no error copy), 202 → the control stays + * disabled until the terminal (the poll's new cancelled branch settles + * everything — the status object, not the request, is authoritative), + * any other non-2xx / network failure → the phase-55 neutral retry + * copy ("Couldn't cancel the sync — try again.") + re-enable (the + * run is still going — the owner can retry). The poll tree gains its + * 5th branch (the 7-branch tree becomes 8): state "cancelled" — after + * the failed branch, before the upload branches — stops the poll and + * runs applySyncCancelled: the NEUTRAL result (A1 — cancel is intent, + * not failure): settle the main button retry-ready (which also hides + * the Cancel control), "Sync cancelled" on the aria-live result line, + * the error banner hidden, the status frame emitted — no modal, no + * error styling. The initSyncButton re-attach renders the same result + * for a reload during/right after a cancel (a page reload shows + * "Sync cancelled", never a stuck running state). The bor:sync-status + * event shape is unchanged — the cancelled status object flows through + * emitSyncStatus exactly like every other state. */ import { fetchIsAdmin } from "./header.js"; @@ -606,14 +639,41 @@ export async function mount(root) { * sources" contract, A3); * 3. sync success → the phase-32 settle (counts + catalog refresh); * 4. sync failed → the phase-32 failure (banner + modal); - * 5. upload success → settle "Sync sources" + catalog refresh + * 5. sync cancelled → the phase-134 neutral terminal (task 02 — + * A1: cancel is intent, not failure): settle + * the main button retry-ready + "Sync + * cancelled" on #sync-result, no banner / no + * modal (applySyncCancelled); the settle also + * hides the Cancel control; + * 6. upload success → settle "Sync sources" + catalog refresh * (loadTree — phase 90: an upload no longer * changes the KB, the re-read is a no-op safety * net); the upload's result line lives on the * Sources page, never in #sync-result (A3); - * 6. upload failed → settle "Sync sources" — the failure is the + * 7. upload failed → settle "Sync sources" — the failure is the * Sources page's error banner, never this page's (A3); - * 7. both idle → retry-ready idle. + * 8. both idle → retry-ready idle. + * Phase 134 (task 02) — the Cancel control (#sync-cancel-btn, the + * static markup next to #sync-btn — ship-hidden, the same + * convention): visible + enabled ONLY while a SYNC run is in + * flight. The show/hide contract has two choke points: + * enterSyncRunningState shows it for kind "sync" (the live 202/409 + * click, the load-time re-attach, and every running tick of branch + * 1) and hides it for kind "upload" (the upload job is NOT + * cancellable in this phase — its run keeps the control hidden); + * settleSyncButton hides it on EVERY settle (branches 3–8 all pass + * through it). The click runs requestCancel(): the control disables + * IMMEDIATELY (one request at a time — never stale) before + * POST /api/sync/cancel; 403 → the whoami-backstop branch (stop + * polling, hide the main button, settle idle — the same depth as + * every other 403 in this module); 409 → SILENT (the run ended + * between the tick and the click — the poll settles on the next + * tick and its settle hides the control; no error copy); 202 → + * nothing (the control stays disabled — the poll's branch 5 settles + * everything; the run may already be terminal by then — the status + * object, not the request, is authoritative); any other non-2xx / + * network failure → the phase-55 neutral retry copy on #sync-result + * + re-enable (the run is still going — the owner can retry). * The live label is the status endpoint's full source/relative/path * (A4) — or, in the post-import phases, the phase's own copy (Phase * 98 task 02): CSS ellipsizes #sync-label; the full untruncated @@ -632,11 +692,18 @@ export async function mount(root) { * pinned "(n/m)" position (hidden when absent); phase 129 (task * 04): + .sync-label-chunk, the pinned "· chunk n/m" embed-batch * position of the in-flight file (hidden in every state but the - * import phase after the file's first batch)), #sync-icon (the + * import phase after the file's first batch)), #sync-cancel-btn + * (the Cancel control — phase 134 task 02, ship-hidden, revealed + * only while a sync run is in flight), #sync-icon (the * spinner icon), #sync-result (aria-live result line), * #sync-error-banner / #sync-error-text (error banner). */ const syncBtn = root.querySelector("#sync-btn"); + // Phase 134 (task 02): the Cancel control — visible + enabled ONLY + // while a SYNC run is in flight (the section header carries the + // show/hide contract); the ship-hidden attribute stays until the + // first running sync state. + const syncCancelBtn = root.querySelector("#sync-cancel-btn"); const syncLabel = syncBtn ? syncBtn.querySelector(".sync-label") : null; const syncLabelMain = syncLabel ? syncLabel.querySelector(".sync-label-main") : null; const syncLabelCount = syncLabel ? syncLabel.querySelector(".sync-label-count") : null; @@ -841,10 +908,19 @@ export async function mount(root) { syncBtn.setAttribute("aria-label", "Sync sources"); syncBtn.classList.remove("is-error"); if (syncIcon) syncIcon.classList.add("is-spinning"); + // Phase 134 (task 02): the Cancel control is visible + enabled + // ONLY while a SYNC run is in flight — the upload job is NOT + // cancellable in this phase (its run keeps the control hidden). + if (kind === "sync") showSyncCancelBtn(); + else hideSyncCancelBtn(); } function settleSyncButton(label) { if (!syncBtn) return; + // Phase 134 (task 02): EVERY settle hides the Cancel control — + // every terminal (sync success/failed/cancelled, upload + // success/failed) and both idle settles pass through here. + hideSyncCancelBtn(); syncBtn.disabled = false; syncBtn.removeAttribute("aria-busy"); syncBtn.removeAttribute("title"); @@ -868,6 +944,27 @@ export async function mount(root) { if (syncErrorBanner) syncErrorBanner.hidden = true; } + /* Phase 134 (task 02): the Cancel control's show/hide — visible + + * enabled ONLY while a SYNC run is in flight (the upload job is NOT + * cancellable in this phase). Show = unhide + enable + the aria + * label (the accessible name stays stable — the visible "Cancel" + * text is the WCAG 2.4 name); hide = hide + disable, so a hidden + * control is never focusable or activatable in any state (the + * never-stale contract — the settle paths below are the only hides, + * and they run on every terminal). */ + function showSyncCancelBtn() { + if (!syncCancelBtn) return; + syncCancelBtn.hidden = false; + syncCancelBtn.disabled = false; + syncCancelBtn.setAttribute("aria-label", "Cancel sync"); + } + + function hideSyncCancelBtn() { + if (!syncCancelBtn) return; + syncCancelBtn.hidden = true; + syncCancelBtn.disabled = true; + } + /* ---------- sync failure modal (recreated here since the navbar button is gone) ---------- */ let syncModal = null; let syncModalReturnFocus = null; @@ -936,6 +1033,23 @@ export async function mount(root) { showSyncModal(error); } + /* Phase 134 (task 02): the cancelled terminal — the NEUTRAL result + * (A1: cancel is intent, not failure — no error banner, no modal, + * no error styling): settle the main button retry-ready (which also + * hides the Cancel control), announce "Sync cancelled" on the + * aria-live result line, hide the error banner (a failed run's + * banner must not survive the cancel), and emit the status frame + * (the bor:sync-status detail is the status object, as for every + * state). The settle's "Sync sources" label lands on #sync-result + * FIRST and is superseded by the result line here — the main + * button's title keeps "Sync sources" (the retry-ready name). */ + function applySyncCancelled(status) { + settleSyncButton("Sync sources"); + if (syncResult) syncResult.textContent = "Sync cancelled"; + hideSyncError(); + emitSyncStatus(status); + } + function applySyncIdle(status) { settleSyncButton("Sync sources"); emitSyncStatus(status || { state: "idle" }); @@ -1005,7 +1119,16 @@ export async function mount(root) { applySyncFailure(syncStatus); return; } - // 5. upload success: settle + catalog refresh (A3 — the upload's + // 5. sync cancelled: the NEUTRAL terminal (phase 134 task 02 — + // A1: cancel is intent, not failure — no banner, no modal): + // the "Sync cancelled" result line + the retry-ready main + // button; the settle also hides the Cancel control. + if (syncStatus.state === "cancelled") { + stopSyncPolling(); + applySyncCancelled(syncStatus); + return; + } + // 6. upload success: settle + catalog refresh (A3 — the upload's // counts live on the Sources page; #sync-result stays empty). if (uploadStatus && uploadStatus.state === "success") { stopSyncPolling(); @@ -1016,7 +1139,7 @@ export async function mount(root) { loadTree(); return; } - // 6. upload failed: settle only — the failure is the Sources page's + // 7. upload failed: settle only — the failure is the Sources page's // error banner, never this page's (A3). if (uploadStatus && uploadStatus.state === "failed") { stopSyncPolling(); @@ -1026,7 +1149,7 @@ export async function mount(root) { emitSyncStatus({ state: "idle" }); return; } - // 7. both idle: settle retry-ready. + // 8. both idle: settle retry-ready. stopSyncPolling(); applySyncIdle(syncStatus); }; @@ -1068,6 +1191,49 @@ export async function mount(root) { }); } + /* Phase 134 (task 02): the cancel request — the §7.4 never-stale + * contract applies exactly as to the main button: the control + * disables IMMEDIATELY (one request at a time — no double fire) + * BEFORE the fetch, and #sync-result clears with it. + * • 403 → the whoami-backstop branch (stop polling, hide the + * main button, settle idle) — the same depth as every + * other 403 in this module; + * • 409 → NO-OP: the run ended between the tick and the click — + * the poll settles on the next tick (its settle hides + * the control); no error copy, no state change; + * • 202 → nothing: the control stays disabled — the poll's + * `cancelled` branch settles everything (the run may + * already be terminal by the next tick — the status + * object, not the request, is authoritative); + * • any other non-2xx / network failure → the phase-55 neutral + * retry copy on #sync-result + the control re-enables (the run + * is still going — the owner can retry). */ + async function requestCancel() { + if (!syncCancelBtn) return; + syncCancelBtn.disabled = true; // FIRST — never stale, one request at a time + if (syncResult) syncResult.textContent = ""; + let r; + try { + r = await fetch("/api/sync/cancel", { method: "POST" }); + } catch { + // Network failure: the run is still going — the phase-55 + // neutral retry copy + the control re-enables (retryable). + if (syncResult) syncResult.textContent = "Couldn't cancel the sync — try again."; + syncCancelBtn.disabled = false; + return; + } + if (r.status === 403) { + stopSyncPolling(); + if (syncBtn) syncBtn.hidden = true; + applySyncIdle(); + return; + } + if (r.status === 409) return; // race: the run ended — the poll settles + if (r.ok) return; // 202: the poll's cancelled branch settles everything + if (syncResult) syncResult.textContent = "Couldn't cancel the sync — try again."; + syncCancelBtn.disabled = false; // the run is still going — retry + } + /* Load-time re-attach (ADMIN ONLY): a running run re-enters running * state, a terminal run renders its last result. Phase 64 (A3): with * the sync IDLE, an in-flight background upload RUN (phase 90: unpack @@ -1106,6 +1272,12 @@ export async function mount(root) { applySyncFailure(status); return; } + // Phase 134 (task 02): a reload during/right after a cancel shows + // the neutral result — never a stuck running state. + if (status.state === "cancelled") { + applySyncCancelled(status); + return; + } // Sync idle: check the SECOND job — an in-flight upload run // re-attaches (the bare "Importing…" label — phase 90: unpack + // register only). @@ -1129,6 +1301,12 @@ export async function mount(root) { syncBtn.addEventListener("click", startSync); initSyncButton(); } + // Phase 134 (task 02): the Cancel control — armed next to the main + // button (the control is visible only while a sync run is in + // flight, so it can never be reached in any other state). + if (syncCancelBtn) { + syncCancelBtn.addEventListener("click", requestCancel); + } const tbody = root.querySelector("#docs-tbody"); const emptyEl = root.querySelector("#sources-empty"); diff --git a/frontend/assets/styles.css b/frontend/assets/styles.css index c1b4155..fad352e 100644 --- a/frontend/assets/styles.css +++ b/frontend/assets/styles.css @@ -2130,6 +2130,41 @@ a.answer-image:hover { text-decoration: underline; } /* flat — the caption und border-color: var(--err-line); } .sync-btn.is-error:hover { background: var(--err-bg); color: var(--err-ink); } +/* Phase 134 (task 02): the Cancel control — the in-flight sync run's + cooperative stop (visible ONLY while a SYNC run is in flight; the + upload job is not cancellable in this phase — sources.js owns the + show/hide, the same never-stale contract as the main button). The + SECONDARY pill language of the house button family (the + .kb-summary-cancel pair): transparent fill, 1px --line border, + --ink-soft text (5.1:1 on --surface, AA), the global 3px + :focus-visible ring, the >=44px touch floor. :disabled = the cancel + request in flight (one request at a time — never stale); the hover + is guarded on :not(:disabled) so a disabled button under the cursor + never reads as actionable. The [hidden] rule keeps the attribute + authoritative over the display above (the .kb-summary-edit pattern). + No layout regression on the sync-control row: the row already + flexes with a 1rem gap (desktop) and wraps (mobile, where the pill + goes full-width like the main button — the 360px case). */ +.sync-cancel-btn { + display: inline-flex; + align-items: center; + justify-content: center; + min-height: 44px; + padding: 0.5rem 0.9rem; + border-radius: 999px; + border: 1px solid var(--line); + background: transparent; + color: var(--ink-soft); /* 5.1:1 on --surface (AA) */ + font: inherit; + font-weight: 600; + font-size: 0.95rem; + white-space: nowrap; + cursor: pointer; + flex-shrink: 0; +} +.sync-cancel-btn:hover:not(:disabled) { background: var(--brand-soft); color: var(--brand-ink); } +.sync-cancel-btn:disabled { opacity: 0.6; cursor: default; } /* the cancel request is in flight */ +.sync-cancel-btn[hidden] { display: none; } /* the hidden attr must beat the display above */ .sync-icon { width: 16px; height: 16px; display: block; flex: 0 0 auto; } .sync-btn .sync-icon.is-spinning { animation: spin 1s linear infinite; } @media (prefers-reduced-motion: reduce) { @@ -5421,6 +5456,7 @@ a.answer-image:hover { text-decoration: underline; } /* flat — the caption und the accessible names. */ .page-head-row { flex-wrap: wrap; } .sync-btn { width: 100%; } + .sync-cancel-btn { width: 100%; } /* phase 134: the Cancel pill stacks full-width under the Sync pill (the row already wraps) */ .sync-label { max-width: none; flex: 1 1 auto; min-width: 0; } .history-refresh { width: 100%; } .history-refresh svg { display: block; } diff --git a/frontend/index.html b/frontend/index.html index 2811e1c..aea5fc2 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -466,6 +466,15 @@ Sync sources + +

Every file indexed from your configured sources — git repositories, diff --git a/tests/e2e/test_sync_cancel.py b/tests/e2e/test_sync_cancel.py new file mode 100644 index 0000000..2e7b450 --- /dev/null +++ b/tests/e2e/test_sync_cancel.py @@ -0,0 +1,598 @@ +"""Phase 134 (task 03) E2E (Playwright, mock-only): the owner's cancel +path pinned end to end against the REAL pipeline (the owner's TODO item, +L3 — "Need to be able to cancel a sync after I've clicked the button."). + +While a sync is in flight the Sources page shows a Cancel button; one +click requests the cancel, the pipeline stops at the next boundary, and +the run lands in the new ``cancelled`` TERMINAL state (reported by the +status endpoint with ``error: null`` and rendered by the UI as the +neutral "Sync cancelled" — cancel is intent, not failure: no error +banner, no modal, the controls settle retry-ready). The re-sync after +the partial run then imports the FULL fixture (the importer's +content-hash delta makes the restart clean — a partial cancel never +leaves a corrupted or permanently-incomplete KB). + +Run in isolation (DB must be up: ``podman compose up -d db``; git on +PATH — a documented environment prerequisite, phase 28): + + uv run pytest tests/e2e/test_sync_cancel.py -v --no-cov + +Isolation (the combined-run convention): a per-module app on a +dedicated port (the conftest session app is never started in this +isolated run), the mock LLM on the conftest port, a dedicated +``BOR_SOURCES_DIR`` for the real ``file://`` fixture repo's checkout, +and the shared E2E Postgres truncated before AND after every test. + +MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is not supported — the cancel +window IS the timing fixture, and the phase-64 ``tests/e2e/slow_llm.py`` +delay proxy is the ONLY thing that makes it deterministic: it sleeps +``SLOW_LLM_DELAY_S = 0.3`` before forwarding every LLM request, so the +40-file fixture's import span is ≈ 40 × 0.3 s ≈ 12+ s (plus the prelude +probes, and the change-gated overview + folder summaries on the +re-sync) — wide against the UI's 2 s status poll and against the +tests' ~100 ms ``page.request`` status poll (the slow_llm module's +deterministic-layer convention). The running state, the cancel, the +``cancelled`` terminal, and the re-sync are observed as a SEQUENCE, not +a race — a rerun on a quiet machine passes. The cancel is cooperative +(phase 134 design): a git subprocess in flight at the cancel moment +runs to completion and the run stops at the NEXT boundary (before the +import, between files, or before the overview/summaries span) — but +the full import outlasts the ~0.5 s cancel by ~10×, so the cancelled +terminal's ``files_done`` is always < N_FILES, and the re-sync's +added + unchanged is exactly N_FILES. + +Wall-time guard: the whole module (two syncs under the proxy + page +setup) is budgeted at < 120 s and pinned by the module-scoped +``_wall_guard`` fixture — if the suite drifts, cut the file count, not +the guard. + +Test → observable mapping (Playwright Mapping Rule): +1. ``test_anonymous_cancel_403`` — the admin-only surface: an + anonymous POST /api/sync/cancel is 403 (same as the other two sync + endpoints), and #sync-cancel-btn is never revealed to a logged-out + visitor (the whoami gate). +2. ``test_cancel_when_idle_409`` — no run in flight: 409 with the + exact detail "no sync is running"; the UI shows no cancel + affordance at boot + idle (the button is hidden, the main button + settled retry-ready). +3. ``test_admin_cancel_lifecycle`` — the owner's exact flow: Cancel + visible + enabled while the run is in flight (the 202 enters the + running state synchronously) → click (the control disables + immediately — never stale) → the status endpoint reports the + ``cancelled`` terminal (``error: null``, ``detail: {}``, the + in-flight positions cleared, ``files_done < N_FILES``) → the UI + settles to "Sync cancelled" (main button retry-ready, Cancel + control hidden, no error banner, no modal in the DOM) → the + re-sync imports the FULL fixture ("Synced HH:MM", added + + unchanged == N_FILES, the sentinel row in the drill-down catalog, + the sentinel's content intact in the DB — the cancel never + corrupted the KB). +""" +from __future__ import annotations + +import os +import re +import subprocess +import sys +import time +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +import httpx +import pytest +from playwright.sync_api import Browser, ConsoleMessage, Page, expect +from sqlalchemy import text + +from app.db import SessionLocal +from e2e.auth_helpers import login +from e2e.conftest import ( + ADMIN_PASSWORD, + MOCK_PORT, + SESSION_SECRET, + USE_REAL_LLM, + _wait_http, +) + +REPO = Path(__file__).resolve().parents[2] + +# Phase 79 (task 04, full inventory): the conftest session app owns its +# port in a combined run — this module app binds its own port instead +# (a same-port second uvicorn dies on bind and would drive the wrong +# server). Env-overridable. +APP_PORT = int(os.environ.get("E2E_APP_PORT_SYCNCEL", "8133")) +APP_URL = f"http://127.0.0.1:{APP_PORT}" + +#: The slow-LLM proxy's port (the conftest's mock LLM stays on +#: MOCK_PORT — the proxy fronts it). +SLOW_PORT = int(os.environ.get("E2E_SLOW_LLM_PORT_SYCNCEL", "8907")) +SLOW_URL = f"http://127.0.0.1:{SLOW_PORT}" + +#: Per-LLM-request delay on the proxy — 0.3 s: the 40-file fixture's +#: import span is ≈ 12+ s, a wide, deterministic cancel window against +#: the UI's 2 s poll and the tests' ~100 ms status poll (the module +#: docstring's timing paragraph). +SLOW_DELAY_S = "0.3" + +#: The fixture repo's directory name — the source name the import +#: records for it (the repo_name basename rule, phase 28). +GIT_SOURCE_NAME = "cancel-fixture" + +#: The fixture's file count — the re-sync's full-pass bar (added + +#: unchanged must reach exactly N_FILES after the partial cancel). +N_FILES = 40 + +#: The ONE sentinel file's marker (task 03 step 1) — the assertion +#: target for the re-sync: its presence in the catalog + its intact +#: content in the DB prove the REAL clone was fully indexed and the +#: interrupted run left no partial junk. +SENTINEL = "RESE-CANCEL-SENTINEL-4f1e" +FIXTURE_DOC = "notes/note-40.md" + +#: "Synced HH:MM" — the local-time last-result label (sources.js's +#: fmtSyncTime), any hour/minute (the test_sync_button.py pattern). +SYNCED_LABEL = re.compile(r"Synced \d{1,2}:\d{2}") + +#: The settled SUCCESS result line — "N added" or "N added · M +#: unchanged" (sources.js's fmtSyncResult — updated/pruned are zero on +#: the re-sync: the repo is unchanged and nothing was pruned). +RESULT_LINE = re.compile(r"^(\d+) added(?: · (\d+) unchanged)?$") + +#: Settle budgets: the cancel terminal lands ~1 s after the click and +#: the UI's 2 s poll settles at most one tick after the terminal +#: (15 s is ~7× the worst case); the re-sync is ≈ 45 LLM calls × +#: 0.3 s ≈ 14 s under the proxy (60 s is ~4×). +SYNC_SETTLE_TIMEOUT_MS = 15_000 +RESYNC_TIMEOUT_MS = 60_000 + +#: The browser logs "Failed to load resource: the server responded with +#: a status of 403" for the anonymous endpoint probes (the whoami gate +#: + the admin-only status fetch) — expected noise, not a defect. +_CONSOLE_NOISE = re.compile(r"Failed to load resource", re.IGNORECASE) + + +def _git(cwd: Path, *args: str) -> None: + """Run git in *cwd*; a non-zero exit fails the fixture loudly.""" + proc = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True) + if proc.returncode != 0: + raise AssertionError(f"git {' '.join(args)} failed: {proc.stderr.strip()}") + + +def _note_md(k: int) -> str: + """One deterministic fixture note — unique per file (each one + imports independently under the content-hash delta); the LAST file + is the sentinel (the re-sync's assertion target).""" + marker = SENTINEL if k == N_FILES else f"RESE-CANCEL-NOTE-{k:02d}" + return ( + f"# Note {k:02d}\n" + "\n" + f"Deterministic homelab note number {k} of the {GIT_SOURCE_NAME} " + "fixture repo. It exists only to give the sync a wide, " + "file-by-file cancel window under the slow-LLM proxy — every " + "file is unique, so every one imports independently.\n" + "\n" + f"Marker: {marker}\n" + ) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def sync_git_repo(tmp_path_factory: pytest.TempPathFactory) -> Path: + """A real one-commit git repo the sync must clone (the + ``test_sync_button.sync_git_repo`` pattern): N_FILES small A9-format + ``.md`` files under ``notes/`` — the ~12+ s import span under the + 0.3 s proxy that IS the cancel window (the module docstring's + timing paragraph).""" + root = tmp_path_factory.mktemp("sync_cancel_git") + repo = root / GIT_SOURCE_NAME + (repo / "notes").mkdir(parents=True) + for k in range(1, N_FILES + 1): + (repo / "notes" / f"note-{k:02d}.md").write_text(_note_md(k), encoding="utf-8") + _git(repo, "init", "-q") + _git(repo, "add", "-A") + _git( + repo, + "-c", "user.email=e@x", "-c", "user.name=t", + "-c", "commit.gpgsign=false", # the fixture commit never signs + "commit", "-qm", "one", + ) + assert (repo / ".git").is_dir() + return repo + + +@pytest.fixture(scope="module") +def slow_llm(mock_llm: int) -> Iterator[int]: + """The delay-injecting reverse proxy in front of the mock LLM + (``tests/e2e/slow_llm.py``, the conftest spawn pattern) — the ONLY + thing that makes the cancel window deterministic (the module + docstring's timing paragraph).""" + if USE_REAL_LLM: + pytest.skip( + "mock-only suite — the slow proxy fronts the deterministic " + "mock (E2E_REAL_LLM unsupported)" + ) + env = dict(os.environ) + env.pop("DEBUGPY", None) + env["SLOW_LLM_DELAY_S"] = SLOW_DELAY_S + env["E2E_MOCK_PORT"] = str(MOCK_PORT) + proc = subprocess.Popen( + [sys.executable, "-m", "uvicorn", "tests.e2e.slow_llm:app", + "--host", "127.0.0.1", "--port", str(SLOW_PORT), "--log-level", "warning"], + cwd=REPO, + env=env, + ) + try: + _wait_http(f"{SLOW_URL}/v1/models") + yield SLOW_PORT + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + + +@pytest.fixture(scope="module") +def app_server(mock_llm: int, slow_llm: int, sync_git_repo: Path) -> Iterator[str]: + """The real app under test — per-module env (the + ``test_sync_button.app_server`` pattern): ``BOR_LLM_BASE_URL`` is + the SLOW proxy in front of the deterministic mock (the timing + fixture), ``BOR_GIT_SOURCES`` the one real ``file://`` fixture + repo, its own ``BOR_SOURCES_DIR`` (the checkout home).""" + env = dict(os.environ) + env.pop("DEBUGPY", None) + env["BOR_ENVIRONMENT"] = "e2e" + env["BOR_STATIC_DIR"] = str(REPO / "frontend") + env["BOR_LLM_BASE_URL"] = f"{SLOW_URL}/v1" + # Mock-calibrated threshold (conftest pattern) — keeps the suite's + # deterministic gate behavior; production default stays 0.62. + env["BOR_RELEVANCE_THRESHOLD"] = "0.30" + env.setdefault( + "BOR_DATABASE_URL", + "postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese", + ) + # Phase 16: admin auth must be set or create_app() refuses to boot. + env["BOR_ADMIN_PASSWORD"] = ADMIN_PASSWORD + env["BOR_SESSION_SECRET"] = SESSION_SECRET + # The sync's subject — one real local repo (N_FILES files). + env["BOR_GIT_SOURCES"] = f"file://{sync_git_repo}" + env["BOR_SOURCES_DIR"] = str(sync_git_repo.parent / "checkouts") + proc = subprocess.Popen( + [sys.executable, "-m", "uvicorn", "app.main:app", + "--host", "127.0.0.1", "--port", str(APP_PORT), "--log-level", "warning"], + cwd=REPO, + env=env, + ) + try: + _wait_http(f"{APP_URL}/api/health") + yield APP_URL + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + + +@pytest.fixture(scope="module") +def app_url(app_server: str) -> str: + return app_server + + +def _truncate_all() -> None: + """Fresh registry + KB (the E2E isolation pattern): the shared + Postgres may carry leftover rows from other suites, and a leftover + git_sources row would pollute the source list (effective_sources + prefers the DB list over the app's BOR_GIT_SOURCES env fallback).""" + with SessionLocal() as db: + db.execute( + text( + "TRUNCATE chunks, documents, query_log, steering_notes, " + "kb_overview, git_sources, folder_summaries" + ) + ) + db.commit() # without the commit the TRUNCATE rolls back (the house pattern) + + +def _wait_sync_settled(app_url: str, timeout_s: float = 180.0) -> None: + """No background sync may leak across tests (the run state lives in + the app's memory, and a still-running run would keep importing into + the NEXT test's truncated KB): wait for the app's sync status to be + non-running BEFORE the truncate. Own admin session (the endpoint is + admin-only) — usually a no-op: the tests settle only after their + run's terminal state.""" + with httpx.Client(base_url=app_url, timeout=5.0) as client: + r = client.post("/api/login", json={"password": ADMIN_PASSWORD}) + assert r.status_code == 204, r.text + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + r = client.get("/api/sync/status") + assert r.status_code == 200, r.text + if r.json()["state"] != "running": + return + time.sleep(0.2) + raise AssertionError(f"a sync was still running at a test boundary: {app_url}") + + +@pytest.fixture(autouse=True) +def _clean(app_url: str, db_ready: None) -> Iterator[None]: + """Per-test KB isolation (the phase-96 cleanup pattern): the shared + E2E Postgres is truncated before AND after every test; the pre-wait + keeps a leaked run from the previous test (the module app is + shared) from racing the truncate.""" + _wait_sync_settled(app_url) + _truncate_all() + yield + _truncate_all() + + +@pytest.fixture(scope="module", autouse=True) +def _wall_guard() -> Iterator[None]: + """The task's wall-time guard: the whole module (two syncs under + the 0.3 s proxy + page setup) must fit inside 120 s — if it + drifts, cut the file count, not the guard.""" + t0 = time.monotonic() + yield + elapsed = time.monotonic() - t0 + assert elapsed < 120.0, ( + f"module wall time {elapsed:.1f}s blew the 120 s budget — cut " + "the fixture file count, not the guard" + ) + + +@pytest.fixture() +def page(browser: Browser) -> Iterator[Page]: + """The conftest's page + the console-error collector (task 02's + criterion: the page renders with NO console errors): an uncaught JS + exception (pageerror) or a console error that is not the browser's + expected "Failed to load resource" 403 noise (the anonymous + endpoint probes) fails the owning test at teardown.""" + pg = browser.new_page(viewport={"width": 1280, "height": 800}) + errors: list[str] = [] + + def _on_console(msg: ConsoleMessage) -> None: + if msg.type == "error" and not _CONSOLE_NOISE.search(msg.text): + errors.append(msg.text) + + pg.on("console", _on_console) + pg.on("pageerror", lambda err: errors.append(str(err))) + try: + yield pg + finally: + pg.close() + assert not errors, f"console/page errors: {errors}" + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _wait_sync_state( + page: Page, app_url: str, states: set[str], timeout_s: float = 30.0 +) -> dict[str, Any]: + """Poll the (cookie-authenticated) status endpoint at ~100 ms + cadence until the run's state is in *states* — the slow_llm + module's deterministic layer (the ``test_sync_button._wait_sync_done`` + helper generalized to a set of terminal states: here ``{"cancelled"}`` + and ``{"success", "failed"}`` shapes for the re-sync).""" + deadline = time.monotonic() + timeout_s + body: dict[str, Any] = {} + while time.monotonic() < deadline: + r = page.request.get(f"{app_url}/api/sync/status") + assert r.status == 200, r.text + body = r.json() + if body["state"] in states: + return body + time.sleep(0.1) + raise AssertionError(f"sync did not reach {sorted(states)}: {body}") + + +def _go_top(page: Page) -> None: + """Back to the top level (no-op when already there — the + breadcrumb is hidden at the top).""" + if not page.locator("#kb-crumb").is_hidden(): + page.locator("#kb-crumb a.kb-crumb-link").first.click() + + +def _expect_fixture_row(page: Page) -> None: + """The REAL clone was fully imported after the cancel — the + sentinel's fixture path is listed. Phase 97: the catalog is the + drill-down tree, so drill source → folder ``notes`` first.""" + _go_top(page) + page.click(f'#folders-tbody a.folder-link:text-is("{GIT_SOURCE_NAME}")') + page.click('#folders-tbody a.folder-link:text-is("notes")') + expect(page.locator("#docs-tbody tr", has_text=FIXTURE_DOC)).to_have_count(1) + + +# --------------------------------------------------------------------------- +# 1. Anonymous: locked endpoint, the Cancel control is never revealed +# --------------------------------------------------------------------------- + + +def test_anonymous_cancel_403(page: Page, app_url: str, db_ready: None) -> None: + """The admin-only surface (same as the other two sync endpoints): + an anonymous POST /api/sync/cancel is 403, and #sync-cancel-btn is + NEVER revealed to a logged-out visitor (the whoami gate — the + button ships hidden and only a running sync state reveals it, which + the anonymous page never enters).""" + page.goto(f"{app_url}/sources.html") + expect(page.locator("#sync-btn")).to_be_hidden() + expect(page.locator("#sync-cancel-btn")).to_be_hidden() + # The same whoami gate that hides the buttons also hides the admin + # nav link and shows the catalog sign-in gate. + expect(page.locator("#nav-sources")).to_be_hidden() + expect(page.locator("#sources-gate")).to_be_visible() + assert page.request.get(f"{app_url}/api/sync/status").status == 403 + assert page.request.post(f"{app_url}/api/sync/cancel").status == 403 + + +# --------------------------------------------------------------------------- +# 2. Idle: 409 with the exact detail, no cancel affordance in the UI +# --------------------------------------------------------------------------- + + +def test_cancel_when_idle_409(page: Page, app_url: str, db_ready: None) -> None: + """No run in flight (no test before this one started one — the + anonymous test never does): POST /api/sync/cancel is 409 with the + exact detail "no sync is running" (no run was started, and no + terminal one is in memory — the module app's state is fresh), and + the UI shows NO cancel affordance at boot + idle: the button is + hidden and the main button is settled retry-ready. The rejected + cancel changes nothing.""" + login(page, app_url) # lands on /sources.html (the button's home) + + btn = page.locator("#sync-btn") + expect(btn).to_be_visible() + expect(btn).to_be_enabled() + expect(page.locator("#sync-label")).to_have_text("Sync sources") + cancel = page.locator("#sync-cancel-btn") + expect(cancel).to_be_hidden() # boot + idle: no cancel affordance + + r = page.request.post(f"{app_url}/api/sync/cancel") + assert r.status == 409, r.text + assert r.json()["detail"] == "no sync is running" + + # The rejected cancel is a no-op: still settled + hidden. + expect(btn).to_be_enabled() + expect(page.locator("#sync-label")).to_have_text("Sync sources") + expect(cancel).to_be_hidden() + + +# --------------------------------------------------------------------------- +# 3. The owner's exact flow: Cancel visible → click → "Sync cancelled" +# → the clean re-sync (the full fixture, the sentinel intact) +# --------------------------------------------------------------------------- + + +def test_admin_cancel_lifecycle(page: Page, app_url: str, db_ready: None) -> None: + """The owner's TODO item, L3, end to end: a running sync shows the + Cancel button; one click requests the cancel and the control + disables IMMEDIATELY (never stale); the pipeline stops at the next + boundary and lands in the ``cancelled`` terminal (the status + endpoint reports ``error: null`` + ``detail: {}`` with the + in-flight positions cleared and ``files_done < N_FILES`` — the + full import outlasts the ~0.5 s cancel by ~10×); the UI settles to + the NEUTRAL "Sync cancelled" (main button retry-ready, Cancel + control hidden, no error banner, no modal — a cancel is intent, + not failure); and the re-sync after the partial run imports the + FULL fixture ("Synced HH:MM", added + unchanged == N_FILES, the + sentinel row in the catalog, the sentinel's content intact — the + cancel never corrupted the KB).""" + page.set_default_timeout(30_000) + login(page, app_url) # lands on /sources.html (the button's home) + + btn = page.locator("#sync-btn") + cancel = page.locator("#sync-cancel-btn") + expect(btn).to_be_visible() + expect(btn).to_be_enabled() + expect(page.locator("#sync-label")).to_have_text("Sync sources") + expect(cancel).to_be_hidden() # idle: no cancel affordance + + # --- run 1: start + cancel ------------------------------------------- + btn.click() + # The 202 enters the running state SYNCHRONOUSLY (the click handler + #'s 202 branch, before the first 2 s poll tick): the main button + # disables + "Syncing…", and the Cancel control reveals + enables + # (enterSyncRunningState, kind "sync"). + expect(btn).to_be_disabled() + expect(page.locator("#sync-label")).to_have_text("Syncing…") + expect(cancel).to_be_visible() + expect(cancel).to_be_enabled() + + # The deterministic layer (the slow_llm module's convention): + # confirm via the status endpoint that the run is actually in + # flight (page.request poll at ~100 ms). + _wait_sync_state(page, app_url, {"running"}) + + # The owner's click: the control disables IMMEDIATELY — the first + # line of requestCancel(), before the fetch (never stale, one + # request at a time). + cancel.click() + expect(cancel).to_be_disabled() + + # --- the cancelled terminal (the deterministic layer) ----------------- + body = _wait_sync_state(page, app_url, {"cancelled"}, timeout_s=30.0) + assert body["state"] == "cancelled" + assert body["error"] is None # a cancel is intent, not failure + assert body["detail"] == {} # a partial run reports no counts + assert body["started_at"] is not None + assert body["finished_at"] is not None + # The terminal clears the in-flight positions but keeps the run's + # final file counts (the phase-64/98/129 keep-final-counts + # convention, the phase-134 fifth state end to end). + assert body["current_file"] is None + assert body["phase"] is None + assert body["current_summary"] is None + assert body["current_chunk"] == 0 + assert body["chunks_total"] == 0 + # The cancel interrupted the run: the import (a full pass is ≥12 s + # under the 0.3 s proxy) had no time to finish all N_FILES. + assert body["files_done"] < N_FILES, body + + # --- the neutral UI settle (timeout-bound: the 2 s poll sees the + # terminal within one tick) ------------------------------------------- + expect( + page.locator("#sync-label") + ).to_have_text("Sync sources", timeout=SYNC_SETTLE_TIMEOUT_MS) + expect(btn).to_be_enabled() # retry-ready + expect(btn).not_to_have_attribute("aria-busy") + expect(cancel).to_be_hidden() + expect(page.locator("#sync-result")).to_have_text( + "Sync cancelled", timeout=SYNC_SETTLE_TIMEOUT_MS + ) + # A1: the cancel is never rendered as a failure — no error banner, + # no error modal in the DOM. + expect(page.locator("#sync-error-banner")).to_be_hidden() + assert page.locator(".sync-modal-backdrop").count() == 0 + + # --- run 2: the re-sync is clean -------------------------------------- + btn.click() + expect(btn).to_be_disabled() + expect(page.locator("#sync-label")).to_have_text("Syncing…") + expect(cancel).to_be_visible() # the fresh run is cancellable again + expect(page.locator("#sync-label")).to_have_text( + SYNCED_LABEL, timeout=RESYNC_TIMEOUT_MS + ) + expect(btn).to_be_enabled() + expect(cancel).to_be_hidden() + + # The FULL fixture landed: added + unchanged == N_FILES — the + # cancelled run's partial import is idempotent under the + # content-hash delta (already-indexed files re-import unchanged, + # the rest are added; the interrupted file's uncommitted + # transaction rolled back). + line = page.locator("#sync-result").inner_text() + m = RESULT_LINE.fullmatch(line) + assert m, f"unexpected result line: {line!r}" + added = int(m.group(1)) + unchanged = int(m.group(2) or 0) + assert added >= 1, (line, body["files_done"]) + assert added + unchanged == N_FILES, (line, body["files_done"]) + + # The KB is complete AND uncorrupted: every fixture file is + # indexed, and the sentinel's content is intact (the REAL clone's + # full content — not a partial write from the interrupted run). + with SessionLocal() as db: + n_docs = db.execute( + text("SELECT count(*) FROM documents WHERE source = :s"), + {"s": GIT_SOURCE_NAME}, + ).scalar_one() + sentinel_content = db.execute( + text("SELECT content FROM documents WHERE source = :s AND path = :p"), + {"s": GIT_SOURCE_NAME, "p": FIXTURE_DOC}, + ).scalar_one() + assert n_docs == N_FILES, n_docs + assert SENTINEL in sentinel_content + + # And the sentinel row is present in the catalog (the + # _expect_fixture_row pattern — drill source → notes). + _expect_fixture_row(page) + + # No error surface on the success settle either. + expect(page.locator("#sync-error-banner")).to_be_hidden() + assert page.locator(".sync-modal-backdrop").count() == 0 diff --git a/tests/integration/test_sync_api.py b/tests/integration/test_sync_api.py index 2e773d3..a61c4b6 100644 --- a/tests/integration/test_sync_api.py +++ b/tests/integration/test_sync_api.py @@ -75,6 +75,17 @@ orthogonal: it still calls the generator exactly as before (``only_missing=True``, no change set). The fakes above record the ``changed_docs=`` kwarg (``FakeFolderSummaries.changed_docs_sets``) so the wiring is asserted, not inferred. + +Phase 134 (task 01): the cancel — anonymous 403 on the new +``POST /api/sync/cancel``; admin: 409 ``no sync is running`` idle and +after a terminal run, 202 ``cancel requested`` while a run is in +flight (the run is parked inside the import — the file's existing +gated-import pattern over the app's own event loop); the status then +reports the new ``cancelled`` terminal (``error`` null, ``detail`` +empty, the final file counts kept, the chunk position cleared) and a +second run after the cancel completes to ``success`` (the restart is +clean — the stale cancel event did not bleed in, and only the +completed run bumps the sources version). """ from __future__ import annotations @@ -111,12 +122,18 @@ from tests.fakes import FakeEmbedder def _fresh_sync_state() -> Iterator[None]: """The module-level status object + task are process-global: reset them around every test (both before — a previous test's terminal state - would leak into the idle assertion — and after).""" + would leak into the idle assertion — and after). + + Phase 134 (task 01): the module ``_cancel_event`` joins the reset — + a stale (set) event from a previous test's cancelled run would + otherwise bleed into the next run's boundary checks.""" sync_api._status = sync_api.SyncStatus() sync_api._task = None + sync_api._cancel_event = asyncio.Event() yield sync_api._status = sync_api.SyncStatus() sync_api._task = None + sync_api._cancel_event = asyncio.Event() @pytest.fixture(autouse=True) @@ -422,6 +439,15 @@ def test_anonymous_gets_403_on_both_endpoints(client: TestClient) -> None: assert r.json() == {"detail": "admin only"} +def test_anonymous_gets_403_on_cancel(client: TestClient) -> None: + """Phase 134 (task 01): the cancel endpoint rides the router's + ``require_admin`` dependency — anonymous gets the same 403 as the + other sync surface (the completion criterion's 403 pin).""" + r = client.post("/api/sync/cancel") + assert r.status_code == 403 + assert r.json() == {"detail": "admin only"} + + # --- admin: success -------------------------------------------------------- @@ -1864,3 +1890,106 @@ def test_sync_builds_include_hidden_map_by_root_string_with_or( # string — the flag-off row's False is overridden by the True. assert fake_import.sources == [[tmp_path / "bor" / "shared", tmp_path / "bor" / "shared"]] assert fake_import.include_hidden_maps == [{shared: True}] + + +# --- phase 134 (task 01): the cancel (the fifth state) ------------------- + + +def test_cancel_mid_run_lands_in_cancelled_then_resyncs_clean( + sync_client: TestClient, + monkeypatch: pytest.MonkeyPatch, + db: Session, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """Phase 134 (task 01), the API contract end to end: a real run + started through the API is cancelled mid-run (parked inside the + import) → ``POST /api/sync/cancel`` answers 202 ``cancel + requested``, the status reports the new ``cancelled`` terminal + (``error`` null, ``detail`` empty, the final file counts kept, the + positions cleared, the PLAN §9 line logged); a cancel with no run + in flight is 409 ``no sync is running`` (idle AND after a + terminal); and the FOLLOWING run starts cleanly (202 — the stale + cancel event did not bleed in) and completes to ``success`` (the + restart after a partial cancel is clean — the importer's hash + delta re-imports what the cancel skipped).""" + repo_url = f"file://{tmp_path / 'repo.git'}" + _seed(db, repo_url) + _stub_env(monkeypatch) + monkeypatch.setattr( + sync_api, + "get_settings", + lambda: _settings(sources_dir=str(tmp_path / "bor")), + ) + _, fake_clone = _fake_clone() + monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone) + _stub_probe(monkeypatch) + import_release = threading.Event() + gated_import = _GatedImport( + ImportSummary(files=3, added=1, updated=1, unchanged=1), + threading.Event(), + import_release, + ) + monkeypatch.setattr(sync_api, "import_sources", gated_import) + monkeypatch.setattr(sync_api, "regenerate_overview", FakeOverview(ok=True)) + monkeypatch.setattr(sync_api, "generate_folder_summaries", FakeFolderSummaries()) + + _login(sync_client) + # No run in flight (idle) → 409, the exact detail (the UI's + # harmless race — no error copy). + r = sync_client.post("/api/sync/cancel") + assert r.status_code == 409 + assert r.json() == {"detail": "no sync is running"} + + assert sync_client.post("/api/sync").status_code == 202 + try: + assert gated_import.started.wait(5.0), "the run never reached the import" + # Parked inside the import (between files): the cancel request. + with caplog.at_level(logging.INFO, logger="app.api.sync"): + r = sync_client.post("/api/sync/cancel") + assert r.status_code == 202 + assert r.json() == {"detail": "cancel requested"} + body = _poll(sync_client, "cancelled", timeout=5.0) + finally: + import_release.set() + + assert body["error"] is None # a cancel is intent, not failure + assert body["detail"] == {} + assert body["started_at"] is not None + assert body["finished_at"] is not None + assert body["current_file"] is None + assert body["phase"] is None + assert body["current_summary"] is None + # The final file counts are kept (the phase-64/98 convention) ... + assert body["files_done"] == 1 and body["files_total"] == 3 + # ... and the chunk position clears (the phase-129 convention). + assert body["current_chunk"] == 0 and body["chunks_total"] == 0 + # PLAN §9: the ample-logging line for the converted cancel. + assert any( + r.getMessage() + == "sync: cancelled by user state=cancelled files_done=1 files_total=3" + for r in caplog.records + ) + # The cancelled run never reached the version bump. + assert current_sources_version(db) == 0 + + # No run in flight anymore (the task is done) → 409 again. + r = sync_client.post("/api/sync/cancel") + assert r.status_code == 409 + assert r.json() == {"detail": "no sync is running"} + + # The restart is clean: a fresh run starts (202 — the stale event + # did not bleed in; start_sync cleared it) and the fresh (full) + # import completes to success. + second_import = FakeImportSources( + ImportSummary(files=3, added=1, updated=1, unchanged=1) + ) + monkeypatch.setattr(sync_api, "import_sources", second_import) + assert sync_client.post("/api/sync").status_code == 202 + body = _poll(sync_client, "success") + assert body["error"] is None + assert body["detail"]["added"] == 1 + assert body["detail"]["sources_version"] == 1 # only the completed run bumps + assert current_sources_version(db) == 1 + assert len(second_import.sources) == 1 + assert second_import.prune_flags == [True] diff --git a/tests/unit/test_frontend_sync_upload.py b/tests/unit/test_frontend_sync_upload.py index bd3535f..a8c9643 100644 --- a/tests/unit/test_frontend_sync_upload.py +++ b/tests/unit/test_frontend_sync_upload.py @@ -82,6 +82,16 @@ contract as the count part, and the UNTRUNCATED full label — main + --mono, the rule right after .sync-label-count). A node harness (the test-document-viewer pattern) behaviorally pins the rendered parts + the full title across every label state. + +Phase 134 (task 02) extends the node harness to the Cancel control: +the extracted functions gain the REAL show/hide helpers + the REAL +settleSyncButton (step 8 now runs the settle itself — it goes through +the writer as before, plus it hides the Cancel control), and every +row snapshots the control's hidden/disabled pair — the behavioral +contract: shown + enabled in EVERY sync-running state (the entry +shows it for kind "sync"), hidden + disabled for the upload run +(the upload job is NOT cancellable in this phase) and after the +settle (settleSyncButton always hides it). """ from __future__ import annotations @@ -514,6 +524,16 @@ const syncBtn = { classList: { add() {}, remove() {} }, }; const syncIcon = { classList: { add() {}, remove() {} } }; +// Phase 134 (task 02): the Cancel control — ships hidden + disabled +// (the markup's state); the extracted REAL show/hide helpers mutate +// it (show also sets the aria-label — the attrs pocket records it). +const syncCancelBtn = { + hidden: true, + disabled: true, + attrs: {}, + setAttribute(k, v) { this.attrs[k] = v; }, + removeAttribute(k) { delete this.attrs[k]; }, +}; """ _CHUNK_NODE_TAIL = """ @@ -523,6 +543,7 @@ const show = (tag) => { syncLabelMain.textContent, syncLabelCount.textContent, syncLabelCount.hidden, syncLabelChunk.textContent, syncLabelChunk.hidden, + syncCancelBtn.hidden, syncCancelBtn.disabled, ])); }; // 1. import, batch in flight (the task's fixture values, 14/38): @@ -561,8 +582,10 @@ enterSyncRunningState("sync", null, 4, 4, { summaries_done: 2, summaries_total: 3, current_chunk: 0, chunks_total: 0, }); show("summaries"); -// 8. the idle settle goes through the writer with no chunk text: -setSyncLabel("Sync sources"); +// 8. the idle settle goes through the REAL settle (phase 134 task +// 02: it hides the Cancel control too) — the settle itself goes +// through the writer with no chunk text: +settleSyncButton("Sync sources"); show("idle"); """ @@ -590,16 +613,34 @@ test-document-viewer pattern). The pinned "· chunk n/m" part (the status), the upload run (no status, D2), overview, summaries (its own (n/m) is its position), the idle settle — leaves the chunk part hidden + empty. + + Phase 134 (task 02): every row also snapshots the Cancel + control's hidden/disabled pair — the behavioral contract: + shown + enabled in EVERY sync-running state, hidden + disabled + for the upload run (the upload job is NOT cancellable in this + phase) and after the (real) settle. """ js = _js() fns = "\n".join( _fn(js, name) - for name in ("fmtSyncLabel", "fmtSyncPhaseLabel", "setSyncLabel", "enterSyncRunningState") + for name in ( + "fmtSyncLabel", + "fmtSyncPhaseLabel", + "setSyncLabel", + # Phase 134 (task 02): the running entry's REAL cancel + # show/hide + the REAL settle (step 8) — extracted against + # the syncCancelBtn stub. + "showSyncCancelBtn", + "hideSyncCancelBtn", + "enterSyncRunningState", + "settleSyncButton", + ) ) script = _CHUNK_NODE_HEAD + fns + _CHUNK_NODE_TAIL rows = [json.loads(line) for line in _run_node(script).splitlines() if line] by_tag = {row[0]: row for row in rows} - # row = [tag, title, announcer, main, count, countHidden, chunk, chunkHidden] + # row = [tag, title, announcer, main, count, countHidden, chunk, + # chunkHidden, cancelHidden, cancelDisabled] # 1. the in-flight import — the chunk part renders, the full # untruncated label rides the title + the announcer (A4). @@ -634,6 +675,24 @@ test-document-viewer pattern). The pinned "· chunk n/m" part (the assert row[2] == row[1], (tag, row) assert row[6] == "" and row[7] is True, f"{tag} must hide the chunk part" + # Phase 134 (task 02): the Cancel control's behavioral contract — + # shown + enabled in EVERY sync-running state (the entry shows it + # for kind "sync"), hidden + disabled for the upload run (the + # upload job is NOT cancellable in this phase) and hidden + + # disabled after the real settle (settleSyncButton always hides + # it — every terminal + idle passes through it). + for tag in ("import-inflight", "import-prebatch", "prelude", "click", "overview", "summaries"): + row = by_tag[tag] + assert row[8] is False and row[9] is False, ( + f"{tag}: the Cancel control must be shown + enabled while a sync run is in flight" + ) + for tag in ("upload", "idle"): + row = by_tag[tag] + assert row[8] is True and row[9] is True, ( + f"{tag}: the Cancel control must be hidden + disabled " + "(the upload job is not cancellable / the settle hides it)" + ) + def test_every_other_state_passes_no_chunk_text() -> None: """Source-level pin of the render-only-while-chunks_total>0 diff --git a/tests/unit/test_sync_button.py b/tests/unit/test_sync_button.py index 52057e0..c3b740c 100644 --- a/tests/unit/test_sync_button.py +++ b/tests/unit/test_sync_button.py @@ -59,18 +59,59 @@ positions between ticks (1/38 → 13/38) through the runner's own position clears at every point the import phase ends (the overview/summaries transitions + both terminals) while the FILE counts keep their final values. + +Phase 134 (task 01): the FIFTH state — ``cancelled`` — joins the +contract. The admin-only ``POST /api/sync/cancel`` (the 403 is pinned +in the integration suite — the router's ``require_admin`` dependency, +no DB/HTTP layer here) is 202 ``cancel requested`` while a run is in +flight (it sets the module ``_cancel_event`` AND ``_task.cancel()``) +and 409 ``no sync is running`` idle / after a terminal run. A +user-requested cancel converts the ``CancelledError`` into the +``cancelled`` terminal (``error`` None, ``detail`` empty, the final +file counts kept, the task ends CANCELLED); an UNREQUESTED cancel +(bare ``task.cancel()`` — the app-shutdown path) re-raises with the +state untouched (never swallowed — the completion criterion's pin); +``start_sync`` clears a stale event so the next run completes to +``success``. The cancel runs here are driven through the REAL +``start_sync`` on a worker loop — ``sync_api._task`` is the endpoint's +target — and the cancel is issued FROM that loop thread (a same-thread +``Task.cancel()`` — a cross-thread cancel would schedule through the +loop's selector wake, which a parked loop never receives). + +Phase 134 (task 02): the FRONTEND cancel contract is pinned at the +source level (the browser behavior is E2E-covered in +tests/e2e/test_sync_cancel.py, task 03): the ship-hidden +``#sync-cancel-btn`` markup right after ``#sync-btn`` (the visible +"Cancel" label is the accessible name — WCAG 2.4), the +``.sync-cancel-btn`` secondary-pill CSS (the 44px floor, the AA +ink-soft pair, the guarded hover, the :disabled + ``[hidden]`` rules, +the full-width mobile rule next to the main button's), the show/hide +helpers (show = unhide + enable + aria-label, hide = hide + disable), +the two choke points (``enterSyncRunningState`` shows for kind +"sync" / hides for kind "upload" — the upload job is NOT cancellable +in this phase — and ``settleSyncButton`` hides on EVERY settle), the +``requestCancel`` never-stale lifecycle (disable FIRST, then +POST /api/sync/cancel; the 403 backstop, the silent 409, the 202 +wait, the phase-55 retry copy + re-enable on any other failure), the +poll tree's new 5th branch (after failed, before the upload branches +— the 7-branch tree is now 8), the neutral ``applySyncCancelled`` +(no modal, no banner, no error styling), and the +``initSyncButton`` re-attach of a ``cancelled`` run. """ from __future__ import annotations import asyncio +import logging import re import threading import time from collections.abc import Callable, Iterator, Sequence from datetime import datetime from pathlib import Path +from typing import Any import pytest +from fastapi import HTTPException from app.api import sync as sync_api from app.config import Settings @@ -754,12 +795,18 @@ def test_sync_modal_respects_reduced_motion() -> None: def fresh_sync_status() -> Iterator[None]: """The module-level status object + task are process-global: reset them before AND after every progress test (the integration suite's - ``_fresh_sync_state`` pattern).""" + ``_fresh_sync_state`` pattern). + + Phase 134 (task 01): the module ``_cancel_event`` joins the reset — + a stale (set) event from a previous test's cancelled run would + otherwise bleed into the next run's boundary checks.""" sync_api._status = sync_api.SyncStatus() sync_api._task = None + sync_api._cancel_event = asyncio.Event() yield sync_api._status = sync_api.SyncStatus() sync_api._task = None + sync_api._cancel_event = asyncio.Event() class _GatedClone: @@ -861,8 +908,8 @@ class _GatedImport: def _patch_sync_seams( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, - fake_import: _GatedImport, - fake_clone: _GatedClone, + fake_import: object, # any import-shaped fake (the phase-134 fakes too) + fake_clone: object, # any clone-shaped fake (the phase-134 fakes too) ) -> None: """The runner's seams, monkeypatched on ``app.api.sync`` (the house mock-import pattern): fresh settings (no ``.env`` leak), a no-op @@ -1333,3 +1380,750 @@ def test_chunk_position_clears_at_phase_transitions_and_terminal( assert s["current_file"] is None assert s["current_chunk"] == 0 and s["chunks_total"] == 0 assert s["files_done"] == 1 and s["files_total"] == 3 + + +# ---------- phase 134 (task 01): the cooperative cancel (5th state) ---------- +# +# The cancel runs are driven through the REAL ``start_sync`` on a +# worker thread's own event loop — ``sync_api._task`` is the running +# task (the cancel endpoint's target), unlike the direct-``_run_sync`` +# tests above. Two driver shapes: +# +# * ``_run_via_start_sync`` — the ACTION runs ON the loop thread once +# the run is in flight (a same-thread ``Task.cancel()`` — a +# cross-thread cancel would schedule through the loop's selector +# wake, which a parked loop never receives); the loop is drained +# afterwards so the runner's terminal step (the +# ``except CancelledError`` conversion) lands before the test reads +# the status. +# * ``_start_run_via_api`` — the OLD house pattern (blocking threading +# gates; the loop thread may park inside a sync span — the +# clone/``file_commit_dates``) with the main thread orchestrating +# the event set / releases. + + +def _fast_clone(url: str, dest: Path | str) -> Path: + """A non-gated ``clone_or_pull`` — the loop thread must STAY FREE + for the loop-thread action, so nothing in the prelude may block it + (the import's gate is an ``asyncio.to_thread`` park — loop-safe).""" + dest = Path(dest) + dest.mkdir(parents=True, exist_ok=True) + (dest / "notes.md").write_text("# repo\ncontent for the KB\n", encoding="utf-8") + return dest + + +def _run_via_start_sync( + action: Callable[[asyncio.AbstractEventLoop], Any], +) -> tuple[threading.Thread, list[BaseException], dict[str, Any]]: + """Start a run through the real ``start_sync`` on a worker thread's + own event loop; once the run is in flight, run *action* ON that + loop thread, then drain the loop so the runner's terminal step + (the ``except CancelledError`` conversion) lands before the test + reads the status. Returns (thread, the exceptions the run + surfaced — the CancelledError of a converted cancel, the action's + result under ``"result"``).""" + errors: list[BaseException] = [] + out: dict[str, Any] = {} + started = threading.Event() + + def _run() -> None: + loop = asyncio.new_event_loop() + try: + asyncio.set_event_loop(loop) + loop.run_until_complete(sync_api.start_sync()) + started.set() + out["result"] = action(loop) + for _ in range(20): + if sync_api._task is not None and sync_api._task.done(): + break + loop.run_until_complete(asyncio.sleep(0)) + assert sync_api._task is not None and sync_api._task.done(), ( + "the runner task never finished" + ) + except BaseException as e: # noqa: BLE001 — surfaced to the test + errors.append(e) + finally: + asyncio.set_event_loop(None) + loop.close() + + thread = threading.Thread(target=_run, daemon=True) + thread.start() + assert started.wait(5.0), "start_sync never ran" + return thread, errors, out + + +def _start_run_via_api() -> tuple[threading.Thread, list[BaseException]]: + """The house background-task pattern over the real ``start_sync`` — + the worker loop thread may park inside a SYNC span (the blocking + clone / ``file_commit_dates`` park); the main thread orchestrates + the event set / gate releases. The runner's surfaced exception + (the CancelledError of a converted cancel) is captured.""" + errors: list[BaseException] = [] + + def _run() -> None: + loop = asyncio.new_event_loop() + try: + asyncio.set_event_loop(loop) + loop.run_until_complete(sync_api.start_sync()) + if sync_api._task is not None: + loop.run_until_complete(sync_api._task) + except BaseException as e: # noqa: BLE001 — surfaced to the test + errors.append(e) + finally: + asyncio.set_event_loop(None) + loop.close() + + thread = threading.Thread(target=_run, daemon=True) + thread.start() + return thread, errors + + +def test_cancel_is_409_idle_and_after_a_terminal_run( + fresh_sync_status: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The 409 contract (the exact detail ``no sync is running``): + idle (never started — ``_task`` None) AND after a terminal run + (``_task`` done) — the harmless race the UI's poll settles (no + error copy, no state change).""" + fake_import = _GatedImport( + ImportSummary(files=1, added=1), threading.Event(), threading.Event() + ) + _patch_sync_seams(monkeypatch, tmp_path, fake_import, _fast_clone) + + # Idle (never started): 409, the state untouched. + with pytest.raises(HTTPException) as idle: + asyncio.run(sync_api.cancel_sync()) + assert idle.value.status_code == 409 + assert idle.value.detail == "no sync is running" + assert sync_api.sync_status()["state"] == "idle" + + # A full (uncancelled) run to the success terminal ... + def act(loop: asyncio.AbstractEventLoop) -> None: + assert fake_import.started.wait(5.0), "the run never reached the import" + fake_import.release.set() + task = sync_api._task + assert task is not None + loop.run_until_complete(task) + + thread, errors, out = _run_via_start_sync(act) + thread.join(10.0) + assert not thread.is_alive() + assert errors == [], f"the runner raised: {errors!r}" + assert sync_api.sync_status()["state"] == "success" + + # ... and after a terminal run the cancel is 409 again. + with pytest.raises(HTTPException) as after: + asyncio.run(sync_api.cancel_sync()) + assert after.value.status_code == 409 + assert after.value.detail == "no sync is running" + # The state is untouched by the rejected cancel. + assert sync_api.sync_status()["state"] == "success" + + +def test_cancel_endpoint_stops_the_in_flight_run( + fresh_sync_status: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """The 202 path, end to end at the function level: a started run + (parked inside the import) + ``cancel_sync`` → 202 ``cancel + requested``; the event is set AND the task is cancelled, the run + converts the ``CancelledError`` into the ``cancelled`` terminal — + ``error`` None, ``detail`` empty, ``finished_at`` set, the FINAL + file counts kept (the hook fired 1/3 before the park), the file / + phase / summary / chunk positions cleared — the PLAN §9 line is + logged, and the task ends CANCELLED (the re-raise; the status + object stays authoritative).""" + fake_import = _GatedImport( + ImportSummary(files=3, added=1, updated=1, unchanged=1), + threading.Event(), + threading.Event(), + ) + _patch_sync_seams(monkeypatch, tmp_path, fake_import, _fast_clone) + + def act(loop: asyncio.AbstractEventLoop) -> dict[str, str]: + assert fake_import.started.wait(5.0), "the run never reached the import" + return loop.run_until_complete(sync_api.cancel_sync()) + + try: + with caplog.at_level(logging.INFO, logger="app.api.sync"): + thread, errors, out = _run_via_start_sync(act) + thread.join(10.0) + finally: + fake_import.release.set() # free the to_thread park thread (non-daemon) + assert not thread.is_alive() + assert out.get("result") == {"detail": "cancel requested"} + # The endpoint did BOTH: the event (the cooperative marker) ... + assert sync_api._cancel_event.is_set() + # ... and the task cancel (the task ended CANCELLED — the re-raise + # happened; a swallowed cancel would have ended the task plain). + # The CancelledError itself is stored in the task (nothing awaits + # it directly — start_sync's create_task), so no thread-level + # exception surfaces — and nothing ELSE may have either. + assert sync_api._task is not None + assert sync_api._task.cancelled() + assert errors == [], f"the runner raised: {errors!r}" + s = sync_api.sync_status() + assert s["state"] == "cancelled" + assert s["error"] is None # intent, not failure + assert s["detail"] == {} + assert s["started_at"] is not None and s["finished_at"] is not None + assert s["current_file"] is None + assert s["phase"] is None + assert s["current_summary"] is None + assert s["files_done"] == 1 and s["files_total"] == 3 # final counts kept + assert s["summaries_done"] == 0 and s["summaries_total"] == 0 + assert s["current_chunk"] == 0 and s["chunks_total"] == 0 + # PLAN §9: the ample-logging line for the converted cancel. + assert any( + r.getMessage() + == "sync: cancelled by user state=cancelled files_done=1 files_total=3" + for r in caplog.records + ) + + +def test_unrequested_cancel_is_never_swallowed_and_never_converts( + fresh_sync_status: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The shutdown contract pin (the completion criterion): a bare + ``task.cancel()`` WITHOUT the event set (the app-shutdown path — + no one pressed Cancel) re-raises the ``CancelledError`` with the + state UNTOUCHED — the task dies mid-run in ``running`` (no + terminal state, exactly as today's shutdown path does), the event + stays unset, and nothing is swallowed.""" + fake_import = _GatedImport( + ImportSummary(files=3, added=1, updated=1, unchanged=1), + threading.Event(), + threading.Event(), + ) + _patch_sync_seams(monkeypatch, tmp_path, fake_import, _fast_clone) + + def act(loop: asyncio.AbstractEventLoop) -> None: + assert fake_import.started.wait(5.0), "the run never reached the import" + assert sync_api._cancel_event.is_set() is False + task = sync_api._task + assert task is not None + task.cancel() # NO endpoint, NO event — the shutdown path + loop.run_until_complete(task) # raises the CancelledError + + try: + thread, errors, out = _run_via_start_sync(act) + thread.join(10.0) + finally: + fake_import.release.set() # free the to_thread park thread (non-daemon) + assert not thread.is_alive() + assert "result" not in out # the action raised + assert len(errors) == 1 + assert isinstance(errors[0], asyncio.CancelledError) + s = sync_api.sync_status() + assert s["state"] == "running" # the task died mid-run — no terminal + assert s["finished_at"] is None + assert s["error"] is None + assert s["detail"] == {} + assert sync_api._cancel_event.is_set() is False + + +def test_check_cancelled_raises_only_when_the_event_is_set( + fresh_sync_status: None, +) -> None: + """The boundary helper is pure polling: a no-op while the event is + unset, ``asyncio.CancelledError`` when it is set (the run's + ``except CancelledError`` clause converts THAT into the + ``cancelled`` terminal).""" + assert sync_api._cancel_event.is_set() is False + sync_api._check_cancelled() # unset: no raise + sync_api._cancel_event.set() + with pytest.raises(asyncio.CancelledError): + sync_api._check_cancelled() + + +def test_fresh_run_clears_a_stale_cancel_event( + fresh_sync_status: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The stale-event pin: run ONE is cancelled (the terminal keeps + the event SET — only start_sync clears it), then a FRESH run + starts → ``start_sync`` cleared the event before the task was + created → the new run completes to ``success`` with no cancel + request (a stale event cannot bleed into the next run).""" + first_import = _GatedImport( + ImportSummary(files=3, added=1, updated=1, unchanged=1), + threading.Event(), + threading.Event(), + ) + _patch_sync_seams(monkeypatch, tmp_path, first_import, _fast_clone) + + def act1(loop: asyncio.AbstractEventLoop) -> dict[str, str]: + assert first_import.started.wait(5.0), "run 1 never reached the import" + return loop.run_until_complete(sync_api.cancel_sync()) + + try: + thread, errors, out = _run_via_start_sync(act1) + thread.join(10.0) + finally: + first_import.release.set() # free the to_thread park thread + assert not thread.is_alive() + assert errors == [], f"run 1 raised: {errors!r}" + task1 = sync_api._task + assert task1 is not None + assert task1.cancelled() # the re-raise ended it cancelled + assert out.get("result") == {"detail": "cancel requested"} + assert sync_api.sync_status()["state"] == "cancelled" + assert sync_api._cancel_event.is_set() # the terminal keeps the event set + + # The FRESH run — a new gated import (the first one's gates are + # spent), the same fast clone. + second_import = _GatedImport( + ImportSummary(files=1, added=1), threading.Event(), threading.Event() + ) + monkeypatch.setattr(sync_api, "import_sources", second_import) + + def act2(loop: asyncio.AbstractEventLoop) -> None: + assert second_import.started.wait(5.0), "run 2 never reached the import" + # start_sync cleared the stale event before the task was + # created — by the import park it is definitely clear. + assert sync_api._cancel_event.is_set() is False + second_import.release.set() + task = sync_api._task + assert task is not None + loop.run_until_complete(task) + + try: + thread2, errors2, out2 = _run_via_start_sync(act2) + thread2.join(10.0) + finally: + second_import.release.set() # free the to_thread park thread + assert not thread2.is_alive() + assert errors2 == [], f"the fresh run raised: {errors2!r}" + s = sync_api.sync_status() + assert s["state"] == "success" # no cancel request — clean completion + + +def test_event_alone_cancels_at_the_import_boundary( + fresh_sync_status: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The cooperative path WITHOUT ``task.cancel()``: the event is set + while the run is parked in the clone prelude (simulating a + registered cancel whose ``_task.cancel()`` hit a blocking + subprocess and cannot interrupt it) — the span runs to completion + and the NEXT boundary (immediately before ``import_sources``) + converts it: the import never runs (the hook never fired), the + state is ``cancelled``, the counts stay 0/0.""" + clone_parked = threading.Event() + clone_release = threading.Event() + clone_calls: list[tuple[str, Path]] = [] + + def parking_clone(url: str, dest: Path | str) -> Path: + dest = Path(dest) + dest.mkdir(parents=True, exist_ok=True) + (dest / "notes.md").write_text("# repo\ncontent for the KB\n", encoding="utf-8") + clone_calls.append((url, dest)) + clone_parked.set() + clone_release.wait() # blocking is fine: the clone is a sync call + return dest + + fake_import = _GatedImport( + ImportSummary(files=3, added=1, updated=1, unchanged=1), + threading.Event(), + threading.Event(), + ) + _patch_sync_seams(monkeypatch, tmp_path, fake_import, parking_clone) + + thread, errors = _start_run_via_api() + try: + assert clone_parked.wait(5.0), "the run never reached the clone" + # The event ALONE (no task.cancel — the subprocess cannot be + # interrupted): the cooperative boundary must catch it. + sync_api._cancel_event.set() + finally: + clone_release.set() + thread.join(10.0) + assert not thread.is_alive() + assert clone_calls == [("https://git.example.com/repo.git", tmp_path / "bor" / "repo")] + assert len(errors) == 1 + assert isinstance(errors[0], asyncio.CancelledError) + assert fake_import.hook_calls == [] # the import never ran + s = sync_api.sync_status() + assert s["state"] == "cancelled" + assert s["error"] is None + assert s["detail"] == {} + assert s["files_done"] == 0 and s["files_total"] == 0 + assert s["current_file"] is None + assert s["phase"] is None + + +def test_event_set_during_import_cancels_before_the_lite_spans( + fresh_sync_status: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The overview + summaries boundary: the cancel's event lands + WHILE the import span runs (the fake import sets it mid-call — + the endpoint would have set it too) — the run converts at the + boundary immediately before ``if summary.added + summary.updated + > 0:``: the overview AND the folder summaries never start, the + state is ``cancelled``, and the import's final file counts are + kept.""" + overview_calls: list[Any] = [] + folder_calls: list[Any] = [] + + class _CancelDuringImport: + def __init__(self, summary: ImportSummary) -> None: + self.summary = summary + self.calls = 0 + + async def __call__( + self, + sources: list[Path], + llm: object, + *, + prune: bool = False, + limit: int | None = None, + session: object = None, + progress: Callable[[str, str, int, int], None] | None = None, + on_chunk_progress: Callable[[int, int], None] | None = None, # phase 129 + ignore_by_root: dict[str, list[str]] | None = None, # phase 89 + include_hidden_by_root: dict[str, bool] | None = None, # phase 105 + doc_dates_by_root: dict[str, dict[str, datetime]] | None = None, # phase 106 + ) -> ImportSummary: + self.calls += 1 + if progress is not None: + progress("repo", "notes/deep.md", 1, 2) + # The user's cancel lands mid-import (the endpoint's + # _task.cancel() would interrupt here at the next per-file + # await; the event ALONE isolates the boundary check — the + # next boundary is the overview gate). + sync_api._cancel_event.set() + return self.summary + + fake_import = _CancelDuringImport(ImportSummary(files=2, added=1, updated=0)) + _patch_sync_seams(monkeypatch, tmp_path, fake_import, _fast_clone) + + async def spy_overview(llm: object, session: object = None) -> bool: + overview_calls.append(llm) + return False + + async def spy_folders( + db: object, + llm: object, + *, + skip: bool = False, + only_missing: bool = False, + changed_docs: Sequence[tuple[str, str]] | None = None, # phase 127 + on_progress: Callable[[int, int, str, str], None] | None = None, + ) -> dict[str, int]: + folder_calls.append(llm) + return {"generated": 0, "failed": 0, "pruned": 0} + + monkeypatch.setattr(sync_api, "regenerate_overview", spy_overview) + monkeypatch.setattr(sync_api, "generate_folder_summaries", spy_folders) + + thread, errors = _start_run_via_api() + thread.join(10.0) + assert not thread.is_alive() + assert fake_import.calls == 1 # the import itself ran ... + assert len(errors) == 1 + assert isinstance(errors[0], asyncio.CancelledError) + assert overview_calls == [] # ... but the overview never started + assert folder_calls == [] # ... and neither did the folder summaries + s = sync_api.sync_status() + assert s["state"] == "cancelled" + assert s["error"] is None + assert s["detail"] == {} + assert s["files_done"] == 1 and s["files_total"] == 2 # final counts kept + assert s["current_file"] is None + assert s["phase"] is None + assert s["current_summary"] is None + + +def test_event_cancels_at_the_top_of_the_source_loop( + fresh_sync_status: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The per-source boundary (two git rows): the event is set while + the FIRST source's post-clone date walk runs (a blocking span — + the cooperative consequence: it runs to completion) — the cancel + converts at the top of iteration TWO, before the second source's + clone/pull: exactly one clone ran, the import never started, the + state is ``cancelled``.""" + parked = threading.Event() + release = threading.Event() + clone_calls: list[tuple[str, Path]] = [] + + def parking_dates(root: Path) -> dict[str, datetime]: + parked.set() + release.wait() # blocking is fine: the date walk is a sync call + return {} + + def fast_clone(url: str, dest: Path | str) -> Path: + dest = Path(dest) + dest.mkdir(parents=True, exist_ok=True) + (dest / "notes.md").write_text("# repo\ncontent for the KB\n", encoding="utf-8") + clone_calls.append((url, dest)) + return dest + + class _NeverReachedImport: + def __init__(self) -> None: + self.calls = 0 + + async def __call__(self, *args: Any, **kwargs: Any) -> ImportSummary: + self.calls += 1 + return ImportSummary() + + fake_import = _NeverReachedImport() + _patch_sync_seams(monkeypatch, tmp_path, fake_import, fast_clone) + monkeypatch.setattr(sync_api, "file_commit_dates", parking_dates) + # Two git rows: the cancel must fire at the top of iteration TWO + # (after the first source's git ops ran to completion). + monkeypatch.setattr( + sync_api, + "effective_sources", + lambda session: ( + [ + GitSource(url="https://git.example.com/one.git", kind="git"), + GitSource(url="https://git.example.com/two.git", kind="git"), + ], + "db", + ), + ) + + thread, errors = _start_run_via_api() + try: + assert parked.wait(5.0), "the run never reached the first date walk" + sync_api._cancel_event.set() + finally: + release.set() + thread.join(10.0) + assert not thread.is_alive() + assert len(errors) == 1 + assert isinstance(errors[0], asyncio.CancelledError) + # Exactly the first source's git ops ran — the second clone never + # started (the boundary fired before it). + assert clone_calls == [("https://git.example.com/one.git", tmp_path / "bor" / "one")] + assert fake_import.calls == 0 + s = sync_api.sync_status() + assert s["state"] == "cancelled" + assert s["error"] is None + assert s["detail"] == {} + assert s["files_done"] == 0 and s["files_total"] == 0 + assert s["current_file"] is None + + +# ---------- phase 134 (task 02): the frontend cancel contract ---------- +# +# The browser behavior is E2E-covered (tests/e2e/test_sync_cancel.py, +# task 03); here the source-level contract is pinned the same way as +# the main button above — so a silent regression (a hidden leak, a +# branch-order swap, a stale re-enable) is caught without a browser. + + +def test_sync_cancel_button_ships_hidden_and_labeled() -> None: + """#sync-cancel-btn SHIPS hidden (anonymous-safe + the idle state + — sources.js reveals it the same way it reveals #sync-btn) in the + sync-control row right after the main button, with the visible + "Cancel" label (the accessible name — WCAG 2.4, never + icon-only).""" + text = _text(SHELL_HTML) + m = re.search(r"]*)id=\"sync-cancel-btn\"([^>]*)>([^<]*)", text) + assert m, "the shell must carry the #sync-cancel-btn (RAG view)" + attrs = m.group(1) + m.group(2) + assert 'class="sync-cancel-btn"' in attrs + assert 'type="button"' in attrs + assert re.search(r"\bhidden\b", attrs), "#sync-cancel-btn must ship hidden" + assert m.group(3).strip() == "Cancel", "the visible label is the accessible name" + # Same row: right after the main button closes, before the announcer. + main_end = text.find("", text.find('id="sync-btn"')) + cancel_start = text.find('id="sync-cancel-btn"') + assert -1 < main_end < cancel_start < text.find('id="sync-result"') + + +def test_sync_cancel_btn_css_secondary_pill_and_states() -> None: + """.sync-cancel-btn is the house SECONDARY pill (the + .kb-summary-cancel language): >=44px touch floor, --line border, + the AA ink-soft text pair on the surface, the global 3px + :focus-visible ring (no per-button override), the :disabled look + (the request in flight — one request at a time), and the [hidden] + rule that keeps the attribute authoritative over the display. The + hover is guarded on :not(:disabled) — a disabled button under the + cursor never reads as actionable.""" + css = _text(STYLES_CSS) + block = re.search(r"\.sync-cancel-btn\s*\{([^}]*)\}", css) + assert block, "styles.css must define .sync-cancel-btn" + body = block.group(1) + for prop in ( + "display: inline-flex", + "min-height: 44px", + "border-radius: 999px", + "border: 1px solid var(--line)", + "color: var(--ink-soft)", + ): + assert prop in body, f".sync-cancel-btn must carry {prop!r}" + hover = re.search(r"\.sync-cancel-btn:hover:not\(:disabled\)\s*\{([^}]*)\}", css) + assert hover, "the hover must be guarded on :not(:disabled)" + disabled = re.search(r"\.sync-cancel-btn:disabled\s*\{([^}]*)\}", css) + assert disabled, ".sync-cancel-btn:disabled must be styled" + assert "opacity: 0.6" in disabled.group(1) + hidden_rule = re.search(r"\.sync-cancel-btn\[hidden\]\s*\{([^}]*)\}", css) + assert hidden_rule, "the [hidden] rule must beat the display" + assert "display: none" in hidden_rule.group(1) + + +def test_sync_cancel_btn_css_full_width_on_mobile() -> None: + """≤640px: the sync-control row already wraps (the 360px case) — + the Cancel pill stacks full-width under the full-width Sync pill + (the same mobile rule as the main button, next to it).""" + css = _text(STYLES_CSS) + main = css.find(".sync-btn { width: 100%; }") + assert main != -1, "the main button's mobile full-width rule" + near = css[main : main + 300] + assert ".sync-cancel-btn { width: 100%; }" in near, ( + "the Cancel pill must join the main button's mobile full-width rule" + ) + + +def test_sources_js_owns_the_cancel_button_and_helpers() -> None: + """The control ref is null-safe like the main button's, and the + show/hide helpers carry the exact contract: show = unhide + + enable + aria-label "Cancel sync" (the stable accessible name); + hide = hide + disable (a hidden control is never focusable or + activatable in any state — the never-stale guarantee).""" + js = _text(SOURCES_JS) + assert 'querySelector("#sync-cancel-btn")' in js + show = _body(js, "showSyncCancelBtn") + assert "syncCancelBtn.hidden = false" in show + assert "syncCancelBtn.disabled = false" in show + assert 'syncCancelBtn.setAttribute("aria-label", "Cancel sync")' in show + hide = _body(js, "hideSyncCancelBtn") + assert "syncCancelBtn.hidden = true" in hide + assert "syncCancelBtn.disabled = true" in hide + + +def test_sources_js_cancel_visible_only_while_a_sync_runs() -> None: + """The show/hide contract's two choke points: enterSyncRunningState + shows the control for kind "sync" (the live click, the re-attach, + every running tick) and hides it for kind "upload" (the upload job + is NOT cancellable in this phase — its run keeps the control + hidden); settleSyncButton hides it on EVERY settle (every + terminal + both idles pass through it).""" + js = _text(SOURCES_JS) + enter = _body(js, "enterSyncRunningState") + assert 'if (kind === "sync") showSyncCancelBtn()' in enter + assert "else hideSyncCancelBtn();" in enter + settle = _body(js, "settleSyncButton") + assert "hideSyncCancelBtn()" in settle, ( + "every settle (terminal + idle) must hide the Cancel control" + ) + + +def test_sources_js_cancel_request_is_never_stale() -> None: + """requestCancel: the control disables IMMEDIATELY (before the + fetch — one request at a time, no double fire) and #sync-result + clears with it; then the branch set — 403 the whoami backstop + (stop polling, hide the main button, settle idle — the same depth + as every other 403 in this module), 409 the silent no-op (the + poll settles), 202 (r.ok) nothing (the poll's cancelled branch + settles everything), any other non-2xx / network failure the + phase-55 neutral retry copy + re-enable (the run is still going — + the owner can retry).""" + js = _text(SOURCES_JS) + body = _body(js, "requestCancel") + disable = body.find("syncCancelBtn.disabled = true") + fetch = body.find('fetch("/api/sync/cancel", { method: "POST" })') + assert disable != -1 and fetch != -1 and disable < fetch, ( + "the disable must land BEFORE the fetch (never stale)" + ) + assert 'syncResult.textContent = ""' in body, "the result line clears with the click" + assert "r.status === 403" in body + assert "stopSyncPolling()" in body + assert "syncBtn.hidden = true" in body + assert "applySyncIdle()" in body + assert "r.status === 409" in body + assert "r.ok" in body + # The phase-55 neutral retry copy + the re-enable — in the + # network-failure catch AND the non-2xx fall-through. + assert body.count("Couldn't cancel the sync — try again.") == 2 + assert body.count("syncCancelBtn.disabled = false") == 2 + + +def test_sources_js_wires_the_cancel_click_next_to_the_main() -> None: + """The click listener is armed right after the main button's + (guarded on the control's presence — a page without the markup is + a complete no-op, the main button's convention).""" + js = _text(SOURCES_JS) + main = js.find('syncBtn.addEventListener("click", startSync)') + cancel = js.find('syncCancelBtn.addEventListener("click", requestCancel)') + assert main != -1 and cancel != -1 + assert cancel > main, "the cancel wiring sits next to the main button's" + + +def test_sources_js_poll_tree_cancelled_branch_in_order() -> None: + """The poll tick's decision tree (now 8 branches) gains the + cancelled branch AFTER the failed branch and BEFORE the + upload-success branch: stop polling + applySyncCancelled — the + neutral terminal, not the failure path.""" + js = _text(SOURCES_JS) + start = js.find("function startSyncPolling") + end = js.find("async function startSync", start) + tick = js[start:end] + failed = tick.find('syncStatus.state === "failed"') + cancelled = tick.find('syncStatus.state === "cancelled"') + upload_success = tick.find('uploadStatus.state === "success"') + assert -1 < failed < cancelled < upload_success, ( + "the cancelled branch must sit between failed and upload success" + ) + branch = tick[cancelled : cancelled + 300] + assert "stopSyncPolling()" in branch + assert "applySyncCancelled(syncStatus)" in branch + + +def test_sources_js_apply_sync_cancelled_is_neutral() -> None: + """applySyncCancelled renders the NEUTRAL result (A1 — cancel is + intent, not failure): settle the main button retry-ready (which + also hides the Cancel control), the "Sync cancelled" line on the + aria-live result (superseding the settle's label write), the + error banner hidden (a failed run's banner must not survive), the + status frame emitted — NO modal, NO banner, NO error styling.""" + js = _text(SOURCES_JS) + body = _body(js, "applySyncCancelled") + assert 'settleSyncButton("Sync sources")' in body + assert 'syncResult.textContent = "Sync cancelled"' in body + assert "hideSyncError()" in body + assert "emitSyncStatus(status)" in body + assert "showSyncModal" not in body, "cancel is intent — no modal" + assert "showSyncError" not in body, "cancel is intent — no banner" + assert "is-error" not in body, "cancel is intent — no error styling" + + +def test_sources_js_reattaches_a_cancelled_run() -> None: + """initSyncButton: a reload during/right after a cancel renders + the neutral cancelled result (the branch sits after the failed + branch, before the upload check) — never a stuck running state.""" + js = _text(SOURCES_JS) + body = _body(js, "initSyncButton") + assert 'status.state === "cancelled"' in body + assert "applySyncCancelled(status)" in body + failed = body.find('status.state === "failed"') + cancelled = body.find('status.state === "cancelled"') + assert -1 < failed < cancelled + + +def test_sources_js_section_header_documents_the_cancel_contract() -> None: + """The house pattern: the sync section's header docstring carries + the decision tree (now 8 branches — the cancelled one is the + 5th) and the Cancel control's show/hide contract.""" + js = _text(SOURCES_JS) + start = js.find("Sync sources button (Sources page only)") + assert start != -1, "the sync section marker comment" + header = js[start : js.find("const syncBtn = root.querySelector", start)] + assert "5. sync cancelled" in header, "the tree's cancelled branch (the 5th)" + assert "8. both idle" in header, "the tree is now 8 branches" + assert "#sync-cancel-btn" in header, "the control's contract is documented"