From fb6b42bcaf0216e822fe8fd9b14f937c7abd2bc0 Mon Sep 17 00:00:00 2001 From: timmy Date: Sat, 8 Aug 2026 18:54:08 +0000 Subject: [PATCH] feat: bound autonomous release command runtime (#329) --- README.md | 10 +++ docs/release-engine-spec.md | 10 ++- src/release_engine.py | 90 ++++++++++++++++++++++---- tests/test_release_engine.py | 121 ++++++++++++++++++++++++++++++++++- 4 files changed, 216 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 0e411d3..69b9482 100644 --- a/README.md +++ b/README.md @@ -270,6 +270,9 @@ python3 -m src.release_engine \ --repo stackchain/stackchain-dashboard \ --agent timmy \ --issue 19 \ + --agent-timeout 1800 \ + --test-timeout 900 \ + --termination-grace 5 \ --test-command 'python3 -m pytest tests/ -q' ``` @@ -286,6 +289,13 @@ text. Shell substitutions, redirects, and pipelines are intentionally not interpreted. Put any trusted shell workflow in a reviewed wrapper script and configure that script as `RELEASE_AGENT_COMMAND` instead. +Agent and test deadlines are independent. On expiry, the engine terminates the +command's entire process group, escalates from `SIGTERM` to `SIGKILL` after the +configured grace period, and records `agent_timed_out` or `tests_timed_out` +without advancing the release. The equivalent environment variables are +`RELEASE_AGENT_TIMEOUT`, `RELEASE_TEST_TIMEOUT`, and +`RELEASE_TERMINATION_GRACE`; every value must be positive. + Durable state defaults to `.release-engine/state.json`. Full behavior and safety gates are documented in [`docs/release-engine-spec.md`](docs/release-engine-spec.md). diff --git a/docs/release-engine-spec.md b/docs/release-engine-spec.md index 3ae4c13..2d4c1cd 100644 --- a/docs/release-engine-spec.md +++ b/docs/release-engine-spec.md @@ -14,13 +14,14 @@ Convert one eligible Gitea issue into a tested, traceable pull request without d - Optional explicit issue number; otherwise deterministic queue selection. - Coding-agent command, parsed into an argument vector. - Test command, parsed into an argument vector. +- Positive, independent coding-agent and test deadlines plus a termination grace period. - Local repository path and durable state-file path. ## State machine `discovered → claimed → agent_complete → tests_passed → pushed → pr_opened` -Terminal failure states are `claim_failed`, `agent_failed`, `tests_failed`, and `push_failed`. A rerun resumes from persisted state and never opens a duplicate PR. +Terminal failure states include `claim_failed`, `agent_failed`, `agent_timed_out`, `tests_failed`, `tests_timed_out`, and `push_failed`. A rerun resumes from persisted state and never opens a duplicate PR. ## Queue selection @@ -45,6 +46,10 @@ Terminal failure states are `claim_failed`, `agent_failed`, `tests_failed`, and - Empty or malformed commands abort before the issue claim. - Non-zero coding-agent exit blocks tests and PR creation. - Tests run using the configured command; stdout/stderr and exit code become evidence. +- Coding-agent and test commands each run in a new process session. Their deadlines default to 1800 and 900 seconds, respectively. +- On expiry, the engine sends `SIGTERM` to the command's entire process group, waits for the termination grace period (5 seconds by default), then sends `SIGKILL` to the group and reaps the command. This prevents descendants from keeping the worker wedged. +- Deadlines are configured with `--agent-timeout`, `--test-timeout`, and `--termination-grace`, or `RELEASE_AGENT_TIMEOUT`, `RELEASE_TEST_TIMEOUT`, and `RELEASE_TERMINATION_GRACE`. Non-positive values abort before issue discovery or claim. +- Timeout evidence is bounded in durable state. An agent timeout blocks tests; a test timeout blocks push and PR creation. ## PR and release gate @@ -71,3 +76,6 @@ Terminal failure states are `claim_failed`, `agent_failed`, `tests_failed`, and 6. Passing tests produce a linked PR request with evidence. 7. Existing state/PR prevents duplicate work. 8. Dry-run against live Gitea returns a plan and performs no mutation. +9. A timed-out command terminates its descendants and returns within its deadline plus grace period. +10. Agent and test timeouts persist distinct terminal states and block every later delivery stage. +11. Non-positive deadline configuration aborts before issue claim. diff --git a/src/release_engine.py b/src/release_engine.py index ae4305e..e0f44e0 100644 --- a/src/release_engine.py +++ b/src/release_engine.py @@ -5,6 +5,7 @@ import argparse import json import os import re +import signal import shlex import subprocess from dataclasses import asdict, dataclass @@ -31,6 +32,7 @@ class CommandResult: returncode: int stdout: str stderr: str + timed_out: bool = False @dataclass(frozen=True) @@ -42,6 +44,9 @@ class EngineConfig: agent_command: str test_command: str base_branch: str = "main" + agent_timeout_seconds: float = 1800.0 + test_timeout_seconds: float = 900.0 + termination_grace_seconds: float = 5.0 @dataclass(frozen=True) @@ -63,7 +68,14 @@ class Client(Protocol): class Runner(Protocol): - def run(self, command: list[str], cwd: Path, env: dict[str, str] | None = None) -> CommandResult: ... + def run( + self, + command: list[str], + cwd: Path, + env: dict[str, str] | None = None, + timeout_seconds: float | None = None, + termination_grace_seconds: float = 5.0, + ) -> CommandResult: ... class StateStore: @@ -124,19 +136,38 @@ def parse_command(command: str, name: str) -> list[str]: class ShellRunner: - def run(self, command: list[str], cwd: Path, env: dict[str, str] | None = None) -> CommandResult: + def run( + self, + command: list[str], + cwd: Path, + env: dict[str, str] | None = None, + timeout_seconds: float | None = None, + termination_grace_seconds: float = 5.0, + ) -> CommandResult: merged_env = os.environ.copy() if env: merged_env.update(env) - completed = subprocess.run( + process = subprocess.Popen( command, cwd=str(cwd), env=merged_env, shell=False, text=True, - capture_output=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + start_new_session=True, ) - return CommandResult(completed.returncode, completed.stdout, completed.stderr) + try: + stdout, stderr = process.communicate(timeout=timeout_seconds) + return CommandResult(process.returncode, stdout, stderr) + except subprocess.TimeoutExpired: + os.killpg(process.pid, signal.SIGTERM) + try: + stdout, stderr = process.communicate(timeout=termination_grace_seconds) + except subprocess.TimeoutExpired: + os.killpg(process.pid, signal.SIGKILL) + stdout, stderr = process.communicate() + return CommandResult(process.returncode, stdout, stderr, timed_out=True) class GiteaClient: @@ -207,6 +238,15 @@ class ReleaseEngine: self.store = StateStore(config.state_path) def execute(self, issue_number: int | None = None, dry_run: bool = False) -> dict[str, Any]: + deadlines = { + "agent timeout seconds": self.config.agent_timeout_seconds, + "test timeout seconds": self.config.test_timeout_seconds, + "termination grace seconds": self.config.termination_grace_seconds, + } + for name, value in deadlines.items(): + if value <= 0: + raise ValueError(f"{name} must be positive") + issue = select_issue(self.client.list_open_issues(self.config.repo), self.config.agent, issue_number) branch = branch_name(self.config.agent, issue) @@ -245,20 +285,38 @@ class ReleaseEngine: if branch_result.returncode != 0: raise RuntimeError(f"branch creation failed: {branch_result.stderr.strip()}") - agent_result = self.runner.run(agent_command, self.config.repo_path, { - "RELEASE_ISSUE_NUMBER": str(issue.number), - "RELEASE_ISSUE_TITLE": issue.title, - "RELEASE_ISSUE_BODY": issue.body, - "RELEASE_REPO": self.config.repo, - "RELEASE_BRANCH": branch, - }) + agent_result = self.runner.run( + agent_command, + self.config.repo_path, + { + "RELEASE_ISSUE_NUMBER": str(issue.number), + "RELEASE_ISSUE_TITLE": issue.title, + "RELEASE_ISSUE_BODY": issue.body, + "RELEASE_REPO": self.config.repo, + "RELEASE_BRANCH": branch, + }, + timeout_seconds=self.config.agent_timeout_seconds, + termination_grace_seconds=self.config.termination_grace_seconds, + ) + if agent_result.timed_out: + evidence = (agent_result.stdout + "\n" + agent_result.stderr).strip()[-2000:] + self.store.save(RunState(issue.number, branch, "agent_timed_out", evidence)) + raise RuntimeError(f"coding agent timed out: {evidence}") if agent_result.returncode != 0: self.store.save(RunState(issue.number, branch, "agent_failed", agent_result.stderr[-2000:])) raise RuntimeError(f"coding agent failed: {agent_result.stderr.strip()}") self.store.save(RunState(issue.number, branch, "agent_complete")) - test_result = self.runner.run(test_command, self.config.repo_path) + test_result = self.runner.run( + test_command, + self.config.repo_path, + timeout_seconds=self.config.test_timeout_seconds, + termination_grace_seconds=self.config.termination_grace_seconds, + ) evidence = (test_result.stdout + "\n" + test_result.stderr).strip()[-4000:] + if test_result.timed_out: + self.store.save(RunState(issue.number, branch, "tests_timed_out", evidence)) + raise RuntimeError(f"tests timed out: {evidence}") if test_result.returncode != 0: self.store.save(RunState(issue.number, branch, "tests_failed", evidence)) raise RuntimeError(f"tests failed: {evidence}") @@ -315,6 +373,9 @@ def _parse_args() -> argparse.Namespace: parser.add_argument("--api", default=os.getenv("GITEA_API", "http://127.0.0.1:3000/api/v1")) parser.add_argument("--agent-command", default=os.getenv("RELEASE_AGENT_COMMAND", "")) parser.add_argument("--test-command", default=os.getenv("RELEASE_TEST_COMMAND", "python3 -m pytest tests/ -q")) + parser.add_argument("--agent-timeout", type=float, default=float(os.getenv("RELEASE_AGENT_TIMEOUT", "1800"))) + parser.add_argument("--test-timeout", type=float, default=float(os.getenv("RELEASE_TEST_TIMEOUT", "900"))) + parser.add_argument("--termination-grace", type=float, default=float(os.getenv("RELEASE_TERMINATION_GRACE", "5"))) return parser.parse_args() @@ -332,6 +393,9 @@ def main() -> int: state_path=args.state, agent_command=args.agent_command, test_command=args.test_command, + agent_timeout_seconds=args.agent_timeout, + test_timeout_seconds=args.test_timeout, + termination_grace_seconds=args.termination_grace, ) result = ReleaseEngine(config, GiteaClient(args.api, token), ShellRunner()).execute( issue_number=args.issue, diff --git a/tests/test_release_engine.py b/tests/test_release_engine.py index 1926725..9ad635a 100644 --- a/tests/test_release_engine.py +++ b/tests/test_release_engine.py @@ -1,6 +1,8 @@ import json import sys import threading +import time +from dataclasses import replace from http.server import BaseHTTPRequestHandler, HTTPServer from pathlib import Path @@ -15,6 +17,7 @@ from src.release_engine import ( RunState, ShellRunner, StateStore, + _parse_args, branch_name, select_issue, ) @@ -41,6 +44,33 @@ def test_shell_runner_treats_metacharacters_as_literal_arguments(tmp_path): assert not injected.exists() +def test_shell_runner_times_out_and_terminates_spawned_children(tmp_path): + marker = tmp_path / "child-survived.txt" + child = ( + "import time; from pathlib import Path; " + f"time.sleep(0.3); Path({str(marker)!r}).write_text('survived')" + ) + parent = ( + "import subprocess, sys, time; " + f"subprocess.Popen([sys.executable, '-c', {child!r}]); " + "time.sleep(10)" + ) + + started = time.monotonic() + result = ShellRunner().run( + [sys.executable, "-c", parent], + tmp_path, + timeout_seconds=0.05, + termination_grace_seconds=0.05, + ) + elapsed = time.monotonic() - started + time.sleep(0.35) + + assert result.timed_out is True + assert elapsed < 0.5 + assert not marker.exists() + + def test_stale_token_error_explains_how_to_restore_cron_authentication(): class UnauthorizedHandler(BaseHTTPRequestHandler): def do_GET(self): @@ -94,10 +124,12 @@ class FakeRunner: self.results = list(results) self.commands = [] self.environments = [] + self.options = [] - def run(self, command, cwd, env=None): + def run(self, command, cwd, env=None, **options): self.commands.append(command) self.environments.append(env) + self.options.append(options) return self.results.pop(0) @@ -164,6 +196,41 @@ def test_empty_agent_command_fails_before_claim(tmp_path): assert client.claimed == [] +@pytest.mark.parametrize( + ("field", "value"), + [ + ("agent_timeout_seconds", 0), + ("test_timeout_seconds", -1), + ("termination_grace_seconds", 0), + ], +) +def test_non_positive_command_deadlines_fail_before_claim(tmp_path, field, value): + issue = Issue(14, "Engine", "body", "open", [], None) + client = FakeClient([issue]) + cfg = replace(config(tmp_path), **{field: value}) + + with pytest.raises(ValueError, match=field.replace("_", " ")): + ReleaseEngine(cfg, client, FakeRunner([])).execute(issue_number=14) + + assert client.claimed == [] + + +def test_command_deadlines_are_configurable_from_cli(monkeypatch): + monkeypatch.setattr(sys, "argv", [ + "release-engine", + "--repo", "stackchain/stackchain-dashboard", + "--agent-timeout", "120", + "--test-timeout", "45.5", + "--termination-grace", "2", + ]) + + args = _parse_args() + + assert args.agent_timeout == 120 + assert args.test_timeout == 45.5 + assert args.termination_grace == 2 + + def test_agent_failure_blocks_tests_and_pr(tmp_path): issue = Issue(14, "Engine", "body", "open", [], None) client = FakeClient([issue]) @@ -183,6 +250,33 @@ def test_agent_failure_blocks_tests_and_pr(tmp_path): assert client.created_prs == [] +def test_agent_timeout_records_terminal_state_and_blocks_tests(tmp_path): + issue = Issue(14, "Engine", "body", "open", [], None) + client = FakeClient([issue]) + runner = FakeRunner([ + CommandResult(0, "", ""), + CommandResult(-9, "partial output", "stalled", timed_out=True), + ]) + cfg = config(tmp_path) + + with pytest.raises(RuntimeError, match="coding agent timed out"): + ReleaseEngine(cfg, client, runner).execute(issue_number=14) + + state = StateStore(cfg.state_path).load() + assert state is not None + assert state.status == "agent_timed_out" + assert "partial output" in state.evidence + assert runner.commands == [ + ["git", "checkout", "-B", "timmy/14-engine"], + ["agent", "--issue-from-env"], + ] + assert runner.options[1] == { + "timeout_seconds": cfg.agent_timeout_seconds, + "termination_grace_seconds": cfg.termination_grace_seconds, + } + assert client.created_prs == [] + + def test_issue_content_reaches_agent_only_through_environment(tmp_path): title = "$(touch /tmp/title-injected); `id`" body = "line one\n&& touch /tmp/body-injected > stolen" @@ -229,6 +323,31 @@ def test_failed_tests_block_push_and_pr(tmp_path): assert client.created_prs == [] +def test_test_timeout_records_terminal_state_and_blocks_push(tmp_path): + issue = Issue(14, "Engine", "body", "open", [], None) + client = FakeClient([issue]) + runner = FakeRunner([ + CommandResult(0, "", ""), + CommandResult(0, "implemented", ""), + CommandResult(-15, "3 passed before stall", "", timed_out=True), + ]) + cfg = config(tmp_path) + + with pytest.raises(RuntimeError, match="tests timed out"): + ReleaseEngine(cfg, client, runner).execute(issue_number=14) + + state = StateStore(cfg.state_path).load() + assert state is not None + assert state.status == "tests_timed_out" + assert "3 passed before stall" in state.evidence + assert runner.options[2] == { + "timeout_seconds": cfg.test_timeout_seconds, + "termination_grace_seconds": cfg.termination_grace_seconds, + } + assert not any(command[:2] == ["git", "push"] for command in runner.commands) + assert client.created_prs == [] + + def test_passing_run_pushes_and_opens_linked_pr(tmp_path): issue = Issue(14, "Engine MVP", "body", "open", ["P1"], None) client = FakeClient([issue]) -- 2.43.0