diff --git a/README.md b/README.md index 774d9a2..b9901b6 100644 --- a/README.md +++ b/README.md @@ -307,6 +307,25 @@ The server speaks MCP `2025-11-25` (and negotiates down to `2025-06-18`, `2025-0 - `clodex_audit` accepts `base` (everything since that ref diverged) and `commit` (one commit) as well as the default uncommitted diff. +### Native handoffs + +- **Decisions follow your audit settings.** `clodex_handoff_decide` evaluates the reports recorded + with `clodex_handoff_update` against the same `audit.reviewers` and `audit.quorum` as a classic + run (`unanimous`, `majority` or a number; only `required` reviewers count). A report is a verdict + when it has `approved` and a diff hash (on the update or in the report); it only counts for that + hash, a new hash resets everything, and a hashless `approved: false` withdraws the reviewer's + approval. `report.reviewer_id` names which configured reviewer the verdict is for; without it + the actor (`claude` / `codex`) stands for the first *required* reviewer of that backend, so the + default two-reviewer flow needs no extra fields. An unknown `reviewer_id` is rejected. The + decision lists `required_pending` and `approved_reviewers` so the orchestrator knows whom to ask. +- **Workspaces.** `clodex_handoff_create` takes `workspace: "git-worktree" | "local" | "none"` + (default `none`). A worktree is an isolated checkout under `.clodex/workspaces/` (the + repo must have no uncommitted changes to tracked files); if the handoff cannot be recorded the + worktree is rolled back. `clodex clean ` removes it once the handoff has finished. +- **Ledger.** A handoff gets a row in the task ledger (shown by `clodex status`, kept in step + with the run: `done` / `blocked` / `failed`), every verdict report becomes an `audits` row, and + `report.artifacts` (paths, or `{name, path, kind}`) become artifact rows. + ## Safety Clodex defaults to git worktree isolation, Codex `workspace-write` sandboxing, diff --git a/clodex/mcp_server.py b/clodex/mcp_server.py index f832e66..1133dc7 100644 --- a/clodex/mcp_server.py +++ b/clodex/mcp_server.py @@ -3,6 +3,7 @@ import base64 import json import sqlite3 +import subprocess import sys import threading import uuid @@ -12,9 +13,11 @@ from . import __version__ from .config import resolve_repo_root +from .quorum import evaluate_handoff, resolve_reviewer from .schemas import validate as validate_schema from .tasks import TaskManager from .workflow import ClodexWorkflow +from .workspace import DirtyWorkspaceError, WorkspaceManager TOOLS = [ @@ -105,6 +108,11 @@ "owner": {"type": "string"}, "phase": {"type": "string"}, "handoff_budget": {"type": "integer"}, + "workspace": { + "type": "string", + "enum": ["none", "git-worktree", "local"], + "description": "Give the handoff its own isolated git worktree (or the repo itself) for Codex to work in. Default none.", + }, }, "required": ["task"], }, @@ -112,7 +120,7 @@ { "name": "clodex_handoff_update", "title": "Update Clodex handoff", - "description": "Record native handoff phase, actor, report, status, and budget usage.", + "description": "Record native handoff phase, actor, report, status, and budget usage. A report with `approved` and a diff hash is a verdict; `reviewer_id` names which configured reviewer it is for (default: the first required reviewer of the actor's backend).", "inputSchema": { "type": "object", "properties": { @@ -532,20 +540,41 @@ def tool_call(name: str, arguments: dict[str, Any]) -> dict[str, Any]: return call_text(str(exc), is_error=True) return {"content": [{"type": "text", "text": json.dumps(result.__dict__, indent=2)}], "isError": False} elif name == "clodex_handoff_create": + run_id = str(arguments.get("run_id") or f"native-{uuid.uuid4().hex[:12]}") + workspace = None + manager = WorkspaceManager(workflow.repo_root, workflow.config) try: handoff_budget = handoff_budget_argument(arguments) + task = str(required_argument(arguments, "task")) + backend = arguments.get("workspace") + if backend not in (None, "none", "git-worktree", "local"): + raise ValueError("workspace must be one of: none, git-worktree, local") + if backend in {"git-worktree", "local"}: + if workflow.state.get_run(run_id) is not None: + raise ValueError(f"run already exists: {run_id}") # before a worktree is created for it + workspace = manager.prepare(run_id, backend) run = workflow.state.create_handoff( - str(arguments.get("run_id") or f"native-{uuid.uuid4().hex[:12]}"), - str(required_argument(arguments, "task")), + run_id, + task, owner=str(arguments.get("owner") or "claude"), phase=str(arguments.get("phase") or "planning"), handoff_budget=handoff_budget, + workspace_path=str(workspace.path) if workspace else None, ) - except (KeyError, TypeError, ValueError, sqlite3.IntegrityError) as exc: + if workspace is not None: + workflow.state.add_workspace_lock(run_id, str(workspace.source_path), str(workspace.path), workspace.backend) + except (KeyError, TypeError, ValueError, sqlite3.IntegrityError, DirtyWorkspaceError, subprocess.CalledProcessError) as exc: + if workspace is not None and workspace.is_worktree: + manager.release(workspace.path) # do not leave a worktree behind for a handoff that was never created return call_text(expected_handoff_error(exc), is_error=True) - return call_json(run) + return call_json({**run, **({"workspace": workspace.as_dict()} if workspace else {})}) elif name == "clodex_handoff_update": try: + report_arg = arguments.get("report") if isinstance(arguments.get("report"), dict) else None + named = (report_arg or {}).get("reviewer_id") + if named is not None and resolve_reviewer(named, workflow.config.reviewers) is None: + configured = ", ".join(str(r.get("id")) for r in workflow.config.reviewers) + raise ValueError(f"unknown reviewer_id: {named} (configured reviewers: {configured})") run = workflow.state.update_handoff( str(required_argument(arguments, "run_id")), phase=arguments.get("phase"), @@ -580,6 +609,7 @@ def tool_call(name: str, arguments: dict[str, Any]) -> dict[str, Any]: run = data["run"] status = run.get("status") + agreement = evaluate_handoff(data, workflow.config.reviewers, workflow.config.audit.get("quorum", "unanimous")) if status == "approved": decision = {"decision": "approved", "run_id": run["id"], "diff_hash": run.get("diff_hash")} is_error = False @@ -591,7 +621,7 @@ def tool_call(name: str, arguments: dict[str, Any]) -> dict[str, Any]: "blocked_reason": run.get("blocked_reason") or run.get("error"), } is_error = True - elif agreement := handoff_agreement(data): + elif agreement["approved"]: try: approved_run = workflow.state.approve_handoff(run["id"], agreement["diff_hash"], approved_by=agreement["approved_by"]) except (KeyError, TypeError, ValueError, sqlite3.IntegrityError) as exc: @@ -601,6 +631,7 @@ def tool_call(name: str, arguments: dict[str, Any]) -> dict[str, Any]: "run_id": approved_run["id"], "diff_hash": approved_run.get("diff_hash"), "approved_by": agreement["approved_by"], + "approved_reviewers": agreement["approved_reviewers"], } is_error = False else: @@ -610,6 +641,9 @@ def tool_call(name: str, arguments: dict[str, Any]) -> dict[str, Any]: "phase": run.get("phase"), "budget_remaining": data["budget_remaining"], "next_expected_actor": data["next_expected_actor"], + "required_pending": agreement["required_pending"], + "approved_reviewers": agreement["approved_reviewers"], + "quorum": agreement["quorum"], } is_error = False workflow.state.add_event(run["id"], "handoff.decide", decision) @@ -642,57 +676,12 @@ def handoff_budget_argument(arguments: dict[str, Any]) -> int: return value -def handoff_agreement(data: dict[str, Any]) -> dict[str, Any] | None: - latest_hash: str | None = None - approvals: set[str] = set() - for event in data.get("events") or []: - if event.get("event") != "handoff.update": - continue - event_data = event.get("data") - if not isinstance(event_data, dict): - continue - report = event_data.get("report") - if not isinstance(report, dict): - report = {} - actor = normalized_actor(event_data.get("actor") or report.get("actor")) - diff_hash = normalized_diff_hash(event_data.get("diff_hash") or report.get("diff_hash")) - if diff_hash is None: - if latest_hash is not None and actor is not None and report.get("approved") is False: - approvals.discard(actor) - continue - if diff_hash != latest_hash: - latest_hash = diff_hash - approvals = set() - if actor is None: - continue - if report.get("approved") is True: - approvals.add(actor) - elif report.get("approved") is False: - approvals.discard(actor) - - if latest_hash is not None and {"claude", "codex"}.issubset(approvals): - return {"diff_hash": latest_hash, "approved_by": sorted(approvals)} - return None - - -def normalized_actor(value: Any) -> str | None: - if value is None: - return None - actor = str(value).strip().lower() - return actor if actor in {"claude", "codex"} else None - - -def normalized_diff_hash(value: Any) -> str | None: - if value is None: - return None - diff_hash = str(value).strip() - return diff_hash or None - - def expected_handoff_error(exc: Exception) -> str: if isinstance(exc, KeyError): key = exc.args[0] if exc.args else "argument" return f"Missing required argument: {key}" + if isinstance(exc, subprocess.CalledProcessError): + return (exc.stderr or "").strip() or str(exc) return str(exc) diff --git a/clodex/quorum.py b/clodex/quorum.py index ccb7c78..5b0bfc1 100644 --- a/clodex/quorum.py +++ b/clodex/quorum.py @@ -90,3 +90,95 @@ def required_fixes(verdicts: list[dict[str, Any]], diff_hash: str) -> list[str]: findings.append((len(SEVERITY_ORDER), str(finding))) ordered = fixes + [text for _, text in sorted(findings, key=lambda item: item[0])] return list(dict.fromkeys(ordered)) or ["Resolve audit disagreement and make the diff satisfy the accepted plan."] + + +# ---------------------------------------------------------------- native handoffs + + +def normalized_actor(value: Any) -> str | None: + if value is None: + return None + actor = str(value).strip().lower() + return actor if actor in {"claude", "codex"} else None + + +def normalized_diff_hash(value: Any) -> str | None: + if value is None: + return None + diff_hash = str(value).strip() + return diff_hash or None + + +def resolve_reviewer(identifier: Any, reviewers: list[dict[str, Any]]) -> dict[str, Any] | None: + """Map a report's reviewer id, or just its actor (`claude` / `codex`), onto a configured reviewer. + + An actor with no explicit reviewer id stands for the first *required* reviewer of its backend + (the first of that backend if none is required), so the default claude/codex flow keeps working. + """ + if identifier is None: + return None + name = str(identifier).strip() + for reviewer in reviewers: + if str(reviewer.get("id")) == name: + return reviewer + backend = normalized_actor(name) + if backend is None: + return None + same_backend = [r for r in reviewers if str(r.get("backend")) == backend] + required = [r for r in same_backend if r.get("required", True)] + return (required or same_backend or [None])[0] + + +def evaluate_handoff(data: dict[str, Any], reviewers: list[dict[str, Any]], quorum: Any = "unanimous") -> dict[str, Any]: + """Whether the reports recorded on a handoff satisfy the configured reviewers and quorum. + + Same rules as the classic audit: a report approves only for the diff hash it names, a new + hash resets every approval, and a hashless rejection withdraws that reviewer's approval. + """ + latest_hash: str | None = None + approvals: dict[str, bool] = {} + for event in data.get("events") or []: + if event.get("event") != "handoff.update": + continue + event_data = event.get("data") + if not isinstance(event_data, dict): + continue + report = event_data.get("report") + if not isinstance(report, dict): + report = {} + reviewer = resolve_reviewer(report.get("reviewer_id") or event_data.get("reviewer_id") or event_data.get("actor") or report.get("actor"), reviewers) + diff_hash = normalized_diff_hash(event_data.get("diff_hash") or report.get("diff_hash")) + reviewer_id = str(reviewer["id"]) if reviewer else None + if diff_hash is None: + if latest_hash is not None and reviewer_id is not None and report.get("approved") is False: + approvals[reviewer_id] = False + continue + if diff_hash != latest_hash: + latest_hash = diff_hash + approvals = {} + if reviewer_id is None: + continue + if report.get("approved") is True: + approvals[reviewer_id] = True + elif report.get("approved") is False: + approvals[reviewer_id] = False + + verdicts = [ + { + "reviewer_id": str(reviewer["id"]), + "approved": bool(approvals.get(str(reviewer["id"]))), + "diff_hash": latest_hash if approvals.get(str(reviewer["id"])) else None, + "persona": reviewer.get("persona"), + "_required": bool(reviewer.get("required", True)), + } + for reviewer in reviewers + ] + result = evaluate(verdicts, latest_hash or "", 0, quorum) + result["approved"] = bool(result["approved"] and latest_hash) + result["diff_hash"] = latest_hash + by_id = {str(reviewer["id"]): reviewer for reviewer in reviewers} + approving = [rid for rid in result["required_approved_by"] if rid in by_id] + result["approved_reviewers"] = approving + # `approved_by` lists the agents (claude / codex) behind the approving reviewers, as it always has. + result["approved_by"] = sorted({str(by_id[rid].get("backend")) for rid in approving}) + return result diff --git a/clodex/state.py b/clodex/state.py index b5f417c..46a6396 100644 --- a/clodex/state.py +++ b/clodex/state.py @@ -262,21 +262,32 @@ def create_handoff( phase: str = "planning", handoff_budget: int = 6, task_id: str | None = None, + workspace_path: str | None = None, ) -> dict[str, Any]: if handoff_budget < 0: raise ValueError("handoff_budget must be non-negative") + if phase not in HANDOFF_PHASES: + raise ValueError(f"unknown handoff phase: {phase} (use one of {sorted(HANDOFF_PHASES)})") timestamp = now_iso() + task_id = task_id or run_id # every handoff shows up in the task ledger (`clodex status`) with self.session() as con: con.execute( """ insert into runs( id, task_id, status, prompt, owner, phase, handoff_count, - handoff_budget, created_at, updated_at, started_at + handoff_budget, workspace_path, created_at, updated_at, started_at ) - values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + """, + (run_id, task_id, "handoff", prompt, owner, phase, 0, handoff_budget, workspace_path, timestamp, timestamp, timestamp), + ) + con.execute( + """ + insert into tasks(id, title, status, created_at, updated_at) values (?, ?, ?, ?, ?) + on conflict(id) do update set status=excluded.status, updated_at=excluded.updated_at """, - (run_id, task_id, "handoff", prompt, owner, phase, 0, handoff_budget, timestamp, timestamp, timestamp), + (task_id, prompt.strip().splitlines()[0][:200] if prompt.strip() else run_id, "handoff", timestamp, timestamp), ) self._insert_event( con, @@ -369,6 +380,8 @@ def update_handoff( updated = con.execute("select * from runs where id=?", (run_id,)).fetchone() updated_run = dict(updated) + self._record_report(con, run_id, actor, report, diff_hash, timestamp) + self._sync_task(con, updated_run, timestamp) updated_count = _int_or_default(updated_run.get("handoff_count"), 0) updated_budget = _int_or_default(updated_run.get("handoff_budget"), 6) event_data: dict[str, Any] = { @@ -430,8 +443,44 @@ def approve_handoff(self, run_id: str, diff_hash: str, approved_by: list[str] | timestamp, ) updated = con.execute("select * from runs where id=?", (run_id,)).fetchone() + self._sync_task(con, dict(updated), timestamp) return dict(updated) + # Run status -> task status for the ledger; handoff runs in flight leave the task alone. + _TASK_STATUS = {"approved": "done", "applied": "done", "completed": "done", "blocked": "blocked", "failed": "failed", "cancelled": "cancelled"} + + def _sync_task(self, con: sqlite3.Connection, run: dict[str, Any], timestamp: str) -> None: + task_status = self._TASK_STATUS.get(str(run.get("status"))) + if task_status and run.get("task_id"): + con.execute("update tasks set status=?, updated_at=? where id=?", (task_status, timestamp, run["task_id"])) + + def _record_report(self, con: sqlite3.Connection, run_id: str, actor: str | None, report: dict[str, Any] | None, diff_hash: str | None, timestamp: str) -> None: + """Put what a handoff report says into the same ledger the classic workflow uses. + + A verdict (`approved` true/false) becomes an `audits` row, and `artifacts` entries (paths or + {name, path, kind}) become `artifacts` rows, so `status`, exports and the MCP tools see them. + """ + if not isinstance(report, dict): + return + if isinstance(report.get("approved"), bool): + agent = str(report.get("reviewer_id") or actor or "unknown") + con.execute( + "insert into audits(run_id, agent, approved, diff_hash, verdict_json, created_at) values (?, ?, ?, ?, ?, ?)", + (run_id, agent, int(report["approved"]), diff_hash or report.get("diff_hash"), json.dumps(report, default=str), timestamp), + ) + artifacts = report.get("artifacts") + for item in artifacts if isinstance(artifacts, list) else []: + path = item.get("path") if isinstance(item, dict) else item + if not isinstance(path, str) or not path.strip(): + continue + name = str(item.get("name")) if isinstance(item, dict) and item.get("name") else Path(path).name + kind = str(item.get("kind")) if isinstance(item, dict) and item.get("kind") else (Path(path).suffix.lstrip(".") or "file") + con.execute("delete from artifacts where run_id=? and name=?", (run_id, name)) + con.execute( + "insert into artifacts(run_id, name, path, kind, created_at) values (?, ?, ?, ?, ?)", + (run_id, name, path, kind, timestamp), + ) + def get_handoff(self, run_id: str) -> dict[str, Any] | None: with self.session() as con: row = con.execute("select * from runs where id=?", (run_id,)).fetchone() diff --git a/tests/test_handoff_quorum.py b/tests/test_handoff_quorum.py new file mode 100644 index 0000000..8ddef7f --- /dev/null +++ b/tests/test_handoff_quorum.py @@ -0,0 +1,283 @@ +"""Tests: configurable-quorum handoff decisions, ledger rows and handoff workspaces.""" + +from __future__ import annotations + +import json +import os +import subprocess +import unittest +from pathlib import Path +from unittest import mock + +from clodex import mcp_server +from clodex.config import DEFAULT_CONFIG +from clodex.quorum import evaluate_handoff, resolve_reviewer +from clodex.state import StateStore +from clodex.workflow import ClodexWorkflow +from tests.support import TempRepo + +DEFAULT_REVIEWERS = DEFAULT_CONFIG["audit"]["reviewers"] + + +def update_event(actor=None, diff_hash=None, **report): + return {"event": "handoff.update", "data": {"actor": actor, "diff_hash": diff_hash, "report": report}} + + +def handoff(*events): + return {"events": list(events)} + + +def reviewer(reviewer_id, backend, required=True): + return {"id": reviewer_id, "backend": backend, "persona": reviewer_id, "required": required} + + +class ResolveReviewerTests(unittest.TestCase): + def test_id_beats_backend_and_actor_maps_to_the_first_required_reviewer_of_its_backend(self): + reviewers = [reviewer("claude-extra", "claude", required=False), reviewer("claude-plan", "claude"), reviewer("codex-arch", "codex")] + self.assertEqual(resolve_reviewer("claude-extra", reviewers)["id"], "claude-extra") + self.assertEqual(resolve_reviewer("claude", reviewers)["id"], "claude-plan", "required ones win over earlier optional ones") + self.assertEqual(resolve_reviewer(" Codex ", reviewers)["id"], "codex-arch") + only_optional = [reviewer("a", "claude", required=False)] + self.assertEqual(resolve_reviewer("claude", only_optional)["id"], "a") + self.assertIsNone(resolve_reviewer("gemini", reviewers)) + self.assertIsNone(resolve_reviewer(None, reviewers)) + + +class EvaluateHandoffTests(unittest.TestCase): + def test_default_reviewers_need_claude_and_codex_on_the_same_hash(self): + both = handoff(update_event("claude", "h1", approved=True), update_event("codex", "h1", approved=True)) + result = evaluate_handoff(both, DEFAULT_REVIEWERS) + self.assertTrue(result["approved"]) + self.assertEqual(result["diff_hash"], "h1") + self.assertEqual(result["approved_by"], ["claude", "codex"]) + self.assertEqual(result["approved_reviewers"], ["claude-plan", "codex-architecture"]) + one = evaluate_handoff(handoff(update_event("claude", "h1", approved=True)), DEFAULT_REVIEWERS) + self.assertFalse(one["approved"]) + self.assertEqual(one["required_pending"], ["codex-architecture"]) + + def test_a_new_diff_hash_resets_approvals_and_hashless_rejection_withdraws(self): + moved = handoff(update_event("claude", "h1", approved=True), update_event("codex", "h2", approved=True)) + self.assertFalse(evaluate_handoff(moved, DEFAULT_REVIEWERS)["approved"]) + withdrawn = handoff(update_event("claude", "h1", approved=True), update_event("codex", "h1", approved=True), update_event("claude", None, approved=False)) + result = evaluate_handoff(withdrawn, DEFAULT_REVIEWERS) + self.assertFalse(result["approved"]) + self.assertEqual(result["required_pending"], ["claude-plan"]) + + def test_explicit_reviewer_ids_and_optional_reviewers(self): + reviewers = [reviewer("claude-plan", "claude"), reviewer("codex-arch", "codex"), reviewer("security", "codex", required=False)] + data = handoff( + update_event("claude", "h", approved=True), + update_event("codex", "h", approved=True, reviewer_id="security"), # for the optional reviewer + ) + result = evaluate_handoff(data, reviewers) + self.assertFalse(result["approved"], "an optional reviewer cannot stand in for a required one") + self.assertEqual(result["required_pending"], ["codex-arch"]) + data.get("events").append(update_event("codex", "h", approved=True)) + self.assertTrue(evaluate_handoff(data, reviewers)["approved"]) + data.get("events").append(update_event("codex", "h", approved=False, reviewer_id="security")) + self.assertTrue(evaluate_handoff(data, reviewers)["approved"], "an optional rejection never blocks") + + def test_majority_and_numeric_quorums(self): + reviewers = [reviewer("a", "claude"), reviewer("b", "codex"), reviewer("c", "codex")] + two = handoff(update_event("claude", "h", approved=True), update_event("codex", "h", approved=True, reviewer_id="b")) + self.assertTrue(evaluate_handoff(two, reviewers, "majority")["approved"]) + self.assertFalse(evaluate_handoff(two, reviewers, "unanimous")["approved"]) + self.assertTrue(evaluate_handoff(two, reviewers, 2)["approved"]) + self.assertFalse(evaluate_handoff(two, reviewers, 3)["approved"], "needs all three") + one = handoff(update_event("claude", "h", approved=True)) + self.assertFalse(evaluate_handoff(one, reviewers, 2)["approved"], "one approval is below a quorum of two") + + def test_nothing_recorded_or_no_required_reviewers_is_never_approved(self): + self.assertFalse(evaluate_handoff(handoff(), DEFAULT_REVIEWERS)["approved"]) + self.assertIsNone(evaluate_handoff(handoff(), DEFAULT_REVIEWERS)["diff_hash"]) + optional_only = [reviewer("a", "claude", required=False)] + self.assertFalse(evaluate_handoff(handoff(update_event("claude", "h", approved=True)), optional_only)["approved"]) + + def test_events_that_are_not_updates_or_have_bad_data_are_ignored(self): + data = handoff({"event": "handoff.create", "data": {}}, {"event": "handoff.update", "data": "garbage"}, update_event("claude", "h", approved=True)) + self.assertEqual(evaluate_handoff(data, DEFAULT_REVIEWERS)["approved_reviewers"], ["claude-plan"]) + + +class HandoffCase(unittest.TestCase): + def setUp(self): + self.repo_cm = TempRepo() + self.repo = self.repo_cm.__enter__() + self.addCleanup(self.repo_cm.__exit__, None, None, None) + patcher = mock.patch.dict(os.environ, {"CLODEX_REPO_ROOT": str(self.repo)}) + patcher.start() + self.addCleanup(patcher.stop) + + def contract(self, reviewers: list[dict], quorum: str = "unanimous") -> None: + (self.repo / "CLODEX.md").write_text(f"---\naudit:\n quorum: {quorum}\n reviewers: {json.dumps(reviewers)}\n---\nbody\n", encoding="utf-8") + + def call(self, name: str, **arguments): + result = mcp_server.tool_call(name, arguments) + text = result["content"][0]["text"] + try: + payload = json.loads(text) + except ValueError: + payload = text + return result["isError"], payload + + @property + def state(self) -> StateStore: + return ClodexWorkflow(self.repo).state + + +class DecideTests(HandoffCase): + def test_decide_uses_the_configured_reviewers_and_quorum(self): + self.contract([reviewer("alpha", "claude"), reviewer("beta", "codex"), reviewer("gamma", "codex")], quorum="majority") + self.call("clodex_handoff_create", run_id="h", task="t") + self.call("clodex_handoff_update", run_id="h", actor="claude", diff_hash="d1", report={"approved": True}) + is_error, pending = self.call("clodex_handoff_decide", run_id="h") + self.assertFalse(is_error) + self.assertEqual(pending["decision"], "needs_fix") + self.assertEqual(sorted(pending["required_pending"]), ["beta", "gamma"]) + self.assertEqual(pending["approved_reviewers"], ["alpha"]) + self.assertEqual(pending["quorum"], "majority") + self.call("clodex_handoff_update", run_id="h", actor="codex", diff_hash="d1", report={"approved": True, "reviewer_id": "gamma"}) + is_error, decision = self.call("clodex_handoff_decide", run_id="h") + self.assertEqual(decision["decision"], "approved") + self.assertEqual(decision["approved_reviewers"], ["alpha", "gamma"]) + self.assertEqual(decision["approved_by"], ["claude", "codex"]) + self.assertEqual(self.state.get_run("h")["status"], "approved") + + def test_unanimous_is_still_the_default(self): + self.call("clodex_handoff_create", run_id="u", task="t") + self.call("clodex_handoff_update", run_id="u", actor="claude", diff_hash="d", report={"approved": True}) + self.assertEqual(self.call("clodex_handoff_decide", run_id="u")[1]["decision"], "needs_fix") + self.call("clodex_handoff_update", run_id="u", actor="codex", diff_hash="d", report={"approved": True}) + self.assertEqual(self.call("clodex_handoff_decide", run_id="u")[1]["decision"], "approved") + + def test_unknown_reviewer_id_is_rejected_without_recording_anything(self): + self.call("clodex_handoff_create", run_id="r", task="t") + is_error, message = self.call("clodex_handoff_update", run_id="r", actor="claude", increment_handoff=True, diff_hash="d", report={"approved": True, "reviewer_id": "ghost"}) + self.assertTrue(is_error) + self.assertIn("unknown reviewer_id: ghost", message) + self.assertIn("claude-plan", message, "lists the configured reviewers") + run = self.state.get_run("r") + self.assertEqual(run["handoff_count"], 0) + self.assertEqual(self.state.get_handoff("r")["events"][-1]["event"], "handoff.create") + + +class LedgerTests(HandoffCase): + def rows(self, table: str, run_id: str): + with self.state.session() as con: + return [dict(row) for row in con.execute(f"select * from {table} where run_id=? order by id", (run_id,))] + + def task(self, task_id: str): + return next((t for t in self.state.list_tasks() if t["id"] == task_id), None) + + def test_a_handoff_appears_in_the_task_ledger_and_follows_its_run(self): + self.call("clodex_handoff_create", run_id="led", task="Ship the thing\nwith details") + self.assertEqual((self.task("led")["status"], self.task("led")["title"]), ("handoff", "Ship the thing")) + self.assertEqual(self.state.get_run("led")["task_id"], "led") + tasks = self.call("clodex_status")[1]["tasks"] + self.assertIn("led", [t["id"] for t in tasks]) + self.call("clodex_handoff_update", run_id="led", actor="claude", diff_hash="d", report={"approved": True}) + self.call("clodex_handoff_update", run_id="led", actor="codex", diff_hash="d", report={"approved": True}) + self.call("clodex_handoff_decide", run_id="led") + self.assertEqual(self.task("led")["status"], "done") + + def test_blocked_and_budget_exhausted_handoffs_block_their_task(self): + self.call("clodex_handoff_create", run_id="b1", task="t") + self.call("clodex_handoff_update", run_id="b1", actor="claude", status="blocked", blocked_reason="needs a human") + self.assertEqual(self.task("b1")["status"], "blocked") + self.call("clodex_handoff_create", run_id="b2", task="t", handoff_budget=1) + self.call("clodex_handoff_update", run_id="b2", actor="claude", increment_handoff=True) + self.call("clodex_handoff_update", run_id="b2", actor="codex", increment_handoff=True) + self.assertEqual(self.task("b2")["status"], "blocked") + + def test_verdict_reports_become_audit_rows(self): + self.call("clodex_handoff_create", run_id="aud", task="t") + self.call("clodex_handoff_update", run_id="aud", actor="claude", diff_hash="d1", report={"approved": False, "summary": "no tests"}) + self.call("clodex_handoff_update", run_id="aud", actor="codex", diff_hash="d1", report={"approved": True, "reviewer_id": "codex-architecture"}) + self.call("clodex_handoff_update", run_id="aud", actor="claude", report={"note": "not a verdict"}) + rows = self.rows("audits", "aud") + self.assertEqual([(r["agent"], r["approved"], r["diff_hash"]) for r in rows], [("claude", 0, "d1"), ("codex-architecture", 1, "d1")]) + self.assertEqual(json.loads(rows[0]["verdict_json"])["summary"], "no tests") + + def test_report_artifacts_become_artifact_rows_without_duplicates(self): + self.call("clodex_handoff_create", run_id="art", task="t") + report = {"artifacts": ["notes/plan.md", {"name": "diff", "path": "out/changes.diff", "kind": "patch"}, {"nope": 1}, 5]} + self.call("clodex_handoff_update", run_id="art", actor="claude", report=report) + self.call("clodex_handoff_update", run_id="art", actor="claude", report={"artifacts": ["notes/plan.md"]}) + rows = {r["name"]: r for r in self.rows("artifacts", "art")} + self.assertEqual(sorted(rows), ["diff", "plan.md"]) + self.assertEqual((rows["plan.md"]["kind"], rows["plan.md"]["path"]), ("md", "notes/plan.md")) + self.assertEqual(rows["diff"]["kind"], "patch") + data = self.call("clodex_handoff_get", run_id="art")[1] + self.assertEqual(sorted(a["name"] for a in data["artifacts"]), ["diff", "plan.md"]) + + +class WorkspaceTests(HandoffCase): + def worktrees(self) -> int: + out = subprocess.run(["git", "worktree", "list", "--porcelain"], cwd=self.repo, capture_output=True, text=True).stdout + return out.count("worktree ") + + def lock(self, run_id: str): + with self.state.session() as con: + row = con.execute("select * from workspace_locks where run_id=?", (run_id,)).fetchone() + return dict(row) if row else None + + def test_git_worktree_handoff_gets_an_isolated_checkout(self): + is_error, run = self.call("clodex_handoff_create", run_id="wt", task="t", workspace="git-worktree") + self.assertFalse(is_error, run) + path = Path(run["workspace"]["path"]) + self.assertTrue(path.is_dir()) + self.assertEqual(path.parent.resolve(), (self.repo / ".clodex" / "workspaces").resolve()) + self.assertEqual(self.state.get_run("wt")["workspace_path"], str(path)) + self.assertEqual(self.worktrees(), 2) + self.assertIsNone(self.lock("wt")["released_at"]) + self.assertEqual(self.call("clodex_handoff_get", run_id="wt")[1]["run"]["workspace_path"], str(path)) + + def test_local_none_and_default_workspaces(self): + _, local = self.call("clodex_handoff_create", run_id="loc", task="t", workspace="local") + self.assertEqual(Path(self.state.get_run("loc")["workspace_path"]).resolve(), self.repo.resolve()) + self.assertEqual(self.worktrees(), 1) + self.call("clodex_handoff_create", run_id="none", task="t", workspace="none") + self.call("clodex_handoff_create", run_id="default", task="t") + self.assertIsNone(self.state.get_run("none")["workspace_path"]) + self.assertIsNone(self.state.get_run("default")["workspace_path"]) + self.assertNotIn("workspace", self.call("clodex_handoff_create", run_id="plain", task="t")[1]) + + def test_invalid_backend_and_dirty_tree_are_tool_errors_that_create_nothing(self): + is_error, message = self.call("clodex_handoff_create", run_id="bad", task="t", workspace="docker") + self.assertTrue(is_error) + self.assertIn("workspace must be one of", message) + (self.repo / "seed.txt").write_text("dirty\n", encoding="utf-8") + is_error, message = self.call("clodex_handoff_create", run_id="dirty", task="t", workspace="git-worktree") + self.assertTrue(is_error) + self.assertIn("Tracked changes are present", message) + for run_id in ("bad", "dirty"): + self.assertIsNone(self.state.get_run(run_id)) + self.assertEqual(self.worktrees(), 1) + + def test_duplicate_run_id_creates_no_second_worktree(self): + self.call("clodex_handoff_create", run_id="dup", task="t", workspace="git-worktree") + is_error, message = self.call("clodex_handoff_create", run_id="dup", task="t", workspace="git-worktree") + self.assertTrue(is_error) + self.assertIn("already exists", message) + self.assertEqual(self.worktrees(), 2) + + def test_worktree_is_rolled_back_if_the_handoff_cannot_be_recorded(self): + with mock.patch.object(StateStore, "create_handoff", side_effect=ValueError("simulated failure")): + is_error, message = self.call("clodex_handoff_create", run_id="rb", task="t", workspace="git-worktree") + self.assertTrue(is_error) + self.assertIn("simulated failure", message) + self.assertEqual(self.worktrees(), 1, "no orphaned worktree") + self.assertFalse((self.repo / ".clodex" / "workspaces" / "rb").exists()) + + def test_clean_removes_the_worktree_after_the_handoff_finishes(self): + _, run = self.call("clodex_handoff_create", run_id="fin", task="t", workspace="git-worktree") + path = Path(run["workspace"]["path"]) + workflow = ClodexWorkflow(self.repo) + self.assertEqual(workflow.clean_run("fin").status, "clean-refused", "still in progress") + self.call("clodex_handoff_update", run_id="fin", actor="claude", status="blocked", blocked_reason="done for now") + self.assertEqual(workflow.clean_run("fin").status, "cleaned") + self.assertFalse(path.exists()) + self.assertEqual(self.worktrees(), 1) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_state_integrity.py b/tests/test_state_integrity.py index ef70e56..eba9b80 100644 --- a/tests/test_state_integrity.py +++ b/tests/test_state_integrity.py @@ -88,7 +88,9 @@ def test_cancel_never_overwrites_any_finished_status(self): class HandoffValidationTests(StateCase): def test_unknown_phase_and_status_are_rejected_known_ones_accepted(self): - self.store.create_handoff("h1", "t", "prompt", "claude") + self.store.create_handoff("h1", "prompt", owner="claude") + with self.assertRaisesRegex(ValueError, "unknown handoff phase"): + self.store.create_handoff("h2", "prompt", phase="vibing") for phase in sorted(HANDOFF_PHASES): self.assertEqual(self.store.update_handoff("h1", phase=phase)["phase"], phase) with self.assertRaisesRegex(ValueError, "unknown handoff phase"):