From d889864d2f302adcfdd84ee154dab6541a139efe Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 23:32:24 +0000 Subject: [PATCH] Verify the full schema contract and harden commit and CLI inputs Carries forward the parts of PR #10 that main does not have yet and that do not depend on the push-hook design: - verify_contract() replays the bundled migrations in memory and requires every table, index, and trigger to match the store (ignoring formatting). It rejects missing or altered objects, extra indexes or triggers, and stores migrated by a newer version; the named-trigger check stays first for its clearer message. - git_common_dir() no longer inherits the hook's repository-selecting Git environment, which resolved every registered path to the invoking repository whenever GIT_DIR was absolute. - verify_commit() resolves any commit spec (abbreviated OID, ref, uppercase) and returns the full OID; checkpoints and handoffs store it instead of failing a CHECK constraint with a traceback. - handoff create uses an explicit --last-commit over the worktree HEAD. - Duplicate agents, duplicate --protected-ref values, and malformed operation JSON are clean errors; task creation takes the write lock. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp --- README.md | 10 +- src/quorumgit/cli.py | 23 ++-- src/quorumgit/handoff.py | 9 +- src/quorumgit/registry.py | 28 ++++- src/quorumgit/store.py | 91 ++++++++++++-- src/quorumgit/work.py | 59 ++++++--- tests/test_hardening.py | 245 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 422 insertions(+), 43 deletions(-) create mode 100644 tests/test_hardening.py diff --git a/README.md b/README.md index c2826b5..d4a87a8 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ migrations applied: ['001_core.sql', '002_approval_identities.sql', '004_approva contract: ok ``` -`init` creates the state directory, applies migrations, and verifies the runtime contract (required migrations, tables, and governance triggers, foreign keys on, WAL journal mode). It is idempotent — re-run it any time. +`init` creates the state directory, applies migrations, and verifies the runtime contract: required migrations, tables, and governance triggers, foreign keys on, WAL journal mode, and **every schema object the governance rules depend on**. The expected schema is derived by replaying the bundled migrations in memory, so a store whose triggers or unique indexes are missing or altered — or that carries extra indexes or triggers, or was migrated by a newer version — fails the contract instead of silently losing an invariant. Every command runs this check. It is idempotent — re-run it any time. `quorumgit status` shows the store path, contract state, and row counts. If the store is missing or incomplete, every command exits non-zero: there is **one storage backend and no fallback**, by design. @@ -127,7 +127,7 @@ QUORUMGIT_AGENT=agent-two quorumgit handoff accept 1 # remaining work: wire into CLI, integration tests ``` -Creating a handoff releases the original claim but keeps its worktree; accepting transfers that same worktree to the new claim. When the claim has an active worktree, the handoff records the worktree's `HEAD` as the last commit; `--last-commit` is used when there is none (hub model, or a worktree removed by `doctor --repair`). +Creating a handoff releases the original claim but keeps its worktree; accepting transfers that same worktree to the new claim. The handoff records `--last-commit` as the continuation point when given (any commit spec, stored as the full OID), otherwise the active worktree's `HEAD`; a claim without an active worktree (hub model, or a worktree removed by `doctor --repair`) must pass `--last-commit`. What just *didn't* happen, silently: a second agent claiming task 1 (**BLOCKED**), claiming another task on branch `feat/x` (**CONFLICTING**), or claiming a task whose scopes overlap `src/**` (**OVERLAPPING**, refused unless explicitly overridden and audited). While the handoff was open, the task, branch, and declared scopes were **reserved** for continuation until agent-two accepted. @@ -176,7 +176,7 @@ The hook learns who is pushing from `QUORUMGIT_AGENT` in its own environment, wh Accepting a handoff in the hub model prints `worktree: (none — create manually)`: there is no managed worktree to transfer, so the successor fetches the branch into its own clone and continues from the reported commit. -Checkpoints in the hub model take an explicit `--commit `. Continuation points are **verified, not trusted**: the commit must exist in the registered repository, and when the claimed branch exists, be reachable from it. A typo'd or fabricated OID is rejected. +Checkpoints in the hub model take an explicit `--commit ` (any commit spec Git resolves — a full or abbreviated OID or a ref — recorded as the full OID). Continuation points are **verified, not trusted**: the commit must exist in the registered repository, and when the claimed branch exists, be reachable from it. A typo'd or fabricated OID is rejected. ## Content governance @@ -289,9 +289,9 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c | `quorumgit task done [--note ] [--remove-worktree]` | Close a task for good — live claim holder only; releases the claim | | `quorumgit claim --branch --scope … [--no-worktree] [--takeover] [--override-overlap] [--lease-hours ]` | Claim a task | | `quorumgit renew [--lease-hours ]` | Extend a live, unexpired lease; expired claims must be acquired again | -| `quorumgit checkpoint [--commit ] [--note ]` | Record verified progress | +| `quorumgit checkpoint [--commit ] [--note ]` | Record verified progress | | `quorumgit release [--remove-worktree] [--reason ]` | Release a claim; the task returns to `open` | -| `quorumgit handoff create --completed --remaining [--to ] [--files-changed ]… [--blockers ]… [--validation ] [--last-commit ]` | Hand work off (`--last-commit` only when the claim has no active worktree) | +| `quorumgit handoff create --completed --remaining [--to ] [--files-changed ]… [--blockers ]… [--validation ] [--last-commit ]` | Hand work off (from `--last-commit` when given, else the active worktree's HEAD) | | `quorumgit handoff accept [--lease-hours ]` | Continue handed-off work (addressee, or anyone if unaddressed) | | `quorumgit handoff decline ` | Decline — addressee only; removes the retained worktree, and is refused if it has uncommitted changes | | `quorumgit handoff cancel ` | Cancel — creator only; removes the retained worktree, and is refused if it has uncommitted changes | diff --git a/src/quorumgit/cli.py b/src/quorumgit/cli.py index 5ed33e5..9a59be7 100644 --- a/src/quorumgit/cli.py +++ b/src/quorumgit/cli.py @@ -383,6 +383,7 @@ def cmd_checkpoint(args, cfg) -> int: commit = trees.head_commit(wt["path"]) cp_id = work.add_checkpoint(conn, args.claim_id, agent, commit, note=args.note) + commit = work.checkpoint_commit(conn, cp_id) conn.commit() print(f"checkpoint {cp_id} at {commit}.") return 0 @@ -394,12 +395,16 @@ def cmd_checkpoint(args, cfg) -> int: def cmd_handoff_create(args, cfg) -> int: agent = _agent(args, cfg) with store.session(cfg) as conn: - wt = trees.active_worktree_for_claim(conn, args.claim_id) - last_commit = trees.head_commit(wt["path"]) if wt else args.last_commit + # An explicit --last-commit always wins; otherwise continue from the + # managed worktree's HEAD. + last_commit = args.last_commit if not last_commit: - print("Provide --last-commit (no active worktree for this claim).", - file=sys.stderr) - return 1 + wt = trees.active_worktree_for_claim(conn, args.claim_id) + if wt is None: + print("Provide --last-commit (no active worktree for this claim).", + file=sys.stderr) + return 1 + last_commit = trees.head_commit(wt["path"]) record = { "completed": args.completed, "remaining": args.remaining, @@ -414,6 +419,7 @@ def cmd_handoff_create(args, cfg) -> int: handoff_id = handoff.create_handoff( conn, args.claim_id, agent, record, to_agent=args.to ) + last_commit = handoff.get_handoff(conn, handoff_id)["record"]["last_commit"] conn.commit() print(f"handoff {handoff_id} created (last commit {last_commit}).") return 0 @@ -468,9 +474,12 @@ def cmd_handoff_cancel(args, cfg) -> int: def _operation_from_args(args) -> dict: - operation = json.loads(args.operation) + try: + operation = json.loads(args.operation) + except json.JSONDecodeError as exc: + raise gate.GateError(f"Operation is not valid JSON: {exc}") from exc if not isinstance(operation, dict): - raise SystemExit("Operation must be a JSON object.") + raise gate.GateError("Operation must be a JSON object.") return operation diff --git a/src/quorumgit/handoff.py b/src/quorumgit/handoff.py index dd48ce9..09d5abb 100644 --- a/src/quorumgit/handoff.py +++ b/src/quorumgit/handoff.py @@ -37,9 +37,12 @@ def create_handoff( if not record.get(field): raise HandoffError(f"Handoff record requires {field!r}.") task = work.get_task(conn, claim["task_id"]) - work.verify_commit( - task["repository_path"], record["last_commit"], branch=claim["branch"] - ) + record = { + **record, + "last_commit": work.verify_commit( + task["repository_path"], record["last_commit"], branch=claim["branch"] + ), + } to_agent_id = get_agent(conn, to_agent)["id"] if to_agent else None row = conn.execute( diff --git a/src/quorumgit/registry.py b/src/quorumgit/registry.py index 21436a5..72335d1 100644 --- a/src/quorumgit/registry.py +++ b/src/quorumgit/registry.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os import subprocess from pathlib import Path @@ -96,6 +97,28 @@ def _repository_dict(conn: Connection, row) -> dict: } +# Variables Git exports to hooks that select a repository. A lookup of a +# specific path must not inherit them, or every path resolves to the hook's +# repository. +_REPOSITORY_SELECTING_ENV = ( + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_COMMON_DIR", + "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_QUARANTINE_PATH", +) + + +def path_scoped_git_env() -> dict[str, str]: + return { + key: value + for key, value in os.environ.items() + if key not in _REPOSITORY_SELECTING_ENV + } + + def git_common_dir(path: str | Path) -> Path: """Return the canonical Git common directory for a repository path. @@ -109,6 +132,7 @@ def git_common_dir(path: str | Path) -> Path: capture_output=True, text=True, check=False, + env=path_scoped_git_env(), ) if result.returncode != 0: detail = (result.stderr or result.stdout).strip() @@ -206,7 +230,7 @@ def add_repository( ).fetchone() assert row is not None repo_id = row[0] - for refname in protected_refs or []: + for refname in dict.fromkeys(protected_refs or []): conn.execute( "INSERT INTO protected_refs (repository_id, refname) VALUES (?, ?)", (repo_id, refname), @@ -428,6 +452,8 @@ def add_agent( begin_immediate(conn) if role != DEFAULT_ROLE: require_operator(conn, actor, f"Registering a {role}") + if conn.execute("SELECT 1 FROM agents WHERE name = ?", (name,)).fetchone(): + raise RegistryError(f"Agent is already registered: {name}") row = conn.execute( "INSERT INTO agents (name, role) VALUES (?, ?) RETURNING id", (name, role) ).fetchone() diff --git a/src/quorumgit/store.py b/src/quorumgit/store.py index 7501cbb..7b05c28 100644 --- a/src/quorumgit/store.py +++ b/src/quorumgit/store.py @@ -8,7 +8,9 @@ from __future__ import annotations +import functools import json +import re import sqlite3 from collections.abc import Iterator from contextlib import contextmanager @@ -26,6 +28,14 @@ MINIMUM_SQLITE_VERSION = (3, 38, 0) MIGRATION_SEPARATOR = "-- quorumgit-statement" +# Kept byte-identical to the statement existing stores were created with. +SCHEMA_MIGRATIONS_DDL = """ + CREATE TABLE IF NOT EXISTS schema_migrations ( + version TEXT PRIMARY KEY, + applied_at INTEGER NOT NULL DEFAULT (unixepoch()) + ) + """ + REQUIRED_TABLES = ( "schema_migrations", "repositories", @@ -293,14 +303,7 @@ def migrate(target: Config | str | Path) -> list[str]: applied: list[str] = [] try: begin_immediate(conn) - conn.execute( - """ - CREATE TABLE IF NOT EXISTS schema_migrations ( - version TEXT PRIMARY KEY, - applied_at INTEGER NOT NULL DEFAULT (unixepoch()) - ) - """ - ) + conn.execute(SCHEMA_MIGRATIONS_DDL) conn.commit() done = { @@ -339,6 +342,71 @@ def migrate(target: Config | str | Path) -> list[str]: # ------------------------------------------------------------ contract check +def _normalized_sql(sql: str | None) -> str | None: + """Schema SQL with formatting-only whitespace removed.""" + if sql is None: + return None + return re.sub(r" ?([(),;]) ?", r"\1", " ".join(sql.split())) + + +def _schema_objects(conn: Connection) -> dict[tuple[str, str], tuple[str, str | None]]: + """Every user schema object as {(type, name): (table, normalized SQL)}.""" + rows = conn.execute( + "SELECT type, name, tbl_name, sql FROM sqlite_master " + "WHERE name NOT LIKE 'sqlite_%'" + ).fetchall() + return {(row[0], row[1]): (row[2], _normalized_sql(row[3])) for row in rows} + + +@functools.lru_cache(maxsize=1) +def _reference_schema() -> dict[tuple[str, str], tuple[str, str | None]]: + """The schema this QuorumGit build expects: every migration replayed in memory. + + Deriving the fingerprint from the migrations themselves keeps it exact + without a hand-maintained list: every table, index (including the partial + unique indexes that enforce single ownership and single live approvals) + and trigger must be present with the same definition. + """ + conn = sqlite3.connect(":memory:") + try: + conn.execute(SCHEMA_MIGRATIONS_DDL) + for mig in _migration_files(): + for statement in _migration_statements(mig.read_text(encoding="utf-8")): + conn.execute(statement) + return _schema_objects(conn) + finally: + conn.close() + + +def _describe(keys: set[tuple[str, str]]) -> str: + return ", ".join(f"{kind} {name}" for kind, name in sorted(keys)) + + +def _verify_schema_objects(conn: Connection) -> None: + expected = _reference_schema() + actual = _schema_objects(conn) + missing = set(expected) - set(actual) + changed = { + key for key in set(expected) & set(actual) if expected[key] != actual[key] + } + # Extra tables are inert data; an extra index or trigger can change what + # governance writes succeed or what they do, so it is a violation. + unexpected = {key for key in set(actual) - set(expected) if key[0] != "table"} + problems = [] + if missing: + problems.append(f"missing {_describe(missing)}") + if changed: + problems.append(f"altered {_describe(changed)}") + if unexpected: + problems.append(f"unexpected {_describe(unexpected)}") + if problems: + raise ContractViolation( + "Store schema does not match this QuorumGit version: " + + "; ".join(problems) + + ". Governance invariants cannot be guaranteed." + ) + + def verify_contract(target: Config | Connection | str | Path) -> None: """Fail loudly unless the local store satisfies the runtime contract.""" owned = isinstance(target, (Config, str, Path)) @@ -376,6 +444,12 @@ def verify_contract(target: Config | Connection | str | Path) -> None: f"Missing required migrations: {sorted(missing_migrations)}. " "Run `quorumgit init` to apply migrations." ) + unknown_migrations = applied_migrations - required_migrations + if unknown_migrations: + raise ContractViolation( + f"Store has migrations this QuorumGit does not know: " + f"{sorted(unknown_migrations)}. It was written by a newer version." + ) missing = set(REQUIRED_TABLES) - tables if missing: @@ -395,6 +469,7 @@ def verify_contract(target: Config | Connection | str | Path) -> None: raise ContractViolation( f"Missing required governance triggers: {sorted(missing_triggers)}." ) + _verify_schema_objects(conn) foreign_keys = conn.execute("PRAGMA foreign_keys").fetchone()[0] journal_mode = str(conn.execute("PRAGMA journal_mode").fetchone()[0]).lower() diff --git a/src/quorumgit/work.py b/src/quorumgit/work.py index b107870..81f40c2 100644 --- a/src/quorumgit/work.py +++ b/src/quorumgit/work.py @@ -45,6 +45,7 @@ def _lease_seconds(lease_hours: float) -> int: def create_task( conn: Connection, repository: str, title: str, objective: str = "" ) -> int: + begin_immediate(conn) repo = get_repository(conn, repository) row = conn.execute( """ @@ -411,24 +412,32 @@ def paths_within(paths: list[str], scopes: list[str]) -> list[str]: def verify_commit( repo_path: str, commit_oid: str, branch: str | None = None -) -> None: - """Require an existing commit, reachable from the branch when it exists.""" - exists = ( - subprocess.run( - [ - "git", - "-C", - str(repo_path), - "cat-file", - "-e", - f"{commit_oid}^{{commit}}", - ], - capture_output=True, - check=False, - ).returncode - == 0 +) -> str: + """Resolve a commit to its full object ID and verify it as a continuation. + + Accepts anything Git resolves to a commit (full or abbreviated OID, a ref) + and returns the canonical lowercase full OID. The commit must exist in the + registered repository and be reachable from the branch when it exists. + """ + spec = commit_oid.strip() + if not spec or spec.startswith("-"): + raise WorkError(f"Invalid commit: {commit_oid!r}") + resolved = subprocess.run( + [ + "git", + "-C", + str(repo_path), + "rev-parse", + "--verify", + "--quiet", + f"{spec}^{{commit}}", + ], + capture_output=True, + text=True, + check=False, ) - if not exists: + oid = resolved.stdout.strip().lower() + if resolved.returncode != 0 or not re.fullmatch(r"[0-9a-f]{40}|[0-9a-f]{64}", oid): raise WorkError( f"Commit {commit_oid} does not exist in the registered repository." ) @@ -458,7 +467,7 @@ def verify_commit( str(repo_path), "merge-base", "--is-ancestor", - commit_oid, + oid, f"refs/heads/{branch}", ], capture_output=True, @@ -471,6 +480,7 @@ def verify_commit( f"Commit {commit_oid} is not reachable from branch " f"{branch!r} in the registered repository." ) + return oid def classify( @@ -871,7 +881,9 @@ def add_checkpoint( f"Claim {claim_id} belongs to {claim['agent']}, not {agent}." ) task = get_task(conn, claim["task_id"]) - verify_commit(task["repository_path"], commit_oid, branch=claim["branch"]) + commit_oid = verify_commit( + task["repository_path"], commit_oid, branch=claim["branch"] + ) row = conn.execute( """ INSERT INTO checkpoints (claim_id, commit_oid, note, detail) @@ -889,3 +901,12 @@ def add_checkpoint( detail={"commit": commit_oid, "note": note}, ) return row[0] + + +def checkpoint_commit(conn: Connection, checkpoint_id: int) -> str: + row = conn.execute( + "SELECT commit_oid FROM checkpoints WHERE id = ?", (checkpoint_id,) + ).fetchone() + if row is None: + raise WorkError(f"No such checkpoint: {checkpoint_id}") + return row[0] diff --git a/tests/test_hardening.py b/tests/test_hardening.py new file mode 100644 index 0000000..a2e307a --- /dev/null +++ b/tests/test_hardening.py @@ -0,0 +1,245 @@ +"""Independent hardening carried forward from PR #10. + +- verify_contract() compares the whole schema against the bundled migrations. +- Repository identity lookups ignore the hook's repository-selecting Git env. +- Commit specs resolve to full OIDs; user errors never print tracebacks. +- `handoff create --last-commit` wins over the worktree's HEAD. +""" + +from __future__ import annotations + +import os +import re +import subprocess +import uuid +from pathlib import Path + +import pytest + +from quorumgit import handoff, registry, store, trees, work +from quorumgit.config import Config +from tests.conftest import make_git_repo +from tests.test_cli_hub import _cli + +GIT_ENV = { + **os.environ, + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@localhost", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@localhost", +} + + +def _git(path: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(path), *args], + check=True, + capture_output=True, + text=True, + env=GIT_ENV, + ).stdout.strip() + + +def _local_setup(conn, tmp_path): + """A registered working repository (local model), one worker, one task.""" + suffix = uuid.uuid4().hex[:8] + repo_path = make_git_repo(tmp_path / f"repo-{suffix}") + repo = f"local-{suffix}" + registry.add_repository(conn, repo, repo_path) + agent = f"a-{suffix}" + registry.add_agent(conn, agent) + task = work.create_task(conn, repo, "work") + conn.commit() + return repo_path, task, agent + + +def _claim_id(output: str) -> int: + match = re.search(r"claim (\d+) acquired", output) + assert match is not None, output + return int(match.group(1)) + + +def _no_traceback(result) -> None: + assert "Traceback" not in result.stderr, result.stderr + assert result.returncode == 1, (result.stdout, result.stderr) + assert result.stderr.startswith("[quorumgit] ERROR:"), result.stderr + + +# ------------------------------------------------------------ schema contract + + +@pytest.fixture() +def fresh_store(tmp_path) -> Config: + cfg = Config(data_dir=tmp_path / "contract", agent=None) + store.migrate(cfg) + store.verify_contract(cfg) + return cfg + + +def _tamper(cfg: Config, *statements: str) -> None: + conn = store.open_connection(cfg) + try: + for statement in statements: + conn.execute(statement) + conn.commit() + finally: + conn.close() + + +def test_contract_rejects_a_missing_trigger_outside_the_named_list(fresh_store): + trigger = "approval_requester_updates_require_registration" + assert trigger not in store.REQUIRED_TRIGGERS + _tamper(fresh_store, f"DROP TRIGGER {trigger}") + with pytest.raises(store.ContractViolation, match=f"missing trigger {trigger}"): + store.verify_contract(fresh_store) + with pytest.raises(store.ContractViolation): + store.connect(fresh_store) + + +def test_contract_rejects_a_weakened_unique_index(fresh_store): + _tamper( + fresh_store, + "DROP INDEX claims_one_active_per_task", + "CREATE INDEX claims_one_active_per_task ON claims(task_id) " + "WHERE released_at IS NULL", + ) + with pytest.raises( + store.ContractViolation, match="altered index claims_one_active_per_task" + ): + store.verify_contract(fresh_store) + + +def test_contract_rejects_an_unexpected_trigger(fresh_store): + _tamper( + fresh_store, + "CREATE TRIGGER auto_approve AFTER INSERT ON approvals BEGIN " + "UPDATE approvals SET status = 'approved' WHERE id = NEW.id; END", + ) + with pytest.raises(store.ContractViolation, match="unexpected trigger auto_approve"): + store.verify_contract(fresh_store) + + +def test_contract_rejects_a_store_from_a_newer_version(fresh_store): + _tamper( + fresh_store, "INSERT INTO schema_migrations (version) VALUES ('999_future.sql')" + ) + with pytest.raises(store.ContractViolation, match="newer version"): + store.verify_contract(fresh_store) + + +def test_contract_tolerates_formatting_only_schema_differences(fresh_store): + _tamper( + fresh_store, + "DROP INDEX approvals_one_live_per_operation", + "CREATE UNIQUE INDEX approvals_one_live_per_operation ON approvals " + "( operation_hash ) WHERE status IN ('pending','approved')", + ) + store.verify_contract(fresh_store) + + +# ------------------------------------------------------ repository identity + + +def test_identity_lookup_ignores_the_hooks_git_dir(tmp_path, monkeypatch): + hub = make_git_repo(tmp_path / "hook-repo") + other = make_git_repo(tmp_path / "other-repo") + monkeypatch.setenv("GIT_DIR", str(hub / ".git")) + assert registry.git_common_dir(other) == (other / ".git").resolve() + + +# ------------------------------------------------- commit specs and errors + + +def test_checkpoint_accepts_abbreviated_commit(committed_conn, tmp_path, cfg): + repo_path, task, agent = _local_setup(committed_conn, tmp_path) + claimed = _cli(cfg, "claim", str(task), "--branch", "feat/short", + "--scope", "src/**", "--no-worktree", agent=agent) + assert claimed.returncode == 0, claimed.stderr + claim_id = _claim_id(claimed.stdout) + full = _git(repo_path, "rev-parse", "main") + + for spec in (full[:9], full.upper(), "main"): + result = _cli(cfg, "checkpoint", str(claim_id), "--commit", spec, agent=agent) + assert result.returncode == 0, result.stderr + assert full in result.stdout + + bogus = _cli(cfg, "checkpoint", str(claim_id), "--commit", "nope", agent=agent) + _no_traceback(bogus) + assert "does not exist" in bogus.stderr + + +def test_duplicate_agent_registration_is_a_clean_error(committed_conn, tmp_path, cfg): + _repo_path, _task, agent = _local_setup(committed_conn, tmp_path) + result = _cli(cfg, "agent", "add", agent) + _no_traceback(result) + assert "already registered" in result.stderr + + +def test_duplicate_protected_ref_is_deduplicated(committed_conn, tmp_path): + repo_path = make_git_repo(tmp_path / "dup-refs") + name = f"dup-{uuid.uuid4().hex[:8]}" + registry.add_repository( + committed_conn, + name, + repo_path, + protected_refs=["refs/heads/main", "refs/heads/main"], + ) + assert registry.get_repository(committed_conn, name)["protected_refs"] == [ + "refs/heads/main" + ] + + +@pytest.mark.parametrize("payload", ["{bad", "[1, 2]"]) +def test_invalid_operation_json_is_a_clean_error(cfg, initialized_store, payload): + result = _cli(cfg, "approve", "hash", payload) + _no_traceback(result) + assert "Operation" in result.stderr + + +# ------------------------------------------------------ handoff last commit + + +def test_explicit_last_commit_wins_over_worktree_head(committed_conn, tmp_path, cfg): + conn = committed_conn + _repo_path, task, agent = _local_setup(conn, tmp_path) + claimed = _cli(cfg, "claim", str(task), "--branch", "feat/hand", + "--scope", "src/**", agent=agent) + assert claimed.returncode == 0, claimed.stderr + claim_id = _claim_id(claimed.stdout) + wt = trees.active_worktree_for_claim(conn, claim_id) + assert wt is not None + wt_path = Path(wt["path"]) + base = _git(wt_path, "rev-parse", "HEAD") + (wt_path / "src" / "more.py").write_text("x\n") + _git(wt_path, "add", "-A") + _git(wt_path, "commit", "-m", "later work") + assert _git(wt_path, "rev-parse", "HEAD") != base + + created = _cli(cfg, "handoff", "create", str(claim_id), "--completed", "c", + "--remaining", "r", "--last-commit", base[:10], agent=agent) + assert created.returncode == 0, created.stderr + assert base in created.stdout + match = re.search(r"handoff (\d+) created", created.stdout) + assert match is not None, created.stdout + assert handoff.get_handoff(conn, int(match.group(1)))["record"]["last_commit"] == base + + +def test_handoff_without_worktree_requires_last_commit(committed_conn, tmp_path, cfg): + conn = committed_conn + repo_path, task, agent = _local_setup(conn, tmp_path) + claimed = _cli(cfg, "claim", str(task), "--branch", "feat/gone", + "--scope", "src/**", agent=agent) + assert claimed.returncode == 0, claimed.stderr + claim_id = _claim_id(claimed.stdout) + trees.remove_worktree(conn, claim_id, agent) + conn.commit() + + missing = _cli(cfg, "handoff", "create", str(claim_id), "--completed", "c", + "--remaining", "r", agent=agent) + assert missing.returncode == 1 + assert "--last-commit" in missing.stderr + + head = _git(repo_path, "rev-parse", "feat/gone") + created = _cli(cfg, "handoff", "create", str(claim_id), "--completed", "c", + "--remaining", "r", "--last-commit", head, agent=agent) + assert created.returncode == 0, created.stderr