Merge pull request #30 from usjoh/feat/worktree-ref-and-sighup

feat: worktree_ref pinning + SIGHUP graceful shutdown

Author: usjoh

README.md

@@ -108,6 +108,7 @@ Each task gets its own directory, its own worker, its own log, and its own verdi
 | `verified` | One plain-English sentence saying what the check proves — shown on the results page next to "finished & checked" |
 | `full_access` | Worker runs unsandboxed — required for workers that spawn their own sub-workers; must also be enabled in config |
 | `worktrees` (run-level) | Give each task an isolated git worktree of `repo` so parallel workers can't collide |
+| `worktree_ref` (run-level) | Commit the task worktrees are created at (default `HEAD`). Pin it when the work is anchored to an older immutable baseline — worktree creation runs orchestrator-side and unsandboxed, so this is free, whereas a sandboxed worker told to `git checkout` inside its worktree fails outright (the shared `.git/worktrees` metadata is outside its writable surface) |
 
 > **Worktree footgun:** on PASS the task's worktree is removed — including anything written inside it. In worktrees mode, worker logs live outside task worktrees in `workdir/logs/`; have workers write deliverables outside the worktree too, or have your `check` copy artifacts out before it exits 0.
 

docs/MODEL-NOTES.md

@@ -406,6 +406,7 @@ checks and raw logs support — no vibes, no worker self-reports.
 - Both fixture-based checks passed while hiding 2 real-substrate bugs (EXIT-trap lock cleanup deleting a live lock; tilde-expansion in ${var#~/} — fixture used absolute paths). Codex not at fault; checks were fixture-only. Lesson: for ops scripts, add one real-repo scenario to the check or budget an acceptance pass after integration.
 
 ## gpt-5.6-sol (codex)
+- 2026-07-31 (observer-triad, phase-1 closeout) SCOREBOARD CORRECTION: three FAIL rows stamped `failure_class=model` that day are orchestration/environment-class, not model. (1+2) Both lanes of `bounded-primordial-v4-independent-review` (13:34Z): the spec ordered `git checkout` INSIDE the Ringer worktree, which needs a write to the shared `.git/worktrees/<id>/index.lock` — outside workspace-write's writable roots — so the first command failed EPERM. Both workers did exactly the right thing: halted immediately, changed nothing, reported the blocked command and the fix verbatim (~14k tokens each). The check then failed on the absent baseline/report and the rows recorded as model failures. Same defect class as the cttc r1 correction above (sandbox write-boundary vs spec contract); fixed structurally by the manifest-level `worktree_ref` field (worktree creation is orchestrator-side and unsandboxed) and, spec-side, by the clone-into-taskdir pattern. (3) `contract-authority` attempt 1 in the first `adversarial-review` dispatch (22:23Z, FAIL at 9m): the check rejected the draft report, and the retry was still in flight when the whole run died with its parent session (network drop → session death → unhandled SIGHUP; the ghost-run signature that put SIGHUP into the shutdown handler). At most a first-try format slip with the retry mechanism never allowed to finish — the identical three-lane review re-run later that evening went 3/3 first-try, READY×3. Discount all three rows when reading Sol's code-review cell.
 - 2026-07-28 (meridian, cttc-idea-join-notif — live Apps Script family board, 6 rounds): medium effort 3/3 first-try on code-feature/code-fix (impl 49.5k tok/108s; five-finding fix round 40k/81s; N1 lock restructure 43.5k/86s) — every fix landed exactly per the reviewer's minimal-concrete-edit blocks including comment placement; medium stays the build lane. High effort earned its review tier again: r1 NO-SHIP with 5 findings ALL reproduced by throwaway fixtures (incl. a minor-recipient gap the spec's own invariant demanded and a quota-arithmetic case tied to that night's planned family nudge); r2 caught a NEW HIGH the fix round introduced (script lock held across sendEmail → silent family-write loss, proven via instrumented LockService stub); r3 verified the replacement design with zero-violation lock-state evidence and an explicit trade judgment (105k/73k tokens for r2/r3). This is what high is FOR: last-gate review of live-user deploys. SCOREBOARD CORRECTION: two recorded FAILs on Sol's code-review cell (r1, attempts 1+2) are orchestrator-class, not model — the rows are STAMPED failure_class=model and this note is the correction. The review manifest demanded the report at an absolute path outside the sandbox's writable roots (workspace-write covers [workdir,/tmp,$TMPDIR] only) and passed no writable_roots override; the worker's reports were substance-complete BOTH attempts and were harvested from the taskdir. Same defect class as harness-audit-remediation r1 earlier the same day — the lesson was already in the session record and was reproduced anyway. Do not read Sol's code-review first-try rate at face value until these rows are discounted. Check-craft: (1) absolute-path deliverables from sandboxed workers need writable_roots, or use a RELATIVE taskdir deliverable + harvest-in-check (checks run outside the sandbox — the pattern that fixed r2/r3 here); (2) grep -E with \| alternation silently matches nothing — two false "risk-area-unaddressed" FAILs in the r1 check were this; lint cannot catch your check's own regexes.
 - 2026-07-15 ringer-self-update run (3 serial tasks, direct-repo-edit mode): code-fix baseline-test repair 1/1 first-try (61k tokens, 1.6m); code-feature self-update mechanism (git fetch/ff-pull/re-exec + HUD staleness restart + 20-test suite) 1/1 first-try at high effort (153k, 8.1m); code-feature signal-contract (all 3 scoreboard surfaces + canonical-route lint enforcement) passed on retry (358k, 13.7m) — attempt 1 died on stale old-column assertions in pre-existing tests it hadn't finished updating; the retry prompt's injected FAIL list was enough to close it out. Lesson: when a task rewrites a display contract, name every test file asserting the old contract in the spec's ownership list AND tell it to update them FIRST.
 - 2026-07-09 code-feature/code-fix (ringside-overhaul): 4/4 first-try — a ringer.py logging change with tests, a 265-line stdlib backfill CLI (atomic rewrite, dry-run, idempotence all check-verified), a ~1500-line single-file HTML redesign (running-now pills + worker-card grid + multi-expansion refactor, 30KB patch, node --check + contract greps + unittest), and a render-gating change where it correctly UPDATED tests asserting the old behavior instead of gaming the check. Medium/high reasoning, 65–120k tokens/task.

ringer.py

@@ -1727,22 +1727,22 @@ class Manifest:
     repo: Path | None
     tasks: tuple[TaskSpec, ...]
     source_path: Path | None = None
+    # The commit worktrees are created at. Defaults to HEAD, but a run whose
+    # candidate is anchored to an older immutable baseline can pin it here —
+    # worktree creation happens orchestrator-side, UNSANDBOXED, so this is the
+    # only place a non-HEAD base costs nothing. A sandboxed worker cannot
+    # `git checkout` inside its worktree (the shared .git/worktrees metadata
+    # is outside its writable surface), and the workaround it is forced into
+    # (cloning the repo into its own taskdir) burns tokens on plumbing.
+    worktree_ref: str = "HEAD"
 
     @classmethod
     def from_path(cls, path: Path) -> "Manifest":
         data = json.loads(path.read_text(encoding="utf-8"))
         if not isinstance(data, dict):
             raise ValueError("manifest root must be a JSON object")
         manifest = cls.from_obj(data)
-        return cls(
-            run_name=manifest.run_name,
-            workdir=manifest.workdir,
-            max_parallel=manifest.max_parallel,
-            worktrees=manifest.worktrees,
-            repo=manifest.repo,
-            tasks=manifest.tasks,
-            source_path=path,
-        )
+        return dataclass_replace(manifest, source_path=path)
 
     @classmethod
     def from_obj(cls, obj: dict[str, Any]) -> "Manifest":
@@ -1769,6 +1769,15 @@ def from_obj(cls, obj: dict[str, Any]) -> "Manifest":
         if duplicates:
             raise ValueError(f"duplicate task keys: {', '.join(duplicates)}")
         worktrees = bool(obj.get("worktrees", False))
+        worktree_ref_raw = obj.get("worktree_ref")
+        if worktree_ref_raw is not None and not isinstance(worktree_ref_raw, str):
+            raise ValueError("worktree_ref must be a string")
+        worktree_ref = (worktree_ref_raw or "HEAD").strip() or "HEAD"
+        if worktree_ref != "HEAD" and not worktrees:
+            raise ValueError(
+                "worktree_ref is set but worktrees is false — the ref only pins "
+                "where task worktrees are created, so enable worktrees or drop it"
+            )
         if worktrees:
             reserved_logs_dir = (workdir / "logs").resolve()
             collisions = []
@@ -1788,22 +1797,15 @@ def from_obj(cls, obj: dict[str, Any]) -> "Manifest":
             worktrees=worktrees,
             repo=repo,
             tasks=tasks,
+            worktree_ref=worktree_ref,
         )
 
     def with_max_parallel(self, value: int | None) -> "Manifest":
         if value is None:
             return self
         if value <= 0:
             raise ValueError("--max-parallel must be positive")
-        return Manifest(
-            run_name=self.run_name,
-            workdir=self.workdir,
-            max_parallel=value,
-            worktrees=self.worktrees,
-            repo=self.repo,
-            tasks=self.tasks,
-            source_path=self.source_path,
-        )
+        return dataclass_replace(self, max_parallel=value)
 
 
 FILE_TEST_OPS = {"-e", "-f", "-s", "-d", "-r", "-w", "-x", "-L"}
@@ -9432,8 +9434,11 @@ async def _prepare_taskdir(self, runtime: TaskRuntime) -> tuple[bool, str | None
                 str(self.manifest.repo),
                 "worktree",
                 "add",
+                # --detach so a branch-name ref cannot fail on "already
+                # checked out" — every taskdir gets a detached copy either way.
+                "--detach",
                 str(taskdir),
-                "HEAD",
+                self.manifest.worktree_ref,
                 stdin=asyncio.subprocess.DEVNULL,
                 stdout=asyncio.subprocess.PIPE,
                 stderr=asyncio.subprocess.STDOUT,
@@ -10456,7 +10461,7 @@ async def run_baseline(manifest: Manifest, *, config: AppConfig) -> int:
                     "add",
                     "--detach",
                     str(taskdir),
-                    "HEAD",
+                    manifest.worktree_ref,
                     stdin=asyncio.subprocess.DEVNULL,
                     stdout=asyncio.subprocess.PIPE,
                     stderr=asyncio.subprocess.STDOUT,
@@ -11123,7 +11128,20 @@ def request_shutdown() -> None:
         shutdown_started = True
         task.cancel()
 
-    for sig in (signal.SIGINT, signal.SIGTERM):
+    # SIGHUP is in the set because a run's parent is often an interactive
+    # agent session: when that session dies (network drop, app crash, closed
+    # terminal), the shell HUPs the group. Unhandled, the orchestrator died
+    # mid-flush and left a ghost — state live/unfinished forever, workers
+    # killed with no ERROR verdicts recorded (observed 2026-07-31, when a
+    # network interruption took down the session driving a 3-lane review).
+    # An INHERITED ignore is respected: `nohup ./ringer.py run` means
+    # "survive hangups", and installing a handler over SIG_IGN would turn
+    # nohup into a graceful shutdown — the opposite of what it promises.
+    shutdown_signals = [signal.SIGINT, signal.SIGTERM]
+    hup = getattr(signal, "SIGHUP", None)  # absent on Windows
+    if hup is not None and signal.getsignal(hup) is not signal.SIG_IGN:
+        shutdown_signals.append(hup)
+    for sig in shutdown_signals:
         with contextlib.suppress(NotImplementedError, RuntimeError):
             loop.add_signal_handler(sig, request_shutdown)
             registered_signals.append(sig)

tests/test_baseline_mode.py

@@ -240,6 +240,62 @@ def test_task_key_cannot_escape_baseline_scratch_root(self) -> None:
             self.assertIn("task key escapes the baseline scratch root", output)
             self.assertIn("0 pass, 0 fail, 1 error of 1 check(s)", output)
 
+    def test_baseline_worktrees_honor_the_pinned_worktree_ref(self) -> None:
+        # Baseline checks must run against the SAME tree the workers will get:
+        # a manifest pinned to an older commit whose baseline ran at HEAD
+        # would judge check-craft against the wrong sources.
+        import asyncio
+        import contextlib
+        import importlib.util
+        import io
+
+        spec = importlib.util.spec_from_file_location("ringer_baseline_ref_test", ROOT / "ringer.py")
+        assert spec is not None and spec.loader is not None
+        ringer = importlib.util.module_from_spec(spec)
+        sys.modules[spec.name] = ringer
+        spec.loader.exec_module(ringer)
+
+        with tempfile.TemporaryDirectory() as temp_root:
+            root = Path(temp_root)
+            repo = root / "repo"
+            init_git_repo(repo)
+            env = os.environ.copy()
+            env.update({"GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t",
+                        "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t"})
+            sha1 = subprocess.run(
+                ["git", "-C", str(repo), "rev-parse", "HEAD"],
+                capture_output=True, text=True, check=True,
+            ).stdout.strip()
+            (repo / "README.md").write_text("changed at head\n", encoding="utf-8")
+            subprocess.run(
+                ["git", "-C", str(repo), "commit", "--quiet", "-am", "second"],
+                check=True, env=env, capture_output=True,
+            )
+
+            manifest = ringer.Manifest.from_obj(
+                {
+                    "run_name": "baseline-pinned-ref",
+                    "workdir": str(root / "work"),
+                    "repo": str(repo),
+                    "worktrees": True,
+                    "worktree_ref": sha1,
+                    "tasks": [
+                        {
+                            "key": "pinned",
+                            "spec": "Placeholder; baseline spawns nothing.",
+                            # Passes ONLY in the first commit's tree.
+                            "check": "grep -q 'hello baseline' README.md",
+                        }
+                    ],
+                }
+            )
+            buffer = io.StringIO()
+            with contextlib.redirect_stdout(buffer):
+                rc = asyncio.run(ringer.run_baseline(manifest, config=None))
+            output = buffer.getvalue()
+            self.assertEqual(0, rc, output)
+            self.assertIn("1 pass, 0 fail, 0 error of 1 check(s)", output)
+
 
 if __name__ == "__main__":
     unittest.main(verbosity=2)

tests/test_lint.py

@@ -86,6 +86,50 @@ def test_task_fields_must_be_strings(self) -> None:
         with self.assertRaisesRegex(ValueError, r"task key must be a string"):
             self.manifest([task])
 
+    def test_worktree_ref_defaults_to_head_and_round_trips(self) -> None:
+        self.assertEqual("HEAD", self.manifest([self.task()]).worktree_ref)
+        manifest = self.manifest([self.task()], worktrees=True)
+        self.assertEqual("HEAD", manifest.worktree_ref)
+        # with_max_parallel must carry the field — it rebuilds the manifest.
+        obj = {
+            "run_name": "ref-test",
+            "workdir": manifest.workdir.as_posix(),
+            "repo": str(manifest.repo),
+            "worktrees": True,
+            "worktree_ref": "bd61708d195d6535b716ec9a8b1974464848b214",
+            "tasks": [self.task()],
+        }
+        parsed = Manifest.from_obj(obj)
+        self.assertEqual("bd61708d195d6535b716ec9a8b1974464848b214", parsed.worktree_ref)
+        self.assertEqual(
+            "bd61708d195d6535b716ec9a8b1974464848b214",
+            parsed.with_max_parallel(4).worktree_ref,
+        )
+
+    def test_worktree_ref_requires_worktrees(self) -> None:
+        with self.assertRaisesRegex(ValueError, r"worktree_ref is set but worktrees is false"):
+            Manifest.from_obj(
+                {
+                    "run_name": "ref-test",
+                    "workdir": "/tmp/ref-test-work",
+                    "worktree_ref": "bd61708",
+                    "tasks": [self.task()],
+                }
+            )
+
+    def test_worktree_ref_must_be_a_string(self) -> None:
+        with self.assertRaisesRegex(ValueError, r"worktree_ref must be a string"):
+            Manifest.from_obj(
+                {
+                    "run_name": "ref-test",
+                    "workdir": "/tmp/ref-test-work",
+                    "worktrees": True,
+                    "repo": "/tmp",
+                    "worktree_ref": 123,
+                    "tasks": [self.task()],
+                }
+            )
+
     def test_w1_unverifiable_check(self) -> None:
         manifest = self.manifest([self.task(check="echo ok && echo done")])
         self.assertHasFinding(