fix(opal-server): git resilience — never stuck on an offline repo (PR3)

.claude/plans/docs/05-config-reference.md

@@ -0,0 +1,43 @@
+# 05 — OPAL Config Reference (private)
+
+Private, internal supplement to the public operator docs under `documentation/docs/`. Tracks
+`OPAL_*` env vars added or clarified by the OPAL Server Git Fixes work, with the declaring
+`file:line` so contributors can jump straight to the `Confi` declaration. Each key maps to an
+`OPAL_<NAME>` env var (the `OPAL_` prefix is added once by the component's `Confi(prefix="OPAL_")`
+instantiation — the bare name is what appears in the table).
+
+## 4. opal-server keys
+
+| Env var | Type | Default | Purpose | Declared at |
+|---|---|---|---|---|
+| `OPAL_SCOPES_GIT_FETCH_TIMEOUT` | float (seconds) | `120.0` | Hard timeout for a single scope git clone/fetch. On timeout the operation is logged and skipped (retried next cycle), so one unreachable repo can never block boot or other scopes *indefinitely*. `0` = no timeout. | `packages/opal-server/opal_server/config.py:214-221` |
+| `OPAL_SCOPES_GIT_MAX_WORKERS` | int | `10` | Bounds how many scope git operations (clone/fetch) may be *LIVE* (not yet timed out) at once, via an `asyncio.Semaphore` — there is no shared thread pool. Each operation still gets its own single-use daemon-thread executor, so a timed-out operation releases its capacity slot immediately rather than waiting for its thread to die; also bounds how many scopes are synced concurrently. | `packages/opal-server/opal_server/config.py:222-226` |
+| `OPAL_SCOPES_PURGE_CHANNEL` | str | `__opal_scope_purge__` | fleet-wide scope purge channel | `packages/opal-server/opal_server/config.py:311-318` |
+
+> **Caveat (timeout is soft, not a hard kill).** `OPAL_SCOPES_GIT_FETCH_TIMEOUT` is enforced via
+> `asyncio.wait`, which unblocks the event loop and the awaiting coroutine — but the underlying
+> pygit2 call keeps running on its own private daemon-thread executor until the OS network timeout
+> fires (a "zombie"). There is no shared pool for these zombies to exhaust: each git operation gets a
+> single-use `_DaemonThreadPoolExecutor(max_workers=1)`, so a lingering op occupies only its own
+> thread and never holds capacity another operation needs. A per-repo in-flight guard
+> (`git_op_in_flight`) prevents a second git op from touching the same (non-thread-safe) pygit2 repo
+> while the first is still lingering. Hard-kill via subprocess is out of scope. See spec §6.
+>
+> **Boot / concurrency.** Scope syncs run concurrently, bounded by `OPAL_SCOPES_GIT_MAX_WORKERS` via
+> an asyncio semaphore that gates only LIVE (not-yet-timed-out) operations. On timeout, the awaiting
+> coroutine unblocks *and* its semaphore slot is released immediately — before the zombie thread
+> returns — so a queued op waits for a freed **slot**, not a freed thread. That makes
+> `ceil(offline / workers) × timeout` the correct worst-case bound for how long a healthy scope waits
+> before it can start; `OPAL_SCOPES_GIT_FETCH_TIMEOUT` should be set well below any deployment's
+> acceptable serve latency. What the worker count does *not* bound is the zombie count: with `N`
+> sources unreachable and `M` live slots, worst-case thread count during an outage is `M` (live) plus
+> up to `N` (lingering zombies, each alive until its own OS/TCP network timeout — which can far exceed
+> `OPAL_SCOPES_GIT_FETCH_TIMEOUT`). (This bears on the `app-tests/git-leak` offline test, which runs
+> 40 offline repos against the default 10 workers.)
+>
+> **What this actually guarantees.** (1) **event-loop isolation** — the HTTP surface and bundle
+> serving run on the loop's default executor, never a git operation's thread, so they never block on a
+> hung repo; (2) a **bounded per-slot stall** — each sync's coroutine waits at most
+> `OPAL_SCOPES_GIT_FETCH_TIMEOUT` before the event loop moves on and its capacity slot is freed for
+> the next queued operation; and (3) **daemon threads** — every operation's private executor thread is
+> a daemon thread, so a lingering zombie never blocks process shutdown, however many are outstanding.

app-tests/git-leak/README.md

@@ -52,7 +52,7 @@ Gate-coverage matrix (what each flagship test actually does):
 | `test_repeat_sync_rss_stays_bounded` | **RSS guard** | PASSES; an RSS-budget guard against per-sync allocation leaks (the cache *count* can't grow for any impl, so there is no count assertion — see below) |
 | `test_server_recovers_after_postgres_bounce` | **guard (PER-15065 + gap publishes)** | PASSES on this branch (which has #915); guards the in-place broadcaster reconnect and that a scope PUT *during* the outage is buffered/replayed, not dropped |
 | `test_delete_recreate_storm` | **guard (lock re-mint)** | PASSES — rapid delete/re-create of the same source serializes on the repo lock and ends with clean caches; guards 89e090be |
-| `test_randomized_churn_holds_invariants` | **guard (seeded churn)** | PASSES — seeded random put/refresh/settled-delete churn holds invariants at every settle point (replay a failure with `CHURN_SEED=<seed>`); repoints and delete-vs-inflight-sync races are deliberately excluded, both lifted when PR3's fleet purge lands |
+| `test_randomized_churn_holds_invariants` | **guard (seeded churn)** | PASSES — seeded random put/refresh/delete churn holds invariants at every settle point (replay a failure with `CHURN_SEED=<seed>`); `repoint` ops remain deliberately excluded (covered separately by the red repoint gates), but the delete-vs-inflight-sync exclusion is lifted now that PR3's fleet purge lands |
 | `test_delete_during_hung_fetch_no_crash` | **guard (use-after-free)** | PASSES — deleting a scope whose clone is hung never crashes a worker; guards the use-after-free class 89e090be fixed |
 | `test_delete_during_hung_fetch_returns_bounded` | **gate (PR3)** | FAILS without the PR3 fetch timeout — the purge waits on the repo lock and a hung clone holds that lock indefinitely, so the DELETE never returns in bounded time |
 | `test_repoint_during_inflight_fetch_drains_old_source` | **gate (PR3, update path)** | Green half passes today — repointing while the old source's clone is hung still serves the new source; red half FAILS without PR3's update-path purge — the old source's cache entries never drain |
@@ -119,3 +119,7 @@ Docker + docker compose v2, plus host Python with `pytest pytest-timeout request
 - First-sync of a fresh scope takes the clone path, which fills only `repo_locks`;
   `repos` / `repos_last_fetched` are filled by the discover/fetch path on a second
   sync, so the load helpers issue a `refresh_all()` before asserting on `repos`.
+- `OPAL_SCOPES_GIT_FETCH_TIMEOUT` is set to `10` (code default: `120`) so hung
+  clones/fetches against the `blackhole` sidecar fail fast — the code default
+  exceeds this bed's serve windows, so offline-repo/timeout gates need the
+  short timeout to observe a healthy scope recover within their deadlines.

app-tests/git-leak/docker-compose.yml

@@ -119,6 +119,12 @@ services:
       # orphaning all previous clone dirs — a real ops event worth gating.
       OPAL_SCOPES_REPO_CLONES_SHARDS: "${OPAL_TEST_SHARDS:-1}"
       OPAL_SCOPES: "1"
+      # Fail hung clones/fetches fast. The code default (120s) exceeds the
+      # bed's serve windows: with FIFO queueing over SCOPES_GIT_MAX_WORKERS
+      # slots, a burst of blackholed sources delays a healthy scope by up to
+      # ceil(burst/slots) x timeout — 10s keeps that well under every gate's
+      # window while still exercising the timeout+zombie machinery for real.
+      OPAL_SCOPES_GIT_FETCH_TIMEOUT: "10"
       OPAL_REDIS_URL: "redis://redis:6379"
       OPAL_BROADCAST_URI: "postgres://opal:opal@postgres:5432/opal"
       # Make the broadcaster reconnect fast + deterministic for the postgres-

app-tests/git-leak/test_boot.py

@@ -7,6 +7,10 @@
 
 
 @pytest.mark.timeout(2400)
+# The mid-test --force-recreate resets container-local /proc PIDs, so the
+# fixture's before/after pid comparison spans two container generations —
+# same exemption every force-recreating test in test_boot_states.py carries.
+@pytest.mark.allow_worker_restart
 def test_boot_loads_all_scopes(opal, repo_count):
     """Measure how long a fresh boot takes to *serve* all scope repos.
 

app-tests/git-leak/test_boot_states.py

@@ -230,6 +230,20 @@ def test_shard_reconfig_still_serves_but_orphans_old_clones(opal, tmp_path):
         opal.wait_healthy()
         # restore the old-shard dirs next to whatever the new boot creates
         compose("cp", str(tmp_path / "saved") + "/.", "opal_server:/opal/git_sources")
+        # docker cp writes root-owned files; the sweep runs as the non-root
+        # `opal` user (uid 1000), so it would PermissionError on the rmtree
+        # without this.
+        compose(
+            "exec",
+            "-u",
+            "root",
+            "-T",
+            "opal_server",
+            "chown",
+            "-R",
+            "opal:opal",
+            "/opal/git_sources",
+        )
         opal.refresh_all()
         assert wait_until(
             lambda: opal.get_scope_policy("shard-0").status_code == 200,

app-tests/git-leak/test_leak.py

@@ -3,6 +3,7 @@
 import pytest
 import requests
 from helpers import gitea_repo_url, list_seeded_repos
+from invariants import source_id
 
 
 def _wait_until(predicate, timeout=30, interval=0.5):
@@ -180,21 +181,35 @@ def test_shared_repo_survives_sibling_scope_delete(opal, repo_count):
 @pytest.mark.timeout(900)
 @pytest.mark.invariant_exempt("I1", "I3", "I4")
 def test_scope_repoint_releases_old_repo_cache(opal, repo_count):
-    """Re-pointing a scope to a new repo URL, then deleting it, must drain ALL
-    cache entries — including the orphaned old URL's.
+    """Re-pointing a scope to a new repo URL must drain the OLD URL's cache
+    entries once the new source has taken over serving.
 
     ``PUT /scopes`` is create-or-update, so PUT-ing an existing scope_id with a
-    *different* repo URL orphans the old URL's cache entries: the same leak as
-    churn, via a path ``test_churn_releases_caches`` (delete-only) never takes.
-    A purge hooked solely into scope deletion would look up the scope's
-    *current* URL and leave the old entry behind forever.
-
-    FAILS on this branch (without PR2, nothing purges — the same right-reason
-    failure as churn); stays red after PR2 unless its purge also covers the
-    update path (or sweeps entries no live scope references).
+    *different* repo URL orphans the old URL's cache entries unless the purge
+    also covers the update path (not just deletes, which
+    ``test_churn_releases_caches`` covers).
+
+    GREEN GUARD, not a red gate: PR3's repoint purge
+    (``ScopePurgeCommand(reason="repoint")``, handled by ``LeaderScopePurger``)
+    fires synchronously with the repoint and, when the old source isn't
+    mid-fetch, completes in single-digit milliseconds — confirmed via server
+    logs (~4ms between the new source's lock being minted and the old
+    source's cache entries being purged; see
+    .superpowers/sdd/bed-deep-dive.md, finding 1). An earlier version of this
+    test asserted ``repo_locks >= 2`` right after the repoint as proof both
+    URLs were briefly visible together — that's a race against an
+    already-instant purge, and loses almost every time. This version instead
+    proves the purge itself: wait for the new source to actually be served,
+    then assert the old source's cache entries are gone. It stays
+    discriminating (would fail if the repoint purge were reverted) because it
+    checks the OLD source's keys, not just cache counts. Symmetric with
+    ``test_repoint_during_inflight_fetch_drains_old_source``'s red half, which
+    covers the case where the old source's clone is still hung at repoint
+    time.
     """
     assert repo_count >= 2, "needs --boot-scopes >= 2"
     repo_a, repo_b = list_seeded_repos(2)
+    old_sid = source_id(gitea_repo_url(repo_a))
 
     opal.put_scope("repoint", gitea_repo_url(repo_a))
     locked = _wait_until(lambda: opal.stats()["repo_locks"] >= 1, timeout=600)
@@ -203,30 +218,36 @@ def test_scope_repoint_releases_old_repo_cache(opal, repo_count):
     fetched = _wait_until(lambda: opal.stats()["repos"] >= 1, timeout=600)
     assert fetched, f"refresh never populated the initial repo: {opal.stats()}"
 
+    served = _wait_until(
+        lambda: opal.get_scope_policy("repoint").status_code == 200, timeout=120
+    )
+    assert served, "repo_a never served before the repoint"
+    content_a = opal.get_scope_policy("repoint").content
+
     # Re-point the same scope_id at a different repo; repo_a's entries are now
-    # referenced by no scope. The sync triggered by the PUT clones repo_b
-    # (filling repo_locks), and the refresh fills its repos /
-    # repos_last_fetched — so both URLs are visible in the caches, proof the
-    # orphan exists before the delete (this guards the setup, not the leak).
+    # referenced by no scope.
     opal.put_scope("repoint", gitea_repo_url(repo_b))
-    locked = _wait_until(lambda: opal.stats()["repo_locks"] >= 2, timeout=600)
-    assert locked, f"re-pointed repo never locked: {opal.stats()}"
-    opal.refresh_all()
-    fetched = _wait_until(lambda: opal.stats()["repos"] >= 2, timeout=600)
-    assert fetched, (
-        f"expected cache entries for both the old and new URL after the "
-        f"re-point: {opal.stats()}"
-    )
 
-    opal.delete_scope("repoint")
+    def _serving_repo_b() -> bool:
+        resp = opal.get_scope_policy("repoint")
+        # bundle content, not just status 200 (which was already true for
+        # repo_a) — this is what proves the scope actually switched sources.
+        return resp.status_code == 200 and resp.content != content_a
 
-    def _all_caches_empty() -> bool:
-        s = opal.stats(samples=1)
-        return s["repo_locks"] == 0 and s["repos"] == 0 and s["repos_last_fetched"] == 0
+    assert _wait_until(
+        _serving_repo_b, timeout=300
+    ), "scope never switched to serving the re-pointed (repo_b) content"
 
-    released = _wait_until(_all_caches_empty, timeout=60)
-    assert released, (
-        f"caches did not drain after deleting the re-pointed scope (the old "
-        f"URL's orphaned entry leaks unless the purge covers updates too): "
-        f"{opal.stats()}"
+    def _old_source_purged() -> bool:
+        s = opal.stats(samples=1)
+        return (
+            old_sid not in s["repo_locks_keys"]
+            # repos_keys holds full on-disk paths, not bare source_ids
+            and not any(old_sid in key for key in s["repos_keys"])
+            and old_sid not in s["repos_last_fetched_keys"]
+        )
+
+    assert _wait_until(_old_source_purged, timeout=60), (
+        f"old source {old_sid[:12]}… cache entries leaked after repoint "
+        f"(PR3 update-path purge gate): {opal.stats()}"
     )

app-tests/git-leak/test_transitions.py

@@ -37,22 +37,22 @@ def test_delete_recreate_storm(opal, repo_count):
 
 @pytest.mark.timeout(1200)
 def test_randomized_churn_holds_invariants(opal, repo_count):
-    """Seeded random put/refresh churn with settled deletes; invariants must
-    hold at every settle point. Replay a failure with CHURN_SEED=<printed>.
-
-    Two deliberate constraints, both lifted when PR3's fleet purge lands
-    (no silent caps):
-    - 'repoint' ops are EXCLUDED: a repoint orphans the old source's cache
-      entries by design today (the red repoint gate covers it). A `put` on a
-      live scope therefore reuses that scope's existing repo.
-    - Deletes run only at round END, after every live scope has settled: a
-      DELETE racing an in-flight sync loses its purge (the sync re-populates
-      the caches for the dead scope — same PR3 class; proven deterministically
-      by seed 309006536 during this test's development; deterministic red-gate
-      coverage lands in test_repoint_during_inflight_fetch_drains_old_source).
-      A bounded residual window remains (a served scope's re-sync can still be
-      in flight); in practice the settle polling latency dwarfs a tiny repo's
-      sync time.
+    """Seeded random put/refresh churn with end-of-round deletes; invariants
+    must hold at every settle point. Replay a failure with
+    CHURN_SEED=<printed>.
+
+    One deliberate constraint remains: 'repoint' ops are EXCLUDED — a repoint
+    orphans the old source's cache entries by design today (the red repoint
+    gate covers it: test_scope_repoint_releases_old_repo_cache and
+    test_repoint_during_inflight_fetch_drains_old_source). A `put` on a live
+    scope therefore reuses that scope's existing repo.
+
+    The delete-vs-inflight-sync exclusion is LIFTED: deletes used to run only
+    after every live scope had settled, because a DELETE racing an in-flight
+    sync lost its purge (the sync re-populated the caches for the dead scope;
+    proven deterministically by seed 309006536 during this test's
+    development). PR3's fleet-wide purge channel closes that race, so deletes
+    now fire without waiting for the round's live scopes to settle first.
     """
     import os
     import random
@@ -74,13 +74,9 @@ def test_randomized_churn_holds_invariants(opal, repo_count):
                 live[sid_] = repo
             else:
                 opal.refresh_all()
-        # settle every live scope before any delete
-        for sid_ in list(live):
-            assert wait_until(
-                lambda s=sid_: opal.get_scope_policy(s).status_code == 200,
-                timeout=300,
-            ), f"round {round_no}: live scope {sid_} never settled (seed {seed})"
-        # settled deletes: each live scope has a coin-flip chance to go
+        # deletes race in-flight syncs: PR3's fleet purge closes this class,
+        # so each live scope has a coin-flip chance to go without waiting for
+        # it to settle first.
         for sid_ in list(live):
             if rng.random() < 0.5:
                 opal.delete_scope(sid_)

packages/opal-server/opal_server/config.py

@@ -211,6 +211,20 @@ class OpalServerConfig(Confi):
         0,
         description="The timeout for cloning the policy repository (0 means wait forever)",
     )
+    SCOPES_GIT_FETCH_TIMEOUT = confi.float(
+        "SCOPES_GIT_FETCH_TIMEOUT",
+        120.0,
+        description="Soft timeout in seconds for a single scope git clone/fetch "
+        "(the awaiting operation is abandoned, but the underlying git call keeps "
+        "running on its thread until the OS network timeout). On timeout the "
+        "operation is logged and skipped (retried next cycle), so one unreachable "
+        "repo can never block boot or other scopes indefinitely (0 = no timeout).",
+    )
+    SCOPES_GIT_MAX_WORKERS = confi.int(
+        "SCOPES_GIT_MAX_WORKERS",
+        10,
+        description="Maximum number of LIVE scope git operations (clone/fetch) running concurrently; also bounds how many scopes are synced at once. A timed-out operation stops counting against this limit (its lingering thread persists on its own until the OS network timeout), so capacity is never starved by hung remotes — but worst-case thread count during an outage is this limit plus the number of lingering timed-out operations.",
+    )
     LEADER_LOCK_FILE_PATH = confi.str(
         "LEADER_LOCK_FILE_PATH",
         "/tmp/opal_server_leader.lock",
@@ -295,6 +309,14 @@ class OpalServerConfig(Confi):
         20,
         description="Timeout for forgetting a server from which a keep-alive haven't been seen (keep-alive frequency would be half of this value)",
     )
+    SCOPES_PURGE_CHANNEL = confi.str(
+        "SCOPES_PURGE_CHANNEL",
+        "__opal_scope_purge__",
+        description="Pub/sub channel (worker-to-worker, over the broadcaster) used to "
+        "purge GitPolicyFetcher caches fleet-wide when a scope is deleted, repointed "
+        "to a new source, or its clone dir is reclaimed as an orphan. Every worker "
+        "subscribes; the leader additionally removes the clone dir.",
+    )
 
     # Data updates
     ALL_DATA_TOPIC = confi.str(