feat(admin): local directory sources — kind/path on git_sources, combined sync + import, page form + badges
An existing, non-git directory is now a first-class source alongside
the git repos: one table (git_sources + kind discriminator — A13
reversible migration), one admin page, one Sync button (phase locked
decisions; the phase-35 table is extended, not duplicated). The DB is
the local-source registry — no env var for local paths;
BOR_GIT_SOURCES stays a git-only empty-table fallback.
Migration 0007 (reversible, up/down integration-tested):
git_sources.kind TEXT NOT NULL DEFAULT 'git' + ck_git_sources_kind
(kind IN ('git','local')); git_sources.path TEXT NULL +
uq_git_sources_path (mirrors 0006's uq_git_sources_url). Existing rows
read kind='git', path=NULL.
API (phase-35 contract extended, git byte-identical): POST kind=local
requires path — trimmed, ~-expanded, absolute + an existing server
directory, else 422 naming the path (fail loud at add-time); duplicate
path 409 (named); wrong field combos 422. GET rows carry kind + path
(git and env rows: path null); anonymous still 403 on every route (A10).
Sync + import_docs resolve DB git + local rows together: git →
clone_or_pull (unchanged); local → re-verified .is_dir() AT SYNC TIME
(it may have moved/deleted since add-time) — a missing dir raises
"local source missing: <path>" (sanitized) before anything imports;
one import_sources(..., prune=True) over the single combined list
(pruning covers the union). Both-empty fails loudly ("no sources
configured (git or local)"); --source still wins; the env fallback
stays git-only.
Page: second "Add a local directory" form (the same §7.4 never-stale
button + inline-error lifecycle as the git form; 422/409 details name
the path), Git/Local badges on rows (text + color, never color alone —
WCAG), updated hint (git + local together, union prune); the
anonymous sign-in gate is unchanged.
Tests: 0007 up/down; the API local-kind matrix (403/201/422/409) with
the git-kind suite green unchanged; the sync pipeline local/git/
mixed/missing against a host temp dir (the KB actually updated);
import_docs DB resolution + --source precedence. Story E2E (isolated,
deterministic across runs): add (Local badge) → missing path inline
422 naming it / duplicate 409 → the real Sync button imports the
fixture file (GET /api/docs + sentinel in its content) → file deleted
+ sync prunes it (union prune) → row removed; anonymous gate + 403s
(phase-35 regression). test_git_sources_admin.py (phase 35) green
UNCHANGED — no selector collision with the new form;
test_sync_button.py green.
Docs: README — the two managed kinds (git = clone/pull mirror; local =
direct in-place walk), add-time validation, union pruning, "the DB is
the local-source registry (no env var for local paths)";
.env.example — the env fallback is git-only.
This commit is contained in:
@@ -0,0 +1,496 @@
|
||||
"""Phase 38 story E2E (Playwright): local directory sources.
|
||||
|
||||
Story: ``.agent/user_stories/local-directory-sources.md``
|
||||
Run in isolation (DB must be up: ``podman compose up -d db``):
|
||||
|
||||
uv run pytest tests/e2e/test_local_directory_sources.py -v --no-cov
|
||||
|
||||
The story gate for the **local directory** kind of the admin-managed
|
||||
source registry (phase 38): the admin adds an existing, non-git
|
||||
directory on the same admin page as the git repos (phase 35), and the
|
||||
real Sync button (phase 32) imports it — with add-time fail-loud
|
||||
validation (a missing/relative path is rejected inline, naming the
|
||||
path) and union pruning (a file deleted from the directory leaves the
|
||||
index on the next sync; removing the row stops it being a source).
|
||||
|
||||
The fixture is a **host temp dir** (``tmp_path_factory`` — the app
|
||||
server runs on the same host, so the path is visible to it) containing
|
||||
one plain ``.md`` with a distinctive sentinel token. No git anywhere in
|
||||
this suite (the directory is deliberately NOT a git repo — that is the
|
||||
point of the story), so no ``BOR_GIT_SOURCES`` and no clone: the sync
|
||||
pipeline under test is the ``kind=local`` branch (direct directory
|
||||
walk, re-verified ``.is_dir()`` at sync time) with prune over the union
|
||||
(the KB was truncated, so the fixture file is the only thing the sync
|
||||
can import — and the only thing it can prune).
|
||||
|
||||
Per-module app env (the conftest pattern, module-scoped — as in
|
||||
``test_sync_button.py``): this story's app boots WITHOUT
|
||||
``BOR_GIT_SOURCES`` (the env fallback is git-only by the phase's locked
|
||||
decision — local directories are DB-registered, no env var), so an
|
||||
empty table means "no sources configured" until the admin adds the
|
||||
directory through the real page.
|
||||
|
||||
Contract under test:
|
||||
|
||||
* anonymous: the sign-in gate (the phase-16/35 ``#git-sources-gate``
|
||||
pattern), the manager hidden (list + BOTH add forms inert), NO
|
||||
``/api/git-sources`` call, and 403 on the source routes + the sync
|
||||
trigger (the phase-35 regression assertions, A10);
|
||||
* admin: a missing path (``/nonexistent/bor-e2e``) 422s inline naming
|
||||
the path with no row added and the button never stale; the host temp
|
||||
dir adds (201 → row with the **Local** badge + the full path in a
|
||||
mono cell, input cleared, button re-enabled); the same path again
|
||||
409s inline ("already exists", path named) with no second row;
|
||||
* admin: the header **Sync** button (the phase-32 lifecycle, "Syncing…"
|
||||
→ "Synced HH:MM") imports the fixture file — it appears in
|
||||
``GET /api/docs`` (and its sentinel is in ``GET
|
||||
/api/documents/content``); deleting the file and syncing again prunes
|
||||
it (``pruned: 1``, gone from ``GET /api/docs`` — union prune); then
|
||||
removing the row on the page makes it disappear (accept the confirm;
|
||||
the empty state returns);
|
||||
* the new local form's a11y basics (UI Structure Check, AGENTS.md rule
|
||||
5): labeled input, role=alert error line, ≥44px target, 3px
|
||||
focus-visible outline.
|
||||
|
||||
Test → story mapping (Playwright Mapping Rule):
|
||||
1. ``test_anonymous_soft_gate_and_403s``
|
||||
2. ``test_admin_add_missing_path_then_dir_then_duplicate``
|
||||
3. ``test_admin_sync_imports_fixture_prunes_after_delete_removes_row``
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import time
|
||||
from collections.abc import Iterator
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from playwright.sync_api import Dialog, Page, expect
|
||||
from sqlalchemy import text
|
||||
|
||||
from app.db import SessionLocal
|
||||
from e2e.auth_helpers import login
|
||||
from e2e.conftest import (
|
||||
ADMIN_PASSWORD,
|
||||
APP_PORT,
|
||||
SESSION_SECRET,
|
||||
USE_REAL_LLM,
|
||||
_wait_http,
|
||||
)
|
||||
|
||||
REPO = Path(__file__).resolve().parents[2]
|
||||
APP_URL = f"http://127.0.0.1:{APP_PORT}"
|
||||
|
||||
GIT_SOURCES_URL = "/git-sources.html"
|
||||
|
||||
#: The unique sentinel inside the fixture doc — its import into the KB
|
||||
#: (visible via GET /api/docs + /api/documents/content) proves the real
|
||||
#: sync walked the local directory.
|
||||
SENTINEL = "RESE-LOCAL-DIR-TOKEN-4d7e"
|
||||
FIXTURE_REL = "notes/bor-local-fixture.md"
|
||||
|
||||
#: A path that must NOT exist on the host — the add-time 422 subject.
|
||||
MISSING_PATH = "/nonexistent/bor-e2e"
|
||||
|
||||
#: "Synced HH:MM" — the local-time last-result label (header.js's
|
||||
#: fmtSyncTime), any hour/minute.
|
||||
SYNCED_LABEL = re.compile(r"Synced \d{1,2}:\d{2}")
|
||||
|
||||
#: A single-file import against the mock LLM is fast, but the sync runs
|
||||
#: the full pipeline (verify → walk → embed → overview) — same generous
|
||||
#: budget as test_sync_button.py, no client-side hard timeout.
|
||||
SYNC_TIMEOUT_MS = 60_000
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def local_dir(tmp_path_factory: pytest.TempPathFactory) -> Path:
|
||||
"""The story's ``~/Notes``: a plain (non-git) directory the admin
|
||||
registers as a source. Built under ``tmp_path_factory`` (module
|
||||
lifetime, like ``test_sync_button.py``'s fixture repo) and holding
|
||||
one A9-format fixture doc with the sentinel token. The app server
|
||||
runs on the same host, so this path is visible to it."""
|
||||
root = tmp_path_factory.mktemp("bor_local_dir") / "notes-dir"
|
||||
(root / "notes").mkdir(parents=True)
|
||||
(root / FIXTURE_REL).write_text(
|
||||
"# Local directory fixture\n"
|
||||
"\n"
|
||||
"One small note that exists only to prove the local-directory\n"
|
||||
"source story end to end: the admin adds this directory on the\n"
|
||||
"git sources page, the real Sync button walks it and imports it,\n"
|
||||
"and deleting the file + syncing again prunes it (union prune).\n"
|
||||
"\n"
|
||||
f"Marker: {SENTINEL}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
assert (root / FIXTURE_REL).is_file()
|
||||
assert not (root / ".git").exists() # the story: NOT a git repo
|
||||
return root
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def app_server(mock_llm: int, local_dir: Path) -> Iterator[str]:
|
||||
"""The real app under test — per-module env: NO ``BOR_GIT_SOURCES``
|
||||
(the env fallback is git-only; local directories are DB-registered)
|
||||
and a scratch ``BOR_SOURCES_DIR`` (no git row ever syncs here, it is
|
||||
set for hygiene). The session app is never started in this isolated
|
||||
run, so no port clash."""
|
||||
env = dict(os.environ)
|
||||
env.pop("DEBUGPY", None)
|
||||
env["BOR_ENVIRONMENT"] = "e2e"
|
||||
env["BOR_STATIC_DIR"] = str(REPO / "frontend")
|
||||
env["BOR_LLM_BASE_URL"] = (
|
||||
"https://aipi.reeseapps.com/v1" if USE_REAL_LLM else f"http://127.0.0.1:{mock_llm}/v1"
|
||||
)
|
||||
# Mock-calibrated threshold (conftest pattern) — no chat turn is ever
|
||||
# sent in this suite, but the app boots with the same env shape.
|
||||
env["BOR_RELEVANCE_THRESHOLD"] = "0.30"
|
||||
env.setdefault(
|
||||
"BOR_DATABASE_URL",
|
||||
"postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese",
|
||||
)
|
||||
# Phase 16: admin auth must be set or create_app() refuses to boot.
|
||||
env["BOR_ADMIN_PASSWORD"] = ADMIN_PASSWORD
|
||||
env["BOR_SESSION_SECRET"] = SESSION_SECRET
|
||||
# The repo's .env file carries the owner's BOR_GIT_SOURCES (the app
|
||||
# reads it from cwd) — override it with an EMPTY value (the env var
|
||||
# beats the .env file): the story is local-kind only, the env
|
||||
# fallback stays git-only, and an empty table + empty fallback must
|
||||
# mean "no sources configured" until the admin adds the directory.
|
||||
env["BOR_GIT_SOURCES"] = ""
|
||||
env["BOR_SOURCES_DIR"] = str(local_dir.parent / "checkouts")
|
||||
proc = subprocess.Popen(
|
||||
[sys.executable, "-m", "uvicorn", "app.main:app",
|
||||
"--host", "127.0.0.1", "--port", str(APP_PORT), "--log-level", "warning"],
|
||||
cwd=REPO,
|
||||
env=env,
|
||||
)
|
||||
try:
|
||||
_wait_http(f"{APP_URL}/api/health")
|
||||
yield APP_URL
|
||||
finally:
|
||||
proc.terminate()
|
||||
try:
|
||||
proc.wait(timeout=10)
|
||||
except subprocess.TimeoutExpired:
|
||||
proc.kill()
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def app_url(app_server: str) -> str:
|
||||
return app_server
|
||||
|
||||
|
||||
def _truncate_all() -> None:
|
||||
"""Fresh registry + KB per test (the E2E isolation pattern): the
|
||||
sync's counts and every GET /api/docs assertion must be this test's
|
||||
own doing. The E2E suites share one Postgres, and a leftover
|
||||
git_sources row would flip the sync from "no sources configured" to
|
||||
importing another suite's source (or a leftover document would show
|
||||
up in the docs list the pruned-union assertions inspect)."""
|
||||
with SessionLocal() as db:
|
||||
db.execute(text("TRUNCATE chunks, documents, query_log, kb_overview, git_sources"))
|
||||
db.commit()
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clean(db_ready: None) -> Iterator[None]:
|
||||
_truncate_all()
|
||||
yield
|
||||
_truncate_all()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _admin_git_sources_page(page: Page, app_url: str) -> None:
|
||||
"""Real form login landing on the sources page (admin settled:
|
||||
Sign out visible, the manager revealed by the page module)."""
|
||||
login(page, app_url, next=GIT_SOURCES_URL)
|
||||
expect(page).to_have_url(app_url + GIT_SOURCES_URL, timeout=30_000)
|
||||
expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000)
|
||||
expect(page.locator("#git-sources-gate")).to_be_hidden()
|
||||
expect(page.locator("#git-sources-content")).to_be_visible()
|
||||
|
||||
|
||||
def _add_local_dir(page: Page, path: str) -> None:
|
||||
"""Add a local directory through the real page form and wait for the
|
||||
new row (the 201 → reload → row lifecycle of git-sources.js)."""
|
||||
page.fill("#local-source-path", path)
|
||||
page.click("#local-source-add")
|
||||
expect(
|
||||
page.locator("#git-sources-tbody tr", has_text=path)
|
||||
).to_have_count(1, timeout=30_000)
|
||||
|
||||
|
||||
def _click_sync(page: Page) -> None:
|
||||
"""The phase-32 button lifecycle: click → disabled + "Syncing…" →
|
||||
"Synced HH:MM" (re-enabled — never stale). The server status poll
|
||||
underneath is what the 2 s UI loop observes."""
|
||||
btn = page.locator("#sync-btn")
|
||||
expect(btn).to_be_visible()
|
||||
btn.click()
|
||||
expect(btn).to_be_disabled()
|
||||
expect(page.locator("#sync-label")).to_have_text("Syncing…")
|
||||
expect(page.locator("#sync-label")).to_have_text(SYNCED_LABEL, timeout=SYNC_TIMEOUT_MS)
|
||||
expect(btn).to_be_enabled()
|
||||
|
||||
|
||||
def _wait_sync_done(page: Page, app_url: str, timeout_s: float = 60.0) -> dict[str, Any]:
|
||||
"""Poll the (cookie-authenticated) status endpoint until the run
|
||||
reaches a terminal state — exactly what the UI's 2 s poll loop
|
||||
observes (test_sync_button.py's helper)."""
|
||||
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
|
||||
body = r.json()
|
||||
if body["state"] in ("success", "failed"):
|
||||
return body
|
||||
time.sleep(0.5)
|
||||
raise AssertionError(f"sync did not reach a terminal state: {body}")
|
||||
|
||||
|
||||
def _docs(page: Page, app_url: str) -> list[dict[str, Any]]:
|
||||
"""GET /api/docs as the signed-in page (admin cookie rides along)."""
|
||||
r = page.request.get(f"{app_url}/api/docs")
|
||||
assert r.status == 200, r.text
|
||||
return r.json()["documents"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 1. Anonymous: the soft gate, inert manager, no API calls, 403s
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_anonymous_soft_gate_and_403s(
|
||||
page: Page, app_url: str, db_ready: None
|
||||
) -> None:
|
||||
"""The phase-16/35 gate on this page (regression through the phase-38
|
||||
form): anonymous visitors see the sign-in gate and a fully hidden
|
||||
manager (list + git form + local form), the page never calls the
|
||||
admin API, and every admin route 403s (A10)."""
|
||||
page.set_default_timeout(30_000)
|
||||
|
||||
api_calls: list[str] = []
|
||||
page.on(
|
||||
"request",
|
||||
lambda r: api_calls.append(r.url)
|
||||
if "/api/git-sources" in r.url
|
||||
else None,
|
||||
)
|
||||
|
||||
page.goto(app_url + GIT_SOURCES_URL)
|
||||
expect(page.locator("#sign-in-link")).to_be_visible(timeout=15_000)
|
||||
expect(page.locator("#sign-out-btn")).to_be_hidden()
|
||||
|
||||
gate = page.locator("#git-sources-gate")
|
||||
expect(gate).to_be_visible()
|
||||
expect(gate).to_contain_text("Sign in to manage the git sources")
|
||||
|
||||
# The manager is absent/inert: list, BOTH add forms, env note — all
|
||||
# inside the hidden #git-sources-content.
|
||||
expect(page.locator("#git-sources-content")).to_be_hidden()
|
||||
expect(page.locator("#git-sources-table")).to_be_hidden()
|
||||
expect(page.locator("#git-source-form")).to_be_hidden()
|
||||
expect(page.locator("#local-source-form")).to_be_hidden()
|
||||
expect(page.locator("#git-sources-env-note")).to_be_hidden()
|
||||
|
||||
# The gate never called the admin API…
|
||||
assert api_calls == [], f"anonymous page called the git sources API: {api_calls}"
|
||||
# …and the API 403s anonymous callers (the phase-35 assertions):
|
||||
# all three source routes, for BOTH kinds, plus the sync trigger.
|
||||
assert page.request.get(f"{app_url}/api/git-sources").status == 403
|
||||
assert (
|
||||
page.request.post(
|
||||
f"{app_url}/api/git-sources", data={"url": "https://example.com/x.git"}
|
||||
).status
|
||||
== 403
|
||||
)
|
||||
assert (
|
||||
page.request.post(
|
||||
f"{app_url}/api/git-sources", data={"kind": "local", "path": "/tmp"}
|
||||
).status
|
||||
== 403
|
||||
)
|
||||
assert (
|
||||
page.request.delete(
|
||||
f"{app_url}/api/git-sources/00000000-0000-0000-0000-000000000000"
|
||||
).status
|
||||
== 403
|
||||
)
|
||||
assert page.request.post(f"{app_url}/api/sync").status == 403
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 2. Admin: add validation (missing path, dir, duplicate) + local form
|
||||
# a11y basics
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_admin_add_missing_path_then_dir_then_duplicate(
|
||||
page: Page, app_url: str, local_dir: Path, db_ready: None
|
||||
) -> None:
|
||||
"""Add-time fail-loud validation on the real page: a missing path
|
||||
422s inline NAMING the path (no row, never-stale button, the input
|
||||
survives for one edit); the host temp dir adds (row with the Local
|
||||
badge + full path, input cleared); the same path again 409s inline
|
||||
("already exists", path named, no second row)."""
|
||||
page.set_default_timeout(30_000)
|
||||
_admin_git_sources_page(page, app_url)
|
||||
expect(page.locator("#git-sources-tbody tr")).to_have_count(0)
|
||||
|
||||
error = page.locator("#local-source-error")
|
||||
add_btn = page.locator("#local-source-add")
|
||||
|
||||
# --- missing path: inline 422 naming it, NO row, button recovers ---
|
||||
page.fill("#local-source-path", MISSING_PATH)
|
||||
add_btn.click()
|
||||
expect(error).to_be_visible(timeout=30_000)
|
||||
assert error.get_attribute("role") == "alert"
|
||||
expect(error).to_contain_text(MISSING_PATH)
|
||||
expect(error).to_contain_text("not a directory")
|
||||
expect(page.locator("#git-sources-tbody tr")).to_have_count(0)
|
||||
expect(add_btn).to_be_enabled()
|
||||
expect(add_btn).to_have_text("Add directory")
|
||||
expect(page.locator("#local-source-path")).to_have_value(MISSING_PATH)
|
||||
|
||||
# --- the temp dir: 201 → the row appears with the Local badge ------
|
||||
_add_local_dir(page, str(local_dir))
|
||||
row = page.locator("#git-sources-tbody tr", has_text=str(local_dir))
|
||||
expect(row).to_have_count(1)
|
||||
badge = row.locator("span.git-source-kind")
|
||||
expect(badge).to_have_text("Local")
|
||||
expect(badge).to_have_class(re.compile(r"\bis-local\b"))
|
||||
# The mono cell carries the full path (rendered as text)…
|
||||
expect(row.locator("td.git-source-url-cell code")).to_have_text(str(local_dir))
|
||||
# …and the row's Remove button is labeled with the kind + path.
|
||||
expect(row.locator(".git-source-remove")).to_have_attribute(
|
||||
"aria-label", f"Remove local source: {local_dir}"
|
||||
)
|
||||
# The 201 cleared the input and re-enabled the button (never stale).
|
||||
expect(page.locator("#local-source-path")).to_have_value("")
|
||||
expect(add_btn).to_be_enabled()
|
||||
expect(add_btn).to_have_text("Add directory")
|
||||
# The API agrees: kind=local with the stored (expanded) path.
|
||||
r = page.request.get(f"{app_url}/api/git-sources")
|
||||
assert r.status == 200, r.text
|
||||
body = r.json()
|
||||
assert body["from_env"] is False
|
||||
assert [
|
||||
(s["kind"], s["path"]) for s in body["sources"]
|
||||
] == [("local", str(local_dir))]
|
||||
|
||||
# --- duplicate: inline 409 naming the path, NO second row -----------
|
||||
page.fill("#local-source-path", str(local_dir))
|
||||
add_btn.click()
|
||||
expect(error).to_be_visible(timeout=30_000)
|
||||
expect(error).to_contain_text("already exists")
|
||||
expect(error).to_contain_text(str(local_dir))
|
||||
expect(page.locator("#git-sources-tbody tr")).to_have_count(1)
|
||||
expect(add_btn).to_be_enabled()
|
||||
expect(add_btn).to_have_text("Add directory")
|
||||
expect(page.locator("#local-source-path")).to_have_value(str(local_dir))
|
||||
|
||||
# --- the new form's a11y basics (UI Structure Check, AGENTS.md 5) ---
|
||||
expect(page.get_by_label("Add a local directory")).to_have_count(1)
|
||||
box = add_btn.bounding_box()
|
||||
assert box is not None and box["height"] >= 44, f"target too small: {box}"
|
||||
page.focus("#local-source-path")
|
||||
outline = page.evaluate(
|
||||
"() => getComputedStyle(document.querySelector('#local-source-path')).outlineWidth"
|
||||
)
|
||||
assert outline == "3px", f"focus-visible outline missing: {outline!r}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 3. Admin: the real Sync imports the fixture; deleting the file +
|
||||
# syncing again prunes it (union prune); removing the row ends it
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_admin_sync_imports_fixture_prunes_after_delete_removes_row(
|
||||
page: Page, app_url: str, local_dir: Path, db_ready: None
|
||||
) -> None:
|
||||
"""The phase-32 button drives the phase-38 pipeline: the header Sync
|
||||
imports the local directory's fixture file (GET /api/docs shows it,
|
||||
the sentinel is in its content); deleting the file and syncing again
|
||||
prunes it (``pruned: 1`` — prune over the union); then removing the
|
||||
row on the page makes it disappear (the empty state returns)."""
|
||||
page.set_default_timeout(30_000)
|
||||
_admin_git_sources_page(page, app_url)
|
||||
|
||||
# Fresh registry (the autouse fixture truncated it) — add the source
|
||||
# through the real page, then run the sync lifecycle against it.
|
||||
_add_local_dir(page, str(local_dir))
|
||||
|
||||
# --- run 1: the real sync walks the local dir and imports the file -
|
||||
_click_sync(page)
|
||||
body = _wait_sync_done(page, app_url)
|
||||
assert body["state"] == "success", body
|
||||
assert body["detail"]["added"] == 1, body["detail"]
|
||||
assert body["detail"]["pruned"] == 0, body["detail"]
|
||||
|
||||
# The fixture doc is in GET /api/docs…
|
||||
docs = _docs(page, app_url)
|
||||
fixture_docs = [d for d in docs if d["path"] == FIXTURE_REL]
|
||||
assert len(fixture_docs) == 1, f"fixture doc missing from /api/docs: {docs}"
|
||||
assert fixture_docs[0]["source"] == local_dir.name
|
||||
# …and its content carries the sentinel (the walk imported THIS file).
|
||||
content = page.request.get(
|
||||
f"{app_url}/api/documents/content"
|
||||
f"?source={local_dir.name}&path={FIXTURE_REL}"
|
||||
)
|
||||
assert content.status == 200, content.text
|
||||
assert SENTINEL in content.json()["content"]
|
||||
|
||||
# --- run 2: file deleted → the next sync prunes it (union prune) ---
|
||||
(local_dir / FIXTURE_REL).unlink()
|
||||
_click_sync(page)
|
||||
body = _wait_sync_done(page, app_url)
|
||||
assert body["state"] == "success", body
|
||||
assert body["detail"]["pruned"] == 1, body["detail"]
|
||||
assert body["detail"]["added"] == 0, body["detail"]
|
||||
docs = _docs(page, app_url)
|
||||
assert [d for d in docs if d["path"] == FIXTURE_REL] == [], (
|
||||
f"fixture doc survived the prune: {docs}"
|
||||
)
|
||||
|
||||
# --- remove the row: accept the confirm → it disappears ------------
|
||||
removes: list[str] = []
|
||||
page.on(
|
||||
"request",
|
||||
lambda r: removes.append(r.url)
|
||||
if r.method == "DELETE" and "/api/git-sources/" in r.url
|
||||
else None,
|
||||
)
|
||||
|
||||
def handle_dialog(dialog: Dialog) -> None:
|
||||
# "Remove this local source…? Its documents stay indexed until
|
||||
# the next sync prunes them." — accept it.
|
||||
dialog.accept()
|
||||
|
||||
page.on("dialog", handle_dialog)
|
||||
try:
|
||||
row = page.locator("#git-sources-tbody tr", has_text=str(local_dir))
|
||||
row.locator(".git-source-remove").click()
|
||||
expect(page.locator("#git-sources-tbody tr")).to_have_count(0, timeout=30_000)
|
||||
expect(page.locator("#git-sources-empty")).to_be_visible()
|
||||
finally:
|
||||
page.remove_listener("dialog", handle_dialog)
|
||||
assert len(removes) == 1, f"expected one DELETE, saw: {removes}"
|
||||
# The registry is empty again — and with no env git list, a further
|
||||
# sync would fail loudly ("no sources configured (git or local)").
|
||||
r = page.request.get(f"{app_url}/api/git-sources")
|
||||
assert r.json() == {"sources": [], "from_env": True}
|
||||
@@ -1,4 +1,5 @@
|
||||
"""Integration: the admin git-sources CRUD API (phase 35, task 02).
|
||||
"""Integration: the admin sources CRUD API (phase 35, task 02; local
|
||||
kind, phase 38, task 02).
|
||||
|
||||
Real Postgres (``podman compose up -d db``); the ``BOR_GIT_SOURCES``
|
||||
fallback is exercised deterministically by monkeypatching the router's
|
||||
@@ -7,17 +8,25 @@ fallback is exercised deterministically by monkeypatching the router's
|
||||
|
||||
Contract under test:
|
||||
|
||||
* anonymous → 403 ``{"detail": "admin only"}`` on GET, POST, and DELETE
|
||||
(phase 16 pattern, same as ``/api/sync``);
|
||||
* GET — empty table + env set → the env rows with ``from_env: true`` and
|
||||
null ``id``/``added_at``; empty table + empty env → ``sources: []``
|
||||
with ``from_env: true``; any DB rows → ``from_env: false`` and the env
|
||||
var is ignored (the phase's locked decision); DB rows ordered by
|
||||
``(added_at, id)``;
|
||||
* POST — 201 stored trimmed; duplicate (even with different surrounding
|
||||
whitespace) → 409 with a generic detail that never echoes the URL
|
||||
(credential safety), including when only the DB unique index catches
|
||||
it; bad shape / blank / >500 chars → 422, also input-free;
|
||||
* anonymous → 403 ``{"detail": "admin only"}`` on GET, POST (git and
|
||||
local), and DELETE (phase 16 pattern, same as ``/api/sync``);
|
||||
* GET — rows carry ``kind`` + ``path`` (phase 38); empty table + env
|
||||
set → the git-only env rows (``kind: "git"``, ``path: null``) with
|
||||
``from_env: true`` and null ``id``/``added_at``; empty table + empty
|
||||
env → ``sources: []`` with ``from_env: true``; any DB rows →
|
||||
``from_env: false`` and the env var is ignored (the phase's locked
|
||||
decision); DB rows ordered by ``(added_at, id)``;
|
||||
* POST ``kind=git`` (default) — 201 stored trimmed; duplicate (even with
|
||||
different surrounding whitespace) → 409 with a generic detail that
|
||||
never echoes the URL (credential safety), including when only the DB
|
||||
unique index catches it; bad shape / blank / >500 chars → 422, also
|
||||
input-free (the phase-35 contract, unchanged);
|
||||
* POST ``kind=local`` (phase 38) — existing directory → 201, stored row
|
||||
carries ``kind=local`` + the path expanded (``~`` resolved) and
|
||||
trimmed; relative / missing / not-a-directory path → 422 naming the
|
||||
path (not a secret); duplicate path → 409 naming the path (unique
|
||||
index as backstop); wrong field combinations (git without url, local
|
||||
without path, both kinds' fields) → 422;
|
||||
* DELETE — 204 and gone; an emptied table falls back to the env list
|
||||
again; unknown id → 404.
|
||||
|
||||
@@ -28,6 +37,7 @@ from __future__ import annotations
|
||||
import uuid
|
||||
from collections.abc import Iterator
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
@@ -66,6 +76,10 @@ def test_anonymous_gets_403_on_all_routes(client: TestClient, db: Session) -> No
|
||||
r = client.post("/api/git-sources", json={"url": "https://anon.example.com/x.git"})
|
||||
assert r.status_code == 403
|
||||
assert r.json() == {"detail": "admin only"}
|
||||
# The phase-38 local kind is gated the same way.
|
||||
r = client.post("/api/git-sources", json={"kind": "local", "path": "/tmp"})
|
||||
assert r.status_code == 403
|
||||
assert r.json() == {"detail": "admin only"}
|
||||
r = client.delete(f"/api/git-sources/{uuid.uuid4()}")
|
||||
assert r.status_code == 403
|
||||
assert r.json() == {"detail": "admin only"}
|
||||
@@ -89,10 +103,23 @@ def test_get_empty_table_with_env_returns_env_rows(
|
||||
assert r.status_code == 200
|
||||
body = r.json()
|
||||
assert body["from_env"] is True
|
||||
# Whitespace-trimmed, empty entries dropped, order preserved; null ids.
|
||||
# Whitespace-trimmed, empty entries dropped, order preserved; null
|
||||
# ids; the env fallback is git-only (phase 38: kind + path fields).
|
||||
assert body["sources"] == [
|
||||
{"id": None, "url": "https://a.example.com/one.git", "added_at": None},
|
||||
{"id": None, "url": "git@b.example.com:two.git", "added_at": None},
|
||||
{
|
||||
"id": None,
|
||||
"kind": "git",
|
||||
"url": "https://a.example.com/one.git",
|
||||
"path": None,
|
||||
"added_at": None,
|
||||
},
|
||||
{
|
||||
"id": None,
|
||||
"kind": "git",
|
||||
"url": "git@b.example.com:two.git",
|
||||
"path": None,
|
||||
"added_at": None,
|
||||
},
|
||||
]
|
||||
|
||||
|
||||
@@ -248,6 +275,200 @@ def test_post_rejects_blank_and_oversized_urls(admin_client: TestClient, db: Ses
|
||||
assert db.execute(text("SELECT count(*) FROM git_sources")).scalar_one() == 1
|
||||
|
||||
|
||||
# --- POST: local kind (phase 38, task 02) ----------------------------------
|
||||
|
||||
|
||||
def test_post_local_creates_stored_row_with_expanded_path(
|
||||
admin_client: TestClient,
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""``kind=local`` + an existing directory → 201; the stored row
|
||||
carries ``kind=local`` and the path expanded (``~`` resolved via the
|
||||
server's ``HOME``, whitespace trimmed)."""
|
||||
monkeypatch.setenv("HOME", str(tmp_path / "home"))
|
||||
real_dir = tmp_path / "home" / "notes"
|
||||
real_dir.mkdir(parents=True)
|
||||
|
||||
r = admin_client.post("/api/git-sources", json={"kind": "local", "path": " ~/notes\t"})
|
||||
assert r.status_code == 201, r.text
|
||||
body = r.json()
|
||||
# The phase-35 response shape is unchanged — the local row reports
|
||||
# its (expanded) path in ``url``; ``kind`` + ``path`` via GET.
|
||||
assert set(body) == {"id", "url", "added_at"}
|
||||
uuid.UUID(body["id"])
|
||||
assert body["url"] == str(real_dir)
|
||||
assert body["added_at"] is not None
|
||||
|
||||
row = admin_client.get("/api/git-sources").json()["sources"][0]
|
||||
assert row["kind"] == "local"
|
||||
assert row["path"] == str(real_dir)
|
||||
assert row["url"] == str(real_dir)
|
||||
assert row["id"] is not None
|
||||
assert row["added_at"] is not None
|
||||
|
||||
|
||||
def test_post_local_stores_trimmed_normalized_path(
|
||||
admin_client: TestClient, tmp_path: Path
|
||||
) -> None:
|
||||
"""Absolute path with surrounding whitespace + trailing slash →
|
||||
stored clean (trimmed, normalized)."""
|
||||
real_dir = tmp_path / "plain"
|
||||
real_dir.mkdir()
|
||||
|
||||
r = admin_client.post("/api/git-sources", json={"kind": "local", "path": f" {real_dir}/ "})
|
||||
assert r.status_code == 201, r.text
|
||||
row = admin_client.get("/api/git-sources").json()["sources"][0]
|
||||
assert row["path"] == str(real_dir)
|
||||
|
||||
|
||||
def test_post_local_relative_path_returns_422_naming_path(
|
||||
admin_client: TestClient, db: Session
|
||||
) -> None:
|
||||
"""A relative path fails loud at add-time — 422 naming the path
|
||||
(relative or not, it is never stored)."""
|
||||
r = admin_client.post("/api/git-sources", json={"kind": "local", "path": "relative/dir"})
|
||||
assert r.status_code == 422
|
||||
assert r.json()["detail"] == "local source path is not a directory: relative/dir"
|
||||
assert db.execute(text("SELECT count(*) FROM git_sources")).scalar_one() == 0
|
||||
|
||||
|
||||
def test_post_local_missing_path_returns_422_naming_path(
|
||||
admin_client: TestClient, db: Session
|
||||
) -> None:
|
||||
"""A missing (absolute) path is a user error → 422 naming the path so
|
||||
the owner sees exactly which directory failed."""
|
||||
missing = f"/nonexistent/bor-test-{uuid.uuid4()}"
|
||||
r = admin_client.post("/api/git-sources", json={"kind": "local", "path": missing})
|
||||
assert r.status_code == 422
|
||||
assert r.json()["detail"] == f"local source path is not a directory: {missing}"
|
||||
assert db.execute(text("SELECT count(*) FROM git_sources")).scalar_one() == 0
|
||||
|
||||
|
||||
def test_post_local_file_not_dir_returns_422(
|
||||
admin_client: TestClient, tmp_path: Path
|
||||
) -> None:
|
||||
"""An existing *file* is not a directory → 422 naming the path."""
|
||||
a_file = tmp_path / "a-file.md"
|
||||
a_file.write_text("not a directory")
|
||||
r = admin_client.post("/api/git-sources", json={"kind": "local", "path": str(a_file)})
|
||||
assert r.status_code == 422
|
||||
assert r.json()["detail"] == f"local source path is not a directory: {a_file}"
|
||||
|
||||
|
||||
def test_post_local_duplicate_path_returns_409_naming_path(
|
||||
admin_client: TestClient, tmp_path: Path
|
||||
) -> None:
|
||||
"""Duplicate path (even with different surrounding whitespace) → 409
|
||||
naming the path (a path is not a secret, unlike a git URL)."""
|
||||
real_dir = tmp_path / "dups"
|
||||
real_dir.mkdir()
|
||||
|
||||
assert admin_client.post("/api/git-sources", json={"kind": "local", "path": str(real_dir)})
|
||||
r = admin_client.post("/api/git-sources", json={"kind": "local", "path": f" {real_dir}\t"})
|
||||
assert r.status_code == 409
|
||||
detail = r.json()["detail"]
|
||||
assert detail == f"a local source with this path already exists: {real_dir}"
|
||||
# Exactly one row stored.
|
||||
assert len(admin_client.get("/api/git-sources").json()["sources"]) == 1
|
||||
|
||||
|
||||
def test_post_local_concurrent_insert_backstop_still_409(
|
||||
admin_client: TestClient, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""If the duplicate pre-check misses (a concurrent insert lands
|
||||
between the check and the commit), the DB unique index on ``path``
|
||||
still yields the 409 naming the path — never a 500."""
|
||||
real_dir = tmp_path / "backstop"
|
||||
real_dir.mkdir()
|
||||
assert (
|
||||
admin_client.post("/api/git-sources", json={"kind": "local", "path": str(real_dir)})
|
||||
.status_code
|
||||
== 201
|
||||
)
|
||||
|
||||
real_select = git_sources_api.select
|
||||
|
||||
def blind_select(*args: Any, **kwargs: Any) -> Any:
|
||||
if args and args[0] is GitSource: # the duplicate pre-check
|
||||
# …now never matches — only the unique index can catch it.
|
||||
return real_select(GitSource).where(GitSource.url == "zz-never-matches")
|
||||
return real_select(*args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(git_sources_api, "select", blind_select)
|
||||
r = admin_client.post("/api/git-sources", json={"kind": "local", "path": str(real_dir)})
|
||||
assert r.status_code == 409
|
||||
assert r.json()["detail"] == f"a local source with this path already exists: {real_dir}"
|
||||
|
||||
|
||||
def test_post_wrong_field_combinations_return_422(
|
||||
admin_client: TestClient, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
"""git without url, local without path, and both kinds' fields are
|
||||
422 with fixed details — nothing is stored."""
|
||||
real_dir = tmp_path / "combo"
|
||||
real_dir.mkdir()
|
||||
|
||||
assert admin_client.post("/api/git-sources", json={"kind": "git"}).status_code == 422
|
||||
r = admin_client.post(
|
||||
"/api/git-sources",
|
||||
json={"kind": "git", "url": "https://example.com/both.git", "path": str(real_dir)},
|
||||
)
|
||||
assert r.status_code == 422
|
||||
assert r.json()["detail"] == "a git source takes a url, not a path"
|
||||
assert admin_client.post("/api/git-sources", json={"kind": "local"}).status_code == 422
|
||||
# Whitespace-only path trims to empty → 422 as well (the schema's
|
||||
# min-length guard).
|
||||
assert (
|
||||
admin_client.post("/api/git-sources", json={"kind": "local", "path": " "}).status_code
|
||||
== 422
|
||||
)
|
||||
r = admin_client.post(
|
||||
"/api/git-sources",
|
||||
json={"kind": "local", "url": "https://example.com/both.git", "path": str(real_dir)},
|
||||
)
|
||||
assert r.status_code == 422
|
||||
assert r.json()["detail"] == "a local source takes a path, not a url"
|
||||
# Unknown kind and an oversized path are 422 too.
|
||||
assert (
|
||||
admin_client.post(
|
||||
"/api/git-sources", json={"kind": "svn", "url": "https://example.com/x.git"}
|
||||
).status_code
|
||||
== 422
|
||||
)
|
||||
assert (
|
||||
admin_client.post(
|
||||
"/api/git-sources", json={"kind": "local", "path": "/" + "x" * 2000}
|
||||
).status_code
|
||||
== 422
|
||||
)
|
||||
assert db.execute(text("SELECT count(*) FROM git_sources")).scalar_one() == 0
|
||||
|
||||
|
||||
def test_get_mixed_kinds_in_added_order(
|
||||
admin_client: TestClient, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
"""GET mixes git + local rows in ``(added_at, id)`` order; git rows
|
||||
report ``path: null``, local rows their stored path."""
|
||||
git_url = "https://example.com/mixed.git"
|
||||
db.add(GitSource(url=git_url, kind="git", added_at=datetime.now(UTC) - timedelta(hours=1)))
|
||||
db.commit()
|
||||
real_dir = tmp_path / "mixed"
|
||||
real_dir.mkdir()
|
||||
assert admin_client.post("/api/git-sources", json={"kind": "local", "path": str(real_dir)})
|
||||
|
||||
body = admin_client.get("/api/git-sources").json()
|
||||
assert body["from_env"] is False
|
||||
assert [s["url"] for s in body["sources"]] == [git_url, str(real_dir)]
|
||||
git_row, local_row = body["sources"]
|
||||
assert git_row["kind"] == "git"
|
||||
assert git_row["path"] is None
|
||||
assert git_row["id"] is not None
|
||||
assert local_row["kind"] == "local"
|
||||
assert local_row["path"] == str(real_dir)
|
||||
assert local_row["id"] is not None
|
||||
|
||||
|
||||
# --- DB rows win over env ---------------------------------------------------
|
||||
|
||||
|
||||
@@ -284,11 +505,18 @@ def test_delete_removes_row_and_falls_back_to_env(
|
||||
|
||||
assert admin_client.delete(f"/api/git-sources/{created.json()['id']}").status_code == 204
|
||||
|
||||
# The table is empty again → the env fallback is live once more.
|
||||
# The table is empty again → the env fallback is live once more
|
||||
# (git-only rows, phase 38).
|
||||
body = admin_client.get("/api/git-sources").json()
|
||||
assert body["from_env"] is True
|
||||
assert body["sources"] == [
|
||||
{"id": None, "url": "https://env.example.com/env.git", "added_at": None}
|
||||
{
|
||||
"id": None,
|
||||
"kind": "git",
|
||||
"url": "https://env.example.com/env.git",
|
||||
"path": None,
|
||||
"added_at": None,
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
|
||||
@@ -1,28 +1,36 @@
|
||||
"""Integration test: ``import_docs`` git-source resolution (phase 28, task 03).
|
||||
"""Integration test: ``import_docs`` source resolution (phase 28, task
|
||||
03; phase 35 re-points at the shared resolver; phase 38 adds the local
|
||||
kind).
|
||||
|
||||
Drives ``scripts.import_docs`` end to end with a fake ``clone_or_pull`` (no
|
||||
real git, no network) and a recording fake ``import_sources`` (no real
|
||||
DB), covering:
|
||||
Drives ``scripts.import_docs`` end to end with a fake ``clone_or_pull``
|
||||
(no real git, no network) and a recording fake ``import_sources`` (no
|
||||
real DB), covering:
|
||||
|
||||
- Effective git sources set (phase 35: the shared resolver — stubbed
|
||||
here, keeping this file's no-real-DB style) → each URL is cloned/pulled
|
||||
into ``BOR_SOURCES_DIR/<repo-name>/`` and exactly those dirs are
|
||||
imported.
|
||||
- DB rows win over ``BOR_GIT_SOURCES`` (the resolver's ``db`` origin —
|
||||
the env list is ignored).
|
||||
- ``--source`` still wins over git sources (no git at all, no resolver
|
||||
- Effective sources set (phase 35: the shared resolver — stubbed here,
|
||||
keeping this file's no-real-DB style) → each git URL is cloned/pulled
|
||||
into ``BOR_SOURCES_DIR/<repo-name>/``; local rows are their existing
|
||||
directories, walked directly; exactly those dirs are imported.
|
||||
- DB rows (both kinds) win over ``BOR_GIT_SOURCES`` (the resolver's
|
||||
``db`` origin — the env list is ignored; the env fallback stays
|
||||
git-only).
|
||||
- ``--source`` still wins over the DB rows (no git at all, no resolver
|
||||
call).
|
||||
- No git sources + no ``--source`` → the legacy ``DEFAULT_SOURCES``.
|
||||
- No sources configured + no ``--source`` → the legacy
|
||||
``DEFAULT_SOURCES``.
|
||||
- A failing git sync → exit code 1, an error naming the failing repo on
|
||||
stderr, and **zero** import attempts.
|
||||
- A missing local directory → the same pre-import fail-loud: exit code
|
||||
1, ``local source missing: <path>`` on stderr, zero import attempts.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
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
|
||||
@@ -33,6 +41,16 @@ def _settings(git_sources: str = "", sources_dir: str = "~/bor-sources") -> Sett
|
||||
return Settings(_env_file=None, git_sources=git_sources, sources_dir=sources_dir) # pyright: ignore[reportCallIssue]
|
||||
|
||||
|
||||
def _git_row(url: str) -> GitSource:
|
||||
return GitSource(url=url, kind="git")
|
||||
|
||||
|
||||
def _local_row(path: str) -> GitSource:
|
||||
"""A local row as the phase-38 API stores it: the expanded path in
|
||||
both ``path`` and the NOT-NULL ``url`` location column."""
|
||||
return GitSource(url=path, kind="local", path=path)
|
||||
|
||||
|
||||
class FakeImportSources:
|
||||
"""Records every ``import_sources`` call instead of touching a DB."""
|
||||
|
||||
@@ -97,11 +115,15 @@ def test_resolve_sources_git_urls_cloned_into_sources_dir(
|
||||
calls, fake = _fake_clone_factory()
|
||||
monkeypatch.setattr(import_docs, "clone_or_pull", fake)
|
||||
# Phase 35: resolution goes through the shared resolver (stubbed —
|
||||
# this file keeps its no-real-DB style); the URLs are the env list.
|
||||
# this file keeps its no-real-DB style); the rows are the env list,
|
||||
# surfaced as synthetic git rows.
|
||||
monkeypatch.setattr(
|
||||
import_docs,
|
||||
"effective_git_sources",
|
||||
lambda db: (["https://host/a/homelab.git", "git@host:user/deploy.git"], "env"),
|
||||
"effective_sources",
|
||||
lambda db: (
|
||||
[_git_row("https://host/a/homelab.git"), _git_row("git@host:user/deploy.git")],
|
||||
"env",
|
||||
),
|
||||
)
|
||||
settings = _settings(
|
||||
git_sources="https://host/a/homelab.git, git@host:user/deploy.git ,",
|
||||
@@ -140,8 +162,8 @@ def test_resolve_sources_db_rows_win_over_env(
|
||||
monkeypatch.setattr(import_docs, "clone_or_pull", fake)
|
||||
monkeypatch.setattr(
|
||||
import_docs,
|
||||
"effective_git_sources",
|
||||
lambda db: (["https://db.example/only.git"], "db"),
|
||||
"effective_sources",
|
||||
lambda db: ([_git_row("https://db.example/only.git")], "db"),
|
||||
)
|
||||
settings = _settings(
|
||||
git_sources="https://env.example/ignored.git",
|
||||
@@ -158,7 +180,7 @@ def test_resolve_sources_defaults_when_nothing_configured(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
# Both origins empty (the resolver's ``([], "env")``) → legacy dirs.
|
||||
monkeypatch.setattr(import_docs, "effective_git_sources", lambda db: ([], "env"))
|
||||
monkeypatch.setattr(import_docs, "effective_sources", lambda db: ([], "env"))
|
||||
sources = import_docs._resolve_sources(None, _settings())
|
||||
assert sources == [p.expanduser() for p in import_docs.DEFAULT_SOURCES]
|
||||
|
||||
@@ -178,8 +200,11 @@ def test_main_git_sources_clone_then_import(
|
||||
monkeypatch.setattr(import_docs, "get_settings", lambda: settings)
|
||||
monkeypatch.setattr(
|
||||
import_docs,
|
||||
"effective_git_sources",
|
||||
lambda db: (["https://host/a/homelab.git", "https://host/a/deploy.git"], "env"),
|
||||
"effective_sources",
|
||||
lambda db: (
|
||||
[_git_row("https://host/a/homelab.git"), _git_row("https://host/a/deploy.git")],
|
||||
"env",
|
||||
),
|
||||
)
|
||||
calls, fake = _fake_clone_factory()
|
||||
monkeypatch.setattr(import_docs, "clone_or_pull", fake)
|
||||
@@ -227,6 +252,55 @@ def test_main_cli_source_still_imports_manual_dir(
|
||||
assert fake_import.calls[0]["prune"] is False
|
||||
|
||||
|
||||
def test_resolve_sources_mixed_git_and_local(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
"""Phase 38: DB rows of both kinds — the git row is cloned into
|
||||
``BOR_SOURCES_DIR``, the local row is its existing directory itself
|
||||
(no clone), in row order; the env list is ignored."""
|
||||
calls, fake = _fake_clone_factory()
|
||||
monkeypatch.setattr(import_docs, "clone_or_pull", fake)
|
||||
local_dir = tmp_path / "LocalDocs"
|
||||
local_dir.mkdir()
|
||||
(local_dir / "a.md").write_text("# A\nlocal fixture\n", encoding="utf-8")
|
||||
monkeypatch.setattr(
|
||||
import_docs,
|
||||
"effective_sources",
|
||||
lambda db: (
|
||||
[_git_row("https://db.example/only.git"), _local_row(str(local_dir))],
|
||||
"db",
|
||||
),
|
||||
)
|
||||
settings = _settings(
|
||||
git_sources="https://env.example/ignored.git",
|
||||
sources_dir=str(tmp_path / "bor"),
|
||||
)
|
||||
|
||||
sources = import_docs._resolve_sources(None, settings)
|
||||
|
||||
assert sources == [tmp_path / "bor" / "only", local_dir]
|
||||
assert calls == [("https://db.example/only.git", tmp_path / "bor" / "only")]
|
||||
|
||||
|
||||
def test_resolve_sources_missing_local_dir_aborts(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
"""Phase 38: a local row whose directory is gone at run time →
|
||||
``GitSyncError`` naming the path, before any import (the same
|
||||
pre-import fail-loud as a failing git clone)."""
|
||||
monkeypatch.setattr(import_docs, "clone_or_pull", _fake_clone_factory()[1])
|
||||
missing = tmp_path / "Gone"
|
||||
monkeypatch.setattr(
|
||||
import_docs,
|
||||
"effective_sources",
|
||||
lambda db: ([_local_row(str(missing))], "db"),
|
||||
)
|
||||
settings = _settings(sources_dir=str(tmp_path / "bor"))
|
||||
|
||||
with pytest.raises(GitSyncError, match=f"local source missing: {re.escape(str(missing))}"):
|
||||
import_docs._resolve_sources(None, settings)
|
||||
|
||||
|
||||
def test_main_git_failure_aborts_before_import(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
@@ -236,7 +310,9 @@ def test_main_git_failure_aborts_before_import(
|
||||
)
|
||||
monkeypatch.setattr(import_docs, "get_settings", lambda: settings)
|
||||
monkeypatch.setattr(
|
||||
import_docs, "effective_git_sources", lambda db: (["https://host/a/bad.git"], "env")
|
||||
import_docs,
|
||||
"effective_sources",
|
||||
lambda db: ([_git_row("https://host/a/bad.git")], "env"),
|
||||
)
|
||||
|
||||
def failing_clone(url: str, dest: Path | str) -> Path:
|
||||
@@ -253,7 +329,34 @@ def test_main_git_failure_aborts_before_import(
|
||||
|
||||
assert rc == 1
|
||||
err = capsys.readouterr().err
|
||||
assert "import_docs: git sync failed" in err
|
||||
assert "import_docs: source sync failed" in err
|
||||
assert "bad.git" in err # the failing repo is named
|
||||
assert fake_import.calls == [] # no partial import
|
||||
assert not (tmp_path / "bor").exists()
|
||||
|
||||
|
||||
def test_main_missing_local_dir_aborts_before_import(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
"""Phase 38: a DB local row whose directory is missing → exit code
|
||||
1, ``local source missing: <path>`` on stderr, zero import attempts
|
||||
(no ``--source`` given, so the DB row is what should have been
|
||||
imported)."""
|
||||
settings = _settings(sources_dir=str(tmp_path / "bor"))
|
||||
monkeypatch.setattr(import_docs, "get_settings", lambda: settings)
|
||||
missing = tmp_path / "Gone"
|
||||
monkeypatch.setattr(
|
||||
import_docs,
|
||||
"effective_sources",
|
||||
lambda db: ([_local_row(str(missing))], "db"),
|
||||
)
|
||||
fake_import = FakeImportSources()
|
||||
monkeypatch.setattr(import_docs, "import_sources", fake_import)
|
||||
|
||||
rc = import_docs.main([])
|
||||
|
||||
assert rc == 1
|
||||
err = capsys.readouterr().err
|
||||
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
|
||||
|
||||
@@ -0,0 +1,261 @@
|
||||
"""Integration: migration 0007 (git_sources.kind + path) schema contract.
|
||||
|
||||
Drives the **real Alembic engine** against the live dev database
|
||||
(``podman compose up -d db``), mirroring the style of
|
||||
``test_migration_0004.py`` / ``test_migration_0006.py``
|
||||
(information_schema assertions on the state the migration must leave).
|
||||
The tests target revision ``0007`` explicitly so later migrations
|
||||
cannot break them:
|
||||
|
||||
* upgrade 0006 → 0007 → ``git_sources`` gains ``kind TEXT NOT NULL``
|
||||
(server default ``'git'``, check constraint ``ck_git_sources_kind``:
|
||||
``kind IN ('git', 'local')``) and ``path TEXT`` (nullable) with the
|
||||
unique index ``uq_git_sources_path``; a row inserted before the
|
||||
upgrade (the pre-0007 insert shape) keeps ``kind='git'`` /
|
||||
``path=NULL`` after it;
|
||||
* the check constraint rejects any kind other than ``git``/``local``;
|
||||
* the unique index rejects duplicate local paths but tolerates NULL
|
||||
paths (git rows);
|
||||
* downgrade to 0006 → both columns, the constraint, and the index are
|
||||
gone;
|
||||
* upgrade back to 0007 → they are 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
|
||||
|
||||
from collections.abc import Iterator
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from alembic.config import Config
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from alembic import command
|
||||
from app.db import db_available
|
||||
|
||||
URL_BASE = "https://git.example.com/mig0007"
|
||||
PATH_BASE = "/tmp/brain-of-reese-mig0007"
|
||||
|
||||
|
||||
@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:
|
||||
command.upgrade(cfg, "head")
|
||||
|
||||
|
||||
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 _version(db: Session) -> str | None:
|
||||
return db.execute(text("SELECT version_num FROM alembic_version")).scalar()
|
||||
|
||||
|
||||
def _check_constraint_def(db: Session) -> str | None:
|
||||
"""Definition of ``ck_git_sources_kind``, or None if it does not exist.
|
||||
|
||||
Only call while ``git_sources`` exists (the ``::regclass`` cast errors
|
||||
otherwise).
|
||||
"""
|
||||
row = db.execute(
|
||||
text(
|
||||
"SELECT pg_get_constraintdef(oid) FROM pg_constraint"
|
||||
" WHERE conname = 'ck_git_sources_kind'"
|
||||
" AND conrelid = 'git_sources'::regclass"
|
||||
)
|
||||
).fetchone()
|
||||
return row[0] if row is not None else None
|
||||
|
||||
|
||||
def _unique_path_index(db: Session) -> int:
|
||||
"""1 iff ``uq_git_sources_path`` exists as a UNIQUE index."""
|
||||
count: Any = db.execute(
|
||||
text(
|
||||
"SELECT count(*) FROM pg_indexes"
|
||||
" WHERE tablename = 'git_sources' AND indexname = 'uq_git_sources_path'"
|
||||
" AND indexdef ILIKE 'CREATE UNIQUE%'"
|
||||
)
|
||||
).scalar()
|
||||
assert count is not None, "pg_indexes count must be an int"
|
||||
return int(count)
|
||||
|
||||
|
||||
def _insert(db: Session, url: str, kind: str | None = None, path: str | None = None) -> None:
|
||||
"""Insert one git_sources row; kind/path omitted → pre-0007 shape."""
|
||||
if kind is None and path is None:
|
||||
db.execute(
|
||||
text("INSERT INTO git_sources (id, url) VALUES (gen_random_uuid(), :u)"),
|
||||
{"u": url},
|
||||
)
|
||||
else:
|
||||
db.execute(
|
||||
text(
|
||||
"INSERT INTO git_sources (id, url, kind, path)"
|
||||
" VALUES (gen_random_uuid(), :u, :k, :p)"
|
||||
),
|
||||
{"u": url, "k": kind, "p": path},
|
||||
)
|
||||
db.commit()
|
||||
|
||||
|
||||
def _delete_by_url(db: Session, url: str) -> None:
|
||||
db.execute(text("DELETE FROM git_sources WHERE url = :u"), {"u": url})
|
||||
db.commit()
|
||||
|
||||
|
||||
def test_upgrade_to_0007_adds_kind_and_path(db: Session, alembic: Config) -> None:
|
||||
"""Upgrade 0006 → 0007: both columns exist with the locked types,
|
||||
nullability, and defaults, plus the CHECK constraint and the unique
|
||||
path index."""
|
||||
command.downgrade(alembic, "0006") # start from the pre-0007 state
|
||||
assert _version(db) == "0006"
|
||||
|
||||
command.upgrade(alembic, "0007")
|
||||
assert _version(db) == "0007", "alembic_version must be at 0007"
|
||||
|
||||
kind = _column(db, "kind")
|
||||
assert kind is not None, "git_sources.kind is missing"
|
||||
assert kind[0] == "text", "git_sources.kind must be TEXT"
|
||||
assert kind[1] == "NO", "git_sources.kind must be NOT NULL"
|
||||
assert kind[2] is not None and "'git'" in kind[2], (
|
||||
"git_sources.kind must have server default 'git'"
|
||||
)
|
||||
|
||||
path = _column(db, "path")
|
||||
assert path is not None, "git_sources.path is missing"
|
||||
assert path[0] == "text", "git_sources.path must be TEXT"
|
||||
assert path[1] == "YES", "git_sources.path must be NULLABLE"
|
||||
|
||||
constraint = _check_constraint_def(db)
|
||||
assert constraint is not None, "ck_git_sources_kind is missing"
|
||||
assert "git" in constraint and "local" in constraint, (
|
||||
f"ck_git_sources_kind must restrict kind to git|local, got: {constraint}"
|
||||
)
|
||||
|
||||
assert _unique_path_index(db) == 1, "uq_git_sources_path unique index is missing"
|
||||
|
||||
|
||||
def test_pre_0007_row_reads_as_git(db: Session, alembic: Config) -> None:
|
||||
"""A row inserted before the upgrade (url only — the pre-0007 insert
|
||||
shape) reads as ``kind='git'``, ``path=NULL`` after it."""
|
||||
command.downgrade(alembic, "0006")
|
||||
url = f"{URL_BASE}/pre-existing.git"
|
||||
_insert(db, url) # no kind/path columns exist at 0006
|
||||
try:
|
||||
command.upgrade(alembic, "0007")
|
||||
kind, path = db.execute(
|
||||
text("SELECT kind, path FROM git_sources WHERE url = :u"), {"u": url}
|
||||
).one()
|
||||
assert kind == "git", "a pre-0007 row must read as kind='git'"
|
||||
assert path is None, "a pre-0007 row must keep path=NULL"
|
||||
finally:
|
||||
_delete_by_url(db, url)
|
||||
|
||||
|
||||
def test_kind_defaults_to_git_for_new_inserts(db: Session, alembic: Config) -> None:
|
||||
"""An insert that omits kind (the API's pre-phase-38 shape) lands as
|
||||
``kind='git'`` via the server default."""
|
||||
command.upgrade(alembic, "head")
|
||||
url = f"{URL_BASE}/default-kind.git"
|
||||
_insert(db, url)
|
||||
try:
|
||||
kind, path = db.execute(
|
||||
text("SELECT kind, path FROM git_sources WHERE url = :u"), {"u": url}
|
||||
).one()
|
||||
assert kind == "git", "git_sources.kind must default to 'git'"
|
||||
assert path is None, "git_sources.path must default to NULL"
|
||||
finally:
|
||||
_delete_by_url(db, url)
|
||||
|
||||
|
||||
def test_check_constraint_rejects_unknown_kind(db: Session, alembic: Config) -> None:
|
||||
"""``ck_git_sources_kind`` is what later API validation relies on:
|
||||
any kind other than git|local raises IntegrityError."""
|
||||
command.upgrade(alembic, "head")
|
||||
with pytest.raises(IntegrityError):
|
||||
_insert(db, f"{URL_BASE}/bogus-kind.git", kind="bogus")
|
||||
db.rollback() # the IntegrityError aborts the open transaction
|
||||
|
||||
|
||||
def test_duplicate_local_path_rejected(db: Session, alembic: Config) -> None:
|
||||
"""The unique path index is what the API's 409 relies on: two local
|
||||
rows with the same path are rejected (NULL paths stay distinct —
|
||||
git rows are unaffected)."""
|
||||
command.upgrade(alembic, "head")
|
||||
url_a = f"{URL_BASE}/dup-path-a.git"
|
||||
url_b = f"{URL_BASE}/dup-path-b.git"
|
||||
url_c = f"{URL_BASE}/dup-path-c.git"
|
||||
path = f"{PATH_BASE}/shared"
|
||||
try:
|
||||
_insert(db, url_a, kind="local", path=path)
|
||||
with pytest.raises(IntegrityError):
|
||||
_insert(db, url_b, kind="local", path=path)
|
||||
db.rollback()
|
||||
# NULL paths are distinct under the unique index (git rows).
|
||||
_insert(db, url_b)
|
||||
_insert(db, url_c)
|
||||
finally:
|
||||
db.rollback()
|
||||
_delete_by_url(db, url_a)
|
||||
_delete_by_url(db, url_b)
|
||||
_delete_by_url(db, url_c)
|
||||
|
||||
|
||||
def test_downgrade_to_0006_drops_columns(db: Session, alembic: Config) -> None:
|
||||
"""Downgrade to 0006: both columns, the CHECK constraint, and the
|
||||
unique index are dropped (A13 — reversible)."""
|
||||
command.downgrade(alembic, "0006")
|
||||
assert _version(db) == "0006"
|
||||
|
||||
assert _column(db, "kind") is None, "git_sources.kind must be dropped"
|
||||
assert _column(db, "path") is None, "git_sources.path must be dropped"
|
||||
assert _check_constraint_def(db) is None, "ck_git_sources_kind must be dropped"
|
||||
assert _unique_path_index(db) == 0, "uq_git_sources_path must be dropped"
|
||||
|
||||
|
||||
def test_upgrade_round_trip_restores_columns(db: Session, alembic: Config) -> None:
|
||||
"""Downgrade to 0006, then upgrade back to 0007: columns, default,
|
||||
constraint, and index are back."""
|
||||
command.downgrade(alembic, "0006")
|
||||
command.upgrade(alembic, "0007")
|
||||
assert _version(db) == "0007", "round-trip upgrade must land at 0007"
|
||||
|
||||
kind = _column(db, "kind")
|
||||
assert kind is not None, "git_sources.kind must be back"
|
||||
assert kind[1] == "NO" and kind[2] is not None and "'git'" in kind[2], (
|
||||
"git_sources.kind must keep its NOT NULL 'git' default after the round-trip"
|
||||
)
|
||||
|
||||
path = _column(db, "path")
|
||||
assert path is not None and path[1] == "YES", "git_sources.path must be back"
|
||||
|
||||
constraint = _check_constraint_def(db)
|
||||
assert constraint is not None, "ck_git_sources_kind must be back"
|
||||
|
||||
assert _unique_path_index(db) == 1, "uq_git_sources_path must be back"
|
||||
@@ -1,20 +1,31 @@
|
||||
"""Integration: the admin sources-sync API (phase 32, task 01; phase 35,
|
||||
task 03 re-points the URL resolution at the shared resolver).
|
||||
task 03 re-points the URL resolution at the shared resolver; phase 38,
|
||||
task 03 adds the local kind).
|
||||
|
||||
Covers the in-process sync runner end to end over HTTP: anonymous 403s
|
||||
on both endpoints; admin idle → 202 → ``success`` with the full
|
||||
ImportSummary detail; 409 on a double trigger while a run is in flight;
|
||||
``GitSyncError`` → ``failed`` with the failing repo named and **zero**
|
||||
import attempts; empty on *both* origins (``git_sources`` table +
|
||||
``BOR_GIT_SOURCES``) → ``failed`` loudly; an embedding failure →
|
||||
``failed`` with any credentials masked; the import always runs with
|
||||
``prune=True``; and the phase-31 overview trigger is change-gated (no
|
||||
``lite`` call on an unchanged KB).
|
||||
import attempts; empty on *both* origins (no git rows, no local rows,
|
||||
no env URLs) → ``failed`` loudly (``no sources configured
|
||||
(git or local)``); an embedding failure → ``failed`` with any
|
||||
credentials masked; the import always runs with ``prune=True``; and the
|
||||
phase-31 overview trigger is change-gated (no ``lite`` call on an
|
||||
unchanged KB).
|
||||
|
||||
Phase 35: the runner resolves the repos through
|
||||
:func:`app.rag.git_sources.effective_git_sources` — the **real**
|
||||
resolver against the **real** ``git_sources`` table (truncated around
|
||||
every test), so DB-over-env and the env fallback go through the actual
|
||||
Phase 38 (local kind): local-only, git-only, and mixed syncs over a
|
||||
**host temp local dir** (the app server runs on the same host) — the
|
||||
mixed run goes through the **real** ``import_sources`` (deterministic
|
||||
in-process ``FakeEmbedder``, no network), so the local file verifiably
|
||||
lands in the KB via ``GET /api/docs`` and union pruning holds (a file
|
||||
deleted out of the local dir is pruned on the next sync while the git
|
||||
doc survives); a local directory missing at sync time → ``failed`` with
|
||||
``local source missing: <path>`` and **zero** import attempts.
|
||||
|
||||
Phase 35: the runner resolves the sources through
|
||||
:func:`app.rag.git_sources.effective_sources` — the **real** resolver
|
||||
against the **real** ``git_sources`` table (truncated around every
|
||||
test), so DB-over-env and the env fallback go through the actual
|
||||
indirection; the env list is driven by a fresh ``Settings`` on the
|
||||
resolver's module (the dev ``.env`` never leaks in).
|
||||
|
||||
@@ -34,7 +45,7 @@ import asyncio
|
||||
import logging
|
||||
import time
|
||||
from collections.abc import Iterator
|
||||
from datetime import datetime
|
||||
from datetime import UTC, datetime
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -51,6 +62,7 @@ from app.rag.importer import ImportSummary
|
||||
from app.rag.llm import EmbeddingError, LLMClient
|
||||
from scripts.git_sync import GitSyncError
|
||||
from tests.conftest import ADMIN_PASSWORD
|
||||
from tests.fakes import FakeEmbedder
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
@@ -110,6 +122,30 @@ def _seed(db: Session, url: str) -> None:
|
||||
db.commit()
|
||||
|
||||
|
||||
def _seed_local(db: Session, path: Path) -> None:
|
||||
"""A ``kind=local`` row as the phase-38 API stores it: the expanded
|
||||
absolute path in both ``path`` and the NOT-NULL ``url`` column."""
|
||||
db.add(GitSource(url=str(path), kind="local", path=str(path)))
|
||||
db.commit()
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def clean_documents(db: Session) -> Iterator[None]:
|
||||
"""The real-import tests write ``documents``/``chunks`` (the canonical
|
||||
KB state) — global, truncated around every such test."""
|
||||
db.execute(text("TRUNCATE chunks, documents"))
|
||||
db.commit()
|
||||
yield
|
||||
db.execute(text("TRUNCATE chunks, documents"))
|
||||
db.commit()
|
||||
|
||||
|
||||
def _real_llm(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""The pipeline's ``LLMClient`` becomes the deterministic in-process
|
||||
``FakeEmbedder`` (real import, no network)."""
|
||||
monkeypatch.setattr(sync_api, "LLMClient", lambda: FakeEmbedder())
|
||||
|
||||
|
||||
def _login(client: TestClient) -> None:
|
||||
r = client.post("/api/login", json={"password": ADMIN_PASSWORD})
|
||||
assert r.status_code == 204, f"admin login failed: {r.status_code} {r.text}"
|
||||
@@ -372,11 +408,12 @@ def test_git_failure_marks_failed_and_skips_import(
|
||||
_poll(sync_client, "failed")
|
||||
|
||||
|
||||
def test_no_git_sources_configured_fails_loudly(
|
||||
def test_no_sources_configured_fails_loudly(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
"""Both origins empty (the truncate fixture + a blank env) → the
|
||||
fail-loud error names *both* (phase 35)."""
|
||||
"""Both origins empty — no git rows, no local rows, no env URLs
|
||||
(the truncate fixture + a blank env) → the fail-loud error
|
||||
(phase 38: git-only message retired)."""
|
||||
# Whitespace-only is just as unconfigured as empty.
|
||||
_stub_env(monkeypatch, " , ")
|
||||
monkeypatch.setattr(
|
||||
@@ -393,9 +430,7 @@ def test_no_git_sources_configured_fails_loudly(
|
||||
assert sync_client.post("/api/sync").status_code == 202
|
||||
|
||||
body = _poll(sync_client, "failed")
|
||||
assert body["error"] == (
|
||||
"no git sources configured (git_sources table empty and BOR_GIT_SOURCES unset)"
|
||||
)
|
||||
assert body["error"] == "no sources configured (git or local)"
|
||||
assert clone_calls == [] # git is never touched
|
||||
assert fake_import.sources == []
|
||||
|
||||
@@ -464,6 +499,141 @@ def test_env_fallback_when_table_empty(
|
||||
assert any("sync: started repos=1 origin=env" in r.getMessage() for r in caplog.records)
|
||||
|
||||
|
||||
# --- phase 38: the local kind (real import, host temp dirs) ---------------
|
||||
|
||||
|
||||
def test_local_only_sync_imports_dir(
|
||||
sync_client: TestClient,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
db: Session,
|
||||
tmp_path: Path,
|
||||
clean_documents: None,
|
||||
) -> None:
|
||||
"""Local-only config: the host temp dir (one fixture ``.md``) is
|
||||
walked directly — no clone at all — and the file lands in the KB
|
||||
(``GET /api/docs`` as admin)."""
|
||||
local_dir = tmp_path / "LocalDocs"
|
||||
local_dir.mkdir()
|
||||
(local_dir / "notes.md").write_text("# Local Notes\nthe local fixture\n", encoding="utf-8")
|
||||
_seed_local(db, local_dir)
|
||||
_stub_env(monkeypatch) # env must not matter once the table has a row
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
clone_calls, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
_real_llm(monkeypatch) # real import_sources, deterministic embeddings
|
||||
|
||||
_login(sync_client)
|
||||
assert sync_client.post("/api/sync").status_code == 202
|
||||
body = _poll(sync_client, "success")
|
||||
|
||||
assert clone_calls == [] # nothing to clone — local is walked directly
|
||||
assert body["detail"]["added"] == 1
|
||||
assert body["detail"]["errors"] == 0
|
||||
# The local file is in the KB, sourced by the directory's basename.
|
||||
docs = sync_client.get("/api/docs").json()["documents"]
|
||||
assert [(d["source"], d["path"]) for d in docs] == [("LocalDocs", "notes.md")]
|
||||
|
||||
|
||||
def test_mixed_git_local_sync_imports_both_and_prunes_union(
|
||||
sync_client: TestClient,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
db: Session,
|
||||
tmp_path: Path,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
clean_documents: None,
|
||||
) -> None:
|
||||
"""Mixed config: the git row is cloned, the local dir walked, and
|
||||
both are imported in one run over the single combined list. The
|
||||
started log carries the kind counts; a file deleted out of the
|
||||
local dir is pruned on the next sync (union prune) while the git
|
||||
doc survives."""
|
||||
repo_url = f"file://{tmp_path / 'repo.git'}"
|
||||
_seed(db, repo_url)
|
||||
local_dir = tmp_path / "LocalDocs"
|
||||
local_dir.mkdir()
|
||||
(local_dir / "a.md").write_text("# A\nfirst local file\n", encoding="utf-8")
|
||||
(local_dir / "b.md").write_text("# B\nsecond local file\n", encoding="utf-8")
|
||||
_seed_local(db, local_dir)
|
||||
_stub_env(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
clone_calls, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
_real_llm(monkeypatch)
|
||||
|
||||
_login(sync_client)
|
||||
with caplog.at_level(logging.INFO, logger="app.api.sync"):
|
||||
assert sync_client.post("/api/sync").status_code == 202
|
||||
body = _poll(sync_client, "success")
|
||||
|
||||
# Git cloned into BOR_SOURCES_DIR, local dir walked in row order.
|
||||
assert clone_calls == [(repo_url, tmp_path / "bor" / "repo")]
|
||||
assert body["detail"]["added"] == 3
|
||||
assert any(
|
||||
"sync: started repos=2 origin=db git=1 local=1" in r.getMessage() for r in caplog.records
|
||||
)
|
||||
docs = {(d["source"], d["path"]) for d in sync_client.get("/api/docs").json()["documents"]}
|
||||
assert docs == {("repo", "notes.md"), ("LocalDocs", "a.md"), ("LocalDocs", "b.md")}
|
||||
|
||||
# Union prune: delete one local file → the next sync prunes exactly
|
||||
# it; the git doc (and the surviving local file) stay.
|
||||
(local_dir / "b.md").unlink()
|
||||
assert sync_client.post("/api/sync").status_code == 202
|
||||
body = _poll(sync_client, "success")
|
||||
assert body["detail"]["pruned"] == 1
|
||||
assert body["detail"]["added"] == 0
|
||||
docs = {(d["source"], d["path"]) for d in sync_client.get("/api/docs").json()["documents"]}
|
||||
assert docs == {("repo", "notes.md"), ("LocalDocs", "a.md")}
|
||||
|
||||
|
||||
def test_missing_local_dir_fails_loudly_and_imports_nothing(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
"""A local row whose directory is gone at sync time (moved/deleted
|
||||
since add-time) → ``failed`` naming the path, **zero** import
|
||||
attempts — the git row before it in row order was still cloned
|
||||
(per-row walk; a clone is not an import)."""
|
||||
repo_url = f"file://{tmp_path / 'repo.git'}"
|
||||
missing = tmp_path / "Gone"
|
||||
db.add(
|
||||
GitSource(url=repo_url, kind="git", added_at=datetime(2026, 1, 1, tzinfo=UTC))
|
||||
)
|
||||
db.add(
|
||||
GitSource(url=str(missing), kind="local", path=str(missing),
|
||||
added_at=datetime(2026, 1, 2, tzinfo=UTC))
|
||||
)
|
||||
db.commit()
|
||||
_stub_env(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
clone_calls, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
fake_import = FakeImportSources(ImportSummary())
|
||||
monkeypatch.setattr(sync_api, "import_sources", fake_import)
|
||||
fake_overview = FakeOverview(ok=True)
|
||||
monkeypatch.setattr(sync_api, "regenerate_overview", fake_overview)
|
||||
|
||||
_login(sync_client)
|
||||
assert sync_client.post("/api/sync").status_code == 202
|
||||
|
||||
body = _poll(sync_client, "failed")
|
||||
assert f"local source missing: {missing}" in body["error"] # the path is named
|
||||
assert body["detail"] == {}
|
||||
assert clone_calls == [(repo_url, tmp_path / "bor" / "repo")] # git row walked first
|
||||
assert fake_import.sources == [] # no partial import
|
||||
assert fake_overview.llms == []
|
||||
|
||||
|
||||
def test_import_error_is_reported_with_credentials_masked(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
|
||||
@@ -1,12 +1,16 @@
|
||||
"""Unit: the shared git-source resolver (phase 35, task 03).
|
||||
"""Unit: the shared source resolver (phase 35, task 03; local kind,
|
||||
phase 38, task 03).
|
||||
|
||||
``effective_git_sources`` is driven with a stubbed session (no Postgres)
|
||||
``effective_sources`` is driven with a stubbed session (no Postgres)
|
||||
and a fresh ``Settings(_env_file=None)`` env (monkeypatched into the
|
||||
resolver's module — the dev ``.env`` never leaks in, same pattern as the
|
||||
integration suites): DB rows win in ``(added_at, id)`` order (env
|
||||
ignored), the ``BOR_GIT_SOURCES`` list is a fallback only while the
|
||||
table is empty (phase-28 CSV parse reused), and both-empty yields
|
||||
``([], "env")`` so the callers keep their fail-loud behavior.
|
||||
resolver's module — the dev ``.env`` never leaks in, same pattern as
|
||||
the integration suites): DB rows of **both kinds** win in
|
||||
``(added_at, id)`` order (env ignored), the ``BOR_GIT_SOURCES`` list is
|
||||
a git-only fallback while the table is empty (surfaced as synthetic
|
||||
``kind='git'`` rows, phase-28 CSV parse reused), and both-empty yields
|
||||
``([], "env")`` so the callers keep their fail-loud behavior. The
|
||||
phase-35 ``effective_git_sources`` alias is kept covered as well: it
|
||||
returns the repo URLs of the git rows only, same origin.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -52,33 +56,70 @@ def _stub_env(monkeypatch: pytest.MonkeyPatch, git_sources: str) -> None:
|
||||
)
|
||||
|
||||
|
||||
def _git(url: str) -> GitSource:
|
||||
return GitSource(url=url, kind="git")
|
||||
|
||||
|
||||
def _local(path: str) -> GitSource:
|
||||
"""A local row as phase-38 task 02 stores it: the expanded path in
|
||||
both ``path`` and the NOT-NULL ``url`` location column."""
|
||||
return GitSource(url=path, kind="local", path=path)
|
||||
|
||||
|
||||
# --- the three branches ----------------------------------------------------
|
||||
|
||||
|
||||
def test_db_rows_win_over_env(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Seeded table + env set → the DB list, origin ``db`` (env ignored)."""
|
||||
"""Seeded table + env set → the DB rows, origin ``db`` (env ignored)."""
|
||||
_stub_env(monkeypatch, "https://env.example/ignored.git")
|
||||
session = _FakeSession([GitSource(url="https://db.example/a.git"), GitSource(url="https://db.example/b.git")])
|
||||
session = _FakeSession(
|
||||
[GitSource(url="https://db.example/a.git"), GitSource(url="https://db.example/b.git")]
|
||||
)
|
||||
|
||||
urls, origin = resolver.effective_git_sources(session) # pyright: ignore[reportArgumentType]
|
||||
rows, origin = resolver.effective_sources(session) # pyright: ignore[reportArgumentType]
|
||||
|
||||
assert urls == ["https://db.example/a.git", "https://db.example/b.git"]
|
||||
assert [row.url for row in rows] == ["https://db.example/a.git", "https://db.example/b.git"]
|
||||
assert origin == "db"
|
||||
|
||||
|
||||
def test_env_fallback_while_table_empty(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Empty table + env set → the env list, origin ``env``.
|
||||
def test_mixed_kinds_returned_in_row_order(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Phase 38: the table holds git **and** local rows — both come back,
|
||||
in ``(added_at, id)`` row order, kinds and paths intact, env ignored."""
|
||||
_stub_env(monkeypatch, "https://env.example/ignored.git")
|
||||
local = "/abs/notes"
|
||||
session = _FakeSession(
|
||||
[_git("https://db.example/a.git"), _local(local), _git("git@db.example:b.git")]
|
||||
)
|
||||
|
||||
The CSV parse is ``Settings.git_source_list`` itself (phase 28):
|
||||
rows, origin = resolver.effective_sources(session) # pyright: ignore[reportArgumentType]
|
||||
|
||||
assert origin == "db"
|
||||
assert [row.kind for row in rows] == ["git", "local", "git"]
|
||||
assert [row.url for row in rows] == [
|
||||
"https://db.example/a.git",
|
||||
local,
|
||||
"git@db.example:b.git",
|
||||
]
|
||||
assert [row.path for row in rows] == [None, local, None]
|
||||
|
||||
|
||||
def test_env_fallback_git_only_while_table_empty(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Empty table + env set → synthetic ``kind='git'`` rows, origin
|
||||
``env``.
|
||||
|
||||
The env fallback is git-only (no local rows can come from it); the
|
||||
CSV parse is ``Settings.git_source_list`` itself (phase 28):
|
||||
whitespace-trimmed, empty entries dropped, order preserved.
|
||||
"""
|
||||
_stub_env(monkeypatch, " https://env.example/one.git , ,git@env.example:two.git ")
|
||||
session = _FakeSession([])
|
||||
|
||||
urls, origin = resolver.effective_git_sources(session) # pyright: ignore[reportArgumentType]
|
||||
rows, origin = resolver.effective_sources(session) # pyright: ignore[reportArgumentType]
|
||||
|
||||
assert urls == ["https://env.example/one.git", "git@env.example:two.git"]
|
||||
assert origin == "env"
|
||||
assert [row.url for row in rows] == ["https://env.example/one.git", "git@env.example:two.git"]
|
||||
assert all(row.kind == "git" for row in rows)
|
||||
assert all(row.path is None for row in rows)
|
||||
|
||||
|
||||
def test_both_empty_returns_empty_env(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
@@ -86,9 +127,9 @@ def test_both_empty_returns_empty_env(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
_stub_env(monkeypatch, " , ") # whitespace-only is just as unconfigured as empty
|
||||
session = _FakeSession([])
|
||||
|
||||
urls, origin = resolver.effective_git_sources(session) # pyright: ignore[reportArgumentType]
|
||||
rows, origin = resolver.effective_sources(session) # pyright: ignore[reportArgumentType]
|
||||
|
||||
assert urls == []
|
||||
assert rows == []
|
||||
assert origin == "env"
|
||||
|
||||
|
||||
@@ -99,12 +140,42 @@ def test_db_rows_ordered_by_added_at_then_id(monkeypatch: pytest.MonkeyPatch) ->
|
||||
"""The statement orders by ``(added_at, id)`` — oldest first, the id
|
||||
tie-break deciding same-timestamp inserts (matches the API's GET)."""
|
||||
_stub_env(monkeypatch, "")
|
||||
session = _FakeSession([GitSource(url="https://db.example/a.git")])
|
||||
session = _FakeSession([_git("https://db.example/a.git")])
|
||||
|
||||
resolver.effective_git_sources(session) # pyright: ignore[reportArgumentType]
|
||||
resolver.effective_sources(session) # pyright: ignore[reportArgumentType]
|
||||
|
||||
assert len(session.statements) == 1
|
||||
compiled = str(session.statements[0].compile(compile_kwargs={"literal_binds": True}))
|
||||
assert re.search(
|
||||
r"ORDER BY\s+git_sources\.added_at ASC,\s+git_sources\.id ASC", compiled
|
||||
), compiled
|
||||
|
||||
|
||||
# --- the phase-35 back-compat alias ----------------------------------------
|
||||
|
||||
|
||||
def test_alias_returns_git_urls_only(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""``effective_git_sources`` (phase-35 name) — repo URLs of the git
|
||||
rows only; local rows are filtered out (they carry no clone URL),
|
||||
origin unchanged."""
|
||||
_stub_env(monkeypatch, "")
|
||||
session = _FakeSession(
|
||||
[_git("https://db.example/a.git"), _local("/abs/notes"), _git("git@db.example:b.git")]
|
||||
)
|
||||
|
||||
urls, origin = resolver.effective_git_sources(session) # pyright: ignore[reportArgumentType]
|
||||
|
||||
assert urls == ["https://db.example/a.git", "git@db.example:b.git"]
|
||||
assert origin == "db"
|
||||
|
||||
|
||||
def test_alias_env_fallback_unchanged(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""The alias's env fallback is byte-identical to the phase-35 one
|
||||
(git-only CSV list, origin ``env``)."""
|
||||
_stub_env(monkeypatch, " https://env.example/one.git , ")
|
||||
session = _FakeSession([])
|
||||
|
||||
urls, origin = resolver.effective_git_sources(session) # pyright: ignore[reportArgumentType]
|
||||
|
||||
assert urls == ["https://env.example/one.git"]
|
||||
assert origin == "env"
|
||||
|
||||
Reference in New Issue
Block a user