feat(sources): admin page to add and remove git sources (TODO.md L4)
This commit is contained in:
@@ -1,17 +1,26 @@
|
||||
"""Integration: the admin sources-sync API (phase 32, task 01).
|
||||
"""Integration: the admin sources-sync API (phase 32, task 01; phase 35,
|
||||
task 03 re-points the URL resolution at the shared resolver).
|
||||
|
||||
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 ``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 (``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).
|
||||
|
||||
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
|
||||
indirection; the env list is driven by a fresh ``Settings`` on the
|
||||
resolver's module (the dev ``.env`` never leaks in).
|
||||
|
||||
The git / import / overview layers are monkeypatched in ``app.api.sync``
|
||||
(same fake style as ``test_import_docs_git.py``) — no real git, no real
|
||||
DB, no LLM: the runner's state machine and HTTP surface are under test.
|
||||
(same fake style as ``test_import_docs_git.py``) — no real git, no LLM:
|
||||
the runner's state machine and HTTP surface are under test.
|
||||
|
||||
The admin client is used **as a context manager** on purpose: the
|
||||
background sync task lives on the app's event loop, so the loop must
|
||||
@@ -22,6 +31,7 @@ request and would cancel the task on request exit.)
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
import time
|
||||
from collections.abc import Iterator
|
||||
from datetime import datetime
|
||||
@@ -29,11 +39,14 @@ from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.api import sync as sync_api
|
||||
from app.config import Settings
|
||||
from app.main import app as fastapi_app
|
||||
from app.models import GitSource
|
||||
from app.rag import git_sources as git_sources_resolver
|
||||
from app.rag.importer import ImportSummary
|
||||
from app.rag.llm import EmbeddingError, LLMClient
|
||||
from scripts.git_sync import GitSyncError
|
||||
@@ -52,6 +65,18 @@ def _fresh_sync_state() -> Iterator[None]:
|
||||
sync_api._task = None
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clean_git_sources(db: Session) -> Iterator[None]:
|
||||
"""Phase 35: the runner resolves through the real ``git_sources``
|
||||
table — global state, truncated around every test (the ``db``
|
||||
fixture skips the file when Postgres is down)."""
|
||||
db.execute(text("TRUNCATE git_sources"))
|
||||
db.commit()
|
||||
yield
|
||||
db.execute(text("TRUNCATE git_sources"))
|
||||
db.commit()
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def sync_client() -> Iterator[TestClient]:
|
||||
"""Context-managed TestClient — one app event loop across requests
|
||||
@@ -60,9 +85,29 @@ def sync_client() -> Iterator[TestClient]:
|
||||
yield client
|
||||
|
||||
|
||||
def _settings(git_sources: str = "", sources_dir: str = "~/bor-sources") -> Settings:
|
||||
"""Fresh settings (no .env file); explicit kwargs beat any env leaks."""
|
||||
return Settings(_env_file=None, git_sources=git_sources, sources_dir=sources_dir) # pyright: ignore[reportCallIssue]
|
||||
def _settings(sources_dir: str = "~/bor-sources") -> Settings:
|
||||
"""Fresh settings (no .env file); explicit kwargs beat any env leaks.
|
||||
|
||||
(The ``git_sources`` kwarg is gone with phase 35 — the runner reads
|
||||
the URLs from the resolver, not from its own settings; the env list
|
||||
is stubbed on the resolver's module via :func:`_stub_env`.)
|
||||
"""
|
||||
return Settings(_env_file=None, sources_dir=sources_dir) # pyright: ignore[reportCallIssue]
|
||||
|
||||
|
||||
def _stub_env(monkeypatch: pytest.MonkeyPatch, git_sources: str = "") -> None:
|
||||
"""The resolver's env fallback, driven by a fresh ``Settings`` (the
|
||||
dev ``.env`` never leaks in — the task-02 pattern)."""
|
||||
monkeypatch.setattr(
|
||||
git_sources_resolver,
|
||||
"get_settings",
|
||||
lambda: Settings(_env_file=None, git_sources=git_sources), # pyright: ignore[reportCallIssue]
|
||||
)
|
||||
|
||||
|
||||
def _seed(db: Session, url: str) -> None:
|
||||
db.add(GitSource(url=url))
|
||||
db.commit()
|
||||
|
||||
|
||||
def _login(client: TestClient) -> None:
|
||||
@@ -157,13 +202,15 @@ def test_anonymous_gets_403_on_both_endpoints(client: TestClient) -> None:
|
||||
|
||||
|
||||
def test_admin_sync_success_reports_full_detail(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
repo_url = f"file://{tmp_path / 'repo.git'}"
|
||||
_seed(db, repo_url) # the phase-35 resolver picks the DB row up
|
||||
_stub_env(monkeypatch) # env must not matter once the table has a row
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(git_sources=repo_url, sources_dir=str(tmp_path / "bor")),
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
clone_calls, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
@@ -214,14 +261,16 @@ def test_admin_sync_success_reports_full_detail(
|
||||
|
||||
|
||||
def test_unchanged_kb_skips_overview_refresh(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
"""Phase-31 trigger is change-gated: added + updated == 0 → no ``lite`` call."""
|
||||
repo_url = f"file://{tmp_path / 'repo.git'}"
|
||||
_seed(db, repo_url)
|
||||
_stub_env(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(git_sources=repo_url, sources_dir=str(tmp_path / "bor")),
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
_, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
@@ -244,13 +293,15 @@ def test_unchanged_kb_skips_overview_refresh(
|
||||
|
||||
|
||||
def test_double_trigger_while_running_returns_409(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
repo_url = f"file://{tmp_path / 'repo.git'}"
|
||||
_seed(db, repo_url)
|
||||
_stub_env(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(git_sources=repo_url, sources_dir=str(tmp_path / "bor")),
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
_, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
@@ -282,13 +333,15 @@ def test_double_trigger_while_running_returns_409(
|
||||
|
||||
|
||||
def test_git_failure_marks_failed_and_skips_import(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
repo_url = f"file://{tmp_path / 'bad.git'}"
|
||||
_seed(db, repo_url)
|
||||
_stub_env(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(git_sources=repo_url, sources_dir=str(tmp_path / "bor")),
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
|
||||
def failing_clone(url: str, dest: Path | str) -> Path:
|
||||
@@ -322,11 +375,14 @@ def test_git_failure_marks_failed_and_skips_import(
|
||||
def test_no_git_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)."""
|
||||
# Whitespace-only is just as unconfigured as empty.
|
||||
_stub_env(monkeypatch, " , ")
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
# Whitespace-only is just as unconfigured as empty.
|
||||
lambda: _settings(git_sources=" , ", sources_dir=str(tmp_path / "bor")),
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
clone_calls, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
@@ -337,19 +393,87 @@ 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 (BOR_GIT_SOURCES)"
|
||||
assert body["error"] == (
|
||||
"no git sources configured (git_sources table empty and BOR_GIT_SOURCES unset)"
|
||||
)
|
||||
assert clone_calls == [] # git is never touched
|
||||
assert fake_import.sources == []
|
||||
|
||||
|
||||
def test_import_error_is_reported_with_credentials_masked(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
def test_db_rows_win_over_env(
|
||||
sync_client: TestClient,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
db: Session,
|
||||
tmp_path: Path,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
repo_url = f"file://{tmp_path / 'repo.git'}"
|
||||
"""Phase 35: a stored row beats ``BOR_GIT_SOURCES`` — only the DB URL
|
||||
is cloned, and the started log names the origin."""
|
||||
db_url = "https://db.example.com/managed.git"
|
||||
_seed(db, db_url)
|
||||
_stub_env(monkeypatch, "https://env.example.com/ignored.git")
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(git_sources=repo_url, sources_dir=str(tmp_path / "bor")),
|
||||
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(files=1, added=1))
|
||||
monkeypatch.setattr(sync_api, "import_sources", fake_import)
|
||||
monkeypatch.setattr(sync_api, "regenerate_overview", FakeOverview(ok=True))
|
||||
|
||||
_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")
|
||||
|
||||
assert clone_calls == [(db_url, tmp_path / "bor" / "managed")]
|
||||
assert fake_import.sources == [[tmp_path / "bor" / "managed"]]
|
||||
assert "env.example.com" not in str(body) # the env URL never reaches the UI
|
||||
assert any("sync: started repos=1 origin=db" in r.getMessage() for r in caplog.records)
|
||||
|
||||
|
||||
def test_env_fallback_when_table_empty(
|
||||
sync_client: TestClient,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Phase 35: with the table empty (the truncate fixture), the
|
||||
``BOR_GIT_SOURCES`` list is what gets cloned — origin ``env``."""
|
||||
env_url = "https://env.example.com/fallback.git"
|
||||
_stub_env(monkeypatch, env_url)
|
||||
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(files=1, added=1))
|
||||
monkeypatch.setattr(sync_api, "import_sources", fake_import)
|
||||
monkeypatch.setattr(sync_api, "regenerate_overview", FakeOverview(ok=True))
|
||||
|
||||
_login(sync_client)
|
||||
with caplog.at_level(logging.INFO, logger="app.api.sync"):
|
||||
assert sync_client.post("/api/sync").status_code == 202
|
||||
_poll(sync_client, "success")
|
||||
|
||||
assert clone_calls == [(env_url, tmp_path / "bor" / "fallback")]
|
||||
assert any("sync: started repos=1 origin=env" in r.getMessage() for r in caplog.records)
|
||||
|
||||
|
||||
def test_import_error_is_reported_with_credentials_masked(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
repo_url = f"file://{tmp_path / 'repo.git'}"
|
||||
_seed(db, repo_url)
|
||||
_stub_env(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
_, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
|
||||
Reference in New Issue
Block a user