diff --git a/.agents/phases/todo/135_git_source_branch/00_phase.md b/.agents/phases/complete/135_git_source_branch/00_phase.md similarity index 100% rename from .agents/phases/todo/135_git_source_branch/00_phase.md rename to .agents/phases/complete/135_git_source_branch/00_phase.md diff --git a/.agents/phases/todo/135_git_source_branch/01_branch_column.md b/.agents/phases/complete/135_git_source_branch/01_branch_column.md similarity index 100% rename from .agents/phases/todo/135_git_source_branch/01_branch_column.md rename to .agents/phases/complete/135_git_source_branch/01_branch_column.md diff --git a/.agents/phases/todo/135_git_source_branch/02_branch_api.md b/.agents/phases/complete/135_git_source_branch/02_branch_api.md similarity index 100% rename from .agents/phases/todo/135_git_source_branch/02_branch_api.md rename to .agents/phases/complete/135_git_source_branch/02_branch_api.md diff --git a/.agents/phases/todo/135_git_source_branch/03_clone_pull_branch.md b/.agents/phases/complete/135_git_source_branch/03_clone_pull_branch.md similarity index 100% rename from .agents/phases/todo/135_git_source_branch/03_clone_pull_branch.md rename to .agents/phases/complete/135_git_source_branch/03_clone_pull_branch.md diff --git a/.agents/phases/todo/135_git_source_branch/04_branch_ui.md b/.agents/phases/complete/135_git_source_branch/04_branch_ui.md similarity index 100% rename from .agents/phases/todo/135_git_source_branch/04_branch_ui.md rename to .agents/phases/complete/135_git_source_branch/04_branch_ui.md diff --git a/.agents/phases/todo/135_git_source_branch/05_e2e_git_source_branch.md b/.agents/phases/complete/135_git_source_branch/05_e2e_git_source_branch.md similarity index 100% rename from .agents/phases/todo/135_git_source_branch/05_e2e_git_source_branch.md rename to .agents/phases/complete/135_git_source_branch/05_e2e_git_source_branch.md diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__00_phase.a1.err b/.agents/reports/135_git_source_branch/135_git_source_branch__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__00_phase.a1.md b/.agents/reports/135_git_source_branch/135_git_source_branch__00_phase.a1.md new file mode 100644 index 0000000..4462975 --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__00_phase.a1.md @@ -0,0 +1,10 @@ +All verification complete. No edits were needed by this pass — the phase work and docs were already consistent and green. + +**Phase 135 final verification pass — report** + +- Verified all 5 tasks' work present & correct: migration 0024 (reversible), `GitSource.branch` + 4 schema shapes, `validate_branch_name` (full check order per design), `clone_or_pull(…, branch)` (HEAD guard first, `--branch` clone, fetch+ff merge), both call sites pass `row.branch`, UI (add-form field, editor field, row tag), `file://` URL transport. +- Completion criteria: (1) branch-mirrored through Sync + CLI — **pass** (E2E + integration); (2) `branch=None` byte-identical — **pass** (`test_branch_none_argv_is_byte_identical_pre_phase_sequence` green); (3) fixed 422s / local-row 422 / tri-state — **pass**; (4) mismatch fails loudly with remove-and-re-add — **pass** (E2E); (5) coverage 99% >90%, E2E isolated green — **pass**; (6) no regressions — **pass**. +- Tests: `uv run pytest --cov=app --cov-report=term-missing` → 3223 passed, app/ 99%; `uv run pytest tests/e2e/test_git_source_branch.py -v --no-cov` → 4 passed (16.4 s); `uv run ruff check .` clean; `uv run pyright` → 0 errors, 0 warnings. +- Documentation: README.md reviewed in full — phase-135 branch section accurate; the phase's README diff also correctly fixed a pre-existing stale line (shallow `--depth 1` clone vs full-history, phase 107). Routes, env defaults, feature lists verified against code. AGENTS.md — all 10 rules + quick-reference verified (referenced tests, PLAN.md, phases dirs, commands all accurate). **Verified consistent — no changes needed.** +- No live-infrastructure changes (dev DB already running and healthy; no live steps in this phase). +- Next pending phase: `.agents/phases/todo/136_force_push_reset`. diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__00_phase.a1.validate b/.agents/reports/135_git_source_branch/135_git_source_branch__00_phase.a1.validate new file mode 100644 index 0000000..6d3902c --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__00_phase.a1.validate @@ -0,0 +1,116 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 8%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 40%] +........................................................................ [ 42%] +........................................................................ [ 44%] +........................................................................ [ 46%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 67%] +........................................................................ [ 69%] +........................................................................ [ 71%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 78%] +........................................................................ [ 80%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 87%] +........................................................................ [ 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 259 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 133 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 56 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 422 0 100% +-------------------------------------------------- +TOTAL 5334 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/135_git_source_branch/135_git_source_branch__01_branch_column.a1.err b/.agents/reports/135_git_source_branch/135_git_source_branch__01_branch_column.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__01_branch_column.a1.md b/.agents/reports/135_git_source_branch/135_git_source_branch__01_branch_column.a1.md new file mode 100644 index 0000000..89666f0 --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__01_branch_column.a1.md @@ -0,0 +1,21 @@ +Task 01 complete — all gates green. Report: + +**Implemented** +- `alembic/versions/0024_git_source_branch.py`: reversible migration adding `git_sources.branch` (TEXT NULL, no default; NULL = repo's default branch) — `add_column`/`drop_column`, follows the 0021 pattern +- `app/models.py::GitSource.branch: Mapped[str | None] = mapped_column(Text, nullable=True)` with the phase-135 docstring +- `app/schemas.py`: `branch` on all four shapes — `GitSourceIn` (optional, max 200, `_trim_branch` before the length constraint; docstring documents the create-absent vs PATCH-clear split), `GitSourceOut`/`GitSourceRow` (explicit `str | None`, `extra="forbid"` kept; env rows report `None`), `GitSourcePatchIn` (tri-state documented, token precedent) +- `app/api/git_sources.py`: `branch` passed through at the four shape-construction sites (GET DB/env rows, POST 201, PATCH 200) + GET docstring — POST/PATCH storage/validation deliberately left to task 02 per its spec +- `tests/integration/test_migration_0024.py` (new, 0021 pattern): upgrade adds the column with the full contract, ALL pre-0024 rows (with/without token) backfill NULL, ORM round-trip, downgrade drops, round-trip restores +- Defect fix: `test_migration_0023.py` stale head pin ("land at head (0023)") now pins the 0022→0023 step explicitly (house 0021 pattern); exact-shape pins in `test_git_sources_api.py` (7 tests) and `test_git_source_token.py` kwargs updated for the new reported field + +**Results** +- `uv run alembic upgrade head && downgrade -1 && upgrade head` → 0023→0024→0023→0024, dev DB at `0024 (head)` +- `uv run pytest tests/integration/test_migration_0024.py -v --no-cov` → 4 passed +- `uv run pytest --cov=app --cov-report=term-missing` → **3154 passed, 0 failed**, `app/` coverage **99%** (>90%) +- `uv run ruff check . && uv run pyright` → clean (0 errors/warnings) + +**Docs**: no README/AGENTS.md changes — column/schema storage is not user-visible yet (API/UI land in tasks 02–04; README documents feature-level behavior, unchanged today). + +**Decisions**: kept POST/PATCH branch *storage* out of scope — task 02's file explicitly owns `_create_git_row`/`_create_local_row`/`patch_git_source` wiring; this task makes the shapes accept/report and the DB persist. The two uncovered `_trim_branch` bodies are exercised by task 02's API tests. + +**Next pending task**: `.agents/phases/todo/135_git_source_branch/02_branch_api.md` diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__01_branch_column.a1.validate b/.agents/reports/135_git_source_branch/135_git_source_branch__01_branch_column.a1.validate new file mode 100644 index 0000000..f9b5ee1 --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__01_branch_column.a1.validate @@ -0,0 +1,115 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 50%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 57%] +........................................................................ [ 59%] +........................................................................ [ 61%] +........................................................................ [ 63%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 79%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 93%] +........................................................................ [ 95%] +........................................................................ [ 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 133 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 422 2 99% +-------------------------------------------------- +TOTAL 5298 24 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/135_git_source_branch/135_git_source_branch__02_branch_api.a1.err b/.agents/reports/135_git_source_branch/135_git_source_branch__02_branch_api.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__02_branch_api.a1.md b/.agents/reports/135_git_source_branch/135_git_source_branch__02_branch_api.a1.md new file mode 100644 index 0000000..7e8f189 --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__02_branch_api.a1.md @@ -0,0 +1,17 @@ +Everything is green. Final report: + +**Implemented (task 02 — branch API wiring):** +- `validate_branch_name` in `app/rag/git_sources.py` (pure, next to `normalize_credential`; check order: trim → empty → 200-char → charset `[A-Za-z0-9._/-]+` → `..`/`//` → leading `-`/`.`/`/` → trailing `/`/`.` → HEAD ci; all `ValueError` reasons fixed strings, input never echoed) +- `app/api/git_sources.py`: POST git rows validate + store the branch (URL → credential → duplicate → branch order; absent/empty → NULL); POST local + PATCH-on-local → 422 `branch is only valid for git sources`; PATCH branch tri-state identical to the token (absent = keep, non-empty = validated set, `""` = clear); invalid name → fixed 422 `not a valid branch name` (exact, no input echo); module + route docstrings gained the branch clauses +- `tests/unit/test_git_sources_branch.py` (new, 33 tests: full accept/reject matrix, boundaries, input-free reasons) +- `tests/integration/test_git_sources_api.py` (+7 tests: create/store/report, trim, empty→NULL, invalid 422s, oversized, local 422s, full tri-state round-trip, local-PATCH 422, no-half-apply); env-row `branch: null` already pinned by task 01's tests +- README: documented branch pinning; fixed stale phase-107 doc drift ("shallow `--depth 1`" → full-history clone + one-time `--unshallow` self-heal). AGENTS.md: no doc impact + +**Verification (all 100% green):** +- `uv run pytest tests/unit/test_git_sources_branch.py tests/integration/test_git_sources_api.py` → 96 passed +- `uv run pytest --cov=app --cov-report=term-missing` (full suite, final code) → exit 0, no failures; `app/` coverage **99%** (`app/api/git_sources.py` 100%, `app/rag/git_sources.py` 100%) +- `uv run ruff check . && uv run pyright` → clean (0 errors) + +**Notes:** oversized-branch 422 is FastAPI's standard schema 422 (same house precedent as oversized URLs); API-layer branch 422s are the fixed-detail ones. + +**Next pending task:** `.agents/phases/todo/135_git_source_branch/03_clone_pull_branch.md` diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__02_branch_api.a1.validate b/.agents/reports/135_git_source_branch/135_git_source_branch__02_branch_api.a1.validate new file mode 100644 index 0000000..0f327e1 --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__02_branch_api.a1.validate @@ -0,0 +1,116 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 40%] +........................................................................ [ 42%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 54%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 63%] +........................................................................ [ 65%] +........................................................................ [ 67%] +........................................................................ [ 69%] +........................................................................ [ 72%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 78%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 85%] +........................................................................ [ 87%] +........................................................................ [ 90%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 96%] +........................................................................ [ 99%] +............................ [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 259 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 133 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 56 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 422 0 100% +-------------------------------------------------- +TOTAL 5334 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/135_git_source_branch/135_git_source_branch__03_clone_pull_branch.a1.err b/.agents/reports/135_git_source_branch/135_git_source_branch__03_clone_pull_branch.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__03_clone_pull_branch.a1.md b/.agents/reports/135_git_source_branch/135_git_source_branch__03_clone_pull_branch.a1.md new file mode 100644 index 0000000..f020968 --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__03_clone_pull_branch.a1.md @@ -0,0 +1,17 @@ +Done. All completion criteria met: + +**Implemented** +- `scripts/git_sync.py::clone_or_pull(url, dest, branch=None)`: fresh clone → `git clone --branch `; existing checkout → HEAD guard (`rev-parse --abbrev-ref HEAD`, fail-loud remove-and-re-add `GitSyncError` — URL-free message, before any fetch) → unchanged shallow probe + one-time `fetch --unshallow` (now the shared `_unshallow_if_shallow` helper) → `fetch origin ` + `merge --ff-only FETCH_HEAD`; `branch=None` argv byte-identical (module + function docstrings updated) +- Both call sites pass `row.branch`: `app/api/sync.py::_run_sync` (Sync button) and `scripts/import_docs.py::_resolve_sources` (CLI), with phase-121-style comments + module docstrings +- Tests: unit argv matrix in `test_git_sync.py` (branch=None byte-identical pin, `--branch` clone, matching/shallow guard→probe→fetch→merge sequences, mismatch raises with both branch names + checkout path and NO fetch argv, URL-free even with token URL, fetch/merge failure propagation); real-git two-branch `file://` integration scenarios in `test_import_docs_git.py` (fresh branch checkout, ff-merge pull, mismatch leaves checkout untouched, `_resolve_sources` with `branch="release"` row imports file B, env row clones default branch) + button-path tests in `test_sync_api.py` (real `clone_or_pull` through `_run_sync`: branch row + mismatch fail-loud); updated all `clone_or_pull` fakes across 5 test files for the 3-arg signature + +**Results** +- `uv run pytest --cov=app --cov-report=term-missing`: **3211 passed, 0 failed**, `app/` coverage **99%** (>90%); `scripts/git_sync.py` 100% in its test set +- `uv run pytest tests/unit/test_git_sync.py tests/integration/test_import_docs_git.py -v --no-cov`: 45 passed +- `uv run ruff check .` clean; `uv run pyright`: 0 errors + +**Docs**: README already carried the phase-135 branch paragraph from tasks 01/02 (accurate as-is); AGENTS.md no doc impact. + +**Decisions**: date-map assertion relaxed — `file_commit_dates` walks the reachable *log*, so a branch checkout also lists files from shared ancestor commits (harmless: the importer only consults paths it finds); left `file_commit_dates` untouched per the task. + +**Next pending task**: `.agents/phases/todo/135_git_source_branch/04_branch_ui.md` diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__03_clone_pull_branch.a1.validate b/.agents/reports/135_git_source_branch/135_git_source_branch__03_clone_pull_branch.a1.validate new file mode 100644 index 0000000..5bfc252 --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__03_clone_pull_branch.a1.validate @@ -0,0 +1,116 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 8%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 38%] +........................................................................ [ 40%] +........................................................................ [ 42%] +........................................................................ [ 44%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 65%] +........................................................................ [ 67%] +........................................................................ [ 69%] +........................................................................ [ 71%] +........................................................................ [ 73%] +........................................................................ [ 76%] +........................................................................ [ 78%] +........................................................................ [ 80%] +........................................................................ [ 82%] +........................................................................ [ 85%] +........................................................................ [ 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 259 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 133 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 56 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 422 0 100% +-------------------------------------------------- +TOTAL 5334 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/135_git_source_branch/135_git_source_branch__04_branch_ui.a1.err b/.agents/reports/135_git_source_branch/135_git_source_branch__04_branch_ui.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__04_branch_ui.a1.md b/.agents/reports/135_git_source_branch/135_git_source_branch__04_branch_ui.a1.md new file mode 100644 index 0000000..fe210e0 --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__04_branch_ui.a1.md @@ -0,0 +1,13 @@ +All gates green. Task complete. + +**Report — phase 135, task 04 (branch UI)** + +- **Add form** (`index.html` + `git-sources.js`): optional `#git-source-branch` (maxlength 200, autocomplete off, "optional — default: the repo's default branch" hint) between URL and Token; submit body `(url, token, branch) => ({ url, ...(branch ? {branch} : {}), ...(token ? {token} : {}) })` — blank omits the key; 201 clears all three inputs. +- **Per-row editor**: `#ignore-editor-branch` after the source line, before the ignore textarea (hint line "Leave empty for the repo's default branch."); git rows only (local rows hide label+input+hint and the body omits the key), prefilled from `s.branch ?? ""`; **tri-state asymmetry**: git rows' PATCH always includes `branch` (empty save = clear), token stays blank-keeps; existing §7.4 save lifecycle untouched. +- **Row tag**: `branch: ` text tag (the "hidden on" idiom, grouped CSS) for stored git rows with a branch; source-cell `title` names the branch; module docstring gained the phase-135 paragraph. +- **CSS**: branch input joins the URL/token box group (+placeholder/focus/mobile squeeze); editor field mirrors the token box (44px floor); tag joins the hidden-count rule. +- **Tests**: new `tests/unit/test_git_source_branch_ui.py` (12 source-level pins, house pattern); updated 5 stale pre-phase pins (3 unit, 1 E2E exact-body in `test_git_source_tokens.py` — now expects the mandated always-branch key) + the phase-121 docstring pin kept on its historical literal. +- **Results**: `uv run pytest --cov=app --cov-report=term` → 3223 passed, `app/` 99% (>90%); `uv run ruff check .` clean; `uv run pyright` → 0 errors; 7 related existing E2E suites green in isolation (admin, tokens, ignore-paths, hidden-toggle, local-dirs, archive-upload, dates). +- **Docs**: no doc impact — README already documents the branch feature (incl. the form field) from task 02; AGENTS.md is protocol-level. +- **Decisions**: task-05 E2E remains the formal browser gate for the new behavior (per task file); verified no regressions via the 7 existing real-browser suites. +- **Next pending task**: `05_e2e_git_source_branch.md`. diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__04_branch_ui.a1.validate b/.agents/reports/135_git_source_branch/135_git_source_branch__04_branch_ui.a1.validate new file mode 100644 index 0000000..6d3902c --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__04_branch_ui.a1.validate @@ -0,0 +1,116 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 8%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 40%] +........................................................................ [ 42%] +........................................................................ [ 44%] +........................................................................ [ 46%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 67%] +........................................................................ [ 69%] +........................................................................ [ 71%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 78%] +........................................................................ [ 80%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 87%] +........................................................................ [ 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 259 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 133 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 56 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 422 0 100% +-------------------------------------------------- +TOTAL 5334 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/135_git_source_branch/135_git_source_branch__05_e2e_git_source_branch.a1.err b/.agents/reports/135_git_source_branch/135_git_source_branch__05_e2e_git_source_branch.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__05_e2e_git_source_branch.a1.md b/.agents/reports/135_git_source_branch/135_git_source_branch__05_e2e_git_source_branch.a1.md new file mode 100644 index 0000000..916d9dc --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__05_e2e_git_source_branch.a1.md @@ -0,0 +1,17 @@ +Everything is green. Final report: + +**Implemented** +- `tests/e2e/test_git_source_branch.py` (new): the phase-135 dedicated Playwright suite — two-branch `file://` fixture (diverged branches: `release` drops `main-only.md`, shared `common.md` differs `VERSION: 1`/`2`) + a main-only clone; 4 tests: (1) UI add **with** branch → 201 → `branch: release` row tag → real sync indexes the branch's tree (sentinel docs present, `main-only.md` absent, modal content `VERSION: 2`) + idempotent "0 added · 2 unchanged"; (2) `branch=None` path on the clone = default branch exactly as pre-phase (`main-only.md` present, `release-only.md` absent, `VERSION: 1`, no tag); (3) per-row editor branch change → failed sync with the fixed `remove and re-add the source` copy in status/banner/modal, then restore → success; (4) API contracts via admin cookie (exact 422s, tri-state, local-row 422). Port 8134, dedicated `BOR_SOURCES_DIR`, empty `BOR_GIT_SOURCES`, 180 s wall guard. +- Fixed a blocking gap the task's UI flow exposed: `POST /api/git-sources` rejected `file://` (phase-35 URL contract). Extended `URL_RE` + fixed 422 detail to include `file://`, updated docstrings, and pinned the new shape in `tests/integration/test_git_sources_api.py`. + +**Test / lint / coverage** +- `uv run pytest tests/e2e/test_git_source_branch.py -v --no-cov` → **4 passed** in ~17 s (run 3×, stable) +- `uv run pytest --cov=app --cov-report=term` → **3223 passed, 0 failed**, app/ coverage **99%** (>90% gate) +- `uv run ruff check .` → clean; `uv run pyright` → 0 errors +- Sibling E2E suites touching the changed 422 detail (`test_git_sources_admin.py`, `test_git_source_tokens.py`) → 10 passed + +**Docs**: README.md gained the accepted-URL-shapes note (`file://` now a valid source); the branch-feature paragraph was already added by this phase's earlier tasks. AGENTS.md: no change needed (no new setup steps, commands, or env vars). + +**Notable decisions**: (a) the `file://` prefix addition (phase-35's "exactly four prefixes" was an assumption, not a PLAN LOCKED decision; the task's owner-via-UI flow requires it); (b) fixture branches must diverge — `release` deletes `main-only.md`, else the "wrong branch's doc absent" promise can't hold; (c) `sync_and_wait` closes a re-rendered failure modal before clicking (a Sources-view remount over a stale `failed` terminal re-renders it, and its fixed backdrop intercepts the `#sync-btn` click). + +**Next pending task**: none — task 05 is the final task of phase `135_git_source_branch`. diff --git a/.agents/reports/135_git_source_branch/135_git_source_branch__05_e2e_git_source_branch.a1.validate b/.agents/reports/135_git_source_branch/135_git_source_branch__05_e2e_git_source_branch.a1.validate new file mode 100644 index 0000000..6d3902c --- /dev/null +++ b/.agents/reports/135_git_source_branch/135_git_source_branch__05_e2e_git_source_branch.a1.validate @@ -0,0 +1,116 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 8%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 40%] +........................................................................ [ 42%] +........................................................................ [ 44%] +........................................................................ [ 46%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 67%] +........................................................................ [ 69%] +........................................................................ [ 71%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 78%] +........................................................................ [ 80%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 87%] +........................................................................ [ 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 259 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 133 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 56 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 422 0 100% +-------------------------------------------------- +TOTAL 5334 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/README.md b/README.md index 019d108..b1c1abb 100644 --- a/README.md +++ b/README.md @@ -241,15 +241,30 @@ BOR_SOURCES_DIR=~/bor-sources # default; each repo lands in // ``` - Public repos need no setup: auth is whatever the machine supplies — - HTTPS via the OS credential helper, or SSH via your key. + HTTPS via the OS credential helper, or SSH via your key. The API + accepts `https://`, `http://`, `ssh://`, `git@`, and `file://` URLs + (phase 135: a `file://` local-transport repo — e.g. on a mounted + volume — is a valid source too). - Private repos: the add/edit form on the Git sources page takes an optional **masked token** field (phase 121). The token is stored in a dedicated DB column and injected into the clone URL only at sync/clone time — it is never returned by any API response or shown in the UI (legacy rows that already embed `user:token@` in the stored URL keep working — the token is sanitized on output). -- Every run **clones** (first time, shallow `--depth 1`) or **pulls** - (`git pull --ff-only`) each repo, then indexes the checkout. +- A git source can pin the **branch it mirrors** (phase 135): the + add/edit form's optional branch field (or the API's `branch`) stores + the branch in the `git_sources.branch` column — a fresh clone checks + that branch out, an existing checkout tracks it (`git fetch origin + ` + `git merge --ff-only FETCH_HEAD`), and a checkout sitting + on a *different* branch fails the sync loudly with a remove-and- + re-add message (never a silent switch). Rows without a branch — + every pre-existing row and the env-fallback rows — keep the default + branch with byte-identical clone/pull behavior. +- Every run **clones** (first time, full history) or **pulls** + (`git pull --ff-only`) each repo, then indexes the checkout. A legacy + shallow checkout is unshallowed once on its first post-phase-107 sync + (`git fetch --unshallow` — the one-time self-heal) so commit dates + stay real. - A failed sync aborts the run — no partial junk. ### Archive upload sources diff --git a/alembic/versions/0024_git_source_branch.py b/alembic/versions/0024_git_source_branch.py new file mode 100644 index 0000000..96fa66a --- /dev/null +++ b/alembic/versions/0024_git_source_branch.py @@ -0,0 +1,45 @@ +"""git_sources.branch: per-source branch to mirror (phase 135, task 01) + +Revision ID: 0024 +Revises: 0023 +Create Date: 2026-09-27 + +Phase 135 (a git source can pin the branch it mirrors — task 01, storage +only): + +* ``git_sources.branch`` — TEXT NULLABLE, no server default: the branch + the owner specifies for a git source (TODO-derived phase). NULL = + the repo's DEFAULT branch — every pre-phase-135 row and every + env-fallback synthetic row is a default-branch source by definition, + so no row is touched and no backfill runs. A non-NULL value is + validated at the API boundary + (``app.rag.git_sources.validate_branch_name`` — task 02); the column + itself is plain ``Text`` (the ``git_sources.token`` (0021) + precedent: the column stores, the boundary validates). + +One additive, fully reversible migration (A13); no other schema +change. The branch-aware clone/pull wiring (``clone_or_pull(…, branch)`` +— task 03) and the API tri-state (task 02) are code — this revision +only carries the column. +""" +from __future__ import annotations + +import sqlalchemy as sa + +from alembic import op + +revision = "0024" +down_revision = "0023" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("git_sources", sa.Column("branch", sa.Text(), nullable=True)) + + +def downgrade() -> None: + # The branch column is the only 0024 artefact — dropping it leaves + # 0023's schema byte-identical (A13, fully reversible). NULL = + # default branch, so no data migration in either direction. + op.drop_column("git_sources", "branch") diff --git a/app/api/git_sources.py b/app/api/git_sources.py index c2a2bdc..157ca5f 100644 --- a/app/api/git_sources.py +++ b/app/api/git_sources.py @@ -16,19 +16,33 @@ Routes: ``GET`` (DB rows oldest-first, or the env list with ``from_env: true`` while the table is empty; rows carry ``kind`` + ``path``, git rows — and env rows — report ``path: null``; every row reports ``ignore_paths`` — DB rows their stored normalized list, env -rows ``[]``, phase 89 — and ``include_hidden`` — DB rows their stored -flag, env rows ``False``, phase 105), ``POST`` -(201, validated create; ``kind`` selects the validation: git → exactly -the phase-35 URL contract, local → an existing absolute directory, else -422 naming the path; both kinds accept ``ignore_paths`` — optional, -absent → ``[]``, stored normalized, phase 89 — and ``include_hidden`` -— optional, absent → ``False``, stored as sent, phase 105), -``PATCH /{source_id}`` (phase 89, A5 + phase 105 — the ignore list -(replace) and/or the hidden-folders flag (phase 105) — each optional, -present-wins: 404 unknown id; ``ignore_paths`` when present is -normalized + A4-validated with fixed-detail 422s and REPLACES the -row's list wholesale — an empty list clears all; ``include_hidden`` -when present sets the flag; both absent → 200 no-op; 200 → the +rows ``[]``, phase 89 — ``include_hidden`` — DB rows their stored +flag, env rows ``False``, phase 105 — and ``branch`` — DB rows their +stored value (NULL = the repo's default branch), env rows ``null``, +phase 135), ``POST`` +(201, validated create; ``kind`` selects the validation: git → the +phase-35 URL contract + the ``file://`` local transport (phase 135, +task 05 — the E2E fixture repos the UI adds end to end), local → an +existing absolute directory, else 422 naming the path; both kinds +accept ``ignore_paths`` — optional, absent → ``[]``, stored +normalized, phase 89 — and ``include_hidden`` +— optional, absent → ``False``, stored as sent, phase 105; git rows +accept ``branch`` — optional, absent/empty → NULL (the default +branch), non-empty validated by +:func:`app.rag.git_sources.validate_branch_name` with the fixed-detail +422 ``not a valid branch name`` (input never echoed, phase 135) — a +``branch`` on a LOCAL row is 422 ``branch is only valid for git +sources``), +``PATCH /{source_id}`` (phase 89, A5 + phase 105 + phase 121 + phase +135 — the ignore list (replace), the hidden-folders flag (phase 105), +the token (phase 121 tri-state), and the branch (phase 135 tri-state) +— each optional, present-wins: 404 unknown id; ``ignore_paths`` when +present is normalized + A4-validated with fixed-detail 422s and +REPLACES the row's list wholesale — an empty list clears all; +``include_hidden`` when present sets the flag; ``token`` and ``branch`` +when present are tri-state — absent/None = no change, non-empty = +set/replace (``branch`` re-validated, ``local`` rows 422), empty +string = clear (stored NULL); all absent → 200 no-op; 200 → the ``GitSourceOut`` shape), ``POST /upload`` (phase 49, backgrounded in phase 64 task 03, scan deferred in phase 90 — admin archive upload: the @@ -115,7 +129,11 @@ from app.rag.archive_upload import ( swap_in, unpack_archive, ) -from app.rag.git_sources import normalize_credential, sanitize_url +from app.rag.git_sources import ( + normalize_credential, + sanitize_url, + validate_branch_name, +) from app.rag.importer import normalize_ignore_path from app.rag.llm import LLMClient from app.rag.overview import regenerate_overview @@ -196,9 +214,12 @@ _upload_status = UploadStatus() #: Accepted git URL shapes — the trimmed URL must *start* with one of them. #: Covers the phase-28 real URLs (HTTPS + ``git@`` SSH); scp-style -#: ``host:repo`` is deliberately rejected (422). ASSUMPTION (task 02): the -#: accepted shapes are exactly these four prefixes. -URL_RE = re.compile(r"^(https?://|ssh://|git@)") +#: ``host:repo`` is deliberately rejected (422). ``file://`` (phase 135, +#: task 05) is the deterministic local-transport source the E2E fixture +#: repos use — a real ``git clone`` of a local repo (the phase-28/32/107 +#: transport-true E2E idiom, promoted from the ``BOR_GIT_SOURCES`` env +#: fallback to a stored row the UI can add). +URL_RE = re.compile(r"^(https?://|ssh://|file://|git@)") #: Phase 89, A4 — the per-source ignore-list limits, enforced in #: :func:`_validate_ignore_paths` (shared by POST and PATCH): at most @@ -219,13 +240,14 @@ def list_git_sources( same-timestamp inserts) with ``from_env: false`` — each row carries its ``kind``, its ``ignore_paths`` (phase 89 — the stored, normalized list; ``or []`` guards a row that predated the column), - its ``include_hidden`` (phase 105 — the stored flag), and, for - local rows, the stored ``path`` (git rows and env rows report - ``path: null``); while the table is empty, the + its ``include_hidden`` (phase 105 — the stored flag), its + ``branch`` (phase 135 — the stored value; NULL = the repo's default + branch), and, for local rows, the stored ``path`` (git rows and env + rows report ``path: null``); while the table is empty, the ``BOR_GIT_SOURCES`` env URLs as git rows (the env fallback is - git-only) with null ``id``/``added_at``, ``ignore_paths: []`` and - ``include_hidden: False`` (no DB row to store a list or a flag on), - and ``from_env: true``. + git-only) with null ``id``/``added_at``, ``ignore_paths: []``, + ``include_hidden: False`` and ``branch: None`` (no DB row to store a + list, a flag or a branch on), and ``from_env: true``. Every URL is masked on the way out (phase 121, LOCKED A2 — :func:`sanitize_url`): an env value or a legacy stored URL may @@ -248,6 +270,7 @@ def list_git_sources( added_at=row.added_at, ignore_paths=row.ignore_paths or [], include_hidden=row.include_hidden, + branch=row.branch, # phase 135: NULL = default branch ) for row in rows ], @@ -263,6 +286,7 @@ def list_git_sources( added_at=None, ignore_paths=[], include_hidden=False, + branch=None, # phase 135: no DB row to store a branch on ) for url in get_settings().git_source_list ], @@ -310,6 +334,16 @@ def create_git_source( credential in the token column, an explicit ``token`` wins over the embedded one (LOCKED A6), and the duplicate check runs on the bare URL. + + ``branch`` (phase 135) — git rows only: absent/None → stored NULL + (the repo's DEFAULT branch — the pre-phase-135 behavior), a + trimmed-EMPTY value likewise stores NULL (at CREATE, empty is + absent — the split from PATCH's clear leg, see :class:`GitSourceIn`), + non-empty → validated by :func:`validate_branch_name` and stored + (a ``ValueError`` → 422 ``not a valid branch name`` — FIXED detail, + the input is never echoed). A ``branch`` on a LOCAL row is 422 + ``branch is only valid for git sources`` (local rows are walked, + not cloned). """ row = _create_git_row(payload, db) if payload.kind == "git" else _create_local_row(payload, db) return GitSourceOut( @@ -318,6 +352,7 @@ def create_git_source( added_at=row.added_at, ignore_paths=row.ignore_paths, include_hidden=row.include_hidden, + branch=row.branch, # phase 135: NULL = default branch ) @@ -355,16 +390,21 @@ def _commit_new(row: GitSource, duplicate_detail: str, db: Session) -> GitSource def _create_git_row(payload: GitSourceIn, db: Session) -> GitSource: - """``kind=git`` — the phase-35 URL contract, unchanged (A10: no - credential echo, so every detail is a fixed string).""" + """``kind=git`` — the phase-35 URL contract, extended with the + ``file://`` local transport (phase 135, task 05 — the E2E fixture + repos the UI adds end to end); A10: no credential echo, so every + detail is a fixed string.""" if payload.path is not None: raise HTTPException(status_code=422, detail="a git source takes a url, not a path") if payload.url is None: raise HTTPException(status_code=422, detail="a git source requires a url") url = payload.url if not URL_RE.match(url): + # The detail lists the accepted prefixes (fixed string — the + # input is never echoed; phase 135 task 05 added file://). raise HTTPException( - status_code=422, detail="not a valid git URL (expected https://, ssh:// or git@…)" + status_code=422, + detail="not a valid git URL (expected https://, ssh://, file:// or git@…)", ) # Phase 121 (task 02, LOCKED A6): normalize the credential — an # old-style embedded ``user:pass@`` URL is stored BARE and the @@ -375,6 +415,18 @@ def _create_git_row(payload: GitSourceIn, db: Session) -> GitSource: url, effective_token = normalize_credential(url, payload.token) if db.scalar(select(GitSource).where(GitSource.url == url)) is not None: raise HTTPException(status_code=409, detail="a git source with this URL already exists") + # Phase 135: the branch pin (ASSUMPTION B2/B3) — absent or + # trimmed-EMPTY stores NULL (the repo's default branch, the + # pre-phase-135 behavior); non-empty is validated at the boundary + # with the FIXED-detail 422 (the raw name never echoes). + branch = (payload.branch or "").strip() + if branch: + try: + branch = validate_branch_name(branch) + except ValueError: + raise HTTPException( + status_code=422, detail="not a valid branch name" + ) from None return _commit_new( GitSource( url=url, @@ -382,6 +434,7 @@ def _create_git_row(payload: GitSourceIn, db: Session) -> GitSource: ignore_paths=_validate_ignore_paths(payload.ignore_paths), include_hidden=bool(payload.include_hidden), token=effective_token or None, + branch=branch or None, ), "a git source with this URL already exists", db, @@ -391,9 +444,17 @@ def _create_git_row(payload: GitSourceIn, db: Session) -> GitSource: def _create_local_row(payload: GitSourceIn, db: Session) -> GitSource: """``kind=local`` — fail-loud add-time validation (phase 38): trimmed → ``expanduser()`` → absolute + existing directory, else 422 - naming the path (not a secret, unlike a git URL).""" + naming the path (not a secret, unlike a git URL). A ``branch`` on a + local row (phase 135) is 422 — local rows are walked, not cloned.""" if payload.url is not None: raise HTTPException(status_code=422, detail="a local source takes a path, not a url") + if (payload.branch or "").strip(): + # Phase 135: local rows are walked, not cloned — a branch pin + # has no meaning for them (the kind-field rules' precedent: + # fixed detail, no input echo). + raise HTTPException( + status_code=422, detail="branch is only valid for git sources" + ) if payload.path is None: raise HTTPException(status_code=422, detail="a local source requires a path") expanded = Path(payload.path).expanduser() @@ -432,26 +493,37 @@ def patch_git_source( payload: GitSourcePatchIn, db: Session = Depends(get_db), # noqa: B008 ) -> GitSourceOut: - """Edit one source's ignore list, hidden-folders flag, and/or - private-repo token. + """Edit one source's ignore list, hidden-folders flag, private- + repo token, and/or branch. Phase 89 A5 (ignore list) + phase 105 (the flag) + phase 121 - (the token): 404 unknown id; each PRESENT body field applies - independently — ``ignore_paths`` REPLACES the list (normalized + - A4-validated, fixed 422 details); ``include_hidden`` sets the - flag; ``token`` is TRI-STATE (LOCKED A2): absent/None = no change - (the row's stored credential survives an edit that does not touch - the masked field), non-empty = replace, empty string = clear - (stored NULL). A PRESENT token also re-normalizes the (current - url, new token) pair with the POST write-path rules — a legacy - embedded-token URL gets its userinfo stripped (moved to the - column) the first time an explicit credential is written; a clean - URL comes back untouched. The 409 backstop: re-normalizing can - make the stored URL collide with another row's bare URL (a - legacy ``user:pass@`` row and a bare row for the same repo) — the - unique index yields the generic 409, never a 500. Returns the - updated row's public shape (id, url — masked, added_at, - ignore_paths, include_hidden); the token is never echoed. + (the token) + phase 135 (the branch): 404 unknown id; each PRESENT + body field applies independently — ``ignore_paths`` REPLACES the + list (normalized + A4-validated, fixed 422 details); + ``include_hidden`` sets the flag; ``token`` is TRI-STATE (LOCKED + A2): absent/None = no change (the row's stored credential survives + an edit that does not touch the masked field), non-empty = replace, + empty string = clear (stored NULL). A PRESENT token also + re-normalizes the (current url, new token) pair with the POST + write-path rules — a legacy embedded-token URL gets its userinfo + stripped (moved to the column) the first time an explicit + credential is written; a clean URL comes back untouched. The 409 + backstop: re-normalizing can make the stored URL collide with + another row's bare URL (a legacy ``user:pass@`` row and a bare row + for the same repo) — the unique index yields the generic 409, + never a 500. ``branch`` (phase 135) is an independent PRESENT- + wins field with the token's EXACT tri-state semantics: + absent/None = no change (the row's stored branch survives an edit + that does not touch it), non-empty = set (re-validated by + :func:`validate_branch_name` — a ``ValueError`` is the FIXED-detail + 422 ``not a valid branch name``, input never echoed), empty string + = clear (stored NULL = the repo's default branch). A PRESENT + branch on a LOCAL row is 422 ``branch is only valid for git + sources`` (local rows are walked, not cloned). Branch never + touches a unique index — no Integrity interaction, no 409 path. + Returns the updated row's public shape (id, url — masked, + added_at, ignore_paths, include_hidden, branch); the token is + never echoed. """ row = db.get(GitSource, source_id) if row is None: @@ -467,6 +539,24 @@ def patch_git_source( # bare + column credential. row.url, effective = normalize_credential(row.url, payload.token) row.token = effective or None + if payload.branch is not None: + # Phase 135: the branch tri-state (the token's exact semantics + # — see the docstring). Local rows are walked, not cloned — a + # branch pin is meaningless there (the same fixed 422 as POST). + if row.kind != "git": + raise HTTPException( + status_code=422, detail="branch is only valid for git sources" + ) + branch = payload.branch.strip() + if not branch: + row.branch = None # the tri-state's clear leg (stored NULL) + else: + try: + row.branch = validate_branch_name(branch) + except ValueError: + raise HTTPException( + status_code=422, detail="not a valid branch name" + ) from None try: db.commit() except IntegrityError: @@ -485,6 +575,7 @@ def patch_git_source( added_at=row.added_at, ignore_paths=row.ignore_paths, include_hidden=row.include_hidden, + branch=row.branch, # phase 135: NULL = default branch ) diff --git a/app/api/sync.py b/app/api/sync.py index 2164c82..9727404 100644 --- a/app/api/sync.py +++ b/app/api/sync.py @@ -37,7 +37,14 @@ decisions): — the row's ``token`` column injected as ``https://x-access-token:@…`` only for https? rows; NULL token → the bare stored URL verbatim, so public repos and legacy - embedded-token rows clone exactly as before) into + embedded-token rows clone exactly as before) and the row's + BRANCH PIN as the third argument (phase 135 — a fresh clone checks + it out, ``git clone --branch``; an existing checkout is guarded — a + mismatched branch fails loudly, the remove-and-re-add error — and + tracked, ``git fetch origin `` + ``git merge --ff-only + FETCH_HEAD``; ``branch=None``, i.e. every pre-phase-135 row and + every env-fallback row, keeps the pre-phase argv byte-identical) + into ``BOR_SOURCES_DIR//`` (phase 28 — reused, not re-implemented; the checkout name stays on the bare URL — credential-free); ``kind=local`` → the stored directory, re-verified @@ -498,7 +505,17 @@ async def _run_sync() -> None: # URL ONLY here (clone_url_for — NULL token → the bare # stored URL verbatim); repo_name stays on the bare URL # so the checkout directory name is credential-free. - root = clone_or_pull(clone_url_for(row), sources_root / repo_name(row.url)) + # Phase 135: the row's branch pin is the third + # argument — NULL (every pre-phase-135 row + every + # env-fallback row) keeps the pre-phase argv + # byte-identical; a pinned branch is checked out on a + # fresh clone (--branch) and guarded + tracked on an + # existing checkout (fetch + ff-merge of that ref). + root = clone_or_pull( + clone_url_for(row), + sources_root / repo_name(row.url), + row.branch, + ) # Phase 106 (D2): the checkout's per-file last-commit # dates, keyed by the SAME root string the importer # sees (full-history checkouts → true per-file diff --git a/app/models.py b/app/models.py index 76ce4bc..e3cab64 100644 --- a/app/models.py +++ b/app/models.py @@ -325,6 +325,12 @@ class GitSource(Base): #: (:func:`app.rag.git_sources.clone_url_for`); NEVER returned by #: any API shape (the output models gain no token field). token: Mapped[str | None] = mapped_column(Text, default=None) + #: 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) added_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) diff --git a/app/rag/git_sources.py b/app/rag/git_sources.py index 6f842a3..db4229e 100644 --- a/app/rag/git_sources.py +++ b/app/rag/git_sources.py @@ -36,6 +36,12 @@ resolver — three pure helpers, no DB of their own: old-style ``https://user:pass@host/repo.git`` URL pasted into the API is stored bare and the embedded credential is moved into the ``token`` column (an explicit ``token`` field wins — LOCKED A6). + +Phase 135 (per-source branch pinning) adds the write-path branch +validator next to them — :func:`validate_branch_name`, the single +choke point: the API maps its ``ValueError`` to the fixed-detail 422 +(the raw input is never echoed, the house credential-safety +discipline, applied for consistency). """ from __future__ import annotations @@ -153,6 +159,51 @@ def normalize_credential(url: str, token: str | None) -> tuple[str, str | None]: return sanitize_url(url), effective +#: Phase 135 (ASSUMPTION B2) — the safe branch-name charset: +#: alphanumerics + dot, underscore, hyphen, and slash (a slash allows +#: ``release/1.2``-style names — git branch names are paths). Anything +#: else (whitespace, ``@``, ``~``, ``^``, ``:``, ``?``, ``*``, ``[``, +#: backslash, …) is rejected by the character-set check. +_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 and + the raw input is never echoed) 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 — git's refname; a + branch literally named ``head`` is a documented cost of the simple + rule, the plan's design note). + + Pure — no DB, no logging of the raw value (the reason strings are + fixed, input-free: the same discipline that keeps credentials out + of the 422 details, applied here for consistency). + """ + name = raw.strip() + if not name: + raise ValueError("branch name must not be empty") + if len(name) > 200: + raise ValueError("branch name exceeds 200 characters") + if not _BRANCH_RE.match(name): + raise ValueError("branch name contains invalid characters") + if ".." in name or "//" in name: + raise ValueError("branch name contains a disallowed path sequence") + if name[0] in ("-", ".", "/"): + raise ValueError("branch name has a disallowed leading character") + if name[-1] in ("/", "."): + raise ValueError("branch name has a disallowed trailing character") + if name.upper() == "HEAD": + # Fixed reason, no echo: the input here is the reserved name + # itself, and the reason must stay input-free (the log-line + # discipline — a typed name never surfaces, reserved or not). + raise ValueError("branch name is reserved") + return name + + def effective_sources(db: Session) -> tuple[list[GitSource], Literal["db", "env"]]: """``(rows, origin)`` — the effective source rows (both kinds) and where they came from. diff --git a/app/schemas.py b/app/schemas.py index 210f2b5..cc69089 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -759,6 +759,18 @@ class GitSourceIn(BaseModel): WRITE-ONLY field — stored in the dedicated ``git_sources.token`` column and NEVER echoed back by any output shape (``GitSourceOut`` / ``GitSourceRow`` carry no token field by contract). + + ``branch`` (phase 135) is the branch the git source mirrors: + optional at create time (absent/None → stored NULL = the repo's + DEFAULT branch — every such row keeps its byte-identical + clone/pull behavior), trimmed *before* the length constraint runs + (the ``_trim_url`` precedent), max 200. Note the split from PATCH: + a trimmed-EMPTY value is treated as ABSENT at create (stored NULL, + same as omitted) — at PATCH, ``""`` is the tri-state's CLEAR leg + (see :class:`GitSourcePatchIn`). Non-empty values are validated at + the API boundary (``app.rag.git_sources.validate_branch_name``); + a ``branch`` on a ``local`` row is a 422 there (local rows are + walked, not cloned). """ kind: Literal["git", "local"] = "git" @@ -767,6 +779,7 @@ class GitSourceIn(BaseModel): ignore_paths: list[str] | None = Field(default=None) include_hidden: bool | None = Field(default=None) token: str | None = Field(default=None, max_length=500) + branch: str | None = Field(default=None, max_length=200) @field_validator("url", mode="before") @classmethod @@ -783,6 +796,11 @@ class GitSourceIn(BaseModel): def _trim_token(cls, v: object) -> object: return v.strip() if isinstance(v, str) else v + @field_validator("branch", mode="before") + @classmethod + def _trim_branch(cls, v: object) -> object: + return v.strip() if isinstance(v, str) else v + class GitSourceOut(BaseModel): """One created git source as returned by ``POST`` (phase 35, task 02). @@ -804,6 +822,12 @@ class GitSourceOut(BaseModel): the omission a structural contract, not an accident: constructing this model with a ``token`` key raises, so a regression that tries to echo the credential back cannot even build the shape. + + ``branch`` (phase 135) is the stored value — the row's + ``git_sources.branch`` (NULL → ``null`` in the response; NULL = + the repo's default branch). The field is explicit on the shape, so + ``extra="forbid"`` carries no new omission risk — the token's + structural-omission contract above is untouched. """ model_config = ConfigDict(extra="forbid") @@ -813,6 +837,7 @@ class GitSourceOut(BaseModel): added_at: datetime | None ignore_paths: list[str] include_hidden: bool + branch: str | None class GitSourceRow(BaseModel): @@ -829,6 +854,10 @@ class GitSourceRow(BaseModel): store a list on) report ``[]``. ``include_hidden`` (phase 105) is the row's stored flag — env-fallback rows (no DB row to store a flag on) report ``False`` (the ``ignore_paths: []`` precedent). + ``branch`` (phase 135) is the row's stored value (NULL → ``null`` + = the repo's default branch); env-fallback synthetic rows report + ``None`` — there is no DB row to store one on (the + ``ignore_paths: []`` / ``include_hidden: False`` precedent). There is deliberately NO ``token`` field (phase 121, LOCKED A2): same contract as :class:`GitSourceOut` — the credential never @@ -846,6 +875,7 @@ class GitSourceRow(BaseModel): added_at: datetime | None ignore_paths: list[str] include_hidden: bool + branch: str | None class GitSourcePatchIn(BaseModel): @@ -865,18 +895,31 @@ class GitSourcePatchIn(BaseModel): clear** (the UI offers replace; clear exists for API completeness). Trimmed *before* the length constraints run (the ``GitSourceIn`` ``_trim_token`` precedent — whitespace-only counts as a clear), - max 500. All absent → 200 no-op (the row is untouched). + max 500. ``branch`` (phase 135) is TRI-STATE with the token's + exact three-way semantics: **absent/None = no change** (keep the + row's stored branch), **non-empty = set** (re-validated at the API + boundary — ``app.rag.git_sources.validate_branch_name``), **empty + string = clear** (stored NULL = the repo's default branch). Trimmed + *before* the length constraint runs (the ``_trim_token`` precedent + — whitespace-only counts as a clear), max 200. All absent → 200 + no-op (the row is untouched). """ ignore_paths: list[str] | None = Field(default=None) include_hidden: bool | None = Field(default=None) token: str | None = Field(default=None, max_length=500) + branch: str | None = Field(default=None, max_length=200) @field_validator("token", mode="before") @classmethod def _trim_token(cls, v: object) -> object: return v.strip() if isinstance(v, str) else v + @field_validator("branch", mode="before") + @classmethod + def _trim_branch(cls, v: object) -> object: + return v.strip() if isinstance(v, str) else v + class GitSourceList(BaseModel): """``GET /api/git-sources`` response (phase 35, task 02). diff --git a/frontend/assets/git-sources.js b/frontend/assets/git-sources.js index 05d1d71..eb21c93 100644 --- a/frontend/assets/git-sources.js +++ b/frontend/assets/git-sources.js @@ -222,6 +222,36 @@ * token-free with no per-site change; the UI must never re-embed a * credential. * + * Phase 135 (task 04) — the branch pin (the field, the tri-state + * asymmetry, the tag rule): the add form gains the optional + * `#git-source-branch` (maxlength 200, autocomplete=off, the visible + * "optional — default: the repo's default branch" hint) between the + * URL and Token fields; the submit body is + * `(url, token, branch) => ({ url, ...(branch ? { branch } : {}), + * ...(token ? { token } : {}) })` — a BLANK branch OMITS the key + * (NULL = the repo's default branch), and 201 clears the input + * alongside the URL/token. The per-row editor mirrors it: + * `#ignore-editor-branch` (after the source line, before the ignore + * textarea, with the "Leave empty for the repo's default branch." hint + * line) opens PREFILLED from the stored value (`s.branch ?? ""` — + * unlike the token, the API reports it, so there IS a field to read it + * from) and is VISIBLE FOR GIT ROWS ONLY — local rows hide the label + + * input + hint and the save body OMITS the key (local rows are walked, + * not cloned — the API's 422 is the backstop, the UI simply doesn't + * offer it). THE TRI-STATE ASYMMETRY: the git row's PATCH body ALWAYS + * includes `branch: ` — an EMPTY save is the deliberate CLEAR + * leg (NULL = the repo's default branch), unlike the token's blank = + * keep (the token stays the blank = keep tri-state — untouched). Every + * STORED git row with a branch shows a `branch: ` text tag in the + * source cell (the "hidden on" tag idiom — text + background, never + * color alone: the text IS the signal); env-fallback rows (no `s.id`) + * and NULL-branch rows show no tag, and the source cell's title names + * the branch (the hover-precision idiom). The §7.4 lifecycle is the + * existing save handler's, unchanged: Save disables in flight, 200 → + * close the dialog + loadSources() (the row re-renders from the server + * — the tag appears/disappears), failure → the dialog's error line + + * the inputs keep the user's text. + * * Scope boundary (phase locked decisions): adding a git repo does * NOT clone — the sync service (server-side) does that. Removing a * source, however, performs the FULL cleanup server-side (phase 69): @@ -261,6 +291,9 @@ export async function mount(root) { /* Phase 121: the add form's optional masked token field — blank = no credential (the key is omitted from the POST body). */ const tokenInput = root.querySelector("#git-source-token"); + /* Phase 135: the add form's optional branch pin — blank = the key + is omitted (NULL = the repo's default branch). */ + const branchInput = root.querySelector("#git-source-branch"); const addBtn = root.querySelector("#git-source-add"); const addError = root.querySelector("#git-source-error"); /* Phase 49: the archive upload form (replaces the phase-38 local @@ -300,6 +333,14 @@ export async function mount(root) { /* Phase 121: the editor's masked token field — blank = keep the current token (the PATCH omits the key, the tri-state no-change). */ const ignoreTokenInput = root.querySelector("#ignore-editor-token"); + /* Phase 135: the editor's branch field group (label + input + hint) + — git rows only (local rows hide all three); prefilled from the + stored value, and a git row's save ALWAYS sends the key (an empty + save is the tri-state's CLEAR leg — unlike the token's blank = + keep). */ + const ignoreBranchLabel = root.querySelector('label[for="ignore-editor-branch"]'); + const ignoreBranchInput = root.querySelector("#ignore-editor-branch"); + const ignoreBranchHint = root.querySelector(".ignore-editor-branch-hint"); const ignoreErrorEl = root.querySelector("#ignore-editor-error"); const ignoreCancelBtn = root.querySelector("#ignore-editor-cancel"); const ignoreSaveBtn = root.querySelector("#ignore-editor-save"); @@ -423,7 +464,10 @@ export async function mount(root) { const urlTd = document.createElement("td"); urlTd.className = "git-source-url-cell"; - urlTd.title = value; // full URL/path on hover (long values scroll the wrapper) + // Full URL/path on hover (long values scroll the wrapper) — phase + // 135: a pinned branch is named too (the tag below is the + // at-a-glance signal, the title the full context). + urlTd.title = s.id && s.branch ? `${value} — branch: ${s.branch}` : value; const badge = document.createElement("span"); badge.className = `git-source-kind is-${isLocal ? "local" : "git"}`; badge.textContent = isLocal ? "Local" : "Git"; // text + color, never color alone @@ -450,6 +494,17 @@ export async function mount(root) { hiddenTag.textContent = "hidden on"; urlTd.append(hiddenTag); } + /* Phase 135 (A6): the "branch: " state tag — STORED git rows + with a branch only (env-fallback rows carry no id, NULL-branch + rows no truthy branch — no tag); the .git-source-hidden-count + idiom (TEXT + background, never color alone — WCAG 1.4.1: the + text IS the signal). */ + if (s.id && s.branch) { + const branchTag = document.createElement("span"); + branchTag.className = "git-source-branch-count"; + branchTag.textContent = `branch: ${s.branch}`; + urlTd.append(branchTag); + } tr.appendChild(urlTd); const addedTd = document.createElement("td"); @@ -726,6 +781,19 @@ export async function mount(root) { // token field to prefill from (LOCKED A2); blank = the PATCH // omits the key (no change — the stored token is kept). if (ignoreTokenInput) ignoreTokenInput.value = ""; + // Phase 135: the branch field — git rows open PREFILLED from the + // stored value (the API reports it on every git row — unlike the + // token, there IS a field to read it from); local rows HIDE the + // field group (label + input + hint — local rows are walked, not + // cloned; the API's 422 is the backstop, the UI simply doesn't + // offer it). + const branchIsGit = s.kind !== "local"; + if (ignoreBranchLabel) ignoreBranchLabel.hidden = !branchIsGit; + if (ignoreBranchHint) ignoreBranchHint.hidden = !branchIsGit; + if (ignoreBranchInput) { + ignoreBranchInput.hidden = !branchIsGit; + ignoreBranchInput.value = branchIsGit ? (s.branch ?? "") : ""; + } if (ignoreErrorEl) { ignoreErrorEl.textContent = ""; ignoreErrorEl.hidden = true; // a new attempt starts clean @@ -750,6 +818,7 @@ export async function mount(root) { ignoreInFlight = false; if (ignoreTextarea) ignoreTextarea.value = ""; if (ignoreTokenInput) ignoreTokenInput.value = ""; // phase 121: re-opens blank + if (ignoreBranchInput) ignoreBranchInput.value = ""; // phase 135: re-opens prefilled if (ignoreErrorEl) { ignoreErrorEl.textContent = ""; ignoreErrorEl.hidden = true; @@ -795,6 +864,15 @@ export async function mount(root) { // credential is kept); non-blank replaces it. const token = ignoreTokenInput ? ignoreTokenInput.value.trim() : ""; const t = ignoreTarget; + // Phase 135: the branch pin — the tri-state ASYMMETRY (module + // docstring): a GIT row's PATCH body ALWAYS includes `branch` + // (the trimmed value — an EMPTY save is the deliberate CLEAR leg: + // NULL = the repo's default branch), unlike the token's blank = + // keep. A LOCAL row's body OMITS the key (the field group is + // hidden in the dialog — local rows are walked, not cloned; the + // API's 422 is the backstop). + const isGitRow = t.kind !== "local"; + const branch = ignoreBranchInput ? ignoreBranchInput.value.trim() : ""; const value = t.kind === "local" ? (t.path ?? t.url) : t.url; ignoreInFlight = true; if (ignoreErrorEl) { @@ -810,7 +888,11 @@ export async function mount(root) { const r = await fetch(`/api/git-sources/${encodeURIComponent(t.id)}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ ignore_paths: lines, ...(token ? { token } : {}) }), + body: JSON.stringify({ + ignore_paths: lines, + ...(isGitRow ? { branch } : {}), // phase 135: git rows always ("" = clear) + ...(token ? { token } : {}), + }), }); if (r.ok) { // 200: the server replaced the row's list (A5). @@ -917,7 +999,7 @@ export async function mount(root) { * 409/422 details are fixed generic strings (credential safety — the * URL is never echoed). */ function wireAddForm(opts) { - const { form, input, btn, error, tokenInput } = opts; + const { form, input, btn, error, tokenInput, branchInput } = opts; if (!form || !input || !btn) return; form.addEventListener("submit", async (e) => { e.preventDefault(); @@ -938,10 +1020,13 @@ export async function mount(root) { // Phase 121: the masked token rides the same POST — blank = // the key is omitted (None = no credential; LOCKED A2). const token = tokenInput ? tokenInput.value.trim() : ""; + // Phase 135: the branch pin rides the same POST — blank = the + // key is omitted (NULL = the repo's default branch). + const branch = branchInput ? branchInput.value.trim() : ""; const r = await fetch("/api/git-sources", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify(opts.body(value, token)), + body: JSON.stringify(opts.body(value, token, branch)), }); if (r.ok) { let createdId = null; @@ -952,6 +1037,7 @@ export async function mount(root) { } input.value = ""; // 201: the source is stored if (tokenInput) tokenInput.value = ""; // the credential is stored (write-only) + if (branchInput) branchInput.value = ""; // the branch is stored (NULL when blank) announce(opts.addedMessage); await loadSources(); // the new row lands in the table focusNewRow(createdId); // a11y: land the caret on the new row @@ -980,11 +1066,14 @@ export async function mount(root) { form: formEl, input: urlInput, tokenInput, + branchInput, btn: addBtn, error: addError, + // Phase 135: the branch pin is included ONLY when non-blank + // (blank = key omitted = the repo's default branch, stored NULL). // Phase 121: the masked token is included ONLY when non-blank // (blank = key omitted = no credential — the API's tri-state). - body: (url, token) => ({ url, ...(token ? { token } : {}) }), + body: (url, token, branch) => ({ url, ...(branch ? { branch } : {}), ...(token ? { token } : {}) }), emptyMessage: "Enter a git URL to add.", failMessage: "Could not add the git source — try again.", networkMessage: "Could not add the git source — is the app reachable?", diff --git a/frontend/assets/styles.css b/frontend/assets/styles.css index fad352e..d6e97ce 100644 --- a/frontend/assets/styles.css +++ b/frontend/assets/styles.css @@ -2788,6 +2788,7 @@ a.answer-image:hover { text-decoration: underline; } /* flat — the caption und credential — the URL input's treatment VERBATIM (mono, >=44px target, brand focus); type=password masks the value in display. */ #git-source-url, +#git-source-branch, #git-source-token { flex: 1; min-width: 14rem; @@ -2801,8 +2802,10 @@ a.answer-image:hover { text-decoration: underline; } /* flat — the caption und padding: 0.45rem 0.7rem; } #git-source-url::placeholder, +#git-source-branch::placeholder, #git-source-token::placeholder { color: var(--ink-soft); } #git-source-url:focus-visible, +#git-source-branch:focus-visible, #git-source-token:focus-visible { outline-offset: 0; border-color: var(--brand); } #git-source-add, #archive-upload-btn { @@ -3226,6 +3229,34 @@ a.answer-image:hover { text-decoration: underline; } /* flat — the caption und border-radius: var(--radius-sm); } +/* Phase 135 (task 04): the editor's branch field — the token field's + box language (full panel width, --bg fill, the line border, mono) + at the 44px touch floor; type=text (a branch is a name, not a + credential — no masking) + autocomplete=off live in the markup, and + the label reuses .ignore-editor-label (the source line's 0.75rem + bottom margin is the field pair's top gap — no extra margin class). + The muted hint line carries the .field-hint values (ink-soft, 400, + 0.8rem — the token field's hint styling) as a standalone

; the + bottom margin is the gap to the copy line (which has no top margin). + Git rows only — git-sources.js hides the group for local rows. */ +#ignore-editor-branch { + display: block; + width: 100%; + min-height: 44px; + padding: 0.55rem 0.65rem; + font-family: var(--mono); + font-size: 0.85rem; + color: var(--ink); + background: var(--bg); + border: 1px solid var(--line); + border-radius: var(--radius-sm); +} +.ignore-editor-branch-hint { + margin: 0.35rem 0 1rem; + color: var(--ink-soft); + font-size: 0.8rem; +} + /* The in-dialog failure line (role=alert): the err pair (err-ink on err-bg 9.3:1, the err-line border) — the .remove-confirm-error language. */ @@ -3450,8 +3481,11 @@ a.answer-image:hover { text-decoration: underline; } /* flat — the caption und .git-source-hidden:disabled { opacity: 0.5; cursor: wait; } /* Phase 105 (A5): the "hidden on" state tag — a copy of the .git-source-ignore-count idiom (TEXT + background, never - color alone — WCAG 1.4.1: --ink on --bg 16.7:1). */ -.git-source-hidden-count { + color alone — WCAG 1.4.1: --ink on --bg 16.7:1). Phase 135 (A6): + the "branch: " state tag joins the same rule — stored git + rows with a branch only, the text IS the signal. */ +.git-source-hidden-count, +.git-source-branch-count { display: inline-block; margin-left: 0.55rem; padding: 0.08rem 0.5rem; @@ -5405,6 +5439,7 @@ a.answer-image:hover { text-decoration: underline; } /* flat — the caption und #git-source-form > label, #archive-upload-form > label { white-space: normal; } #git-source-url, + #git-source-branch, #git-source-token, #archive-upload-file { min-width: 0; } #git-source-add, diff --git a/frontend/index.html b/frontend/index.html index aea5fc2..0684255 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -707,6 +707,22 @@ placeholder="https://github.com/you/your-repo.git" required > + + + + + +

Leave empty for the repo's default branch.

One path per line. A file is ignored when its path in the source starts with any of these — /my/files/, diff --git a/scripts/git_sync.py b/scripts/git_sync.py index a5475a9..8284532 100644 --- a/scripts/git_sync.py +++ b/scripts/git_sync.py @@ -1,9 +1,17 @@ """Git source sync for import_docs (phase 28). -clone_or_pull(url, dest) clones ``url`` into ``dest`` with FULL history -the first time, or — for an existing checkout — unshallows a shallow -one first (``git fetch --unshallow``, the one-time self-heal) and then -fast-forwards it with ``git pull --ff-only``. +clone_or_pull(url, dest, branch=None) clones ``url`` into ``dest`` with +FULL history the first time — ``git clone --branch `` when a branch +is pinned (phase 135: the source's branch is checked out from the +first commit), plain ``git clone`` when not — or, for an existing +checkout, runs the branch guard first when a branch is pinned (a +checkout on any other branch fails loudly — the source must be removed +and re-added to switch, phase 135), unshallows a shallow one first +(``git fetch --unshallow``, the one-time self-heal), and then +fast-forwards it — ``git pull --ff-only`` with no branch, or the +single-fetch ``git fetch origin `` + ``git merge --ff-only +FETCH_HEAD`` of the pinned ref with one (``branch=None`` keeps the +pre-phase argv byte-identical — the unit argv pins are the proof). Auth: nothing special — an ``https://…`` URL uses the OS credential helper / prompts; a ``git@host:repo.git`` URL uses the machine's SSH key. @@ -12,11 +20,16 @@ used. This module is the only place the ``git`` CLI is invoked (A11: stdlib ``subprocess`` only, no new packages) — every git command goes through -:func:`run_git`: the clone, the shallow probe, the one-time -``git fetch --unshallow``, and the ``git pull --ff-only`` in -:func:`clone_or_pull`, the per-file last-commit-date walk in -:func:`file_commit_dates` (phase 106), and the docs-push sequence in -:mod:`app.core.docs_push` (phase 59). +:func:`run_git`: the clone (with ``--branch`` when a branch is +pinned, phase 135), the branch guard's ``git rev-parse --abbrev-ref +HEAD`` probe, the shallow probe, the one-time ``git fetch +--unshallow``, the pinned branch's ``git fetch origin `` + +``git merge --ff-only FETCH_HEAD`` pair, and the ``git pull +--ff-only`` in :func:`clone_or_pull`; the per-file last-commit-date +walk in :func:`file_commit_dates` (phase 106 — it walks the +checkout's own log, so a branch checkout yields the BRANCH's history: +the head-of-branch semantics phase 135 pins, untouched); and the +docs-push sequence in :mod:`app.core.docs_push` (phase 59). Per-file last-commit dates (phase 106 D2; phase 107 D11 supersedes phase 106 D10) — behavior verified against scratch and live repos @@ -25,11 +38,13 @@ phase 106 D10) — behavior verified against scratch and live repos * every :func:`clone_or_pull` checkout is FULL history for EVERY transport (https/ssh/``file://``/local-path): a fresh clone carries no ``--depth`` (D11 — D10's ``--depth 1`` shallow URL clones are -gone), and an existing shallow checkout (made pre-phase 107 — i.e. - every deployed one, live + dev included) is probed with ``git +gone; ``--branch`` clones, phase 135, included), and an existing + shallow checkout (made pre-phase 107 — i.e. every deployed one, + live + dev included) is probed with ``git rev-parse --is-shallow-repository`` and, while shallow, self-healed with the ONE-TIME ``git fetch --unshallow`` before the usual ``git - pull --ff-only`` — no re-clone; + pull --ff-only`` (no branch) / ``git merge --ff-only FETCH_HEAD`` + (pinned branch) — no re-clone; * on a full-history checkout :func:`file_commit_dates` yields the TRUE per-file last-commit date for ALL git sources. The old shallow-clone behavior was the bug the owner reported 2026-09-16: @@ -57,45 +72,96 @@ class GitSyncError(RuntimeError): """A git clone/pull failed (or git is missing); carries git's stderr.""" -def clone_or_pull(url: str, dest: Path | str) -> Path: +def clone_or_pull(url: str, dest: Path | str, branch: str | None = None) -> Path: """Clone ``url`` into ``dest`` (full history, first run) or fast-forward it. - dest without a ``.git`` (or absent) → ``git clone url dest`` (full history for every transport — phase 107 D11: no ``--depth``, so a URL-transport checkout carries the whole commit log, not just the - tip). - - dest with a ``.git`` → probe ``git rev-parse + tip); with a pinned ``branch`` → ``git clone --branch url + dest`` (phase 135: the source's branch is checked out from the + first commit — no post-clone checkout; ``branch=None`` keeps the + two-argument argv byte-identical). + - dest with a ``.git`` and no ``branch`` → probe ``git rev-parse --is-shallow-repository``; while shallow, ``git fetch --unshallow`` (the ONE-TIME self-heal for checkouts made shallow pre-phase 107 — the next sync of a deployed checkout becomes full-history without a re-clone), then ``git pull --ff-only`` (refuses to merge unrelated histories — a broken checkout fails loudly rather than producing a dirty index). + - dest with a ``.git`` and a pinned ``branch`` (phase 135), in + order: the HEAD GUARD first — ``git rev-parse --abbrev-ref + HEAD``; a checkout sitting on any other branch raises the + remove-and-re-add :class:`GitSyncError` (naming the checkout and + BOTH branch names — never the URL, so no credential exposure) + BEFORE any fetch or merge (no silent branch switch: the checkout + may serve other rows' documents, and a switch would silently + re-map the source's file set); a matching checkout then runs the + SAME shallow probe + one-time unshallow self-heal, and pulls the + pinned ref with the single-fetch sequence ``git fetch origin + `` + ``git merge --ff-only FETCH_HEAD`` (a non-ff merge still + fails loudly HERE — the force-push reset fallback is phase 136's + extension of this sequence, not this phase's). Returns the destination path. Raises :class:`GitSyncError` when git is missing or a git invocation exits non-zero (with git's stderr in the message, so the caller can name the failing repo + reason). D12 - fail-loud: a failed probe/unshallow/pull propagates exactly like a - clone failure — never a silent fallback to tip dates or mtimes (a - continued shallow checkout would silently re-serve the uniform tip - date, i.e. the bug D11 fixes). + fail-loud: a failed probe/unshallow/pull/merge propagates exactly + like a clone failure — never a silent fallback to tip dates or + mtimes (a continued shallow checkout would silently re-serve the + uniform tip date, i.e. the bug D11 fixes) and never a silent branch + switch (phase 135 — the fail-loud guard, above). """ dest = Path(dest) if not dest.exists() or not (dest / ".git").exists(): dest.parent.mkdir(parents=True, exist_ok=True) - run_git(["git", "clone", url, str(dest)], cwd=dest.parent) - else: - shallow = ( - run_git(["git", "rev-parse", "--is-shallow-repository"], cwd=dest) - .strip() - == "true" - ) - if shallow: - run_git(["git", "fetch", "--unshallow"], cwd=dest) + argv = ["git", "clone"] + if branch is not None: + argv += ["--branch", branch] + argv += [url, str(dest)] + run_git(argv, cwd=dest.parent) + elif branch is None: + _unshallow_if_shallow(dest) run_git(["git", "pull", "--ff-only"], cwd=dest) + else: + # Phase 135 HEAD guard (first — before the probe, any fetch, + # or any merge): a mismatched checkout fails loudly. The + # message names the checkout and both branch names; it never + # carries the URL (the only place a token could ride). + current = run_git(["git", "rev-parse", "--abbrev-ref", "HEAD"], cwd=dest).strip() + if current != branch: + raise GitSyncError( + f"checkout {dest} is on branch '{current}' but the source " + f"specifies branch '{branch}' — remove and re-add the " + "source to switch branches" + ) + _unshallow_if_shallow(dest) + # The single-fetch ff pull of the pinned ref (phase 136 + # extends this sequence with its reset fallback). + run_git(["git", "fetch", "origin", branch], cwd=dest) + run_git(["git", "merge", "--ff-only", "FETCH_HEAD"], cwd=dest) return dest +def _unshallow_if_shallow(dest: Path) -> None: + """The one-time shallow self-heal (phase 107 D11). + + Probe ``git rev-parse --is-shallow-repository``; while shallow, + ``git fetch --unshallow`` (a pre-phase-107 deployed checkout + becomes full-history on its next sync without a re-clone). BOTH + pull paths of :func:`clone_or_pull` (no branch / pinned branch) + run this before their pull — the argv sequence is the pre-phase + one in both (the unit argv pins are the proof). + """ + shallow = ( + run_git(["git", "rev-parse", "--is-shallow-repository"], cwd=dest) + .strip() + == "true" + ) + if shallow: + run_git(["git", "fetch", "--unshallow"], cwd=dest) + + def _parse_commit_dates(output: str) -> dict[str, datetime]: """Parse ``git log --name-only --format=@@%cI`` output (newest first). diff --git a/scripts/import_docs.py b/scripts/import_docs.py index 7cf8a4d..42ff27d 100644 --- a/scripts/import_docs.py +++ b/scripts/import_docs.py @@ -24,7 +24,14 @@ precedence order: column injected as ``https://x-access-token:@…`` only for https? rows, NULL token → the bare stored URL verbatim (public repos and legacy embedded-token rows clone exactly as before); - the checkout name stays on the bare URL (credential-free); local + the checkout name stays on the bare URL (credential-free). Phase + 135: a row's BRANCH PIN (``branch``) is honored by the clone — a + fresh clone checks it out (``git clone --branch``), an existing + checkout is guarded (a mismatched branch fails loudly — remove and + re-add the source to switch) and tracked (``git fetch origin `` + + ``git merge --ff-only FETCH_HEAD``); rows without a branch (all + pre-phase-135 rows + the env-fallback rows) keep the pre-phase + clone/pull byte-identical; local rows are the existing directories themselves, walked directly. A failing clone/pull — or a local directory that is missing at run time — aborts the whole run *before* anything is imported. @@ -245,7 +252,17 @@ def _resolve_sources( # URL ONLY here (clone_url_for — NULL token → the bare # stored URL verbatim); repo_name stays on the bare URL # so the checkout directory name is credential-free. - root = clone_or_pull(clone_url_for(row), sources_root / repo_name(row.url)) + # Phase 135: the row's branch pin is the third + # argument — NULL (every pre-phase-135 row + every + # env-fallback row) keeps the pre-phase argv + # byte-identical; a pinned branch is checked out on a + # fresh clone (--branch) and guarded + tracked on an + # existing checkout (fetch + ff-merge of that ref). + root = clone_or_pull( + clone_url_for(row), + sources_root / repo_name(row.url), + row.branch, + ) # Phase 106 (D2): the checkout's per-file last-commit # dates, keyed by the SAME root string the importer # sees; local rows contribute nothing (mtime fallback). diff --git a/tests/e2e/test_git_source_branch.py b/tests/e2e/test_git_source_branch.py new file mode 100644 index 0000000..273df62 --- /dev/null +++ b/tests/e2e/test_git_source_branch.py @@ -0,0 +1,753 @@ +"""Phase 135 story E2E (Playwright): a git source pins the branch it +mirrors. + +Source: ``TODO.md:4`` — "Need to be able to specify the branch for a git +source." + +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_git_source_branch.py -v --no-cov + +The branch feature is pinned end to end against the REAL pipeline: a +real ``git clone`` of the ``file://`` fixture repo, a real import +against the mock LLM, a real KB — the owner's exact flow (add the +source WITH a branch through the UI → sync → the branch's content is +indexed, not the default branch's). The ``slow_llm`` proxy is NOT +needed: the branch assertions are about WHICH files land, not about a +running window. + +Per-module app env (the E2E conftest pattern, module-scoped — the +``test_sync_button.py`` + ``test_git_sources_admin.py`` combination): +this module's app boots on its own port (a same-port second uvicorn +dies on bind and would drive the wrong server) with a dedicated +``BOR_SOURCES_DIR`` and NO ``BOR_GIT_SOURCES`` — the sources under +test are DB rows the UI adds, so the env fallback stays EMPTY (pinned +"" — the phase-61 leak guard: an operator's local gitignored ``.env`` +list must not leak into the app under test; process env ranks above +the ``.env`` file). The session app (no git sources) is never started +in this isolated run, so no port clash. + +The fixture (module-scoped, real ``git``): a two-branch repo — +``main`` (``notes/main-only.md`` with ``Marker: RESE-BRANCH-MAIN-7a31`` ++ a shared ``notes/common.md`` saying ``VERSION: 1``) and ``release`` +(``notes/release-only.md`` with ``Marker: RESE-BRANCH-REL-9c2d`` + the +same ``notes/common.md`` saying ``VERSION: 2``) — the branches +DIVERGE: release does not carry ``notes/main-only.md`` (a release +checkout must not hold the default branch's doc). A SECOND module +fixture is a main-only clone of it (identical history) — the +``branch=None`` path's subject. + +Test → contract mapping (four tests): + +1. ``test_add_with_branch_syncs_that_branch`` — THE FEATURE: the + owner adds the fixture WITH branch ``release`` through the UI (201 + → the row's ``branch: release`` tag) → sync succeeds → the catalog + holds ``release-only.md`` + ``common.md`` (the modal's stored + content says ``VERSION: 2``) while ``main-only.md`` is ABSENT (the + default branch's doc was never indexed); the idempotent re-sync + reports "0 added · 2 unchanged". +2. ``test_default_branch_source_unaffected`` — the ``branch=None`` + path against the main-only clone: no branch given → sync → + ``main-only.md`` present, ``release-only.md`` absent, ``common.md`` + holds ``VERSION: 1`` (the pre-phase byte-identical behavior, + observable through the UI; the row carries NO branch tag). +3. ``test_row_editor_branch_change_fails_loud`` — the ASSUMPTION B5 + guard end to end: the per-row editor (prefilled) changes the + branch to ``main`` → the PATCH lands (row re-renders with the + ``branch: main`` tag) → the sync FAILS (the checkout sits on + ``release``) with the fixed mismatch copy naming + ``remove and re-add the source`` in the sanitized status error, + the banner, and the modal; restoring ``release`` makes the sync + succeed again (the guard is symmetric — the ff path works). +4. ``test_api_branch_contracts`` — the API layer through the logged-in + page context's request client (the admin cookie): invalid branch + names (``a..b``, ``HEAD``) → 422 with the detail EXACTLY + ``not a valid branch name`` (no echo; the list unchanged); the + PATCH tri-state (set ``hotfix/x`` → 200 reports it; a PATCH that + does not touch the branch leaves it untouched; ``""`` → 200 + reports ``null``); a branch on a LOCAL row → 422 ``branch is only + valid for git sources``. + +Isolation: per-test TRUNCATE of the KB + registry tables (the +``test_sync_button.py``/``test_git_source_dates.py`` shape, extended +with ``sources_meta``) so each test's sync counts are its own; own +``BOR_SOURCES_DIR``; the checkout dirs survive across tests WITHIN the +module on purpose — test 3's fail-loud mismatch keys on test 1's +checkout (on ``release``) still being on disk. The whole module is +held to a 180 s wall budget (the ``_wall_time_guard`` fixture — the +three-plus full syncs of a 3-file repo against the mock LLM are a +regression canary for the sync pipeline's speed). +""" +from __future__ import annotations + +import os +import re +import subprocess +import sys +import time +from collections.abc import Iterator +from datetime import datetime +from pathlib import Path +from typing import Any + +import pytest +from playwright.sync_api import Locator, Page, expect +from sqlalchemy import text + +from app.db import SessionLocal +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 +# (env-overridable, as in the sibling per-module suites). +APP_PORT = int(os.environ.get("E2E_APP_PORT_GITBR", "8134")) +APP_URL = f"http://127.0.0.1:{APP_PORT}" + +GIT_SOURCES_URL = "/git-sources.html" +SOURCES_URL = "/sources.html" + +#: The fixture repo's directory names — the source names the import +#: records for each (the repo_name basename rule, phase 28). The two +#: names DIFFER, so the two checkouts (and the two catalog sources) +#: never collide. +FIXTURE_NAME = "branch-fixture" +CLONE_NAME = "branch-fixture-main" + +#: "Synced HH:MM" — the local-time last-result label (sources.js's +#: fmtSyncTime), any hour/minute. +SYNCED_LABEL = re.compile(r"Synced \d{1,2}:\d{2}") + +#: The user-visible fragment of the phase-135 mismatch error (the fixed +#: copy — safe to pin): the checkout guard's remove-and-re-add fix. +MISMATCH_FRAGMENT = "remove and re-add the source" + +#: The module's wall budget (task 05 step 2): three-plus full syncs of +#: a 3-file repo against the mock LLM must stay well under this. +WALL_BUDGET_S = 180.0 + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +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 _commit(repo: Path, message: str) -> None: + """A fixture commit (never signed — the house ``commit.gpgsign`` + flag, the test_sync_button.py idiom).""" + _git( + repo, + "-c", "user.email=e@x", "-c", "user.name=t", + "-c", "commit.gpgsign=false", + "commit", "-qm", message, + ) + + +@pytest.fixture(scope="module") +def branch_fixture_repo(tmp_path_factory: pytest.TempPathFactory) -> Path: + """A real TWO-branch git repo the sync must clone over ``file://`` + (task 05 step 1): ``main`` (``notes/main-only.md`` + + ``notes/common.md`` = ``VERSION: 1``), then the DIVERGED + ``release`` branch (drops ``notes/main-only.md``, adds + ``notes/release-only.md``, rewrites ``notes/common.md`` = + ``VERSION: 2``). + + Built under ``tmp_path_factory`` (``tmp_path`` is function-scoped + while the module-scoped app fixture needs the repo for the + module's lifetime) via real ``git`` subprocesses — the + ``test_sync_button.py`` idiom. ``init -b main`` pins the default + branch NAME (the task's fixture shape — the system's + ``init.defaultBranch`` may differ). + """ + root = tmp_path_factory.mktemp("bor_git_br") + repo = root / FIXTURE_NAME + (repo / "notes").mkdir(parents=True) + (repo / "notes" / "main-only.md").write_text( + "# Main only note\n" + "\n" + "A note that exists only on the main branch.\n" + "\n" + "Marker: RESE-BRANCH-MAIN-7a31\n", + encoding="utf-8", + ) + (repo / "notes" / "common.md").write_text( + "# Common note\n\nVERSION: 1\n", encoding="utf-8" + ) + _git(repo, "init", "-q", "-b", "main") + _git(repo, "add", "-A") + _commit(repo, "main: base") + # The release branch DIVERGES from main (the feature's observable + # promise): it drops main-only.md (a release checkout must NOT hold + # the default branch's doc), adds a branch-only file, and rewrites + # the shared common.md (the two VERSIONs). + _git(repo, "checkout", "-q", "-b", "release") + (repo / "notes" / "main-only.md").unlink() + (repo / "notes" / "release-only.md").write_text( + "# Release only note\n" + "\n" + "A note that exists only on the release branch.\n" + "\n" + "Marker: RESE-BRANCH-REL-9c2d\n", + encoding="utf-8", + ) + (repo / "notes" / "common.md").write_text( + "# Common note\n\nVERSION: 2\n", encoding="utf-8" + ) + _git(repo, "add", "-A") + _commit(repo, "release: branch content") + assert (repo / ".git").is_dir() + return repo + + +@pytest.fixture(scope="module") +def branch_main_clone( + branch_fixture_repo: Path, tmp_path_factory: pytest.TempPathFactory +) -> Path: + """A main-only clone of the fixture (identical history) — the + ``branch=None`` path's subject (task 05 test 2): a SECOND + ``file://`` repo whose default branch is ``main``. The clone's + ``origin/release`` ref is remote-tracking only — the app's + no-branch clone/pull never touches it (its one sync is a FRESH + clone of the default branch, the pre-phase byte-identical path).""" + root = tmp_path_factory.mktemp("bor_git_br_main") + clone = root / CLONE_NAME + proc = subprocess.run( + ["git", "clone", "-q", "-b", "main", f"file://{branch_fixture_repo}", str(clone)], + capture_output=True, + text=True, + ) + if proc.returncode != 0: + raise AssertionError(f"git clone of the fixture failed: {proc.stderr.strip()}") + assert (clone / ".git").is_dir() + return clone + + +@pytest.fixture(scope="module") +def app_server(mock_llm: int, branch_fixture_repo: Path) -> Iterator[str]: + """The real app under test — per-module env: NO + ``BOR_GIT_SOURCES`` (pinned EMPTY — the phase-61 leak guard; the + sources under test are DB rows the UI adds, so the env fallback + stays empty) and a dedicated ``BOR_SOURCES_DIR`` under the fixture + root (the checkouts never aim at anything outside the temp area). + The session app is never started in this isolated run.""" + 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). + 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 git sources (task 05 step 1): the rows under test are DB + # rows — the fallback list stays EMPTY (pinned, not absent: an + # operator's local .env list must not leak into the app under test). + env["BOR_GIT_SOURCES"] = "" + env["BOR_SOURCES_DIR"] = str(branch_fixture_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_kb() -> None: + """Fresh KB + registry per test (the E2E isolation pattern): each + test's sync counts are its own. ``git_sources`` too — this suite + adds its rows through the UI, and a leftover row from another suite + would flip ``effective_sources`` to a list this test does not + control. ``sources_meta`` (the KB generation counter) resets with + the KB — the bump is an upsert, so the sync re-creates the row.""" + with SessionLocal() as db: + db.execute(text( + "TRUNCATE chunks, documents, query_log, kb_overview, " + "git_sources, sources_meta" + )) + db.commit() + + +@pytest.fixture(autouse=True) +def _clean_kb(db_ready: None) -> Iterator[None]: + _truncate_kb() + yield + _truncate_kb() + + +@pytest.fixture(scope="module", autouse=True) +def _wall_time_guard() -> Iterator[None]: + """The module's wall-time budget (task 05 step 2): the whole module + — app boot + the full syncs of a 3-file repo against the mock LLM + — stays under 180 s. A breach is a regression canary (the sync + pipeline slowed), not a flake tolerance.""" + start = time.monotonic() + yield + elapsed = time.monotonic() - start + assert elapsed < WALL_BUDGET_S, ( + f"the module took {elapsed:.1f}s — over the {WALL_BUDGET_S:.0f}s wall " + "budget (the small-fixture syncs against the mock LLM are a " + "speed canary)" + ) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _git_source_row(page: Page, url: str) -> Locator: + """The table row whose source cell shows ``url`` (the + ``test_git_sources_admin.py`` _row idiom).""" + return page.locator("#git-sources-tbody tr", has_text=url) + + +def add_git_source( + page: Page, url: str, branch: str | None = None, token: str | None = None +) -> Locator: + """Fill + submit the add form — the branch field when given (blank + → the key is omitted from the POST, NULL = the default branch), + the masked token field when given — and wait for the new row + (the 201 → reload → row lifecycle, §7.4 never-stale).""" + page.fill("#git-source-url", url) + if branch is not None: + page.fill("#git-source-branch", branch) + if token is not None: + page.fill("#git-source-token", token) + page.locator("#git-source-add").click() + row = _git_source_row(page, url) + expect(row).to_have_count(1, timeout=30_000) + return row + + +def _started_at(body: dict[str, Any]) -> float: + """The status body's ``started_at`` as a wall-clock epoch (the + server stamps it in UTC; this process runs on the same host — + same clock, the phase-64 recorder's freshness convention).""" + started = body.get("started_at") + if not started: + return 0.0 + try: + return datetime.fromisoformat(str(started)).timestamp() + except ValueError: + return 0.0 + + +def _close_sync_failure_modal(page: Page) -> None: + """Close the sync failure modal when it is open. + + A Sources-view remount over a stale ``failed`` terminal re-renders + the failure modal (``initSyncButton`` — the never-stale contract: + the page boots to the last run's state), and the fixed-inset + backdrop would intercept the next ``#sync-btn`` click — closing it + is exactly what the owner does after reading the error.""" + if page.locator(".sync-modal-backdrop.is-open").count(): + page.click(".sync-modal-close") + expect(page.locator(".sync-modal-backdrop.is-open")).to_have_count( + 0, timeout=5_000 + ) + + +def sync_and_wait( + page: Page, app_url: str, timeout_s: float = 90.0 +) -> dict[str, Any]: + """Click ``#sync-btn`` and poll the (cookie-authenticated) status + endpoint until THIS run reaches a terminal state — the + ``_wait_sync_done`` pattern with the stale-terminal freshness gate + (the ``test_incremental_folder_summaries.py`` idiom — the module + app is shared, so the PREVIOUS run's terminal lives in the app's + memory until this run supersedes it; a terminal is accepted only + after a running tick was observed or its ``started_at`` + post-dates the click). A re-rendered failure modal (the remount + case above) is closed before the click. Waiting for the + synchronous "Syncing…" label first also pins every LATER UI + terminal assertion (the label / result line / banner) to this run + — a stale "Synced HH:MM" from a prior test cannot satisfy them.""" + _close_sync_failure_modal(page) + r = page.request.get(f"{app_url}/api/sync/status") + assert r.status == 200, r.text + t0 = time.time() + page.click("#sync-btn") + # The click handler adopts the run on 202 (the label flips + # "Syncing…" before the first poll). + expect(page.locator("#sync-label")).to_have_text("Syncing…", timeout=15_000) + saw_running = False + 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() + state = body["state"] + if state == "running": + saw_running = True + elif state in ("success", "failed", "cancelled") and ( + saw_running or _started_at(body) >= t0 - 1.0 + ): + return body + time.sleep(0.25) + raise AssertionError(f"sync did not reach a terminal state: {body}") + + +def _go_top(page: Page) -> None: + """Back to the catalog's top level (no-op when already there — + the breadcrumb is hidden at the top, the test_sync_button.py + idiom).""" + if not page.locator("#kb-crumb").is_hidden(): + page.locator("#kb-crumb a.kb-crumb-link").first.click() + + +def catalog_has_doc(page: Page, source_name: str, doc_name: str) -> Locator: + """Drill the catalog tree (top → *source_name* → ``notes``) and + return the locator for *doc_name*'s file row (the + ``_expect_fixture_row`` pattern, parameterized — the count + assertion is the caller's: 1 = present, 0 = absent).""" + _go_top(page) + source_link = page.locator( + f'#folders-tbody a.folder-link:text-is("{source_name}")' + ) + expect(source_link).to_have_count(1) + source_link.click() + folder_link = page.locator('#folders-tbody a.folder-link:text-is("notes")') + expect(folder_link).to_have_count(1) + folder_link.click() + return page.locator("#docs-tbody tr", has_text=doc_name) + + +def _assert_admin_settled(page: Page) -> None: + """The git-sources view settled as admin (the + ``test_git_sources_admin.py`` _admin_git_sources_page tail).""" + 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() + + +# --------------------------------------------------------------------------- +# 1. THE FEATURE: add WITH a branch → the branch's content is indexed +# --------------------------------------------------------------------------- + + +def test_add_with_branch_syncs_that_branch( + page: Page, app_url: str, db_ready: None, branch_fixture_repo: Path +) -> None: + """The owner's exact flow: the add form WITH the branch field → + 201 → the row's ``branch: release`` tag → the real sync indexes + the BRANCH's tree (``release-only.md`` + ``common.md`` at + ``VERSION: 2``) — never the default branch's (``main-only.md`` + absent) — and the idempotent re-sync reports "0 added · 2 + unchanged".""" + page.set_default_timeout(30_000) + url = f"file://{branch_fixture_repo}" + login(page, app_url, next=GIT_SOURCES_URL) + expect(page).to_have_url(app_url + GIT_SOURCES_URL, timeout=30_000) + _assert_admin_settled(page) + + # --- add through the UI form, WITH the branch (→ 201) -------------- + post_statuses: list[int] = [] + + def track_post(r: Any) -> None: + if r.request.method == "POST" and r.url.endswith("/api/git-sources"): + post_statuses.append(r.status) + + page.on("response", track_post) + try: + add_git_source(page, url, branch="release") + finally: + page.remove_listener("response", track_post) + assert post_statuses == [201], ( + f"the add must be a single 201 POST, saw: {post_statuses}" + ) + + # The row renders with the ``branch: release`` tag in the source + # cell (the "hidden on" tag idiom — text + background)… + row = _git_source_row(page, url) + expect(row).to_have_count(1) + expect(row.locator("td.git-source-url-cell code")).to_have_text(url) + tag = row.locator(".git-source-branch-count") + expect(tag).to_have_count(1) + expect(tag).to_have_text("branch: release") + # …and the 201 cleared the credential-free inputs (never stale). + expect(page.locator("#git-source-url")).to_have_value("") + expect(page.locator("#git-source-branch")).to_have_value("") + + # --- the real sync (switch to the Sources/RAG view) ---------------- + page.goto(app_url + SOURCES_URL) + expect(page.locator("#sync-btn")).to_be_visible(timeout=30_000) + body = sync_and_wait(page, app_url) + assert body["state"] == "success", body + expect(page.locator("#sync-label")).to_have_text(SYNCED_LABEL, timeout=15_000) + # Both branch docs are fresh after the per-test truncate. + expect(page.locator("#sync-result")).to_have_text("2 added") + + # --- the branch's content is what got indexed ---------------------- + expect(catalog_has_doc(page, FIXTURE_NAME, "release-only.md")).to_have_count(1) + expect(catalog_has_doc(page, FIXTURE_NAME, "common.md")).to_have_count(1) + # The default branch's doc was NEVER imported… + expect(page.locator("#docs-tbody tr", has_text="main-only.md")).to_have_count(0) + # …and the shared file holds the RELEASE-side content: drill into + # it and assert the stored content (the document modal — the + # dates suite's content-check idiom, at the UI level). + common_row = page.locator("#docs-tbody tr", has_text="common.md") + expect(common_row).to_have_count(1) + common_row.locator("td:nth-child(2) a.doc-link").click() + expect(page.locator(".doc-modal")).to_be_visible() + expect(page.locator("#doc-modal-content")).to_contain_text("VERSION: 2") + page.keyboard.press("Escape") + expect(page.locator(".doc-modal")).to_be_hidden() + + # --- the idempotent re-sync: the ff pull of the pinned ref -------- + body = sync_and_wait(page, app_url) + assert body["state"] == "success", body + expect(page.locator("#sync-label")).to_have_text(SYNCED_LABEL, timeout=15_000) + # Nothing re-embeds (sha256 delta) — the no-op run announces the + # unchanged count (release's two files). + expect(page.locator("#sync-result")).to_have_text("0 added · 2 unchanged") + # The branch's doc survived the prune re-import. + expect(catalog_has_doc(page, FIXTURE_NAME, "release-only.md")).to_have_count(1) + + +# --------------------------------------------------------------------------- +# 2. The branch=None path: the default branch, exactly as pre-phase +# --------------------------------------------------------------------------- + + +def test_default_branch_source_unaffected( + page: Page, app_url: str, db_ready: None, branch_main_clone: Path +) -> None: + """A SECOND source (a main-only clone of the fixture — a second + ``file://`` repo, identical history, ``main`` only) added with NO + branch: the ``branch=None`` path mirrors the pre-phase behavior — + the DEFAULT branch's tree is indexed (``main-only.md`` present, + ``release-only.md`` absent, ``common.md`` at ``VERSION: 1``), and + the row carries NO branch tag.""" + page.set_default_timeout(30_000) + url = f"file://{branch_main_clone}" + login(page, app_url, next=GIT_SOURCES_URL) + expect(page).to_have_url(app_url + GIT_SOURCES_URL, timeout=30_000) + _assert_admin_settled(page) + + # Add with NO branch: the blank field omits the key (stored NULL). + add_git_source(page, url) + row = _git_source_row(page, url) + expect(row).to_have_count(1) + expect(row.locator(".git-source-branch-count")).to_have_count(0) + + page.goto(app_url + SOURCES_URL) + expect(page.locator("#sync-btn")).to_be_visible(timeout=30_000) + body = sync_and_wait(page, app_url) + assert body["state"] == "success", body + expect(page.locator("#sync-label")).to_have_text(SYNCED_LABEL, timeout=15_000) + expect(page.locator("#sync-result")).to_have_text("2 added") + + # The default branch (main) is what landed — the pre-phase + # byte-identical guarantee, observable through the UI… + expect(catalog_has_doc(page, CLONE_NAME, "main-only.md")).to_have_count(1) + expect(page.locator("#docs-tbody tr", has_text="release-only.md")).to_have_count(0) + # …and the shared file holds MAIN's content (VERSION: 1 — the + # branch the owner did NOT name). + common_row = page.locator("#docs-tbody tr", has_text="common.md") + expect(common_row).to_have_count(1) + common_row.locator("td:nth-child(2) a.doc-link").click() + expect(page.locator(".doc-modal")).to_be_visible() + expect(page.locator("#doc-modal-content")).to_contain_text("VERSION: 1") + page.keyboard.press("Escape") + expect(page.locator(".doc-modal")).to_be_hidden() + + +# --------------------------------------------------------------------------- +# 3. The fail-loud mismatch (ASSUMPTION B5, end to end) +# --------------------------------------------------------------------------- + + +def test_row_editor_branch_change_fails_loud( + page: Page, app_url: str, db_ready: None, branch_fixture_repo: Path +) -> None: + """The per-row editor changes the branch to one the checkout is + NOT on: the PATCH lands (the row re-renders with the new tag), + the sync FAILS — the checkout guard names the remove-and-re-add + fix in the sanitized status error, the banner, and the modal — + and restoring the matching branch makes the sync succeed again + (the guard is symmetric: back to ``release``, the ff path works). + The fixture state is explicit: the autouse truncate re-creates the + row fresh, while test 1's checkout (on ``release``) is what the + guard runs against (the checkout dirs outlive the per-test + truncate — on purpose).""" + page.set_default_timeout(30_000) + url = f"file://{branch_fixture_repo}" + login(page, app_url, next=GIT_SOURCES_URL) + expect(page).to_have_url(app_url + GIT_SOURCES_URL, timeout=30_000) + _assert_admin_settled(page) + + # Re-add the release source fresh (the autouse truncate emptied the + # registry; the KB is empty too — nothing is indexed yet). + add_git_source(page, url, branch="release") + expect( + _git_source_row(page, url).locator(".git-source-branch-count") + ).to_have_text("branch: release") + + # --- the per-row editor: prefilled, change to main, save ---------- + _git_source_row(page, url).locator(".git-source-ignore").click() + dialog = page.locator("#ignore-editor-dialog") + expect(dialog).to_be_visible() + # Git rows open the branch field VISIBLE, prefilled from the stored + # value (unlike the token — there IS a field to read it from). + expect(page.locator("#ignore-editor-branch")).to_have_value("release") + page.fill("#ignore-editor-branch", "main") + page.locator("#ignore-editor-save").click() + expect(dialog).to_be_hidden(timeout=30_000) + # The PATCH landed — the row re-renders from the server with the + # changed tag (the registry is updated; the sync is what guards). + expect( + _git_source_row(page, url).locator(".git-source-branch-count") + ).to_have_text("branch: main", timeout=15_000) + + # --- sync → FAILED: the checkout is on release, the row says main - + page.goto(app_url + SOURCES_URL) + expect(page.locator("#sync-btn")).to_be_visible(timeout=30_000) + body = sync_and_wait(page, app_url) + assert body["state"] == "failed", body + # The status error carries the fixed mismatch copy (sanitized — + # never the URL, never a credential)… + assert body["error"] is not None and MISMATCH_FRAGMENT in body["error"], body + # …and the UI surfaces it in the sanitized error banner AND the + # failure modal (the owner is never left with a silently-wrong + # checkout — the fail-loud promise, ASSUMPTION B5). + expect(page.locator("#sync-error-banner")).to_be_visible(timeout=15_000) + expect(page.locator("#sync-error-text")).to_contain_text(MISMATCH_FRAGMENT) + expect(page.locator(".sync-modal-backdrop")).to_be_visible(timeout=15_000) + expect(page.locator("#sync-modal-error")).to_contain_text(MISMATCH_FRAGMENT) + # The owner's next step: close the modal (its backdrop intercepts + # clicks until it does). + page.locator(".sync-modal-close").click() + expect(page.locator(".sync-modal-backdrop.is-open")).to_have_count(0) + + # --- restore: back to the matching branch → the ff path works ----- + page.goto(app_url + GIT_SOURCES_URL) + expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000) + row = _git_source_row(page, url) + expect(row).to_have_count(1) + row.locator(".git-source-ignore").click() + expect(page.locator("#ignore-editor-dialog")).to_be_visible(timeout=30_000) + # The editor now prefills the (wrong-branch) stored value… + expect(page.locator("#ignore-editor-branch")).to_have_value("main") + page.fill("#ignore-editor-branch", "release") + page.locator("#ignore-editor-save").click() + expect(page.locator("#ignore-editor-dialog")).to_be_hidden(timeout=30_000) + expect( + _git_source_row(page, url).locator(".git-source-branch-count") + ).to_have_text("branch: release", timeout=15_000) + + page.goto(app_url + SOURCES_URL) + body = sync_and_wait(page, app_url) + assert body["state"] == "success", body + expect(page.locator("#sync-label")).to_have_text(SYNCED_LABEL, timeout=15_000) + # The KB is still empty (the failed sync aborted before the + # import) — the restored branch's two docs are fresh additions. + expect(page.locator("#sync-result")).to_have_text("2 added") + # And the branch's doc is what is indexed again. + expect(catalog_has_doc(page, FIXTURE_NAME, "release-only.md")).to_have_count(1) + + +# --------------------------------------------------------------------------- +# 4. The API contracts (the tri-state, the fixed 422s, the local-row +# rejection) through the logged-in page context's request client +# --------------------------------------------------------------------------- + + +def test_api_branch_contracts( + page: Page, app_url: str, db_ready: None, tmp_path: Path +) -> None: + """The branch's API layer, admin-cookie only (the real API login — + no form, no page): invalid branch names 422 with the detail + EXACTLY ``not a valid branch name`` (fixed — no echo; the list + unchanged), the PATCH tri-state round-trips (set → untouched → + clear), and a branch on a LOCAL row is the fixed 422. The token + field is untouched by all of the above (the phase-121 tests stay + green — no regression pin needed here).""" + base = f"{app_url}/api/git-sources" + r = page.request.post(f"{app_url}/api/login", data={"password": ADMIN_PASSWORD}) + assert r.status == 204, r.text + + # --- POST with an invalid branch name → the EXACT fixed 422 ------ + for bad in ("a..b", "HEAD"): + r = page.request.post( + base, + data={"url": f"https://example.com/reese/branch-{bad}.git", "branch": bad}, + ) + assert r.status == 422, r.text + # The detail is the FIXED string — never a substring of the + # input (the credential-safety discipline, applied to names). + assert r.json()["detail"] == "not a valid branch name" + # …and no row was created (the list is unchanged). + r = page.request.get(base) + assert r.status == 200, r.text + assert r.json()["sources"] == [] + + # --- the PATCH tri-state on a stored row --------------------------- + r = page.request.post( + base, data={"url": "https://example.com/reese/branch-patch.git"} + ) + assert r.status == 201, r.text + body = r.json() + assert body["branch"] is None # absent at create → NULL (default branch) + row_id = body["id"] + + # Set: non-empty → 200 reports it (re-validated). + r = page.request.patch(f"{base}/{row_id}", data={"branch": "hotfix/x"}) + assert r.status == 200, r.text + assert r.json()["branch"] == "hotfix/x" + # A PATCH that does not touch the branch leaves it untouched. + r = page.request.patch(f"{base}/{row_id}", data={"include_hidden": True}) + assert r.status == 200, r.text + assert r.json()["branch"] == "hotfix/x" + assert r.json()["include_hidden"] is True + # Clear: "" → 200 reports null (the tri-state's clear leg). + r = page.request.patch(f"{base}/{row_id}", data={"branch": ""}) + assert r.status == 200, r.text + assert r.json()["branch"] is None + + # --- a branch on a LOCAL row → the fixed 422 ---------------------- + local_dir = tmp_path / "local-src" + local_dir.mkdir() + r = page.request.post( + base, data={"kind": "local", "path": str(local_dir), "branch": "x"} + ) + assert r.status == 422, r.text + assert r.json()["detail"] == "branch is only valid for git sources" diff --git a/tests/e2e/test_git_source_tokens.py b/tests/e2e/test_git_source_tokens.py index a9cb539..2a105bb 100644 --- a/tests/e2e/test_git_source_tokens.py +++ b/tests/e2e/test_git_source_tokens.py @@ -371,10 +371,14 @@ def test_admin_editor_token_opens_blank_and_save_keeps_token( # One PATCH, 200, and the body OMITS the token key (blank = the # tri-state no-change — the key is not sent, let alone empty). + # Phase 135 (task 04): a GIT row's PATCH body ALWAYS includes the + # branch key (the tri-state asymmetry — a blank save is the + # deliberate CLEAR leg: "" here; the seeded row stores no branch, + # so the clear is a no-op and the stored token is untouched). assert patch_statuses == [200], f"the blank-token save did not 200: {patch_statuses}" assert len(patch_bodies) == 1 and patch_bodies[0] is not None assert "token" not in patch_bodies[0], "a blank token must OMIT the PATCH key" - assert patch_bodies[0] == {"ignore_paths": []} + assert patch_bodies[0] == {"ignore_paths": [], "branch": ""} # The row is intact and the stored token is KEPT (the no-change # tri-state, verified in the trusted store). diff --git a/tests/integration/test_git_sources_api.py b/tests/integration/test_git_sources_api.py index bcbfbb8..e8195ee 100644 --- a/tests/integration/test_git_sources_api.py +++ b/tests/integration/test_git_sources_api.py @@ -58,6 +58,22 @@ injected ``https://x-access-token:@…`` URL with a credential-free checkout path (mock ``clone_or_pull``) and a legacy row's clone still receives its ORIGINAL stored URL (the credential keeps working). + +Phase 135 (task 01 — branch storage + reporting): the output shapes +gain ``branch`` (NULL = the repo's default branch) — list DB rows +report the stored value, env-fallback rows report ``null`` (no DB row +to store one on), and POST 201 / PATCH 200 report the stored value. + +Phase 135 (task 02 — branch validation + editing): POST stores a +branch on GIT rows only (absent/empty → NULL = the default branch; +non-empty → :func:`app.rag.git_sources.validate_branch_name`, a +``ValueError`` → the FIXED-detail 422 ``not a valid branch name`` — +the input is never echoed, and the row is not created); a ``branch`` +on a LOCAL row is 422 ``branch is only valid for git sources`` (POST +and PATCH alike — local rows are walked, not cloned); PATCH edits +the branch with the token's EXACT tri-state — absent/None = no +change, non-empty = set (re-validated), empty string = clear (stored +NULL). """ from __future__ import annotations @@ -161,10 +177,11 @@ def test_get_empty_table_with_env_returns_env_rows( "url": "https://a.example.com/one.git", "path": None, "added_at": None, - # Env rows have no DB row to store a list on (phase 89) or - # a flag on (phase 105). + # Env rows have no DB row to store a list on (phase 89), a + # flag on (phase 105) or a branch on (phase 135). "ignore_paths": [], "include_hidden": False, + "branch": None, }, { "id": None, @@ -174,6 +191,7 @@ def test_get_empty_table_with_env_returns_env_rows( "added_at": None, "ignore_paths": [], "include_hidden": False, + "branch": None, }, ] @@ -241,10 +259,11 @@ def test_post_creates_trimmed_and_list_stops_using_env( assert body["added_at"] is not None # Phase 89: the response gains ``ignore_paths`` — absent at create # time → ``[]``; phase 105 adds ``include_hidden`` — absent → - # ``False`` (A4). - assert set(body) == {"id", "url", "added_at", "ignore_paths", "include_hidden"} + # ``False`` (A4); phase 135 adds ``branch`` — absent → ``null``. + assert set(body) == {"id", "url", "added_at", "ignore_paths", "include_hidden", "branch"} assert body["ignore_paths"] == [] assert body["include_hidden"] is False + assert body["branch"] is None # The DB row now wins: from_env False, the env URL is gone from the list. body = admin_client.get("/api/git-sources").json() @@ -258,6 +277,10 @@ def test_post_accepts_accepted_url_shapes(admin_client: TestClient) -> None: "http://git.local/repo.git", "ssh://git@example.com/repo.git", "git@github.com:owner/repo.git", + # Phase 135 (task 05): the file:// local transport — the E2E + # fixture repos the UI adds end to end (the phase-28/32/107 + # transport-true idiom, promoted from the env fallback). + "file:///srv/repos/homelab-notes", ): r = admin_client.post("/api/git-sources", json={"url": url}) assert r.status_code == 201, f"{url} must be accepted: {r.text}" @@ -356,10 +379,12 @@ def test_post_local_creates_stored_row_with_expanded_path( # The phase-35 response shape is unchanged — the local row reports # its (expanded) path in ``url``; ``kind`` + ``path`` via GET; # phase 89 adds ``ignore_paths`` (absent → ``[]``); phase 105 adds - # ``include_hidden`` (absent → ``False``, A4). - assert set(body) == {"id", "url", "added_at", "ignore_paths", "include_hidden"} + # ``include_hidden`` (absent → ``False``, A4); phase 135 adds + # ``branch`` (absent → ``null``). + assert set(body) == {"id", "url", "added_at", "ignore_paths", "include_hidden", "branch"} assert body["ignore_paths"] == [] assert body["include_hidden"] is False + assert body["branch"] is None uuid.UUID(body["id"]) assert body["url"] == str(real_dir) assert body["added_at"] is not None @@ -592,7 +617,8 @@ def test_delete_removes_row_and_falls_back_to_env( "path": None, "added_at": None, "ignore_paths": [], # env rows: no DB row to store a list on - "include_hidden": False, # … or a flag on (phase 105) + "include_hidden": False, # … a flag on (phase 105) + "branch": None, # … or a branch on (phase 135) } ] db.execute(text("TRUNCATE chunks, documents")) @@ -709,6 +735,7 @@ def test_get_reports_stored_ignore_paths_and_env_rows_empty( "added_at": None, "ignore_paths": [], "include_hidden": False, + "branch": None, # env rows: no DB row to store a branch on (135) } ] @@ -728,9 +755,10 @@ def test_patch_replaces_ignore_paths_including_clear(admin_client: TestClient) - r = admin_client.patch(f"/api/git-sources/{before['id']}", json={"ignore_paths": ["a/", "b"]}) assert r.status_code == 200, r.text body = r.json() - assert set(body) == {"id", "url", "added_at", "ignore_paths", "include_hidden"} + assert set(body) == {"id", "url", "added_at", "ignore_paths", "include_hidden", "branch"} assert body["ignore_paths"] == ["a", "b"] # normalized assert body["include_hidden"] is False # untouched by a list-only PATCH + assert body["branch"] is None # untouched by a list-only PATCH for key in ("id", "url", "added_at"): assert body[key] == before[key] # Round-trip through GET. @@ -988,7 +1016,8 @@ def test_post_with_token_stores_column_and_never_echoes( assert r.status_code == 201, r.text body = r.json() assert body["url"] == "https://github.com/owner/private.git" - assert set(body) == {"id", "url", "added_at", "ignore_paths", "include_hidden"} + assert set(body) == {"id", "url", "added_at", "ignore_paths", "include_hidden", "branch"} + assert body["branch"] is None # no branch sent → the default-branch NULL assert TOKEN not in r.text row = db.scalars(select(GitSource)).one() @@ -1329,7 +1358,8 @@ def _stub_sync_stack( clone_calls: list[tuple[str, Path]] = [] - def fake_clone_or_pull(url: str, dest: Path | str) -> Path: + def fake_clone_or_pull(url: str, dest: Path | str, branch: str | None = None) -> Path: + # phase 135: the row's branch pin (None for this file's rows) dest = Path(dest) dest.mkdir(parents=True, exist_ok=True) clone_calls.append((url, dest)) @@ -1421,3 +1451,229 @@ def test_sync_public_row_clones_with_verbatim_url( assert _poll_sync(sync_admin_client, "success")["error"] is None assert clone_calls == [(url, tmp_path / "sources" / "public")] + + +# --- branch: create + validation (phase 135, task 02) ---------------------- + + +def test_post_git_source_with_branch_stores_and_reports( + admin_client: TestClient, db: Session +) -> None: + """POST a git source with ``branch: "release"`` → 201 reports the + branch; the stored model row carries it; the GET round-trip + reports it too (201, list DB row).""" + r = admin_client.post( + "/api/git-sources", + json={"url": "https://example.com/branched.git", "branch": "release"}, + ) + assert r.status_code == 201, r.text + body = r.json() + assert body["branch"] == "release" + assert set(body) == {"id", "url", "added_at", "ignore_paths", "include_hidden", "branch"} + + row = db.scalars(select(GitSource)).one() + assert row.branch == "release" # the column, not response-only + + listed = admin_client.get("/api/git-sources").json()["sources"][0] + assert listed["branch"] == "release" + + +def test_post_git_source_branch_is_trimmed(admin_client: TestClient) -> None: + """Surrounding whitespace on the branch is trimmed (the schema's + ``_trim_branch`` + the API's defensive strip) before storage.""" + r = admin_client.post( + "/api/git-sources", + json={"url": "https://example.com/trim.git", "branch": " release/1.2\t"}, + ) + assert r.status_code == 201, r.text + assert r.json()["branch"] == "release/1.2" + + +def test_post_git_source_empty_branch_stores_null(admin_client: TestClient) -> None: + """Absent/None AND trimmed-EMPTY branch at create → stored NULL + (the repo's default branch — the pre-phase-135 behavior): the + create-time split from PATCH's clear leg (GitSourceIn docstring).""" + for i, branch_value in enumerate((None, "", " ")): + payload: dict[str, object] = { + "url": f"https://example.com/def-{i}.git" + } # distinct URLs: the loop must not 409 on itself + if branch_value is not None: + payload["branch"] = branch_value + r = admin_client.post("/api/git-sources", json=payload) + assert r.status_code == 201, r.text + assert r.json()["branch"] is None + + +def test_post_git_source_invalid_branch_422_fixed_detail_no_row( + admin_client: TestClient, db: Session +) -> None: + """Invalid branch names are 422 with the EXACT fixed detail + ``not a valid branch name`` — the input is never echoed (the + credential-safety discipline, applied for consistency) — and no + row is created.""" + for bad in ("a..b", "a b", "a@b", "-x", ".x", "/x", "x/", "x.", "HEAD", "head"): + r = admin_client.post( + "/api/git-sources", + json={"url": "https://example.com/bad-branch.git", "branch": bad}, + ) + assert r.status_code == 422, f"{bad!r} must be rejected: {r.text}" + assert r.json()["detail"] == "not a valid branch name" # EXACT + assert bad not in r.text, f"422 must not echo the input ({bad!r})" + # Nothing stored. + assert db.execute(text("SELECT count(*) FROM git_sources")).scalar_one() == 0 + + +def test_post_git_source_oversized_branch_422(admin_client: TestClient, db: Session) -> None: + """>200 chars: the schema's ``max_length=200`` rejects it before + the handler (FastAPI's standard schema 422 — the same house + precedent as the oversized-URL 422, which this router also + answers with the framework's standard shape); no row is created. + The API-layer branch 422s (charset/sequence/leading/trailing/ + HEAD) are the FIXED-detail ones — see the adjacent tests.""" + r = admin_client.post( + "/api/git-sources", + json={"url": "https://example.com/long-branch.git", "branch": "a" * 201}, + ) + assert r.status_code == 422 + assert db.execute(text("SELECT count(*) FROM git_sources")).scalar_one() == 0 + + +def test_post_local_source_with_branch_422( + admin_client: TestClient, tmp_path: Path, db: Session +) -> None: + """A ``branch`` on a LOCAL row is 422 with the fixed detail (local + rows are walked, not cloned) — for BOTH valid and invalid branch + names (the kind check precedes validation); nothing is stored.""" + real_dir = tmp_path / "local-branch" + real_dir.mkdir() + for bad in ("main", "a..b"): + r = admin_client.post( + "/api/git-sources", + json={"kind": "local", "path": str(real_dir), "branch": bad}, + ) + assert r.status_code == 422, f"{bad!r} must be rejected: {r.text}" + assert r.json()["detail"] == "branch is only valid for git sources" + assert db.execute(text("SELECT count(*) FROM git_sources")).scalar_one() == 0 + + +# --- branch: PATCH tri-state (phase 135, task 02) --------------------------- + + +def test_patch_branch_set_change_clear_and_noop( + admin_client: TestClient, db: Session +) -> None: + """The branch tri-state on a git row — the token's exact + semantics field-for-field: + + * set: ``"hotfix/x"`` → 200 reports it (stored + GET round-trip); + * change: ``"hotfix/y"`` → 200 reports the new value; + * clear: ``""`` → 200 reports ``null`` (stored NULL = the + default branch); whitespace-only trims to empty → the same + clear leg; + * no-op: branch ABSENT (or explicit None) → 200, the stored + branch survives (a flag-only PATCH must not touch it). + """ + created = admin_client.post( + "/api/git-sources", json={"url": "https://example.com/patch-branch.git"} + ) + assert created.status_code == 201 + sid = created.json()["id"] + assert created.json()["branch"] is None # started on the default branch + + def stored_branch() -> str | None: + # The endpoint commits in its own session — expire this one's + # identity map before reading back (the house pattern). + db.expire_all() + row = db.get(GitSource, uuid.UUID(sid)) + assert row is not None + return row.branch + + # SET — "hotfix/x". + r = admin_client.patch(f"/api/git-sources/{sid}", json={"branch": "hotfix/x"}) + assert r.status_code == 200, r.text + assert r.json()["branch"] == "hotfix/x" + assert stored_branch() == "hotfix/x" + assert ( + admin_client.get("/api/git-sources").json()["sources"][0]["branch"] == "hotfix/x" + ) + + # CHANGE — "hotfix/y". + r = admin_client.patch(f"/api/git-sources/{sid}", json={"branch": "hotfix/y"}) + assert r.status_code == 200, r.text + assert r.json()["branch"] == "hotfix/y" + assert stored_branch() == "hotfix/y" + + # NO-OP — an unrelated field only: the branch is UNTOUCHED. + r = admin_client.patch(f"/api/git-sources/{sid}", json={"include_hidden": True}) + assert r.status_code == 200, r.text + assert r.json()["branch"] == "hotfix/y" + assert r.json()["include_hidden"] is True + assert stored_branch() == "hotfix/y" + + # NO-OP — explicit None is absent too. + r = admin_client.patch(f"/api/git-sources/{sid}", json={"branch": None}) + assert r.status_code == 200, r.text + assert r.json()["branch"] == "hotfix/y" + assert stored_branch() == "hotfix/y" + + # CLEAR — "" (the tri-state's clear leg; a deliberate empty save). + r = admin_client.patch(f"/api/git-sources/{sid}", json={"branch": ""}) + assert r.status_code == 200, r.text + assert r.json()["branch"] is None + assert stored_branch() is None + + # Whitespace-only trims to empty → the same clear leg. + admin_client.patch(f"/api/git-sources/{sid}", json={"branch": "hotfix/x"}) + r = admin_client.patch(f"/api/git-sources/{sid}", json={"branch": " "}) + assert r.status_code == 200, r.text + assert r.json()["branch"] is None + assert stored_branch() is None + + +def test_patch_branch_invalid_422_leaves_row_unchanged( + admin_client: TestClient, db: Session +) -> None: + """A PRESENT invalid branch 422s with the EXACT fixed detail — + and the stored branch is untouched (validation raises before the + assignment; the row's other fields in the same body never + half-apply).""" + created = admin_client.post( + "/api/git-sources", + json={"url": "https://example.com/patch-bad.git", "branch": "stable"}, + ) + assert created.status_code == 201 + sid = created.json()["id"] + + for bad in ("a..b", "HEAD", "x/"): + r = admin_client.patch( + f"/api/git-sources/{sid}", json={"branch": bad, "include_hidden": True} + ) + assert r.status_code == 422, f"{bad!r}: {r.text}" + assert r.json()["detail"] == "not a valid branch name" + assert bad not in r.text + # Nothing half-applied: the branch AND the flag are still the + # originals. + row = admin_client.get("/api/git-sources").json()["sources"][0] + assert row["branch"] == "stable" + assert row["include_hidden"] is False + + +def test_patch_branch_on_local_row_422(admin_client: TestClient, tmp_path: Path) -> None: + """A PRESENT branch on a LOCAL row is the same fixed 422 as POST + (local rows are walked, not cloned) — valid name or not; the row + is untouched.""" + real_dir = tmp_path / "local-patch" + real_dir.mkdir() + created = admin_client.post( + "/api/git-sources", json={"kind": "local", "path": str(real_dir)} + ) + assert created.status_code == 201 + sid = created.json()["id"] + + for branch_value in ("main", "a..b"): + r = admin_client.patch(f"/api/git-sources/{sid}", json={"branch": branch_value}) + assert r.status_code == 422, f"{branch_value!r}: {r.text}" + assert r.json()["detail"] == "branch is only valid for git sources" + row = admin_client.get("/api/git-sources").json()["sources"][0] + assert row["branch"] is None + assert row["kind"] == "local" diff --git a/tests/integration/test_import_docs_git.py b/tests/integration/test_import_docs_git.py index e3d75bf..6869f05 100644 --- a/tests/integration/test_import_docs_git.py +++ b/tests/integration/test_import_docs_git.py @@ -23,10 +23,22 @@ is asserted against the canned value). stderr, and **zero** import attempts. - A missing local directory → the same pre-import fail-loud: exit code 1, ``local source missing: `` on stderr, zero import attempts. + +Phase 135 (the branch pin) adds the REAL-git section at the bottom: +a two-branch ``file://`` fixture repo (``main`` with file A, ``release`` +with file B) drives the real :func:`scripts.git_sync.clone_or_pull` — +the fresh ``--branch`` checkout, the ff-merge pull of a new commit on +the pinned branch, the fail-loud branch MISMATCH (checkout untouched), +and the ``_resolve_sources`` seam with a ``branch='release'`` row +(importing file B, not file A) + an env-origin row without a branch +(the default branch, byte-identical pre-phase behavior). Skipped, not +failed, on a machine without the git CLI (the house pattern). """ from __future__ import annotations +import os import re +import subprocess from collections.abc import Iterator from datetime import datetime from pathlib import Path @@ -39,7 +51,7 @@ from app.config import Settings from app.models import GitSource from app.rag.importer import ImportSummary from scripts import import_docs -from scripts.git_sync import GitSyncError +from scripts.git_sync import GitSyncError, clone_or_pull from tests.fakes import FakeEmbedder @@ -106,10 +118,15 @@ class FakeImportSources: def _fake_clone_factory() -> tuple[list[tuple[str, Path]], object]: - """A ``clone_or_pull`` that materialises a checkout with one .md file.""" + """A ``clone_or_pull`` that materialises a checkout with one .md + file. Phase 135: accepts the row's branch pin (``None`` for every + row in this file's fixtures — the real-git branch behavior is + pinned by the phase-135 section below, which uses the REAL + ``clone_or_pull``).""" calls: list[tuple[str, Path]] = [] - def fake_clone_or_pull(url: str, dest: Path | str) -> Path: + def fake_clone_or_pull(url: str, dest: Path | str, branch: str | None = None) -> Path: + assert branch is None # every row in this file's fixtures pins no branch dest = Path(dest) dest.mkdir(parents=True, exist_ok=True) (dest / "notes.md").write_text(f"# {dest.name}\ncontent for the KB\n", encoding="utf-8") @@ -687,7 +704,7 @@ def test_main_git_failure_aborts_before_import( lambda db: ([_git_row("https://host/a/bad.git")], "env"), ) - def failing_clone(url: str, dest: Path | str) -> Path: + def failing_clone(url: str, dest: Path | str, branch: str | None = None) -> Path: raise GitSyncError( f"git clone {url} failed (exit 128): " "fatal: repository not found" @@ -732,3 +749,201 @@ def test_main_missing_local_dir_aborts_before_import( assert "import_docs: source sync failed" in err assert f"local source missing: {missing}" in err # the path is named assert fake_import.calls == [] # no partial import + + +# --- phase 135: the branch pin (real git, two-branch file:// origin) ------- + + +def _git_available() -> bool: + try: + proc = subprocess.run(["git", "--version"], capture_output=True, check=False) + return proc.returncode == 0 + except (FileNotFoundError, OSError): + return False + + +#: Real ``git`` in the test environment — skipped cleanly without it +#: (the ``test_git_file_dates.py`` house pattern). +GIT = _git_available() + + +@pytest.fixture() +def two_branch_repo(tmp_path: Path) -> Path: + """A real-git scratch origin (the phase-135 recipe): ``main`` holds + ``a.md``, ``release`` holds ``b.md`` — different content per branch + (a branch checkout is the branch's tree, not main's).""" + repo = tmp_path / "origin" + repo.mkdir() + _git(repo, "init", "-q", "-b", "main") + _git(repo, "config", "commit.gpgsign", "false") + (repo / "a.md").write_text("# A\nmain branch content\n", encoding="utf-8") + _git(repo, "add", "a.md") + _git(repo, "commit", "-q", "-m", "main: add a.md") + _git(repo, "checkout", "-q", "-b", "release") + (repo / "a.md").unlink() + (repo / "b.md").write_text("# B\nrelease branch content v1\n", encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "release: add b.md") + _git(repo, "checkout", "-q", "main") + return repo + + +def _git(cwd: Path, *argv: str) -> None: + """Run one git command for the test harness (fixture setup); a + non-zero exit fails the fixture, not the test under test.""" + env = os.environ.copy() + env["GIT_AUTHOR_NAME"] = "T" + env["GIT_AUTHOR_EMAIL"] = "t@example.com" + env["GIT_COMMITTER_NAME"] = "T" + env["GIT_COMMITTER_EMAIL"] = "t@example.com" + proc = subprocess.run( + ["git", *argv], cwd=cwd, env=env, capture_output=True, text=True, check=False + ) + assert proc.returncode == 0, f"git {' '.join(argv)} failed: {proc.stderr}" + + +def _git_out(cwd: Path, *argv: str) -> str: + """One harness git read (a non-zero exit fails the test).""" + proc = subprocess.run( + ["git", *argv], cwd=cwd, capture_output=True, text=True, check=False + ) + assert proc.returncode == 0, f"git {' '.join(argv)} failed: {proc.stderr}" + return proc.stdout + + +@pytest.mark.skipif(not GIT, reason="git CLI not available") +def test_clone_or_pull_fresh_branch_checkout_is_on_that_branch( + tmp_path: Path, two_branch_repo: Path +) -> None: + """Phase 135: a FRESH ``clone_or_pull(url, dest, 'release')`` + (real git, ``file://``) checks out the pinned branch — HEAD is + ``release``, file B present, file A (main's tree) absent.""" + url = f"file://{two_branch_repo}" + dest = tmp_path / "bor" / "origin" + + result = clone_or_pull(url, dest, "release") + + assert result == dest + assert _git_out(dest, "rev-parse", "--abbrev-ref", "HEAD").strip() == "release" + assert (dest / "b.md").is_file() + assert (dest / "b.md").read_text(encoding="utf-8") == "# B\nrelease branch content v1\n" + assert not (dest / "a.md").exists() + + +@pytest.mark.skipif(not GIT, reason="git CLI not available") +def test_clone_or_pull_existing_branch_checkout_ff_merges_new_commits( + tmp_path: Path, two_branch_repo: Path +) -> None: + """Phase 135: a SECOND ``clone_or_pull`` of the same pinned branch + after the origin gains a commit on it → the guarded checkout is + ff-merged onto the new tip (file B updated, no error, still on + ``release``).""" + url = f"file://{two_branch_repo}" + dest = tmp_path / "bor" / "origin" + + clone_or_pull(url, dest, "release") + # the origin gains a commit on release (v2 of file B) + _git(two_branch_repo, "checkout", "-q", "release") + (two_branch_repo / "b.md").write_text("# B\nrelease branch content v2\n", encoding="utf-8") + _git(two_branch_repo, "add", "b.md") + _git(two_branch_repo, "commit", "-q", "-m", "release: bump b.md") + _git(two_branch_repo, "checkout", "-q", "main") + + clone_or_pull(url, dest, "release") + + assert _git_out(dest, "rev-parse", "--abbrev-ref", "HEAD").strip() == "release" + assert (dest / "b.md").read_text(encoding="utf-8") == "# B\nrelease branch content v2\n" + assert not (dest / "a.md").exists() + + +@pytest.mark.skipif(not GIT, reason="git CLI not available") +def test_clone_or_pull_branch_mismatch_fails_loud_checkout_untouched( + tmp_path: Path, two_branch_repo: Path +) -> None: + """Phase 135 (ASSUMPTION B5): a checkout on the DEFAULT branch + (fresh clone with no branch → ``main``) + a source row pinning + ``release`` → the remove-and-re-add ``GitSyncError`` — and the + checkout is UNTOUCHED (still on main, still main's tree: no silent + branch switch).""" + url = f"file://{two_branch_repo}" + dest = tmp_path / "bor" / "origin" + + clone_or_pull(url, dest) # no branch → the origin's default (main) + assert _git_out(dest, "rev-parse", "--abbrev-ref", "HEAD").strip() == "main" + assert (dest / "a.md").is_file() + + with pytest.raises(GitSyncError) as excinfo: + clone_or_pull(url, dest, "release") + + msg = str(excinfo.value) + assert "remove and re-add the source to switch branches" in msg + assert "'main'" in msg and "'release'" in msg + assert url not in msg # the guard message never carries the URL + # the checkout is untouched: still on main, still main's tree + assert _git_out(dest, "rev-parse", "--abbrev-ref", "HEAD").strip() == "main" + assert (dest / "a.md").is_file() + assert not (dest / "b.md").exists() + + +@pytest.mark.skipif(not GIT, reason="git CLI not available") +def test_resolve_sources_git_row_with_branch_checks_out_that_branch( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, two_branch_repo: Path +) -> None: + """Phase 135, the CLI seam: a ``git_sources`` row carrying + ``branch='release'`` → ``_resolve_sources`` passes ``row.branch`` to + the REAL ``clone_or_pull`` (no fake — the branch is honored + end to end): the resolved root is the RELEASE tree (file B imported, + file A absent), and the date walk over the real checkout lists ONLY + the branch's tree (head-of-branch semantics, ``file_commit_dates`` + untouched).""" + url = f"file://{two_branch_repo}" + monkeypatch.setattr( + import_docs, + "effective_sources", + lambda db: ([GitSource(url=url, kind="git", branch="release")], "db"), + ) + settings = _settings(sources_dir=str(tmp_path / "bor")) + + sources, ignore_map, hidden_map, date_map = import_docs._resolve_sources(None, settings) + + root = tmp_path / "bor" / "origin" + assert sources == [root] + assert ignore_map == {} + assert hidden_map == {str(root): False} + assert (root / "b.md").is_file() # the branch's file is what gets imported + assert not (root / "a.md").exists() + # the (real, untouched) date walk over the branch checkout yields + # TRUE dates for the branch's files (not the fail-soft ``{}`` — the + # checkout is a real full-history repo). The walk reads the + # reachable LOG (``git log --name-only``), so it also lists files + # from shared ancestor commits (``a.md`` — release was cut after + # main's first commit); the importer only consults the map for the + # files it actually finds in the working tree, so that is harmless. + dates = date_map[str(root)] + assert "b.md" in dates + assert isinstance(dates["b.md"], datetime) + + +@pytest.mark.skipif(not GIT, reason="git CLI not available") +def test_resolve_sources_row_without_branch_clones_default_branch( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, two_branch_repo: Path +) -> None: + """Phase 135, the byte-identical leg: a row WITHOUT a branch (the + env-fallback synthetic rows' shape — ``GitSource(url=…, kind="git")`` + carries the model default ``branch=None``) → the real + ``clone_or_pull`` takes the pre-phase path: the origin's DEFAULT + branch (``main``) is checked out — file A present, file B absent.""" + url = f"file://{two_branch_repo}" + monkeypatch.setattr( + import_docs, + "effective_sources", + lambda db: ([_git_row(url)], "env"), # branch defaults to None + ) + settings = _settings(git_sources=url, sources_dir=str(tmp_path / "bor")) + + sources, *_ = import_docs._resolve_sources(None, settings) + + root = tmp_path / "bor" / "origin" + assert sources == [root] + assert (root / "a.md").is_file() # main's file — the default branch + assert not (root / "b.md").exists() diff --git a/tests/integration/test_migration_0023.py b/tests/integration/test_migration_0023.py index 9e6caed..acd8ff8 100644 --- a/tests/integration/test_migration_0023.py +++ b/tests/integration/test_migration_0023.py @@ -319,10 +319,8 @@ def test_upgrade_round_trip_restores_the_column(db: Session, alembic: Config) -> with the full contract (TEXT, NULLABLE, no server default) — a clean upgrade → downgrade → upgrade round-trip (A13).""" command.downgrade(alembic, "0022") - command.upgrade(alembic, "head") - assert _version(db) == "0023", ( - "round-trip upgrade must land at head (0023)" - ) + command.upgrade(alembic, "0023") + assert _version(db) == "0023", "round-trip upgrade must land at 0023" context = _column(db, "image_context") assert context is not None, "documents.image_context must be back" diff --git a/tests/integration/test_migration_0024.py b/tests/integration/test_migration_0024.py new file mode 100644 index 0000000..4449015 --- /dev/null +++ b/tests/integration/test_migration_0024.py @@ -0,0 +1,315 @@ +"""Integration: migration 0024 (git_sources.branch) schema contract +(phase 135, task 01). + +Drives the **real Alembic engine** against the live dev database +(``podman compose up -d db``), mirroring the house pattern of +``test_migration_0021.py`` (information_schema assertions on the state +the migration must leave). The tests target the 0023 → 0024 step +explicitly so later migrations cannot break the pins: + +* upgrade 0023 → 0024 → ``branch`` exists with the full contract — + TEXT, NULLABLE, no server default — while the 0023 ``git_sources`` + schema (``url`` NOT NULL + the unique index, ``kind``, ``path``, + ``ignore_paths``, ``include_hidden``, ``token``, ``added_at``) + survives; +* rows inserted while the DB is at 0023 backfill ``branch`` to NULL — + ALL of them (the byte-identical guarantee: a pre-phase-135 row is a + default-branch source by definition; no backfill, no default); +* the ORM contract agrees: a freshly inserted ``GitSource`` with an + explicit ``branch`` round-trips it through a fresh session, and a + row without one reads ``branch is None``; +* downgrade to 0023 → the column is GONE (A13) while the row + its + ``url`` survive; upgrade back to 0024 → the column is back + (round-trip). + +The ``alembic`` fixture guarantees the DB ends at head even if a test +fails or the process is interrupted. +""" +from __future__ import annotations + +import uuid +from collections.abc import Iterator +from typing import Any + +import pytest +from alembic.config import Config +from sqlalchemy import text +from sqlalchemy.orm import Session + +from alembic import command +from app.db import SessionLocal, db_available +from app.models import GitSource + +URL_A = "https://github.com/mig0024/repo-a.git" +URL_B = "https://github.com/mig0024/repo-b.git" +BRANCH = "release/1.0" + + +@pytest.fixture() +def alembic(db: Session) -> Iterator[Config]: + """Real Alembic config bound to the dev DB (URL from app settings). + + Starts at head (repairs an interrupted earlier run); teardown + upgrades to head no matter what happened, so the dev DB is never + left below head. + """ + if not db_available(): + pytest.skip("Postgres not reachable — run `podman compose up -d db` first") + cfg = Config() # no alembic.ini file — env.py gets the URL from app config + cfg.set_main_option("script_location", "alembic") + command.upgrade(cfg, "head") + try: + yield cfg + finally: + # Release the test session's open transaction BEFORE the repair + # DDL: an idle-in-transaction SELECT holds an ACCESS SHARE lock + # on ``git_sources``, which would deadlock the repair's + # ``ALTER TABLE`` (0024) forever. + db.rollback() + command.upgrade(cfg, "head") + + +def _version(db: Session) -> str | None: + return db.execute(text("SELECT version_num FROM alembic_version")).scalar() + + +def _column(db: Session, column: str) -> tuple[Any, ...] | None: + """(data_type, is_nullable, column_default) for one git_sources + column.""" + row = db.execute( + text( + "SELECT data_type, is_nullable, column_default" + " FROM information_schema.columns" + " WHERE table_name = 'git_sources' AND column_name = :c" + ), + {"c": column}, + ).fetchone() + return tuple(row) if row is not None else None + + +def _insert_sql(db: Session, url: str, token: str | None = None) -> uuid.UUID: + """Insert one git_sources row with the PRE-0024 column set (the + 0023 shape — the branch column, when present, is omitted so a NULL + backfill is what the row reads).""" + row_id = uuid.uuid4() + db.execute( + text( + "INSERT INTO git_sources (id, url, kind, path, ignore_paths," + " include_hidden, token)" + " VALUES (:id, :u, 'git', NULL, '[]', false, :t)" + ), + {"id": row_id, "u": url, "t": token}, + ) + db.commit() + return row_id + + +def _delete(db: Session, row_id: uuid.UUID) -> None: + db.execute(text("DELETE FROM git_sources WHERE id = :id"), {"id": row_id}) + db.commit() + + +def test_upgrade_to_0024_adds_branch(db: Session, alembic: Config) -> None: + """Upgrade 0023 → 0024: ``branch`` exists with the full contract + (TEXT, NULLABLE, no server default — NULL = the repo's default + branch), is ABSENT at 0023, and every pre-0024 row (WITH and + WITHOUT a stored token) backfills ``branch`` to NULL — the + byte-identical guarantee, no backfill and no default — while the + 0023 table contract (``url`` + unique index, ``kind``, + ``ignore_paths``, ``include_hidden``, ``token``, ``added_at``) + survives.""" + command.downgrade(alembic, "0023") # start from the pre-0024 state + assert _version(db) == "0023" + assert _column(db, "branch") is None, "branch must be absent at 0023" + + pre_a = _insert_sql(db, URL_A) # the 0023 column set + pre_b = _insert_sql(db, URL_B, token="ghp_mig0024secret") + try: + command.upgrade(alembic, "0024") + assert _version(db) == "0024", "alembic_version must be at 0024" + + branch = _column(db, "branch") + assert branch is not None, "git_sources.branch is missing" + assert branch[0] == "text", "branch must be TEXT" + assert branch[1] == "YES", "branch must be NULLABLE" + assert branch[2] is None, ( + "branch must carry NO server default — NULL is the" + " default-branch value" + ) + + # Every pre-0024 row backfilled to NULL (the repo's default + # branch — the byte-identical guarantee for ALL existing rows). + for pre_id, url in ((pre_a, URL_A), (pre_b, URL_B)): + row = db.execute( + text("SELECT url, token, branch FROM git_sources WHERE id = :id"), + {"id": pre_id}, + ).fetchone() + assert row is not None and row[0] == url, ( + "the pre-0024 row must survive the upgrade" + ) + assert row[2] is None, ( + "every pre-0024 row must backfill branch to NULL — no" + " backfill, no default" + ) + # The tokened row kept its credential (the upgrade is + # branch-only). + assert db.execute( + text("SELECT token FROM git_sources WHERE id = :id"), + {"id": pre_b}, + ).scalar() == "ghp_mig0024secret", ( + "a pre-0024 stored token must survive the branch upgrade" + ) + + # A row written without the column stays NULL (no default to + # fill it in — the Python-side default is None, same value). + new_id = _insert_sql(db, URL_A.replace("repo-a", "repo-c")) + try: + omitted = db.execute( + text("SELECT branch FROM git_sources WHERE id = :id"), + {"id": new_id}, + ).scalar() + assert omitted is None, "an omitted branch must stay NULL" + finally: + _delete(db, new_id) + + # The 0023 schema survives the additive upgrade. + url = _column(db, "url") + assert url is not None and url[0] == "text" and url[1] == "NO", ( + "git_sources.url (0006) must keep its 0023 contract" + ) + kind = _column(db, "kind") + assert kind is not None and kind[0] == "text" and kind[1] == "NO", ( + "git_sources.kind (0007) must survive the upgrade" + ) + ignore = _column(db, "ignore_paths") + assert ignore is not None and ignore[0] == "jsonb" and ignore[1] == "NO", ( + "git_sources.ignore_paths (0013) must survive the upgrade" + ) + hidden = _column(db, "include_hidden") + assert hidden is not None and hidden[0] == "boolean" and hidden[1] == "NO", ( + "git_sources.include_hidden (0019) must survive the upgrade" + ) + token = _column(db, "token") + assert token is not None and token[0] == "text" and token[1] == "YES", ( + "git_sources.token (0021) must survive the upgrade" + ) + added = _column(db, "added_at") + assert added is not None and added[0] == "timestamp with time zone", ( + "git_sources.added_at (0006) must survive the upgrade" + ) + assert added[1] == "NO" and "now()" in str(added[2]), ( + "git_sources.added_at must keep its `now()` server default" + ) + index = db.execute( + text( + "SELECT indexname FROM pg_indexes" + " WHERE tablename = 'git_sources'" + " AND indexname = 'uq_git_sources_url'" + ) + ).scalar() + assert index is not None, ( + "the uq_git_sources_url unique index must survive the upgrade" + ) + finally: + _delete(db, pre_a) + _delete(db, pre_b) + + +def test_orm_branch_round_trips(db: Session, alembic: Config) -> None: + """The ORM contract agrees with the column contract: a freshly + inserted ``GitSource`` with an explicit ``branch`` round-trips the + value through a FRESH session, and a row without one reads + ``branch is None`` (the NULL default-branch state).""" + command.upgrade(alembic, "head") + row_branched = GitSource(url=URL_A, kind="git", branch=BRANCH) + row_default = GitSource(url=URL_B, kind="git") + db.add(row_branched) + db.add(row_default) + db.commit() + try: + with SessionLocal() as fresh: + reloaded_branched = fresh.get(GitSource, row_branched.id) + assert reloaded_branched is not None, "the branched row must be readable" + assert reloaded_branched.branch == BRANCH, ( + "the explicit branch must round-trip through the DB" + ) + reloaded_default = fresh.get(GitSource, row_default.id) + assert reloaded_default is not None, "the default row must be readable" + assert reloaded_default.branch is None, ( + "a row without a branch must read branch is None" + ) + finally: + _delete(db, row_branched.id) + _delete(db, row_default.id) + + +def test_downgrade_to_0023_drops_the_column(db: Session, alembic: Config) -> None: + """Downgrade 0024 → 0023: the branch column is gone (A13 — fully + reversible) while the row + its ``url`` (and ``token``) survive, + and the rest of the 0023 table contract (``url`` unique index, + ``kind``, ``added_at``) is intact.""" + command.upgrade(alembic, "head") + row = GitSource(url=URL_A, kind="git", branch=BRANCH, token="ghp_mig0024secret") + db.add(row) + db.commit() + try: + command.downgrade(alembic, "0023") + assert _version(db) == "0023" + assert _column(db, "branch") is None, "branch must be dropped" + + surviving = db.execute( + text( + "SELECT url, kind, ignore_paths, include_hidden, token, added_at" + " FROM git_sources WHERE id = :id" + ), + {"id": row.id}, + ).fetchone() + assert surviving is not None and surviving[0] == URL_A, ( + "the row must survive the column drop" + ) + assert surviving[1] == "git" and surviving[2] == [] and surviving[3] is False, ( + "kind + ignore_paths + include_hidden must survive the drop" + ) + assert surviving[4] == "ghp_mig0024secret", "token must survive the drop" + assert surviving[5] is not None, "added_at must survive the drop" + + index = db.execute( + text( + "SELECT indexname FROM pg_indexes" + " WHERE tablename = 'git_sources'" + " AND indexname = 'uq_git_sources_url'" + ) + ).scalar() + assert index is not None, "the unique index must survive the downgrade" + finally: + _delete(db, row.id) + # Repair: the fixture teardown re-upgrades to head. + + +def test_upgrade_round_trip_restores_the_column(db: Session, alembic: Config) -> None: + """Downgrade to 0023, then upgrade back to 0024: ``branch`` is back + with the full contract (TEXT, NULLABLE, no server default), and a + row seeded at 0023 reads ``branch is None`` (the byte-identical + guarantee on the round-trip).""" + command.downgrade(alembic, "0023") + pre_id = _insert_sql(db, URL_B) + try: + command.upgrade(alembic, "0024") + assert _version(db) == "0024", "round-trip upgrade must land at 0024" + + branch = _column(db, "branch") + assert branch is not None, "git_sources.branch must be back" + assert branch[0] == "text", "branch must be TEXT after the round-trip" + assert branch[1] == "YES", "branch must be NULLABLE after the round-trip" + assert branch[2] is None, ( + "branch must still carry NO server default after the round-trip" + ) + + assert db.execute( + text("SELECT branch FROM git_sources WHERE id = :id"), + {"id": pre_id}, + ).scalar() is None, ( + "a row seeded at 0023 must read branch NULL after the round-trip" + ) + finally: + _delete(db, pre_id) diff --git a/tests/integration/test_sync_api.py b/tests/integration/test_sync_api.py index a61c4b6..65098a2 100644 --- a/tests/integration/test_sync_api.py +++ b/tests/integration/test_sync_api.py @@ -91,6 +91,8 @@ from __future__ import annotations import asyncio import logging +import os +import subprocess import threading import time from collections.abc import Callable, Iterator, Sequence @@ -113,7 +115,7 @@ from app.rag.folder_summaries import affected_folder_keys from app.rag.importer import ImportSummary from app.rag.llm import EmbeddingError, LLMClient, ModelUnavailableError from app.rag.sources_meta import current_sources_version -from scripts.git_sync import GitSyncError +from scripts.git_sync import GitSyncError, clone_or_pull from tests.conftest import ADMIN_PASSWORD from tests.fakes import FakeEmbedder @@ -414,10 +416,12 @@ class FakeFolderSummaries: def _fake_clone() -> tuple[list[tuple[str, Path]], object]: - """A ``clone_or_pull`` that materialises a checkout with one .md file.""" + """A ``clone_or_pull`` that materialises a checkout with one .md + file. Phase 135: accepts the row's branch pin (the third argument + — ``None`` for every row this file seeds).""" calls: list[tuple[str, Path]] = [] - def fake_clone_or_pull(url: str, dest: Path | str) -> Path: + def fake_clone_or_pull(url: str, dest: Path | str, branch: str | None = None) -> Path: dest = Path(dest) dest.mkdir(parents=True, exist_ok=True) (dest / "notes.md").write_text(f"# {dest.name}\ncontent for the KB\n", encoding="utf-8") @@ -1193,7 +1197,7 @@ def test_git_failure_marks_failed_and_skips_import( lambda: _settings(sources_dir=str(tmp_path / "bor")), ) - def failing_clone(url: str, dest: Path | str) -> Path: + def failing_clone(url: str, dest: Path | str, branch: str | None = None) -> Path: raise GitSyncError( f"git clone {url} failed (exit 128): " "fatal: repository not found" @@ -1993,3 +1997,142 @@ def test_cancel_mid_run_lands_in_cancelled_then_resyncs_clean( assert current_sources_version(db) == 1 assert len(second_import.sources) == 1 assert second_import.prune_flags == [True] + + +# --- phase 135: the branch pin (real git, two-branch file:// origin) ------- + + +def _git_available() -> bool: + try: + proc = subprocess.run(["git", "--version"], capture_output=True, check=False) + return proc.returncode == 0 + except (FileNotFoundError, OSError): + return False + + +#: Real ``git`` in the test environment — skipped cleanly without it +#: (the ``test_git_file_dates.py`` house pattern). +GIT = _git_available() + + +def _git(cwd: Path, *argv: str) -> None: + """Run one git command for the test harness (fixture setup); a + non-zero exit fails the fixture, not the test under test.""" + env = os.environ.copy() + env["GIT_AUTHOR_NAME"] = "T" + env["GIT_AUTHOR_EMAIL"] = "t@example.com" + env["GIT_COMMITTER_NAME"] = "T" + env["GIT_COMMITTER_EMAIL"] = "t@example.com" + proc = subprocess.run( + ["git", *argv], cwd=cwd, env=env, capture_output=True, text=True, check=False + ) + assert proc.returncode == 0, f"git {' '.join(argv)} failed: {proc.stderr}" + + +def _two_branch_origin(tmp_path: Path) -> Path: + """A real-git scratch origin (the phase-135 recipe): ``main`` holds + ``a.md``, ``release`` holds ``b.md`` — different content per + branch (a branch checkout is the branch's tree, not main's).""" + origin = tmp_path / "origin" + origin.mkdir() + _git(origin, "init", "-q", "-b", "main") + _git(origin, "config", "commit.gpgsign", "false") + (origin / "a.md").write_text("# A\nmain branch content\n", encoding="utf-8") + _git(origin, "add", "a.md") + _git(origin, "commit", "-q", "-m", "main: add a.md") + _git(origin, "checkout", "-q", "-b", "release") + (origin / "a.md").unlink() + (origin / "b.md").write_text("# B\nrelease branch content\n", encoding="utf-8") + _git(origin, "add", "-A") + _git(origin, "commit", "-q", "-m", "release: add b.md") + _git(origin, "checkout", "-q", "main") + return origin + + +@pytest.mark.skipif(not GIT, reason="git CLI not available") +def test_git_row_with_branch_button_path_clones_that_branch( + sync_client: TestClient, + monkeypatch: pytest.MonkeyPatch, + db: Session, + tmp_path: Path, +) -> None: + """Phase 135: the BUTTON path honors the row's branch pin — a + ``git_sources`` row with ``branch='release'`` is cloned with the + REAL ``clone_or_pull`` (no fake in this test): the imported root + is the RELEASE tree (file B present, file A absent).""" + origin = _two_branch_origin(tmp_path) + db.add(GitSource(url=f"file://{origin}", kind="git", branch="release")) + db.commit() + _stub_env(monkeypatch) + monkeypatch.setattr( + sync_api, + "get_settings", + lambda: _settings(sources_dir=str(tmp_path / "bor")), + ) + _stub_probe(monkeypatch) + fake_import = FakeImportSources(ImportSummary(files=1, added=1)) + monkeypatch.setattr(sync_api, "import_sources", fake_import) + monkeypatch.setattr(sync_api, "regenerate_overview", FakeOverview(ok=True)) + monkeypatch.setattr(sync_api, "generate_folder_summaries", FakeFolderSummaries()) + + _login(sync_client) + assert sync_client.post("/api/sync").status_code == 202 + body = _poll(sync_client, "success") + assert body["error"] is None + + root = tmp_path / "bor" / "origin" + assert fake_import.sources == [[root]] + assert (root / "b.md").is_file() # the branch's file is what got imported + assert not (root / "a.md").exists() + + +@pytest.mark.skipif(not GIT, reason="git CLI not available") +def test_git_row_branch_mismatch_button_path_fails_loud( + sync_client: TestClient, + monkeypatch: pytest.MonkeyPatch, + db: Session, + tmp_path: Path, +) -> None: + """Phase 135 (ASSUMPTION B5): a row pinning a branch the EXISTING + checkout does not sit on fails the button sync LOUDLY — the + remove-and-re-add message, zero import attempts, and the checkout + untouched (still on its branch: no silent switch).""" + origin = _two_branch_origin(tmp_path) + url = f"file://{origin}" + dest = tmp_path / "bor" / "origin" + # A pre-existing checkout on the default branch (main) — e.g. a + # row that existed before the branch was pinned (the sync's + # sources_dir is seeded with it before the run starts). + clone_or_pull(url, dest) + db.add(GitSource(url=url, kind="git", branch="release")) + db.commit() + _stub_env(monkeypatch) + monkeypatch.setattr( + sync_api, + "get_settings", + lambda: _settings(sources_dir=str(tmp_path / "bor")), + ) + _stub_probe(monkeypatch) + fake_import = FakeImportSources(ImportSummary()) + monkeypatch.setattr(sync_api, "import_sources", fake_import) + monkeypatch.setattr(sync_api, "regenerate_overview", FakeOverview(ok=True)) + monkeypatch.setattr(sync_api, "generate_folder_summaries", FakeFolderSummaries()) + + _login(sync_client) + assert sync_client.post("/api/sync").status_code == 202 + + body = _poll(sync_client, "failed") + assert "remove and re-add the source to switch branches" in body["error"] + assert "'main'" in body["error"] and "'release'" in body["error"] + assert body["detail"] == {} + assert fake_import.sources == [] # no partial import + # Phase 53: a FAILED sync never bumps. + assert current_sources_version(db) == 0 + # the checkout is untouched: still on main, still main's tree + proc = subprocess.run( + ["git", "rev-parse", "--abbrev-ref", "HEAD"], + cwd=dest, capture_output=True, text=True, check=False, + ) + assert proc.returncode == 0 and proc.stdout.strip() == "main" + assert (dest / "a.md").is_file() + assert not (dest / "b.md").exists() diff --git a/tests/unit/test_git_source_branch_ui.py b/tests/unit/test_git_source_branch_ui.py new file mode 100644 index 0000000..7bb06d0 --- /dev/null +++ b/tests/unit/test_git_source_branch_ui.py @@ -0,0 +1,383 @@ +"""Unit: the branch-pin UI on the Sources page (phase 135, task 04). + +``TODO.md`` L4: "Need to be able to specify the branch for a git +source." The Sources manager lets the owner see, set, change, and +clear a source's branch (ASSUMPTION B6): + +* the ADD form gains the optional ``#git-source-branch`` (maxlength + 200, autocomplete=off, the visible "optional — default: the repo's + default branch" hint) BETWEEN the URL and Token fields; the submit + body is ``(url, token, branch) => ({ url, ...(branch ? { branch } : + {}), ...(token ? { token } : {}) })`` — a BLANK branch OMITS the key + (NULL = the repo's default branch — the pre-phase-135 clone/pull + behavior), and 201 clears all three credential-free inputs; +* the per-row EDITOR dialog gains ``#ignore-editor-branch`` (after the + source line, before the ignore textarea, with the "Leave empty for + the repo's default branch." hint line) — VISIBLE FOR GIT ROWS ONLY + (local rows hide label + input + hint and the save body OMITS the + key — local rows are walked, not cloned; the API's 422 is the + backstop), opens PREFILLED from the stored value (``s.branch ?? ""`` + — unlike the token, the API reports it, so there IS a field to read + it from); +* THE TRI-STATE ASYMMETRY: the git row's PATCH body ALWAYS includes + ``branch`` (the trimmed value — an EMPTY save is the deliberate + CLEAR leg: NULL = the repo's default branch), unlike the token's + blank = keep (the token stays the conditional key — untouched); +* every STORED git row with a branch shows a ``branch: `` text + tag in the source cell (the "hidden on" tag idiom — text + + background, never color alone: the text IS the signal); + env-fallback rows (no ``s.id``) and NULL-branch rows show no tag; + the source cell's title names the branch (hover precision). + +The browser behavior itself is E2E-gated by the phase-135 story suite +(``tests/e2e/test_git_source_branch.py``, task 05); like the phase-121 +``test_git_source_token.py`` house pattern, this module pins the +source-level contract a silent regression would break. +""" +from __future__ import annotations + +import re +from pathlib import Path + +from app.schemas import GitSourceIn, GitSourceOut, GitSourcePatchIn, GitSourceRow + +FRONTEND = Path(__file__).resolve().parents[2] / "frontend" +SHELL_HTML = FRONTEND / "index.html" +GIT_SOURCES_JS = FRONTEND / "assets" / "git-sources.js" +STYLES_CSS = FRONTEND / "assets" / "styles.css" + +ADD_BRANCH_PLACEHOLDER = "main, release/1.2, …" +EDIT_BRANCH_PLACEHOLDER = "(default branch)" +ADD_HINT_TEXT = "optional — default: the repo's default branch" +EDITOR_HINT_TEXT = "Leave empty for the repo's default branch." +ADD_BODY_EXPR = ( + "(url, token, branch) => ({ url, ...(branch ? { branch } : {}), " + "...(token ? { token } : {}) })" +) +BRANCH_TAG_CLASS = "git-source-branch-count" +BRANCH_TAG_TEXT_EXPR = "`branch: ${s.branch}`" +TITLE_EXPR = 's.id && s.branch ? `${value} — branch: ${s.branch}` : value' + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _js_text() -> str: + return _read(GIT_SOURCES_JS) + + +def _fn(js: str, name: str) -> str: + """The source of a (possibly async) top-level function via + balanced-brace counting (the test_git_source_token.py helper).""" + for prefix in ("async function ", "function "): + start = js.find(f"{prefix}{name}(") + if start != -1: + depth = 0 + for i in range(js.find("{", start), len(js)): + if js[i] == "{": + depth += 1 + elif js[i] == "}": + depth -= 1 + if depth == 0: + return js[start : i + 1] + raise AssertionError(f"unbalanced braces in {name}()") + raise AssertionError(f"{name}() must exist in git-sources.js") + + +def _input_tag(html: str, id_attr: str) -> str: + m = re.search(rf"]*id=\"{id_attr}\"[^>]*>", html, re.S) + assert m, f"missing in the shell" + return m.group(0) + + +# --------------------------------------------------------------------------- +# The add form — #git-source-branch (between URL and Token) +# --------------------------------------------------------------------------- + + +def test_add_form_branch_field_is_optional_text() -> None: + """``#git-source-branch`` (task 04 step 1): ``type="text"`` (a + branch is a name, not a credential — no masking), the 200-char cap + mirrored (the schema's max_length), ``autocomplete="off"``, the + placeholder, INSIDE ``#git-source-form`` BETWEEN the URL input and + the Token input (before the submit button), and a visible + ``", html, re.S) + assert label, "a visible ", html, re.S) + assert label and "Branch" in label.group(1), "a visible label (WCAG)" + assert 'class="ignore-editor-label"' in (label.group(0) if label else ""), ( + "the house dialog-label styling" + ) + hint = re.search( + r"

(.*?)

", html, re.S + ) + assert hint and hint.group(1).strip() == EDITOR_HINT_TEXT, "the muted hint line" + + +def test_editor_opens_prefilled_for_git_hidden_for_local() -> None: + """openIgnoreEditor: git rows open the branch field prefilled from + the stored value (``s.branch ?? ""`` — there IS a field to read it + from, unlike the token) and VISIBLE; local rows HIDE the whole + field group (label + input + hint — local rows are walked, not + cloned) and clear the value.""" + js = _js_text() + open_body = _fn(js, "openIgnoreEditor") + assert "const branchIsGit = s.kind !== \"local\";" in open_body, ( + "the git-row gate" + ) + prefill_i = open_body.find('ignoreBranchInput.value = branchIsGit ? (s.branch ?? "") : "";') + assert prefill_i != -1, "prefilled from the stored value (s.branch ?? \"\")" + for frag in ( + "ignoreBranchLabel.hidden = !branchIsGit", + "ignoreBranchHint.hidden = !branchIsGit", + "ignoreBranchInput.hidden = !branchIsGit", + ): + assert frag in open_body, f"the field group hides for local rows: {frag!r}" + label_i = open_body.find("ignoreBranchLabel.hidden = !branchIsGit") + hint_i = open_body.find("ignoreBranchHint.hidden = !branchIsGit") + assert -1 < label_i < hint_i < prefill_i, "the group hides before the prefill" + + +def test_editor_save_always_sends_branch_for_git_rows() -> None: + """saveIgnorePaths: the branch is read (trimmed) before the PATCH; + the git row's body ALWAYS includes ``branch`` — + ``...(isGitRow ? { branch } : {})`` — an EMPTY value is the + deliberate CLEAR leg (the tri-state asymmetry, documented at the + read site), while a LOCAL row's body OMITS the key; the token + stays the conditional key (blank = keep — untouched) and lands + AFTER the branch key.""" + js = _js_text() + save = _fn(js, "saveIgnorePaths") + read_i = save.find('const branch = ignoreBranchInput ? ignoreBranchInput.value.trim() : "";') + assert read_i != -1, "the branch is read (trimmed) before the PATCH" + assert "const isGitRow = t.kind !== \"local\";" in save, "the git-row gate" + # The asymmetry is documented at the read site (the docstring + # comment above the read names the clear leg). + comment_i = save.find("the tri-state ASYMMETRY", 0, read_i) + assert -1 < comment_i < read_i, "the tri-state asymmetry is documented at the read" + body_i = save.find("...(isGitRow ? { branch } : {})") + tok_i = save.find("...(token ? { token } : {})", body_i) + assert -1 < read_i < body_i < tok_i, ( + "git rows always send branch (\"\" = clear); the token stays conditional" + ) + assert "ignore_paths: lines" in save[body_i - 60 : body_i + 60], ( + "the ignore_paths replace is untouched" + ) + + +def test_editor_close_resets_the_branch_field() -> None: + """closeIgnoreEditor: the branch input resets to "" (it re-opens + prefilled from the server row — the phase-121 token reset + precedent, with a prefilled twist).""" + js = _js_text() + close_body = _fn(js, "closeIgnoreEditor") + assert 'ignoreBranchInput.value = ""' in close_body, "resets on close" + + +# --------------------------------------------------------------------------- +# makeRow — the "branch: " tag + the hover title +# --------------------------------------------------------------------------- + + +def test_make_row_branch_tag_is_stored_git_rows_only() -> None: + """makeRow: the ``branch: `` text tag is appended to the + source cell IFF ``s.id && s.branch`` (env-fallback rows carry no + id, NULL-branch rows no truthy branch — no tag either way); the + tag uses the "hidden on" idiom (text + background — the + .git-source-branch-count class, grouped with + .git-source-hidden-count in styles.css) and the TEXT is the signal + (``branch: `` — never color alone).""" + js = _js_text() + make = _fn(js, "makeRow") + guard_i = make.find("if (s.id && s.branch) {") + assert guard_i != -1, "stored git rows with a branch only" + block = make[guard_i :] + block = block[: block.find("\n }")] + assert f"branchTag.className = \"{BRANCH_TAG_CLASS}\";" in block + assert f"branchTag.textContent = {BRANCH_TAG_TEXT_EXPR};" in block, ( + "the text IS the signal (branch: )" + ) + assert "urlTd.append(branchTag)" in block, "the tag lands in the source cell" + # The tag comes after the hidden tag (the source-cell tag order: + # ignored → hidden → branch) and before the cell is appended. + hidden_i = make.find('hiddenTag.className = "git-source-hidden-count"') + append_i = make.find("tr.appendChild(urlTd)") + assert -1 < hidden_i < guard_i < append_i + + +def test_make_row_title_names_the_branch() -> None: + """The source cell's title (the hover-precision idiom) includes the + pinned branch for stored git rows with one; every other row keeps + the bare value (the pre-phase-135 title — the token E2E's + to_have_attribute("title", URL) pins stay green).""" + js = _js_text() + make = _fn(js, "makeRow") + assert f"urlTd.title = {TITLE_EXPR};" in make, ( + "the title names the branch (stored git rows only)" + ) + + +# --------------------------------------------------------------------------- +# Cross-file wire contract — the JS key names match the API shapes +# --------------------------------------------------------------------------- + + +def test_js_branch_keys_match_the_api_shapes() -> None: + """The wire contract: the POST body key and the PATCH body key are + both ``branch`` (task 04 step 3 — the branch rides the existing + payloads), and every API shape carries the field (task 01/02): + ``GitSourceIn`` (create), ``GitSourcePatchIn`` (tri-state), + ``GitSourceOut`` + ``GitSourceRow`` (the row the UI renders — + ``s.branch`` is read off it). A rename on either side breaks.""" + assert "branch" in GitSourceIn.model_fields + assert "branch" in GitSourcePatchIn.model_fields + assert "branch" in GitSourceOut.model_fields + assert "branch" in GitSourceRow.model_fields + js = _js_text() + assert "...(branch ? { branch } : {})" in js, "the POST body key (blank-omits)" + assert "...(isGitRow ? { branch } : {})" in js, "the PATCH body key (always for git)" + assert "(s.branch ?? \"\")" in js, "the prefill reads s.branch off the row" + + +# --------------------------------------------------------------------------- +# The module docstring — the phase-135 paragraph (the house pattern) +# --------------------------------------------------------------------------- + + +def test_module_docstring_carries_the_phase_135_contract() -> None: + """The git-sources.js module docstring gained the phase-135 + paragraph (the field, the tri-state asymmetry, the tag rule).""" + doc = _js_text().split("*/", 2)[0] # the module docstring (first block) + for frag in ( + "Phase 135 (task 04)", + "#git-source-branch", + "#ignore-editor-branch", + "s.branch ?? \"\"", + "THE TRI-STATE ASYMMETRY", + "branch: ", + "the tag appears/disappears", + ): + assert frag in doc, f"the module docstring lost: {frag!r}" + + +# --------------------------------------------------------------------------- +# styles.css — house-styled rules (no second input language) +# --------------------------------------------------------------------------- + + +def test_branch_field_css_rules_are_house_styled() -> None: + """styles.css: the add form's branch input joins the URL/token box + group (mono, >=44px floor, placeholder + focus rules); the editor's + branch field is full panel width at the 44px floor (the token + field's box language); the hint line carries the .field-hint values + (ink-soft, 0.8rem); the row tag joins the .git-source-hidden-count + rule (text + a distinct background, never color alone); the mobile + squeeze groups the add input with URL/token.""" + css = _read(STYLES_CSS) + grouped = css.find("#git-source-url,\n#git-source-branch,\n#git-source-token {") + assert grouped != -1, "#git-source-branch shares the URL/token box" + rule = css[grouped : css.find("}", grouped)] + assert "min-height: 44px" in rule and "var(--mono)" in rule + assert "#git-source-branch::placeholder" in css, "the muted placeholder" + assert "#git-source-branch:focus-visible" in css, "the brand focus border" + br = css.find("#ignore-editor-branch {") + assert br != -1, "the editor's branch field rule" + br_rule = css[br : css.find("}", br)] + assert "width: 100%" in br_rule and "min-height: 44px" in br_rule + hint = css.find(".ignore-editor-branch-hint {") + assert hint != -1, "the muted hint line rule" + hint_rule = css[hint : css.find("}", hint)] + assert "var(--ink-soft)" in hint_rule and "font-size: 0.8rem" in hint_rule, ( + "the .field-hint values (the token field's hint styling)" + ) + tag = css.find(".git-source-hidden-count,\n.git-source-branch-count {") + assert tag != -1, "the row tag joins the 'hidden on' tag rule" + tag_rule = css[tag : css.find("}", tag)] + assert "background: var(--bg)" in tag_rule and "border" in tag_rule, ( + "text + background (never color alone — WCAG 1.4.1)" + ) + mobile = css.find("#git-source-url,\n #git-source-branch,\n #git-source-token,") + assert mobile != -1, "the mobile squeeze groups the branch input" diff --git a/tests/unit/test_git_source_token.py b/tests/unit/test_git_source_token.py index 0746b4c..4b3bbec 100644 --- a/tests/unit/test_git_source_token.py +++ b/tests/unit/test_git_source_token.py @@ -174,6 +174,7 @@ def _out_kwargs() -> dict: "added_at": None, "ignore_paths": [], "include_hidden": False, + "branch": None, } @@ -186,6 +187,7 @@ def _row_kwargs() -> dict: "added_at": None, "ignore_paths": [], "include_hidden": False, + "branch": None, } @@ -464,8 +466,19 @@ STYLES_CSS = FRONTEND / "assets" / "styles.css" ADD_TOKEN_PLACEHOLDER = "ghp_… or another PAT" EDIT_TOKEN_PLACEHOLDER = "leave blank to keep the current token" -ADD_BODY_EXPR = "(url, token) => ({ url, ...(token ? { token } : {}) })" -PATCH_BODY_EXPR = "JSON.stringify({ ignore_paths: lines, ...(token ? { token } : {}) })" +#: Phase 135 (task 04) widened the body to the branch pin — the token +#: key keeps its conditional shape (the contract under test) and the +#: branch key (git rows always — "" = the tri-state's clear leg) lands +#: first (test_git_source_branch_ui.py pins the branch contract). +ADD_BODY_EXPR = ( + "(url, token, branch) => ({ url, ...(branch ? { branch } : {}), " + "...(token ? { token } : {}) })" +) +#: The phase-121 paragraph of the module docstring records the body AS +#: IT WAS in phase 121 (a historical record — the phase-135 paragraph +#: records the widened expression, pinned by test_git_source_branch_ui.py). +ADD_BODY_EXPR_PHASE_121 = "(url, token) => ({ url, ...(token ? { token } : {}) })" +PATCH_BODY_EXPR = "...(token ? { token } : {})" def _read(path: Path) -> str: @@ -526,20 +539,22 @@ def test_add_form_token_field_is_masked_and_optional() -> None: def test_add_form_submit_omits_a_blank_token() -> None: - """The submit body is `(url, token) => ({ url, ...(token ? - { token } : {}) })` — a BLANK token omits the key (None = no - credential); wireAddForm reads the masked field and passes it to - the body callback, and 201 clears BOTH inputs (the credential is - stored — write-only).""" + """The submit body is `(url, token, branch) => ({ url, ...(branch ? + { branch } : {}), ...(token ? { token } : {}) })` — a BLANK token + omits the key (None = no credential; phase 135 added the branch + key, which also omits when blank); wireAddForm reads the masked + field and passes it to the body callback, and 201 clears ALL THREE + credential-free inputs (the credential is stored — write-only).""" js = _js_text() assert f"body: {ADD_BODY_EXPR}" in js, "the submit body omits the key when blank" wire = _fn(js, "wireAddForm") read_i = wire.find('tokenInput ? tokenInput.value.trim() : ""') - call_i = wire.find("opts.body(value, token)", read_i) + call_i = wire.find("opts.body(value, token, branch)", read_i) assert -1 < read_i < call_i, "the token is read from the field and passed to the body" clear_url_i = wire.find('input.value = ""') clear_tok_i = wire.find("tokenInput.value = \"\"", clear_url_i) - assert -1 < clear_url_i < clear_tok_i, "201: both inputs clear" + clear_br_i = wire.find("branchInput.value = \"\"", clear_tok_i) + assert -1 < clear_url_i < clear_tok_i < clear_br_i, "201: all three inputs clear" def test_editor_token_field_is_masked_and_blanks_to_keep() -> None: @@ -570,15 +585,23 @@ def test_editor_token_field_is_masked_and_blanks_to_keep() -> None: def test_editor_patch_omits_a_blank_token() -> None: """saveIgnorePaths: the token is read AFTER the line parse and the - PATCH body is `{ ignore_paths: lines, ...(token ? { token } : {}) }` - — a BLANK token omits the key (the task-01 tri-state: absent = no - change, the row's stored credential is kept).""" + PATCH body keeps the token as the CONDITIONAL key + (`...(token ? { token } : {})` — a BLANK token omits the key, the + task-01 tri-state: absent = no change, the row's stored credential + is kept). Phase 135 (task 04) widens the body with the branch key + (`...(isGitRow ? { branch } : {})` — ALWAYS for git rows: an empty + save is the deliberate clear leg, unlike the token's blank = keep) + landing before the token key — the token's semantics are untouched + (the branch contract is pinned by test_git_source_branch_ui.py).""" js = _js_text() save = _fn(js, "saveIgnorePaths") drop_i = save.find(".filter(Boolean)") read_i = save.find('ignoreTokenInput ? ignoreTokenInput.value.trim() : ""', drop_i) - body_i = save.find(PATCH_BODY_EXPR, read_i) - assert -1 < drop_i < read_i < body_i, ("blank token → key omitted (tri-state no-change)") + branch_i = save.find("...(isGitRow ? { branch } : {})", read_i) + body_i = save.find(PATCH_BODY_EXPR, branch_i) + assert -1 < drop_i < read_i < branch_i < body_i, ( + "blank token → key omitted (tri-state no-change; the branch key lands first)" + ) def test_display_sites_render_the_server_bare_url_only() -> None: @@ -609,7 +632,7 @@ def test_module_docstring_carries_the_phase_121_token_contract() -> None: "#git-source-token", "type=password", "optional — private repos", - ADD_BODY_EXPR, + ADD_BODY_EXPR_PHASE_121, "leave blank to keep the current token", "sanitize_url", ): @@ -625,7 +648,9 @@ def test_token_field_css_rules_are_house_styled() -> None: width at the 44px floor; the mobile media query groups it in the same min-width:0 rule as the URL input.""" css = _read(STYLES_CSS) - grouped = css.find("#git-source-url,\n#git-source-token {") + # Phase 135 (task 04): the branch input joined the group — the + # token input still shares the URL input's box. + grouped = css.find("#git-source-url,\n#git-source-branch,\n#git-source-token {") assert grouped != -1, "#git-source-token shares #git-source-url's box" rule = css[grouped : css.find("}", grouped)] assert "min-height: 44px" in rule and "var(--mono)" in rule @@ -637,5 +662,5 @@ def test_token_field_css_rules_are_house_styled() -> None: assert tok != -1, "the editor's token field rule" tok_rule = css[tok : css.find("}", tok)] assert "width: 100%" in tok_rule and "min-height: 44px" in tok_rule - mobile = css.find("#git-source-url,\n #git-source-token,") + mobile = css.find("#git-source-url,\n #git-source-branch,\n #git-source-token,") assert mobile != -1, "the mobile squeeze groups the token input" diff --git a/tests/unit/test_git_sources_branch.py b/tests/unit/test_git_sources_branch.py new file mode 100644 index 0000000..5f1d720 --- /dev/null +++ b/tests/unit/test_git_sources_branch.py @@ -0,0 +1,150 @@ +"""Unit: the branch-name validator (phase 135, task 02). + +``validate_branch_name`` is the single choke point for every branch +value that enters the API (POST create + PATCH set/change legs — +ASSUMPTION B2). The contract, checked in the documented order (trim → +empty → length → charset → ``..``/``//`` → leading → trailing → +HEAD): + +* TRIM first — the returned name is the trimmed input, so a + schema-trimmed value is a fixed point; +* empty (or whitespace-only) → ``ValueError``; +* > 200 chars → ``ValueError`` (exactly 200 passes — the schema's + ``max_length=200`` is the first line, the validator the choke point + a future caller must not bypass); +* outside the safe charset ``[A-Za-z0-9._/-]+`` → ``ValueError`` + (whitespace, ``@``, ``~``, ``^``, ``:``, ``?``, ``*``, ``[``, + backslash — all out); +* contains ``..`` or ``//`` → ``ValueError`` (path-traversal-shaped + names, even though the charset would allow the pieces); +* leading ``-`` / ``.`` / ``/`` → ``ValueError``; +* trailing ``/`` or ``.`` → ``ValueError``; +* ``HEAD`` (any case) → ``ValueError`` (git's refname — a branch + literally named ``head`` is a documented cost of the simple rule). + +The raised ``ValueError`` messages are FIXED reason strings — the +API maps them to the fixed-detail 422 and the raw input is never +echoed (the house credential-safety discipline, applied for +consistency): the tests pin that no message repeats the rejected +input. +""" +from __future__ import annotations + +import pytest + +from app.rag.git_sources import validate_branch_name + +# --------------------------------------------------------------------------- +# Accept +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "raw", + [ + "main", + "release/1.2", + "feature_x", + "a.b/c", + "v2-rc.1", + ], +) +def test_valid_names_pass_through_trimmed(raw: str) -> None: + """The accepted shapes — plain, slashed, dotted, dashed, + underscored — return exactly the trimmed input.""" + assert validate_branch_name(raw) == raw + + +def test_trim_applied_before_validation_and_returned() -> None: + """Surrounding whitespace is stripped: the returned value is the + trimmed name (``" main "`` → ``main``).""" + assert validate_branch_name(" main ") == "main" + assert validate_branch_name("\trelease/1.2\n") == "release/1.2" + + +def test_exactly_200_chars_passes() -> None: + """The length boundary is > 200: a 200-char name is valid + (the charset allows it — a long but well-formed branch path).""" + name = "a" * 196 + "/x.1" + assert len(name) == 200 + assert validate_branch_name(name) == name + + +# --------------------------------------------------------------------------- +# Reject +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "raw", + [ + "", + " ", + "a b", # whitespace inside + "a@b", + "a~b", + "a:b", + "a?b", + "a*b", + "a[b", + "a\\b", + "a^b", + ], +) +def test_empty_and_charset_violations_rejected(raw: str) -> None: + """Empty/whitespace-only and non-charset characters all raise.""" + with pytest.raises(ValueError): + validate_branch_name(raw) + + +def test_201_chars_rejected() -> None: + """One past the length limit raises (even though the charset + allows every character).""" + with pytest.raises(ValueError): + validate_branch_name("a" * 201) + + +@pytest.mark.parametrize("raw", ["a..b", "a//b", "..", "//"]) +def test_path_sequences_rejected(raw: str) -> None: + """``..`` and ``//`` are disallowed — traversal-shaped names, + including the two-character bare forms.""" + with pytest.raises(ValueError): + validate_branch_name(raw) + + +@pytest.mark.parametrize("raw", ["-x", ".x", "/x"]) +def test_leading_characters_rejected(raw: str) -> None: + """A leading ``-`` (git would read it as an option), ``.``, or + ``/`` raises.""" + with pytest.raises(ValueError): + validate_branch_name(raw) + + +@pytest.mark.parametrize("raw", ["x/", "x."]) +def test_trailing_characters_rejected(raw: str) -> None: + """A trailing ``/`` or ``.`` raises.""" + with pytest.raises(ValueError): + validate_branch_name(raw) + + +@pytest.mark.parametrize("raw", ["HEAD", "head", "HeAd", " hEaD "]) +def test_head_rejected_case_insensitively(raw: str) -> None: + """``HEAD`` in any case is git's refname — rejected (the trim runs + first, so even `` hEaD `` is the reserved name).""" + with pytest.raises(ValueError): + validate_branch_name(raw) + + +def test_reject_messages_are_fixed_and_input_free() -> None: + """The ``ValueError`` reasons are fixed strings — none of them + echoes the rejected input (the API maps to the fixed-detail 422 + on the same discipline; a secret-ish name typed into the field + must not surface in a log line either).""" + for raw in ("a b", "a..b", "x/", "HEAD", "a" * 201, ""): + try: + validate_branch_name(raw) + except ValueError as e: + if raw: + assert raw not in str(e) + else: + pytest.fail(f"{raw!r} must be rejected") diff --git a/tests/unit/test_git_sync.py b/tests/unit/test_git_sync.py index d2ea65c..929c251 100644 --- a/tests/unit/test_git_sync.py +++ b/tests/unit/test_git_sync.py @@ -1,9 +1,16 @@ -"""Unit tests: git clone/pull utility (phase 28; phase 107 D11/D12). +"""Unit tests: git clone/pull utility (phase 28; phase 107 D11/D12; +phase 135 adds the branch pin — the argv matrix). ``clone_or_pull`` dispatches to ``git clone`` (full history, fresh dest) or a shallow-probe + optional ``git fetch --unshallow`` + ``git pull --ff-only`` (existing checkout) with ``subprocess`` fully mocked — the real git CLI is never invoked, so the tests run anywhere. +Phase 135: with a pinned ``branch`` the fresh clone gains ``--branch`` +and the existing checkout runs the HEAD guard → probe → (unshallow) → +``fetch origin `` → ``merge --ff-only FETCH_HEAD`` sequence; +``branch=None`` (and the two-argument call) keeps the pre-phase argv +BYTE-IDENTICAL — the argv pins below are the regression gate for +every existing deployment. """ from __future__ import annotations @@ -236,3 +243,258 @@ def test_run_git_captures_and_returns_stdout( assert git_sync.run_git(["git", "status"], cwd=tmp_path) == "From example.com\n + abc..def main" assert len(calls) == 1 + + +# --- phase 135: the branch pin (the argv matrix) ---------------------------- + + +def test_branch_none_argv_is_byte_identical_pre_phase_sequence( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The regression gate: ``branch=None`` (explicit, the call sites' + form for every pre-phase-135 row) produces the EXACT pre-phase argv + — fresh: one ``git clone`` without ``--branch``; existing (not + shallow): probe → ``git pull --ff-only``; existing (shallow): + probe → ``fetch --unshallow`` → ``git pull --ff-only``. A future + refactor that rewrites the default path breaks this pin.""" + url = "https://github.com/user/homelab.git" + dest = tmp_path / "homelab" + + # fresh (dest absent) + calls = _fake_run(monkeypatch) + clone_or_pull(url, dest, None) + assert [call["argv"] for call in calls] == [["git", "clone", url, str(dest)]] + + # existing, not shallow + (dest / ".git").mkdir(parents=True) + calls = _fake_run( + monkeypatch, + responses={("git", "rev-parse", "--is-shallow-repository"): (0, "false\n", "")}, + ) + clone_or_pull(url, dest, None) + assert [call["argv"] for call in calls] == [ + ["git", "rev-parse", "--is-shallow-repository"], + ["git", "pull", "--ff-only"], + ] + + # existing, shallow (the one-time D11 self-heal between probe and pull) + calls = _fake_run( + monkeypatch, + responses={ + ("git", "rev-parse", "--is-shallow-repository"): (0, "true\n", ""), + ("git", "fetch", "--unshallow"): (0, "", ""), + }, + ) + clone_or_pull(url, dest, None) + assert [call["argv"] for call in calls] == [ + ["git", "rev-parse", "--is-shallow-repository"], + ["git", "fetch", "--unshallow"], + ["git", "pull", "--ff-only"], + ] + + +def test_fresh_clone_with_branch_uses_clone_branch( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Phase 135: fresh dest + pinned branch → ``git clone --branch + url dest`` (the branch is checked out from the first commit — no + post-clone checkout; still full history, no ``--depth``).""" + url = "https://github.com/user/homelab.git" + dest = tmp_path / "homelab" + calls = _fake_run(monkeypatch) + + result = clone_or_pull(url, dest, "release") + + assert result == dest + assert [call["argv"] for call in calls] == [ + ["git", "clone", "--branch", "release", url, str(dest)] + ] + assert calls[0]["cwd"] == dest.parent + + +def test_existing_checkout_matching_branch_fetches_and_ff_merges_the_ref( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Phase 135: existing checkout ALREADY on the pinned branch → the + full sequence in order: the HEAD guard (``rev-parse --abbrev-ref + HEAD``) → the shallow probe → the single-fetch ``fetch origin `` + → ``merge --ff-only FETCH_HEAD`` (not ``git pull`` — the default + path's argv is reserved for ``branch=None``).""" + dest = tmp_path / "homelab" + (dest / ".git").mkdir(parents=True) + calls = _fake_run( + monkeypatch, + responses={ + ("git", "rev-parse", "--abbrev-ref", "HEAD"): (0, "release\n", ""), + ("git", "rev-parse", "--is-shallow-repository"): (0, "false\n", ""), + ("git", "fetch", "origin", "release"): (0, " * branch release -> FETCH_HEAD", ""), + ("git", "merge", "--ff-only", "FETCH_HEAD"): (0, "Already up to date.\n", ""), + }, + ) + + result = clone_or_pull("https://github.com/user/homelab.git", dest, "release") + + assert result == dest + assert [call["argv"] for call in calls] == [ + ["git", "rev-parse", "--abbrev-ref", "HEAD"], + ["git", "rev-parse", "--is-shallow-repository"], + ["git", "fetch", "origin", "release"], + ["git", "merge", "--ff-only", "FETCH_HEAD"], + ] + for call in calls: + assert call["cwd"] == dest + + +def test_existing_shallow_checkout_matching_branch_unshallows_before_fetch( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Phase 135 + phase 107 D11: the branch path runs the SAME one-time + self-heal — a shallow matching checkout is unshallowed BETWEEN the + guard/probe and the branch fetch (order pinned).""" + dest = tmp_path / "homelab" + (dest / ".git").mkdir(parents=True) + calls = _fake_run( + monkeypatch, + responses={ + ("git", "rev-parse", "--abbrev-ref", "HEAD"): (0, "release\n", ""), + ("git", "rev-parse", "--is-shallow-repository"): (0, "true\n", ""), + ("git", "fetch", "--unshallow"): (0, "", ""), + ("git", "fetch", "origin", "release"): (0, "", ""), + ("git", "merge", "--ff-only", "FETCH_HEAD"): (0, "", ""), + }, + ) + + clone_or_pull("https://github.com/user/homelab.git", dest, "release") + + assert [call["argv"] for call in calls] == [ + ["git", "rev-parse", "--abbrev-ref", "HEAD"], + ["git", "rev-parse", "--is-shallow-repository"], + ["git", "fetch", "--unshallow"], + ["git", "fetch", "origin", "release"], + ["git", "merge", "--ff-only", "FETCH_HEAD"], + ] + + +def test_existing_checkout_branch_mismatch_fails_loud_before_any_fetch( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Phase 135 (ASSUMPTION B5, D12): a checkout on a DIFFERENT branch + raises the remove-and-re-add ``GitSyncError`` — naming the checkout + and BOTH branch names — and the guard is FIRST: no shallow probe, + no fetch, no merge argv was invoked (no silent branch switch).""" + url = "https://github.com/user/homelab.git" + dest = tmp_path / "homelab" + (dest / ".git").mkdir(parents=True) + calls = _fake_run( + monkeypatch, + responses={("git", "rev-parse", "--abbrev-ref", "HEAD"): (0, "main\n", "")}, + ) + + with pytest.raises(GitSyncError) as excinfo: + clone_or_pull(url, dest, "release") + + msg = str(excinfo.value) + assert f"checkout {dest}" in msg # the checkout is named + assert "'main'" in msg # the checkout's branch + assert "'release'" in msg # the source's pinned branch + assert "remove and re-add the source to switch branches" in msg + # credential safety: the message carries NO URL (the only place a + # token could ride) — even for a plain URL. + assert url not in msg + assert "github" not in msg + # the guard is first: exactly ONE argv (the guard probe) ran. + assert [call["argv"] for call in calls] == [ + ["git", "rev-parse", "--abbrev-ref", "HEAD"] + ] + + +def test_branch_mismatch_message_never_contains_url_even_with_token( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Phase 135 credential-safety: even when the CLONE URL carries the + injected token (the phase-121 ``clone_url_for`` form), the guard + message is URL-free — the checkout path and the two branch names + only.""" + url = "https://x-access-token:ghp_supersecret@git.example.com/user/repo.git" + dest = tmp_path / "repo" + (dest / ".git").mkdir(parents=True) + calls = _fake_run( + monkeypatch, + responses={("git", "rev-parse", "--abbrev-ref", "HEAD"): (0, "main\n", "")}, + ) + + with pytest.raises(GitSyncError) as excinfo: + clone_or_pull(url, dest, "release") + + msg = str(excinfo.value) + assert url not in msg + assert "ghp_supersecret" not in msg + assert "git.example.com" not in msg + assert f"checkout {dest} is on branch 'main' but the source " \ + "specifies branch 'release'" in msg + assert [call["argv"] for call in calls] == [ + ["git", "rev-parse", "--abbrev-ref", "HEAD"] + ] + + +def test_existing_checkout_branch_fetch_failure_propagates_before_merge( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Phase 135 D12 fail-loud: a failed ``fetch origin `` raises + (with git's stderr) and the merge NEVER runs.""" + dest = tmp_path / "homelab" + (dest / ".git").mkdir(parents=True) + calls = _fake_run( + monkeypatch, + responses={ + ("git", "rev-parse", "--abbrev-ref", "HEAD"): (0, "release\n", ""), + ("git", "rev-parse", "--is-shallow-repository"): (0, "false\n", ""), + ("git", "fetch", "origin", "release"): (128, "", "fatal: couldn't find remote ref\n"), + }, + ) + + with pytest.raises( + GitSyncError, + match=r"git fetch origin release failed \(exit 128\): fatal: couldn't find remote ref", + ): + clone_or_pull("https://github.com/user/homelab.git", dest, "release") + + assert [call["argv"] for call in calls] == [ + ["git", "rev-parse", "--abbrev-ref", "HEAD"], + ["git", "rev-parse", "--is-shallow-repository"], + ["git", "fetch", "origin", "release"], + ] # the merge never ran + + +def test_existing_checkout_branch_non_ff_merge_fails_loud( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Phase 135: a non-ff ``merge --ff-only FETCH_HEAD`` (diverged / + force-pushed origin) fails loudly HERE with git's stderr — the + reset fallback is phase 136, not a silent fix in this phase.""" + dest = tmp_path / "homelab" + (dest / ".git").mkdir(parents=True) + calls = _fake_run( + monkeypatch, + responses={ + ("git", "rev-parse", "--abbrev-ref", "HEAD"): (0, "release\n", ""), + ("git", "rev-parse", "--is-shallow-repository"): (0, "false\n", ""), + ("git", "fetch", "origin", "release"): (0, "", ""), + ("git", "merge", "--ff-only", "FETCH_HEAD"): ( + 1, + "", + "fatal: Not possible to fast-forward, aborting.\n", + ), + }, + ) + + with pytest.raises( + GitSyncError, + match=( + r"git merge --ff-only FETCH_HEAD failed \(exit 1\): " + r"fatal: Not possible to fast-forward" + ), + ): + clone_or_pull("https://github.com/user/homelab.git", dest, "release") + + assert len(calls) == 4 # all four argv ran; the merge failed loud diff --git a/tests/unit/test_hidden_folders_toggle.py b/tests/unit/test_hidden_folders_toggle.py index 1e6f03b..d5f8642 100644 --- a/tests/unit/test_hidden_folders_toggle.py +++ b/tests/unit/test_hidden_folders_toggle.py @@ -389,7 +389,10 @@ def test_hidden_checkbox_css_rules_exist_with_accent_color() -> None: assert "opacity" in disabled and "cursor: wait" in disabled, ( "the .git-source-remove:disabled idiom" ) - tag = _css_rule(css, ".git-source-hidden-count") + # Phase 135 (task 04): the "branch: " state tag joined this + # rule — the "hidden on" tag keeps the same idiom (text + a + # distinct background, never color alone). + tag = _css_rule(css, ".git-source-hidden-count,\n.git-source-branch-count") assert "var(--ink)" in tag and "var(--bg)" in tag, ( "text + a distinct background (never color alone)" ) diff --git a/tests/unit/test_source_ignore_paths.py b/tests/unit/test_source_ignore_paths.py index 5a426d1..2d4b436 100644 --- a/tests/unit/test_source_ignore_paths.py +++ b/tests/unit/test_source_ignore_paths.py @@ -384,14 +384,20 @@ def test_save_runs_the_inflight_never_stale_lifecycle() -> None: label_i = body.find(f'"{SAVING_LABEL}"', dis_s) fetch_i = body.find("`/api/git-sources/${", label_i) method_i = body.find('method: "PATCH"', fetch_i) - body_i = body.find( - "JSON.stringify({ ignore_paths: lines, ...(token ? { token } : {}) })", method_i - ) + # Phase 135 (task 04) widened the body: the branch key lands + # BEFORE the (still conditional) token key — git rows always send + # it ("" = the tri-state's clear leg), local rows omit it. + body_i = body.find("JSON.stringify({\n ignore_paths: lines,", method_i) + branch_key_i = body.find("...(isGitRow ? { branch } : {})", body_i) + token_key_i = body.find("...(token ? { token } : {})", branch_key_i) assert -1 < guard_i < parse_i < trim_i < drop_i < token_i < inflight_i, ( "guard → split + trim + drop blank lines → read the token → in-flight" ) assert -1 < dis_c < dis_s < label_i < fetch_i < method_i < body_i, ( - "disable both + 'Saving…' → the PATCH (lines + token only when non-blank)" + "disable both + 'Saving…' → the PATCH" + ) + assert -1 < body_i < branch_key_i < token_key_i, ( + "the body: lines, branch for git rows, token only when non-blank" ) # Success: close → reload → announce (the exact order) — the # update confirmation is the LAST announcement: the reload's diff --git a/tests/unit/test_sync_button.py b/tests/unit/test_sync_button.py index c3b740c..be13679 100644 --- a/tests/unit/test_sync_button.py +++ b/tests/unit/test_sync_button.py @@ -812,14 +812,15 @@ def fresh_sync_status() -> Iterator[None]: class _GatedClone: """A ``clone_or_pull`` that parks between start and finish on a threading gate, so the test can read the status during the - clone/pull phase (A4: no file yet).""" + clone/pull phase (A4: no file yet). Phase 135: accepts the row's + branch pin (the third argument — ``None`` for this file's rows).""" def __init__(self, started: threading.Event, release: threading.Event) -> None: self.started = started self.release = release self.calls: list[tuple[str, Path]] = [] - def __call__(self, url: str, dest: Path | str) -> Path: + def __call__(self, url: str, dest: Path | str, branch: str | None = None) -> Path: dest = Path(dest) self.calls.append((url, dest)) dest.mkdir(parents=True, exist_ok=True) @@ -1402,10 +1403,12 @@ def test_chunk_position_clears_at_phase_transitions_and_terminal( # the event set / releases. -def _fast_clone(url: str, dest: Path | str) -> Path: +def _fast_clone(url: str, dest: Path | str, branch: str | None = None) -> 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).""" + (the import's gate is an ``asyncio.to_thread`` park — loop-safe). + Phase 135: accepts the row's branch pin (the third argument — + ``None`` for this file's rows).""" dest = Path(dest) dest.mkdir(parents=True, exist_ok=True) (dest / "notes.md").write_text("# repo\ncontent for the KB\n", encoding="utf-8") @@ -1721,7 +1724,8 @@ def test_event_alone_cancels_at_the_import_boundary( clone_release = threading.Event() clone_calls: list[tuple[str, Path]] = [] - def parking_clone(url: str, dest: Path | str) -> Path: + def parking_clone(url: str, dest: Path | str, branch: str | None = None) -> Path: + # phase 135: the row's branch pin (None for this file's rows) dest = Path(dest) dest.mkdir(parents=True, exist_ok=True) (dest / "notes.md").write_text("# repo\ncontent for the KB\n", encoding="utf-8") @@ -1864,7 +1868,8 @@ def test_event_cancels_at_the_top_of_the_source_loop( release.wait() # blocking is fine: the date walk is a sync call return {} - def fast_clone(url: str, dest: Path | str) -> Path: + def fast_clone(url: str, dest: Path | str, branch: str | None = None) -> Path: + # phase 135: the row's branch pin (None for this file's rows) dest = Path(dest) dest.mkdir(parents=True, exist_ok=True) (dest / "notes.md").write_text("# repo\ncontent for the KB\n", encoding="utf-8")