Skip to content
Merged
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
62 changes: 62 additions & 0 deletions koan/app/claude_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import shlex
import subprocess
import sys
import threading
import time
from pathlib import Path
from typing import Callable, List, Optional, Tuple
Expand Down Expand Up @@ -247,13 +248,52 @@ def strip_cli_noise(text: str) -> str:
return "\n".join(lines).strip()


class _HeartbeatTimer:
"""Periodic heartbeat emitter for keeping outer watchdogs alive.

Prints a marker to sys.stdout at a fixed interval while the inner
CLI process runs in print mode (silent during tool use). The marker
goes to the skill subprocess's stdout, which the parent run.py reads
to reset its LivenessWatchdog. It does NOT appear in the inner CLI's
captured output (separate pipe).
"""

def __init__(self, proc: subprocess.Popen, interval: int):
self._proc = proc
self._interval = interval
self._stop = threading.Event()
self._thread = threading.Thread(target=self._run, daemon=True)

def start(self) -> "_HeartbeatTimer":
self._thread.start()
return self

def cancel(self):
self._stop.set()

def _run(self):
while not self._stop.wait(self._interval):
if self._proc.poll() is not None:
break
try:
print("[still working...]", flush=True)
except (OSError, ValueError):
break


def _start_heartbeat(proc: subprocess.Popen, interval: int) -> _HeartbeatTimer:
"""Start a heartbeat timer for the given subprocess."""
return _HeartbeatTimer(proc, interval).start()


def run_claude(
cmd: list,
cwd: str,
timeout: int = 600,
*,
idle_timeout: Optional[int] = None,
max_duration: Optional[int] = None,
heartbeat_interval: Optional[int] = None,
) -> dict:
"""Run a Claude Code CLI command, streaming stdout in real time.

Expand All @@ -273,6 +313,12 @@ def run_claude(
subprocesses) from holding the stdout pipe open and turning a
``TimeoutExpired`` into an indefinite hang during pipe drain.

Args:
heartbeat_interval: When set, a daemon thread prints a periodic
marker to sys.stdout while the CLI is running. This keeps
the parent process's LivenessWatchdog alive even when the
CLI runs in print mode (no output during tool use).

Returns:
Dict with keys: success (bool), output (str), error (str).
"""
Expand All @@ -299,6 +345,10 @@ def run_claude(
"error": f"Failed to spawn CLI: {e}",
}

heartbeat_thread = None
if heartbeat_interval and heartbeat_interval > 0:
heartbeat_thread = _start_heartbeat(proc, heartbeat_interval)

try:
stream_result = stream_with_timeout(
proc,
Expand All @@ -308,6 +358,8 @@ def run_claude(
max_duration=max_duration,
)
finally:
if heartbeat_thread is not None:
heartbeat_thread.cancel()
cleanup()

stdout_text = stream_result.stdout
Expand Down Expand Up @@ -555,13 +607,23 @@ def run_claude_step(
)

from app.commit_conventions import parse_commit_subject
from app.config import get_first_output_timeout

# Emit periodic heartbeat to keep the parent process's LivenessWatchdog
# alive. Print-mode CLI sessions produce no stdout during tool use,
# which would trigger the outer first_output_timeout (default 600s).
# Heartbeat at half the timeout interval ensures the watchdog is reset
# well before it fires.
_fot = get_first_output_timeout()
_heartbeat = max(60, _fot // 2) if _fot > 0 else 120

result = run_claude(
cmd,
project_path,
timeout=timeout,
idle_timeout=idle_timeout,
max_duration=max_duration,
heartbeat_interval=_heartbeat,
)
cleaned_output = strip_cli_noise(result.get("output", ""))
if result["success"]:
Expand Down
72 changes: 72 additions & 0 deletions koan/tests/test_claude_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,78 @@ def test_cleanup_called_on_success(self, mock_popen):
run_claude(["claude", "-p", "test"], "/project")
cleanup.assert_called_once()

@patch("app.claude_step.popen_cli")
def test_heartbeat_emits_while_cli_silent(self, mock_popen, capsys):
"""Heartbeat prints markers even when CLI produces no stdout.

This prevents the parent process's LivenessWatchdog from killing
skill subprocesses that run print-mode Claude sessions (no output
during tool use). Reproduces the bug where /recreate of large PRs
was killed after first_output_timeout (600s) because the inner CLI
was silent while doing tool work.
"""
import threading
import time

# Simulate a CLI that produces no output for a while then finishes.
output_event = threading.Event()

class _BlockingStream:
"""stdout that blocks until signalled, then yields nothing."""
def __init__(self, event):
self._event = event
def __iter__(self):
self._event.wait(timeout=5)
return iter([])
def close(self):
pass

class SlowProc:
pid = 99999
returncode = 0
stderr = _FakeStream(read_text="")

def __init__(self):
self._finished = False
self.stdout = _BlockingStream(output_event)

def wait(self, timeout=None):
return 0

def poll(self):
return 0 if self._finished else None

proc = SlowProc()
mock_popen.return_value = (proc, lambda: None)

# Use a very short heartbeat so the test runs fast
def run_with_heartbeat():
return run_claude(
["claude", "-p", "test"], "/project",
heartbeat_interval=1,
)

# Let the heartbeat emit a few markers then unblock
runner = threading.Thread(target=run_with_heartbeat)
runner.start()
time.sleep(2.5) # Allow ~2 heartbeat emissions
output_event.set()
runner.join(timeout=5)

captured = capsys.readouterr()
assert "[still working...]" in captured.out

@patch("app.claude_step.popen_cli")
def test_heartbeat_cancelled_after_completion(self, mock_popen):
"""Heartbeat timer is cancelled when CLI finishes."""
proc = _fake_proc(["ok\n"], returncode=0)
mock_popen.return_value = (proc, lambda: None)
result = run_claude(
["claude", "-p", "test"], "/project",
heartbeat_interval=1,
)
assert result["success"] is True


# ---------- commit_if_changes ----------

Expand Down
Loading