From c3587986272bd8bd20a98c9bccc6fc71bc9fd5b0 Mon Sep 17 00:00:00 2001 From: Dobromir Popov Date: Mon, 29 Jun 2026 15:54:00 +0300 Subject: [PATCH] chore: harden Ralph session controls --- scripts/ralph_progress.py | 92 +++++++++++++++++++++++++++++++-------- 1 file changed, 75 insertions(+), 17 deletions(-) diff --git a/scripts/ralph_progress.py b/scripts/ralph_progress.py index 54a5a92..8127169 100644 --- a/scripts/ralph_progress.py +++ b/scripts/ralph_progress.py @@ -17,6 +17,7 @@ import argparse import datetime as dt import json import os +import select import shutil import subprocess import sys @@ -132,6 +133,20 @@ def _ralph_status_json() -> dict[str, Any] | None: return {"status": "unknown", "raw": result.stdout.strip()} +def _positive_float(value: str) -> float: + parsed = float(value) + if parsed <= 0: + raise argparse.ArgumentTypeError("value must be > 0") + return parsed + + +def _non_negative_float(value: str) -> float: + parsed = float(value) + if parsed < 0: + raise argparse.ArgumentTypeError("value must be >= 0") + return parsed + + def print_dashboard(prd_path: Path, *, include_git: bool = False, include_ralph: bool = True) -> None: data = _load_prd(prd_path) stories = _stories(data) @@ -211,13 +226,21 @@ def _run_with_updates(cmd: list[str], prd_path: Path, *, interval: float) -> int ) next_update = time.monotonic() + interval assert proc.stdout is not None - for line in proc.stdout: - print(line, end="") + stdout_fd = proc.stdout.fileno() + while proc.poll() is None: + timeout = max(0.1, next_update - time.monotonic()) + readable, _, _ = select.select([stdout_fd], [], [], timeout) + if readable: + line = proc.stdout.readline() + if line: + print(line, end="") if time.monotonic() >= next_update: print("\n--- Ralph progress update ---") print_dashboard(prd_path, include_git=True, include_ralph=True) print("--- end update ---\n") next_update = time.monotonic() + interval + for line in proc.stdout: + print(line, end="") return proc.wait() @@ -230,7 +253,7 @@ def command_watch(args: argparse.Namespace) -> int: while True: os.system("clear" if sys.stdout.isatty() else "true") print(dt.datetime.now().isoformat(timespec="seconds")) - print_dashboard(args.prd, include_git=args.git, include_ralph=True) + print_dashboard(args.prd, include_git=args.git, include_ralph=not args.no_ralph_status) if args.once: return 0 time.sleep(args.interval) @@ -256,10 +279,15 @@ def command_run_next(args: argparse.Namespace) -> int: def command_auto(args: argparse.Namespace) -> int: completed_this_run = 0 while True: - data = _load_prd(args.prd) - story = _next_ready(data) + data_before = _load_prd(args.prd) + story = _next_ready(data_before) if story is None: - print("No ready tasks remain. Backlog is complete or blocked.") + _, _, blocked = _story_sets(data_before) + if blocked: + print("No ready tasks remain; backlog is blocked, not complete.") + print_dashboard(args.prd, include_git=True, include_ralph=True) + return 2 + print("No ready tasks remain. Backlog is complete.") print_dashboard(args.prd, include_git=True, include_ralph=True) return 0 if args.max_tasks is not None and completed_this_run >= args.max_tasks: @@ -278,6 +306,14 @@ def command_auto(args: argparse.Namespace) -> int: if code != 0: return code completed_this_run += 1 + data_after = _load_prd(args.prd) + before_done = {str(story.get("id")) for story in _story_sets(data_before)[0]} + after_done = {str(story.get("id")) for story in _story_sets(data_after)[0]} + next_after = _next_ready(data_after) + if after_done == before_done and next_after and next_after.get("id") == story.get("id"): + raise SystemExit( + f"Ralph exited without advancing {story.get('id')}; stopping to avoid an infinite auto loop." + ) if _git_dirty() and not args.allow_dirty: print(_git_status_short()) raise SystemExit( @@ -295,14 +331,31 @@ def _read_first_lines(path: Path, limit: int = 80) -> str: return "\n".join(lines[:limit]) +IGNORED_REVIEW_PARTS = {".git", ".venv", "__pycache__", "build", "dist", ".mypy_cache", ".pytest_cache"} + + +def _review_path_allowed(path: Path) -> bool: + return not (IGNORED_REVIEW_PARTS & set(path.relative_to(REPO_ROOT).parts)) + + +def _iter_review_files(pattern: str) -> list[Path]: + return [path for path in REPO_ROOT.glob(pattern) if path.is_file() and _review_path_allowed(path)] + + +def _iter_files(root: Path, pattern: str) -> list[Path]: + return [path for path in root.rglob(pattern) if path.is_file() and _review_path_allowed(path)] + + def _review_report(prd_path: Path) -> str: data = _load_prd(prd_path) stories = _stories(data) issue_files = sorted((prd_path.parent / "issues").glob("*.md")) adr_files = sorted((REPO_ROOT / "docs/adr").glob("*.md")) - grill_files = sorted(REPO_ROOT.glob("**/*grill*.md")) - code_roots = [REPO_ROOT / "packages", REPO_ROOT / "tests"] - code_files = [p for root in code_roots if root.exists() for p in root.rglob("*.py")] + doc_files = sorted((REPO_ROOT / "docs").rglob("*.md")) + grill_files = sorted(_iter_review_files("**/*grill*.md")) + code_roots = [REPO_ROOT / "packages", REPO_ROOT / "tests", REPO_ROOT / "scripts"] + code_files = sorted(path for root in code_roots if root.exists() for path in _iter_files(root, "*.py")) + config_files = [path for pattern in ("*.toml", "*.md") for path in _iter_review_files(pattern)] lines = [ "# Task / docs / code review brief", @@ -326,6 +379,8 @@ def _review_report(prd_path: Path) -> str: lines.extend(f"- {_rel(path)}" for path in issue_files) lines.extend(["", "### ADR / generated design docs", ""]) lines.extend(f"- {_rel(path)}" for path in adr_files) + lines.extend(["", "### Other docs", ""]) + lines.extend(f"- {_rel(path)}" for path in doc_files if path not in adr_files) if grill_files: lines.extend(["", "### Grill-generated docs", ""]) lines.extend(f"- {_rel(path)}" for path in grill_files) @@ -333,6 +388,8 @@ def _review_report(prd_path: Path) -> str: lines.extend(["", "### Grill-generated docs", "", "- No *grill*.md files found."]) lines.extend(["", "### Code files", ""]) lines.extend(f"- {_rel(path)}" for path in sorted(code_files)[:200]) + lines.extend(["", "### Config / top-level docs", ""]) + lines.extend(f"- {_rel(path)}" for path in sorted(config_files)[:100]) lines.extend([ "", @@ -353,6 +410,9 @@ def _review_report(prd_path: Path) -> str: def command_review(args: argparse.Namespace) -> int: + if args.run_ralph and _git_dirty() and not args.allow_dirty: + print(_git_status_short()) + raise SystemExit("Working tree is dirty. Commit/review changes first, or pass --allow-dirty.") report = _review_report(args.prd) output = args.output if output is None: @@ -364,9 +424,6 @@ def command_review(args: argparse.Namespace) -> int: print(f"Review brief written: {_rel(output)}") if not args.run_ralph: return 0 - if _git_dirty() and not args.allow_dirty: - print(_git_status_short()) - raise SystemExit("Working tree is dirty. Commit/review changes first, or pass --allow-dirty.") prompt = tempfile.NamedTemporaryFile("w", encoding="utf-8", suffix=".md", prefix="ralph-review-", delete=False) prompt_path = Path(prompt.name) with prompt: @@ -401,15 +458,16 @@ def build_parser() -> argparse.ArgumentParser: watch = subparsers.add_parser("watch", help="Refresh progress every N seconds") watch.add_argument("--prd", type=Path, default=DEFAULT_PRD) - watch.add_argument("--interval", type=float, default=DEFAULT_INTERVAL) + watch.add_argument("--interval", type=_positive_float, default=DEFAULT_INTERVAL) watch.add_argument("--git", action="store_true") + watch.add_argument("--no-ralph-status", action="store_true", help="Skip ralph-tui status lookup") watch.add_argument("--once", action="store_true") watch.set_defaults(func=command_watch) run_next = subparsers.add_parser("run-next", help="Start one fresh Ralph session for the next ready task") run_next.add_argument("--prd", type=Path, default=DEFAULT_PRD) run_next.add_argument("--agent", default=DEFAULT_AGENT) - run_next.add_argument("--interval", type=float, default=DEFAULT_INTERVAL, help="Progress update interval while Ralph runs") + run_next.add_argument("--interval", type=_positive_float, default=DEFAULT_INTERVAL, help="Progress update interval while Ralph runs") run_next.add_argument("--allow-dirty", action="store_true") run_next.add_argument("--dry-run", action="store_true") run_next.set_defaults(func=command_run_next) @@ -417,8 +475,8 @@ def build_parser() -> argparse.ArgumentParser: auto = subparsers.add_parser("auto", help="Run fresh Ralph sessions until complete/blocked") auto.add_argument("--prd", type=Path, default=DEFAULT_PRD) auto.add_argument("--agent", default=DEFAULT_AGENT) - auto.add_argument("--interval", type=float, default=DEFAULT_INTERVAL) - auto.add_argument("--delay", type=float, default=3.0, help="Delay between sessions") + auto.add_argument("--interval", type=_positive_float, default=DEFAULT_INTERVAL) + auto.add_argument("--delay", type=_non_negative_float, default=3.0, help="Delay between sessions") auto.add_argument("--max-tasks", type=int, default=None) auto.add_argument("--allow-dirty", action="store_true", help="Do not stop between dirty-tree Ralph sessions") auto.add_argument("--dry-run", action="store_true") @@ -429,7 +487,7 @@ def build_parser() -> argparse.ArgumentParser: review.add_argument("--output", type=Path, default=None) review.add_argument("--run-ralph", action="store_true", help="Start a fresh Ralph cleanup/review session with this brief") review.add_argument("--agent", default=DEFAULT_AGENT) - review.add_argument("--interval", type=float, default=DEFAULT_INTERVAL) + review.add_argument("--interval", type=_positive_float, default=DEFAULT_INTERVAL) review.add_argument("--allow-dirty", action="store_true") review.set_defaults(func=command_review)