phase: 179_git_clone_server_freeze
All gates green; working tree holds exactly the phase's changes for the harness to commit. ## Phase 179 — Final verification report - **Verified (all 3 tasks already complete in `complete/`)**: repro + CONFIRMED verdict, `asyncio.to_thread` fix, dedicated E2E suite. - `app/api/sync.py::_run_sync`: both git ops (`clone_or_pull`, `file_commit_dates`) now `await asyncio.to_thread(...)`; phase-134 boundary note + module docstring document worker-thread-runs-to-completion and the A3 DB-calls-stay-on-loop note. - **Tests / lint / coverage** (full suite 100% green): - `uv run pytest tests/integration/test_sync_git_freeze.py -v` → **4 passed** (post-fix probe <1.5 s during 3 s stub) - `uv run pytest tests/e2e/test_sync_git_freeze.py -v --no-cov` → **2 passed in 25.65 s in isolation** (wall guard 120 s holds) - `uv run pytest --cov=app --cov-report=term-missing` → **4173 passed**, `app/` coverage **99%** (`app/api/sync.py` 100%) - `uv run ruff check .` → All checks passed; `uv run pyright` → **0 errors** - **Completion criteria**: ① integration probe green + verdict (CONFIRMED, mechanism, measured 3.04 s→0.003 s) in docstring — **met**; ② off-loop git ops + boundary/A3 notes — **met**; ③ full suite green, coverage 99% >90%, ruff+pyright clean — **met**; ④ E2E green in isolation (responsive-poll + cancel-during-clone contracts) — **met**; ⑤ atomic commit + move to `complete/` — **harness-owned** (pipeline rule overrides per-phase self-commit; all changes staged-ready in the working tree). - **Documentation**: full README (1526 l) + AGENTS.md review — phase-179 bullets present and accurate (verified against fixture constants, `.env.example` fully covered by the config reference, pages table matches `app/main.py` shell routes, quick-reference commands/artifacts exist); **consistent — no further changes needed**. - **Notable**: task-03 pipeline sessions died on context overflow but their harness validation passed; this pass independently re-ran every gate to confirm. No live infra touched; no git index commands run. - **Next pending phase**: `180_markdown_inline_links_images`.
This commit is contained in:
+71
-12
@@ -133,11 +133,42 @@ local-dir check), immediately before ``import_sources``, and
|
||||
immediately before the overview + summaries span (the
|
||||
``added + updated > 0`` block). Consequence a future reader must
|
||||
know: a git subprocess already running at the cancel moment runs to
|
||||
completion (the blocking ``subprocess.run`` has no await inside) —
|
||||
the cancel lands at the NEXT boundary (after that source's git ops,
|
||||
between files, or before overview/summaries); during the import
|
||||
span, the ``_task.cancel()`` from the endpoint interrupts at the
|
||||
next per-file await.
|
||||
completion (Python threads are not interruptible — since phase 179
|
||||
it occupies a ``to_thread`` worker rather than the loop, so it no
|
||||
longer parks the EVENT LOOP while it does) — the cancel lands at the
|
||||
NEXT boundary (after that source's git ops, between files, or before
|
||||
overview/summaries); during the import span, the ``_task.cancel()``
|
||||
from the endpoint interrupts at the next per-file await. Phase 179
|
||||
changes only WHO waits: the run's await on the git worker is
|
||||
unblocked immediately by ``_task.cancel()`` (so the run reaches that
|
||||
next boundary) and every other request — the 2 s status poll
|
||||
included — is served while the git thread finishes on its own. The
|
||||
observable contract is phase 134's, unchanged: stop at the next
|
||||
boundary, ``cancelled`` terminal, no corruption.
|
||||
|
||||
Phase 179 (task 02) — the sync's git operations run OFF the event
|
||||
loop. Before it, ``clone_or_pull`` and ``file_commit_dates`` were
|
||||
called as plain synchronous functions from inside ``_run_sync`` — the
|
||||
coroutine ``POST /api/sync`` spawns with ``asyncio.create_task`` — so
|
||||
their blocking ``subprocess.run`` (``scripts/git_sync.py``, ``run_git``)
|
||||
parked the app's SINGLE uvicorn loop thread for the whole git op:
|
||||
nothing else got scheduled, the ENTIRE HTTP surface stalled (the UI's
|
||||
2 s ``GET /api/sync/status`` poll included) and the server resumed
|
||||
only when git returned. That is the owner's ``TODO.md`` L3 suspicion
|
||||
("a long git clone seems to freeze the server?"), CONFIRMED by the
|
||||
in-process repro in ``tests/integration/test_sync_git_freeze.py`` — a
|
||||
3 s stubbed clone made the status probe take ≈ 3.0 s (and
|
||||
``GET /api/health`` too). Both git steps now go through
|
||||
``asyncio.to_thread``, so the loop stays free for the whole clone
|
||||
while the clone/pull and the commit-date walk occupy a worker thread.
|
||||
Nothing else changes: the pipeline sequence, the error surfaces and
|
||||
the cancel terminals above are exactly as documented — only the
|
||||
SCHEDULING of the git calls moved (``scripts/git_sync.py`` itself is
|
||||
untouched, and it stays the single git-invocation site). Known
|
||||
BOUNDED non-fix (phase 179 decision A3): the run's short-lived
|
||||
``SessionLocal()`` / ``effective_sources`` / version-bump calls stay
|
||||
ON the loop — millisecond-scale SQLAlchemy, not the reported symptom,
|
||||
deliberately out of scope.
|
||||
|
||||
The ``CancelledError`` contract has TWO paths (the former
|
||||
"deliberately not caught" rule refined, not broken): in
|
||||
@@ -472,8 +503,11 @@ def _check_cancelled() -> None:
|
||||
pipeline boundaries (the module docstring's cancel paragraph) —
|
||||
the top of the per-source loop, before ``import_sources``, and
|
||||
before the overview + summaries span. A git subprocess in flight
|
||||
at the cancel moment runs to completion (no await inside
|
||||
``subprocess.run``); the cancel lands at the NEXT boundary.
|
||||
at the cancel moment runs to completion in its ``to_thread``
|
||||
worker (phase 179: threads are not interruptible, but the loop is
|
||||
no longer parked by it); the cancel lands at the NEXT boundary —
|
||||
or, when ``_task.cancel()`` interrupts the ``to_thread`` await
|
||||
itself, right there, with the worker finishing on its own.
|
||||
"""
|
||||
if _cancel_event.is_set():
|
||||
raise asyncio.CancelledError()
|
||||
@@ -497,6 +531,12 @@ async def _run_sync() -> None:
|
||||
UNREQUESTED cancel (the app-shutdown path — the event unset)
|
||||
re-raises unchanged: a background task is never swallowed, and
|
||||
swallowing a real stop would mask it.
|
||||
|
||||
Phase 179 (task 02): the per-source git steps (``clone_or_pull``,
|
||||
``file_commit_dates``) run off the loop via ``asyncio.to_thread``
|
||||
so a long clone no longer stalls the rest of the HTTP surface (the
|
||||
module docstring's freeze paragraph); the run's own sequencing is
|
||||
byte-for-byte the pipeline above.
|
||||
"""
|
||||
_status.state = "running"
|
||||
_status.started_at = datetime.now(UTC)
|
||||
@@ -559,9 +599,16 @@ async def _run_sync() -> None:
|
||||
doc_dates_by_root: dict[str, dict[str, datetime]] = {}
|
||||
for row in rows:
|
||||
# Phase 134 (task 01): the per-source boundary — a cancel
|
||||
# registered while the previous source's git ops run (a
|
||||
# blocking subprocess runs to completion) lands here,
|
||||
# before this source's clone/pull or local-dir check.
|
||||
# registered while the previous source's git ops run lands
|
||||
# here, before this source's clone/pull or local-dir check.
|
||||
# Phase 179 (task 02): those git ops now run in a
|
||||
# ``to_thread`` worker, so a cancel parked on them no
|
||||
# longer means a PARKED EVENT LOOP — ``_task.cancel()``
|
||||
# unblocks this run at the ``await`` and the loop serves
|
||||
# everything else immediately, while the in-flight git
|
||||
# thread runs its subprocess to completion (Python threads
|
||||
# are not interruptible). Contract unchanged (phase 134):
|
||||
# stop at the next boundary, ``cancelled`` terminal.
|
||||
_check_cancelled()
|
||||
if row.kind == "git":
|
||||
# Phase 121: the token column is injected into the clone
|
||||
@@ -574,7 +621,14 @@ async def _run_sync() -> None:
|
||||
# byte-identical; a pinned branch is checked out on a
|
||||
# fresh clone (--branch) and guarded + tracked on an
|
||||
# existing checkout (fetch + ff-merge of that ref).
|
||||
root = clone_or_pull(
|
||||
# Phase 179 (task 02): OFF the event loop — the
|
||||
# blocking subprocess inside clone_or_pull used to
|
||||
# stall the whole HTTP surface for the duration of the
|
||||
# clone, the UI's 2 s GET /api/sync/status poll first
|
||||
# among the victims; to_thread keeps the loop serving
|
||||
# while the git worker runs.
|
||||
root = await asyncio.to_thread(
|
||||
clone_or_pull,
|
||||
clone_url_for(row),
|
||||
sources_root / repo_name(row.url),
|
||||
row.branch,
|
||||
@@ -586,7 +640,12 @@ async def _run_sync() -> None:
|
||||
# D11 supersedes phase 106 D10's shallow tip-date
|
||||
# behavior). Local rows contribute nothing — the
|
||||
# importer's mtime fallback applies to them.
|
||||
doc_dates_by_root[str(root)] = file_commit_dates(root)
|
||||
# Phase 179 (task 02): off the loop for the same
|
||||
# reason — a full-history ``git log`` walk over a big
|
||||
# checkout is as blocking as the clone was.
|
||||
doc_dates_by_root[str(root)] = await asyncio.to_thread(
|
||||
file_commit_dates, root
|
||||
)
|
||||
else:
|
||||
# kind=local — the stored expanded path (phase 38 also
|
||||
# mirrors it in the NOT-NULL ``url`` location column, the
|
||||
|
||||
Reference in New Issue
Block a user