From 07fbc4c4502c5b3f5521caefa195f521f47857f1 Mon Sep 17 00:00:00 2001 From: ducoterra Date: Sun, 27 Sep 2026 13:26:30 -0400 Subject: [PATCH] chore(agent): phase roadmap from TODO.md, 3 phases (134-136) Protocol B append: the TODO's three items became phases 134 (cancel a running sync), 135 (per-source git branch), and 136 (force-push reset). TODO.md cleared; each phase carries a dedicated Playwright E2E task. LLM-Generated: true --- .../phases/todo/134_sync_cancel/00_phase.md | 36 ++++++++++ .../01_backend_cancel_state.md | 63 ++++++++++++++++ .../02_frontend_cancel_button.md | 45 ++++++++++++ .../134_sync_cancel/03_e2e_sync_cancel.md | 36 ++++++++++ .../todo/135_git_source_branch/00_phase.md | 40 +++++++++++ .../135_git_source_branch/01_branch_column.md | 38 ++++++++++ .../135_git_source_branch/02_branch_api.md | 52 ++++++++++++++ .../03_clone_pull_branch.md | 48 +++++++++++++ .../135_git_source_branch/04_branch_ui.md | 43 +++++++++++ .../05_e2e_git_source_branch.md | 44 ++++++++++++ .../todo/136_force_push_reset/00_phase.md | 39 ++++++++++ .../136_force_push_reset/01_ff_or_reset.md | 71 +++++++++++++++++++ .../02_e2e_force_push_resync.md | 42 +++++++++++ 13 files changed, 597 insertions(+) create mode 100644 .agents/phases/todo/134_sync_cancel/00_phase.md create mode 100644 .agents/phases/todo/134_sync_cancel/01_backend_cancel_state.md create mode 100644 .agents/phases/todo/134_sync_cancel/02_frontend_cancel_button.md create mode 100644 .agents/phases/todo/134_sync_cancel/03_e2e_sync_cancel.md create mode 100644 .agents/phases/todo/135_git_source_branch/00_phase.md create mode 100644 .agents/phases/todo/135_git_source_branch/01_branch_column.md create mode 100644 .agents/phases/todo/135_git_source_branch/02_branch_api.md create mode 100644 .agents/phases/todo/135_git_source_branch/03_clone_pull_branch.md create mode 100644 .agents/phases/todo/135_git_source_branch/04_branch_ui.md create mode 100644 .agents/phases/todo/135_git_source_branch/05_e2e_git_source_branch.md create mode 100644 .agents/phases/todo/136_force_push_reset/00_phase.md create mode 100644 .agents/phases/todo/136_force_push_reset/01_ff_or_reset.md create mode 100644 .agents/phases/todo/136_force_push_reset/02_e2e_force_push_resync.md diff --git a/.agents/phases/todo/134_sync_cancel/00_phase.md b/.agents/phases/todo/134_sync_cancel/00_phase.md new file mode 100644 index 0000000..fa57764 --- /dev/null +++ b/.agents/phases/todo/134_sync_cancel/00_phase.md @@ -0,0 +1,36 @@ +# Phase 134 — The owner can cancel a running sync from the Sources page + +**Source:** `TODO.md` L3 — "Need to be able to cancel a sync after I've clicked the button." +**Story:** n/a (TODO-derived phase, Protocol B append). +**Context:** `app/api/sync.py` — `SyncStatus` (the four-state dataclass: `idle` / `running` / `success` / `failed`, phase-64 progress fields, phase-98 phase-machine fields), the module-level `_status` + `_task`, `POST /api/sync` (202 / 409 one-sync-at-a-time), `GET /api/sync/status`, and `_run_sync` — the in-process `asyncio` background task: the blocking git prelude (`scripts.git_sync.clone_or_pull` → `subprocess.run`, no await inside), then `await import_sources(...)` (per-file LLM awaits — a cancellation point between files), then `await regenerate_overview(...)` and `await generate_folder_summaries(...)` (both only on a changed KB / gap). The module docstring's shutdown contract: `CancelledError` is deliberately NOT swallowed — app shutdown cancels the task, and swallowing would mask a real stop. `import_sources` closes its own private session in a `finally` (leak-safe under a mid-import cancel). `frontend/assets/sources.js` — the sync-button lifecycle: `enterSyncRunningState` (live click + re-attach + every running tick), `settleSyncButton` (every terminal), `applySyncSuccess` / `applySyncFailure` / `applySyncIdle`, `startSyncPolling` (the 2 s two-job decision tree, 7 branches), `initSyncButton` (load-time re-attach). `frontend/index.html` — the `#sync-btn` markup (the Sources view section). E2E patterns: `tests/e2e/test_sync_button.py` (per-module app on a dedicated port, the `file://` fixture repo, the `_wait_sync_done` status-poll helper), `tests/e2e/slow_llm.py` (the delay-injecting proxy that makes a run's duration deterministic), `tests/e2e/mock_llm.py`. Unit: `tests/unit/test_sync_button.py`; integration: `tests/integration/test_sync_api.py`. + +## Objective +A sync that is already running can be stopped by the owner: while a run is in flight the Sources page shows a **Cancel** button; one click requests the cancel, the pipeline stops at the next boundary, the run lands in a new `cancelled` terminal state (reported by the status endpoint and rendered by the UI as a neutral result — cancel is intent, not failure), and the controls settle back to the retry-ready idle state. A subsequent sync starts cleanly (the partial run leaves a consistent KB — the importer's content-hash delta makes the next run idempotent). + +## Dependencies +- `133_pdf_screenshot_fallback` (todo) — pipeline order only; no code dependency. + +## Design (shared by all tasks — the executor reads this, not the chat) +- **The fifth state — `cancelled`** (ASSUMPTION A1): `SyncStatus.state` becomes `Literal["idle", "running", "success", "failed", "cancelled"]`. It is TERMINAL like `success` / `failed`: carries `finished_at`, clears `current_file` / `phase` / `current_summary` but keeps the run's final `files_done` / `files_total` and summary counts (the phase-64/98 keep-final-counts convention), `detail = {}`, and **`error = None`** (no error string — the UI never treats it as a failure: no error banner, no error modal, just the neutral "Sync cancelled" result line). `GET /api/sync/status` gains nothing structurally — the `state` key simply reports `"cancelled"`. +- **`POST /api/sync/cancel`** (same `router`, so `require_admin` — 403 anonymous): while a run is in flight → set the module-level `_cancel_event` (an `asyncio.Event`, next to `_status` / `_task`) AND call `_task.cancel()` (unblocks whatever `await` the pipeline is parked on — e.g. a per-file LLM call inside the import) → **202** `{"detail": "cancel requested"}`. No run in flight (`_task` None or done) → **409** `{"detail": "no sync is running"}`. `start_sync` resets `_cancel_event` (clear) when it starts a run. +- **Cancellation semantics — cooperative, best-effort** (ASSUMPTION A2): `_run_sync` gains (a) a private `_check_cancelled()` helper that raises `asyncio.CancelledError()` when `_cancel_event.is_set()`, called at the pipeline boundaries — the top of the per-source loop (before each clone/pull or local-dir check), immediately before `import_sources(...)`, and immediately before the `if summary.added + summary.updated > 0:` block (covers the overview + summaries spans); and (b) an `except asyncio.CancelledError:` clause (placed before the existing `except Exception`): if `_cancel_event.is_set()` → land the run in the `cancelled` terminal state + one INFO log line (PLAN §9: `sync: cancelled by user state=cancelled`), then `raise` (the task ends cancelled — `_task.done()` is True, the status object stays authoritative); otherwise (no user cancel — the app-shutdown path) → `raise` unchanged (the module docstring's shutdown contract holds: an unrequested cancellation is never swallowed). Consequence the UI/tests may rely on: a git subprocess already running at the cancel moment runs to completion (the blocking `subprocess.run` has no await inside) — the cancel takes effect 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 UI** (ASSUMPTION A3): a `#sync-cancel-btn` button in the same sync-control area as `#sync-btn` (`frontend/index.html`), `hidden` by default; secondary button styling consistent with the theme (contrast ≥4.5:1, `:focus-visible` ring — house UI rules). `sources.js` rules: SHOWN + ENABLED only while a SYNC run is running (`enterSyncRunningState` with `kind === "sync"` — the upload job is NOT cancellable in this phase, so the cancel button stays hidden while only the upload run drives the button); HIDDEN + DISABLED by `settleSyncButton` (every terminal/idle settle) and by every `applySync*` path. Click → `requestCancel()`: the cancel button disables IMMEDIATELY (never stale, no double request), `POST /api/sync/cancel`; 403 → the whoami-backstop hide-everything branch (same as the other endpoints); 409 → a harmless race (the run ended between ticks — the poll settles on the next tick, no error copy); 202 → the button stays disabled until the terminal (the poll's new `cancelled` branch settles everything); any other non-2xx → the phase-55 neutral retry copy in `#sync-result` ("Couldn't cancel the sync — try again.") + the cancel button re-enables. New poll-tree branch (after the `failed` branch, before the idle branch): `syncStatus.state === "cancelled"` → stop polling, settle the main button to "Sync sources", `#sync-result` = "Sync cancelled" (the aria-live announcer reads it), hide the error banner, hide the cancel button, `emitSyncStatus`. `initSyncButton` re-attach: `state === "cancelled"` → the same neutral settle (a page reload during/right after a cancel shows "Sync cancelled", never a stuck running state). + +## Tasks +1. `01_backend_cancel_state.md` — the `cancelled` state, `POST /api/sync/cancel`, and the cooperative cancel mechanics in `_run_sync`. +2. `02_frontend_cancel_button.md` — the `#sync-cancel-btn` UI: show/hide rules, the cancel request lifecycle, the `cancelled` terminal branch in the poll tree + re-attach. +3. `03_e2e_sync_cancel.md` — the dedicated Playwright E2E (the slow-LLM cancel window, the API contract, the full UI lifecycle, the clean re-sync after cancel). + +## Testing & Quality +- Unit: the cancel endpoint's contract (202/409/403), `_check_cancelled`, the `CancelledError` conversion (event set → `cancelled` state + re-raise; event unset → re-raise with the state untouched), the `SyncStatus` reset on a fresh start (a stale cancel event cannot bleed into the next run). +- Integration: a real run started through the API is cancelled mid-run → the status reports `cancelled` (with `error: None`); a second run after the cancel completes successfully (the restart is clean). +- E2E: one dedicated Playwright file, run in isolation (`tests/e2e/test_sync_cancel.py`) — the mock-only app with the `slow_llm` proxy for a deterministic cancel window. +- Coverage: **>90%** on new/modified code (`app/`). + +## Completion Criteria +- [ ] `POST /api/sync/cancel` (admin): 202 while a run is in flight, 409 `no sync is running` otherwise, 403 anonymous +- [ ] a cancelled run lands in the `cancelled` terminal state (`error: None`, final counts kept) and the status endpoint reports it +- [ ] the app-shutdown cancel contract is preserved (an unrequested `CancelledError` is never swallowed — a unit test pins it) +- [ ] the UI: the Cancel button is visible exactly while a sync run is running, the `cancelled` terminal settles to "Sync cancelled" with no error banner/modal, and a re-sync after a cancel succeeds +- [ ] `uv run pytest --cov=app --cov-report=term-missing` green, `app/` coverage >90%; `uv run pytest tests/e2e/test_sync_cancel.py -v --no-cov` green in isolation +- [ ] no behavior change in completed phases (the uncancelled run is byte-identical: same states, same labels, same 409 double-trigger) diff --git a/.agents/phases/todo/134_sync_cancel/01_backend_cancel_state.md b/.agents/phases/todo/134_sync_cancel/01_backend_cancel_state.md new file mode 100644 index 0000000..b76c408 --- /dev/null +++ b/.agents/phases/todo/134_sync_cancel/01_backend_cancel_state.md @@ -0,0 +1,63 @@ +# Task 01 — Backend: the `cancelled` state, `POST /api/sync/cancel`, and the cooperative cancel in `_run_sync` + +**Phase:** `134_sync_cancel` · **Source:** `TODO.md:3` — "Need to be able to cancel a sync after I've clicked the button." + +## Objective +The sync API can stop its own in-flight run: a new admin-only `POST /api/sync/cancel` requests the cancel; `_run_sync` converts a user-requested cancellation into a new `cancelled` terminal state (never an error); the shutdown-cancel contract is untouched. + +## Work +1. `app/api/sync.py` — the state machine: + - `SyncStatus.state`: `Literal["idle", "running", "success", "failed", "cancelled"]` (the docstring's four-state paragraph becomes five; `cancelled` is terminal like `success` / `failed`). + - Module level, next to `_status` / `_task`: `_cancel_event = asyncio.Event()`. + - `start_sync`: reset the run's cancellation — `_cancel_event.clear()` before `_task = asyncio.create_task(_run_sync())` (a stale event from a previous cancelled run must never bleed into the next run). +2. `app/api/sync.py` — the endpoint (same `router` → `require_admin` applies, 403 anonymous): + ```python + @router.post("/cancel", status_code=202) + async def cancel_sync() -> dict[str, str]: + 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"} + ``` + 202 is answered the moment the request is REGISTERED (the run may already be terminal — the UI's poll then settles on the terminal state; a 202-then-immediate-404-style race is impossible because the status object, not the task, is authoritative). +3. `app/api/sync.py` — `_run_sync` mechanics: + - A private helper: `def _check_cancelled() -> None:` — `if _cancel_event.is_set(): raise asyncio.CancelledError()`. + - Call it at the boundaries: (a) the top of the per-source loop (before the `kind == "git"` / `kind == "local"` split), (b) immediately before `await import_sources(...)`, (c) immediately before the `if summary.added + summary.updated > 0:` block (covers both the overview and the summaries spans). + - New `except asyncio.CancelledError:` clause placed BEFORE the existing `except Exception`: + ```python + except asyncio.CancelledError: + if _cancel_event.is_set(): + _status.state = "cancelled" + _status.finished_at = datetime.now(UTC) + _status.error = None + _status.current_file = None + _status.phase = None + _status.current_summary = None + logger.info( + "sync: cancelled by user state=cancelled files_done=%d files_total=%d", + _status.files_done, _status.files_total, + ) + raise + raise # app shutdown — the module-docstring contract: never swallow an unrequested cancel + ``` + The terminal keeps `files_done` / `files_total` and the summary counts (the phase-64/98 keep-final-counts convention); `detail` stays `{}` (a partial run reports no counts — ASSUMPTION A1: `error` is None, `detail` is empty). + - Update the module docstring: the state machine is five-state; the cancel semantics are cooperative (a git subprocess in flight at the cancel moment runs to completion — the cancel lands at the next boundary); the `CancelledError` paragraph now names BOTH conversion paths (user-requested → `cancelled` state + re-raise; unrequested → re-raise). +4. `GET /api/sync/status` — no shape change; update its docstring (the `state` value set now includes `cancelled`; `error` is null in that state). +5. Unit tests — `tests/unit/test_sync_button.py` (extend) or a new `tests/unit/test_sync_cancel.py` (your call — the existing file owns the endpoint's contract, so extending it is the default): + - `POST /api/sync/cancel` anonymous → 403; idle (never started, and after a terminal run) → 409 with the exact detail `no sync is running`. + - A started run: the endpoint returns 202 `{"detail": "cancel requested"}`, sets the event, and cancels the task (drive `_run_sync` with a controllable await — e.g. a monkeypatched `import_sources` that awaits a `asyncio.Event` the test controls — then cancel via the endpoint and assert the task finished + `_status.state == "cancelled"`, `error is None`, `finished_at` set, final counts kept, `detail == {}`). + - The shutdown contract pin: cancel the task WITHOUT setting the event (bare `task.cancel()`) → `_status.state` is NOT `cancelled` (stays `running` — the task died mid-run without a terminal state, exactly as today's shutdown path does) and the `CancelledError` propagated. + - The stale-event pin: a run that was cancelled, then a FRESH run starts → `_cancel_event` is cleared (the new run completes to `success` with no cancel request). + +## Testing & Quality +- Unit: the behaviors in Work step 5 (the suite above IS the task's unit layer). +- Integration: `tests/integration/test_sync_api.py` (extend) — start a real run against the `file://` fixture pattern the file already uses (a slow/many-file fixture or a monkeypatched delay at the per-file hook seam — follow the file's existing fakes), `POST /api/sync/cancel` while running → poll `GET /api/sync/status` → `state == "cancelled"` (and `error is None`); then a second full run → `state == "success"` (the restart after a partial cancel is clean — the importer's hash delta re-imports what the cancel skipped). +- Coverage: **>90%** on new/modified code. + +## Completion Criteria +- [ ] `uv run pytest tests/unit/test_sync_button.py tests/integration/test_sync_api.py -v` green (plus the new test file, if split) +- [ ] the five-state machine is exercised in tests: `cancelled` is reachable ONLY through a user-requested cancel (the unrequested-cancel pin is green) +- [ ] the module docstring names the cooperative boundary semantics (a future reader sees why a long clone can outlive the click) +- [ ] `uv run ruff check . && uv run pyright` clean +- [ ] no behavior change for the uncancelled run (the existing sync tests stay green untouched) diff --git a/.agents/phases/todo/134_sync_cancel/02_frontend_cancel_button.md b/.agents/phases/todo/134_sync_cancel/02_frontend_cancel_button.md new file mode 100644 index 0000000..acbf437 --- /dev/null +++ b/.agents/phases/todo/134_sync_cancel/02_frontend_cancel_button.md @@ -0,0 +1,45 @@ +# Task 02 — Frontend: the `#sync-cancel-btn` and the `cancelled` terminal branch in the poll tree + +**Phase:** `134_sync_cancel` · **Source:** `TODO.md:3` — "Need to be able to cancel a sync after I've clicked the button." + +## Objective +The Sources page shows a Cancel button exactly while a sync run is in flight (live click, re-attached run, every running poll tick); clicking it requests the cancel and the UI settles into the neutral "Sync cancelled" terminal — no error banner, no modal, never stale. + +## Work +1. `frontend/index.html` (the Sources view section, next to `#sync-btn`): add the cancel control — + ```html + + ``` + - `hidden` by default (anonymous-safe + the idle state; the module reveals it, the same convention as `#sync-btn`). + - A visible "Cancel" label (no icon-only — WCAG 2.4 name/role; the accessible name is the text). + - `frontend/assets/styles.css`: `.sync-cancel-btn` — a secondary button style consistent with the theme (the house tokens, contrast ≥4.5:1 on `--surface`, `:focus-visible` ring like the other buttons, disabled state). No layout regression on the sync-control row (mobile included — the row already wraps; verify the 360 px case). +2. `frontend/assets/sources.js` — the lifecycle (all inside the existing sync section; the §7.4 never-stale contract applies exactly as to the main button): + - Element lookup: `const syncCancelBtn = root.querySelector("#sync-cancel-btn");` + - Show/hide helpers: `showSyncCancelBtn()` (unhide + enable + `aria-label="Cancel sync"`) / `hideSyncCancelBtn()` (hide + disable). + - `enterSyncRunningState(kind, …, status)`: when `kind === "sync"` → `showSyncCancelBtn()`; when `kind === "upload"` → `hideSyncCancelBtn()` (the upload job is NOT cancellable in this phase — the cancel button belongs to the sync run only). + - `settleSyncButton(label)`: always `hideSyncCancelBtn()` (every terminal/idle settle — sync success/failed/cancelled, upload success/failed, both idle). + - `requestCancel()` + the click listener (armed next to `syncBtn.addEventListener("click", startSync)`): + - `syncCancelBtn.disabled = true` FIRST (never stale, one request at a time), `#sync-result` cleared. + - `POST /api/sync/cancel`: 403 → the whoami-backstop branch (stop polling, hide the main button, `applySyncIdle()`) — 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; the cancel button is hidden by that settle; no error copy); 202 → nothing (the button stays disabled — the poll's `cancelled` branch settles everything); any other non-2xx / network failure → `#sync-result` = "Couldn't cancel the sync — try again." (the phase-55 neutral retry copy) + `syncCancelBtn.disabled = false` (the run is still going — the owner can retry). + - New terminal branch in `startSyncPolling`'s tick — placed AFTER the `failed` branch and BEFORE the upload-success branch: + ```js + if (syncStatus.state === "cancelled") { + stopSyncPolling(); + applySyncCancelled(syncStatus); + return; + } + ``` + - `applySyncCancelled(status)`: `settleSyncButton("Sync sources")` (re-enables the main button, hides the cancel button), `syncResult.textContent = "Sync cancelled"`, `hideSyncError()`, `emitSyncStatus(status)`. No modal, no error styling (ASSUMPTION A1 — cancel is intent). + - `initSyncButton` re-attach: `status.state === "cancelled"` → `applySyncCancelled(status)` (a reload during/right after a cancel shows the neutral result, never a stuck running state). + - Update the section-header docstring (the decision tree gains the `cancelled` branch — the tree's "7" becomes "8" — and the cancel button's show/hide contract is documented there, the house pattern). +3. No change to the upload job, the main sync button's existing labels, or the `bor:sync-status` event shape (the `cancelled` state simply flows through `emitSyncStatus` — the event's `detail` is the status object, as for every other state). + +## Testing & Quality +- No JS unit layer in this repo — the UI contract is pinned by the phase's E2E (task 03) and by the never-stale rules above. +- Coverage: **>90%** on new/modified code (test-only-for-`app/` task — the bar is carried by the suite run; the JS is covered by the E2E). + +## Completion Criteria +- [ ] the Cancel button is visible + enabled ONLY while a sync run is running (hidden on boot/idle, hidden while only the upload run drives the button, hidden on every terminal) +- [ ] clicking Cancel settles to "Sync cancelled" in `#sync-result` with the main button re-enabled, the cancel button hidden, no error banner and no modal +- [ ] a 409 cancel (race) is silent; a failed cancel request shows the neutral retry copy and re-enables the button (the run keeps going) +- [ ] `uv run ruff check . && uv run pyright` clean; the page renders with no console errors (verified in the E2E, task 03) diff --git a/.agents/phases/todo/134_sync_cancel/03_e2e_sync_cancel.md b/.agents/phases/todo/134_sync_cancel/03_e2e_sync_cancel.md new file mode 100644 index 0000000..0fb2d44 --- /dev/null +++ b/.agents/phases/todo/134_sync_cancel/03_e2e_sync_cancel.md @@ -0,0 +1,36 @@ +# Task 03 — E2E: `tests/e2e/test_sync_cancel.py` (the dedicated Playwright suite, run in isolation) + +**Phase:** `134_sync_cancel` · **Source:** `TODO.md:3` — "Need to be able to cancel a sync after I've clicked the button." + +## Objective +The owner's cancel path is pinned end to end against the REAL pipeline: a real `file://` fixture sync whose duration is long enough to cancel (the `slow_llm` proxy), the API contract (403/409/202), the UI lifecycle (Cancel button visible → click → "Sync cancelled" settle), and the clean re-sync after a partial run. + +## Work +1. `tests/e2e/test_sync_cancel.py` (new) — the `test_sync_button.py` module-app pattern, adapted: + - **Ports/env:** per-module app on its own port — `APP_PORT = int(os.environ.get("E2E_APP_PORT_SYCNCEL", "8133"))`; `BOR_STATIC_DIR`, `BOR_RELEVANCE_THRESHOLD=0.30` (mock-calibrated, the conftest pattern), `BOR_ADMIN_PASSWORD` / `BOR_SESSION_SECRET` from `e2e.conftest`, `BOR_GIT_SOURCES=file://`, a dedicated `BOR_SOURCES_DIR`. + - **The slow-LLM proxy:** spawn `tests/e2e/slow_llm.py` the conftest way (the `SLOW_LLM_DELAY_S` env — 0.3 s is a good default: ~40 fixture files → ~12+ s of import — a wide, deterministic cancel window against the UI's 2 s poll) in front of the mock LLM (the `mock_llm` fixture from `e2e.conftest`); `BOR_LLM_BASE_URL` points at the proxy. + - **The fixture repo** (module-scoped, `tmp_path_factory`, real `git` subprocess — the `sync_git_repo` pattern): ~40 small A9-format `.md` files under a `notes/` folder (each one unique enough to import independently; ONE sentinel file — `Marker: RESE-CANCEL-SENTINEL-4f1e` — the assertion target for the re-sync). + - **`_wait_sync_state(page, app_url, states, timeout_s)`** — the `test_sync_button._wait_sync_done` helper generalized to a set of terminal states (here `{"cancelled"}` and `{"success", "failed"}` for the re-sync). +2. Tests (Playwright Mapping Rule — each test names the behavior it pins): + - `test_anonymous_cancel_403` — anonymous: `POST /api/sync/cancel` → 403 (the admin-only surface, same as the other two endpoints); `#sync-cancel-btn` is never revealed (the whoami gate). + - `test_cancel_when_idle_409` — logged-in, no run in flight: `POST /api/sync/cancel` → 409 with the exact detail `no sync is running`; the Cancel button is hidden in the UI (boot + idle). + - `test_admin_cancel_lifecycle` — the owner's exact flow: + 1. login (lands on `/sources.html`); + 2. click `#sync-btn` → `#sync-label` "Syncing…", the main button disabled, **`#sync-cancel-btn` visible + enabled** (within the 2 s poll tick — the running state is already entered synchronously by the 202); + 3. confirm via the status endpoint that the run is `running` (the deterministic layer — `page.request` poll at ~100 ms, the `slow_llm` module's convention); + 4. click `#sync-cancel-btn` → the cancel button disables immediately (never stale); + 5. `_wait_sync_state(…, {"cancelled"})` → the status endpoint reports `state == "cancelled"` with `error is None`; + 6. the UI settles (timeout-bound): main button ENABLED + label "Sync sources", `#sync-cancel-btn` hidden, `#sync-result` text "Sync cancelled", the error banner hidden, no error modal in the DOM; + 7. **the re-sync is clean:** click `#sync-btn` again → "Synced HH:MM" (the `SYNCED_LABEL` regex) + `#sync-result` includes "40 added" (or the fixture's file count — the first FULL pass; the cancelled run's partial import is idempotent under the hash delta — assert the sentinel row is present in the catalog via the drill-down tree, the `_expect_fixture_row` pattern) → the cancel never corrupted the KB. + - Wall-time guard: the whole module < 120 s (40 files × 0.3 s proxy delay + the re-sync's 40 × 0.3 s — if it drifts, cut the file count, not the guard). +3. Header docstring: the story ("the owner's TODO item, L3"), the run command (`uv run pytest tests/e2e/test_sync_cancel.py -v --no-cov`), the isolation note (per-module app, dedicated port — the combined-run convention), the slow-LLM provenance (the proxy is the only thing that makes the cancel window deterministic). + +## Testing & Quality +- E2E: this file IS the task's suite; it runs in ISOLATION (`--no-cov`, the AGENTS.md rule 9 gate) with the DB up (`podman compose up -d db`) and git on PATH. +- Coverage: **>90%** on new/modified code (test-only task — the bar is carried by the suite run). + +## Completion Criteria +- [ ] `uv run pytest tests/e2e/test_sync_cancel.py -v --no-cov` green in isolation +- [ ] the cancel window is deterministic (the test does not race the 2 s poll — the `slow_llm` delay is the gate; a rerun on a quiet machine passes) +- [ ] the re-sync after a cancel imports the FULL fixture (the sentinel row present — a partial cancel never leaves a corrupted or permanently-incomplete KB) +- [ ] the `cancelled` state's `error` is null end to end (the UI can never render it as a failure) diff --git a/.agents/phases/todo/135_git_source_branch/00_phase.md b/.agents/phases/todo/135_git_source_branch/00_phase.md new file mode 100644 index 0000000..fccd426 --- /dev/null +++ b/.agents/phases/todo/135_git_source_branch/00_phase.md @@ -0,0 +1,40 @@ +# Phase 135 — A git source can pin the branch it mirrors + +**Source:** `TODO.md` L4 — "Need to be able to specify the branch for a git source" +**Story:** n/a (TODO-derived phase, Protocol B append). +**Context:** `app/models.py::GitSource` (id, url, kind, path, ignore_paths, include_hidden, token, added_at — no branch column; the phase-121 `token` column is the nullable-write-only precedent); `alembic/versions/` (0023 is the newest; every migration is reversible — the house rule); `app/schemas.py` — `GitSourceIn` / `GitSourceOut` / `GitSourceRow` / `GitSourcePatchIn` (the token field is the write-trimmed precedent; the output shapes use `extra="forbid"`); `app/api/git_sources.py` — `POST` (the `_create_git_row` / `_create_local_row` split, the fixed-detail 422s, the token normalization), `PATCH /{source_id}` (the per-field present-wins tri-state — the token's absent/None = no change, `""` = clear, non-empty = replace), `GET` (DB rows vs the env-fallback synthetic rows); `scripts/git_sync.py::clone_or_pull(url, dest)` (fresh clone with full history; the shallow probe + one-time `git fetch --unshallow` self-heal (phase 107 D11); `git pull --ff-only` — the fail-loud D12 contract); the TWO call sites that resolve effective sources and clone git rows: `app/api/sync.py::_run_sync` and `scripts/import_docs.py::_resolve_sources` (both already pass `clone_url_for(row)` + `repo_name(row.url)`); `file_commit_dates` (walks the checkout — untouched); the UI: `frontend/index.html` — the add form `#git-source-form` (the url + token inputs) and the per-row editor dialog `#ignore-editor-dialog` (the source line, the ignore textarea, the token field); `frontend/assets/git-sources.js` — `loadSources` / `makeRow` (the "hidden on" text-tag idiom in the source cell), `wireAddForm` (`opts.body(value, token)` builds the POST body), the dialog open/save handlers (the PATCH body builder), the `toggleHidden` never-stale lifecycle as the per-row control precedent. + +## Objective +A git source can specify the branch it mirrors: the owner gives a branch when adding a git source (or edits it per-row later), the clone/pull honors it (a fresh clone checks the branch out; an existing checkout tracks it), BOTH entry points — the Sync button and the `import_docs` CLI — honor it, and every row without a branch (all existing rows + the env-fallback rows) behaves byte-identically to today (default branch, `git pull --ff-only`). + +## Dependencies +- `134_sync_cancel` (todo) — pipeline order only; no code dependency (independent surfaces). + +## Design (shared by all tasks — the executor reads this, not the chat) +- **The column** (ASSUMPTION B1): `git_sources.branch` — `TEXT NULL`, server default NULL; alembic **0024** (reversible — `add_column` / `drop_column`, no data migration: NULL = "the repo's default branch", so every existing row is unchanged by definition). `GitSource.branch: Mapped[str | None] = mapped_column(Text, nullable=True)`. +- **Validation** (ASSUMPTION B2): a pure helper `validate_branch_name(raw: str) -> str` in `app/rag/git_sources.py` (next to `normalize_credential` — the write-path helper home; the API layer maps its `ValueError` to the fixed-detail 422). Contract: trim → reject empty; reject length > 200; reject anything not matching `^[A-Za-z0-9._/-]+$` (alphanumerics + dot, underscore, hyphen, slash — whitespace/`@`/`~`/`^`/`:`/`?`/`*`/`[`/backslash are all out); reject a name containing `..` or `//`; reject a leading `-`, `.`, or `/`; reject a trailing `/` or `.`; reject `HEAD` (case-insensitive — git's refname; a branch literally named `head` is a documented cost of the simple rule). Returns the trimmed value. The API's 422 detail is the FIXED string `not a valid branch name` (never echoes the input — the house credential-safety discipline, applied for consistency). +- **The write paths** (ASSUMPTION B3): `GitSourceIn.branch: str | None` (optional at create — absent/empty → NULL; trimmed in the schema before the length constraint, the `_trim_url` precedent). `GitSourcePatchIn.branch: str | None = None` — TRI-STATE exactly like the token: absent/None = no change; `""` = clear (stored NULL); non-empty = set (re-validated). A `branch` on a LOCAL row (POST or PATCH) → 422 fixed detail `branch is only valid for git sources` (local rows are walked, not cloned). `GitSourceOut.branch: str | None` + `GitSourceRow.branch: str | None` (env-fallback synthetic rows report `None` — no DB row to store one on, the `ignore_paths: []` precedent). +- **`clone_or_pull(url, dest, branch: str | None = None)`** (ASSUMPTION B4): `branch=None` → the EXISTING code path byte-identical (same argv: clone without `--branch`; probe; unshallow-if-shallow; `git pull --ff-only`). A fresh clone with a branch → `git clone --branch url dest`. An existing checkout with a branch → (1) the HEAD GUARD first: `git rev-parse --abbrev-ref HEAD` — if the result is not `` → `GitSyncError("checkout is on branch '' but the source specifies branch '' — remove and re-add the source to switch branches")` (ASSUMPTION B5 — fail loud, the D12 convention; NO silent checkout switch in this phase: the checkout may serve other rows' documents, and a switch would silently re-map the source's file set; phase 136's explicit-ref reset makes the ref unambiguous later without changing this guard); (2) the shallow probe + one-time unshallow (unchanged); (3) `git fetch origin `; (4) `git merge --ff-only FETCH_HEAD` (the single-fetch ff pull — the sequence phase 136 extends with its reset fallback). +- **Wiring:** both call sites pass `row.branch` — `app/api/sync.py::_run_sync` and `scripts/import_docs.py::_resolve_sources`: `clone_or_pull(clone_url_for(row), sources_root / repo_name(row.url), row.branch)`. Env-fallback synthetic rows carry `branch=None` (the `GitSource(url=…, kind="git")` constructor default). `file_commit_dates`, the checkout naming (`repo_name` stays on the BARE url), and the ignore/hidden/date maps are untouched. +- **The UI** (ASSUMPTION B6): the add form gains an optional `#git-source-branch` text input (maxlength 200, `autocomplete="off"`, hint "optional — default: the repo's default branch") between the URL and Token fields; the submit body becomes `(url, token, branch) => ({ url, ...(branch ? { branch } : {}), ...(token ? { token } : {}) })` — a blank branch OMITS the key (NULL). The per-row editor dialog gains a `#ignore-editor-branch` field (visible for GIT rows only — local rows' dialogs hide it; it opens prefilled from `s.branch ?? ""`; the PATCH body for a git row ALWAYS includes `branch: ` — empty = the tri-state's clear leg, so clearing a branch is a deliberate empty save, unlike the token's blank = keep). Every STORED git row with a branch shows a `branch: ` text tag in the source cell (the "hidden on" tag idiom — text + background, WCAG: the text IS the signal, never color alone). + +## Tasks +1. `01_branch_column.md` — the alembic 0024 migration, the `GitSource.branch` column, the schema fields, the migration round-trip test. +2. `02_branch_api.md` — `validate_branch_name` + POST/PATCH/GET wiring (the tri-state, the local-row 422, the env rows). +3. `03_clone_pull_branch.md` — `clone_or_pull(…, branch)` (the `--branch` clone, the HEAD guard, the fetch+ff sequence) + both call sites. +4. `04_branch_ui.md` — the add-form field, the per-row editor field, the row tag. +5. `05_e2e_git_source_branch.md` — the dedicated Playwright E2E (the two-branch `file://` fixture, the UI add-with-branch → sync → branch content, the fail-loud mismatch, the API contracts). + +## Testing & Quality +- Unit: `validate_branch_name` (the full accept/reject matrix); `clone_or_pull`'s argv sequences with mocked `subprocess` (the `tests/unit/test_git_sync.py` pattern — fresh-clone `--branch`, the existing-checkout guard → probe → fetch → merge, the mismatch raise, `branch=None` byte-identical). +- Integration: the migration round-trip (0024); the API round-trips (create/patch/list, the tri-state, the 422s); a real `file://` two-branch clone through `clone_or_pull` (the `tests/integration/test_import_docs_git.py` pattern) + the sync pipeline importing the branch's content. +- E2E: one dedicated Playwright file, run in isolation. +- Coverage: **>90%** on new/modified code (`app/`). + +## Completion Criteria +- [ ] a git source added with a branch mirrors THAT branch (fresh clone + subsequent pulls) through both the Sync button and the CLI — pinned by the E2E + the integration test +- [ ] a row with no branch (all existing rows + env rows) behaves byte-identically to pre-phase (the `branch=None` argv pin is green) +- [ ] invalid branch names are rejected with the fixed 422 (never echoing input); a branch on a local row is a 422; the PATCH tri-state (set/change/clear) round-trips +- [ ] setting a branch on an existing checkout on a different branch fails the sync LOUDLY with the remove-and-re-add message (no silent switch) +- [ ] `uv run pytest --cov=app --cov-report=term-missing` green, `app/` coverage >90%; `uv run pytest tests/e2e/test_git_source_branch.py -v --no-cov` green in isolation +- [ ] no behavior change in completed phases (the token/ignore/hidden round-trips, the sync pipeline, the CLI — all existing tests green untouched) diff --git a/.agents/phases/todo/135_git_source_branch/01_branch_column.md b/.agents/phases/todo/135_git_source_branch/01_branch_column.md new file mode 100644 index 0000000..9b31719 --- /dev/null +++ b/.agents/phases/todo/135_git_source_branch/01_branch_column.md @@ -0,0 +1,38 @@ +# Task 01 — The `git_sources.branch` column: migration 0024, the model field, the schema fields + +**Phase:** `135_git_source_branch` · **Source:** `TODO.md:4` — "Need to be able to specify the branch for a git source" + +## Objective +The registry can store a per-source branch: a reversible alembic 0024 adds the nullable `branch` column, the ORM model carries it, and the four API schema shapes report/accept it — with NULL meaning "the repo's default branch" so every existing row is unchanged. + +## Work +1. `alembic/versions/0024_git_source_branch.py` (new) — follow the shape of `0021_git_source_token.py` (read it first): + - `revision = "0024"`, `down_revision = "0023"` (verify the actual latest revision id in `alembic/versions/` at execution time — the file numbering is the convention, the `down_revision` chain is the truth). + - `upgrade`: `op.add_column("git_sources", sa.Column("branch", sa.Text(), nullable=True))`. + - `downgrade`: `op.drop_column("git_sources", "branch")` (reversible — the house rule; NULL = default branch means no data migration in either direction). +2. `app/models.py::GitSource` — add: + ```python + #: The branch to mirror (phase 135): NULL = the repo's default branch + #: (every pre-phase-135 row and every env-fallback row — byte-identical + #: clone/pull behavior). A non-NULL value is validated at the API + #: boundary (app.rag.git_sources.validate_branch_name) — the column + #: itself is plain Text. + branch: Mapped[str | None] = mapped_column(Text, nullable=True) + ``` +3. `app/schemas.py` (the four shapes — each docstring gains its phase-135 paragraph): + - `GitSourceIn.branch: str | None = Field(default=None, max_length=200)` — optional at create (absent → NULL); a `field_validator` trims before the length constraint runs (the `_trim_url` precedent) — note the API layer treats a trimmed-EMPTY value as absent at create and as the tri-state's clear leg at PATCH (document that split in the docstring). + - `GitSourceOut.branch: str | None` — the stored value (NULL → null in the response); the shape keeps `extra="forbid"` (no new omission risk — the field is explicit). + - `GitSourceRow.branch: str | None` — DB rows report the stored value; env-fallback synthetic rows report `None` (the `ignore_paths: []` / `include_hidden: False` precedent — no DB row to store one on). + - `GitSourcePatchIn.branch: str | None = None` — the tri-state (absent/None = no change; `""` = clear; non-empty = set — the token's precedent, documented in the docstring). +4. `uv run alembic upgrade head` against the dev DB (and `downgrade -1` + `upgrade head` once — the reversibility proof). +5. `tests/integration/test_migration_0024.py` (new — the `test_migration_0021.py` pattern): the upgrade adds the nullable column to a table WITH pre-existing rows (all report NULL — the byte-identical guarantee), a row can take a branch value, the downgrade drops the column. + +## Testing & Quality +- Integration: the migration round-trip (the suite above). +- Coverage: **>90%** on new/modified code (the model/schema surface is exercised by task 02's API tests — this task's own gate is the migration test + the suite run). + +## Completion Criteria +- [ ] `uv run alembic upgrade head && uv run alembic downgrade -1 && uv run alembic upgrade head` succeeds on a scratch DB (the reversibility proof) +- [ ] pre-existing rows read `branch: None` (the integration test asserts on seeded rows — no backfill, no default) +- [ ] `uv run pytest tests/integration/test_migration_0024.py -v` green +- [ ] `uv run ruff check . && uv run pyright` clean diff --git a/.agents/phases/todo/135_git_source_branch/02_branch_api.md b/.agents/phases/todo/135_git_source_branch/02_branch_api.md new file mode 100644 index 0000000..5cd1cac --- /dev/null +++ b/.agents/phases/todo/135_git_source_branch/02_branch_api.md @@ -0,0 +1,52 @@ +# Task 02 — API: branch validation, POST/PATCH/GET wiring (the tri-state, the local-row 422, the env rows) + +**Phase:** `135_git_source_branch` · **Source:** `TODO.md:4` — "Need to be able to specify the branch for a git source" + +## Objective +The admin API accepts, validates, stores, and reports a branch per git source: a pure `validate_branch_name` helper is the single choke point, POST stores it (git rows only), PATCH edits it with the token's tri-state, and GET/POST-201/PATCH-200 report it — invalid names get a fixed-detail 422 that never echoes the input. + +## Work +1. `app/rag/git_sources.py` — the pure validator (next to `normalize_credential`; no DB, no logging of the raw value beyond the fixed reason): + ```python + _BRANCH_RE = re.compile(r"^[A-Za-z0-9._/-]+$") + + def validate_branch_name(raw: str) -> str: + """Trim + validate a branch name (phase 135, ASSUMPTION B2). + + Returns the trimmed name; raises ValueError (fixed reason strings, + for logs only — the API maps to the fixed-detail 422) when the + name is empty, > 200 chars, outside the safe charset + ([A-Za-z0-9._/-]+), contains ".." or "//", starts with "-"/"."/"", + ends with "/" or ".", or is HEAD (case-insensitive). + """ + ``` + (The leading-slash check is `startswith("/")`; write the helper with the checks in this order: trim → empty → length → charset → `..`/`//` → leading → trailing → HEAD.) +2. `app/api/git_sources.py`: + - `_create_git_row`: after the URL contract (and the token normalization — order: URL shape → credential normalization → duplicate check → branch): `branch = (payload.branch or "").strip()`; if `branch`: `validate_branch_name` → store; else store `None`. A `ValueError` → `HTTPException(422, detail="not a valid branch name")` (FIXED detail — the input is never echoed). + - `_create_local_row`: `payload.branch` present and non-empty after strip → `HTTPException(422, detail="branch is only valid for git sources")` (local rows are walked, not cloned — the fixed detail, the kind-field rules' precedent). + - `patch_git_source`: a new independent present-wins field — `if payload.branch is not None:` the row must be `kind == "git"` (local → the same fixed 422); `""` → `row.branch = None` (the tri-state's clear leg); non-empty → `validate_branch_name` → set. No Integrity interaction (branch never touches a unique index) — no 409 path. Update the docstring's field list (ignore_paths / include_hidden / token / branch). + - `list_git_sources`: DB rows report `branch=row.branch`; env-fallback rows report `branch=None`. Update the docstring. + - `create_git_source` / `patch_git_source` responses: include `branch=row.branch` in the `GitSourceOut` construction. + - The module docstring's route summary gains the branch clauses (the house one-paragraph-per-route contract). +3. Unit tests — `tests/unit/test_git_sources_branch.py` (new; the validator has no existing home): + - Accept: `main`, `release/1.2`, `feature_x`, `a.b/c`, `v2-rc.1` (trim applied: `" main "` → `main`). + - Reject (each → `ValueError`): `""`, `" "`, 201 chars, `a b`, `a@b`, `a~b`, `a:b`, `a?b`, `a*b`, `a[b`, `a\b`, `a..b`, `a//b`, `-x`, `.x`, `/x`, `x/`, `x.`, `HEAD`, `head`, `HeAd`. +4. Integration tests — `tests/integration/test_git_sources_api.py` (extend — the file already owns this router's contract): + - POST a git source with `branch: "release"` → 201 reports `branch: "release"`; GET reports it. + - POST with an invalid branch (`"a..b"`) → 422 `not a valid branch name` (the detail is EXACT — no input echo); the row is not created. + - POST a LOCAL source with a branch → 422 `branch is only valid for git sources`. + - PATCH the tri-state: set (`"hotfix/x"`) → 200 reports it; change (`"hotfix/y"`); clear (`""`) → 200 reports `null`; absent (PATCH `{"include_hidden": true}` only) → the branch is untouched. + - PATCH a local row's branch → 422. + - Env-fallback rows (empty table + `BOR_GIT_SOURCES` env): GET reports `branch: null` for every synthetic row. + - The existing token/ignore/hidden round-trips stay green (no regression). + +## Testing & Quality +- Unit: the validator matrix (step 3). +- Integration: the API round-trips (step 4). +- Coverage: **>90%** on new/modified code. + +## Completion Criteria +- [ ] `uv run pytest tests/unit/test_git_sources_branch.py tests/integration/test_git_sources_api.py -v` green +- [ ] every 422 detail is a fixed string (a grep of the new code shows no f-string echoing the branch input) +- [ ] the tri-state matches the token's semantics field-for-field (the PATCH docstring documents both) +- [ ] `uv run ruff check . && uv run pyright` clean diff --git a/.agents/phases/todo/135_git_source_branch/03_clone_pull_branch.md b/.agents/phases/todo/135_git_source_branch/03_clone_pull_branch.md new file mode 100644 index 0000000..5ac68f7 --- /dev/null +++ b/.agents/phases/todo/135_git_source_branch/03_clone_pull_branch.md @@ -0,0 +1,48 @@ +# Task 03 — `clone_or_pull(…, branch)` and both call sites (sync pipeline + CLI) + +**Phase:** `135_git_source_branch` · **Source:** `TODO.md:4` — "Need to be able to specify the branch for a git source" + +## Objective +The git plumbing honors a row's branch: a fresh clone checks it out (`--branch`), an existing checkout is tracked (fetch + ff-merge of that ref) with a fail-loud guard when the checkout is on a different branch, and `branch=None` keeps the pre-phase argv byte-identical — wired through BOTH entry points (the Sync button and the `import_docs` CLI). + +## Work +1. `scripts/git_sync.py::clone_or_pull` — the new optional parameter (the module docstring's clone/pull paragraph + the function docstring gain the branch contract): + ```python + def clone_or_pull(url: str, dest: Path | str, branch: str | None = None) -> Path: + ``` + - **`branch is None`** — the EXISTING code path, byte-identical: clone without `--branch`; `rev-parse --is-shallow-repository` probe; `fetch --unshallow` while shallow; `git pull --ff-only`. (The argv pin in the unit tests is the proof.) + - **Fresh clone + branch** (`dest` absent or no `.git`): `git clone --branch url dest`. + - **Existing checkout + branch**, in order: + 1. HEAD GUARD: `git rev-parse --abbrev-ref HEAD` → let `current` be the result. If `current != branch` → `GitSyncError(f"checkout {dest} is on branch '{current}' but the source specifies branch '{branch}' — remove and re-add the source to switch branches")` (ASSUMPTION B5 — fail loud per D12; the message names the checkout and both branch names — no URL, so no credential exposure). + 2. the shallow probe + one-time `git fetch --unshallow` self-heal (UNCHANGED from today — the phase-107 D11 contract). + 3. `git fetch origin ` → `git merge --ff-only FETCH_HEAD` (the single-fetch ff pull of the specified ref — the sequence phase 136 extends with its reset fallback; a non-ff merge still fails loud HERE — the force-push fix is phase 136, not this one). + - All commands go through `run_git` (the A11 single invocation site — unchanged). +2. The call sites — pass `row.branch`: + - `app/api/sync.py::_run_sync` (the per-source loop, the `kind == "git"` arm): `root = clone_or_pull(clone_url_for(row), sources_root / repo_name(row.url), row.branch)`. + - `scripts/import_docs.py::_resolve_sources` (the same shape): `root = clone_or_pull(clone_url_for(row), sources_root / repo_name(row.url), row.branch)`. + - Both files' docstrings/comments name the branch argument (the phase-106/121 comment style). Env-fallback synthetic rows carry `branch=None` (the `GitSource(url=…, kind="git")` constructor — no argument needed, the model default is None — verified by the env-fallback integration path). + - `file_commit_dates(root)` is untouched (it walks whatever the checkout holds — for a branch checkout that is the branch's tree, exactly the head-of-branch semantics the owner asked for). +3. Unit tests — `tests/unit/test_git_sync.py` (extend — the mocked-`subprocess` argv pattern the file already uses): + - `branch=None` fresh + existing: the argv sequence is byte-identical to the pre-phase expectations (the existing tests stay green + add an explicit argv-equality pin if the file doesn't already pin it). + - Fresh clone + `branch="release"`: `["git", "clone", "--branch", "release", url, dest]`. + - Existing + matching HEAD (`rev-parse --abbrev-ref HEAD` → `"release"`): the sequence `rev-parse --abbrev-ref HEAD` → `rev-parse --is-shallow-repository` → (`fetch --unshallow` when the probe says shallow) → `fetch origin release` → `merge --ff-only FETCH_HEAD`. + - Existing + mismatched HEAD (`"main"` vs `branch="release"`): raises `GitSyncError` whose message contains both branch names and the checkout path — and NO fetch/merge argv was invoked (the guard is first). + - The mismatch message never contains the URL (credential-safety — even for a plain URL the message is URL-free). +4. Integration tests — `tests/integration/test_import_docs_git.py` (extend — the real-`git`-`file://` pattern the file already uses): + - Build a two-branch fixture repo (real git: `main` with file A, `release` with file B — different content per branch). + - `clone_or_pull(url, dest, "release")` fresh → the checkout's HEAD branch is `release` and file B is present (file A absent). + - A second `clone_or_pull` after the origin gains a commit on `release` → ff-merged (file B updated, no error). + - The mismatch case: clone fresh with no branch (HEAD = `main`), then `clone_or_pull(url, dest, "release")` → `GitSyncError` (the remove-and-re-add message), the checkout untouched (still on `main`). + - The full sync pipeline with a branch row: a `git_sources` row (`kind="git"`, `branch="release"`) → `_run_sync`'s source resolution imports file B (the `tests/integration/test_sync_api.py` fixture shape) — or at minimum `_resolve_sources`-level for the CLI path (follow whichever seam the file already exercises; the E2E, task 05, pins the button path end to end). + +## Testing & Quality +- Unit: the argv matrix (step 3) — the `branch=None` byte-identical pin is the regression gate for every existing deployment. +- Integration: the real-git two-branch scenarios (step 4). +- Coverage: **>90%** on new/modified code. + +## Completion Criteria +- [ ] `uv run pytest tests/unit/test_git_sync.py tests/integration/test_import_docs_git.py -v` green +- [ ] `branch=None` produces the exact pre-phase argv (the pin is in the suite — a future refactor that rewrites the default path breaks it) +- [ ] the mismatch fails loud BEFORE any fetch (the unit test asserts no fetch argv after the guard) +- [ ] both entry points pass `row.branch` (a grep of the two call sites + the integration test covering each) +- [ ] `uv run ruff check . && uv run pyright` clean diff --git a/.agents/phases/todo/135_git_source_branch/04_branch_ui.md b/.agents/phases/todo/135_git_source_branch/04_branch_ui.md new file mode 100644 index 0000000..1c0741a --- /dev/null +++ b/.agents/phases/todo/135_git_source_branch/04_branch_ui.md @@ -0,0 +1,43 @@ +# Task 04 — UI: the add-form field, the per-row editor field, and the row tag + +**Phase:** `135_git_source_branch` · **Source:** `TODO.md:4` — "Need to be able to specify the branch for a git source" + +## Objective +The Sources manager (git-sources view) lets the owner see, set, change, and clear a source's branch: an optional Branch input in the add form, a Branch field in the per-row editor dialog (git rows only, prefilled, empty-save = clear), and a `branch: ` text tag on every stored git row that has one — all on the §7.4 never-stale lifecycle. + +## Work +1. `frontend/index.html` (the `#view-git-sources` section): + - **The add form** (`#git-source-form`) — between the URL field and the Token field: + ```html + + + ``` + (The visible `