ci: gate the ~1,830 tests that had no job, and fix the 6 that blocked it

.console/log.md

@@ -1,3 +1,57 @@
+## 2026-08-19 — CI found what 3.12 could not
+
+The new `test-rest` job went red on its first run, which is the job doing its
+job. Three failures, all in `dependency_drift`, all invisible locally:
+**CPython 3.11's `glob()` stats every matched path via `exists()`; 3.12's does
+not.** Anything built on those internals behaves differently on the two
+interpreters.
+
+Two of them were assertions counting `Path.stat` calls to prove the collector
+does not re-stat after discovery — a fair question, but the counter was also
+counting the interpreter's probing. The third was a guard test where one
+unreadable run directory has to be skipped while the others are still read.
+
+My first fix made that worse: wrapping the walk in `list(glob(...))` meant a
+single bad entry aborted discovery entirely, turning "skip run1, use run2" into
+"not_available". `glob()` is a generator — the first error closes it, so
+per-entry recovery inside it is not possible at all.
+
+`_latest_dependency_report` now walks with `iterdir()`, which stats nothing.
+Each entry's failure is isolated, and the interpreter's glob internals stay out
+of it. All three pass on 3.11 and 3.12.
+
+Method note: I reproduced CI's 3.11 in a container to iterate. It also showed 2
+failures CI does not report (`test_resolve_repos_root_falls_back_to_checkout_layout`,
+`test_loader_reads_latest_snapshot_with_bounded_history`) — the container mounts
+a flat worktree with no sibling checkouts, which is what those two probe. CI is
+the authority for CI; the container is a proxy that was right about the 3 that
+mattered.
+
+## 2026-08-19 — the CI gap is closed
+
+~1,830 tests had no CI job. Fixed the 6 failures that made switching the gate on
+a decision, then added `test-rest` (`pytest tests/ --ignore=tests/unit`).
+
+The 6, and what they actually were:
+
+* **4 were bad mocks, not bugs.** They raised `FileNotFoundError("msg")` — no
+  errno. pathlib's predicates swallow only ignorable errnos, so the fabricated
+  error escaped `is_file()`/`is_dir()`, which a *real* vanished path never does.
+  Verified on 3.11 and 3.12: deleting a directory mid-scan makes `glob()` return
+  `[]`. My first fix guarded the walk against deletion — defending against
+  something that cannot happen — and I reverted it.
+* **1 was a real gap the bad mock was hiding.** EACCES/EIO are NOT ignorable, so
+  a log directory that becomes *unreadable* (not deleted) does raise out of the
+  walk. The guard is warranted for that, and now says so.
+* **1 was a real production bug**: `_emit`'s dry-run branch for zero findings sat
+  below an early return that already answered, so it was unreachable and a dry
+  run reported "skipped-zero-findings" — the past tense, for something it had not
+  done. Two tests asserted opposite labels for the same call; the module's own
+  `would-` convention and the dead branch settle it.
+* **1 was a stale patch target** from the board migration weeks ago
+  (`proposer.main.PlaneClient`), failing that whole time unnoticed — which is the
+  gap in miniature.
+
 ## 2026-08-19 — an empty directory is still an importable package
 
 After #521 merged, `tests/unit/adapters/test_board_seam.py::test_the_retired_backend_is_actually_gone`

.github/workflows/ci.yml

@@ -174,6 +174,41 @@ jobs:
         # doc-accuracy suite's collection-count assertions.
         run: pytest -q tests/integration/reviewer -p no:flaky-detection
 
+  test-rest:
+    name: Test (suites outside tests/unit)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.11"
+      - name: Install dependencies
+        run: pip install -e .[dev]
+      - name: Run every suite the other jobs do not
+        # The complement of `tests/unit`. Two dedicated jobs above already gate
+        # single files that had drifted (tests/test_pr_review_watcher.py,
+        # tests/integration/reviewer) — each added after that specific suite rotted
+        # unnoticed. This job generalises the fix instead of waiting for the next
+        # one: ~1,830 tests under tests/maintenance/, tests/observer/,
+        # tests/verdicts/, the rest of tests/integration/, and the top-level
+        # tests/test_*.py had NO job at all.
+        #
+        # What that cost, concretely:
+        #   #509 shipped a regression that broke tests/maintenance/ — found a week
+        #        later by hand (#513).
+        #   #521 merged a BRAND NEW failing test in tests/test_dependency_check.py.
+        #        CI was green, the reviewer verdict was SUCCESS, and main stayed red
+        #        until #522.
+        #   tests/test_proposer_entrypoint.py had been failing since the board-seam
+        #        migration weeks earlier; nothing noticed.
+        #
+        # --ignore=tests/unit keeps this isolated from the unit job, whose
+        # doc-accuracy suite asserts collection counts and is perturbed by running
+        # alongside other trees. -p no:flaky-detection for the same reason as the
+        # jobs above: the plugin's pytest11 entry point imports the observer package
+        # before coverage instrumentation.
+        run: pytest -q tests/ --ignore=tests/unit -p no:flaky-detection
+
   performance:
     name: Performance regression tests
     runs-on: ubuntu-latest

src/operations_center/entrypoints/custodian_sweep/main.py

@@ -240,12 +240,14 @@ def _emit(
 ) -> str:
     """Create-or-comment one Plane task per repo. Returns action label."""
     if not sweep.error and sweep.total == 0:
-        return "skipped-zero-findings"
+        # dry_run has to be answered HERE. The dry-run-aware version of this
+        # same condition sat ~5 lines below and was unreachable, so a dry run
+        # reported "skipped-zero-findings" — the past tense, for something it
+        # had not done.
+        return "would-skip-zero-findings" if dry_run else "skipped-zero-findings"
     title = f"[{sweep.repo_key}] custodian sweep: {sweep.total} findings"
     body = _render_body(sweep, deltas)
     existing = existing_tasks.get(sweep.repo_key)
-    if sweep.error is None and sweep.total == 0:
-        return "skipped-zero-findings" if not dry_run else "would-skip-zero-findings"
     if dry_run:
         return "would-comment" if existing else "would-create"
     if existing:

src/operations_center/observer/collectors/check_signal.py

@@ -19,8 +19,32 @@
 
 
 def latest_matching_file(root: Path, pattern: str) -> tuple[Path, float] | None:
+    """Newest file matching *pattern* under *root*, or None if there is none.
+
+    Two different failures, guarded for two different reasons:
+
+    * The per-file `stat()` is a genuine TOCTOU race — a file can vanish
+      between the walk yielding it and us stating it, and `stat()` does not
+      swallow that.
+    * The walk is guarded for I/O errors, NOT for deletion. `glob()` resolves
+      the root through `is_dir()`, which swallows only pathlib's ignorable
+      errnos (ENOENT, ENOTDIR, EBADF, ELOOP) — so a *deleted* root already
+      yields [] with no help from us. EACCES and EIO are not on that list, so
+      a log directory that becomes unreadable raises straight out of the walk
+      and takes the collector with it. This function's contract is "None when
+      nothing is discoverable"; an unreadable directory is that.
+
+    Verified on 3.11 and 3.12: deleting the root mid-scan returns [], while a
+    non-ignorable errno propagates.
+    """
     candidates_with_mtime = []
-    for path in root.glob(pattern):
+    try:
+        discovered = list(root.glob(pattern))
+    except OSError:
+        logger.debug("Log discovery walk failed for %s", root, exc_info=True)
+        return None
+
+    for path in discovered:
         try:
             mtime = path.stat().st_mtime
             candidates_with_mtime.append((path, mtime))

src/operations_center/observer/collectors/dependency_drift.py

@@ -76,9 +76,38 @@ def collect(self, context: ObserverContext) -> DependencyDriftSignal:
         )
 
     def _latest_dependency_report(self, report_root: Path) -> tuple[Path, float] | None:
+        """Newest dependency report under *report_root*, or None.
+
+        Walks with `iterdir()` rather than `glob("*/dependency_report.json")`,
+        for two reasons that turned out to be the same reason:
+
+        * **One bad run directory must not hide the others.** A report dir that
+          is unreadable (EACCES/EIO — pathlib swallows only ENOENT, ENOTDIR,
+          EBADF and ELOOP) should be skipped, not abort discovery. `glob()`
+          cannot offer that: it is a generator, so the first error closes it and
+          the remaining entries are unreachable.
+        * **`glob()` probes differently across versions.** CPython 3.11 stats
+          every matched path through `exists()`; 3.12 does not. Anything built
+          on those internals behaves differently on the two interpreters — which
+          is precisely how this went green locally on 3.12 and red on CI's 3.11.
+
+        `iterdir()` stats nothing, so each entry's failure is isolated and the
+        interpreter's internals stay out of it.
+        """
         candidates_with_mtime = []
-        for path in report_root.glob("*/dependency_report.json"):
+        try:
+            entries = list(report_root.iterdir())
+        except OSError:
+            logger.debug(
+                "Dependency report discovery walk failed for %s", report_root, exc_info=True
+            )
+            return None
+
+        for entry in entries:
+            path = entry / "dependency_report.json"
             try:
+                if not entry.is_dir():
+                    continue
                 mtime = path.stat().st_mtime
                 candidates_with_mtime.append((path, mtime))
             except (FileNotFoundError, OSError):

tests/observer/test_collectors_hardening/test_race_condition_guards.py

@@ -7,6 +7,7 @@
 proper error handling.
 """
 
+import errno
 import json
 import os
 import threading
@@ -69,16 +70,20 @@ def test_file_deleted_during_discovery_skipped(self, tmp_artifact_dir):
         file2 = tmp_artifact_dir / "deleted.log"
         file2.write_text("will be deleted")
 
-        # Patch stat() to delete file2 on first call, succeed on second
+        # Patch stat() so file2 vanishes the moment it is stat'd.
+        #
+        # This used to gate on `call_count == 1`, which silently never fired:
+        # `glob()` stats the parent directory internally (via is_dir()), so the
+        # first call was pathlib's, not ours, and file2 was never deleted. The
+        # identity check is what the test actually meant.
         original_stat = Path.stat
-        call_count = [0]
+        deleted = []
 
         def stat_with_deletion(self):
-            call_count[0] += 1
-            if self == file2 and call_count[0] == 1:
-                # Delete the file on first stat attempt
+            if self == file2 and not deleted:
+                deleted.append(True)
                 file2.unlink()
-                raise FileNotFoundError(f"No such file: {self}")
+                raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
             return original_stat(self)
 
         with patch.object(Path, "stat", stat_with_deletion):
@@ -102,7 +107,7 @@ def test_all_files_deleted_during_discovery_returns_none(self, tmp_artifact_dir)
 
         def stat_with_deletion(self):
             if str(self).endswith(".log"):
-                raise FileNotFoundError(f"No such file: {self}")
+                raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
             return original_stat(self)
 
         with patch.object(Path, "stat", stat_with_deletion):
@@ -363,6 +368,10 @@ def counting_stat(self):
                 stat_call_count[0] += 1
             return original_stat(self)
 
+        # The collector walks with iterdir(), which stats nothing, so this
+        # counts only the collector's own calls — on 3.11 and 3.12 alike. It
+        # used to count glob()'s internal exists() probe as well, which is why
+        # it passed locally on 3.12 and failed on CI's 3.11.
         with patch.object(Path, "stat", counting_stat):
             context = MagicMock()
             context.settings.report_root = tmp_artifact_dir
@@ -392,7 +401,7 @@ def test_all_reports_deleted_during_discovery(self, tmp_artifact_dir):
 
         def stat_with_deletion(self):
             if "dependency_report.json" in str(self):
-                raise FileNotFoundError(f"File deleted: {self}")
+                raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
             return original_stat(self)
 
         with patch.object(Path, "stat", stat_with_deletion):
@@ -503,7 +512,7 @@ def test_symlink_deleted_during_discovery(self, tmp_artifact_dir):
 
         def stat_fail_symlinks(self):
             if self.name == "link.log":
-                raise FileNotFoundError(f"Symlink deleted: {self}")
+                raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
             return original_stat(self)
 
         with patch.object(Path, "stat", stat_fail_symlinks):

tests/test_check_signal_collector.py

@@ -2,6 +2,8 @@
 # Copyright (C) 2026 ProtocolWarden
 from __future__ import annotations
 
+import errno
+import os
 import subprocess
 import sys
 from datetime import UTC, datetime
@@ -249,7 +251,7 @@ def mock_glob(self, pattern):
     def mock_stat(self):
         """Raise FileNotFoundError for log1, normal stat for log2."""
         if self.name == "unit_test.log":
-            raise FileNotFoundError(f"File deleted: {self}")
+            raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
         return original_stat(self)
 
     with patch.object(Path, "glob", mock_glob):
@@ -278,7 +280,7 @@ def mock_glob(self, pattern):
 
     def mock_stat(self):
         """Always raise FileNotFoundError."""
-        raise FileNotFoundError(f"File deleted: {self}")
+        raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
 
     with patch.object(Path, "glob", mock_glob):
         with patch.object(Path, "stat", mock_stat):

tests/test_custodian_sweep.py

@@ -110,7 +110,14 @@ def test_emit_skips_plane_mutation_for_zero_findings() -> None:
     assert action == "skipped-zero-findings"
 
 
-def test_emit_still_reports_zero_findings_skip_in_dry_run() -> None:
+def test_emit_reports_the_would_form_of_zero_findings_skip_in_dry_run() -> None:
+    """A dry run must not answer in the past tense.
+
+    This asserted "skipped-zero-findings" and passed only because the dry-run
+    branch below it was unreachable — the early return fired first. Its twin
+    (test_emit_dry_run_reports_zero_finding_skip) asserted the opposite and had
+    always failed. `would-` is what the rest of _emit uses for dry runs.
+    """
     plane = SimpleNamespace(
         comment_issue=lambda *args, **kwargs: (_ for _ in ()).throw(AssertionError("unexpected")),
         create_issue=lambda *args, **kwargs: (_ for _ in ()).throw(AssertionError("unexpected")),
@@ -119,7 +126,7 @@ def test_emit_still_reports_zero_findings_skip_in_dry_run() -> None:
 
     action = _emit(sweep, {}, plane, existing_tasks={}, dry_run=True)
 
-    assert action == "skipped-zero-findings"
+    assert action == "would-skip-zero-findings"
 
 
 def test_index_open_sweep_tasks_maps_repo_key_to_issue() -> None:

tests/test_dependency_drift_collector.py

@@ -4,6 +4,7 @@
 
 from __future__ import annotations
 
+import errno
 import json
 import os
 from datetime import UTC, datetime
@@ -139,7 +140,7 @@ def test_guard_single_file_deleted_during_discovery(self, tmp_path: Path) -> Non
         def mock_stat(self):
             """Raise FileNotFoundError for old_report (simulating deletion)."""
             if "run_old" in str(self):
-                raise FileNotFoundError(f"File deleted during discovery: {self}")
+                raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
             return original_stat(self)
 
         with patch.object(PathlibPath, "stat", mock_stat):
@@ -164,7 +165,7 @@ def test_guard_all_files_deleted_during_discovery(self, tmp_path: Path) -> None:
 
         def mock_stat(self):
             """Always raise FileNotFoundError."""
-            raise FileNotFoundError(f"File deleted during discovery: {self}")
+            raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
 
         with patch.object(PathlibPath, "stat", mock_stat):
             ctx = _make_context(tmp_path)
@@ -212,6 +213,9 @@ def __init__(self, mtime):
                     )
             return original_stat(self)
 
+        # No glob patching needed: the collector walks with iterdir(), which
+        # stats nothing, so this counter sees only the collector's own calls on
+        # any CPython version.
         with patch.object(PathlibPath, "stat", counting_stat):
             ctx = _make_context(tmp_path)
             signal = DependencyDriftCollector().collect(ctx)
@@ -285,7 +289,7 @@ def mock_stat(self):
             """Fail for run0 and run1."""
             path_str = str(self)
             if "run0" in path_str or "run1" in path_str:
-                raise FileNotFoundError(f"File deleted: {self}")
+                raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
             return original_stat(self)
 
         with patch.object(PathlibPath, "stat", mock_stat):
@@ -312,7 +316,7 @@ def test_guard_read_text_still_fails_after_successful_discovery(self, tmp_path:
         def mock_read_text(self, **kwargs):
             """Fail on read after successful stat."""
             if "dependency_report.json" in str(self):
-                raise FileNotFoundError(f"File deleted before read: {self}")
+                raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), str(self))
             return original_read_text(self, **kwargs)
 
         with patch.object(PathlibPath, "read_text", mock_read_text):

tests/test_proposer_entrypoint.py

@@ -115,7 +115,14 @@ def comment_issue(self, task_id: str, comment_markdown: str) -> None:  # noqa: A
         def close(self) -> None:
             return None
 
-    monkeypatch.setattr("operations_center.entrypoints.proposer.main.PlaneClient", FakeClient)
+    # Patched at the seam: proposer.main has called make_board_client since the
+    # board migration, and `PlaneClient` stopped existing there entirely. This
+    # test has been failing since then — invisible because no CI job runs it,
+    # which is the gap this change closes.
+    monkeypatch.setattr(
+        "operations_center.entrypoints.proposer.main.make_board_client",
+        lambda *a, **k: FakeClient(),
+    )
     monkeypatch.setattr(
         "sys.argv",
         ["propose-from-candidates", "--config", str(_write_config(tmp_path)), "--dry-run"],