From f3174b784f79f9cb75c04d330e842984cd25a999 Mon Sep 17 00:00:00 2001 From: Eric Cao Date: Tue, 7 Jul 2026 17:55:02 +0800 Subject: [PATCH 1/8] =?UTF-8?q?fix(verification):=20honor=20'=E2=86=92=20n?= =?UTF-8?q?o=20output'=20acceptance-criteria=20expectations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parse expectations such as '→ no output' from spec Acceptance Criteria bullets. For no-output commands, treat exit codes 0/1 with empty stdout as passing instead of requiring exit code 0. Closes #64 --- src/owloop/spec_queue.py | 31 +++++++++++++++++++++---- src/owloop/verification.py | 34 +++++++++++++++++++++++++-- tests/test_spec_queue.py | 44 +++++++++++++++++++++++++++++++++++ tests/test_verifier.py | 47 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 6 deletions(-) diff --git a/src/owloop/spec_queue.py b/src/owloop/spec_queue.py index 161d534..d972f10 100644 --- a/src/owloop/spec_queue.py +++ b/src/owloop/spec_queue.py @@ -26,6 +26,7 @@ import fnmatch import re +from dataclasses import dataclass from pathlib import Path _COMPLETE_RE = re.compile(r"^(#{1,3} )?(\*\*)?Status(\*\*)?:\s+COMPLETE", re.MULTILINE) @@ -52,6 +53,23 @@ DEFAULT_PRIORITY = 999 +@dataclass(frozen=True) +class AcceptanceCriterion: + """A single shell command parsed from a spec's Acceptance Criteria bullet. + + The optional ``expect_no_output`` flag is set when the bullet carries an + expectation such as ``→ no output``. In that case the verifier treats a + command that exits 0/1 with empty stdout as passing, instead of requiring a + zero exit code (which grep naturally returns when it finds no matches). + """ + + command: str + expect_no_output: bool = False + + +_NO_OUTPUT_RE = re.compile(r"(?:→|->)\s*no output", re.IGNORECASE) + + def get_root_specs(specs_dir: Path) -> list[Path]: if not specs_dir.is_dir(): return [] @@ -158,10 +176,12 @@ def get_spec_dependencies(spec_file: Path, specs: list[Path]) -> list[Path]: return resolved -def get_acceptance_criteria_commands(spec_file: Path) -> list[str]: +def get_acceptance_criteria_commands(spec_file: Path) -> list[AcceptanceCriterion]: """Extract the first backtick-quoted shell command from each Acceptance Criteria bullet. Bullets without a backtick-quoted command (free-form descriptions) are skipped. + A trailing expectation such as ``→ no output`` is parsed so the verifier can + treat grep-style "no match" exit codes as passing. """ if not spec_file.is_file(): return [] @@ -170,7 +190,7 @@ def get_acceptance_criteria_commands(spec_file: Path) -> list[str]: if match is None: return [] - commands: list[str] = [] + commands: list[AcceptanceCriterion] = [] for line in match.group(1).splitlines(): stripped = line.strip() if "`" not in stripped: @@ -179,8 +199,11 @@ def get_acceptance_criteria_commands(spec_file: Path) -> list[str]: if len(parts) < 3: continue command = parts[1].strip() - if command: - commands.append(command) + if not command: + continue + tail = "".join(parts[2:]).strip() + expect_no_output = bool(_NO_OUTPUT_RE.search(tail)) + commands.append(AcceptanceCriterion(command, expect_no_output)) return commands diff --git a/src/owloop/verification.py b/src/owloop/verification.py index 67a2a4c..214bce8 100644 --- a/src/owloop/verification.py +++ b/src/owloop/verification.py @@ -60,14 +60,44 @@ def run_commands( return passed, failed, failures +def _tail_output(result: subprocess.CompletedProcess[str]) -> str: + output = f"{result.stdout or ''}\n{result.stderr or ''}".strip() + return "\n".join(output.splitlines()[-30:])[-2000:] + + def run_acceptance_criteria( cwd: Path, specs_dir: Path, spec_name: str | None ) -> tuple[int, int, list[dict[str, Any]]]: """Run a spec's Acceptance Criteria shell commands; count passes vs failures.""" if not spec_name: return 0, 0, [] - commands = spec_queue.get_acceptance_criteria_commands(specs_dir / spec_name) - return run_commands(cwd, commands) + + criteria = spec_queue.get_acceptance_criteria_commands(specs_dir / spec_name) + passed = failed = 0 + failures: list[dict[str, Any]] = [] + for criterion in criteria: + result = subprocess.run( # noqa: S602 + criterion.command, shell=True, cwd=cwd, capture_output=True, text=True, + ) + if criterion.expect_no_output: + # grep-style tools exit 1 when they find no matches; the expectation + # is about empty stdout, not the exit code. + ok = result.stdout.strip() == "" and result.returncode in (0, 1) + else: + ok = result.returncode == 0 + + if ok: + passed += 1 + else: + failed += 1 + failures.append( + { + "command": criterion.command, + "returncode": result.returncode, + "output": _tail_output(result), + } + ) + return passed, failed, failures def guarded_hash(cwd: Path, specs_dir: Path, spec_name: str | None) -> str: diff --git a/tests/test_spec_queue.py b/tests/test_spec_queue.py index fb15f04..6d16bd8 100644 --- a/tests/test_spec_queue.py +++ b/tests/test_spec_queue.py @@ -211,6 +211,50 @@ def test_get_acceptance_criteria_section_missing_returns_empty(tmp_path: Path) - assert spec_queue.get_acceptance_criteria_section(spec) == "" +# ── acceptance-criteria command extraction ── + + +def test_get_acceptance_criteria_commands_extracts_plain_command(tmp_path: Path) -> None: + spec = tmp_path / "01-t.md" + spec.write_text( + "# Spec\n\n## Acceptance Criteria\n- `pytest -q`\n- `ruff check .`\n", + encoding="utf-8", + ) + criteria = spec_queue.get_acceptance_criteria_commands(spec) + assert [c.command for c in criteria] == ["pytest -q", "ruff check ."] + assert all(not c.expect_no_output for c in criteria) + + +def test_get_acceptance_criteria_commands_detects_no_output(tmp_path: Path) -> None: + spec = tmp_path / "01-t.md" + spec.write_text( + "# Spec\n\n## Acceptance Criteria\n" + "- `grep foo bar.txt` → no output\n" + "- `grep baz qux.txt` -> no output\n" + "- `echo hi` → at least 1 match\n", + encoding="utf-8", + ) + criteria = spec_queue.get_acceptance_criteria_commands(spec) + assert [c.command for c in criteria] == [ + "grep foo bar.txt", + "grep baz qux.txt", + "echo hi", + ] + assert criteria[0].expect_no_output is True + assert criteria[1].expect_no_output is True + assert criteria[2].expect_no_output is False + + +def test_get_acceptance_criteria_commands_ignores_free_form_bullets(tmp_path: Path) -> None: + spec = tmp_path / "01-t.md" + spec.write_text( + "# Spec\n\n## Acceptance Criteria\n- just a free-form note\n- `true`\n", + encoding="utf-8", + ) + criteria = spec_queue.get_acceptance_criteria_commands(spec) + assert [c.command for c in criteria] == ["true"] + + # ── file-disjoint parallel scheduling (Phase 4) ── diff --git a/tests/test_verifier.py b/tests/test_verifier.py index 3e7f6b1..0bb0546 100644 --- a/tests/test_verifier.py +++ b/tests/test_verifier.py @@ -8,6 +8,7 @@ import subprocess from pathlib import Path +from owloop import verification from owloop.adapters import AgentResult, MockAdapter from owloop.engine import EngineConfig, OwloopEngine @@ -138,3 +139,49 @@ def test_no_verifier_keeps_existing_behavior(tmp_path: Path, monkeypatch) -> Non summary = engine.run() assert summary.stopped_reason == "success" + + +# ── deterministic gate: no-output expectations ── + + +def test_run_acceptance_criteria_treats_grep_no_match_as_pass(tmp_path: Path) -> None: + """A `→ no output` criterion must pass when grep finds nothing (exit 1).""" + specs = tmp_path / ".owloop" / "specs" + specs.mkdir(parents=True) + spec = specs / "01-test.md" + target = tmp_path / "api.py" + target.write_text("def keep(): pass\n", encoding="utf-8") + spec.write_text( + "# Spec\n\n## Acceptance Criteria\n" + f"- `grep removed {target.name}` → no output\n", + encoding="utf-8", + ) + + passed, failed, failures = verification.run_acceptance_criteria( + tmp_path, specs, "01-test.md" + ) + + assert passed == 1 + assert failed == 0 + assert failures == [] + + +def test_run_acceptance_criteria_no_output_fails_on_nonempty_stdout(tmp_path: Path) -> None: + specs = tmp_path / ".owloop" / "specs" + specs.mkdir(parents=True) + spec = specs / "01-test.md" + target = tmp_path / "api.py" + target.write_text("def removed(): pass\n", encoding="utf-8") + spec.write_text( + "# Spec\n\n## Acceptance Criteria\n" + f"- `grep removed {target.name}` → no output\n", + encoding="utf-8", + ) + + passed, failed, failures = verification.run_acceptance_criteria( + tmp_path, specs, "01-test.md" + ) + + assert passed == 0 + assert failed == 1 + assert len(failures) == 1 From 75afe68a116a3f4abe961abbeeb0c898d085bf86 Mon Sep 17 00:00:00 2001 From: Eric Cao Date: Tue, 7 Jul 2026 17:55:31 +0800 Subject: [PATCH 2/8] fix(report): auto-generate HTML report in worktree and fall back to session file Generate the static HTML report inside the worktree at the end of each run so it reflects the actual branch/iterations/status. Also make ReportGenerator fall back to session_latest.json when the detailed summary file is not present in the current directory (e.g. main repo). Closes #65 --- src/owloop/engine.py | 19 +++++++++++++++++++ src/owloop/report.py | 18 ++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/owloop/engine.py b/src/owloop/engine.py index 0be4aac..4233932 100644 --- a/src/owloop/engine.py +++ b/src/owloop/engine.py @@ -1694,10 +1694,29 @@ def run(self) -> RunSummary: dry_run_report=dry_run_report, ) self._write_summary(summary) + self._generate_report() self._notify(summary) self._close_append_handles() return summary + def _generate_report(self) -> None: + """Generate a static HTML report inside the worktree (best-effort).""" + try: + from owloop.report import ReportGenerator + from owloop.report_ai import ReportInsights + + report_path = self.cwd / ".owloop" / "reports" / "owloop_report_latest.html" + report_path.parent.mkdir(parents=True, exist_ok=True) + generator = ReportGenerator(self.cwd) + generator.generate( + output_path=report_path, + insights=ReportInsights(), + use_tailwind=False, + ) + except Exception: + # Report generation must never change the run outcome. + pass + def _notify(self, summary: RunSummary) -> None: """Fire completion notifications for the finished run (best-effort).""" if not self.config.notify_webhook and not self.config.notify_desktop: diff --git a/src/owloop/report.py b/src/owloop/report.py index f5df43f..e896068 100644 --- a/src/owloop/report.py +++ b/src/owloop/report.py @@ -52,6 +52,24 @@ def _load_summary(self) -> dict[str, Any]: if self.summary_path.exists(): with self.summary_path.open(encoding="utf-8") as f: return json.load(f) # type: ignore[no-any-return] + # The engine writes the detailed summary inside the worktree; the main + # repo only gets a lightweight session file. Fall back to it so reports + # generated from the main repo still show branch/iterations/status. + session_path = self.summary_path.with_name("session_latest.json") + if session_path.exists(): + with session_path.open(encoding="utf-8") as f: + session = json.load(f) # type: ignore[no-any-return] + status = session.get("status", "unknown") + # The session file uses "completed" as a generic completion flag; + # the classifier expects the granular stopped_reason "success". + stopped_reason = "success" if status == "completed" else status + return { + "iterations": session.get("iterations", 0), + "branch": session.get("branch", "unknown"), + "tokens_used": session.get("tokens_used", 0), + "estimated_cost_usd": 0, + "stopped_reason": stopped_reason, + } return {} def _load_events(self) -> list[dict[str, Any]]: From 2fb966bf10a804ee35d7215fa053e0db5f9a0918 Mon Sep 17 00:00:00 2001 From: Eric Cao Date: Tue, 7 Jul 2026 17:55:48 +0800 Subject: [PATCH 3/8] fix(engine): ignore owloop-owned paths in dirty-workspace check The workspace was flagged as dirty whenever .owloop/ existed untracked, which is normal because owloop stores specs/logs there. Filter out paths owned by owloop (currently .owloop/) from git status --porcelain before deciding whether to prompt the user. Closes #63 --- src/owloop/engine.py | 23 ++++++++++++++++++++++- tests/test_engine.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/owloop/engine.py b/src/owloop/engine.py index 4233932..82f1a59 100644 --- a/src/owloop/engine.py +++ b/src/owloop/engine.py @@ -504,8 +504,29 @@ def _log_line(self, text: str) -> None: def _is_git_repo(self) -> bool: return self._run_git("rev-parse", "--is-inside-work-tree").returncode == 0 + _OWLOOP_OWNED_PREFIXES = (".owloop",) + + def _is_path_owned(self, path: str) -> bool: + """Return True if a path belongs to owloop's own state directories.""" + for prefix in self._OWLOOP_OWNED_PREFIXES: + if path == prefix or path.startswith(prefix + "/"): + return True + return False + def _is_dirty(self) -> bool: - return bool(self._run_git("status", "--porcelain").stdout.strip()) + result = self._run_git("status", "--porcelain") + for line in result.stdout.splitlines(): + # Porcelain format: "XY path" or "XY orig -> dest". + path_part = line[3:].strip() + # If this is a rename, check both sides. + if " -> " in path_part: + source, _, dest = path_part.partition(" -> ") + if self._is_path_owned(source) and self._is_path_owned(dest): + continue + elif self._is_path_owned(path_part): + continue + return True + return False def _resolve_main_repo_dir(self) -> Path: if not self._is_git_repo(): diff --git a/tests/test_engine.py b/tests/test_engine.py index a18d13b..43f9818 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -1027,3 +1027,40 @@ def test_trim_run_notes_keeps_only_newest_entries() -> None: # Under the cap (or unstructured content), nothing is touched. assert trim_run_notes("free-form note", max_entries=3) == "free-form note" assert trim_run_notes(notes, max_entries=20) == notes + + +def test_is_dirty_ignores_owloop_directory(tmp_path: Path) -> None: + repo = tmp_path / "repo" + repo.mkdir() + _git_init(repo) + adapter = MockAdapter() + engine = _make_engine(repo, adapter) + + (repo / ".owloop" / "specs").mkdir(parents=True) + (repo / ".owloop" / "specs" / "01-test.md").write_text("# test", encoding="utf-8") + + assert not engine._is_dirty() + + +def test_is_dirty_detects_other_untracked_files(tmp_path: Path) -> None: + repo = tmp_path / "repo" + repo.mkdir() + _git_init(repo) + adapter = MockAdapter() + engine = _make_engine(repo, adapter) + + (repo / "untracked.txt").write_text("hi", encoding="utf-8") + + assert engine._is_dirty() + + +def test_is_dirty_detects_modified_tracked_file(tmp_path: Path) -> None: + repo = tmp_path / "repo" + repo.mkdir() + _git_init(repo) + adapter = MockAdapter() + engine = _make_engine(repo, adapter) + + (repo / "README.md").write_text("modified", encoding="utf-8") + + assert engine._is_dirty() From e4d5a7b2c4e5a6553c2ebb82c701b0b0b7efbf37 Mon Sep 17 00:00:00 2001 From: Eric Cao Date: Tue, 7 Jul 2026 17:59:58 +0800 Subject: [PATCH 4/8] feat(engine): include spec slug in owloop branch names Branch names now look like owloop/--, e.g. owloop/20260707-extract-issue-service-3c0c9b3c, so it is obvious what a branch is doing at a glance. Legacy branch names without a slug are still supported for --resume. Closes #68 --- src/owloop/engine.py | 45 ++++++++++++++++++++++++++++++++++++++------ tests/test_engine.py | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/src/owloop/engine.py b/src/owloop/engine.py index 82f1a59..d5254dc 100644 --- a/src/owloop/engine.py +++ b/src/owloop/engine.py @@ -723,6 +723,38 @@ def _latest_owloop_branch(self) -> str | None: branches = [b for b in result.stdout.splitlines() if b.strip()] return branches[0] if branches else None + @staticmethod + def _slugify(text: str) -> str: + """Turn arbitrary text into a git-safe branch slug.""" + slug = text.lower() + slug = re.sub(r"[^a-z0-9]+", "-", slug) + slug = slug.strip("-") + slug = re.sub(r"-+", "-", slug) + return slug[:40] + + _MAX_SLUG_LEN = 40 + + def _spec_slug(self) -> str: + """Derive a short slug from the active spec or fall back to 'run'.""" + spec = spec_queue.get_next_ready_spec(self.specs_dir) + if spec is None: + return "run" + # Strip leading numeric prefix (e.g. "01-extract-issue-service"). + stem = re.sub(r"^\d+-", "", spec.stem) + slug = self._slugify(stem) + return slug or "run" + + @staticmethod + def _session_id_from_branch(branch: str) -> str: + """Extract the trailing session id from an owloop branch name. + + Supports both legacy ``owloop/-`` and sluggified + ``owloop/--`` formats. + """ + rest = branch.split("/", 1)[1] + # The session id is the last '-'-delimited token. + return rest.rsplit("-", 1)[-1] + def _resolve_worktree_session(self) -> tuple[str, str, Path]: """Pick or resume a session id and derive branch/worktree path. @@ -739,8 +771,8 @@ def _resolve_worktree_session(self) -> tuple[str, str, Path]: raise RuntimeError( "--resume requested but no previous owloop session found." ) - # Branch format: owloop/- - session_id = branch.split("/", 1)[1].split("-", 1)[1] + # Branch format: owloop/-- + session_id = self._session_id_from_branch(branch) wt_path = wt_base / f"owloop-{branch.split('/', 1)[1]}" else: session_id = session["session_id"] @@ -752,14 +784,15 @@ def _resolve_worktree_session(self) -> tuple[str, str, Path]: # Reuse the id ``_init_session`` already resolved (if any) so this run # has a single consistent session id, instead of minting a second one. session_id = self.session_id or self.config.session_id or uuid.uuid4().hex[:8] - branch = f"owloop/{wt_date}-{session_id}" - wt_path = wt_base / f"owloop-{wt_date}-{session_id}" + slug = self._spec_slug() + branch = f"owloop/{wt_date}-{slug}-{session_id}" + wt_path = wt_base / f"owloop-{wt_date}-{slug}-{session_id}" # Guard against an extremely unlikely collision. while self._branch_exists(branch): session_id = uuid.uuid4().hex[:8] - branch = f"owloop/{wt_date}-{session_id}" - wt_path = wt_base / f"owloop-{wt_date}-{session_id}" + branch = f"owloop/{wt_date}-{slug}-{session_id}" + wt_path = wt_base / f"owloop-{wt_date}-{slug}-{session_id}" self._save_session(session_id, branch, wt_path) self.session_id = session_id diff --git a/tests/test_engine.py b/tests/test_engine.py index 43f9818..85d6643 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -531,6 +531,46 @@ def test_resolve_worktree_session_resume_falls_back_to_latest_branch(tmp_path: P assert branch == "owloop/20260706-xyz789" +def test_resolve_worktree_session_includes_spec_slug(tmp_path: Path) -> None: + repo = tmp_path / "repo" + repo.mkdir() + adapter = MockAdapter() + engine = _make_engine(repo, adapter) + + specs = repo / ".owloop" / "specs" + specs.mkdir(parents=True) + (specs / "04-extract-issue-service.md").write_text("# Spec\n", encoding="utf-8") + + session_id, branch, path = engine._resolve_worktree_session() + + assert branch.startswith("owloop/") + assert "extract-issue-service" in branch + assert session_id in branch + assert "extract-issue-service" in str(path) + + +def test_resolve_worktree_session_slugifies_title(tmp_path: Path) -> None: + repo = tmp_path / "repo" + repo.mkdir() + adapter = MockAdapter() + engine = _make_engine(repo, adapter) + + specs = repo / ".owloop" / "specs" + specs.mkdir(parents=True) + (specs / "02-Hello World_Refactor.md").write_text("# Spec\n", encoding="utf-8") + + session_id, branch, path = engine._resolve_worktree_session() + + assert "hello-world-refactor" in branch + + +def test_session_id_from_branch_supports_legacy_and_slug_formats() -> None: + from owloop.engine import OwloopEngine + + assert OwloopEngine._session_id_from_branch("owloop/20260706-abc123") == "abc123" + assert OwloopEngine._session_id_from_branch("owloop/20260706-extract-issue-service-abc123") == "abc123" + + def _read_jsonl(path: Path) -> list[dict]: return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines() if line.strip()] From 794b30910dd1647ff344119563efbead219ef3ec Mon Sep 17 00:00:00 2001 From: Eric Cao Date: Tue, 7 Jul 2026 18:03:10 +0800 Subject: [PATCH 5/8] fix(verification): restore excluded files mutated by acceptance-criteria commands Closes #66 --- src/owloop/spec_queue.py | 23 ++++++++++++++++++++++ src/owloop/verification.py | 32 +++++++++++++++++++++++++++++++ tests/test_verifier.py | 39 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) diff --git a/src/owloop/spec_queue.py b/src/owloop/spec_queue.py index d972f10..4a10e14 100644 --- a/src/owloop/spec_queue.py +++ b/src/owloop/spec_queue.py @@ -49,6 +49,10 @@ r"^##\s+Files\s*$\n(.*?)(?=^#{1,2}\s|\Z)", re.IGNORECASE | re.MULTILINE | re.DOTALL, ) +_EXCLUSIONS_SECTION_RE = re.compile( + r"^##\s+Exclusions\s*$\n(.*?)(?=^#{1,2}\s|\Z)", + re.IGNORECASE | re.MULTILINE | re.DOTALL, +) DEFAULT_PRIORITY = 999 @@ -349,6 +353,25 @@ def get_spec_file_scope(spec_file: Path) -> list[str]: return scope +def get_spec_exclusions(spec_file: Path) -> list[str]: + """Return the path/glob tokens listed under a spec's ``## Exclusions`` section. + + Missing/empty sections or ``none`` entries yield an empty list. + """ + if not spec_file.is_file(): + return [] + content = spec_file.read_text(encoding="utf-8", errors="replace") + match = _EXCLUSIONS_SECTION_RE.search(content) + if match is None: + return [] + exclusions: list[str] = [] + for item in _LIST_ITEM_RE.findall(match.group(1)): + cleaned = item.strip().strip("`").strip() + if cleaned and cleaned.lower() != "none": + exclusions.append(cleaned) + return exclusions + + def _paths_conflict(a: str, b: str) -> bool: """True if two path/glob tokens could touch a common file.""" if a == b: diff --git a/src/owloop/verification.py b/src/owloop/verification.py index 214bce8..ece5333 100644 --- a/src/owloop/verification.py +++ b/src/owloop/verification.py @@ -100,6 +100,35 @@ def run_acceptance_criteria( return passed, failed, failures +def _restore_exclusions(cwd: Path, specs_dir: Path, spec_name: str | None) -> None: + """Revert any tracked files listed in the spec's Exclusions section. + + Acceptance-criteria commands (e.g. ``uv run --with pytest``) can mutate + files the spec promised not to touch, such as ``uv.lock``. This best-effort + cleanup restores those tracked files to HEAD so they never leak into the + iteration's commit. + """ + if not spec_name: + return + exclusions = spec_queue.get_spec_exclusions(specs_dir / spec_name) + if not exclusions: + return + for pattern in exclusions: + result = subprocess.run( + ["git", "ls-files", "--", pattern], + cwd=cwd, + capture_output=True, + text=True, + ) + files = [f.strip() for f in result.stdout.splitlines() if f.strip()] + if files: + subprocess.run( + ["git", "checkout", "--", *files], + cwd=cwd, + capture_output=True, + ) + + def guarded_hash(cwd: Path, specs_dir: Path, spec_name: str | None) -> str: """Hash the spec sections + backpressure file the agent must not edit.""" h = hashlib.sha256() @@ -125,8 +154,11 @@ def run_gate( return GateResult(passed=False, tampered=True, passed_count=0, failed_count=0) acc_passed, acc_failed, acc_failures = run_acceptance_criteria(cwd, specs_dir, spec_name) + _restore_exclusions(cwd, specs_dir, spec_name) + bp_commands = [cmd.command for cmd in load_backpressure(cwd)] bp_passed, bp_failed, bp_failures = run_commands(cwd, bp_commands) + _restore_exclusions(cwd, specs_dir, spec_name) passed_count = acc_passed + bp_passed failed_count = acc_failed + bp_failed diff --git a/tests/test_verifier.py b/tests/test_verifier.py index 0bb0546..18ee2a9 100644 --- a/tests/test_verifier.py +++ b/tests/test_verifier.py @@ -185,3 +185,42 @@ def test_run_acceptance_criteria_no_output_fails_on_nonempty_stdout(tmp_path: Pa assert passed == 0 assert failed == 1 assert len(failures) == 1 + + +def test_run_gate_restores_excluded_tracked_files(tmp_path: Path) -> None: + """Acceptance-criteria commands must not leave mutations in spec-excluded files.""" + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run(["git", "init"], cwd=repo, check=True, capture_output=True) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], + cwd=repo, check=True, capture_output=True, + ) + subprocess.run( + ["git", "config", "user.name", "Test"], + cwd=repo, check=True, capture_output=True, + ) + + (repo / "uv.lock").write_text("original\n", encoding="utf-8") + (repo / "README.md").write_text("# test", encoding="utf-8") + subprocess.run(["git", "add", "."], cwd=repo, check=True, capture_output=True) + subprocess.run(["git", "commit", "-m", "init"], cwd=repo, check=True, capture_output=True) + + specs = repo / ".owloop" / "specs" + specs.mkdir(parents=True) + spec = specs / "01-test.md" + spec.write_text( + "# Spec\n\n" + "## Acceptance Criteria\n" + "- `echo changed > uv.lock`\n\n" + "## Exclusions\n" + "- `uv.lock`\n", + encoding="utf-8", + ) + + guard_before = verification.guarded_hash(repo, specs, "01-test.md") + result = verification.run_gate(repo, specs, "01-test.md", guard_before) + + assert result.tampered is False + assert result.passed is True + assert (repo / "uv.lock").read_text(encoding="utf-8") == "original\n" From bee45a19e12f7e2b1228c05b99869515b932e662 Mon Sep 17 00:00:00 2001 From: Eric Cao Date: Tue, 7 Jul 2026 18:05:58 +0800 Subject: [PATCH 6/8] feat(engine): add --no-push for review-before-push workflows Closes #67 --- src/owloop/cli.py | 22 +++++++++++----- src/owloop/engine.py | 8 +++++- src/owloop/reporter.py | 2 ++ src/owloop/tui.py | 2 ++ tests/test_engine.py | 59 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/owloop/cli.py b/src/owloop/cli.py index 87fe8b1..2633869 100644 --- a/src/owloop/cli.py +++ b/src/owloop/cli.py @@ -788,7 +788,7 @@ def _run_engine( ascii: bool = False, no_color: bool = False, compact: bool = False, verifier_model: str | None = None, subagents: bool = False, session_id: str | None = None, resume: bool = False, - no_tui: bool = False, dry_run: bool = False, + no_tui: bool = False, dry_run: bool = False, no_push: bool = False, max_tokens_per_iteration: int = 0, max_turns_per_iteration: int = 0, max_budget_usd: float = 0.0, @@ -819,6 +819,7 @@ def _run_engine( session_id=session_id, resume=resume, dry_run=dry_run, + no_push=no_push, keep_retrying=keep_retrying, rollback=rollback, notify_webhook=resolved_webhook, @@ -1006,6 +1007,13 @@ def _print_dry_run_report(console: Console, summary: RunSummary) -> None: "(no committed changes are left behind). Use to validate specs without " "burning a full overnight run.", ) +@click.option( + "--no-push", + is_flag=True, + default=False, + help="Commit completed specs locally but do not push. Useful for " + "review-before-push workflows or CI jobs that should leave commits on disk.", +) @click.option( "--no-tui", "--plain", "no_tui", is_flag=True, @@ -1060,11 +1068,12 @@ def _print_dry_run_report(console: Console, summary: RunSummary) -> None: show_default=True, ) @_common_run_options -def run(max_iterations: int, resume: bool, dry_run: bool, no_tui: bool, max_tokens_per_iteration: int, - max_turns_per_iteration: int, max_budget_usd: float, keep_retrying: bool, rollback: bool, - notify_webhook: str | None, notify_desktop: bool, converge_sweeps: int, workers: int, - worktree: bool, model: str, agent: str, verifier_model: str | None, subagents: bool, - idle_timeout: float, max_duration: int, max_tokens: int) -> None: +def run(max_iterations: int, resume: bool, dry_run: bool, no_push: bool, no_tui: bool, + max_tokens_per_iteration: int, max_turns_per_iteration: int, max_budget_usd: float, + keep_retrying: bool, rollback: bool, notify_webhook: str | None, notify_desktop: bool, + converge_sweeps: int, workers: int, worktree: bool, model: str, agent: str, + verifier_model: str | None, subagents: bool, idle_timeout: float, max_duration: int, + max_tokens: int) -> None: """Start the autonomous coding loop.""" ascii, no_color, compact, verbose = _cli_options() specs_dir = resolve_specs_dir(Path.cwd()) @@ -1083,6 +1092,7 @@ def run(max_iterations: int, resume: bool, dry_run: bool, no_tui: bool, max_toke resume=resume, no_tui=no_tui, dry_run=dry_run, + no_push=no_push, max_tokens_per_iteration=max_tokens_per_iteration, max_turns_per_iteration=max_turns_per_iteration, max_budget_usd=max_budget_usd, diff --git a/src/owloop/engine.py b/src/owloop/engine.py index d5254dc..0e08db8 100644 --- a/src/owloop/engine.py +++ b/src/owloop/engine.py @@ -321,6 +321,9 @@ class EngineConfig: # When True, run exactly one iteration, skip push, revert any commit the # iteration made, and produce a DryRunReport instead of looping. dry_run: bool = False + # When True, commit and mark specs complete locally but never push. Useful + # for review-before-push workflows or CI dry runs that want real commits. + no_push: bool = False # Completion notifications: fire a webhook and/or desktop notification when # the run stops on an attention-worthy terminal state. None/False = off. notify_webhook: str | None = None @@ -1611,7 +1614,10 @@ def run(self) -> RunSummary: self._append_run_note(iteration, True, note_summary) self._mark_spec_complete(active_spec) self._commit_iteration(iteration, active_spec) - self._push(branch) + if self.config.no_push: + self._emit("push_skipped", branch=branch) + else: + self._push(branch) spec_status = self._spec_status() self._emit( "iteration_end", diff --git a/src/owloop/reporter.py b/src/owloop/reporter.py index ab614c4..b0a732c 100644 --- a/src/owloop/reporter.py +++ b/src/owloop/reporter.py @@ -146,6 +146,8 @@ def on_event(self, kind: str, data: dict) -> None: c.print(f"[{_brand.AMBER}]{self._mark('clock')} token budget reached ({data['tokens']:,} / {data['limit']:,}), stopping loop[/]") elif kind == "push_retry": c.print(f"[{_brand.AMBER}]{self._mark('warn')} push failed, creating remote branch {data['branch']}...[/]") + elif kind == "push_skipped": + c.print(f"[{_brand.CYAN}]{self._mark('info')} push skipped (--no-push); commits remain on {data['branch']}[/]") elif kind == "interrupted": c.print("\n[dim]owloop stopped[/]") diff --git a/src/owloop/tui.py b/src/owloop/tui.py index 7237bd1..77f6ffc 100644 --- a/src/owloop/tui.py +++ b/src/owloop/tui.py @@ -345,6 +345,8 @@ def _handle(self, kind: str, data: dict) -> None: self._flash("⏱ time up", f"bold {AMBER}") elif kind == "push_retry": self._log(f"push failed, creating remote branch {data['branch']}...") + elif kind == "push_skipped": + self._log(f"push skipped (--no-push); commits remain on {data['branch']}") elif kind == "iteration_end": if "specs" in data: s.specs = data["specs"] diff --git a/tests/test_engine.py b/tests/test_engine.py index 85d6643..d5c09c1 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -793,6 +793,65 @@ def test_dry_run_reverts_commit_and_skips_push(tmp_path: Path, monkeypatch) -> N assert summary.dry_run_report.acceptance_passed == 1 +class _FileWritingAdapter(MockAdapter): + """Adapter that writes a file but leaves committing to the engine.""" + + def __init__(self, repo: Path, **kwargs) -> None: + super().__init__(**kwargs) + self._repo = repo + + def run(self, prompt: str, cwd: Path, *, on_line=None) -> AgentResult: + (self._repo / "agent_change.txt").write_text("changed", encoding="utf-8") + return super().run(prompt, cwd, on_line=on_line) + + +def test_no_push_commits_but_skips_remote_push(tmp_path: Path, monkeypatch) -> None: + """--no-push should still commit and complete the spec, but never call _push.""" + repo = tmp_path / "repo" + repo.mkdir() + _git_init(repo) + (repo / ".owloop" / "specs").mkdir(parents=True) + (repo / ".owloop" / "specs" / "01-test.md").write_text( + "# spec\n\n## Acceptance Criteria\n- `true`\n", encoding="utf-8" + ) + monkeypatch.setattr("owloop.engine.time.sleep", lambda _: None) + + original_head = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=repo, check=True, capture_output=True, text=True + ).stdout.strip() + + adapter = _FileWritingAdapter( + repo, + responses=[ + AgentResult( + stdout="ok\nDONE", + returncode=0, + success=True, + has_completion_signal=True, + done_signal="DONE", + ) + ], + ) + engine = _make_engine(repo, adapter, no_push=True) + + push_calls: list[str] = [] + monkeypatch.setattr(engine, "_push", lambda branch: push_calls.append(branch)) + skipped: list[str] = [] + engine.on_event = lambda k, d: skipped.append(k) if k == "push_skipped" else None + + summary = engine.run() + + assert summary.stopped_reason == "success" + assert push_calls == [] + assert "push_skipped" in skipped + assert "**Status**: COMPLETE" in (repo / ".owloop" / "specs" / "01-test.md").read_text() + head_after = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=repo, check=True, capture_output=True, text=True + ).stdout.strip() + assert head_after != original_head + assert (repo / "agent_change.txt").is_file() + + class _InterruptingAdapter(MockAdapter): """Adapter that completes one iteration, then simulates Ctrl+C on the next.""" From 9d31978e8aaa8cfc625d3a4e68f94b656891f6a8 Mon Sep 17 00:00:00 2001 From: Eric Cao Date: Tue, 7 Jul 2026 18:10:59 +0800 Subject: [PATCH 7/8] feat(verification): classify functional vs meta-check failures as soft failures Closes #69 --- src/owloop/engine.py | 54 +++++++++++++++++++++++++++++--------- src/owloop/reporter.py | 5 ++++ src/owloop/tui.py | 6 +++++ src/owloop/verification.py | 34 +++++++++++++++++++----- tests/test_engine.py | 54 ++++++++++++++++++++++++++++++++++++++ tests/test_verifier.py | 8 ++++-- 6 files changed, 140 insertions(+), 21 deletions(-) diff --git a/src/owloop/engine.py b/src/owloop/engine.py index 0e08db8..bfea058 100644 --- a/src/owloop/engine.py +++ b/src/owloop/engine.py @@ -116,6 +116,7 @@ class TerminalState(str, Enum): TAMPERED = "tampered" INTERRUPTED = "interrupted" FAILED = "failed" + SOFT_FAILURE = "soft_failure" def __str__(self) -> str: # so f-strings / json render the value, not "TerminalState.SUCCESS" return self.value @@ -143,6 +144,7 @@ class StopReason(str, Enum): INTERRUPTED = "interrupted" PREFLIGHT_FAILED = "preflight_failed" DIRTY_WORKSPACE_DECLINED = "dirty_workspace_declined" + SOFT_FAILURE = "soft_failure" def __str__(self) -> str: return self.value @@ -182,6 +184,7 @@ def __str__(self) -> str: StopReason.INTERRUPTED: TerminalState.INTERRUPTED, StopReason.PREFLIGHT_FAILED: TerminalState.FAILED, StopReason.DIRTY_WORKSPACE_DECLINED: TerminalState.FAILED, + StopReason.SOFT_FAILURE: TerminalState.SOFT_FAILURE, } @@ -1084,7 +1087,7 @@ def _guarded_hash(self, spec_name: str | None) -> str: def _run_verification_gate( self, iteration: int, spec_name: str | None, guard_before: str - ) -> tuple[bool, bool, list[dict[str, Any]]]: + ) -> verification.GateResult: """Deterministically verify an iteration outside the agent's control. Delegates to the shared gate in ``verification.py`` (the single @@ -1099,7 +1102,7 @@ def _run_verification_gate( if result.tampered: self._emit("spec_tampered", iteration=iteration, spec=spec_name) - return False, True, [] + return verification.GateResult(passed=False, tampered=True, passed_count=0, failed_count=0) if result.passed: self._emit( @@ -1107,6 +1110,14 @@ def _run_verification_gate( iteration=iteration, passed=result.passed_count, ) + elif result.soft_failure: + self._emit( + "verification_gate_soft_failure", + iteration=iteration, + passed=result.passed_count, + failed=result.failed_count, + commands=[f["command"] for f in result.failures], + ) else: self._emit( "verification_gate_failed", @@ -1115,7 +1126,7 @@ def _run_verification_gate( failed=result.failed_count, commands=[f["command"] for f in result.failures], ) - return result.passed, False, result.failures + return result def _head(self) -> str: return str(self._run_git("rev-parse", "HEAD").stdout).strip() @@ -1545,7 +1556,7 @@ def run(self) -> RunSummary: if self.config.dry_run: self._append_run_note(iteration, result.success, result.summary) - acceptance_passed, acceptance_failed, _ = verification.run_acceptance_criteria( + acceptance_passed, acceptance_failed, _, _, _ = verification.run_acceptance_criteria( self.cwd, self.specs_dir, active_spec ) current_head = self._head() @@ -1582,23 +1593,26 @@ def run(self) -> RunSummary: break # ── Deterministic verification gate (engine-owned) ── - gate_passed = False - tampered = False - gate_failures: list[dict[str, Any]] = [] + gate_result = verification.GateResult(passed=False, tampered=False, passed_count=0, failed_count=0) if result.promise_state == "DONE": - gate_passed, tampered, gate_failures = self._run_verification_gate( + gate_result = self._run_verification_gate( iteration, active_spec, guard_before ) # Shell-first ordering: the expensive LLM verifier runs only # on work that already survived the mechanical gate. if ( - gate_passed + gate_result.passed and self.verifier_adapter is not None and not self.config.use_subagents ): - gate_passed = self._apply_llm_verifier(iteration, result) + gate_result = verification.GateResult( + passed=self._apply_llm_verifier(iteration, result), + tampered=False, + passed_count=gate_result.passed_count, + failed_count=gate_result.failed_count, + ) - if gate_passed: + if gate_result.passed: # Verified success: only now does the engine commit, mark the # spec complete, and push — never the agent. consecutive_failures = 0 @@ -1656,8 +1670,22 @@ def run(self) -> RunSummary: # iteration is minutes of work; waiting here buys nothing. continue + # ── Soft failure: functional checks pass, meta-check fails ── + if gate_result.soft_failure: + self._append_run_note(iteration, False, result.summary) + diff = self._run_git("diff", last_good, "--", ".", ":!.owloop") + self._emit( + "soft_failure", + iteration=iteration, + spec=active_spec, + diff=diff.stdout or "", + commands=[f["command"] for f in gate_result.failures], + ) + stopped_reason = StopReason.SOFT_FAILURE + break + # ── Failure: classify, record feedback, roll back, stop on a stall ── - if tampered: + if gate_result.tampered: failure_reason = FailureReason.TAMPERED elif result.timed_out: failure_reason = FailureReason.TIMEOUT @@ -1676,7 +1704,7 @@ def run(self) -> RunSummary: self._append_run_note(iteration, False, result.summary) # Written before rollback; .owloop/ is excluded from the reset, # so the next iteration's prompt starts from this diagnosis. - self._write_failure_feedback(iteration, failure_reason, result, gate_failures) + self._write_failure_feedback(iteration, failure_reason, result, gate_result.failures) self._rollback_iteration(iteration, last_good) consecutive_failures += 1 diff --git a/src/owloop/reporter.py b/src/owloop/reporter.py index b0a732c..a47f1ed 100644 --- a/src/owloop/reporter.py +++ b/src/owloop/reporter.py @@ -131,6 +131,11 @@ def on_event(self, kind: str, data: dict) -> None: f"[{_brand.RED}]{self._mark('fail')} verification gate failed " f"({data.get('failed', 0)} of {data.get('passed', 0) + data.get('failed', 0)} checks)[/]" ) + elif kind == "soft_failure": + c.print( + f"[{_brand.AMBER}]{self._mark('warn')} soft failure: functional checks passed but a " + f"meta-check failed — work is preserved for review ({data.get('spec')})[/]" + ) elif kind == "iteration_rolled_back": c.print(f"[{_brand.CYAN}]{self._mark('info')} rolled back to {data.get('to_commit')} (failed iteration discarded)[/]") elif kind == "iteration_exhausted": diff --git a/src/owloop/tui.py b/src/owloop/tui.py index 77f6ffc..73f7f1d 100644 --- a/src/owloop/tui.py +++ b/src/owloop/tui.py @@ -343,6 +343,12 @@ def _handle(self, kind: str, data: dict) -> None: s.current_action = "Maximum run time reached" self._log(f"⏱ reached maximum run time ({data['minutes']} min), stopping loop") self._flash("⏱ time up", f"bold {AMBER}") + elif kind == "soft_failure": + s.done = True + s.phase = "stuck" + s.current_action = f"Soft failure on {data.get('spec')} — review preserved diff" + self._log(f"⚠ soft failure on {data.get('spec')}: meta-check failed; work preserved for review") + self._flash("⚠ soft failure — review diff", f"bold {AMBER}") elif kind == "push_retry": self._log(f"push failed, creating remote branch {data['branch']}...") elif kind == "push_skipped": diff --git a/src/owloop/verification.py b/src/owloop/verification.py index ece5333..318e1e8 100644 --- a/src/owloop/verification.py +++ b/src/owloop/verification.py @@ -32,6 +32,10 @@ class GateResult: passed_count: int failed_count: int failures: list[dict[str, Any]] = field(default_factory=list) + # True when functional checks pass but a structural/meta check (e.g. grep) + # failed. The engine should surface the diff for human review rather than + # blindly rolling back work that may be correct. + soft_failure: bool = False def run_commands( @@ -67,13 +71,18 @@ def _tail_output(result: subprocess.CompletedProcess[str]) -> str: def run_acceptance_criteria( cwd: Path, specs_dir: Path, spec_name: str | None -) -> tuple[int, int, list[dict[str, Any]]]: - """Run a spec's Acceptance Criteria shell commands; count passes vs failures.""" +) -> tuple[int, int, list[dict[str, Any]], int, int]: + """Run a spec's Acceptance Criteria shell commands; count passes vs failures. + + Returns ``(passed, failed, failures, code_failed, meta_failed)`` where + ``code_failed`` counts functional checks (tests, lints) and ``meta_failed`` + counts structural checks such as ``grep ... → no output``. + """ if not spec_name: - return 0, 0, [] + return 0, 0, [], 0, 0 criteria = spec_queue.get_acceptance_criteria_commands(specs_dir / spec_name) - passed = failed = 0 + passed = failed = code_failed = meta_failed = 0 failures: list[dict[str, Any]] = [] for criterion in criteria: result = subprocess.run( # noqa: S602 @@ -83,13 +92,19 @@ def run_acceptance_criteria( # grep-style tools exit 1 when they find no matches; the expectation # is about empty stdout, not the exit code. ok = result.stdout.strip() == "" and result.returncode in (0, 1) + is_meta = True else: ok = result.returncode == 0 + is_meta = False if ok: passed += 1 else: failed += 1 + if is_meta: + meta_failed += 1 + else: + code_failed += 1 failures.append( { "command": criterion.command, @@ -97,7 +112,7 @@ def run_acceptance_criteria( "output": _tail_output(result), } ) - return passed, failed, failures + return passed, failed, failures, code_failed, meta_failed def _restore_exclusions(cwd: Path, specs_dir: Path, spec_name: str | None) -> None: @@ -153,7 +168,9 @@ def run_gate( if guarded_hash(cwd, specs_dir, spec_name) != guard_before: return GateResult(passed=False, tampered=True, passed_count=0, failed_count=0) - acc_passed, acc_failed, acc_failures = run_acceptance_criteria(cwd, specs_dir, spec_name) + acc_passed, acc_failed, acc_failures, acc_code_failed, acc_meta_failed = run_acceptance_criteria( + cwd, specs_dir, spec_name + ) _restore_exclusions(cwd, specs_dir, spec_name) bp_commands = [cmd.command for cmd in load_backpressure(cwd)] @@ -162,10 +179,15 @@ def run_gate( passed_count = acc_passed + bp_passed failed_count = acc_failed + bp_failed + # Soft failure: functional checks (code + backpressure) pass, but a + # structural/meta check failed. Surface for human review instead of rolling + # back potentially-correct work. + soft_failure = failed_count > 0 and acc_code_failed == 0 and bp_failed == 0 return GateResult( passed=failed_count == 0, tampered=False, passed_count=passed_count, failed_count=failed_count, failures=acc_failures + bp_failures, + soft_failure=soft_failure, ) diff --git a/tests/test_engine.py b/tests/test_engine.py index d5c09c1..7a8ee9c 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -852,6 +852,60 @@ def test_no_push_commits_but_skips_remote_push(tmp_path: Path, monkeypatch) -> N assert (repo / "agent_change.txt").is_file() +def test_soft_failure_preserves_work_when_meta_check_fails(tmp_path: Path, monkeypatch) -> None: + """A functional pass + meta-check fail should stop for review without rollback.""" + repo = tmp_path / "repo" + repo.mkdir() + _git_init(repo) + (repo / "marker.txt").write_text("removed\n", encoding="utf-8") + subprocess.run(["git", "add", "."], cwd=repo, check=True, capture_output=True) + subprocess.run(["git", "commit", "-m", "add marker"], cwd=repo, check=True, capture_output=True) + + (repo / ".owloop" / "specs").mkdir(parents=True) + (repo / ".owloop" / "specs" / "01-test.md").write_text( + "# spec\n\n" + "## Acceptance Criteria\n" + "- `true`\n" + "- `grep removed marker.txt` → no output\n", + encoding="utf-8", + ) + monkeypatch.setattr("owloop.engine.time.sleep", lambda _: None) + + original_head = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=repo, check=True, capture_output=True, text=True + ).stdout.strip() + + adapter = _FileWritingAdapter( + repo, + responses=[ + AgentResult( + stdout="ok\nDONE", + returncode=0, + success=True, + has_completion_signal=True, + done_signal="DONE", + ) + ], + ) + engine = _make_engine(repo, adapter) + + soft_events: list[dict] = [] + engine.on_event = lambda k, d: soft_events.append(d) if k == "soft_failure" else None + + summary = engine.run() + + assert summary.stopped_reason == "soft_failure" + assert summary.terminal_state == "soft_failure" + assert "**Status**: COMPLETE" not in (repo / ".owloop" / "specs" / "01-test.md").read_text() + head_after = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=repo, check=True, capture_output=True, text=True + ).stdout.strip() + assert head_after == original_head # no rollback + assert (repo / "agent_change.txt").is_file() + assert len(soft_events) == 1 + assert "grep removed marker.txt" in soft_events[0]["commands"] + + class _InterruptingAdapter(MockAdapter): """Adapter that completes one iteration, then simulates Ctrl+C on the next.""" diff --git a/tests/test_verifier.py b/tests/test_verifier.py index 18ee2a9..83d3d47 100644 --- a/tests/test_verifier.py +++ b/tests/test_verifier.py @@ -157,13 +157,15 @@ def test_run_acceptance_criteria_treats_grep_no_match_as_pass(tmp_path: Path) -> encoding="utf-8", ) - passed, failed, failures = verification.run_acceptance_criteria( + passed, failed, failures, code_failed, meta_failed = verification.run_acceptance_criteria( tmp_path, specs, "01-test.md" ) assert passed == 1 assert failed == 0 assert failures == [] + assert code_failed == 0 + assert meta_failed == 0 def test_run_acceptance_criteria_no_output_fails_on_nonempty_stdout(tmp_path: Path) -> None: @@ -178,13 +180,15 @@ def test_run_acceptance_criteria_no_output_fails_on_nonempty_stdout(tmp_path: Pa encoding="utf-8", ) - passed, failed, failures = verification.run_acceptance_criteria( + passed, failed, failures, code_failed, meta_failed = verification.run_acceptance_criteria( tmp_path, specs, "01-test.md" ) assert passed == 0 assert failed == 1 assert len(failures) == 1 + assert code_failed == 0 + assert meta_failed == 1 def test_run_gate_restores_excluded_tracked_files(tmp_path: Path) -> None: From 21ddd61d5a23e42bb26870b696e1ee557060b019 Mon Sep 17 00:00:00 2001 From: Eric Cao Date: Tue, 7 Jul 2026 18:13:14 +0800 Subject: [PATCH 8/8] feat(engine): skip subagents for small/scoped specs to reduce token cost Closes #70 --- src/owloop/cli.py | 4 +- src/owloop/engine.py | 19 +++++++- tests/test_engine.py | 107 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 2 deletions(-) diff --git a/src/owloop/cli.py b/src/owloop/cli.py index 2633869..cf0ea66 100644 --- a/src/owloop/cli.py +++ b/src/owloop/cli.py @@ -429,7 +429,9 @@ def _common_run_options(f: Callable[..., Any]) -> Callable[..., Any]: "--subagents", is_flag=True, default=False, - help="Split large iterations into Orient/Implement/Verify subagent phases.", + help="Split large iterations into Orient/Implement/Verify subagent phases. " + "Small/scoped specs (≤3 files by default) still run as a single agent " + "to save tokens.", )(f) f = click.option( "--idle-timeout", type=float, default=DEFAULT_IDLE_TIMEOUT, diff --git a/src/owloop/engine.py b/src/owloop/engine.py index bfea058..39284a8 100644 --- a/src/owloop/engine.py +++ b/src/owloop/engine.py @@ -1196,6 +1196,23 @@ def _rollback_iteration(self, iteration: int, last_good: str) -> None: patch=str(patch_path) if diff.stdout.strip() else None, ) + def _should_use_subagents(self, target_spec: str | None) -> bool: + """Skip expensive subagent orchestration for small/scoped specs. + + Subagents are great for cross-file refactors, but for a single-file or + tightly-scoped change they burn tokens on coordination overhead. When + the spec declares a `## Files` scope that is at or below the threshold, + run a single agent iteration instead. + """ + if not self.config.use_subagents: + return False + if not target_spec: + return True + scope = spec_queue.get_spec_file_scope(self.specs_dir / target_spec) + if not scope: + return True + return len(scope) > self.config.subagent_file_threshold + def run_iteration(self, iteration: int, target_spec: str | None = None) -> IterationResult: owloop_dir = resolve_owloop_dir(self.cwd) prompt_file = owloop_dir / "PROMPT_build.md" @@ -1225,7 +1242,7 @@ def _on_line(line: str) -> None: raise IterationTokenLimitExceededError(iteration_tokens) try: - if self.config.use_subagents: + if self._should_use_subagents(target_spec): orchestrator = SubagentOrchestrator( self.adapter, self.verifier_adapter, self.cwd, on_line=_on_line ) diff --git a/tests/test_engine.py b/tests/test_engine.py index 7a8ee9c..4ed8750 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -915,6 +915,113 @@ def run(self, prompt: str, cwd: Path, *, on_line=None) -> AgentResult: return super().run(prompt, cwd, on_line=on_line) +def _make_repo_with_spec(tmp_path: Path, files_scope: list[str]) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run(["git", "init"], cwd=repo, check=True, capture_output=True) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], + cwd=repo, check=True, capture_output=True, + ) + subprocess.run( + ["git", "config", "user.name", "Test"], + cwd=repo, check=True, capture_output=True, + ) + (repo / "README.md").write_text("# test", encoding="utf-8") + specs = repo / ".owloop" / "specs" + specs.mkdir(parents=True) + scope_lines = "\n".join(f"- `{f}`" for f in files_scope) + (specs / "01-test.md").write_text( + f"# spec\n\n## Files\n{scope_lines}\n\n## Acceptance Criteria\n- `true`\n", + encoding="utf-8", + ) + subprocess.run(["git", "add", "."], cwd=repo, check=True, capture_output=True) + subprocess.run(["git", "commit", "-m", "initial"], cwd=repo, check=True, capture_output=True) + return repo + + +def test_subagents_skipped_for_small_scoped_spec(tmp_path: Path, monkeypatch) -> None: + """Small/scoped specs should not pay the subagent orchestration token cost.""" + repo = _make_repo_with_spec(tmp_path, ["src/small.py"]) + monkeypatch.setattr("owloop.engine.time.sleep", lambda _: None) + + subagent_calls: list[tuple[str, ...]] = [] + + class _FakeOrchestrator: + def __init__(self, adapter, verifier, cwd, on_line=None) -> None: + subagent_calls.append((str(adapter), str(verifier), str(cwd))) + + def run(self) -> AgentResult: + return AgentResult( + stdout="ok\nDONE", + returncode=0, + success=True, + has_completion_signal=True, + done_signal="DONE", + ) + + monkeypatch.setattr("owloop.engine.SubagentOrchestrator", _FakeOrchestrator) + + engine = _make_engine( + repo, + MockAdapter( + responses=[ + AgentResult( + stdout="ok\nDONE", + returncode=0, + success=True, + has_completion_signal=True, + done_signal="DONE", + ) + ] + ), + use_subagents=True, + ) + monkeypatch.setattr(engine, "_push", lambda b: None) + + summary = engine.run() + + assert summary.stopped_reason == "success" + assert subagent_calls == [] + + +def test_subagents_used_for_large_scoped_spec(tmp_path: Path, monkeypatch) -> None: + """Specs touching many files still get the full subagent orchestration.""" + repo = _make_repo_with_spec( + tmp_path, [f"src/file{i}.py" for i in range(5)] + ) + monkeypatch.setattr("owloop.engine.time.sleep", lambda _: None) + + subagent_calls: list[tuple[str, ...]] = [] + + class _FakeOrchestrator: + def __init__(self, adapter, verifier, cwd, on_line=None) -> None: + subagent_calls.append((str(adapter), str(verifier), str(cwd))) + + def run(self) -> AgentResult: + return AgentResult( + stdout="ok\nDONE", + returncode=0, + success=True, + has_completion_signal=True, + done_signal="DONE", + ) + + monkeypatch.setattr("owloop.engine.SubagentOrchestrator", _FakeOrchestrator) + + engine = _make_engine( + repo, + MockAdapter(responses=[]), + use_subagents=True, + ) + monkeypatch.setattr(engine, "_push", lambda b: None) + + summary = engine.run() + + assert summary.stopped_reason == "success" + assert len(subagent_calls) == 1 + + def test_session_state_persisted_on_interrupt(tmp_path: Path) -> None: repo = tmp_path / "repo" repo.mkdir()