From 16456d9900805b07c72ffc1786e8420376de584c Mon Sep 17 00:00:00 2001 From: Oleg Date: Thu, 28 May 2026 14:29:58 -0700 Subject: [PATCH] fix(tmux): self-heal --continue REPL that dies on unresumable transcript MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tmux `claude --continue` exits immediately when the newest transcript for cwd isn't interactive-resumable (classic trigger: an SDK-origin transcript left behind after an SDK→tmux transport switch). The detached session has no remain-on-exit, so tmux auto-reaps it — yet `new-session` already returned rc=0, leaving the state machine CONNECTED against a dead REPL and warm-wakes crash-looping forever. `_has_prior_transcript` can't catch this: it only checks that *some* *.jsonl exists, not that the newest is interactive-resumable. Add a post-launch liveness check (poll has-session over a short grace window, only on --continue launches) and, when the REPL died, retry once with a forced-fresh launch so the agent comes up on a new transcript instead of wedging. A genuine cold-start failure still raises. Co-Authored-By: Claude Opus 4.7 --- src/pinky_daemon/tmux_session.py | 92 ++++++++++++++++++++ tests/test_tmux_session.py | 139 +++++++++++++++++++++++++++++-- 2 files changed, 226 insertions(+), 5 deletions(-) diff --git a/src/pinky_daemon/tmux_session.py b/src/pinky_daemon/tmux_session.py index c0da0517..3308918f 100644 --- a/src/pinky_daemon/tmux_session.py +++ b/src/pinky_daemon/tmux_session.py @@ -651,6 +651,20 @@ class _InflightMeta: # to authenticate / fetch first turn / load CLAUDE.md. _COLD_START_TIMEOUT_SEC = 60.0 +# Post-launch liveness window: how long to confirm the in-pane ``claude`` +# REPL actually survived after ``tmux new-session`` returns rc=0. A +# ``claude --continue`` that finds no *resumable* conversation exits +# almost immediately ("no conversation found to continue"); the detached +# session (no remain-on-exit) is then auto-reaped, so ``new-session``'s +# rc=0 is NOT proof of a live REPL. ``_has_prior_transcript`` can't catch +# this — it only checks that *some* ``*.jsonl`` exists, not that the +# newest one is interactive-resumable (notably an SDK-origin transcript +# left behind after an SDK→tmux transport switch). We poll ``has-session`` +# over this window; only paid on ``--continue`` launches, so healthy fresh +# boots cost nothing and a dead REPL is detected fast (early-return). +_REPL_LIVENESS_GRACE_SEC = 2.0 +_REPL_LIVENESS_POLL_SEC = 0.25 + # Per-turn timeout: how long ANY single in-flight turn can be at the # HEAD of ``_inflight_metas`` without its ``stop_hook_summary`` landing # before the watchdog considers it stuck and triggers ``force_restart``. @@ -1677,6 +1691,55 @@ async def _spawn(): f"{_COLD_START_TIMEOUT_SEC}s" ) from None + # ``tmux new-session`` returning rc=0 means the session was + # created, NOT that the REPL is alive. ``claude --continue`` + # exits 1 when the newest transcript for cwd isn't interactive- + # resumable — the classic trigger is SDK-origin transcripts left + # behind after an SDK→tmux transport switch. ``_has_prior_transcript`` + # green-lights ``--continue`` (a ``*.jsonl`` does exist), the REPL + # dies on boot, and the detached session is auto-reaped — leaving + # the state machine CONNECTED against a dead REPL and warm-wakes + # crash-looping forever. Detect the dead REPL and retry ONCE with + # a forced-fresh launch so the agent comes up on a new transcript + # instead of wedging. Only ``--continue`` launches can hit this, + # so healthy fresh boots pay nothing. Builds on #511 (which only + # gates ``--continue`` on file existence). + if self._last_launch_used_continue and not await self._verify_repl_survived_launch(): + _log( + f"tmux[{self.agent_name}]: --continue REPL died on launch " + f"(unresumable transcript for cwd); retrying with fresh context" + ) + # Reap any remnant, then re-launch fresh. Setting the one-shot + # flag makes ``_build_claude_cmd`` drop ``--continue``; it is + # consumed at the end of this method once the launch succeeds + # as a unit (REPL + tailer). + try: + await self._tmux.kill_session() + except Exception: + pass + self._config.force_fresh_context_once = True + claude_cmd = self._build_claude_cmd() + try: + await asyncio.wait_for(_spawn(), timeout=_COLD_START_TIMEOUT_SEC) + except asyncio.TimeoutError: + try: + await self._tmux.kill_session() + except Exception: + pass + raise RuntimeError( + f"tmux[{self.agent_name}]: cold-start timed out after " + f"{_COLD_START_TIMEOUT_SEC}s (fresh retry)" + ) from None + if not await self._verify_repl_survived_launch(): + try: + await self._tmux.kill_session() + except Exception: + pass + raise RuntimeError( + f"tmux[{self.agent_name}]: REPL died on launch even with " + f"fresh context — genuine cold-start failure" + ) + # NOTE: ``force_fresh_context_once`` consumption is deferred to # the end of this method (after tailer startup also succeeds), # NOT here — see the load-bearing comment at the consume site @@ -1722,6 +1785,35 @@ async def _spawn(): if self._last_launch_forced_fresh: self._config.force_fresh_context_once = False + async def _verify_repl_survived_launch(self) -> bool: + """Confirm the in-pane ``claude`` REPL is still running shortly + after ``tmux new-session`` returned. + + ``new-session`` returns rc=0 the instant the session is created, + even if the command inside it (``claude``) then exits immediately. + The detached session has no ``remain-on-exit``, so on that exit + tmux auto-reaps the whole session — and a rc=0 ``new-session`` is + therefore NOT proof of a live REPL. The canonical trigger is + ``claude --continue`` exiting 1 ("no conversation found to + continue") when the newest transcript for cwd isn't interactive- + resumable (e.g. an SDK-origin transcript after an SDK→tmux + transport switch). + + We poll ``has-session`` over a short grace window: a healthy REPL + keeps the session alive (returns True once the window elapses); a + died-on-launch REPL gets reaped, so the first poll that sees the + session gone returns False immediately (fast recovery — no need to + wait out the full window). Returns True if the session is still + alive at the end of the window, False if it vanished. + """ + deadline = time.monotonic() + _REPL_LIVENESS_GRACE_SEC + while True: + if not await self._tmux.has_session(): + return False + if time.monotonic() >= deadline: + return True + await asyncio.sleep(_REPL_LIVENESS_POLL_SEC) + def _build_claude_cmd(self) -> str: """Build the in-pane ``claude`` invocation as a single shell string. diff --git a/tests/test_tmux_session.py b/tests/test_tmux_session.py index 1fad861d..34b6cd21 100644 --- a/tests/test_tmux_session.py +++ b/tests/test_tmux_session.py @@ -96,9 +96,28 @@ def _make_mock_tmux(*, has_session_initial: bool = False) -> MagicMock: """ tmux = MagicMock(spec=_TmuxControl) tmux.session_name = "pinky-test" - tmux.has_session = AsyncMock(return_value=has_session_initial) - tmux.new_session = AsyncMock(return_value=_ok()) - tmux.kill_session = AsyncMock(return_value=_ok()) + # ``has_session`` tracks real tmux lifecycle: it reports + # ``has_session_initial`` until ``new_session`` is awaited (the + # pre-spawn stale-reap check), then True (a live REPL keeps the + # session alive — so the post-launch liveness check passes), and + # False again after ``kill_session``. Tests that simulate a REPL + # dying on launch override ``has_session`` directly. + _alive = {"v": has_session_initial} + + async def _has_session() -> bool: + return _alive["v"] + + async def _new_session(*_a, **_k) -> TmuxCommandResult: + _alive["v"] = True + return _ok() + + async def _kill_session(*_a, **_k) -> TmuxCommandResult: + _alive["v"] = False + return _ok() + + tmux.has_session = AsyncMock(side_effect=_has_session) + tmux.new_session = AsyncMock(side_effect=_new_session) + tmux.kill_session = AsyncMock(side_effect=_kill_session) tmux.send_keys = AsyncMock(return_value=_ok()) tmux.paste_text = AsyncMock(return_value=_ok()) tmux.capture_pane = AsyncMock(return_value=_ok()) @@ -140,6 +159,15 @@ def _make_session( return ss, tmux +@pytest.fixture(autouse=True) +def _fast_repl_liveness(monkeypatch): + """Zero the post-launch liveness grace/poll so unit tests don't sleep + real seconds. The liveness *logic* (retry on dead REPL) is exercised + via the mock's ``has_session`` flipping, independent of wall-clock.""" + monkeypatch.setattr(tmux_session, "_REPL_LIVENESS_GRACE_SEC", 0.0) + monkeypatch.setattr(tmux_session, "_REPL_LIVENESS_POLL_SEC", 0.0) + + # ────────────────────────────────────────────────────────────────────────── # Construction + identity # ────────────────────────────────────────────────────────────────────────── @@ -269,6 +297,95 @@ async def test_cold_start_omits_continue_when_no_prior_transcript( assert "--continue" not in cmd +@pytest.mark.asyncio +async def test_continue_relaunches_fresh_when_repl_dies_on_launch( + tmp_path, monkeypatch +) -> None: + """Regression: a ``*.jsonl`` exists so ``_has_prior_transcript`` is + True and the first launch uses ``--continue`` — but the newest + transcript isn't interactive-resumable (e.g. SDK-origin after an + SDK→tmux transport switch), so ``claude --continue`` exits 1 and the + detached session is auto-reaped. The cold-start must detect the dead + REPL and retry ONCE with a fresh launch (no ``--continue``) so the + agent comes up instead of wedging CONNECTED-against-dead-REPL. + """ + monkeypatch.setenv("HOME", str(tmp_path)) + tmux = _make_mock_tmux() + spawn_count = {"n": 0} + + async def _new_session(*_a, **_k) -> TmuxCommandResult: + spawn_count["n"] += 1 + return _ok() + + async def _has_session() -> bool: + # Dead after the 1st (--continue) spawn; alive after the 2nd (fresh). + return spawn_count["n"] >= 2 + + tmux.new_session = AsyncMock(side_effect=_new_session) + tmux.has_session = AsyncMock(side_effect=_has_session) + ss, _ = _make_session(tmux=tmux) + project_dir = ss._project_dir() + project_dir.mkdir(parents=True, exist_ok=True) + (project_dir / "seed.jsonl").write_text("") + + await ss.connect() + + assert ss.state == SessionState.CONNECTED + assert tmux.new_session.await_count == 2 + first_cmd = tmux.new_session.call_args_list[0].kwargs["command"] + second_cmd = tmux.new_session.call_args_list[1].kwargs["command"] + assert "--continue" in first_cmd + assert "--continue" not in second_cmd + + +@pytest.mark.asyncio +async def test_repl_dead_even_after_fresh_retry_raises(tmp_path, monkeypatch) -> None: + """If the REPL dies on launch even after the fresh retry, that's a + genuine cold-start failure (not an unresumable-transcript issue) — + raise so the caller transitions DEAD rather than retrying forever. + """ + monkeypatch.setenv("HOME", str(tmp_path)) + tmux = _make_mock_tmux() + + async def _has_session() -> bool: + return False # REPL never survives launch + + tmux.has_session = AsyncMock(side_effect=_has_session) + ss, _ = _make_session(tmux=tmux) + project_dir = ss._project_dir() + project_dir.mkdir(parents=True, exist_ok=True) + (project_dir / "seed.jsonl").write_text("") + + with pytest.raises(RuntimeError, match="genuine cold-start failure"): + await ss.connect() + assert ss.state == SessionState.DEAD + # Original --continue spawn + exactly one fresh retry (no infinite loop). + assert tmux.new_session.await_count == 2 + + +@pytest.mark.asyncio +async def test_fresh_launch_skips_liveness_retry(tmp_path, monkeypatch) -> None: + """A fresh launch (no prior transcript → no ``--continue``) is never + subject to the died-on-launch retry, even if ``has_session`` reports + dead — only ``--continue`` launches can hit the 'no conversation + found' exit, so the liveness check is gated on that. + """ + # Empty HOME → no transcript → fresh launch. + monkeypatch.setenv("HOME", str(tmp_path)) + tmux = _make_mock_tmux() + + async def _has_session() -> bool: + return False + + tmux.has_session = AsyncMock(side_effect=_has_session) + ss, _ = _make_session(tmux=tmux) + + await ss.connect() + + # Exactly one spawn: the liveness/retry path is skipped for fresh launches. + assert tmux.new_session.await_count == 1 + + def test_has_prior_transcript_false_when_project_dir_missing( tmp_path, monkeypatch ) -> None: @@ -714,7 +831,7 @@ def test_build_repl_env_strips_whitespace_in_pinky_session_secret( @pytest.mark.asyncio -async def test_concurrent_cold_start_runs_one_tmux_spawn() -> None: +async def test_concurrent_cold_start_runs_one_tmux_spawn(tmp_path, monkeypatch) -> None: """PR6's canonical concurrent-connect race regression, applied to the greenfield tmux backend. Two concurrent connect() calls must result in exactly one tmux new-session. @@ -725,6 +842,12 @@ async def test_concurrent_cold_start_runs_one_tmux_spawn() -> None: the same-target in-flight branch — subscribes via InFlightHandle, inherits the owner's CONNECTED outcome. """ + # Isolate HOME → no prior transcript → fresh launch. Keeps this dedup + # test deterministic (independent of stray real-HOME transcripts) and + # off the ``--continue`` liveness-retry path, which is covered by its + # own tests and would otherwise re-spawn against this mock's blocking + # ``new_session`` (which doesn't simulate a live REPL). + monkeypatch.setenv("HOME", str(tmp_path)) tmux = _make_mock_tmux() release_spawn = asyncio.Event() spawn_started = asyncio.Event() @@ -893,7 +1016,7 @@ async def test_warm_wake_failure_drives_to_dead() -> None: @pytest.mark.asyncio -async def test_concurrent_warm_wake_runs_one_spawn() -> None: +async def test_concurrent_warm_wake_runs_one_spawn(tmp_path, monkeypatch) -> None: """Concurrent connect() on an IDLE_SLEEPING session must result in exactly one tmux spawn. Same shape as the cold-start Case A regression — caller A wins RECONNECTING ownership, caller B @@ -903,6 +1026,12 @@ async def test_concurrent_warm_wake_runs_one_spawn() -> None: direct-mutated CONNECTED — double-spawn, no subscriber protection. Post-fix: matrix subscriber path applies to warm-wake too. """ + # Isolate HOME → no prior transcript → fresh launch. Keeps this dedup + # test deterministic (independent of stray real-HOME transcripts) and + # off the ``--continue`` liveness-retry path, which is covered by its + # own tests and would otherwise re-spawn against this mock's blocking + # ``new_session`` (which doesn't simulate a live REPL). + monkeypatch.setenv("HOME", str(tmp_path)) tmux = _make_mock_tmux() release_spawn = asyncio.Event() spawn_started = asyncio.Event()