Skip to content

Commit 09cb047

Browse files
committed
feat(cli): --timeout wall-clock watchdog for --diff (#70 mitigation)
Claude-Session: https://claude.ai/code/session_0158oZrMt7GUUdyPq1ehvxom
1 parent eb7a694 commit 09cb047

4 files changed

Lines changed: 136 additions & 24 deletions

File tree

QA.md

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,14 +153,13 @@ shadow:
153153
cd test-repos/<repo>
154154
git pull --ff-only
155155
# Hard-cap runtime. `--timeout` (true wall-clock bound, worker thread + watchdog,
156-
# fast-fails instead of hanging to OOM, #70, 3725f51b) lives ONLY on the
157-
# standalone Rust binary `diffctx/src/main.rs`. The **Python pipx wrapper does
158-
# NOT expose `--timeout` at all** — verified on the released `1.10.2`:
159-
# `diffctx --diff --help | grep timeout` → empty, and `grep -r timeout
160-
# src/diffctx/cli.py` → empty (the flag never crossed into the Python CLI). So
161-
# the external perl cap is MANDATORY for every pipx smoke regardless of release,
162-
# not just "until it ships" — the wrapper needs its own watchdog around the
163-
# pyo3 `compute_scored_state` call first. macOS has no `timeout`; use:
156+
# fast-fails instead of hanging to OOM, #70, 3725f51b) exists on the standalone
157+
# Rust binary AND — since the 2026-07-14 QA pass — on the Python CLI
158+
# (`_call_with_wall_clock_deadline` in `src/diffctx/main.py`, exit 124, daemon
159+
# worker + `os._exit` because a runaway pyo3 call cannot be cancelled). The
160+
# perl cap stays MANDATORY for pipx smokes until a release ≥1.12 ships that
161+
# commit (the released binary on PATH predates it); after that, `--timeout 200`
162+
# on the pipx binary replaces the perl wrapper. macOS has no `timeout`; use:
164163
perl -e 'alarm 200; exec @ARGV' /Users/nikolay/.local/bin/diffctx . --diff HEAD~1
165164
# (or, on the dev build, just pass `--timeout 200` — the watchdog bounds it.)
166165
```

src/diffctx/cli.py

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
DEFAULT_MAX_FILE_BYTES = 256 * 1024 # 256 KB
1515
_DEFAULT_ALPHA = 0.60
1616
_DEFAULT_TAU = 0.12
17+
# Mirrors DEFAULT_PIPELINE_TIMEOUT_SECONDS in diffctx/src/config/limits.rs.
18+
_DEFAULT_TIMEOUT = 300
1719

1820

1921
class _Unset:
@@ -68,6 +70,11 @@ def _validate_budget(budget: int | None) -> None:
6870
_exit_usage_error(f"--budget must be >= -1 (-1 = unlimited, 0 = changed code only), got {budget}")
6971

7072

73+
def _validate_timeout(timeout: int) -> None:
74+
if timeout < 1:
75+
_exit_usage_error(f"--timeout must be >= 1 second, got {timeout}")
76+
77+
7178
def _validate_alpha(alpha: float) -> None:
7279
if not (0 < alpha < 1):
7380
_exit_usage_error(f"--alpha must be between 0 and 1 (exclusive), got {alpha}")
@@ -241,6 +248,7 @@ class ParsedArgs:
241248
alpha: float = _DEFAULT_ALPHA
242249
tau: float = _DEFAULT_TAU
243250
scoring: str = "ego"
251+
timeout: int = _DEFAULT_TIMEOUT
244252
full_diff: bool = False
245253
command: str | None = None
246254
graph: GraphArgs | None = None
@@ -299,6 +307,7 @@ class ParsedArgs:
299307
2 usage error (unknown flag or invalid value)
300308
3 environment error (git missing, not a repository, unknown revision)
301309
4 --diff produced no context (clean tree or empty range)
310+
124 --diff exceeded the --timeout wall-clock deadline
302311
"""
303312

304313

@@ -472,6 +481,16 @@ def _build_main_parser(prog: str = "diffctx", version: str = __version__) -> arg
472481
"bm25 = lexical similarity, for sparse cross-file structure"
473482
),
474483
)
484+
diff_group.add_argument(
485+
"--timeout",
486+
type=int,
487+
default=_UNSET,
488+
metavar="SECONDS",
489+
help=(
490+
f"Wall-clock deadline for --diff analysis (default: {_DEFAULT_TIMEOUT}); "
491+
"on expiry diffctx aborts with exit code 124 instead of hanging"
492+
),
493+
)
475494
diff_group.add_argument(
476495
"--full",
477496
action="store_true",
@@ -495,6 +514,8 @@ def _warn_diff_only_flags(args: argparse.Namespace) -> None:
495514
used.append("--full")
496515
if args.scoring is not _UNSET:
497516
used.append("--scoring")
517+
if args.timeout is not _UNSET:
518+
used.append("--timeout")
498519
if used:
499520
flags = ", ".join(used)
500521
_warn(f"diff-mode flags ignored without --diff: {flags}")
@@ -564,15 +585,17 @@ def _resolve_max_file_bytes(args: argparse.Namespace) -> int | None:
564585
return _validate_max_file_bytes(value, args.no_file_size_limit)
565586

566587

567-
def _resolve_diff_params(args: argparse.Namespace) -> tuple[str | None, int | None, float, float, str]:
588+
def _resolve_diff_params(args: argparse.Namespace) -> tuple[str | None, int | None, float, float, str, int]:
568589
budget = None if args.budget is _UNSET else args.budget
569590
alpha = _DEFAULT_ALPHA if args.alpha is _UNSET else args.alpha
570591
tau = _DEFAULT_TAU if args.tau is _UNSET else args.tau
571592
scoring = "ego" if args.scoring is _UNSET else args.scoring
593+
timeout = _DEFAULT_TIMEOUT if args.timeout is _UNSET else args.timeout
572594

573595
_validate_budget(budget)
574596
_validate_alpha(alpha)
575597
_validate_tau(tau)
598+
_validate_timeout(timeout)
576599
_warn_diff_only_flags(args)
577600
_warn_full_selection_conflict(args)
578601
if args.diff_range and not args.full and args.alpha is not _UNSET and scoring != "ppr":
@@ -583,13 +606,13 @@ def _resolve_diff_params(args: argparse.Namespace) -> tuple[str | None, int | No
583606
diff_range = "HEAD"
584607
if diff_range and args.no_ignores:
585608
_exit_usage_error("--no-ignores is not supported with --diff (git's own ignore rules always apply in diff mode)")
586-
return diff_range, budget, alpha, tau, scoring
609+
return diff_range, budget, alpha, tau, scoring, timeout
587610

588611

589612
def _build_tree_parsed_args(args: argparse.Namespace) -> ParsedArgs:
590613
_validate_max_depth(args.max_depth)
591614
max_file_bytes = _resolve_max_file_bytes(args)
592-
diff_range, budget, alpha, tau, scoring = _resolve_diff_params(args)
615+
diff_range, budget, alpha, tau, scoring, timeout = _resolve_diff_params(args)
593616
_warn_quiet_log_level_conflict(args)
594617

595618
dirs, files = _expand_paths(args.paths)
@@ -623,6 +646,7 @@ def _build_tree_parsed_args(args: argparse.Namespace) -> ParsedArgs:
623646
alpha=alpha,
624647
tau=tau,
625648
scoring=scoring,
649+
timeout=timeout,
626650
full_diff=args.full,
627651
extra_dirs=extra_dirs,
628652
extra_files=extra_files,

src/diffctx/main.py

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@
22

33
import argparse
44
import logging
5+
import os
56
import re
67
import signal
78
import subprocess
89
import sys
910
from pathlib import Path
1011
from typing import TYPE_CHECKING, Any
1112

13+
if TYPE_CHECKING:
14+
from collections.abc import Callable
15+
1216
from .version import __version__
1317

1418
if TYPE_CHECKING:
@@ -20,6 +24,7 @@
2024
_EXIT_USAGE = 2
2125
_EXIT_ENVIRONMENT = 3
2226
_EXIT_EMPTY_DIFF = 4
27+
_EXIT_TIMEOUT = 124
2328
_EXIT_INTERRUPTED = 130
2429
_EXIT_BROKEN_PIPE = 141
2530

@@ -104,24 +109,64 @@ def _warn_empty_diff_result(result: dict[str, Any], prog: str, args: ParsedArgs)
104109
)
105110

106111

112+
def _call_with_wall_clock_deadline(build: Callable[[], dict[str, Any]], timeout_seconds: int, prog: str) -> dict[str, Any]:
113+
# The Rust extension releases the GIL but offers no cancellation, so a
114+
# pathological repo can hang far past any per-phase git timeout (#70).
115+
# Mirror the standalone binary's watchdog: run the pipeline on a worker
116+
# thread and hard-exit 124 on deadline — the runaway worker cannot be
117+
# stopped, so finalizers must not run (os._exit, not sys.exit).
118+
import threading
119+
120+
outcome: list[Any] = []
121+
122+
def worker() -> None:
123+
try:
124+
outcome.append(("ok", build()))
125+
except BaseException as exc:
126+
outcome.append(("err", exc))
127+
128+
thread = threading.Thread(target=worker, name="diffctx-pipeline", daemon=True)
129+
thread.start()
130+
thread.join(timeout_seconds)
131+
if thread.is_alive():
132+
print(
133+
f"{prog}: pipeline exceeded {timeout_seconds}s wall-clock deadline; aborting before "
134+
"OOM/SIGKILL. Narrow the review with an explicit '--diff <from>..<to>' range, "
135+
"run on a smaller subtree, or raise '--timeout'.",
136+
file=sys.stderr,
137+
)
138+
sys.stderr.flush()
139+
sys.stdout.flush()
140+
os._exit(_EXIT_TIMEOUT)
141+
status, value = outcome[0]
142+
if status == "err":
143+
raise value
144+
return value # type: ignore[no-any-return]
145+
146+
107147
def _build_diff_tree(args: ParsedArgs, prog: str) -> dict[str, Any]:
108148
from .diffctx import build_diff_context
109149

110150
if not args.diff_range:
111151
raise RuntimeError("diff_range is required in diff mode")
112152
_ensure_git_repo(args.root_dir, prog)
113-
result = build_diff_context(
114-
root_dir=args.root_dir,
115-
diff_range=args.diff_range,
116-
budget_tokens=args.budget,
117-
alpha=args.alpha,
118-
tau=args.tau,
119-
no_content=args.no_content,
120-
ignore_file=args.ignore_file,
121-
no_default_ignores=args.no_default_ignores,
122-
full=args.full_diff,
123-
whitelist_file=args.whitelist_file,
124-
scoring_mode=getattr(args, "scoring", "ego"),
153+
result = _call_with_wall_clock_deadline(
154+
lambda: build_diff_context(
155+
root_dir=args.root_dir,
156+
diff_range=args.diff_range or "HEAD",
157+
budget_tokens=args.budget,
158+
alpha=args.alpha,
159+
tau=args.tau,
160+
no_content=args.no_content,
161+
ignore_file=args.ignore_file,
162+
no_default_ignores=args.no_default_ignores,
163+
full=args.full_diff,
164+
whitelist_file=args.whitelist_file,
165+
scoring_mode=getattr(args, "scoring", "ego"),
166+
timeout=args.timeout,
167+
),
168+
args.timeout,
169+
prog,
125170
)
126171
_warn_empty_diff_result(result, prog, args)
127172
return result

tests/test_e2e_cli_scenarios.py

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,25 @@
1010
from __future__ import annotations
1111

1212
import json
13+
import os
1314
import re
15+
import subprocess
16+
import sys
1417

1518
import pytest
1619
import yaml
1720

1821
from tests.framework.pygit2_backend import Pygit2Repo
1922
from tests.garbage_data import GARBAGE_FILES
2023

21-
from .conftest import run_diffctx_subprocess
24+
from .conftest import SRC_DIR, run_diffctx_subprocess
2225

2326
EXIT_OK = 0
2427
EXIT_RUNTIME = 1
2528
EXIT_USAGE = 2
2629
EXIT_ENVIRONMENT = 3
2730
EXIT_EMPTY_DIFF = 4
31+
EXIT_TIMEOUT = 124
2832

2933

3034
@pytest.fixture
@@ -282,6 +286,46 @@ def test_diff_invalid_range_fails_cleanly(self, diff_repo):
282286
assert "unknown git revision 'no_such_ref..HEAD'" in result.stderr
283287
assert "internal error" not in result.stderr
284288

289+
def test_timeout_flag_accepted_and_diff_completes(self, diff_repo):
290+
result = run_diffctx_subprocess([".", "--diff", "HEAD~1..HEAD", "--timeout", "300", "-f", "yaml"], cwd=diff_repo.path)
291+
assert result.returncode == EXIT_OK
292+
assert yaml.safe_load(result.stdout)["type"] == "diff_context"
293+
294+
def test_timeout_below_one_second_is_usage_error(self, diff_repo):
295+
result = run_diffctx_subprocess([".", "--diff", "HEAD~1..HEAD", "--timeout", "0"], cwd=diff_repo.path)
296+
assert result.returncode == EXIT_USAGE
297+
assert "--timeout must be >= 1" in result.stderr
298+
299+
def test_timeout_without_diff_warns_and_is_ignored(self, temp_project):
300+
result = run_diffctx_subprocess([".", "--timeout", "5"], cwd=temp_project)
301+
assert result.returncode == EXIT_OK
302+
assert "diff-mode flags ignored without --diff" in result.stderr
303+
assert "--timeout" in result.stderr
304+
305+
def test_expired_deadline_aborts_with_exit_124(self):
306+
"""The wall-clock watchdog must hard-abort a pipeline that outlives
307+
--timeout (#70): a runaway Rust computation cannot be cancelled from
308+
Python, so the process exits 124 like the standalone binary. Exercised
309+
in a real subprocess with a genuinely slow (sleeping) pipeline call."""
310+
watchdog_script = (
311+
"import time\n"
312+
"from diffctx.main import _call_with_wall_clock_deadline\n"
313+
"_call_with_wall_clock_deadline(lambda: time.sleep(60), 1, 'diffctx')\n"
314+
)
315+
env = os.environ.copy()
316+
env["PYTHONPATH"] = str(SRC_DIR)
317+
result = subprocess.run(
318+
[sys.executable, "-c", watchdog_script],
319+
capture_output=True,
320+
text=True,
321+
env=env,
322+
timeout=30,
323+
check=False,
324+
)
325+
assert result.returncode == EXIT_TIMEOUT
326+
assert "wall-clock deadline" in result.stderr
327+
assert "--timeout" in result.stderr
328+
285329
def test_diff_to_clipboard_writes_file_too(self, diff_repo, tmp_path):
286330
out = tmp_path / "diff.yaml"
287331
result = run_diffctx_subprocess([".", "--diff", "HEAD~1..HEAD", "-f", "yaml", "-o", str(out)], cwd=diff_repo.path)

0 commit comments

Comments
 (0)