diff --git a/README.md b/README.md index 4f55253..b29fe38 100644 --- a/README.md +++ b/README.md @@ -25,17 +25,18 @@ QuorumGit makes each of these either impossible or explicitly governed, using me ## The mental model -Seven concepts, in the order you meet them: +Eight concepts, in the order you meet them: | Concept | What it is | |---|---| | **Repository** | A registered Git repo that QuorumGit governs. | -| **Agent** | A registered identity. Set via `QUORUMGIT_AGENT` or `--agent`. | +| **Agent** | A registered identity with one role — `worker` (the default), `reviewer`, or `operator`. Set via `QUORUMGIT_AGENT` or `--agent`. | | **Task** | A unit of work against one repository. | | **Claim** | An agent's exclusive lease on a task: names a branch, declares at least one write **scope** (path glob), and expires at a timestamp (default lease: 8 hours, set with `--lease-hours`). Expired leases make the task reclaimable and cannot be renewed — evaluated at read time, no timers. | -| **Worktree** | An isolated `git worktree` created per claim. Agents never share a mutable checkout; Git itself refuses to check one branch out twice. | +| **Worktree** | An isolated `git worktree` created per claim. Agents never share a mutable checkout; Git itself refuses to check one branch out twice. A claim that supersedes an earlier claim on the same task and branch continues its retained checkout instead of creating a second one. | | **Handoff** | A structured continuation record (done / remaining / exact commit / blockers) that transfers work to a successor instead of abandoning it. | -| **Approval** | A sign-off by registered agents, hash-bound to one exact operation (a specific push, takeover, or deletion), consumed on use. | +| **Approval** | A sign-off by eligible agents, hash-bound to one exact operation (a specific push, takeover, or deletion), consumed on use. | +| **Approval policy** | Owned by each repository: how many votes an approval needs, which roles may cast them, and whether the requester may vote. The agent asking for permission never chooses its own quorum. | Everything an agent does — claim, renew, checkpoint, hand off, release — writes an audit event in the same database transaction. The audit table is append-only, enforced by a trigger. @@ -58,11 +59,11 @@ Then initialize the store once: ```bash $ quorumgit init store: /home/you/.quorumgit/quorumgit.db -migrations applied: ['001_core.sql', '002_approval_identities.sql'] +migrations applied: ['001_core.sql', '002_approval_identities.sql', '004_approval_authority.sql'] contract: ok ``` -`init` creates the state directory, applies migrations, and verifies the runtime contract (required tables, 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). 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. @@ -72,6 +73,7 @@ Register a repository and two agents, then run one full work cycle: ```bash quorumgit repo add myproject /path/to/working-repo +quorumgit agent add lead --role operator # the first operator needs no sponsor quorumgit agent add agent-one quorumgit agent add agent-two export QUORUMGIT_AGENT=agent-one @@ -81,10 +83,12 @@ quorumgit task add --repo myproject --title "implement feature X" quorumgit claim 1 --branch feat/x --scope 'src/**' # claim 1 acquired (CLEAR). -# worktree: ~/.quorumgit/worktrees/myproject/task-1-agent-one-claim-1 +# worktree: ~/.quorumgit/worktrees/repo-1/claim-1-3f9a0c1e # branch: feat/x ``` +Managed worktree paths are built from database IDs plus a random suffix. Repository and agent names are display identities and never become path components, so no registered name can place a checkout outside `QUORUMGIT_DATA_DIR/worktrees`. + The agent now works **inside that worktree** — edits, commits, tests — without touching anyone else's checkout. Along the way: ```bash @@ -110,7 +114,7 @@ quorumgit handoff create 1 \ QUORUMGIT_AGENT=agent-two quorumgit handoff accept 1 # claim 2 acquired via handoff. -# worktree: ~/.quorumgit/worktrees/myproject/task-1-agent-one-claim-1 +# worktree: ~/.quorumgit/worktrees/repo-1/claim-1-3f9a0c1e # branch: feat/x # continue from commit: # remaining work: wire into CLI, integration tests @@ -176,12 +180,11 @@ The flow, driven by the rejection messages themselves: # protected_ref_update on refs/heads/main requires an approval # bound to this exact update (hash sha256:ab12…). -# 2. Any registered agent requests that exact operation, and an approver votes -# on the returned approval instance ID. The approver must be registered too: -quorumgit agent add operator -quorumgit approve request '{"type":"protected_ref_update","repository":"myproject","refname":"refs/heads/main","oldrev":"","newrev":""}' -# approval 17 hash=sha256:ab12… status=pending -quorumgit approve vote 17 --agent operator +# 2. The agent requests that exact operation; an operator votes on the returned +# approval instance ID. The threshold comes from the repository's policy: +QUORUMGIT_AGENT=agent-one quorumgit approve request '{"type":"protected_ref_update","repository":"myproject","refname":"refs/heads/main","oldrev":"","newrev":""}' +# approval 17 hash=sha256:ab12… status=pending threshold=1 +quorumgit approve vote 17 --agent lead # 3. The same push now lands. Pushing it again — or replaying the approval — is rejected: # the approval was consumed atomically when the operation was accepted. @@ -194,10 +197,20 @@ Rules that hold no matter what: - **Consumption is single-use under concurrency.** Two simultaneous pushes racing for one approval produce exactly one accepted push — the loser is rejected, not silently allowed. - **Votes bind to one approval instance.** A delayed vote for an older denied or consumed instance cannot decide a newer request with the same operation hash. Requesters, voters, consumers, and pushers must name registered agents. - **A consumed approval is spent, not blacklisted.** Consumption moves the approval to a terminal `consumed` state (with `consumed_at`) rather than reusing `denied`, and only one *live* (`pending` or `approved`) approval may exist per operation hash. The same operation can therefore be requested and approved again later as a new approval instance — which matters for takeovers, whose payload is stable and legitimately repeatable. What is never possible is one approval authorizing twice. -- The default threshold is 1; `--threshold` sets a higher one. "Operator" is a convention, not a role: **any registered agent can vote, including the agent that requested the approval or is making the push.** Keeping approvals in human hands depends on how agent identities are used, not on anything QuorumGit enforces (see the threat model). +- **Nobody authorizes themselves.** Only agents whose role the repository's policy names may vote, in either direction. The requester may not vote unless the policy says so, the agent a takeover would benefit may not vote on it, and an agent that voted to approve an operation can never be the one that carries it out — pushes and takeovers by an approver are rejected, and a database trigger refuses the consumption even for writers that bypass the CLI. +- **Authority is checked when it is used, not only when it is granted.** Consumption re-derives eligible votes from current roles and policy, so raising the threshold or demoting an approver invalidates approvals that no longer meet it. +- **The repository owns the policy.** New repositories require one vote from an `operator`, and the requester may not vote. Change it with `quorumgit repo policy [--threshold ] [--role ]… [--requester-may-vote | --no-requester-may-vote]`. `approve request` no longer accepts `--threshold`. + +### Roles and administration + +Registering a `reviewer` or `operator`, changing a role (`quorumgit agent role `), and changing repository policy are themselves authority decisions, so each requires an operator acting through `--agent` or `QUORUMGIT_AGENT`, and each is audited. The single exception is bootstrap: while no operator exists, the first one can be designated by anyone. The last remaining operator cannot be demoted, so a store never falls back into bootstrap mode by accident. + +> **Upgrading.** Migration `004_approval_authority.sql` makes every existing agent a `worker` and gives every existing repository the default policy. Approvals granted under the old rules (including self-approvals) stop authorizing anything until an eligible operator votes on a fresh request. After running `quorumgit init`, designate an operator with `quorumgit agent role operator`. Takeovers follow the same pattern: claiming a task someone else holds (`claim --takeover`) prints the takeover operation to approve. Its payload includes the incumbent claim ID, so an unused approval cannot displace a later claim by the same agent. The takeover is atomic — the incumbent is released, the replacement claim created, and the approval consumed in one transaction, or none of it happens. A refused takeover leaves the incumbent untouched and the approval unconsumed. +The incumbent's checkout is not duplicated. When a claim supersedes an earlier claim on the same task and branch — an approved takeover, a reclaim after lease expiry, or a re-claim after `release` without `--remove-worktree` — the retained worktree, including any uncommitted work, is verified against its recorded repository and branch and transferred to the new claim (audited as `worktree.continued`). A retained checkout that has drifted to another branch or belongs to a different task is never adopted; the claim is refused with an explanation and rolls back entirely. A recorded checkout whose directory is gone has nothing to continue, so a fresh worktree is created and `quorumgit doctor` reports the stale record. + ## Command reference | Command | Purpose | @@ -207,7 +220,9 @@ Takeovers follow the same pattern: claiming a task someone else holds (`claim [--protected-ref ]…` | Register a repository | -| `quorumgit agent add ` | Register an agent identity | +| `quorumgit agent add [--role worker\|reviewer\|operator]` | Register an agent identity (non-workers need an operator once one exists) | +| `quorumgit agent role ` | Change an agent's role (operator only, after bootstrap) | +| `quorumgit repo policy [--threshold ] [--role ]… [--[no-]requester-may-vote]` | Show or change a repository's approval policy (changes are operator only) | | `quorumgit task add --repo --title [--objective ]` | Create a task | | `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 | @@ -218,13 +233,13 @@ Takeovers follow the same pattern: claiming a task someone else holds (`claim ` | 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 | | `quorumgit handoff list [--status ] / show ` | Inspect handoffs | -| `quorumgit approve request [--threshold ]` | Open an approval for an exact operation | +| `quorumgit approve request ` | Open an approval for an exact operation, at the repository's threshold | | `quorumgit approve vote [--deny]` | Vote on one approval instance | | `quorumgit approve hash ` | Compute an operation's hash | | `quorumgit hook install --repo ` | Install the pre-receive hook (hub model) | | `quorumgit audit [--entity ] [--entity-id ] [--limit ]` | Read the audit trail | -`repo list`, `agent list`, and `task list [--repo ]` enumerate what's registered. Commands that act as an agent (`claim`, `renew`, `release`, `checkpoint`, `handoff create/accept/decline/cancel`, `approve request/vote`) also take `--agent `, which overrides `QUORUMGIT_AGENT`. At least one `--scope` is required to claim. Exit codes: `0` success, `1` refused/violation/error, `2` usage error. +`repo list`, `agent list`, and `task list [--repo ]` enumerate what's registered. Commands that act as an agent (`claim`, `renew`, `release`, `checkpoint`, `handoff create/accept/decline/cancel`, `approve request/vote`, and the operator actions `agent add`, `agent role`, `repo policy`) also take `--agent `, which overrides `QUORUMGIT_AGENT`. At least one `--scope` is required to claim. Exit codes: `0` success, `1` refused/violation/error, `2` usage error. `quorumgit doctor` only checks worktree paths already recorded by QuorumGit. It reports: @@ -259,7 +274,8 @@ QuorumGit v1 coordinates **cooperating agents**; the adversary is *accident, not - Agent identity is asserted (`QUORUMGIT_AGENT`), not cryptographically authenticated. Any local process can claim to be any agent. - The trust root is write access to the database and the filesystem. An actor with either can bypass governance. - The pre-receive hook governs `git push` only. Direct ref manipulation inside a repository bypasses it. -- Approval hashes provide exact-payload binding and tamper-evidence, not approver authentication. Any registered agent may vote, including the requester or pusher, so an agent can approve its own protected operation. +- Approval hashes provide exact-payload binding and tamper-evidence, not approver authentication. +- Roles separate duties between cooperating agents. Because identity is asserted, a process willing to impersonate an operator can still do so; role separation stops an agent from authorizing its own work by accident or by default, not a determined impersonator. These are the correct trade-offs for preventing well-intentioned agents from colliding on one machine. They are not Byzantine fault tolerance, and this document will not pretend otherwise. diff --git a/src/quorumgit/cli.py b/src/quorumgit/cli.py index ffea5b4..ff3588f 100644 --- a/src/quorumgit/cli.py +++ b/src/quorumgit/cli.py @@ -52,7 +52,7 @@ def cmd_status(args, cfg) -> int: except store.StoreError as exc: print(f"contract: VIOLATED — {exc}") return 1 - with store.connect(cfg) as conn: + with store.session(cfg) as conn: for table in ("repositories", "agents", "tasks", "claims", "handoffs"): row = conn.execute(f"SELECT count(*) FROM {table}").fetchone() assert row is not None @@ -71,7 +71,7 @@ def cmd_destroy(args, cfg) -> int: def cmd_doctor(args, cfg) -> int: """Inspect and optionally reconcile recorded managed-worktree drift.""" - with store.connect(cfg) as conn: + with store.session(cfg) as conn: findings = trees.doctor_worktrees(conn, repair=args.repair) if args.repair: conn.commit() @@ -102,7 +102,7 @@ def cmd_doctor(args, cfg) -> int: def cmd_repo_add(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: repo_id = registry.add_repository( conn, args.name, args.path, protected_refs=args.protected_ref ) @@ -112,25 +112,70 @@ def cmd_repo_add(args, cfg) -> int: def cmd_repo_list(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: for repo in registry.list_repositories(conn): refs = ",".join(repo["protected_refs"]) or "-" print(f"{repo['id']}\t{repo['name']}\t{repo['path']}\tprotected:{refs}") return 0 +def cmd_repo_policy(args, cfg) -> int: + with store.session(cfg) as conn: + changing = ( + args.threshold is not None + or args.role + or args.requester_may_vote is not None + ) + if changing: + policy = registry.set_approval_policy( + conn, + args.name, + actor=getattr(args, "agent", None) or cfg.agent, + threshold=args.threshold, + roles=args.role or None, + requester_may_vote=args.requester_may_vote, + ) + conn.commit() + else: + repo = registry.get_repository(conn, args.name) + policy = registry.approval_policy(conn, repo["id"]) + print(f"repository: {args.name}") + print(f"approval threshold: {policy['threshold']}") + print(f"approving roles: {', '.join(policy['roles'])}") + print(f"requester may vote: {'yes' if policy['requester_may_vote'] else 'no'}") + return 0 + + def cmd_agent_add(args, cfg) -> int: - with store.connect(cfg) as conn: - agent_id = registry.add_agent(conn, args.name) + with store.session(cfg) as conn: + agent_id = registry.add_agent( + conn, + args.name, + role=args.role, + actor=getattr(args, "agent", None) or cfg.agent, + ) conn.commit() - print(f"agent {args.name} registered (id {agent_id}).") + print(f"agent {args.name} registered as {args.role} (id {agent_id}).") + return 0 + + +def cmd_agent_role(args, cfg) -> int: + with store.session(cfg) as conn: + registry.set_agent_role( + conn, + args.name, + args.role, + actor=getattr(args, "agent", None) or cfg.agent, + ) + conn.commit() + print(f"agent {args.name} is now a {args.role}.") return 0 def cmd_agent_list(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: for agent in registry.list_agents(conn): - print(f"{agent['id']}\t{agent['name']}") + print(f"{agent['id']}\t{agent['name']}\t{agent['role']}") return 0 @@ -138,7 +183,7 @@ def cmd_agent_list(args, cfg) -> int: def cmd_task_add(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: task_id = work.create_task(conn, args.repo, args.title, args.objective) conn.commit() print(f"task {task_id} created.") @@ -146,7 +191,7 @@ def cmd_task_add(args, cfg) -> int: def cmd_task_list(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: for task in work.list_tasks(conn, repository=args.repo): print(f"{task['id']}\t{task['repository']}\t{task['status']}\t" f"{task['title']}") @@ -155,7 +200,7 @@ def cmd_task_list(args, cfg) -> int: def cmd_claim(args, cfg) -> int: agent = _agent(args, cfg) - with store.connect(cfg) as conn: + with store.session(cfg) as conn: takeover_operation = None takeover_approval = None if args.takeover: @@ -173,11 +218,17 @@ def cmd_claim(args, cfg) -> int: "from_agent": holder["agent"], "to_agent": agent, } - takeover_approval = gate.approved_instance(conn, takeover_operation) + takeover_approval = gate.approved_instance( + conn, takeover_operation, consumer=agent + ) if takeover_approval is None: + refusal = gate.approval_refusal( + conn, takeover_operation, consumer=agent + ) print( "[quorumgit] REFUSED: lease takeover requires approval.\n" - f"operation hash: " + + (f"{refusal}\n" if refusal else "") + + "operation hash: " f"{gate.operation_hash(takeover_operation)}\n" "Request/approve it via `quorumgit approve request/vote` " "with operation:\n" @@ -219,11 +270,16 @@ def cmd_claim(args, cfg) -> int: return 1 wt = None if not args.no_worktree: - wt = trees.create_worktree(conn, claim_id, cfg.worktrees_dir) + wt = trees.continue_or_create_worktree(conn, claim_id, cfg.worktrees_dir) conn.commit() print(f"claim {claim_id} acquired ({classification}).") if wt: print(f"worktree: {wt['path']}") + if wt.get("continued_from_claim_id"): + print( + "continued retained worktree of claim " + f"{wt['continued_from_claim_id']} (uncommitted work preserved)" + ) print(f"branch: {wt['branch']}") else: print(f"branch: {args.branch} (no worktree — work from your own clone " @@ -232,7 +288,7 @@ def cmd_claim(args, cfg) -> int: def cmd_renew(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: work.renew_claim(conn, args.claim_id, _agent(args, cfg), lease_hours=args.lease_hours) conn.commit() @@ -242,7 +298,7 @@ def cmd_renew(args, cfg) -> int: def cmd_release(args, cfg) -> int: agent = _agent(args, cfg) - with store.connect(cfg) as conn: + with store.session(cfg) as conn: wt = trees.worktree_for_claim(conn, args.claim_id) if wt and wt["removed_at"] is None and args.remove_worktree: trees.remove_worktree(conn, args.claim_id, agent) @@ -254,7 +310,7 @@ def cmd_release(args, cfg) -> int: def cmd_checkpoint(args, cfg) -> int: agent = _agent(args, cfg) - with store.connect(cfg) as conn: + with store.session(cfg) as conn: commit = args.commit if not commit: wt = trees.active_worktree_for_claim(conn, args.claim_id) @@ -275,7 +331,7 @@ def cmd_checkpoint(args, cfg) -> int: def cmd_handoff_create(args, cfg) -> int: agent = _agent(args, cfg) - with store.connect(cfg) as conn: + 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 if not last_commit: @@ -302,7 +358,7 @@ def cmd_handoff_create(args, cfg) -> int: def cmd_handoff_list(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: for h in handoff.list_handoffs(conn, status=args.status): print(f"{h['id']}\ttask {h['task_id']}\t{h['from_agent']} -> " f"{h['to_agent'] or 'anyone'}\t{h['status']}") @@ -310,7 +366,7 @@ def cmd_handoff_list(args, cfg) -> int: def cmd_handoff_show(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: h = handoff.get_handoff(conn, args.handoff_id) print(json.dumps(h, indent=2, default=str)) return 0 @@ -318,7 +374,7 @@ def cmd_handoff_show(args, cfg) -> int: def cmd_handoff_accept(args, cfg) -> int: agent = _agent(args, cfg) - with store.connect(cfg) as conn: + with store.session(cfg) as conn: result = handoff.accept_handoff(conn, args.handoff_id, agent, lease_hours=args.lease_hours) conn.commit() @@ -331,7 +387,7 @@ def cmd_handoff_accept(args, cfg) -> int: def cmd_handoff_decline(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: handoff.decline_handoff(conn, args.handoff_id, _agent(args, cfg)) conn.commit() print(f"handoff {args.handoff_id} declined.") @@ -339,7 +395,7 @@ def cmd_handoff_decline(args, cfg) -> int: def cmd_handoff_cancel(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: handoff.cancel_handoff(conn, args.handoff_id, _agent(args, cfg)) conn.commit() print(f"handoff {args.handoff_id} cancelled.") @@ -357,21 +413,20 @@ def _operation_from_args(args) -> dict: def cmd_approve_request(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: approval = gate.request_approval( - conn, _operation_from_args(args), requested_by=_agent(args, cfg), - threshold=args.threshold, + conn, _operation_from_args(args), requested_by=_agent(args, cfg) ) conn.commit() print( f"approval {approval['id']} hash={approval['operation_hash']} " - f"status={approval['status']}" + f"status={approval['status']} threshold={approval['threshold']}" ) return 0 def cmd_approve_vote(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: approval = gate.vote(conn, args.approval_id, _agent(args, cfg), approve=not args.deny) conn.commit() @@ -391,7 +446,7 @@ def cmd_approve_hash(args, cfg) -> int: def cmd_hook_install(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: path = gate.install_hook(conn, args.repo) conn.commit() print(f"pre-receive hook installed: {path}") @@ -399,7 +454,7 @@ def cmd_hook_install(args, cfg) -> int: def cmd_hook_pre_receive(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: return gate.run_pre_receive(conn, args.repo, sys.stdin) @@ -407,7 +462,7 @@ def cmd_hook_pre_receive(args, cfg) -> int: def cmd_audit(args, cfg) -> int: - with store.connect(cfg) as conn: + with store.session(cfg) as conn: for event in audit.events(conn, entity=args.entity, entity_id=args.entity_id, limit=args.limit): print(f"{event['id']}\t{event['created_at']}\t{event['event_type']}" @@ -445,9 +500,31 @@ def add(name, fn, configure=None, parent=None): sp.add_argument("--protected-ref", action="append", default=[]), ), parent=repo) add("list", cmd_repo_list, parent=repo) + add("policy", cmd_repo_policy, lambda sp: ( + sp.add_argument("name"), + sp.add_argument("--agent", help="operator making the change"), + sp.add_argument("--threshold", type=int, + help="eligible yes votes required"), + sp.add_argument("--role", action="append", default=[], + choices=registry.ROLES, + help="role whose votes count (repeatable; replaces " + "the current set)"), + sp.add_argument("--requester-may-vote", dest="requester_may_vote", + action=argparse.BooleanOptionalAction, default=None), + ), parent=repo) agent = sub.add_parser("agent").add_subparsers(dest="sub", required=True) - add("add", cmd_agent_add, lambda sp: sp.add_argument("name"), parent=agent) + add("add", cmd_agent_add, lambda sp: ( + sp.add_argument("name"), + sp.add_argument("--role", choices=registry.ROLES, + default=registry.DEFAULT_ROLE), + sp.add_argument("--agent", help="operator registering a non-worker"), + ), parent=agent) + add("role", cmd_agent_role, lambda sp: ( + sp.add_argument("name"), + sp.add_argument("role", choices=registry.ROLES), + sp.add_argument("--agent", help="operator making the change"), + ), parent=agent) add("list", cmd_agent_list, parent=agent) task = sub.add_parser("task").add_subparsers(dest="sub", required=True) @@ -527,7 +604,6 @@ def add(name, fn, configure=None, parent=None): add("request", cmd_approve_request, lambda sp: ( sp.add_argument("operation", help="operation JSON object"), sp.add_argument("--agent"), - sp.add_argument("--threshold", type=int, default=1), ), parent=ap) add("vote", cmd_approve_vote, lambda sp: ( sp.add_argument("approval_id", type=int), diff --git a/src/quorumgit/gate.py b/src/quorumgit/gate.py index 948d99d..26e15d6 100644 --- a/src/quorumgit/gate.py +++ b/src/quorumgit/gate.py @@ -1,7 +1,11 @@ """Approval gate and pre-receive enforcement. Protected operations require an approval whose hash binds to the exact -operation payload. Enforcement is fail-closed: any hook error rejects the push. +operation payload. Who may authorize is owned by the operation's repository: +its approval policy names the threshold, the roles whose votes count, and +whether the requester may vote. An agent that approved an operation may never +be the one that carries it out. Enforcement is fail-closed: any hook error +rejects the push. """ from __future__ import annotations @@ -19,6 +23,7 @@ from .canonical import stable_hash from .registry import ( RegistryError, + approval_policy, assert_repository_identity_unique, get_agent, get_repository, @@ -26,7 +31,6 @@ from .store import Connection, begin_immediate, json_dumps, json_loads from .work import live_claim_for_branch, open_handoff_for_branch -DEFAULT_THRESHOLD = 1 HOOK_MARKER = "# quorumgit-managed-pre-receive v1" @@ -78,6 +82,12 @@ def _validate_takeover_operation(conn: Connection, operation: dict[str, Any]) -> ) +_APPROVAL_COLUMNS = ( + "id, operation_hash, operation, threshold, status, consumed_at, " + "repository_id, requested_by_agent_id" +) + + def _approval_dict(row) -> dict: return { "id": row[0], @@ -86,28 +96,126 @@ def _approval_dict(row) -> dict: "threshold": row[3], "status": row[4], "consumed_at": row[5], + "repository_id": row[6], + "requested_by_agent_id": row[7], } +def _operation_repository(conn: Connection, operation: dict[str, Any]) -> dict: + try: + return get_repository(conn, str(operation["repository"])) + except RegistryError as exc: + raise GateError( + f"Approval operations must name a registered repository: {exc}" + ) from exc + + +def _policy_for(conn: Connection, approval: dict) -> dict: + if approval["repository_id"] is None: + raise GateError( + f"Approval {approval['id']} predates repository approval policy " + "and names no registered repository; request it again." + ) + return approval_policy(conn, approval["repository_id"]) + + +def _vote_refusal(approval: dict, policy: dict, voter: dict) -> str | None: + """Why this agent may not vote on this approval, or None if it may.""" + if voter["role"] not in policy["roles"]: + return ( + f"{voter['name']} is a {voter['role']}; this repository accepts " + f"votes only from: {', '.join(policy['roles']) or 'nobody'}." + ) + if ( + not policy["requester_may_vote"] + and approval["requested_by_agent_id"] == voter["id"] + ): + return f"{voter['name']} requested this approval and may not vote on it." + operation = approval["operation"] + if ( + operation.get("type") == "lease_takeover" + and operation.get("to_agent") == voter["name"] + ): + return f"{voter['name']} would receive this takeover and may not vote on it." + return None + + +def _eligible_approvals( + conn: Connection, + approval: dict, + policy: dict, + exclude_agent_id: int | None = None, +) -> int: + """Count yes votes that still satisfy the repository's current policy. + + Roles and policy can change after a vote is cast, so authority is + re-derived from current state rather than trusted from the vote row. + """ + rows = conn.execute( + """ + SELECT a.id, a.name, a.role + FROM votes v JOIN agents a ON a.id = v.voter_agent_id + WHERE v.approval_id = ? AND v.vote = 1 + """, + (approval["id"],), + ).fetchall() + return sum( + 1 + for agent_id, name, role in rows + if agent_id != exclude_agent_id + and _vote_refusal(approval, policy, {"id": agent_id, "name": name, "role": role}) + is None + ) + + +def _authorization_refusal( + conn: Connection, approval: dict, consumer: dict | None +) -> str | None: + """Why an approved instance cannot authorize this consumer now, or None.""" + policy = _policy_for(conn, approval) + if consumer is not None: + approved_by_consumer = conn.execute( + "SELECT 1 FROM votes WHERE approval_id = ? AND voter_agent_id = ? " + "AND vote = 1", + (approval["id"], consumer["id"]), + ).fetchone() + if approved_by_consumer is not None: + return ( + f"{consumer['name']} approved this operation and may not also " + "carry it out." + ) + eligible = _eligible_approvals(conn, approval, policy) + if eligible < policy["threshold"]: + return ( + f"Approval {approval['id']} has {eligible} eligible approval(s); the " + f"repository policy now requires {policy['threshold']}." + ) + return None + + def request_approval( conn: Connection, operation: dict[str, Any], requested_by: str, - threshold: int = DEFAULT_THRESHOLD, ) -> dict: """Create or return the live approval instance for an exact operation. Pending/approved instances are reused. Denied/consumed instances are terminal history, so the same exact operation may be requested again as a fresh approval instance. BEGIN IMMEDIATE makes that lifecycle race-free. + + The requester does not choose the quorum: the threshold is the policy of + the repository the operation names, recorded on the instance for display + and re-evaluated against current policy whenever it is used. """ begin_immediate(conn) requester = get_agent(conn, requested_by) - _validate_takeover_operation(conn, operation) op_hash = operation_hash(operation) + repo = _operation_repository(conn, operation) + _validate_takeover_operation(conn, operation) existing = conn.execute( - """ - SELECT id, operation_hash, operation, threshold, status, consumed_at + f""" + SELECT {_APPROVAL_COLUMNS} FROM approvals WHERE operation_hash = ? AND status IN ('pending', 'approved') ORDER BY id DESC @@ -118,15 +226,17 @@ def request_approval( if existing is not None: return _approval_dict(existing) + threshold = approval_policy(conn, repo["id"])["threshold"] row = conn.execute( """ INSERT INTO approvals ( - operation_hash, operation, threshold, requested_by_agent_id + operation_hash, operation, threshold, requested_by_agent_id, + repository_id ) - VALUES (?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?) RETURNING id """, - (op_hash, json_dumps(operation), threshold, requester["id"]), + (op_hash, json_dumps(operation), threshold, requester["id"], repo["id"]), ).fetchone() assert row is not None audit.record( @@ -143,8 +253,8 @@ def request_approval( def get_approval(conn: Connection, op_hash: str) -> dict: """Return the newest approval instance for an operation hash.""" row = conn.execute( - """ - SELECT id, operation_hash, operation, threshold, status, consumed_at + f""" + SELECT {_APPROVAL_COLUMNS} FROM approvals WHERE operation_hash = ? ORDER BY id DESC @@ -159,10 +269,7 @@ def get_approval(conn: Connection, op_hash: str) -> dict: def get_approval_by_id(conn: Connection, approval_id: int) -> dict: row = conn.execute( - """ - SELECT id, operation_hash, operation, threshold, status, consumed_at - FROM approvals WHERE id = ? - """, + f"SELECT {_APPROVAL_COLUMNS} FROM approvals WHERE id = ?", (approval_id,), ).fetchone() if row is None: @@ -175,13 +282,18 @@ def vote(conn: Connection, approval_id: int, voter: str, approve: bool) -> dict: BEGIN IMMEDIATE serializes competing voters before either reads the current approval state. Denial has precedence and terminal states remain final. + Only agents the repository's policy recognizes may vote, in either + direction; the requester may not vote unless the policy allows it. """ begin_immediate(conn) voter_row = get_agent(conn, voter) approval = get_approval_by_id(conn, approval_id) if approval["status"] != "pending": raise GateError(f"Approval {approval_id} is already {approval['status']}.") - threshold = approval["threshold"] + policy = _policy_for(conn, approval) + refusal = _vote_refusal(approval, policy, voter_row) + if refusal is not None: + raise GateError(f"Vote refused: {refusal}") conn.execute( """ INSERT INTO votes (approval_id, voter, vote, voter_agent_id) @@ -201,17 +313,14 @@ def vote(conn: Connection, approval_id: int, voter: str, approve: bool) -> dict: detail={"vote": approve, "hash": approval["operation_hash"]}, ) - counts = conn.execute( - "SELECT count(*) FILTER (WHERE vote = 1), " - "count(*) FILTER (WHERE vote = 0) " - "FROM votes WHERE approval_id = ?", + denials = conn.execute( + "SELECT count(*) FROM votes WHERE approval_id = ? AND vote = 0", (approval["id"],), ).fetchone() - assert counts is not None - yes, no = counts - if no > 0: + assert denials is not None + if denials[0] > 0: new_status = "denied" - elif yes >= threshold: + elif _eligible_approvals(conn, approval, policy) >= policy["threshold"]: new_status = "approved" else: new_status = "pending" @@ -231,23 +340,51 @@ def vote(conn: Connection, approval_id: int, voter: str, approve: bool) -> dict: return get_approval_by_id(conn, approval_id) -def is_approved(conn: Connection, operation: dict[str, Any]) -> bool: - """True only if the newest instance for this exact operation is approved.""" +def approved_instance( + conn: Connection, + operation: dict[str, Any], + consumer: str | None = None, +) -> dict | None: + """The approved instance for this exact operation, if it can be used now. + + With a consumer, the instance must also be usable by that agent: an agent + that approved an operation cannot be the one that carries it out. + """ try: approval = get_approval(conn, operation_hash(operation)) except GateError: - return False - return approval["status"] == "approved" and approval["operation"] == operation + return None + if approval["status"] != "approved" or approval["operation"] != operation: + return None + try: + consumer_row = get_agent(conn, consumer) if consumer else None + if _authorization_refusal(conn, approval, consumer_row) is not None: + return None + except (GateError, RegistryError): + return None + return approval -def approved_instance(conn: Connection, operation: dict[str, Any]) -> dict | None: +def approval_refusal( + conn: Connection, operation: dict[str, Any], consumer: str | None = None +) -> str | None: + """Why an existing approved instance cannot be used, for error messages.""" try: approval = get_approval(conn, operation_hash(operation)) except GateError: return None - if approval["status"] == "approved" and approval["operation"] == operation: - return approval - return None + if approval["status"] != "approved": + return None + try: + consumer_row = get_agent(conn, consumer) if consumer else None + return _authorization_refusal(conn, approval, consumer_row) + except (GateError, RegistryError) as exc: + return str(exc) + + +def is_approved(conn: Connection, operation: dict[str, Any]) -> bool: + """True only if the newest instance for this exact operation is usable.""" + return approved_instance(conn, operation) is not None def consume_approval( @@ -267,6 +404,9 @@ def consume_approval( f"Approval {op_hash} is not consumable (status " f"{approval['status']}); it may already be used." ) + refusal = _authorization_refusal(conn, approval, consumer) + if refusal is not None: + raise GateError(f"Approval {approval_id} cannot authorize {agent}: {refusal}") cur = conn.execute( "UPDATE approvals SET status = 'consumed', consumed_at = unixepoch(), " "consumed_by_agent_id = ? " @@ -399,11 +539,13 @@ def check_ref_update( "oldrev": oldrev, "newrev": newrev, } - approval = approved_instance(conn, operation) + approval = approved_instance(conn, operation, consumer=pusher) if approval is None: + refusal = approval_refusal(conn, operation, consumer=pusher) raise PushRejected( f"{operation['type']} on {refname} requires an approval " f"bound to this exact update (hash {operation_hash(operation)})." + + (f" {refusal}" if refusal else "") ) assert pusher is not None consume_approval(conn, approval["id"], operation, agent=pusher) diff --git a/src/quorumgit/migrations/004_approval_authority.sql b/src/quorumgit/migrations/004_approval_authority.sql new file mode 100644 index 0000000..81df7a4 --- /dev/null +++ b/src/quorumgit/migrations/004_approval_authority.sql @@ -0,0 +1,116 @@ +-- Approval authority: who may authorize is decided by the repository, not by +-- the agent asking for permission. +-- +-- Agents carry one role. Every repository owns its approval policy: the +-- threshold, the roles whose votes count, and whether the requester may vote. +-- Existing agents become workers, so no historical identity silently gains +-- authority; an operator must be designated explicitly after upgrading. + +ALTER TABLE agents + ADD COLUMN role TEXT NOT NULL DEFAULT 'worker' + CHECK (role IN ('worker', 'reviewer', 'operator')); + +-- quorumgit-statement +ALTER TABLE repositories + ADD COLUMN approval_threshold INTEGER NOT NULL DEFAULT 1 + CHECK (approval_threshold >= 1); + +-- quorumgit-statement +ALTER TABLE repositories + ADD COLUMN requester_may_vote INTEGER NOT NULL DEFAULT 0 + CHECK (requester_may_vote IN (0, 1)); + +-- quorumgit-statement +CREATE TABLE repository_approval_roles ( + id INTEGER PRIMARY KEY, + repository_id INTEGER NOT NULL REFERENCES repositories(id) ON DELETE CASCADE, + role TEXT NOT NULL CHECK (role IN ('worker', 'reviewer', 'operator')), + UNIQUE (repository_id, role) +); + +-- quorumgit-statement +INSERT INTO repository_approval_roles (repository_id, role) +SELECT id, 'operator' FROM repositories; + +-- quorumgit-statement +CREATE TRIGGER repositories_default_approval_roles +AFTER INSERT ON repositories +BEGIN + INSERT INTO repository_approval_roles (repository_id, role) + VALUES (NEW.id, 'operator'); +END; + +-- quorumgit-statement +ALTER TABLE approvals + ADD COLUMN repository_id INTEGER REFERENCES repositories(id); + +-- quorumgit-statement +UPDATE approvals +SET repository_id = ( + SELECT r.id FROM repositories r + WHERE r.name = json_extract(approvals.operation, '$.repository') +) +WHERE repository_id IS NULL; + +-- quorumgit-statement +CREATE TRIGGER approvals_require_registered_repository +BEFORE INSERT ON approvals +WHEN NEW.repository_id IS NULL + OR NOT EXISTS ( + SELECT 1 FROM repositories + WHERE id = NEW.repository_id + AND name = json_extract(NEW.operation, '$.repository') + ) +BEGIN + SELECT RAISE(ABORT, 'approval must name a registered repository'); +END; + +-- quorumgit-statement +CREATE TRIGGER votes_require_eligible_voter +BEFORE INSERT ON votes +WHEN NOT EXISTS ( + SELECT 1 + FROM approvals ap + JOIN repositories r ON r.id = ap.repository_id + JOIN agents v ON v.id = NEW.voter_agent_id + JOIN repository_approval_roles rr + ON rr.repository_id = r.id AND rr.role = v.role + WHERE ap.id = NEW.approval_id + AND (r.requester_may_vote = 1 + OR ap.requested_by_agent_id IS NOT NEW.voter_agent_id) +) +BEGIN + SELECT RAISE(ABORT, 'approval voter is not eligible under repository policy'); +END; + +-- quorumgit-statement +CREATE TRIGGER vote_updates_require_eligible_voter +BEFORE UPDATE OF vote, voter, voter_agent_id ON votes +WHEN NOT EXISTS ( + SELECT 1 + FROM approvals ap + JOIN repositories r ON r.id = ap.repository_id + JOIN agents v ON v.id = NEW.voter_agent_id + JOIN repository_approval_roles rr + ON rr.repository_id = r.id AND rr.role = v.role + WHERE ap.id = NEW.approval_id + AND (r.requester_may_vote = 1 + OR ap.requested_by_agent_id IS NOT NEW.voter_agent_id) +) +BEGIN + SELECT RAISE(ABORT, 'approval voter is not eligible under repository policy'); +END; + +-- quorumgit-statement +CREATE TRIGGER approvals_consumer_is_not_approver +BEFORE UPDATE OF status ON approvals +WHEN NEW.status = 'consumed' + AND EXISTS ( + SELECT 1 FROM votes + WHERE approval_id = NEW.id + AND voter_agent_id = NEW.consumed_by_agent_id + AND vote = 1 + ) +BEGIN + SELECT RAISE(ABORT, 'an approver cannot consume its own approval'); +END; diff --git a/src/quorumgit/registry.py b/src/quorumgit/registry.py index c135695..1dd329b 100644 --- a/src/quorumgit/registry.py +++ b/src/quorumgit/registry.py @@ -8,11 +8,51 @@ from . import audit from .store import Connection, begin_immediate +ROLES = ("worker", "reviewer", "operator") +DEFAULT_ROLE = "worker" + class RegistryError(RuntimeError): pass +def _validate_role(role: str) -> str: + if role not in ROLES: + raise RegistryError( + f"Unknown role {role!r}; expected one of: {', '.join(ROLES)}." + ) + return role + + +def _operator_count(conn: Connection) -> int: + row = conn.execute( + "SELECT count(*) FROM agents WHERE role = 'operator'" + ).fetchone() + assert row is not None + return row[0] + + +def require_operator(conn: Connection, actor: str | None, action: str) -> None: + """Refuse an administrative change unless an operator performs it. + + Roles and approval policy decide who may authorize protected operations, + so changing them is itself an authority decision. Until the first operator + exists there is nobody to ask, so the store is in bootstrap mode and the + change is allowed (and audited with whatever actor was given). + """ + if _operator_count(conn) == 0: + return + if not actor: + raise RegistryError( + f"{action} requires an operator; pass --agent or set QUORUMGIT_AGENT." + ) + row = get_agent(conn, actor) + if row["role"] != "operator": + raise RegistryError( + f"{action} requires an operator; {actor} is a {row['role']}." + ) + + def _protected_refs(conn: Connection, repository_id: int) -> list[str]: return [ row[0] @@ -166,6 +206,82 @@ def get_repository(conn: Connection, name: str) -> dict: } +def approval_policy(conn: Connection, repository_id: int) -> dict: + """The repository-owned rule for who may authorize its operations.""" + row = conn.execute( + "SELECT approval_threshold, requester_may_vote FROM repositories " + "WHERE id = ?", + (repository_id,), + ).fetchone() + if row is None: + raise RegistryError(f"No such repository id: {repository_id}") + roles = [ + r[0] + for r in conn.execute( + "SELECT role FROM repository_approval_roles " + "WHERE repository_id = ? ORDER BY role", + (repository_id,), + ).fetchall() + ] + return { + "threshold": row[0], + "requester_may_vote": bool(row[1]), + "roles": roles, + } + + +def set_approval_policy( + conn: Connection, + repository: str, + *, + actor: str | None, + threshold: int | None = None, + roles: list[str] | None = None, + requester_may_vote: bool | None = None, +) -> dict: + begin_immediate(conn) + repo = get_repository(conn, repository) + require_operator(conn, actor, "Changing approval policy") + before = approval_policy(conn, repo["id"]) + if threshold is not None: + if threshold < 1: + raise RegistryError("Approval threshold must be at least 1.") + conn.execute( + "UPDATE repositories SET approval_threshold = ? WHERE id = ?", + (threshold, repo["id"]), + ) + if roles is not None: + wanted = sorted({_validate_role(role) for role in roles}) + if not wanted: + raise RegistryError("At least one approving role is required.") + conn.execute( + "DELETE FROM repository_approval_roles WHERE repository_id = ?", + (repo["id"],), + ) + for role in wanted: + conn.execute( + "INSERT INTO repository_approval_roles (repository_id, role) " + "VALUES (?, ?)", + (repo["id"], role), + ) + if requester_may_vote is not None: + conn.execute( + "UPDATE repositories SET requester_may_vote = ? WHERE id = ?", + (1 if requester_may_vote else 0, repo["id"]), + ) + after = approval_policy(conn, repo["id"]) + if after != before: + audit.record( + conn, + "repository.policy_changed", + "repository", + repo["id"], + agent=actor, + detail={"before": before, "after": after}, + ) + return after + + def list_repositories(conn: Connection) -> list[dict]: rows = conn.execute( "SELECT id, name, path FROM repositories ORDER BY name" @@ -181,25 +297,68 @@ def list_repositories(conn: Connection) -> list[dict]: ] -def add_agent(conn: Connection, name: str) -> int: +def add_agent( + conn: Connection, + name: str, + role: str = DEFAULT_ROLE, + *, + actor: str | None = None, +) -> int: + _validate_role(role) + begin_immediate(conn) + if role != DEFAULT_ROLE: + require_operator(conn, actor, f"Registering a {role}") row = conn.execute( - "INSERT INTO agents (name) VALUES (?) RETURNING id", (name,) + "INSERT INTO agents (name, role) VALUES (?, ?) RETURNING id", (name, role) ).fetchone() assert row is not None agent_id = row[0] - audit.record(conn, "agent.registered", "agent", agent_id, agent=name) + detail = {"role": role} + if actor: + detail["registered_by"] = actor + audit.record( + conn, "agent.registered", "agent", agent_id, agent=name, detail=detail + ) return agent_id +def set_agent_role( + conn: Connection, name: str, role: str, *, actor: str | None +) -> None: + _validate_role(role) + begin_immediate(conn) + target = get_agent(conn, name) + require_operator(conn, actor, "Changing an agent role") + if target["role"] == role: + return + if target["role"] == "operator" and _operator_count(conn) == 1: + # Demoting the last operator would drop the store back into bootstrap + # mode, where any agent could then promote itself. + raise RegistryError( + f"{name} is the last operator; designate another operator first." + ) + conn.execute("UPDATE agents SET role = ? WHERE id = ?", (role, target["id"])) + audit.record( + conn, + "agent.role_changed", + "agent", + target["id"], + agent=actor, + detail={"agent": name, "from": target["role"], "to": role}, + ) + + def get_agent(conn: Connection, name: str) -> dict: row = conn.execute( - "SELECT id, name FROM agents WHERE name = ?", (name,) + "SELECT id, name, role FROM agents WHERE name = ?", (name,) ).fetchone() if row is None: raise RegistryError(f"Agent is not registered: {name}") - return {"id": row[0], "name": row[1]} + return {"id": row[0], "name": row[1], "role": row[2]} def list_agents(conn: Connection) -> list[dict]: - rows = conn.execute("SELECT id, name FROM agents ORDER BY name").fetchall() - return [{"id": r[0], "name": r[1]} for r in rows] + rows = conn.execute( + "SELECT id, name, role FROM agents ORDER BY name" + ).fetchall() + return [{"id": r[0], "name": r[1], "role": r[2]} for r in rows] diff --git a/src/quorumgit/store.py b/src/quorumgit/store.py index 035c3bd..3625c2e 100644 --- a/src/quorumgit/store.py +++ b/src/quorumgit/store.py @@ -10,6 +10,8 @@ import json import sqlite3 +from collections.abc import Iterator +from contextlib import contextmanager from importlib import resources from pathlib import Path from typing import Any @@ -39,6 +41,23 @@ "votes", "conflict_events", "audit_events", + "repository_approval_roles", +) + +# Engine-level governance rules. A store missing any of these would still +# have every table, so the contract checks them by name as well. +REQUIRED_TRIGGERS = ( + "audit_events_no_update", + "audit_events_no_delete", + "approvals_require_registered_requester", + "approvals_require_registered_consumer", + "votes_require_registered_voter", + "vote_updates_require_registered_voter", + "repositories_default_approval_roles", + "approvals_require_registered_repository", + "votes_require_eligible_voter", + "vote_updates_require_eligible_voter", + "approvals_consumer_is_not_approver", ) @@ -137,6 +156,22 @@ def connect(cfg: Config) -> Connection: return conn +@contextmanager +def session(cfg: Config) -> Iterator[Connection]: + """One command's connection: commit on success, roll back on error, close. + + sqlite3's own context manager ends the transaction but leaves the + connection open until garbage collection; closing it here releases the + database file as soon as the command is done with it. + """ + conn = connect(cfg) + try: + with conn: + yield conn + finally: + conn.close() + + def _is_locked_error(exc: sqlite3.OperationalError) -> bool: return "locked" in str(exc).lower() or "busy" in str(exc).lower() @@ -319,10 +354,9 @@ def verify_contract(target: Config | Connection | str | Path) -> None: "SELECT name FROM sqlite_master WHERE type = 'table'" ).fetchall() } - missing = set(REQUIRED_TABLES) - tables - if missing: + if "schema_migrations" not in tables: raise ContractViolation( - f"Missing required tables: {sorted(missing)}. " + "Missing required tables: ['schema_migrations']. " "Run `quorumgit init` to apply migrations." ) @@ -340,6 +374,25 @@ def verify_contract(target: Config | Connection | str | Path) -> None: "Run `quorumgit init` to apply migrations." ) + missing = set(REQUIRED_TABLES) - tables + if missing: + raise ContractViolation( + f"Missing required tables: {sorted(missing)}. " + "Run `quorumgit init` to apply migrations." + ) + + triggers = { + row[0] + for row in conn.execute( + "SELECT name FROM sqlite_master WHERE type = 'trigger'" + ).fetchall() + } + missing_triggers = set(REQUIRED_TRIGGERS) - triggers + if missing_triggers: + raise ContractViolation( + f"Missing required governance triggers: {sorted(missing_triggers)}." + ) + foreign_keys = conn.execute("PRAGMA foreign_keys").fetchone()[0] journal_mode = str(conn.execute("PRAGMA journal_mode").fetchone()[0]).lower() if foreign_keys != 1: diff --git a/src/quorumgit/trees.py b/src/quorumgit/trees.py index 7bdc1bb..0cb78ad 100644 --- a/src/quorumgit/trees.py +++ b/src/quorumgit/trees.py @@ -8,6 +8,7 @@ from __future__ import annotations +import secrets import subprocess from pathlib import Path @@ -43,6 +44,25 @@ def _git_common_dir(repo_path: str | Path) -> Path: return common.resolve() +def managed_worktree_path( + worktrees_dir: Path, repository_id: int, claim_id: int +) -> Path: + """Filesystem location for a new managed worktree. + + Built only from database-generated integer IDs and a random suffix: + repository and agent names are display identities and never become path + components, so no registered name can steer a checkout outside the + directory QuorumGit owns. The suffix keeps the path fresh even when SQLite + reuses the rowid of a rolled-back claim whose checkout was left behind. + """ + root = Path(worktrees_dir).resolve() + leaf = f"claim-{int(claim_id)}-{secrets.token_hex(4)}" + path = (root / f"repo-{int(repository_id)}" / leaf).resolve() + if not path.is_relative_to(root): + raise WorktreeError(f"Managed worktree path escapes {root}: {path}") + return path + + def create_worktree( conn: Connection, claim_id: int, worktrees_dir: Path, base_ref: str = "HEAD" ) -> dict: @@ -53,14 +73,7 @@ def create_worktree( repo_path = task["repository_path"] branch = claim["branch"] - # Claim identity is part of the managed path. Re-claiming the same task by - # the same agent therefore creates a fresh path without colliding with the - # historical UNIQUE(worktrees.path) row from an earlier claim. - wt_path = ( - worktrees_dir - / task["repository"] - / f"task-{task['id']}-{claim['agent']}-claim-{claim_id}" - ) + wt_path = managed_worktree_path(worktrees_dir, task["repository_id"], claim_id) wt_path.parent.mkdir(parents=True, exist_ok=True) if wt_path.exists(): raise WorktreeError(f"Worktree path already exists: {wt_path}") @@ -131,6 +144,112 @@ def active_worktree_for_claim(conn: Connection, claim_id: int) -> dict | None: return wt +def _retained_worktree_for_branch( + conn: Connection, repository_id: int, branch: str, exclude_claim_id: int +) -> dict | None: + """A live recorded worktree with this branch checked out, if any.""" + row = conn.execute( + """ + SELECT w.id, w.path, w.branch, c.id, c.task_id, c.released_at, + c.release_reason, + EXISTS ( + SELECT 1 FROM handoffs h + WHERE h.from_claim_id = c.id AND h.status = 'open' + ) + FROM worktrees w + JOIN claims c ON c.id = w.claim_id + JOIN tasks t ON t.id = c.task_id + WHERE t.repository_id = ? AND w.branch = ? AND w.removed_at IS NULL + AND c.id <> ? + ORDER BY w.id DESC + LIMIT 1 + """, + (repository_id, branch, exclude_claim_id), + ).fetchone() + if row is None: + return None + return { + "id": row[0], + "path": row[1], + "branch": row[2], + "claim_id": row[3], + "task_id": row[4], + "released_at": row[5], + "release_reason": row[6], + "open_handoff": bool(row[7]), + } + + +def continue_or_create_worktree( + conn: Connection, claim_id: int, worktrees_dir: Path, base_ref: str = "HEAD" +) -> dict: + """Give a new claim its worktree, continuing a retained one when it exists. + + A claim that supersedes an earlier claim on the same task and branch — + after lease expiry, an approved takeover, or a release that kept its + checkout — cannot get a second checkout: Git refuses to check one branch + out twice. Instead the retained checkout, with any unfinished work in it, + is verified and transferred to the new claim, and the transfer is audited. + A retained checkout belonging to a different task is never adopted. + """ + claim = get_claim(conn, claim_id) + if claim["released_at"] is not None: + raise WorktreeError(f"Claim {claim_id} is released.") + task = get_task(conn, claim["task_id"]) + retained = _retained_worktree_for_branch( + conn, task["repository_id"], claim["branch"], exclude_claim_id=claim_id + ) + if retained is None or not Path(retained["path"]).exists(): + # A recorded checkout whose directory is gone holds no work to + # continue; doctor reports and reconciles that record separately. + return create_worktree(conn, claim_id, worktrees_dir, base_ref) + + where = ( + f"Branch {claim['branch']!r} is still checked out in retained worktree " + f"{retained['path']} (claim {retained['claim_id']}, task " + f"{retained['task_id']})" + ) + if retained["task_id"] != task["id"]: + raise WorktreeError( + f"{where}. It belongs to another task and will not be adopted; " + "its owner should release it with --remove-worktree, or claim with " + "--no-worktree." + ) + if retained["released_at"] is None or retained["open_handoff"]: + raise WorktreeError(f"{where}, which is still reserved; refusing to adopt it.") + issue, error = checkout_identity_issue( + task["repository_path"], retained["path"], retained["branch"] + ) + if issue is not None: + raise WorktreeError( + f"{where}, but its checkout no longer matches the record ({issue}" + + (f": {error}" if error else "") + + "). Inspect it manually; refusing to adopt it." + ) + + transfer_worktree(conn, retained["id"], claim_id) + audit.record( + conn, + "worktree.continued", + "worktree", + retained["id"], + agent=claim["agent"], + detail={ + "path": retained["path"], + "branch": retained["branch"], + "from_claim_id": retained["claim_id"], + "to_claim_id": claim_id, + "from_release_reason": retained["release_reason"], + }, + ) + return { + "id": retained["id"], + "path": retained["path"], + "branch": retained["branch"], + "continued_from_claim_id": retained["claim_id"], + } + + def transfer_worktree(conn: Connection, worktree_id: int, new_claim_id: int) -> None: """Reassign a worktree to a new claim (handoff continuation).""" conn.execute( @@ -213,6 +332,30 @@ def cleanup_released_worktree( return True +def checkout_identity_issue( + repo_path: str | Path, wt_path: str | Path, branch: str +) -> tuple[str | None, str | None]: + """Compare an existing checkout with its recorded repository and branch. + + Returns (issue, error): issue is None when the checkout is the recorded + repository's worktree with the recorded branch checked out. + """ + try: + expected_common = _git_common_dir(repo_path) + actual_common = _git_common_dir(wt_path) + actual_root = Path(_git(wt_path, "rev-parse", "--show-toplevel")).resolve() + actual_branch = _git(wt_path, "rev-parse", "--symbolic-full-name", "HEAD") + except WorktreeError as exc: + return "unverifiable_checkout", str(exc) + if actual_common != expected_common or actual_root != Path(wt_path).resolve(): + return "repository_mismatch", None + if actual_branch == "HEAD": + return "detached_head", None + if actual_branch != f"refs/heads/{branch}": + return "branch_mismatch", None + return None, None + + def doctor_worktrees(conn: Connection, repair: bool = False) -> list[dict]: """Report and conservatively repair drift in recorded managed worktrees. @@ -253,27 +396,9 @@ def doctor_worktrees(conn: Connection, repair: bool = False) -> list[dict]: issue: str | None = None identity_error = None if exists: - try: - expected_common = _git_common_dir(repo_path) - actual_common = _git_common_dir(wt["path"]) - actual_root = Path( - _git(wt["path"], "rev-parse", "--show-toplevel") - ).resolve() - actual_branch = _git( - wt["path"], "rev-parse", "--symbolic-full-name", "HEAD" - ) - if ( - actual_common != expected_common - or actual_root != Path(wt["path"]).resolve() - ): - issue = "repository_mismatch" - elif actual_branch == "HEAD": - issue = "detached_head" - elif actual_branch != f"refs/heads/{wt['branch']}": - issue = "branch_mismatch" - except WorktreeError as exc: - issue = "unverifiable_checkout" - identity_error = str(exc) + issue, identity_error = checkout_identity_issue( + repo_path, wt["path"], wt["branch"] + ) if issue is not None: pass elif wt["removed_at"] is None and not exists: diff --git a/tests/conftest.py b/tests/conftest.py index 8d72a63..f4bdf7d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,14 +7,19 @@ import os import subprocess +import uuid from collections.abc import Iterator from pathlib import Path import pytest -from quorumgit import store +from quorumgit import gate, registry, store from quorumgit.config import Config +# The shared store's designated approver. Workers request approvals; this +# operator votes on them, so no test relies on self-authorization. +OPERATOR = "operator" + @pytest.fixture(scope="session") def cfg(tmp_path_factory) -> Iterator[Config]: @@ -90,3 +95,36 @@ def git(*args): @pytest.fixture() def git_repo(tmp_path) -> Path: return make_git_repo(tmp_path / "repo") + + +def ensure_agent(conn, name: str, role: str | None = None) -> None: + """Register an agent if missing; with a role, also (re)assign it.""" + conn.execute( + "INSERT INTO agents (name) VALUES (?) ON CONFLICT DO NOTHING", (name,) + ) + if role is not None: + conn.execute("UPDATE agents SET role = ? WHERE name = ?", (role, name)) + + +def register_repo(conn, path: Path, prefix: str = "repo") -> str: + """Register a fresh real Git repository under a unique name.""" + name = f"{prefix}-{uuid.uuid4().hex[:8]}" + registry.add_repository(conn, name, make_git_repo(path)) + return name + + +def approve(conn, operation: dict, requested_by: str, voters=(OPERATOR,)) -> dict: + """Request an exact operation as one agent and approve it as operators.""" + ensure_agent(conn, requested_by) + for voter in voters: + ensure_agent(conn, voter, "operator") + approval = gate.request_approval(conn, operation, requested_by=requested_by) + for voter in voters: + approval = gate.vote(conn, approval["id"], voter, True) + return approval + + +@pytest.fixture() +def approval_repo(conn, tmp_path) -> str: + """A registered repository name for approval operations.""" + return register_repo(conn, tmp_path / "approval-repo", prefix="approval") diff --git a/tests/test_acceptance.py b/tests/test_acceptance.py index 49c95ca..7642460 100644 --- a/tests/test_acceptance.py +++ b/tests/test_acceptance.py @@ -27,7 +27,7 @@ import pytest from quorumgit import gate, handoff, registry, store, trees, work -from tests.conftest import make_git_repo +from tests.conftest import OPERATOR, ensure_agent, make_git_repo def test_full_workflow(committed_conn, cfg, tmp_path, initialized_store): @@ -41,7 +41,7 @@ def test_full_workflow(committed_conn, cfg, tmp_path, initialized_store): agent_a, agent_b = f"a-{suffix}", f"b-{suffix}" registry.add_agent(conn, agent_a) registry.add_agent(conn, agent_b) - conn.execute("INSERT INTO agents (name) VALUES ('operator') ON CONFLICT DO NOTHING") + ensure_agent(conn, OPERATOR, "operator") task_a = work.create_task( conn, repo_name, "implement feature", objective="build the feature in src/" @@ -152,8 +152,10 @@ def test_full_workflow(committed_conn, cfg, tmp_path, initialized_store): scope_globs=["src/**"], ) takeover_op["from_claim_id"] = holder["id"] - approval = gate.request_approval(conn, takeover_op, requested_by="operator") - gate.vote(conn, approval["id"], "operator", True) + approval = gate.request_approval(conn, takeover_op, requested_by=agent_a) + with pytest.raises(gate.GateError, match="Vote refused"): + gate.vote(conn, approval["id"], agent_a, True) + gate.vote(conn, approval["id"], OPERATOR, True) assert gate.is_approved(conn, takeover_op) gate.consume_approval(conn, approval["id"], takeover_op, agent=agent_a) claim_back, _, _ = work.claim_task( diff --git a/tests/test_approval_authority.py b/tests/test_approval_authority.py new file mode 100644 index 0000000..063b4ce --- /dev/null +++ b/tests/test_approval_authority.py @@ -0,0 +1,389 @@ +"""Approval authority: who may authorize is owned by the repository. + +A worker cannot authorize its own operation: the requester does not choose the +quorum, only roles the repository names may vote, the requester may not vote +unless the policy allows it, and an agent that approved an operation can never +be the one that carries it out. Authority is re-derived at use time, so a +tightened policy or a demoted approver invalidates an earlier approval. +""" + +from __future__ import annotations + +import sqlite3 +import subprocess +import uuid +from pathlib import Path + +import pytest + +from quorumgit import gate, registry, store, work +from quorumgit.config import Config +from quorumgit.registry import RegistryError +from tests.conftest import ( + OPERATOR, + approve, + ensure_agent, + make_git_repo, + register_repo, +) +from tests.test_authorization_binding import _apply_001 +from tests.test_cli_hub import _cli +from tests.test_gate import _commit, _push, _setup + + +def _op(repository: str, **extra) -> dict: + return {"type": "protected_ref_update", "repository": repository, + "nonce": uuid.uuid4().hex, **extra} + + +# ------------------------------------------------------------- voting rules + + +def test_worker_cannot_approve_its_own_request(conn, approval_repo): + ensure_agent(conn, "self-worker") + approval = gate.request_approval(conn, _op(approval_repo), "self-worker") + + with pytest.raises(gate.GateError, match="is a worker"): + gate.vote(conn, approval["id"], "self-worker", True) + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "pending" + + +def test_operator_requester_cannot_vote_by_default(conn, approval_repo): + ensure_agent(conn, "requesting-op", "operator") + approval = gate.request_approval(conn, _op(approval_repo), "requesting-op") + + with pytest.raises(gate.GateError, match="requested this approval"): + gate.vote(conn, approval["id"], "requesting-op", True) + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "pending" + + +def test_policy_can_allow_requester_to_vote(conn, approval_repo): + ensure_agent(conn, OPERATOR, "operator") + ensure_agent(conn, "requesting-op", "operator") + registry.set_approval_policy( + conn, approval_repo, actor=OPERATOR, requester_may_vote=True + ) + approval = gate.request_approval(conn, _op(approval_repo), "requesting-op") + result = gate.vote(conn, approval["id"], "requesting-op", True) + assert result["status"] == "approved" + + +def test_non_approving_role_cannot_deny_either(conn, approval_repo): + ensure_agent(conn, "req") + ensure_agent(conn, "a-reviewer", "reviewer") + approval = gate.request_approval(conn, _op(approval_repo), "req") + with pytest.raises(gate.GateError, match="accepts votes only from: operator"): + gate.vote(conn, approval["id"], "a-reviewer", False) + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "pending" + + +def test_policy_roles_decide_who_votes(conn, approval_repo): + ensure_agent(conn, OPERATOR, "operator") + ensure_agent(conn, "req") + ensure_agent(conn, "a-reviewer", "reviewer") + registry.set_approval_policy( + conn, approval_repo, actor=OPERATOR, roles=["reviewer"] + ) + approval = gate.request_approval(conn, _op(approval_repo), "req") + with pytest.raises(gate.GateError, match="accepts votes only from: reviewer"): + gate.vote(conn, approval["id"], OPERATOR, True) + assert gate.vote(conn, approval["id"], "a-reviewer", True)["status"] == "approved" + + +def test_engine_refuses_ineligible_vote_rows(conn, approval_repo): + """The vote rule holds even for a writer that bypasses the gate module.""" + ensure_agent(conn, "raw-worker") + approval = gate.request_approval(conn, _op(approval_repo), "raw-worker") + worker_id = registry.get_agent(conn, "raw-worker")["id"] + with pytest.raises(sqlite3.IntegrityError, match="not eligible"): + conn.execute( + "INSERT INTO votes (approval_id, voter, vote, voter_agent_id) " + "VALUES (?, 'raw-worker', 1, ?)", + (approval["id"], worker_id), + ) + + +def test_approval_must_name_a_registered_repository(conn): + ensure_agent(conn, "req") + with pytest.raises(gate.GateError, match="registered repository"): + gate.request_approval(conn, _op("no-such-repo"), "req") + + +# -------------------------------------------------------------- threshold + + +def test_threshold_comes_from_repository_policy(conn, approval_repo): + ensure_agent(conn, OPERATOR, "operator") + ensure_agent(conn, "second-op", "operator") + ensure_agent(conn, "req") + registry.set_approval_policy(conn, approval_repo, actor=OPERATOR, threshold=2) + approval = gate.request_approval(conn, _op(approval_repo), "req") + assert approval["threshold"] == 2 + assert gate.vote(conn, approval["id"], OPERATOR, True)["status"] == "pending" + assert gate.vote(conn, approval["id"], "second-op", True)["status"] == "approved" + + +def test_cli_requester_cannot_choose_threshold(committed_conn, cfg, tmp_path): + repo = register_repo(committed_conn, tmp_path / "cli-threshold") + ensure_agent(committed_conn, "cli-req") + committed_conn.commit() + result = _cli( + cfg, "approve", "request", f'{{"type":"x","repository":"{repo}"}}', + "--threshold", "1", agent="cli-req", + ) + assert result.returncode == 2 + assert "--threshold" in result.stderr + + +def test_tightened_policy_invalidates_existing_approval(conn, approval_repo): + op = _op(approval_repo) + approval = approve(conn, op, requested_by="req") + ensure_agent(conn, "pusher") + assert gate.is_approved(conn, op) + + registry.set_approval_policy(conn, approval_repo, actor=OPERATOR, threshold=2) + assert not gate.is_approved(conn, op) + with pytest.raises(gate.GateError, match="now requires 2"): + gate.consume_approval(conn, approval["id"], op, agent="pusher") + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + + +def test_demoted_approver_no_longer_counts(conn, approval_repo): + ensure_agent(conn, OPERATOR, "operator") + ensure_agent(conn, "soon-demoted", "operator") + ensure_agent(conn, "pusher") + op = _op(approval_repo) + approval = approve(conn, op, requested_by="req", voters=("soon-demoted",)) + registry.set_agent_role(conn, "soon-demoted", "worker", actor=OPERATOR) + with pytest.raises(gate.GateError, match="0 eligible"): + gate.consume_approval(conn, approval["id"], op, agent="pusher") + + +# ------------------------------------------------------------ consumption + + +def test_approver_cannot_consume_its_own_approval(conn, approval_repo): + ensure_agent(conn, "req") + op = _op(approval_repo) + approval = approve(conn, op, requested_by="req") + assert gate.approved_instance(conn, op, consumer=OPERATOR) is None + with pytest.raises(gate.GateError, match="may not also carry it out"): + gate.consume_approval(conn, approval["id"], op, agent=OPERATOR) + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + + +def test_engine_refuses_consumption_by_approver(conn, approval_repo): + op = _op(approval_repo) + approval = approve(conn, op, requested_by="req") + operator_id = registry.get_agent(conn, OPERATOR)["id"] + with pytest.raises(sqlite3.IntegrityError, match="cannot consume its own approval"): + conn.execute( + "UPDATE approvals SET status = 'consumed', consumed_at = unixepoch(), " + "consumed_by_agent_id = ? WHERE id = ?", + (operator_id, approval["id"]), + ) + + +def test_takeover_beneficiary_cannot_vote(conn, git_repo): + repo = f"beneficiary-{uuid.uuid4().hex[:8]}" + registry.add_repository(conn, repo, git_repo) + ensure_agent(conn, "holder") + ensure_agent(conn, "ambitious-op", "operator") + task = work.create_task(conn, repo, "held") + claim, _, _ = work.claim_task(conn, task, "holder", "feat/held", ["src/**"]) + operation = { + "type": "lease_takeover", + "repository": repo, + "task_id": task, + "from_claim_id": claim, + "from_agent": "holder", + "to_agent": "ambitious-op", + } + ensure_agent(conn, "req") + approval = gate.request_approval(conn, operation, "req") + with pytest.raises(gate.GateError, match="would receive this takeover"): + gate.vote(conn, approval["id"], "ambitious-op", True) + + +def test_pusher_that_approved_is_rejected_by_hook(committed_conn, tmp_path, cfg): + repo_name, hub, clone, a, _b = _setup(committed_conn, tmp_path) + approver = f"approver-{uuid.uuid4().hex[:8]}" + ensure_agent(committed_conn, approver, "operator") + _commit(clone, "approved-by-pusher.txt") + op = { + "type": "protected_ref_update", + "repository": repo_name, + "refname": "refs/heads/main", + "oldrev": subprocess.run( + ["git", "--git-dir", str(hub), "rev-parse", "refs/heads/main"], + capture_output=True, text=True, check=True, + ).stdout.strip(), + "newrev": subprocess.run( + ["git", "-C", str(clone), "rev-parse", "HEAD"], + capture_output=True, text=True, check=True, + ).stdout.strip(), + } + approval = approve(committed_conn, op, requested_by=a, voters=(approver,)) + committed_conn.commit() + + refused = _push(clone, approver, "main", cfg=cfg) + assert refused.returncode != 0 + assert "may not also carry it out" in refused.stderr + assert gate.get_approval_by_id(committed_conn, approval["id"])["status"] == ( + "approved" + ) + + accepted = _push(clone, a, "main", cfg=cfg) + assert accepted.returncode == 0, accepted.stderr + + +# ------------------------------------------------- operator administration + + +@pytest.fixture() +def fresh_store(tmp_path): + local = Config(data_dir=tmp_path / "authority-data", agent=None) + store.migrate(local) + conn = store.connect(local) + try: + yield local, conn + finally: + conn.close() + store.destroy(local) + + +def test_first_operator_is_bootstrapped_then_roles_are_operator_only( + fresh_store, tmp_path +): + _local, conn = fresh_store + registry.add_agent(conn, "alice") + registry.add_agent(conn, "mallory") + repo = register_repo(conn, tmp_path / "bootstrap") + + # No operator exists yet, so the first designation needs no authority. + registry.set_agent_role(conn, "alice", "operator", actor=None) + conn.commit() + + with pytest.raises(RegistryError, match="requires an operator"): + registry.set_agent_role(conn, "mallory", "operator", actor="mallory") + conn.rollback() + with pytest.raises(RegistryError, match="requires an operator"): + registry.add_agent(conn, "eve", role="operator", actor=None) + conn.rollback() + with pytest.raises(RegistryError, match="requires an operator"): + registry.set_approval_policy(conn, repo, actor="mallory", threshold=1) + conn.rollback() + + registry.set_agent_role(conn, "mallory", "reviewer", actor="alice") + registry.set_approval_policy(conn, repo, actor="alice", roles=["operator", "reviewer"]) + conn.commit() + assert registry.get_agent(conn, "mallory")["role"] == "reviewer" + policy = registry.approval_policy(conn, registry.get_repository(conn, repo)["id"]) + assert policy["roles"] == ["operator", "reviewer"] + + events = { + row[0] + for row in conn.execute("SELECT event_type FROM audit_events").fetchall() + } + assert {"agent.role_changed", "repository.policy_changed"} <= events + + +def test_last_operator_cannot_be_demoted(fresh_store): + _local, conn = fresh_store + registry.add_agent(conn, "solo") + registry.set_agent_role(conn, "solo", "operator", actor=None) + with pytest.raises(RegistryError, match="last operator"): + registry.set_agent_role(conn, "solo", "worker", actor="solo") + + +def test_cli_agent_roles_and_policy(fresh_store, tmp_path): + local, conn = fresh_store + repo = register_repo(conn, tmp_path / "cli-policy") + conn.commit() + + assert _cli(local, "agent", "add", "boss", "--role", "operator").returncode == 0 + refused = _cli(local, "agent", "add", "sneaky", "--role", "operator", agent="nobody") + assert refused.returncode == 1 + assert _cli(local, "agent", "add", "worker-1").returncode == 0 + promoted = _cli(local, "agent", "role", "worker-1", "reviewer", agent="boss") + assert promoted.returncode == 0, promoted.stderr + + listed = _cli(local, "agent", "list") + assert "boss\toperator" in listed.stdout + assert "worker-1\treviewer" in listed.stdout + + changed = _cli( + local, "repo", "policy", repo, "--threshold", "2", + "--role", "operator", "--role", "reviewer", "--requester-may-vote", + agent="boss", + ) + assert changed.returncode == 0, changed.stderr + shown = _cli(local, "repo", "policy", repo) + assert "approval threshold: 2" in shown.stdout + assert "approving roles: operator, reviewer" in shown.stdout + assert "requester may vote: yes" in shown.stdout + + +# ---------------------------------------------------------------- upgrade + + +def test_upgrade_leaves_no_agent_with_authority(tmp_path): + """Pre-policy stores upgrade with every agent a worker, fail closed.""" + local = Config(data_dir=tmp_path / "upgrade", agent=None) + conn = _apply_001(local) + migration = Path(store.__file__).parent / "migrations" / "002_approval_identities.sql" + for statement in store._migration_statements(migration.read_text(encoding="utf-8")): + conn.execute(statement) + conn.execute( + "INSERT INTO schema_migrations (version) VALUES ('002_approval_identities.sql')" + ) + repo_path = make_git_repo(tmp_path / "legacy-repo") + conn.execute( + "INSERT INTO repositories (name, path) VALUES ('legacy', ?)", (str(repo_path),) + ) + agent_id = conn.execute( + "INSERT INTO agents (name) VALUES ('self-approver') RETURNING id" + ).fetchone()[0] + operation = {"type": "protected_ref_update", "repository": "legacy"} + approval_id = conn.execute( + "INSERT INTO approvals (operation_hash, operation, status, " + "requested_by_agent_id, decided_at) " + "VALUES (?, ?, 'approved', ?, unixepoch()) RETURNING id", + (gate.operation_hash(operation), store.json_dumps(operation), agent_id), + ).fetchone()[0] + conn.execute( + "INSERT INTO votes (approval_id, voter, vote, voter_agent_id) " + "VALUES (?, 'self-approver', 1, ?)", + (approval_id, agent_id), + ) + conn.commit() + conn.close() + + assert store.migrate(local) == ["004_approval_authority.sql"] + upgraded = store.connect(local) + try: + assert registry.get_agent(upgraded, "self-approver")["role"] == "worker" + repo = registry.get_repository(upgraded, "legacy") + assert registry.approval_policy(upgraded, repo["id"]) == { + "threshold": 1, + "requester_may_vote": False, + "roles": ["operator"], + } + backfilled = gate.get_approval_by_id(upgraded, approval_id) + assert backfilled["repository_id"] == repo["id"] + assert not gate.is_approved(upgraded, operation) + upgraded.execute("INSERT INTO agents (name) VALUES ('other')") + with pytest.raises(gate.GateError, match="0 eligible"): + gate.consume_approval(upgraded, approval_id, operation, agent="other") + finally: + upgraded.rollback() + upgraded.close() + store.destroy(local) + + +def test_contract_requires_governance_triggers(fresh_store): + local, conn = fresh_store + conn.execute("DROP TRIGGER votes_require_eligible_voter") + conn.commit() + with pytest.raises(store.ContractViolation, match="votes_require_eligible_voter"): + store.verify_contract(local) diff --git a/tests/test_authorization_binding.py b/tests/test_authorization_binding.py index 1c38650..a19faaa 100644 --- a/tests/test_authorization_binding.py +++ b/tests/test_authorization_binding.py @@ -13,7 +13,7 @@ from quorumgit import gate, registry, store, work from quorumgit.config import Config from quorumgit.registry import RegistryError -from tests.conftest import make_git_repo +from tests.conftest import approve, ensure_agent, make_git_repo, register_repo from tests.test_cli_hub import _cli from tests.test_gate import _commit, _push, _setup @@ -36,6 +36,12 @@ def _apply_001(cfg: Config): return conn +UPGRADE_MIGRATIONS = [ + "002_approval_identities.sql", + "004_approval_authority.sql", +] + + def _register_001_agent(conn, name: str) -> int: row = conn.execute( "INSERT INTO agents (name) VALUES (?) RETURNING id", (name,) @@ -78,17 +84,17 @@ def _insert_001_vote(conn, approval_id: int, voter: str) -> None: ) -def test_approval_requester_must_be_registered(conn): - operation = {"type": "test", "repository": "r"} +def test_approval_requester_must_be_registered(conn, approval_repo): + operation = {"type": "test", "repository": approval_repo} before = conn.execute("SELECT count(*) FROM approvals").fetchone()[0] with pytest.raises(RegistryError, match="not registered"): gate.request_approval(conn, operation, requested_by="ghost") assert conn.execute("SELECT count(*) FROM approvals").fetchone()[0] == before -def test_approval_voter_must_be_registered(conn): +def test_approval_voter_must_be_registered(conn, approval_repo): _agents(conn, "requester") - operation = {"type": "test", "repository": "r"} + operation = {"type": "test", "repository": approval_repo} approval = gate.request_approval(conn, operation, requested_by="requester") before = conn.execute("SELECT count(*) FROM votes").fetchone()[0] @@ -98,9 +104,10 @@ def test_approval_voter_must_be_registered(conn): assert conn.execute("SELECT count(*) FROM votes").fetchone()[0] == before -def test_approval_consumer_must_be_registered(conn): - _agents(conn, "requester", "voter") - operation = {"type": "test", "repository": "r"} +def test_approval_consumer_must_be_registered(conn, approval_repo): + _agents(conn, "requester") + ensure_agent(conn, "voter", "operator") + operation = {"type": "test", "repository": approval_repo} approval = gate.request_approval(conn, operation, requested_by="requester") gate.vote(conn, approval["id"], "voter", True) @@ -109,10 +116,13 @@ def test_approval_consumer_must_be_registered(conn): assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" -def test_approval_consumer_must_present_matching_instance_and_operation(conn): - _agents(conn, "requester", "voter", "consumer") - first_operation = {"type": "first", "repository": "r"} - second_operation = {"type": "second", "repository": "r"} +def test_approval_consumer_must_present_matching_instance_and_operation( + conn, approval_repo +): + _agents(conn, "requester", "consumer") + ensure_agent(conn, "voter", "operator") + first_operation = {"type": "first", "repository": approval_repo} + second_operation = {"type": "second", "repository": approval_repo} first = gate.request_approval(conn, first_operation, requested_by="requester") second = gate.request_approval(conn, second_operation, requested_by="requester") gate.vote(conn, first["id"], "voter", True) @@ -124,9 +134,11 @@ def test_approval_consumer_must_present_matching_instance_and_operation(conn): assert gate.get_approval_by_id(conn, second["id"])["status"] == "approved" -def test_delayed_vote_cannot_target_new_approval_instance(conn): - _agents(conn, "requester", "first-voter", "late-voter") - operation = {"type": "repeat", "repository": "r"} +def test_delayed_vote_cannot_target_new_approval_instance(conn, approval_repo): + _agents(conn, "requester") + ensure_agent(conn, "first-voter", "operator") + ensure_agent(conn, "late-voter", "operator") + operation = {"type": "repeat", "repository": approval_repo} first = gate.request_approval(conn, operation, requested_by="requester") gate.vote(conn, first["id"], "first-voter", False) second = gate.request_approval(conn, operation, requested_by="requester") @@ -136,10 +148,12 @@ def test_delayed_vote_cannot_target_new_approval_instance(conn): assert gate.get_approval_by_id(conn, second["id"])["status"] == "pending" -def test_cli_votes_by_approval_instance_id(committed_conn, cfg): - _agents(committed_conn, "cli-requester", "cli-voter") +def test_cli_votes_by_approval_instance_id(committed_conn, cfg, tmp_path): + _agents(committed_conn, "cli-requester") + ensure_agent(committed_conn, "cli-voter", "operator") + repository = register_repo(committed_conn, tmp_path / "cli", prefix="cli") committed_conn.commit() - operation = {"type": "cli", "repository": "r", "nonce": uuid.uuid4().hex} + operation = {"type": "cli", "repository": repository, "nonce": uuid.uuid4().hex} requested = _cli( cfg, "approve", @@ -150,6 +164,7 @@ def test_cli_votes_by_approval_instance_id(committed_conn, cfg): assert requested.returncode == 0, requested.stderr match = re.search(r"approval (\d+) hash=", requested.stdout) assert match is not None + assert "threshold=1" in requested.stdout voted = _cli( cfg, @@ -162,16 +177,18 @@ def test_cli_votes_by_approval_instance_id(committed_conn, cfg): assert "status=approved" in voted.stdout -def test_new_approval_rows_reference_registered_agents(conn): - _agents(conn, "requester", "voter", "consumer") - operation = {"type": "identity-columns", "repository": "r"} +def test_new_approval_rows_reference_registered_agents(conn, approval_repo): + _agents(conn, "requester", "consumer") + ensure_agent(conn, "voter", "operator") + operation = {"type": "identity-columns", "repository": approval_repo} approval = gate.request_approval(conn, operation, requested_by="requester") gate.vote(conn, approval["id"], "voter", True) gate.consume_approval(conn, approval["id"], operation, agent="consumer") row = conn.execute( """ - SELECT a.requested_by_agent_id, a.consumed_by_agent_id, v.voter_agent_id + SELECT a.requested_by_agent_id, a.consumed_by_agent_id, v.voter_agent_id, + a.repository_id FROM approvals a JOIN votes v ON v.approval_id = a.id WHERE a.id = ? """, @@ -186,13 +203,16 @@ def test_contract_rejects_001_only_store_until_all_migrations_apply(tmp_path): conn.commit() conn.close() - message = "Missing required migrations: ['002_approval_identities.sql']" + message = ( + "Missing required migrations: " + "['002_approval_identities.sql', '004_approval_authority.sql']" + ) with pytest.raises(store.ContractViolation, match=re.escape(message)): store.verify_contract(cfg) with pytest.raises(store.ContractViolation, match=re.escape(message)): store.connect(cfg) - assert store.migrate(cfg) == ["002_approval_identities.sql"] + assert store.migrate(cfg) == UPGRADE_MIGRATIONS store.verify_contract(cfg) upgraded = store.connect(cfg) upgraded.close() @@ -209,7 +229,7 @@ def test_identity_migration_preserves_and_backfills_history(tmp_path): conn.commit() conn.close() - assert store.migrate(cfg) == ["002_approval_identities.sql"] + assert store.migrate(cfg) == UPGRADE_MIGRATIONS migrated = store.connect(cfg) try: requester = migrated.execute( @@ -230,7 +250,7 @@ def test_identity_migration_preserves_and_backfills_history(tmp_path): assert store.json_loads(invalidation[0], {})["reason"] == ( "unregistered historical voter" ) - with pytest.raises(sqlite3.IntegrityError, match="registered agent"): + with pytest.raises(sqlite3.IntegrityError, match="registered (agent|repository)"): migrated.execute( "INSERT INTO approvals (operation_hash, operation) VALUES (?, ?)", ("sha256:" + "1" * 64, store.json_dumps(operation)), @@ -249,7 +269,7 @@ def test_migration_does_not_backfill_requester_registered_after_request(tmp_path conn.commit() conn.close() - assert store.migrate(cfg) == ["002_approval_identities.sql"] + assert store.migrate(cfg) == UPGRADE_MIGRATIONS migrated = store.connect(cfg) try: row = migrated.execute( @@ -281,7 +301,7 @@ def test_migration_does_not_backfill_voter_registered_after_vote(tmp_path): conn.commit() conn.close() - assert store.migrate(cfg) == ["002_approval_identities.sql"] + assert store.migrate(cfg) == UPGRADE_MIGRATIONS migrated = store.connect(cfg) try: approval = migrated.execute( @@ -334,8 +354,7 @@ def test_stale_takeover_approval_cannot_displace_reacquired_claim( "from_agent": owner, "to_agent": successor, } - approval = gate.request_approval(conn, operation, requested_by=operator) - gate.vote(conn, approval["id"], operator, True) + approval = approve(conn, operation, requested_by=successor, voters=(operator,)) work.release_claim(conn, old_claim, owner) new_claim, _, _ = work.claim_task( conn, task, owner, "feat/new", ["src/**"] diff --git a/tests/test_gate.py b/tests/test_gate.py index 21b2e58..07b80b6 100644 --- a/tests/test_gate.py +++ b/tests/test_gate.py @@ -13,7 +13,7 @@ import pytest from quorumgit import gate, registry, store, work -from tests.conftest import make_git_repo +from tests.conftest import OPERATOR, approve, ensure_agent, make_git_repo, register_repo def _register_agents(conn, *names): @@ -101,30 +101,34 @@ def test_operation_hash_requires_fields(): gate.operation_hash({"type": "x"}) -def test_vote_threshold_and_deny(conn): - _register_agents(conn, "op", "voter-1", "voter-2") - op = {"type": "test_op", "repository": "r", "n": 1} - approval = gate.request_approval(conn, op, requested_by="op", threshold=2) +def test_vote_threshold_and_deny(conn, approval_repo): + _register_agents(conn, "op") + ensure_agent(conn, OPERATOR, "operator") + ensure_agent(conn, "voter-1", "operator") + ensure_agent(conn, "voter-2", "operator") + registry.set_approval_policy(conn, approval_repo, actor=OPERATOR, threshold=2) + op = {"type": "test_op", "repository": approval_repo, "n": 1} + approval = gate.request_approval(conn, op, requested_by="op") assert approval["status"] == "pending" + assert approval["threshold"] == 2 gate.vote(conn, approval["id"], "voter-1", True) assert gate.get_approval(conn, approval["operation_hash"])["status"] == "pending" result = gate.vote(conn, approval["id"], "voter-2", True) assert result["status"] == "approved" assert gate.is_approved(conn, op) - op2 = {"type": "test_op", "repository": "r", "n": 2} + op2 = {"type": "test_op", "repository": approval_repo, "n": 2} approval2 = gate.request_approval(conn, op2, requested_by="op") result2 = gate.vote(conn, approval2["id"], "voter-1", False) assert result2["status"] == "denied" assert not gate.is_approved(conn, op2) -def test_consume_approval_is_single_use(conn): +def test_consume_approval_is_single_use(conn, approval_repo): """One approval instance cannot authorize two operations.""" - op = {"type": "protected_ref_update", "repository": "r", "n": 1} + op = {"type": "protected_ref_update", "repository": approval_repo, "n": 1} _register_agents(conn, "op", "pusher-1", "pusher-2") - approval = gate.request_approval(conn, op, requested_by="op") - gate.vote(conn, approval["id"], "op", True) + approval = approve(conn, op, requested_by="op") assert gate.is_approved(conn, op) gate.consume_approval(conn, approval["id"], op, agent="pusher-1") consumed = gate.get_approval(conn, gate.operation_hash(op)) @@ -135,46 +139,46 @@ def test_consume_approval_is_single_use(conn): gate.consume_approval(conn, approval["id"], op, agent="pusher-2") -def test_consumed_operation_can_be_approved_again(conn): +def test_consumed_operation_can_be_approved_again(conn, approval_repo): """A consumed exact takeover may be requested again as a new instance.""" op = { "type": "repeatable_test_operation", - "repository": "r", + "repository": approval_repo, "task_id": 7, "from_agent": "a", "to_agent": "b", } - _register_agents(conn, "operator", "b") - first = gate.request_approval(conn, op, requested_by="operator") - gate.vote(conn, first["id"], "operator", True) + _register_agents(conn, "b") + first = approve(conn, op, requested_by="b") gate.consume_approval(conn, first["id"], op, agent="b") assert gate.get_approval(conn, gate.operation_hash(op))["status"] == "consumed" - second = gate.request_approval(conn, op, requested_by="operator") + second = gate.request_approval(conn, op, requested_by="b") assert second["id"] != first["id"] assert second["status"] == "pending" - gate.vote(conn, second["id"], "operator", True) + gate.vote(conn, second["id"], OPERATOR, True) assert gate.is_approved(conn, op) -def test_consume_approval_concurrent_single_winner(initialized_store): +def test_consume_approval_concurrent_single_winner(initialized_store, tmp_path): """Two real SQLite connections race to consume one approval; one wins. The second consumer is already in flight and blocked on SQLite's writer reservation before the first commits. This preserves the original genuine two-connection race while asserting BEGIN IMMEDIATE semantics. """ - op = { - "type": "protected_ref_update", - "repository": "race-repo", - "n": uuid.uuid4().hex, - } setup = store.connect(initialized_store) - _register_agents(setup, "op", "pusher-1", "pusher-2") - approval = gate.request_approval(setup, op, requested_by="op") - gate.vote(setup, approval["id"], "op", True) - setup.commit() - setup.close() + try: + op = { + "type": "protected_ref_update", + "repository": register_repo(setup, tmp_path / "race", prefix="race"), + "n": uuid.uuid4().hex, + } + _register_agents(setup, "op", "pusher-1", "pusher-2") + approval = approve(setup, op, requested_by="op") + setup.commit() + finally: + setup.close() conn1 = store.connect(initialized_store) racer_error: list[Exception] = [] @@ -245,9 +249,7 @@ def test_hook_protected_ref_requires_approval(committed_conn, tmp_path, cfg): "oldrev": oldrev, "newrev": newrev, } - _register_agents(committed_conn, "operator") - approval = gate.request_approval(committed_conn, op, requested_by="operator") - gate.vote(committed_conn, approval["id"], "operator", True) + approve(committed_conn, op, requested_by=a) committed_conn.commit() result = _push(clone, a, "main", cfg=cfg) diff --git a/tests/test_review_fixes.py b/tests/test_review_fixes.py index 069d4fd..c44f2d6 100644 --- a/tests/test_review_fixes.py +++ b/tests/test_review_fixes.py @@ -19,7 +19,7 @@ from quorumgit import gate, handoff, registry, store, trees, work from quorumgit.registry import RegistryError -from tests.conftest import make_git_repo +from tests.conftest import OPERATOR, approve, ensure_agent, make_git_repo, register_repo from tests.test_cli_hub import _cli from tests.test_gate import _commit, _push, _setup from tests.test_handoff import _handoff_record, _head @@ -27,15 +27,18 @@ # ------------------------------------------------------------ finding 1 -def _pending_approval(conn, threshold=1): +def _pending_approval(conn, tmp_path, threshold=1): + repo_name = register_repo(conn, tmp_path / "vote-race", prefix="vote-race") op = { "type": "protected_ref_update", - "repository": "vote-race", + "repository": repo_name, "n": uuid.uuid4().hex, } - for name in ("op", "voter-1", "voter-2"): - conn.execute("INSERT INTO agents (name) VALUES (?) ON CONFLICT DO NOTHING", (name,)) - gate.request_approval(conn, op, requested_by="op", threshold=threshold) + ensure_agent(conn, "op") + for name in (OPERATOR, "voter-1", "voter-2"): + ensure_agent(conn, name, "operator") + registry.set_approval_policy(conn, repo_name, actor=OPERATOR, threshold=threshold) + gate.request_approval(conn, op, requested_by="op") return op @@ -77,10 +80,10 @@ def racer(): conn2.close() -def test_concurrent_no_and_yes_denial_stands(initialized_store): +def test_concurrent_no_and_yes_denial_stands(initialized_store, tmp_path): """A concurrent yes vote cannot override a denial.""" setup = store.connect(initialized_store) - op = _pending_approval(setup, threshold=1) + op = _pending_approval(setup, tmp_path, threshold=1) setup.commit() setup.close() approval, second_error = _race_votes( @@ -90,10 +93,10 @@ def test_concurrent_no_and_yes_denial_stands(initialized_store): assert second_error, "yes vote on a denied approval must raise" -def test_concurrent_yes_votes_satisfy_threshold(initialized_store): +def test_concurrent_yes_votes_satisfy_threshold(initialized_store, tmp_path): """Two concurrent yes votes on a threshold-2 approval both count.""" setup = store.connect(initialized_store) - op = _pending_approval(setup, threshold=2) + op = _pending_approval(setup, tmp_path, threshold=2) setup.commit() setup.close() approval, second_error = _race_votes( @@ -103,8 +106,8 @@ def test_concurrent_yes_votes_satisfy_threshold(initialized_store): assert approval["status"] == "approved" -def test_denial_is_terminal(conn): - op = _pending_approval(conn) +def test_denial_is_terminal(conn, tmp_path): + op = _pending_approval(conn, tmp_path) op_hash = gate.operation_hash(op) approval_id = gate.get_approval(conn, op_hash)["id"] gate.vote(conn, approval_id, "voter-1", False) @@ -113,8 +116,8 @@ def test_denial_is_terminal(conn): gate.vote(conn, approval_id, "voter-2", True) -def test_duplicate_voter_counts_once(conn): - op = _pending_approval(conn, threshold=2) +def test_duplicate_voter_counts_once(conn, tmp_path): + op = _pending_approval(conn, tmp_path, threshold=2) op_hash = gate.operation_hash(op) approval_id = gate.get_approval(conn, op_hash)["id"] gate.vote(conn, approval_id, "voter-1", True) @@ -146,9 +149,7 @@ def _approve_update(conn, repo_name, hub, clone, refname="refs/heads/main"): "oldrev": oldrev, "newrev": newrev, } - conn.execute("INSERT INTO agents (name) VALUES ('operator') ON CONFLICT DO NOTHING") - approval = gate.request_approval(conn, op, requested_by="operator") - gate.vote(conn, approval["id"], "operator", True) + approve(conn, op, requested_by="op") conn.commit() return op @@ -219,9 +220,7 @@ def test_refused_takeover_preserves_holder_and_approval(committed_conn, tmp_path "from_agent": a, "to_agent": c, } - conn.execute("INSERT INTO agents (name) VALUES ('operator') ON CONFLICT DO NOTHING") - approval = gate.request_approval(conn, operation, requested_by="operator") - gate.vote(conn, approval["id"], "operator", True) + approve(conn, operation, requested_by=c) conn.commit() refused = _cli( diff --git a/tests/test_store.py b/tests/test_store.py index 32ff93c..4dae32f 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -156,3 +156,27 @@ def test_audit_append_only(conn): ) with pytest.raises(sqlite3.IntegrityError, match="append-only"): conn.execute("DELETE FROM audit_events WHERE entity = 'probe'") + + +def test_session_commits_rolls_back_and_closes(tmp_path): + """CLI sessions end their transaction and close the native connection.""" + local = Config(data_dir=tmp_path / "session", agent=None) + store.migrate(local) + try: + with store.session(local) as conn: + conn.execute("INSERT INTO agents (name) VALUES ('kept')") + with pytest.raises(sqlite3.ProgrammingError, match="closed"): + conn.execute("SELECT 1") + + with pytest.raises(RuntimeError), store.session(local) as conn: + conn.execute("INSERT INTO agents (name) VALUES ('dropped')") + raise RuntimeError("abort") + + check = store.connect(local) + try: + names = {r[0] for r in check.execute("SELECT name FROM agents").fetchall()} + finally: + check.close() + assert "kept" in names and "dropped" not in names + finally: + store.destroy(local) diff --git a/tests/test_worktree_continuation.py b/tests/test_worktree_continuation.py new file mode 100644 index 0000000..e6c5945 --- /dev/null +++ b/tests/test_worktree_continuation.py @@ -0,0 +1,202 @@ +"""A superseding claim continues the retained checkout instead of colliding. + +Git refuses to check one branch out twice. When a claim on the same task and +branch follows lease expiry, an approved takeover, or a release that kept its +worktree, the retained checkout (including uncommitted work) is verified and +transferred rather than duplicated. Managed paths never contain names. +""" + +from __future__ import annotations + +import re +import subprocess +import uuid +from pathlib import Path + +from quorumgit import audit, registry, trees, work +from tests.conftest import approve, ensure_agent, make_git_repo +from tests.test_cli_hub import _cli + + +def _repo_and_agents(conn, tmp_path, *agents): + suffix = uuid.uuid4().hex[:8] + repo = f"continue-{suffix}" + registry.add_repository(conn, repo, make_git_repo(tmp_path / "repo")) + names = [f"{agent}-{suffix}" for agent in agents] + for name in names: + ensure_agent(conn, name) + return repo, names + + +def _cli_claim(cfg, task, agent, branch, *extra): + return _cli( + cfg, "claim", str(task), "--branch", branch, "--scope", "src/**", *extra, + agent=agent, + ) + + +def _claimed_worktree(result) -> Path: + match = re.search(r"^worktree: (.+)$", result.stdout, re.MULTILINE) + assert match is not None, result.stdout + result.stderr + return Path(match.group(1)) + + +def _expire(conn, task): + conn.execute( + "UPDATE claims SET lease_expires_at = unixepoch() - 1 " + "WHERE task_id = ? AND released_at IS NULL", + (task,), + ) + + +def _events(conn, event_type): + return [ + event + for event in audit.events(conn, entity="worktree", limit=1000) + if event["event_type"] == event_type + ] + + +def test_reclaim_after_expiry_continues_retained_worktree( + committed_conn, cfg, tmp_path +): + conn = committed_conn + repo, (first, second) = _repo_and_agents(conn, tmp_path, "first", "second") + task = work.create_task(conn, repo, "expiring work") + conn.commit() + + claimed = _cli_claim(cfg, task, first, "feat/expiring") + assert claimed.returncode == 0, claimed.stderr + path = _claimed_worktree(claimed) + (path / "src" / "unfinished.py").write_text("work in progress\n") + + _expire(conn, task) + conn.commit() + reclaimed = _cli_claim(cfg, task, second, "feat/expiring") + assert reclaimed.returncode == 0, reclaimed.stderr + assert _claimed_worktree(reclaimed) == path + assert "continued retained worktree" in reclaimed.stdout + assert (path / "src" / "unfinished.py").read_text() == "work in progress\n" + + holder = work.active_claim_for_task(conn, task) + assert holder is not None and holder["agent"] == second + wt = trees.worktree_for_claim(conn, holder["id"]) + assert wt is not None and wt["path"] == str(path) + continued = [ + e for e in _events(conn, "worktree.continued") + if e["detail"]["to_claim_id"] == holder["id"] + ] + assert continued and continued[0]["detail"]["from_release_reason"] == "lease_expired" + + +def test_governed_takeover_continues_incumbent_worktree( + committed_conn, cfg, tmp_path +): + conn = committed_conn + repo, (holder, taker, requester) = _repo_and_agents( + conn, tmp_path, "holder", "taker", "requester" + ) + task = work.create_task(conn, repo, "taken work") + conn.commit() + claimed = _cli_claim(cfg, task, holder, "feat/taken") + assert claimed.returncode == 0, claimed.stderr + path = _claimed_worktree(claimed) + + incumbent = work.active_claim_for_task(conn, task) + assert incumbent is not None + approve( + conn, + { + "type": "lease_takeover", + "repository": repo, + "task_id": task, + "from_claim_id": incumbent["id"], + "from_agent": holder, + "to_agent": taker, + }, + requested_by=requester, + ) + conn.commit() + + taken = _cli_claim(cfg, task, taker, "feat/taken", "--takeover") + assert taken.returncode == 0, taken.stderr + assert _claimed_worktree(taken) == path + new_holder = work.active_claim_for_task(conn, task) + assert new_holder is not None and new_holder["agent"] == taker + + +def test_reclaim_after_release_keeping_worktree(committed_conn, cfg, tmp_path): + conn = committed_conn + repo, (owner,) = _repo_and_agents(conn, tmp_path, "owner") + task = work.create_task(conn, repo, "paused work") + conn.commit() + claimed = _cli_claim(cfg, task, owner, "feat/paused") + assert claimed.returncode == 0, claimed.stderr + path = _claimed_worktree(claimed) + first_claim = work.active_claim_for_task(conn, task) + assert first_claim is not None + released = _cli(cfg, "release", str(first_claim["id"]), agent=owner) + assert released.returncode == 0, released.stderr + + again = _cli_claim(cfg, task, owner, "feat/paused") + assert again.returncode == 0, again.stderr + assert _claimed_worktree(again) == path + + +def test_other_tasks_retained_worktree_is_not_adopted(committed_conn, cfg, tmp_path): + conn = committed_conn + repo, (a, b) = _repo_and_agents(conn, tmp_path, "a", "b") + task_one = work.create_task(conn, repo, "one") + task_two = work.create_task(conn, repo, "two") + conn.commit() + claimed = _cli_claim(cfg, task_one, a, "feat/shared") + assert claimed.returncode == 0, claimed.stderr + _expire(conn, task_one) + conn.commit() + + refused = _cli_claim(cfg, task_two, b, "feat/shared") + assert refused.returncode == 1 + assert "belongs to another task" in refused.stderr + assert "already checked out" not in refused.stderr + assert work.active_claim_for_task(conn, task_two) is None + + +def test_drifted_retained_worktree_is_not_adopted(committed_conn, cfg, tmp_path): + conn = committed_conn + repo, (a, b) = _repo_and_agents(conn, tmp_path, "a", "b") + task = work.create_task(conn, repo, "drifting") + conn.commit() + claimed = _cli_claim(cfg, task, a, "feat/drift") + assert claimed.returncode == 0, claimed.stderr + path = _claimed_worktree(claimed) + subprocess.run( + ["git", "-C", str(path), "checkout", "-q", "-b", "elsewhere"], + check=True, + capture_output=True, + ) + _expire(conn, task) + conn.commit() + + refused = _cli_claim(cfg, task, b, "feat/drift") + assert refused.returncode == 1 + assert "branch_mismatch" in refused.stderr + expired = conn.execute( + "SELECT count(*) FROM claims WHERE task_id = ? AND released_at IS NULL", + (task,), + ).fetchone()[0] + assert expired == 1, "the refused claim must roll back entirely" + + +def test_managed_paths_never_contain_names(conn, cfg, tmp_path): + hostile = "../../../escaped" + repo = f"../../outside-{uuid.uuid4().hex[:8]}" + registry.add_repository(conn, repo, make_git_repo(tmp_path / "named")) + ensure_agent(conn, hostile) + task = work.create_task(conn, repo, "hostile names") + claim, _, _ = work.claim_task(conn, task, hostile, "feat/names", ["src/**"]) + + wt = trees.create_worktree(conn, claim, cfg.worktrees_dir) + path = Path(wt["path"]) + assert path.is_relative_to(cfg.worktrees_dir.resolve()) + assert "escaped" not in wt["path"] and "outside" not in wt["path"] + assert path.is_dir()