All verification is complete and green. Final report:
**Phase 90 (upload_no_scan) — final verification pass: all criteria verified, no fixes needed**
- Verified (not re-implemented): all 3 tasks already complete; working tree carries the phase's uncommitted changes
- Upload pipeline: `_run_upload` stops after row upsert; success = `{"message": "uploaded"}`, null/0/0 progress; `UploadOut` removed from `app/schemas.py`; gates/unpack/swap/failed states intact
- Copy: button "Upload", bare "Processing…", result line "Uploaded \<name\> — press Sync sources to import it."; hint + caption re-pointed at Sync; no "Upload &" remnants in `frontend/`
- Tests: `pytest tests/unit/test_git_sources.py tests/integration/test_git_sources_upload.py tests/unit/test_frontend_sync_upload.py` → 56 passed; E2E phase suite `tests/e2e/test_upload_no_scan.py` → 3 passed (zero docs after upload; ignore edit honored by Sync — 2 added, `notes/skipme.md` excluded; re-upload in-place, still zero docs); affected suites `test_archive_upload_sources.py` + `test_sync_upload_progress.py` + `test_source_removal_cleanup.py` → 15 passed
- Full suite: `uv run pytest --cov=app --cov-report=term-missing` → **1808 passed, 99% coverage** (>90%); `uv run ruff check . && uv run pyright` → clean, 0 errors
**Completion criteria:**
1. Upload settles "ready for sync", zero docs, row + ignore editor — ✅ (E2E test 1)
2. Button "Upload", no "Upload &" copy — ✅ (`rg` empty in frontend/, app/, tests/)
3. RAG-page "Sync sources" imports upload honoring edited ignores — ✅ (isolated E2E test 2)
4. Suite green, >90% coverage, lint clean — ✅ (99%, ruff+pyright clean)
5. Atomic commit — ✅ left to harness per executor rules (no git add/commit run; tree staged-ready)
**Deviations:** `regenerate_overview` still appears once in `git_sources.py` — inside `delete_git_source` (phase-69 locked total-removal behavior, not the upload leg; upload leg is scan-free). `.agents/PLAN.md` does not exist in this repo; locked decisions A1–A4 were enforced from the phase files.
**Next pending phase:** `91_admin_theme_tab` (in `todo/`).
777 lines
32 KiB
Python
777 lines
32 KiB
Python
"""Phase 69 story E2E (Playwright): total source removal — the
|
|
confirmation modal, the row + index prune, and the app-managed disk
|
|
cleanup (``/git-sources.html``).
|
|
|
|
Story: n/a (owner request from chat, 2026-09-02)
|
|
Run in isolation (DB must be up: ``podman compose up -d db``):
|
|
|
|
uv run pytest tests/e2e/test_source_removal_cleanup.py -v --no-cov
|
|
|
|
The story gate for TOTAL source removal (phase 69): removing a source
|
|
removes, in one action, the stored row, every indexed document of that
|
|
source (chunks + embeddings), and — for app-managed sources — the files
|
|
on disk (the git checkout under ``BOR_SOURCES_DIR`` or the unpacked
|
|
upload folder under ``BOR_UPLOAD_DIR``), immediately (not deferred to
|
|
the next sync). The page confirms first through a page-local
|
|
``role="alertdialog"`` modal (the native ``confirm()`` is retired) that
|
|
names the source and states the removal policy; Cancel/Escape close
|
|
without a request, and only "Remove source" sends the DELETE.
|
|
|
|
**Disk paths are resolved exactly like the app**:
|
|
``Path(get_settings().sources_dir).expanduser()`` /
|
|
``Path(get_settings().upload_dir).expanduser()`` — the E2E conftest
|
|
does NOT override those two vars and pytest runs with ``cwd=REPO``, so
|
|
the test process and the app subprocess resolve the same ``.env``
|
|
(``BOR_SOURCES_DIR`` / ``BOR_UPLOAD_DIR``) and the same-host ``pathlib``
|
|
assertions hit the very directories the DELETE handler cleans.
|
|
|
|
The suite triggers **one sync** (test 1 only — phase 90: the upload's
|
|
background run unpacks + registers only, so the uploaded row is
|
|
imported via ``POST /api/sync`` on the mock-LLM pipeline, no network
|
|
beyond the app itself); the git rows are ``example.com`` URLs that
|
|
are never cloned, and every other test stays sync-free. The only real
|
|
on-disk artifact is the API-driven upload of one small archive with a
|
|
unique name (``phase69-<8-hex>.tar.gz``, one ``.md`` file). Seeded
|
|
``Document`` rows (``SessionLocal``, the
|
|
``test_git_sources_admin.py`` pattern) give the prune assertions a
|
|
deterministic KB.
|
|
|
|
Per-module app env (the conftest pattern, module-scoped): the same env
|
|
shape as ``test_git_sources_admin.py`` with ``BOR_GIT_SOURCES`` forced
|
|
empty (a dev ``.env`` fallback URL must never render as an env row on
|
|
the table); **no** ``BOR_SOURCES_DIR`` / ``BOR_UPLOAD_DIR`` override
|
|
(deliberate — see the disk-path note above).
|
|
|
|
Contract under test:
|
|
|
|
* **upload → sync → modal → total removal** (phase 90: the upload
|
|
unpacks + registers only — the sync performs the scan): the
|
|
uploaded folder exists on disk and, after the sync, its document is
|
|
in ``GET /api/docs``; the row's Remove → the alertdialog opens
|
|
(``#remove-confirm-source`` = the upload path, focus on
|
|
``#remove-confirm-cancel``) → "Remove source" → the "Removing…"
|
|
in-flight state (both buttons disabled) → settled: the row is gone,
|
|
the document is pruned, **the folder is gone from disk**, exactly
|
|
one DELETE went out, and the announcer carries the success line;
|
|
* **git checkout removal**: a seeded git row + a hand-made checkout
|
|
dir (marker file) + a seeded document → modal removal → the row is
|
|
gone, **the checkout dir is gone from disk** (marker included) and
|
|
the document is pruned;
|
|
* **no-checkout no-op**: a git row with NO checkout dir (never synced)
|
|
removes cleanly — the absent-dir path, no error state anywhere on
|
|
the page;
|
|
* **foreign local directories are never touched**: a seeded
|
|
``kind='local'`` row pointing at a test-owned dir (marker file) + a
|
|
seeded document → modal removal → the row is gone and the document
|
|
is pruned, **but the dir + marker file are still present**;
|
|
* **cancel + Escape keep everything**: Cancel click → dialog hidden,
|
|
focus back on the trigger button, **zero DELETE requests**, row +
|
|
document remain; re-open → Escape → the same;
|
|
* **modal a11y + no CDN** (UI Structure Check, AGENTS.md rule 5 +
|
|
rule 6): the dialog's aria attributes, the 3px ``:focus-visible``
|
|
outline computable on BOTH buttons (keyboard focus), both buttons
|
|
≥44px tall, the success line on ``#git-sources-announcer``
|
|
(role=status) after a real removal, and the page loads only
|
|
same-origin resources.
|
|
|
|
Test → story mapping (Playwright Mapping Rule):
|
|
1. ``test_uploaded_source_removal_cleans_index_and_disk``
|
|
2. ``test_git_source_removal_removes_checkout_and_index``
|
|
3. ``test_git_source_removal_without_checkout_succeeds``
|
|
4. ``test_local_directory_source_files_never_deleted``
|
|
5. ``test_remove_modal_cancel_and_esc_keep_everything``
|
|
6. ``test_remove_modal_a11y_and_no_cdn``
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import hashlib
|
|
import io
|
|
import os
|
|
import shutil
|
|
import subprocess
|
|
import sys
|
|
import tarfile
|
|
import time
|
|
import uuid
|
|
from collections.abc import Iterator
|
|
from pathlib import Path
|
|
from typing import Any
|
|
|
|
import pytest
|
|
from playwright.sync_api import Page, Route, expect
|
|
from sqlalchemy import text
|
|
|
|
from app.config import get_settings
|
|
from app.db import SessionLocal
|
|
from app.models import Document, GitSource
|
|
from e2e.auth_helpers import login
|
|
from e2e.conftest import (
|
|
ADMIN_PASSWORD,
|
|
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_REMOVAL", "8131"))
|
|
APP_URL = f"http://127.0.0.1:{APP_PORT}"
|
|
|
|
GIT_SOURCES_URL = "/git-sources.html"
|
|
|
|
#: The success line the page announces on a total removal (the
|
|
#: git-sources.js announce() string).
|
|
REMOVAL_ANNOUNCE = "Source removed — its files and index entries were cleaned up."
|
|
|
|
#: Deterministic, never-cloned example.com URLs — the suite never
|
|
#: triggers a sync, so the only on-disk artifact a git row can have is
|
|
#: the checkout dir this suite itself creates (test 2).
|
|
GONE_URL = "https://example.com/reese/phase69-gone.git"
|
|
GONE_REPO = "phase69-gone" # repo_name(GONE_URL)
|
|
NO_CHECKOUT_URL = "https://example.com/reese/phase69-nodir.git"
|
|
KEPT_URL = "https://example.com/reese/phase69-kept.git"
|
|
KEPT_REPO = "phase69-kept"
|
|
A11Y_URL = "https://example.com/reese/phase69-a11y.git"
|
|
A11Y_REPO = "phase69-a11y"
|
|
|
|
#: The upload sentinel — one markdown file in the unique archive.
|
|
UPLOAD_SENTINEL = "PHASE69-UPLOAD-9c2d"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Fixtures
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.fixture(scope="module")
|
|
def app_server(mock_llm: int) -> Iterator[str]:
|
|
"""The real app under test — per-module env: ``BOR_GIT_SOURCES``
|
|
forced empty (no env fallback rows on the table). Deliberately NO
|
|
``BOR_SOURCES_DIR`` / ``BOR_UPLOAD_DIR`` override: the disk
|
|
assertions must resolve the dirs EXACTLY like the app (the module
|
|
docstring explains why the same ``.env`` resolves on both sides)."""
|
|
env = dict(os.environ)
|
|
env.pop("DEBUGPY", None)
|
|
env["BOR_ENVIRONMENT"] = "e2e"
|
|
env["BOR_STATIC_DIR"] = str(REPO / "frontend")
|
|
env["BOR_LLM_BASE_URL"] = (
|
|
"https://aipi.reeseapps.com/v1"
|
|
if USE_REAL_LLM
|
|
else f"http://127.0.0.1:{mock_llm}/v1"
|
|
)
|
|
# Mock-calibrated threshold (conftest pattern) — no chat turn is
|
|
# ever sent in this suite, but the app boots with the same env shape.
|
|
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
|
|
# No env fallback rows: the table state (seeded per test) is the
|
|
# only row source, so row-count assertions are deterministic.
|
|
env["BOR_GIT_SOURCES"] = ""
|
|
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
|
|
|
|
|
|
@pytest.fixture(scope="module")
|
|
def sources_dir() -> Path:
|
|
"""The app's git-checkout root — resolved EXACTLY like the app
|
|
(same ``.env``, same ``cwd=REPO``; the conftest does not override
|
|
``BOR_SOURCES_DIR``)."""
|
|
return Path(get_settings().sources_dir).expanduser()
|
|
|
|
|
|
@pytest.fixture(scope="module")
|
|
def upload_dir() -> Path:
|
|
"""The app's upload root — resolved EXACTLY like the app (see
|
|
``sources_dir``)."""
|
|
return Path(get_settings().upload_dir).expanduser()
|
|
|
|
|
|
def _truncate_all() -> None:
|
|
"""Fresh registry + KB per test (the E2E isolation pattern, the
|
|
``test_archive_upload_sources.py`` set): the row/doc assertions
|
|
must be this test's own doing — a leftover document under one of
|
|
the deterministic source names would survive the prune and flip an
|
|
assertion."""
|
|
with SessionLocal() as db:
|
|
db.execute(text("TRUNCATE chunks, documents, query_log, kb_overview, git_sources"))
|
|
db.commit()
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _clean(db_ready: None) -> Iterator[None]:
|
|
_truncate_all()
|
|
yield
|
|
_truncate_all()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Helpers
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def _seed_git_row(url: str) -> None:
|
|
"""Store a git row directly (deterministic — the
|
|
``test_git_sources_admin.py`` SessionLocal seeding pattern)."""
|
|
with SessionLocal() as db:
|
|
db.add(GitSource(url=url, kind="git"))
|
|
db.commit()
|
|
|
|
|
|
def _seed_local_row(path: str) -> None:
|
|
"""Store a kind=local row (the phase-38 shape: the expanded
|
|
absolute path in BOTH ``path`` and the NOT-NULL ``url``)."""
|
|
with SessionLocal() as db:
|
|
db.add(GitSource(url=path, kind="local", path=path))
|
|
db.commit()
|
|
|
|
|
|
def _seed_doc(source: str, path: str, full_path: str, title: str, content: str) -> None:
|
|
"""One indexed document (the prune subject) — a full row so the
|
|
cascade + the /api/docs listing behave exactly like a scanned doc."""
|
|
with SessionLocal() as db:
|
|
db.add(
|
|
Document(
|
|
source=source,
|
|
path=path,
|
|
full_path=full_path,
|
|
title=title,
|
|
content=content,
|
|
content_hash=hashlib.sha256(content.encode("utf-8")).hexdigest(),
|
|
)
|
|
)
|
|
db.commit()
|
|
|
|
|
|
def _admin_git_sources_page(page: Page, app_url: str) -> None:
|
|
"""Real form login landing on the git sources page (admin settled:
|
|
Sign out visible, the manager revealed by the page module)."""
|
|
login(page, app_url, next=GIT_SOURCES_URL)
|
|
expect(page).to_have_url(app_url + GIT_SOURCES_URL, timeout=30_000)
|
|
expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000)
|
|
expect(page.locator("#git-sources-gate")).to_be_hidden()
|
|
expect(page.locator("#git-sources-content")).to_be_visible()
|
|
|
|
|
|
def _row(page: Page, value: str) -> Any:
|
|
"""The table row whose mono cell shows ``value`` (git URL or
|
|
local path — the row's rendered value)."""
|
|
return page.locator("#git-sources-tbody tr", has_text=value)
|
|
|
|
|
|
def _docs(page: Page, app_url: str) -> list[tuple[str, str]]:
|
|
"""``GET /api/docs`` as the signed-in page → sorted (source, path)
|
|
pairs (the admin cookie rides the browser context)."""
|
|
r = page.request.get(f"{app_url}/api/docs")
|
|
assert r.status == 200, r.text
|
|
return sorted((d["source"], d["path"]) for d in r.json()["documents"])
|
|
|
|
|
|
def _stored_sources(page: Page, app_url: str) -> list[dict[str, Any]]:
|
|
"""``GET /api/git-sources`` as the signed-in page → the row list."""
|
|
r = page.request.get(f"{app_url}/api/git-sources")
|
|
assert r.status == 200, r.text
|
|
return r.json()["sources"]
|
|
|
|
|
|
def _build_targz(path: Path, files: dict[str, str]) -> Path:
|
|
"""A deterministic ``.tar.gz`` (mtime 0) over the given files (the
|
|
``test_archive_upload_sources.py`` pattern)."""
|
|
with tarfile.open(path, "w:gz") as tf:
|
|
for rel, content in files.items():
|
|
data = content.encode("utf-8")
|
|
info = tarfile.TarInfo(rel)
|
|
info.size = len(data)
|
|
info.mtime = 0
|
|
tf.addfile(info, io.BytesIO(data))
|
|
return path
|
|
|
|
|
|
def _upload_and_wait_success(page: Page, app_url: str, archive: Path) -> dict[str, Any]:
|
|
"""POST the archive through the logged-in page's request context
|
|
(the admin cookie rides along) and poll the phase-64 status
|
|
endpoint to ``success`` — returns the terminal status body.
|
|
Phase 90: the upload run is unpack + register only — no scan — so
|
|
the caller follows with :func:`_run_sync` (the new owner flow) to
|
|
import the registered row."""
|
|
r = page.request.post(
|
|
f"{app_url}/api/git-sources/upload",
|
|
multipart={
|
|
"file": {
|
|
"name": archive.name,
|
|
"mimeType": "application/gzip",
|
|
"buffer": archive.read_bytes(),
|
|
}
|
|
},
|
|
)
|
|
assert r.status == 202, f"upload POST failed: {r.status} {r.text}"
|
|
deadline = time.monotonic() + 60.0
|
|
body: dict[str, Any] = {}
|
|
while time.monotonic() < deadline:
|
|
r = page.request.get(f"{app_url}/api/git-sources/upload/status")
|
|
assert r.status == 200, r.text
|
|
body = r.json()
|
|
if body["state"] == "success":
|
|
return body
|
|
if body["state"] == "failed":
|
|
raise AssertionError(f"the upload run failed: {body}")
|
|
time.sleep(0.2)
|
|
raise AssertionError(f"the upload run never settled: {body}")
|
|
|
|
|
|
def _run_sync(page: Page, app_url: str) -> dict[str, Any]:
|
|
"""``POST /api/sync`` → poll ``GET /api/sync/status`` to a terminal
|
|
state (the ``test_sync_button.py`` idiom) — phase 90: the scan the
|
|
upload deferred lands here (the sync imports the uploaded
|
|
``kind='local'`` row with prune). Returns the terminal body."""
|
|
r = page.request.post(f"{app_url}/api/sync")
|
|
assert r.status == 202, f"sync POST failed: {r.status} {r.text}"
|
|
deadline = time.monotonic() + 60.0
|
|
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 ("success", "failed"):
|
|
assert body["state"] == "success", f"sync failed: {body}"
|
|
return body
|
|
time.sleep(0.2)
|
|
raise AssertionError(f"sync never settled: {body}")
|
|
|
|
|
|
def _open_remove_modal(page: Page, value: str) -> None:
|
|
"""Click the row's Remove for ``value`` and assert the opened
|
|
alertdialog: it names the source and focus lands on Cancel (the
|
|
safe default)."""
|
|
_row(page, value).locator(".git-source-remove").click()
|
|
dialog = page.locator("#remove-confirm-dialog")
|
|
expect(dialog).to_be_visible(timeout=15_000)
|
|
assert dialog.get_attribute("role") == "alertdialog"
|
|
expect(page.locator("#remove-confirm-source")).to_have_text(value)
|
|
assert page.evaluate("() => document.activeElement.id") == "remove-confirm-cancel"
|
|
|
|
|
|
def _confirm_removal(page: Page) -> None:
|
|
"""Click "Remove source" and wait for the settled removal: the
|
|
(single) row is gone and the dialog is closed."""
|
|
page.locator("#remove-confirm-remove").click()
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(0, timeout=30_000)
|
|
expect(page.locator("#remove-confirm-dialog")).to_be_hidden()
|
|
|
|
|
|
def _hold_delete_requests(page: Page, hold_s: float) -> None:
|
|
"""Intercept DELETEs to the git-sources API and hold the REQUEST in
|
|
the browser for ``hold_s`` seconds (the
|
|
``test_archive_upload_sources.py`` ``_hold_upload_request``
|
|
pattern) — the modal's "Removing…" in-flight state (§7.4) becomes
|
|
deterministic instead of racing the (fast) mock-LLM cleanup.
|
|
Every other request (the list GETs, the upload status poll) passes
|
|
through untouched."""
|
|
|
|
def handle(route: Route) -> None:
|
|
if route.request.method == "DELETE":
|
|
time.sleep(hold_s)
|
|
route.continue_()
|
|
|
|
page.route("**/api/git-sources/**", handle)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 1. Upload → modal → total removal: index pruned AND the upload
|
|
# folder is gone from disk
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_uploaded_source_removal_cleans_index_and_disk(
|
|
page: Page, app_url: str, db_ready: None, upload_dir: Path, tmp_path: Path
|
|
) -> None:
|
|
"""A uniquely named archive uploaded through the API (202 → status
|
|
success — phase 90: unpack + register only, no scan), then the
|
|
sync that performs the scan (the new owner flow): the folder
|
|
exists on disk and the document is in the KB; then the row's
|
|
Remove → the alertdialog (the upload path named, focus on Cancel)
|
|
→ "Remove source" → the "Removing…" in-flight state → settled:
|
|
row gone, document pruned, **the folder is gone from disk**, one
|
|
DELETE, the announcer's success line."""
|
|
page.set_default_timeout(30_000)
|
|
# Unique per run — never collides with a real (or a crashed-run's)
|
|
# upload, so the disk assertions are safe on the shared dir.
|
|
name = f"phase69-{uuid.uuid4().hex[:8]}"
|
|
archive = _build_targz(
|
|
tmp_path / f"{name}.tar.gz",
|
|
{
|
|
"note.md": (
|
|
"# Upload note\n"
|
|
"\n"
|
|
f"Phase 69 removal subject. Marker: {UPLOAD_SENTINEL}\n"
|
|
)
|
|
},
|
|
)
|
|
|
|
_admin_git_sources_page(page, app_url)
|
|
|
|
# The API-driven upload (202) + the background unpack + register
|
|
# (success — the no-count payload, phase 90). Phase 90: the upload
|
|
# does NOT scan — the scan is the sync's job (the new owner flow:
|
|
# upload → [edit ignore list] → sync), so the uploaded row is
|
|
# imported by a sync before the preconditions below.
|
|
status = _upload_and_wait_success(page, app_url, archive)
|
|
assert status["detail"] == {"message": "uploaded"}
|
|
_run_sync(page, app_url)
|
|
|
|
# Preconditions — the artifact is real: the folder on disk (the
|
|
# app's resolved upload dir — this process resolved the same one),
|
|
# the document in the KB (via the sync), the row in the registry.
|
|
folder = upload_dir / name
|
|
assert (folder / "note.md").is_file(), f"{folder}/note.md missing on disk"
|
|
assert _docs(page, app_url) == [(name, "note.md")]
|
|
r = page.request.get(f"{app_url}/api/git-sources")
|
|
assert r.status == 200, r.text
|
|
assert [(s["kind"], s["path"]) for s in r.json()["sources"]] == [
|
|
("local", str(folder))
|
|
]
|
|
|
|
# The page's table still shows the pre-upload (empty) state — the
|
|
# upload went through the API, not the form (whose 202-success
|
|
# path would have called loadSources): a reload runs the page
|
|
# module's loadSources() and the new row lands.
|
|
page.reload()
|
|
expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000)
|
|
expect(page.locator("#git-sources-content")).to_be_visible()
|
|
row = _row(page, str(folder))
|
|
expect(row).to_have_count(1, timeout=15_000)
|
|
expect(row.locator("span.git-source-kind")).to_have_text("Local")
|
|
|
|
# Track the DELETEs; hold the one the modal sends so the in-flight
|
|
# state is observable deterministically.
|
|
deletes: list[str] = []
|
|
page.on(
|
|
"request",
|
|
lambda r: deletes.append(r.url)
|
|
if r.method == "DELETE" and "/api/git-sources/" in r.url
|
|
else None,
|
|
)
|
|
_hold_delete_requests(page, hold_s=0.8)
|
|
|
|
# The modal: the upload path is named (textContent — the row's
|
|
# value), focus on Cancel.
|
|
_open_remove_modal(page, str(folder))
|
|
|
|
# "Remove source" → the §7.4 in-flight state (both buttons
|
|
# disabled, the confirm relabeled) while the held DELETE is out.
|
|
page.locator("#remove-confirm-remove").click()
|
|
expect(page.locator("#remove-confirm-remove")).to_have_text("Removing…")
|
|
expect(page.locator("#remove-confirm-remove")).to_be_disabled()
|
|
expect(page.locator("#remove-confirm-cancel")).to_be_disabled()
|
|
expect(page.locator("#remove-confirm-error")).to_be_hidden()
|
|
|
|
# Settled: the row is gone, the dialog closed and reset…
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(0, timeout=30_000)
|
|
expect(page.locator("#remove-confirm-dialog")).to_be_hidden()
|
|
expect(page.locator("#remove-confirm-remove")).to_have_text("Remove source")
|
|
expect(page.locator("#remove-confirm-remove")).to_be_enabled()
|
|
# …the announcer carries the success line…
|
|
expect(page.locator("#git-sources-announcer")).to_have_text(REMOVAL_ANNOUNCE)
|
|
# …and exactly one DELETE went out.
|
|
assert len(deletes) == 1, f"expected one DELETE, saw: {deletes}"
|
|
|
|
# The TOTAL removal, proven outside the UI: the registry is empty,
|
|
# the document is pruned from the KB, and the folder is GONE from
|
|
# the app's upload dir (same-host pathlib, the app's resolved dir).
|
|
assert _stored_sources(page, app_url) == []
|
|
assert _docs(page, app_url) == []
|
|
assert not folder.exists(), f"{folder} still on disk after removal"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 2. Git row → the checkout dir is removed from disk + the index pruned
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_git_source_removal_removes_checkout_and_index(
|
|
page: Page, app_url: str, db_ready: None, sources_dir: Path
|
|
) -> None:
|
|
"""A seeded git row + a hand-made checkout dir (marker file, the
|
|
sync's clone target layout) + a seeded document: the modal removal
|
|
leaves the row gone, **the checkout dir gone from disk** (marker
|
|
included), and the document pruned from the KB."""
|
|
page.set_default_timeout(30_000)
|
|
checkout = sources_dir / GONE_REPO
|
|
shutil.rmtree(checkout, ignore_errors=True) # defensive: no leftover
|
|
(checkout / "notes").mkdir(parents=True)
|
|
(checkout / "notes" / "readme.md").write_text(
|
|
f"# Readme\n\nCheckout marker for {GONE_REPO}.\n", encoding="utf-8"
|
|
)
|
|
_seed_git_row(GONE_URL)
|
|
_seed_doc(
|
|
GONE_REPO,
|
|
"notes/readme.md",
|
|
str(checkout / "notes" / "readme.md"),
|
|
"Readme",
|
|
"Readme content for the phase69 checkout-removal test.",
|
|
)
|
|
|
|
_admin_git_sources_page(page, app_url)
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(1)
|
|
assert _docs(page, app_url) == [(GONE_REPO, "notes/readme.md")]
|
|
|
|
_open_remove_modal(page, GONE_URL)
|
|
_confirm_removal(page)
|
|
|
|
# Row gone from the registry…
|
|
assert _stored_sources(page, app_url) == []
|
|
# …the checkout dir is GONE from disk (the marker with it)…
|
|
assert not checkout.exists(), f"{checkout} still on disk after removal"
|
|
# …and the document is pruned from the KB.
|
|
assert _docs(page, app_url) == []
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 3. Git row, no checkout dir (never synced) → clean no-op removal
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_git_source_removal_without_checkout_succeeds(
|
|
page: Page, app_url: str, db_ready: None, sources_dir: Path
|
|
) -> None:
|
|
"""The absent-dir path: a seeded git row whose checkout dir does
|
|
not exist (the repo was never cloned/synced) still removes
|
|
cleanly — row gone, and NO error state anywhere on the page."""
|
|
page.set_default_timeout(30_000)
|
|
shutil.rmtree(sources_dir / "phase69-nodir", ignore_errors=True) # ensure absent
|
|
_seed_git_row(NO_CHECKOUT_URL)
|
|
|
|
_admin_git_sources_page(page, app_url)
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(1)
|
|
|
|
_open_remove_modal(page, NO_CHECKOUT_URL)
|
|
_confirm_removal(page)
|
|
|
|
assert _stored_sources(page, app_url) == []
|
|
# No error state anywhere on the page (in-modal, load, or announcer
|
|
# — the announcer carries the success line, not an error).
|
|
expect(page.locator("#remove-confirm-error")).to_be_hidden()
|
|
expect(page.locator("#git-sources-load-error")).to_be_hidden()
|
|
expect(page.locator("#git-source-error")).to_be_hidden()
|
|
expect(page.locator("#git-sources-announcer")).to_have_text(REMOVAL_ANNOUNCE)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 4. Foreign local directory → row + index go, the files NEVER do
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_local_directory_source_files_never_deleted(
|
|
page: Page, app_url: str, db_ready: None, tmp_path: Path
|
|
) -> None:
|
|
"""A seeded kind=local row pointing at the owner's OWN directory
|
|
(outside the app-managed roots) + a seeded document: the modal
|
|
removal deletes the row and prunes the document, **but the dir and
|
|
its marker file are still present** (locked decision: foreign
|
|
local directories are never touched on disk)."""
|
|
page.set_default_timeout(30_000)
|
|
user_dir = tmp_path / "my-notes"
|
|
user_dir.mkdir()
|
|
marker = user_dir / "keep-me.md"
|
|
marker.write_text("The owner's own file — removal must not touch it.\n", encoding="utf-8")
|
|
_seed_local_row(str(user_dir))
|
|
_seed_doc(
|
|
user_dir.name,
|
|
"keep-me.md",
|
|
str(marker),
|
|
"Keep me",
|
|
"Document of the foreign local dir (the prune subject).",
|
|
)
|
|
|
|
_admin_git_sources_page(page, app_url)
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(1)
|
|
assert _docs(page, app_url) == [(user_dir.name, "keep-me.md")]
|
|
|
|
_open_remove_modal(page, str(user_dir))
|
|
_confirm_removal(page)
|
|
|
|
# Row gone, document pruned…
|
|
assert _stored_sources(page, app_url) == []
|
|
assert _docs(page, app_url) == []
|
|
# …but the foreign directory + its file are STILL on disk.
|
|
assert user_dir.is_dir(), f"{user_dir} was deleted — foreign dirs are never touched"
|
|
assert marker.is_file(), f"{marker} was deleted — foreign files are never touched"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 5. Cancel + Escape → zero DELETEs, everything stays
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_remove_modal_cancel_and_esc_keep_everything(
|
|
page: Page, app_url: str, db_ready: None
|
|
) -> None:
|
|
"""(a) Remove → modal → Cancel: the dialog closes, focus returns
|
|
to the row's Remove button, **zero DELETE requests**, row + doc
|
|
remain. (b) Re-open → Escape: the same (the keyboard cancel path —
|
|
the request tracker stays empty through both)."""
|
|
page.set_default_timeout(30_000)
|
|
_seed_git_row(KEPT_URL)
|
|
_seed_doc(
|
|
KEPT_REPO,
|
|
"a.md",
|
|
f"/nonexistent-but-fine/{KEPT_REPO}/a.md",
|
|
"A",
|
|
"Document that must survive both cancels.",
|
|
)
|
|
|
|
deletes: list[str] = []
|
|
page.on(
|
|
"request",
|
|
lambda r: deletes.append(r.url)
|
|
if r.method == "DELETE" and "/api/git-sources/" in r.url
|
|
else None,
|
|
)
|
|
|
|
_admin_git_sources_page(page, app_url)
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(1)
|
|
assert _docs(page, app_url) == [(KEPT_REPO, "a.md")]
|
|
|
|
# (a) Cancel click — no request, focus back on the trigger.
|
|
_open_remove_modal(page, KEPT_URL)
|
|
page.locator("#remove-confirm-cancel").click()
|
|
expect(page.locator("#remove-confirm-dialog")).to_be_hidden()
|
|
# Focus returned to the row's Remove button (the trigger)…
|
|
assert page.evaluate(
|
|
"() => document.activeElement.classList.contains('git-source-remove')"
|
|
), "focus did not return to the row's Remove button"
|
|
# Let the page settle (a cancel sends nothing) before the
|
|
# "zero DELETEs" claim.
|
|
page.wait_for_timeout(500)
|
|
assert deletes == [], f"cancel sent a request: {deletes}"
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(1)
|
|
assert _docs(page, app_url) == [(KEPT_REPO, "a.md")]
|
|
|
|
# (b) Escape — the keyboard cancel path, same guarantees.
|
|
_open_remove_modal(page, KEPT_URL)
|
|
page.keyboard.press("Escape")
|
|
expect(page.locator("#remove-confirm-dialog")).to_be_hidden()
|
|
assert page.evaluate(
|
|
"() => document.activeElement.classList.contains('git-source-remove')"
|
|
), "focus did not return to the row's Remove button after Escape"
|
|
page.wait_for_timeout(500)
|
|
assert deletes == [], f"Escape sent a request: {deletes}"
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(1)
|
|
expect(_row(page, KEPT_URL)).to_have_count(1)
|
|
assert _docs(page, app_url) == [(KEPT_REPO, "a.md")]
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 6. Modal a11y (AGENTS.md rule 5) + no CDN (rule 6) + the success
|
|
# announcer after a real removal
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_remove_modal_a11y_and_no_cdn(page: Page, app_url: str, db_ready: None) -> None:
|
|
"""The dialog's aria wiring (alertdialog + labelled + described),
|
|
the 3px ``:focus-visible`` outline computable on BOTH buttons
|
|
(keyboard focus), both buttons ≥44px tall, the success line on
|
|
``#git-sources-announcer`` (role=status) after a real removal, and
|
|
the page loading only same-origin resources (rule 6)."""
|
|
page.set_default_timeout(30_000)
|
|
_seed_git_row(A11Y_URL)
|
|
_seed_doc(
|
|
A11Y_REPO,
|
|
"a.md",
|
|
f"/nonexistent-but-fine/{A11Y_REPO}/a.md",
|
|
"A",
|
|
"Document removed by the a11y test's successful removal.",
|
|
)
|
|
|
|
_admin_git_sources_page(page, app_url)
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(1)
|
|
|
|
# The static dialog wiring (the markup ships with the page).
|
|
dialog = page.locator("#remove-confirm-dialog")
|
|
assert dialog.get_attribute("role") == "alertdialog"
|
|
assert dialog.get_attribute("aria-modal") == "true"
|
|
assert dialog.get_attribute("aria-labelledby") == "remove-confirm-title"
|
|
assert dialog.get_attribute("aria-describedby") == "remove-confirm-copy"
|
|
expect(page.locator("#remove-confirm-title")).to_have_text("Remove this source?")
|
|
assert page.locator("#remove-confirm-error").get_attribute("role") == "alert"
|
|
expect(page.locator("#remove-confirm-error")).to_be_hidden()
|
|
expect(page.locator("#remove-confirm-copy")).to_contain_text("permanently removes")
|
|
|
|
# Open: focus lands on Cancel (the safe default)…
|
|
_row(page, A11Y_URL).locator(".git-source-remove").click()
|
|
expect(dialog).to_be_visible(timeout=15_000)
|
|
assert page.evaluate("() => document.activeElement.id") == "remove-confirm-cancel"
|
|
|
|
# …and KEYBOARD focus moves between the two buttons, each drawing
|
|
# the house 3px :focus-visible outline (Tab Cancel→Remove,
|
|
# Shift+Tab back).
|
|
page.keyboard.press("Tab")
|
|
assert page.evaluate("() => document.activeElement.id") == "remove-confirm-remove"
|
|
outline = page.evaluate(
|
|
"() => getComputedStyle(document.activeElement).outlineWidth"
|
|
)
|
|
assert outline == "3px", f"focus-visible outline missing on Remove: {outline!r}"
|
|
page.keyboard.press("Shift+Tab")
|
|
assert page.evaluate("() => document.activeElement.id") == "remove-confirm-cancel"
|
|
outline = page.evaluate(
|
|
"() => getComputedStyle(document.activeElement).outlineWidth"
|
|
)
|
|
assert outline == "3px", f"focus-visible outline missing on Cancel: {outline!r}"
|
|
|
|
# Touch targets ≥44px (both buttons).
|
|
for btn in ("#remove-confirm-cancel", "#remove-confirm-remove"):
|
|
box = page.locator(btn).bounding_box()
|
|
assert box is not None and box["height"] >= 44, f"target too small: {box}"
|
|
|
|
# A real removal: the success line lands in the announcer
|
|
# (role=status, aria-live=polite).
|
|
page.locator("#remove-confirm-remove").click()
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(0, timeout=30_000)
|
|
expect(dialog).to_be_hidden()
|
|
announcer = page.locator("#git-sources-announcer")
|
|
assert announcer.get_attribute("role") == "status"
|
|
assert announcer.get_attribute("aria-live") == "polite"
|
|
expect(announcer).to_have_text(REMOVAL_ANNOUNCE)
|
|
assert _stored_sources(page, app_url) == []
|
|
assert _docs(page, app_url) == []
|
|
|
|
# No CDN (rule 6): no https:// asset tags; every script/link ref is
|
|
# same-origin or a data: URI (the test_git_sources_admin.py pin).
|
|
html = page.content()
|
|
assert 'src="https://' not in html and 'href="https://' not in html
|
|
refs = page.evaluate(
|
|
"""() => [...document.querySelectorAll("script[src], link[href]")]
|
|
.map((el) => el.src || el.href)"""
|
|
)
|
|
assert refs, "expected local asset references"
|
|
for ref in refs:
|
|
assert ref.startswith(app_url) or ref.startswith("data:"), (
|
|
f"non-local asset reference: {ref}"
|
|
)
|