Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions src/pinky_daemon/tmux_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,20 @@
# 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``.
Expand Down Expand Up @@ -1677,6 +1691,55 @@
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:

Check notice

Code scanning / CodeQL

Empty except Note

'except' clause does nothing but pass and there is no explanatory comment.
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:

Check notice

Code scanning / CodeQL

Empty except Note

'except' clause does nothing but pass and there is no explanatory comment.
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:

Check notice

Code scanning / CodeQL

Empty except Note

'except' clause does nothing but pass and there is no explanatory comment.
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
Expand Down Expand Up @@ -1722,6 +1785,35 @@
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.

Expand Down
139 changes: 134 additions & 5 deletions tests/test_tmux_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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
# ──────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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.
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down
Loading