feat(sources): upload tarball/zipfile archives as sources — unpack, scan, and replace in place
Phase 49 (owner request, chat 2026-08-28: "The git sources page should remove local directory and should instead accept a tarball or zipfile upload which it will unpack and scan … reuploading the same tarball should not create a new folder, but should unpack and overwrite the previously unpacked content" — design confirmed in the same conversation): * POST /api/git-sources/upload (admin-only, require_admin): accepts .tar/.tar.gz/.tgz/.zip, streams it with the BOR_UPLOAD_MAX_MB cap (bounds BOTH the compressed upload and the total extracted bytes — zip-bomb guard), safely unpacks (absolute/traversal/symlink/hardlink escape and device/FIFO members rejected), and atomically swaps the content in over BOR_UPLOAD_DIR/<name>/ (name = filename minus the archive suffix — no missing window, a failed upload never touches the existing folder/row/KB). The git_sources row is upserted by path (kind='local', no duplicates, added_at preserved), the models are checked fail-fast (503 sanitized when down — the folder/row stay committed and the next sync/re-upload retries idempotently), and the source is scanned synchronously in the request (single-source import_sources prune=True + change-gated KB overview), answering 200 with the sync-style counts. One upload at a time (409); the request session is released before the scan so a concurrent TRUNCATE cannot deadlock against it. * app/rag/archive_upload.py: ArchiveUploadError, ARCHIVE_SUFFIXES, archive_source_name (safe-name derivation), unpack_archive (guarded zip/tar extraction with the extracted-byte cap, no partial state), swap_in (atomic replace with restore-on-failure) — fully unit-tested. * app/config.py + .env.example: BOR_UPLOAD_DIR (default ~/bor-sources/uploads, deliberately separate from the git checkouts) and BOR_UPLOAD_MAX_MB (default 512; a validator fails loud at startup on <= 0). * python-multipart added to the dependencies — FastAPI's required multipart parser (an A2 implementation detail, phase locked decision). * The Sources page: the phase-38 "Add a local directory" form is removed; #archive-upload-form takes its place (labeled file input, "Upload & scan" button, the §7.4 never-stale lifecycle, inline role=alert error, role=status count line); hint + table caption updated. The POST /api/git-sources kind=local API contract is UNCHANGED — a plain directory is still registrable via the API, and existing Local rows list/remove/sync exactly as before. * The phase-38 story E2E (test_local_directory_sources.py) is rewritten API-driven — the form it drove is gone; its acceptance stands. * The story E2E (test_archive_upload_sources.py): the swap, upload→scan→list (the deterministic "Uploading…" in-flight state, the Local row, /api/docs + the RAG catalog), same-filename re-upload (in-place replace, prune, no duplicate row, v2-only folder), the 422 inline error + recovery (the form is not wedged), and the anonymous gate + 403. * README: the archive-upload section (formats, naming rule, in-place replace, both new settings), the local-directory form removal noted, config reference rows for BOR_UPLOAD_DIR / BOR_UPLOAD_MAX_MB. Gates: unit+integration green, app/ coverage 99%, the story E2E green in isolation, the regression suites (git sources admin, local directory sources, sync button, import documents, nav rename, smoke, shared header) green in isolation, ruff + pyright clean. Note: per this phase's file-level staging, frontend/assets/styles.css also carries the small same-day in-flight owner rework already in the working tree (the .sign-in-mobile companion rule for the phase-48 mobile sign-in copy); the phase-49 change is the upload form's block.
This commit is contained in:
@@ -5,13 +5,27 @@ 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
|
||||
|
||||
**Phase-49 rewrite (owner permission 2026-08-28): the page form this
|
||||
story drove is GONE.** The "Add a local directory" form
|
||||
(``#local-source-form``, phase 38) was removed from
|
||||
``/git-sources.html`` and replaced by the archive upload form
|
||||
(``#archive-upload-form``, phase 49 — its own story E2E is
|
||||
``test_archive_upload_sources.py``). The phase-38 ACCEPTANCE stands —
|
||||
a local directory can still be registered, imported, pruned, and
|
||||
removed — so this suite now adds local sources through the
|
||||
**authenticated API** the page's own JS no longer calls
|
||||
(``POST /api/git-sources {"kind": "local", "path": …}``, the contract
|
||||
the page used to wrap; the admin cookie rides the browser context via
|
||||
``page.request``). Do NOT "restore" a form here: adding a plain
|
||||
directory by hand is an API-only operation now.
|
||||
|
||||
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).
|
||||
source registry (phase 38): the admin registers an existing, non-git
|
||||
directory (API — see above), and the real Sync button (phase 32)
|
||||
imports it — with add-time fail-loud validation (a missing/relative
|
||||
path is rejected with 422, naming the path in the JSON detail) 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
|
||||
@@ -30,27 +44,27 @@ 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:
|
||||
Contract under test (local adds are API-driven — phase-49 rewrite):
|
||||
|
||||
* 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
|
||||
pattern), the manager hidden (list + git add form + archive upload
|
||||
form inert), NO ``/api/git-sources`` call, and 403 on the source
|
||||
routes (incl. the phase-49 upload route) + the sync trigger (the
|
||||
phase-35 regression assertions, A10);
|
||||
* admin: a missing path (``/nonexistent/bor-e2e``) 422s with the JSON
|
||||
detail NAMING the path ("not a directory") and no row added; the
|
||||
host temp dir adds (201 → the row renders with the **Local** badge +
|
||||
the full path in a mono cell once the list re-renders); the same
|
||||
path again 409s ("already exists", path named) with no second row;
|
||||
* admin: the **Sync** button on the Sources page (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.
|
||||
the empty state returns).
|
||||
* (The phase-38 form's a11y assertions moved with the form: the
|
||||
upload form's UI Structure Check lives in the phase-49 story E2E.)
|
||||
|
||||
Test → story mapping (Playwright Mapping Rule):
|
||||
1. ``test_anonymous_soft_gate_and_403s``
|
||||
@@ -223,20 +237,28 @@ def _admin_git_sources_page(page: Page, app_url: str) -> None:
|
||||
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 _add_local_dir_api(page: Page, app_url: str, path: str) -> None:
|
||||
"""Register a local directory through the authenticated API
|
||||
(phase-49 rewrite: the page form is gone — the ``kind=local``
|
||||
POST contract the page used to wrap is unchanged, and the admin
|
||||
cookie rides the browser context, cf. test_git_sources_admin.py).
|
||||
``data`` with a dict is JSON-serialized by Playwright's Python API
|
||||
(there is no ``json=`` kwarg — the JS API's shape is ``json``).
|
||||
201 is the only success — the caller re-renders the page when it
|
||||
needs the row in the table."""
|
||||
r = page.request.post(
|
||||
f"{app_url}/api/git-sources", data={"kind": "local", "path": path}
|
||||
)
|
||||
assert r.status == 201, f"expected 201 for {path}: {r.status} {r.text}"
|
||||
|
||||
|
||||
def _click_sync(page: Page) -> None:
|
||||
def _click_sync(page: Page, app_url: str) -> 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."""
|
||||
underneath is what the 2 s UI loop observes. The button's home is
|
||||
the Sources page (owner rework 2026-08-28 — it left the shared
|
||||
navbar), so the helper visits it first."""
|
||||
page.goto(app_url + "/sources.html")
|
||||
btn = page.locator("#sync-btn")
|
||||
expect(btn).to_be_visible()
|
||||
btn.click()
|
||||
@@ -277,10 +299,11 @@ def _docs(page: Page, app_url: str) -> list[dict[str, Any]]:
|
||||
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)."""
|
||||
"""The phase-16/35 gate on this page (regression through the
|
||||
phase-49 form swap): anonymous visitors see the sign-in gate and a
|
||||
fully hidden manager (list + git form + upload form — the phase-38
|
||||
local form is gone), the page never calls the admin API, and every
|
||||
admin route 403s (A10)."""
|
||||
page.set_default_timeout(30_000)
|
||||
|
||||
api_calls: list[str] = []
|
||||
@@ -299,18 +322,26 @@ def test_anonymous_soft_gate_and_403s(
|
||||
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.
|
||||
# The manager is absent/inert: list, git add form, the phase-49
|
||||
# archive upload form, 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("#archive-upload-form")).to_be_hidden()
|
||||
expect(page.locator("#git-sources-env-note")).to_be_hidden()
|
||||
|
||||
# The phase-38 local-directory form is GONE (phase-49 rewrite) —
|
||||
# the upload form replaced it.
|
||||
expect(page.locator("#local-source-form")).to_have_count(0)
|
||||
expect(page.locator("#local-source-path")).to_have_count(0)
|
||||
expect(page.locator("#local-source-add")).to_have_count(0)
|
||||
|
||||
# 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.
|
||||
# …and the API 403s anonymous callers (the phase-35 assertions,
|
||||
# plus the phase-49 upload route): all four 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(
|
||||
@@ -330,59 +361,66 @@ def test_anonymous_soft_gate_and_403s(
|
||||
).status
|
||||
== 403
|
||||
)
|
||||
# The phase-49 upload route 403s too (require_admin runs before the
|
||||
# multipart body is ever parsed — the body here is a stand-in JSON
|
||||
# payload, not a real multipart upload).
|
||||
assert (
|
||||
page.request.post(
|
||||
f"{app_url}/api/git-sources/upload", data={"file": ""}
|
||||
).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
|
||||
# 2. Admin: add validation (missing path, dir, duplicate) — API-driven
|
||||
# (phase-49 rewrite: the form this drove is gone)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
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)."""
|
||||
"""Add-time fail-loud validation (phase-49 rewrite: the page form is
|
||||
gone, so the same contract is asserted on the API body the page
|
||||
used to render): a missing path 422s NAMING the path in the JSON
|
||||
detail (no row added); the host temp dir adds (201 → the row renders
|
||||
with the Local badge + full path once the list re-renders); the same
|
||||
path again 409s ("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: 422 naming it in the JSON detail, NO row --------
|
||||
r = page.request.post(
|
||||
f"{app_url}/api/git-sources", data={"kind": "local", "path": MISSING_PATH}
|
||||
)
|
||||
assert r.status == 422, r.text
|
||||
detail = r.json()["detail"]
|
||||
assert MISSING_PATH in detail, f"detail does not name the path: {detail!r}"
|
||||
assert "not a directory" in detail
|
||||
r = page.request.get(f"{app_url}/api/git-sources")
|
||||
assert r.status == 200, r.text
|
||||
assert r.json()["sources"] == []
|
||||
|
||||
# --- 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))
|
||||
# --- the temp dir: 201 → the row renders with the Local badge ------
|
||||
_add_local_dir_api(page, app_url, str(local_dir))
|
||||
# The API add is invisible to the open page (its JS no longer adds
|
||||
# local dirs) — re-render the list, exactly as a fresh visit would.
|
||||
page.reload()
|
||||
expect(page.locator("#git-sources-content")).to_be_visible(timeout=30_000)
|
||||
row = page.locator("#git-sources-tbody tr", has_text=str(local_dir))
|
||||
expect(row).to_have_count(1)
|
||||
expect(row).to_have_count(1, timeout=30_000)
|
||||
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)…
|
||||
# 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
|
||||
@@ -392,26 +430,17 @@ def test_admin_add_missing_path_then_dir_then_duplicate(
|
||||
(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"
|
||||
# --- duplicate: 409 naming the path, NO second row -----------------
|
||||
r = page.request.post(
|
||||
f"{app_url}/api/git-sources", data={"kind": "local", "path": str(local_dir)}
|
||||
)
|
||||
assert outline == "3px", f"focus-visible outline missing: {outline!r}"
|
||||
assert r.status == 409, r.text
|
||||
detail = r.json()["detail"]
|
||||
assert "already exists" in detail
|
||||
assert str(local_dir) in detail
|
||||
r = page.request.get(f"{app_url}/api/git-sources")
|
||||
assert r.status == 200, r.text
|
||||
assert len(r.json()["sources"]) == 1 # the row was NOT duplicated
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -423,20 +452,22 @@ def test_admin_add_missing_path_then_dir_then_duplicate(
|
||||
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)."""
|
||||
"""The phase-32 button drives the phase-38 pipeline: the Sources-
|
||||
page 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). The local add is API-driven (phase-49 rewrite)."""
|
||||
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))
|
||||
# Fresh registry (the autouse fixture truncated it) — register the
|
||||
# source via the API (the page form is gone), then run the sync
|
||||
# lifecycle against it.
|
||||
_add_local_dir_api(page, app_url, str(local_dir))
|
||||
|
||||
# --- run 1: the real sync walks the local dir and imports the file -
|
||||
_click_sync(page)
|
||||
_click_sync(page, app_url)
|
||||
body = _wait_sync_done(page, app_url)
|
||||
assert body["state"] == "success", body
|
||||
assert body["detail"]["added"] == 1, body["detail"]
|
||||
@@ -457,7 +488,7 @@ def test_admin_sync_imports_fixture_prunes_after_delete_removes_row(
|
||||
|
||||
# --- run 2: file deleted → the next sync prunes it (union prune) ---
|
||||
(local_dir / FIXTURE_REL).unlink()
|
||||
_click_sync(page)
|
||||
_click_sync(page, app_url)
|
||||
body = _wait_sync_done(page, app_url)
|
||||
assert body["state"] == "success", body
|
||||
assert body["detail"]["pruned"] == 1, body["detail"]
|
||||
@@ -468,6 +499,8 @@ def test_admin_sync_imports_fixture_prunes_after_delete_removes_row(
|
||||
)
|
||||
|
||||
# --- remove the row: accept the confirm → it disappears ------------
|
||||
# Back on the manager page (the sync clicks visited the Sources page).
|
||||
page.goto(app_url + GIT_SOURCES_URL)
|
||||
removes: list[str] = []
|
||||
page.on(
|
||||
"request",
|
||||
|
||||
Reference in New Issue
Block a user