diff --git a/README.md b/README.md index b29fe38..6c4f68f 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ QuorumGit prevents coding agents (or humans) working the same codebase from step agent-one ──┐ agent-two ──┼──► quorumgit CLI ──► local SQLite file (claims, leases, approvals, audit) reviewer ──┘ │ - └──► git pre-receive hook (push enforcement) + └──► git pre-receive + reference-transaction hooks (push enforcement) ``` ## The problem it solves @@ -42,7 +42,7 @@ Everything an agent does — claim, renew, checkpoint, hand off, release — wri ## Installation -Requirements: **Python 3.11–3.14**, **git**. There is no database to install, no server to run, and no extension to provision: the store is a single SQLite 3 database file under `QUORUMGIT_DATA_DIR`, accessed through Python's standard-library [`sqlite3`](https://docs.python.org/3/library/sqlite3.html) module. QuorumGit has no third-party runtime dependencies. The Python interpreter must be linked against SQLite 3.38 or newer (every supported CPython release from python.org, Homebrew, and current Linux distributions is); `quorumgit` refuses to open the store otherwise. +Requirements: **Python 3.11–3.14**, **git 2.28+** (for the reference-transaction hook). There is no database to install, no server to run, and no extension to provision: the store is a single SQLite 3 database file under `QUORUMGIT_DATA_DIR`, accessed through Python's standard-library [`sqlite3`](https://docs.python.org/3/library/sqlite3.html) module. QuorumGit has no third-party runtime dependencies. The Python interpreter must be linked against SQLite 3.38 or newer (every supported CPython release from python.org, Homebrew, and current Linux distributions is); `quorumgit` refuses to open the store otherwise. > **Platform note.** Linux, macOS, and Windows are supported on every Python from 3.11 to 3.14. @@ -59,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', '004_approval_authority.sql'] +migrations applied: ['001_core.sql', '002_approval_identities.sql', '003_ref_updates.sql', '004_approval_authority.sql'] contract: ok ``` -`init` creates the state directory, applies migrations, and verifies the runtime contract (required migrations, tables, and governance triggers, foreign keys on, WAL journal mode). It is idempotent — re-run it any time. +`init` creates the state directory, applies migrations, and verifies the runtime contract: required tables and migrations, foreign keys on, WAL journal mode, and **every schema object the governance rules depend on**. The expected schema is derived by replaying the bundled migrations in memory, so a store whose append-only audit triggers, single-owner/single-live-approval unique indexes, or identity triggers are missing or altered — or that carries extra indexes or triggers — fails the contract instead of silently losing an invariant. Every command runs this check. It is idempotent — re-run it any time. `quorumgit status` shows the store path, contract state, and row counts. If the store is missing or incomplete, every command exits non-zero: there is **one storage backend and no fallback**, by design. @@ -120,7 +120,7 @@ QUORUMGIT_AGENT=agent-two quorumgit handoff accept 1 # remaining work: wire into CLI, integration tests ``` -Creating a handoff releases the original claim but keeps its worktree; accepting transfers that same worktree to the new claim. When the claim has an active worktree, the handoff records the worktree's `HEAD` as the last commit; `--last-commit` is used when there is none (hub model, or a worktree removed by `doctor --repair`). +Creating a handoff releases the original claim but keeps its worktree; accepting transfers that same worktree to the new claim. The handoff records `--last-commit` as the continuation point when given (any commit spec, stored as the full OID), otherwise the worktree's `HEAD`; a claim without an active worktree must pass `--last-commit`. What just *didn't* happen, silently: a second agent claiming task 1 (**BLOCKED**), claiming another task on branch `feat/x` (**CONFLICTING**), or claiming a task whose scopes overlap `src/**` (**OVERLAPPING**, refused unless explicitly overridden and audited). While the handoff was open, the task, branch, and declared scopes were **reserved** for continuation until agent-two accepted. @@ -136,7 +136,7 @@ Every claim attempt is classified against all live claims before it is granted, | `CONFLICTING` | branch already claimed by another task | refused | | `BLOCKED` | task already held by an unexpired claim | refused; takeover requires approval | -Scope overlap uses a conservative literal-prefix test (the prefix of one glob up to its first wildcard against the other's) — it errs toward flagging. +Scopes are normalized when declared (`./src/**`, `/src/**`, `src//**` and `src\**` all become `src/**`; scopes that escape the repository root are refused). Scope overlap then uses a conservative literal-prefix test (the prefix of one glob up to its first wildcard against the other's), compared case-insensitively — it errs toward flagging. ## Two deployment models @@ -144,7 +144,7 @@ Where the registered repository sits determines how enforcement works. Pick one **Local model** — the registered repo is a working checkout on the same machine. Claims create isolated worktrees; agents commit directly in them. Coordination is entirely claim-based; no pushes, no hook needed. This is the default and the simplest setup — the quick start above is the local model. -**Hub model** — the registered repo is a **bare** hub that agents push to from their own clones. Enforcement moves to a pre-receive hook: +**Hub model** — the registered repo is a **bare** hub that agents push to from their own clones. Enforcement moves to a pair of Git hooks: ```bash git clone --bare /path/to/source myproject.git @@ -163,11 +163,15 @@ QUORUMGIT_AGENT=agent-two git push origin feat/x # rejected — branch is cla git push origin feat/x # rejected — unidentified ``` +**Authorization is bound to the ref update itself.** `pre-receive` checks the push and records each exact ref update it accepted, but spends nothing. Git's `reference-transaction` hook then runs at the `prepared` stage — after Git has locked the refs and while it can still abort — re-checks every rule against the store's current state, and only then consumes any approval. If ownership changed after `pre-receive` (a release, takeover, or handoff), the update is aborted. If Git refuses the update after `pre-receive` (another hook, a lost ref lock), no approval is spent; if Git aborts after `prepared`, the consumed approval is restored. The `committed`/`aborted` outcome of every governed update is recorded. `pre-receive` refuses pushes when the transaction hook is missing, modified, or not executable. A transaction that carries an agent identity must match a fresh `pre-receive` validation (and a store that has vanished fails it closed); only unidentified ref transactions — local maintenance in the hub — pass through. + +**Scopes are enforced at push time.** When a branch has a live claim, every commit the push introduces must only touch paths inside that claim's scopes (`**` spans directories, `*` stays within one; a scope without wildcards covers that file or directory). Merge commits count only the paths the merge itself changed, so merging the base branch in is not attributed to the claim holder. In the local model agents commit directly in their worktree, so no hook runs and scopes remain coordination metadata. + The hook learns who is pushing from `QUORUMGIT_AGENT` in its own environment, which Git passes through only when the remote is a **local path** (as `origin` is above). Over SSH or HTTP the variable does not reach the hook, so every push is rejected as unidentified. The hook also opens the store under its own `QUORUMGIT_DATA_DIR`, so pushers must use the same data directory as the rest of the deployment. Accepting a handoff in the hub model prints `worktree: (none — create manually)`: there is no managed worktree to transfer, so the successor fetches the branch into its own clone and continues from the reported commit. -Checkpoints in the hub model take an explicit `--commit `. Continuation points are **verified, not trusted**: the commit must exist in the registered repository, and when the claimed branch exists, be reachable from it. A typo'd or fabricated OID is rejected. +Checkpoints in the hub model take an explicit `--commit ` (any commit spec Git resolves — a full or abbreviated OID or a ref — recorded as the full OID). Continuation points are **verified, not trusted**: the commit must exist in the registered repository, and when the claimed branch exists, be reachable from it. A typo'd or fabricated OID is rejected. ## Protected operations and approvals @@ -187,7 +191,7 @@ QUORUMGIT_AGENT=agent-one quorumgit approve request '{"type":"protected_ref_upda 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. +# the approval was consumed while Git held the ref lock, as the update committed. ``` Rules that hold no matter what: @@ -207,7 +211,7 @@ Registering a `reviewer` or `operator`, changing a role (`quorumgit agent role < > **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. +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. When the replacement claim uses the incumbent's branch, it inherits the incumbent's managed worktree (including uncommitted work), because Git will not check one branch out twice; reclaiming an expired lease on the same branch works the same way. 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. @@ -217,7 +221,7 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c |---|---| | `quorumgit init` | Start/provision the store (idempotent) | | `quorumgit status` | Store health, contract check, row counts | -| `quorumgit doctor [--repair]` | Detect and conservatively reconcile recorded managed-worktree drift | +| `quorumgit doctor [--repair]` | Detect and conservatively reconcile managed-worktree drift and stuck ref updates | | `quorumgit destroy --yes` | Delete the database file (managed worktrees under `QUORUMGIT_DATA_DIR/worktrees` are left in place) | | `quorumgit repo add [--protected-ref ]…` | Register a repository | | `quorumgit agent add [--role worker\|reviewer\|operator]` | Register an agent identity (non-workers need an operator once one exists) | @@ -226,9 +230,9 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c | `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 | -| `quorumgit checkpoint [--commit ] [--note ]` | Record verified progress | +| `quorumgit checkpoint [--commit ] [--note ]` | Record verified progress | | `quorumgit release [--remove-worktree] [--reason ]` | Release a claim; the task returns to `open` | -| `quorumgit handoff create --completed --remaining [--to ] [--files-changed ]… [--blockers ]… [--validation ] [--last-commit ]` | Hand work off (`--last-commit` only when the claim has no active worktree) | +| `quorumgit handoff create --completed --remaining [--to ] [--files-changed ]… [--blockers ]… [--validation ] [--last-commit ]` | Hand work off (from `--last-commit` when given, else the worktree's HEAD) | | `quorumgit handoff accept [--lease-hours ]` | Continue handed-off work (addressee, or anyone if unaddressed) | | `quorumgit handoff decline ` | Decline — addressee only; removes the retained worktree, and is refused if it has uncommitted changes | | `quorumgit handoff cancel ` | Cancel — creator only; removes the retained worktree, and is refused if it has uncommitted changes | @@ -236,7 +240,7 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c | `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 hook install --repo ` | Install the pre-receive and reference-transaction hooks (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`, 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. @@ -250,6 +254,8 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c Repairs use ordinary non-forced `git worktree remove`, so dirty worktrees are reported instead of destroyed. `doctor` exits non-zero while any finding remains unresolved. +`doctor` also finds governed ref updates stuck in `prepared` for more than five minutes — ones whose `committed`/`aborted` hook never recorded an outcome (a killed process, an unreachable store). It reads the outcome from the ref itself: at the update's new value (or gone, for a deletion) means Git committed; still at the old value means Git aborted, and `--repair` restores the approval that was consumed for it. A ref that has since moved elsewhere, or is still locked by Git, is reported for manual inspection and never guessed at. + ## Configuration | Variable | Purpose | Default | @@ -273,7 +279,7 @@ 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. +- The hooks govern `git push` only. Direct ref manipulation inside a repository bypasses them. - 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. diff --git a/src/quorumgit/cli.py b/src/quorumgit/cli.py index ff3588f..a2be736 100644 --- a/src/quorumgit/cli.py +++ b/src/quorumgit/cli.py @@ -70,28 +70,43 @@ def cmd_destroy(args, cfg) -> int: def cmd_doctor(args, cfg) -> int: - """Inspect and optionally reconcile recorded managed-worktree drift.""" + """Inspect and optionally reconcile worktree drift and stuck ref updates.""" with store.session(cfg) as conn: findings = trees.doctor_worktrees(conn, repair=args.repair) + ref_findings = gate.doctor_ref_updates(conn, repair=args.repair) if args.repair: conn.commit() - if not findings: - print("worktrees: ok") - return 0 unresolved = False - for finding in findings: - state = "repaired" if finding.get("repaired") else "detected" + + def state_of(finding: dict) -> str: + nonlocal unresolved if finding.get("error"): - state = f"repair failed: {finding['error']}" - unresolved = True - elif not finding.get("repaired"): unresolved = True + prefix = "repair failed" if args.repair else "unresolvable" + return f"{prefix}: {finding['error']}" + if finding.get("repaired"): + return "repaired" + unresolved = True + return "detected" + + for finding in findings: print( f"worktree {finding['worktree_id']} claim {finding['claim_id']}: " - f"{finding['issue']} — {state} — {finding['path']}" + f"{finding['issue']} — {state_of(finding)} — {finding['path']}" ) - if args.repair and not unresolved: - print("worktrees: reconciled") + for finding in ref_findings: + outcome = f" (git {finding['outcome']})" if finding.get("outcome") else "" + print( + f"ref update {finding['ref_update_id']} {finding['repository']} " + f"{finding['refname']}: {finding['issue']}{outcome} — {state_of(finding)}" + ) + if not findings: + print("worktrees: ok") + if not ref_findings: + print("ref updates: ok") + if not unresolved: + if args.repair and (findings or ref_findings): + print("reconciled.") return 0 if not args.repair: print("re-run with `quorumgit doctor --repair` to reconcile safe cases.") @@ -321,6 +336,7 @@ def cmd_checkpoint(args, cfg) -> int: commit = trees.head_commit(wt["path"]) cp_id = work.add_checkpoint(conn, args.claim_id, agent, commit, note=args.note) + commit = work.checkpoint_commit(conn, cp_id) conn.commit() print(f"checkpoint {cp_id} at {commit}.") return 0 @@ -332,12 +348,16 @@ def cmd_checkpoint(args, cfg) -> int: def cmd_handoff_create(args, cfg) -> int: agent = _agent(args, cfg) with store.session(cfg) as conn: - wt = trees.active_worktree_for_claim(conn, args.claim_id) - last_commit = trees.head_commit(wt["path"]) if wt else args.last_commit + # An explicit --last-commit always wins; otherwise continue from the + # managed worktree's HEAD. + last_commit = args.last_commit if not last_commit: - print("Provide --last-commit (no active worktree for this claim).", - file=sys.stderr) - return 1 + wt = trees.active_worktree_for_claim(conn, args.claim_id) + if wt is None: + print("Provide --last-commit (no active worktree for this claim).", + file=sys.stderr) + return 1 + last_commit = trees.head_commit(wt["path"]) record = { "completed": args.completed, "remaining": args.remaining, @@ -352,6 +372,7 @@ def cmd_handoff_create(args, cfg) -> int: handoff_id = handoff.create_handoff( conn, args.claim_id, agent, record, to_agent=args.to ) + last_commit = handoff.get_handoff(conn, handoff_id)["record"]["last_commit"] conn.commit() print(f"handoff {handoff_id} created (last commit {last_commit}).") return 0 @@ -406,9 +427,12 @@ def cmd_handoff_cancel(args, cfg) -> int: def _operation_from_args(args) -> dict: - operation = json.loads(args.operation) + try: + operation = json.loads(args.operation) + except json.JSONDecodeError as exc: + raise gate.GateError(f"Operation is not valid JSON: {exc}") from exc if not isinstance(operation, dict): - raise SystemExit("Operation must be a JSON object.") + raise gate.GateError("Operation must be a JSON object.") return operation @@ -448,8 +472,11 @@ def cmd_approve_hash(args, cfg) -> int: def cmd_hook_install(args, cfg) -> int: with store.session(cfg) as conn: path = gate.install_hook(conn, args.repo) + repo_path = registry.get_repository(conn, args.repo)["path"] conn.commit() print(f"pre-receive hook installed: {path}") + print("reference-transaction hook installed: " + f"{gate._effective_hook(repo_path, 'reference-transaction')}") return 0 @@ -458,6 +485,34 @@ def cmd_hook_pre_receive(args, cfg) -> int: return gate.run_pre_receive(conn, args.repo, sys.stdin) +def cmd_hook_reference_transaction(args, cfg) -> int: + if not store.database_path(cfg).exists(): + # Without a store, only unidentified local ref maintenance may pass. + # Every push is identified (pre-receive requires it), so a store that + # vanished after pre-receive must not wave its update through. + if args.state == "prepared" and cfg.agent: + print( + "[quorumgit] REJECTED: the store is missing; cannot re-validate " + f"this update by {cfg.agent}.", + file=sys.stderr, + ) + return 1 + return 0 + try: + conn = store.connect(cfg) + except store.StoreError as exc: + label = "REJECTED" if args.state == "prepared" else f"WARNING ({args.state})" + print(f"[quorumgit] {label}: {exc}", file=sys.stderr) + return 1 + try: + with conn: + return gate.run_reference_transaction( + conn, args.repo, args.state, sys.stdin + ) + finally: + conn.close() + + # -------------------------------------------------------------------- audit @@ -618,6 +673,10 @@ def add(name, fn, configure=None, parent=None): lambda sp: sp.add_argument("--repo", required=True), parent=hook) add("pre-receive", cmd_hook_pre_receive, lambda sp: sp.add_argument("--repo", required=True), parent=hook) + add("reference-transaction", cmd_hook_reference_transaction, lambda sp: ( + sp.add_argument("--repo", required=True), + sp.add_argument("state"), + ), parent=hook) add("audit", cmd_audit, lambda sp: ( sp.add_argument("--entity"), diff --git a/src/quorumgit/gate.py b/src/quorumgit/gate.py index 26e15d6..eabe551 100644 --- a/src/quorumgit/gate.py +++ b/src/quorumgit/gate.py @@ -11,7 +11,9 @@ from __future__ import annotations import os +import re import shlex +import sqlite3 import subprocess import sys import tempfile @@ -27,9 +29,15 @@ assert_repository_identity_unique, get_agent, get_repository, + path_scoped_git_env, ) from .store import Connection, begin_immediate, json_dumps, json_loads -from .work import live_claim_for_branch, open_handoff_for_branch +from .work import ( + claim_scopes, + live_claim_for_branch, + open_handoff_for_branch, + path_in_scopes, +) HOOK_MARKER = "# quorumgit-managed-pre-receive v1" @@ -288,7 +296,7 @@ def vote(conn: Connection, approval_id: int, voter: str, approve: bool) -> dict: begin_immediate(conn) voter_row = get_agent(conn, voter) approval = get_approval_by_id(conn, approval_id) - if approval["status"] != "pending": + if approval["status"] not in ("pending", "approved"): raise GateError(f"Approval {approval_id} is already {approval['status']}.") policy = _policy_for(conn, approval) refusal = _vote_refusal(approval, policy, voter_row) @@ -324,11 +332,11 @@ def vote(conn: Connection, approval_id: int, voter: str, approve: bool) -> dict: new_status = "approved" else: new_status = "pending" - if new_status != "pending": + if new_status != approval["status"]: conn.execute( "UPDATE approvals SET status = ?, decided_at = unixepoch() " - "WHERE id = ? AND status = 'pending'", - (new_status, approval["id"]), + "WHERE id = ? AND status = ?", + (new_status, approval["id"], approval["status"]), ) audit.record( conn, @@ -457,6 +465,64 @@ def _is_fast_forward(git_dir: str, oldrev: str, newrev: str) -> bool: return result.returncode == 0 +MAX_REPORTED_PATHS = 10 + + +def _pushed_paths(git_dir: str, newrev: str) -> list[str]: + """Paths changed by the commits this push introduces to the repository. + + `newrev --not --all` is exactly the set of new commits: in pre-receive no + ref points at them yet. `--cc` lists only the paths a merge commit itself + authored (differing from every parent), so merging the base branch in does + not attribute the base's files to the pusher. Renames count as both paths. + """ + result = subprocess.run( + [ + "git", + "--git-dir", + git_dir, + "log", + "--format=", + "--name-only", + "--no-renames", + "--cc", + "-z", + newrev, + "--not", + "--all", + ], + capture_output=True, + check=False, + ) + if result.returncode != 0: + raise PushRejected("Unable to determine the paths changed by this push.") + paths = ( + entry.lstrip("\n") + for entry in result.stdout.decode("utf-8", "surrogateescape").split("\0") + ) + return list(dict.fromkeys(path for path in paths if path)) + + +def enforce_claim_scopes( + conn: Connection, git_dir: str, claim: dict, branch: str, newrev: str +) -> None: + """Reject new commits that touch paths outside the claim's declared scopes.""" + scopes = claim_scopes(conn, claim["id"]) + outside = [ + path for path in _pushed_paths(git_dir, newrev) + if not path_in_scopes(path, scopes) + ] + if outside: + shown = ", ".join(outside[:MAX_REPORTED_PATHS]) + more = len(outside) - MAX_REPORTED_PATHS + if more > 0: + shown += f", and {more} more" + raise PushRejected( + f"Push to {branch!r} changes paths outside claim {claim['id']}'s " + f"scopes {scopes}: {shown}. Claim a scope that covers them first." + ) + + def _invoking_git_common_dir(git_dir: str) -> Path: result = subprocess.run( ["git", "--git-dir", git_dir, "rev-parse", "--git-common-dir"], @@ -493,7 +559,27 @@ def _verify_repository_binding( return repo -def check_ref_update( +def _required_approval( + conn: Connection, operation: dict[str, Any], pusher: str +) -> dict: + """The approval that authorizes `operation` for `pusher`, or PushRejected. + + Uses the repository's approval policy: the pusher must not have approved + the operation itself, and the eligible votes are re-derived from current + roles and policy. + """ + approval = approved_instance(conn, operation, consumer=pusher) + if approval is None: + refusal = approval_refusal(conn, operation, consumer=pusher) + raise PushRejected( + f"{operation['type']} on {operation['refname']} requires an approval " + f"bound to this exact update (hash {operation_hash(operation)})." + + (f" {refusal}" if refusal else "") + ) + return approval + + +def evaluate_ref_update( conn: Connection, repository: str, git_dir: str, @@ -501,8 +587,14 @@ def check_ref_update( oldrev: str, newrev: str, refname: str, -) -> None: - """Enforce governance for one ref update. Raises PushRejected.""" +) -> tuple[dict, dict[str, Any] | None, dict | None]: + """Apply every push rule to one ref update without changing any state. + + Returns (repository, protected operation or None, its approval or None). + Raises PushRejected. Runs twice per governed update: in pre-receive, and + again at the reference transaction's `prepared` stage against the state + current while Git holds the ref locks. + """ repo = _verify_repository_binding(conn, repository, git_dir) branch = refname.removeprefix("refs/heads/") @@ -520,6 +612,8 @@ def check_ref_update( f"(claim {claim['id']}); pusher is " f"{pusher or 'unidentified — set QUORUMGIT_AGENT'}." ) + if claim and not _is_zero(newrev): + enforce_claim_scopes(conn, git_dir, claim, branch, newrev) protected = refname in repo["protected_refs"] deletion = _is_zero(newrev) @@ -528,44 +622,72 @@ def check_ref_update( and not _is_zero(oldrev) and not _is_fast_forward(git_dir, oldrev, newrev) ) + if not (protected or deletion or forced): + return repo, None, None + + operation = { + "type": "protected_ref_update" + if protected + else ("ref_delete" if deletion else "force_update"), + "repository": repository, + "refname": refname, + "oldrev": oldrev, + "newrev": newrev, + } + if pusher is None: + raise PushRejected("Pusher identity required for a protected update.") + return repo, operation, _required_approval(conn, operation, pusher) - if protected or deletion or forced: - operation = { - "type": "protected_ref_update" - if protected - else ("ref_delete" if deletion else "force_update"), - "repository": repository, - "refname": refname, - "oldrev": oldrev, - "newrev": newrev, - } - 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) - audit.record( - conn, - "gate.protected_update_allowed", - "repository", - repo["id"], - agent=pusher, - detail=operation, - ) - return +def check_ref_update( + conn: Connection, + repository: str, + git_dir: str, + pusher: str, + oldrev: str, + newrev: str, + refname: str, +) -> None: + """pre-receive: validate one ref update and record it for its transaction. + + Nothing is consumed here. Git may still refuse the update after + pre-receive (another hook, a lost ref lock), so the approval is only spent + once Git has locked the ref, in the reference-transaction hook. + """ + repo, operation, approval = evaluate_ref_update( + conn, repository, git_dir, pusher, oldrev, newrev, refname + ) + row = conn.execute( + """ + INSERT INTO ref_updates ( + repository_id, refname, oldrev, newrev, pusher_agent_id, operation + ) + VALUES (?, ?, ?, ?, ?, ?) + RETURNING id + """, + ( + repo["id"], + refname, + oldrev, + newrev, + get_agent(conn, pusher)["id"], + json_dumps(operation) if operation else None, + ), + ).fetchone() + assert row is not None audit.record( conn, - "gate.update_allowed", - "repository", - repo["id"], + "gate.update_validated", + "ref_update", + row[0], agent=pusher, - detail={"refname": refname, "oldrev": oldrev, "newrev": newrev}, + detail={ + "refname": refname, + "oldrev": oldrev, + "newrev": newrev, + "operation": operation, + "approval_id": approval["id"] if approval else None, + }, ) @@ -599,6 +721,9 @@ def run_pre_receive( check_ref_update(conn, repository, git_dir, pusher, *parts) if not saw_update: raise PushRejected("No ref updates supplied on stdin.") + # Without the transaction hook nothing would re-validate at update + # time or consume approvals, so a missing one must reject the push. + _require_reference_transaction_hook(conn, repository) except Exception as exc: # noqa: BLE001 — fail closed on anything conn.rollback() print(f"[quorumgit] REJECTED: {exc}", file=sys.stderr) @@ -608,10 +733,386 @@ def run_pre_receive( return 0 -def _effective_pre_receive_hook(repository_path: str | Path) -> Path: +# ------------------------------------------------------ reference transaction + +TRANSACTION_STATES = ("prepared", "committed", "aborted") +# pre-receive and the reference transaction run seconds apart. A validation +# older than this is stale (Git refused the update after pre-receive) and is +# never matched against a later transaction. +VALIDATION_TTL_SECONDS = 300 + + +def _parse_updates(stdin_lines: Iterable[str]) -> list[tuple[str, str, str]]: + updates = [] + for line in stdin_lines: + parts = line.split() + if not parts: + continue + if len(parts) != 3: + raise PushRejected(f"Malformed reference-transaction input: {line!r}") + updates.append((parts[0], parts[1], parts[2])) + return updates + + +def _find_update( + conn: Connection, + repository_id: int, + status: str, + oldrev: str, + newrev: str, + refname: str, +) -> tuple | None: + """The newest recorded update matching one transaction line. + + Git reports the zero OID as the old value when a transaction does not + check it, so a zero old value matches any recorded one. + """ + return conn.execute( + """ + SELECT u.id, u.oldrev, a.name, u.approval_id + FROM ref_updates u JOIN agents a ON a.id = u.pusher_agent_id + WHERE u.repository_id = ? AND u.status = ? AND u.refname = ? + AND u.newrev = ? AND (u.oldrev = ? OR ?) + AND u.created_at >= unixepoch() - ? + ORDER BY u.id DESC + LIMIT 1 + """, + ( + repository_id, + status, + refname, + newrev, + oldrev, + 1 if _is_zero(oldrev) else 0, + VALIDATION_TTL_SECONDS, + ), + ).fetchone() + + +def _prepare_updates( + conn: Connection, + repository: str, + git_dir: str, + pusher: str | None, + updates: list[tuple[str, str, str]], +) -> None: + """Re-validate and authorize validated updates while Git holds the locks. + + Only an unidentified transaction with no matching validation is local ref + maintenance. An identified one (every push carries QUORUMGIT_AGENT, which + pre-receive requires) must match a fresh validation, or it is rejected: + an expired or missing record never downgrades a push to pass-through. + """ + repo = get_repository(conn, repository) + governed = [] + # Classify without the write reservation, so pass-through maintenance + # never waits on (or deadlocks with) a command holding the store. + for oldrev, newrev, refname in updates: + if not refname.startswith("refs/"): + # A symbolic ref such as HEAD is listed alongside the ref it + # points to; that ref appears as its own line and is governed + # there. Pushes only ever name refs under refs/. + continue + found = _find_update(conn, repo["id"], "validated", oldrev, newrev, refname) + if found is not None: + governed.append((oldrev, newrev, refname)) + continue + if pusher is not None: + raise PushRejected( + f"Update of {refname} to {newrev} by {pusher} has no current " + "pre-receive validation; push it again." + ) + stray = conn.execute( + "SELECT id FROM ref_updates WHERE repository_id = ? " + "AND refname = ? AND status = 'validated' " + "AND created_at >= unixepoch() - ? LIMIT 1", + (repo["id"], refname, VALIDATION_TTL_SECONDS), + ).fetchone() + if stray is not None: + raise PushRejected( + f"Update of {refname} to {newrev} does not match the update " + "pre-receive validated." + ) + if not governed: + return + begin_immediate(conn) + for oldrev, newrev, refname in governed: + found = _find_update(conn, repo["id"], "validated", oldrev, newrev, refname) + if found is None: + raise PushRejected(f"Update of {refname} changed while being prepared.") + update_id, validated_old, validated_pusher, _ = found + if pusher != validated_pusher: + raise PushRejected( + f"Update of {refname} was validated for {validated_pusher}, " + f"not {pusher or 'an unidentified pusher'}." + ) + _, operation, approval = evaluate_ref_update( + conn, repository, git_dir, pusher, validated_old, newrev, refname + ) + if operation is not None: + assert approval is not None and pusher is not None + consume_approval(conn, approval["id"], operation, agent=pusher) + cur = conn.execute( + "UPDATE ref_updates SET status = 'prepared', operation = ?, " + "approval_id = ? WHERE id = ? AND status = 'validated'", + ( + json_dumps(operation) if operation else None, + approval["id"] if approval else None, + update_id, + ), + ) + if cur.rowcount != 1: + raise PushRejected(f"Update of {refname} changed while being prepared.") + audit.record( + conn, + "gate.update_prepared", + "ref_update", + update_id, + agent=pusher, + detail={ + "refname": refname, + "oldrev": validated_old, + "newrev": newrev, + "approval_id": approval["id"] if approval else None, + }, + ) + + +def _restore_approval(conn: Connection, update_id: int, approval_id: int) -> None: + """Return an approval consumed for an update Git then aborted.""" + try: + cur = conn.execute( + "UPDATE approvals SET status = 'approved', consumed_at = NULL, " + "consumed_by_agent_id = NULL WHERE id = ? AND status = 'consumed'", + (approval_id,), + ) + except sqlite3.IntegrityError as exc: + # A newer live instance for the same operation now exists; the old + # one stays consumed rather than creating two live approvals. + audit.record( + conn, + "approval.restore_skipped", + "approval", + approval_id, + detail={"ref_update_id": update_id, "reason": str(exc)}, + ) + return + if cur.rowcount == 1: + audit.record( + conn, + "approval.restored", + "approval", + approval_id, + detail={"ref_update_id": update_id}, + ) + + +def _resolve_updates( + conn: Connection, + repository: str, + state: str, + updates: list[tuple[str, str, str]], +) -> None: + begin_immediate(conn) + repo = get_repository(conn, repository) + for oldrev, newrev, refname in updates: + statuses = ("prepared",) if state == "committed" else ("prepared", "validated") + for status in statuses: + found = _find_update(conn, repo["id"], status, oldrev, newrev, refname) + if found is None: + continue + update_id, _, pusher, approval_id = found + conn.execute( + "UPDATE ref_updates SET status = ?, resolved_at = unixepoch() " + "WHERE id = ? AND status = ?", + (state, update_id, status), + ) + if state == "aborted" and approval_id is not None: + _restore_approval(conn, update_id, approval_id) + audit.record( + conn, + f"gate.update_{state}", + "ref_update", + update_id, + agent=pusher, + detail={"refname": refname, "newrev": newrev}, + ) + break + + +# A prepared update normally resolves within milliseconds, when Git commits or +# aborts the locked transaction. One older than this lost its outcome hook +# (killed process, unreachable store) and is reconciled from the ref itself. +PREPARED_STUCK_AFTER_SECONDS = VALIDATION_TTL_SECONDS + + +def _current_ref(repository_path: str, refname: str) -> str | None: + """The ref's object ID in the repository, or None when it does not exist.""" + result = subprocess.run( + ["git", "-C", repository_path, "rev-parse", "--verify", "--quiet", refname], + capture_output=True, + text=True, + env=path_scoped_git_env(), + check=False, + ) + if result.returncode == 0: + return result.stdout.strip().lower() + if result.returncode == 1: + return None + detail = (result.stderr or result.stdout).strip() + raise GateError(f"Unable to read {refname} in {repository_path}: {detail}") + + +def _ref_is_locked(repository_path: str, refname: str) -> bool: + """True while Git holds the files-backend lock for the ref.""" + result = subprocess.run( + ["git", "-C", repository_path, "rev-parse", "--git-path", f"{refname}.lock"], + capture_output=True, + text=True, + env=path_scoped_git_env(), + check=False, + ) + if result.returncode != 0: + return False + lock = Path(result.stdout.strip()) + if not lock.is_absolute(): + lock = Path(repository_path) / lock + return lock.exists() + + +def doctor_ref_updates(conn: Connection, repair: bool = False) -> list[dict]: + """Report and conservatively reconcile ref updates stuck in `prepared`. + + The outcome is read from the ref itself: at the new value (or gone, for + a deletion) means Git committed; still at the old value (or absent, for a + creation) means Git aborted, so the consumed approval is restored. A ref + that has since moved elsewhere, or is still locked, is reported for + manual inspection and never guessed at. + """ + if repair: + begin_immediate(conn) + rows = conn.execute( + """ + SELECT u.id, u.refname, u.oldrev, u.newrev, u.approval_id, r.name, r.path + FROM ref_updates u JOIN repositories r ON r.id = u.repository_id + WHERE u.status = 'prepared' AND u.created_at < unixepoch() - ? + ORDER BY u.id + """, + (PREPARED_STUCK_AFTER_SECONDS,), + ).fetchall() + findings: list[dict] = [] + for update_id, refname, oldrev, newrev, approval_id, repo_name, repo_path in rows: + finding: dict[str, Any] = { + "ref_update_id": update_id, + "repository": repo_name, + "refname": refname, + "issue": "stuck_prepared", + "outcome": None, + "repaired": False, + } + findings.append(finding) + try: + if _ref_is_locked(repo_path, refname): + finding["error"] = "ref is still locked by Git; retry once it is released" + continue + current = _current_ref(repo_path, refname) + except GateError as exc: + finding["error"] = str(exc) + continue + new_value = None if _is_zero(newrev) else newrev + old_value = None if _is_zero(oldrev) else oldrev + if current == new_value: + finding["outcome"] = "committed" + elif current == old_value: + finding["outcome"] = "aborted" + else: + finding["error"] = ( + f"{refname} has since moved to {current or 'deletion'}; the outcome " + "cannot be proven from the ref. Inspect it manually." + ) + continue + if not repair: + continue + cur = conn.execute( + "UPDATE ref_updates SET status = ?, resolved_at = unixepoch() " + "WHERE id = ? AND status = 'prepared'", + (finding["outcome"], update_id), + ) + if cur.rowcount != 1: + finding["error"] = "ref update changed while being reconciled" + continue + if finding["outcome"] == "aborted" and approval_id is not None: + _restore_approval(conn, update_id, approval_id) + audit.record( + conn, + f"gate.update_reconciled_{finding['outcome']}", + "ref_update", + update_id, + detail={"refname": refname, "current": current}, + ) + finding["repaired"] = True + return findings + + +def run_reference_transaction( + conn: Connection, repository: str, state: str, stdin_lines: Iterable[str] +) -> int: + """Hook entry point for Git's reference-transaction hook. + + `prepared` is the authoritative gate: its failure makes Git abort the + transaction, so it fails closed. Git ignores the exit status of + `committed` and `aborted`; they only record the outcome. + """ + if state not in TRANSACTION_STATES: + return 0 + git_dir = os.environ.get("GIT_DIR") or "." + pusher = os.environ.get("QUORUMGIT_AGENT") or None + try: + updates = _parse_updates(stdin_lines) + if state == "prepared": + _prepare_updates(conn, repository, git_dir, pusher, updates) + else: + _resolve_updates(conn, repository, state, updates) + except Exception as exc: # noqa: BLE001 — fail closed on anything + conn.rollback() + label = "REJECTED" if state == "prepared" else f"WARNING ({state})" + print(f"[quorumgit] {label}: {exc}", file=sys.stderr) + return 1 + conn.commit() + return 0 + + +# ------------------------------------------------------------ hook install + +HOOK_NAMES = ("pre-receive", "reference-transaction") +REFERENCE_TRANSACTION_MARKER = "# quorumgit-managed-reference-transaction v1" +# The reference-transaction hook (with its prepared/committed/aborted states). +MIN_GIT_VERSION = (2, 28) + + +def _git_version() -> tuple[int, ...]: + result = subprocess.run( + ["git", "version"], capture_output=True, text=True, check=False + ) + match = re.search(r"(\d+)\.(\d+)", result.stdout) + if result.returncode != 0 or match is None: + raise GateError("Unable to determine the Git version.") + return (int(match.group(1)), int(match.group(2))) + + +def _require_git_version() -> None: + version = _git_version() + if version < MIN_GIT_VERSION: + raise GateError( + f"Git {'.'.join(map(str, version))} lacks the reference-transaction " + f"hook; QuorumGit needs Git {'.'.join(map(str, MIN_GIT_VERSION))} or newer." + ) + + +def _effective_hook(repository_path: str | Path, name: str) -> Path: repo_path = Path(repository_path).resolve() result = subprocess.run( - ["git", "-C", str(repo_path), "rev-parse", "--git-path", "hooks/pre-receive"], + ["git", "-C", str(repo_path), "rev-parse", "--git-path", f"hooks/{name}"], capture_output=True, text=True, check=False, @@ -622,13 +1123,17 @@ def _effective_pre_receive_hook(repository_path: str | Path) -> Path: raise GateError(f"Unable to resolve Git hook path for {repo_path}{suffix}") raw = result.stdout.strip() if not raw: - raise GateError(f"Git returned no pre-receive hook path for {repo_path}") + raise GateError(f"Git returned no {name} hook path for {repo_path}") hook_path = Path(raw) if not hook_path.is_absolute(): hook_path = repo_path / hook_path return hook_path.resolve() +def _effective_pre_receive_hook(repository_path: str | Path) -> Path: + return _effective_hook(repository_path, "pre-receive") + + def _hook_script(repository: str, executable: str | None = None) -> str: python = executable or sys.executable return ( @@ -639,6 +1144,22 @@ def _hook_script(repository: str, executable: str | None = None) -> str: ) +def _reference_transaction_script( + repository: str, executable: str | None = None +) -> str: + python = executable or sys.executable + return ( + "#!/bin/sh\n" + f"{REFERENCE_TRANSACTION_MARKER}\n" + 'case "$1" in\n' + "prepared|committed|aborted) ;;\n" + "*) cat >/dev/null; exit 0 ;;\n" + "esac\n" + f"exec {shlex.quote(python)} -m quorumgit hook reference-transaction " + f'--repo {shlex.quote(repository)} "$1"\n' + ) + + def _legacy_hook_script(repository: str) -> str: """Exact pre-PR6 hook shape, recognized only for safe in-place upgrade.""" return ( @@ -648,6 +1169,36 @@ def _legacy_hook_script(repository: str) -> str: ) +def _hook_plan(repository: str, name: str) -> tuple[str, str, tuple[str, ...]]: + """(expected content, ownership marker, upgradeable legacy contents).""" + if name == "pre-receive": + return _hook_script(repository), HOOK_MARKER, (_legacy_hook_script(repository),) + return _reference_transaction_script(repository), REFERENCE_TRANSACTION_MARKER, () + + +def _require_reference_transaction_hook(conn: Connection, repository: str) -> None: + _require_git_version() + repo = get_repository(conn, repository) + hook_path = _effective_hook(repo["path"], "reference-transaction") + expected, _, _ = _hook_plan(repository, "reference-transaction") + try: + installed = hook_path.read_text(encoding="utf-8") + except (OSError, UnicodeError): + installed = None + if installed != expected: + raise PushRejected( + f"The QuorumGit reference-transaction hook is missing or modified at " + f"{hook_path}; run `quorumgit hook install --repo {repository}`." + ) + # Git silently ignores a hook that is not an executable regular file. + if not hook_path.is_file() or not os.access(hook_path, os.X_OK): + raise PushRejected( + f"The QuorumGit reference-transaction hook at {hook_path} is not " + f"executable, so Git would skip it; run `quorumgit hook install " + f"--repo {repository}`." + ) + + def _write_hook_atomically(hook_path: Path, content: str) -> None: hook_path.parent.mkdir(parents=True, exist_ok=True) fd, tmp_name = tempfile.mkstemp( @@ -670,63 +1221,70 @@ def _write_hook_atomically(hook_path: Path, content: str) -> None: pass +def _inspect_hook(hook_path: Path, expected: str, marker: str, legacy: tuple[str, ...]) -> str: + """Decide what installing over `hook_path` means; refuse foreign content.""" + if not hook_path.exists(): + return "installed" + try: + existing = hook_path.read_text(encoding="utf-8") + except (OSError, UnicodeError) as exc: + raise GateError(f"Cannot inspect existing hook {hook_path}: {exc}") from exc + if existing == expected: + return "verified" + if existing in legacy: + return "upgraded" + if marker in existing.splitlines()[:3]: + raise GateError( + f"Existing QuorumGit-managed hook differs at {hook_path}; " + "refusing to overwrite it." + ) + raise GateError( + f"Existing hook at {hook_path} is not owned by QuorumGit; refusing to " + "overwrite or silently chain it." + ) + + def install_hook(conn: Connection, repository: str) -> Path: - """Safely install QuorumGit at Git's effective pre-receive hook path. + """Safely install QuorumGit's pre-receive and reference-transaction hooks. - Existing unrelated hooks are never overwritten or silently chained. An + Both go at Git's effective hook paths. Existing unrelated hooks are never + overwritten or silently chained, and every existing hook is inspected + before anything is written, so a refusal leaves both paths untouched. An exact current QuorumGit hook is idempotent; an exact legacy QuorumGit hook - for the same repository is upgraded in place. Any other existing content - is refused so operators must make coexistence explicit. + for the same repository is upgraded in place. Returns the pre-receive path. """ begin_immediate(conn) + _require_git_version() repo = get_repository(conn, repository) common_dir = assert_repository_identity_unique(conn, repo) - hook_path = _effective_pre_receive_hook(repo["path"]) - expected = _hook_script(repository) - action = "installed" - if hook_path.exists(): - try: - existing = hook_path.read_text(encoding="utf-8") - except (OSError, UnicodeError) as exc: - raise GateError( - f"Cannot inspect existing pre-receive hook {hook_path}: {exc}" - ) from exc - if existing == expected: + plans = [] + for name in HOOK_NAMES: + expected, marker, legacy = _hook_plan(repository, name) + hook_path = _effective_hook(repo["path"], name) + plans.append( + (name, hook_path, expected, _inspect_hook(hook_path, expected, marker, legacy)) + ) + + for name, hook_path, expected, action in plans: + if action == "verified": hook_path.chmod(0o755) - action = "verified" - elif existing == _legacy_hook_script(repository): - _write_hook_atomically(hook_path, expected) - action = "upgraded" - elif HOOK_MARKER in existing.splitlines()[:3]: - raise GateError( - f"Existing QuorumGit-managed pre-receive hook differs at " - f"{hook_path}; refusing to overwrite it." - ) else: - raise GateError( - f"Existing pre-receive hook at {hook_path} is not owned by " - "QuorumGit; refusing to overwrite or silently chain it." - ) - else: - _write_hook_atomically(hook_path, expected) - - effective = _effective_pre_receive_hook(repo["path"]) - if effective != hook_path or not hook_path.exists(): - raise GateError( - f"Installed hook is not Git's effective pre-receive hook: {hook_path}" + _write_hook_atomically(hook_path, expected) + effective = _effective_hook(repo["path"], name) + if effective != hook_path or not hook_path.exists(): + raise GateError(f"Installed hook is not Git's effective {name} hook: {hook_path}") + if hook_path.read_text(encoding="utf-8") != expected: + raise GateError(f"Installed {name} hook failed verification: {hook_path}") + audit.record( + conn, + f"hook.{action}", + "repository", + repo["id"], + detail={ + "hook": name, + "path": str(hook_path), + "git_common_dir": str(common_dir), + }, ) - if hook_path.read_text(encoding="utf-8") != expected: - raise GateError(f"Installed pre-receive hook failed verification: {hook_path}") - - audit.record( - conn, - f"hook.{action}", - "repository", - repo["id"], - detail={ - "path": str(hook_path), - "git_common_dir": str(common_dir), - }, - ) - return hook_path + return plans[0][1] diff --git a/src/quorumgit/handoff.py b/src/quorumgit/handoff.py index dd48ce9..09d5abb 100644 --- a/src/quorumgit/handoff.py +++ b/src/quorumgit/handoff.py @@ -37,9 +37,12 @@ def create_handoff( if not record.get(field): raise HandoffError(f"Handoff record requires {field!r}.") task = work.get_task(conn, claim["task_id"]) - work.verify_commit( - task["repository_path"], record["last_commit"], branch=claim["branch"] - ) + record = { + **record, + "last_commit": work.verify_commit( + task["repository_path"], record["last_commit"], branch=claim["branch"] + ), + } to_agent_id = get_agent(conn, to_agent)["id"] if to_agent else None row = conn.execute( diff --git a/src/quorumgit/migrations/003_ref_updates.sql b/src/quorumgit/migrations/003_ref_updates.sql new file mode 100644 index 0000000..dfc0c93 --- /dev/null +++ b/src/quorumgit/migrations/003_ref_updates.sql @@ -0,0 +1,37 @@ +-- Bind a push's pre-receive validation to Git's own reference transaction. +-- pre-receive records each exact ref update it validated. The +-- reference-transaction hook re-validates it and consumes any approval while +-- Git holds the ref locks ("prepared"), then records whether Git committed or +-- aborted the update. An approval is therefore never spent on an update that +-- did not happen, and ownership is re-checked at the last abortable moment. + +CREATE TABLE ref_updates ( + id INTEGER PRIMARY KEY, + repository_id INTEGER NOT NULL REFERENCES repositories(id), + refname TEXT NOT NULL CHECK (refname <> ''), + oldrev TEXT NOT NULL CHECK ( + length(oldrev) BETWEEN 40 AND 64 + AND oldrev NOT GLOB '*[^0-9a-f]*' + ), + newrev TEXT NOT NULL CHECK ( + length(newrev) BETWEEN 40 AND 64 + AND newrev NOT GLOB '*[^0-9a-f]*' + ), + pusher_agent_id INTEGER NOT NULL REFERENCES agents(id), + operation TEXT CHECK ( + operation IS NULL + OR (json_valid(operation) AND json_type(operation) = 'object') + ), + approval_id INTEGER REFERENCES approvals(id), + status TEXT NOT NULL DEFAULT 'validated' + CHECK (status IN ('validated', 'prepared', 'committed', 'aborted')), + created_at INTEGER NOT NULL DEFAULT (unixepoch()), + resolved_at INTEGER, + CHECK (status IN ('validated', 'aborted') OR operation IS NULL OR approval_id IS NOT NULL), + CHECK (status IN ('validated', 'prepared') OR resolved_at IS NOT NULL) +); + +-- quorumgit-statement +CREATE INDEX ref_updates_in_flight + ON ref_updates(repository_id, refname, newrev) + WHERE status IN ('validated', 'prepared'); diff --git a/src/quorumgit/registry.py b/src/quorumgit/registry.py index 1dd329b..aa8d202 100644 --- a/src/quorumgit/registry.py +++ b/src/quorumgit/registry.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os import subprocess from pathlib import Path @@ -63,6 +64,28 @@ def _protected_refs(conn: Connection, repository_id: int) -> list[str]: ] +# Variables Git exports to hooks that select a repository. A lookup of a +# specific path must not inherit them, or every path resolves to the hook's +# repository. +_REPOSITORY_SELECTING_ENV = ( + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_COMMON_DIR", + "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_QUARANTINE_PATH", +) + + +def path_scoped_git_env() -> dict[str, str]: + return { + key: value + for key, value in os.environ.items() + if key not in _REPOSITORY_SELECTING_ENV + } + + def git_common_dir(path: str | Path) -> Path: """Return the canonical Git common directory for a repository path. @@ -75,6 +98,7 @@ def git_common_dir(path: str | Path) -> Path: ["git", "-C", str(repo_path), "rev-parse", "--git-common-dir"], capture_output=True, text=True, + env=path_scoped_git_env(), check=False, ) if result.returncode != 0: @@ -172,7 +196,7 @@ def add_repository( ).fetchone() assert row is not None repo_id = row[0] - for refname in protected_refs or []: + for refname in dict.fromkeys(protected_refs or []): conn.execute( "INSERT INTO protected_refs (repository_id, refname) VALUES (?, ?)", (repo_id, refname), @@ -308,6 +332,8 @@ def add_agent( begin_immediate(conn) if role != DEFAULT_ROLE: require_operator(conn, actor, f"Registering a {role}") + if conn.execute("SELECT 1 FROM agents WHERE name = ?", (name,)).fetchone(): + raise RegistryError(f"Agent is already registered: {name}") row = conn.execute( "INSERT INTO agents (name, role) VALUES (?, ?) RETURNING id", (name, role) ).fetchone() diff --git a/src/quorumgit/store.py b/src/quorumgit/store.py index 3625c2e..69783e4 100644 --- a/src/quorumgit/store.py +++ b/src/quorumgit/store.py @@ -8,7 +8,9 @@ from __future__ import annotations +import functools import json +import re import sqlite3 from collections.abc import Iterator from contextlib import contextmanager @@ -60,6 +62,14 @@ "approvals_consumer_is_not_approver", ) +# Kept byte-identical to the statement existing stores were created with. +SCHEMA_MIGRATIONS_DDL = """ + CREATE TABLE IF NOT EXISTS schema_migrations ( + version TEXT PRIMARY KEY, + applied_at INTEGER NOT NULL DEFAULT (unixepoch()) + ) + """ + class StoreError(RuntimeError): pass @@ -290,14 +300,7 @@ def migrate(target: Config | str | Path) -> list[str]: applied: list[str] = [] try: begin_immediate(conn) - conn.execute( - """ - CREATE TABLE IF NOT EXISTS schema_migrations ( - version TEXT PRIMARY KEY, - applied_at INTEGER NOT NULL DEFAULT (unixepoch()) - ) - """ - ) + conn.execute(SCHEMA_MIGRATIONS_DDL) conn.commit() done = { @@ -336,6 +339,74 @@ def migrate(target: Config | str | Path) -> list[str]: # ------------------------------------------------------------ contract check +def _normalized_sql(sql: str | None) -> str | None: + """Schema SQL with formatting-only whitespace removed.""" + if sql is None: + return None + return re.sub(r" ?([(),;]) ?", r"\1", " ".join(sql.split())) + + +def _schema_objects(conn: Connection) -> dict[tuple[str, str], tuple[str, str | None]]: + """Every user schema object as {(type, name): (table, normalized SQL)}.""" + rows = conn.execute( + "SELECT type, name, tbl_name, sql FROM sqlite_master " + "WHERE name NOT LIKE 'sqlite_%'" + ).fetchall() + return {(row[0], row[1]): (row[2], _normalized_sql(row[3])) for row in rows} + + +@functools.lru_cache(maxsize=1) +def _reference_schema() -> dict[tuple[str, str], tuple[str, str | None]]: + """The schema this QuorumGit build expects: every migration replayed in memory. + + Deriving the fingerprint from the migrations themselves keeps it exact + without a hand-maintained list: each table, index (including the partial + unique indexes that enforce single ownership and single live approvals) + and trigger (including the append-only audit guards) must be present with + the same definition. + """ + conn = sqlite3.connect(":memory:") + try: + conn.execute(SCHEMA_MIGRATIONS_DDL) + for mig in _migration_files(): + for statement in _migration_statements(mig.read_text(encoding="utf-8")): + conn.execute(statement) + return _schema_objects(conn) + finally: + conn.close() + + +def _describe(keys: set[tuple[str, str]]) -> str: + return ", ".join(f"{kind} {name}" for kind, name in sorted(keys)) + + +def _verify_schema_objects(conn: Connection) -> None: + expected = _reference_schema() + actual = _schema_objects(conn) + missing = set(expected) - set(actual) + changed = { + key for key in set(expected) & set(actual) if expected[key] != actual[key] + } + # Extra tables are inert data; an extra index or trigger can change what + # governance writes succeed or what they do, so it is a violation. + unexpected = { + key for key in set(actual) - set(expected) if key[0] != "table" + } + problems = [] + if missing: + problems.append(f"missing {_describe(missing)}") + if changed: + problems.append(f"altered {_describe(changed)}") + if unexpected: + problems.append(f"unexpected {_describe(unexpected)}") + if problems: + raise ContractViolation( + "Store schema does not match this QuorumGit version: " + + "; ".join(problems) + + ". Governance invariants cannot be guaranteed." + ) + + def verify_contract(target: Config | Connection | str | Path) -> None: """Fail loudly unless the local store satisfies the runtime contract.""" owned = isinstance(target, (Config, str, Path)) @@ -373,6 +444,13 @@ def verify_contract(target: Config | Connection | str | Path) -> None: f"Missing required migrations: {sorted(missing_migrations)}. " "Run `quorumgit init` to apply migrations." ) + unknown_migrations = applied_migrations - required_migrations + if unknown_migrations: + raise ContractViolation( + f"Store has migrations this QuorumGit does not know: " + f"{sorted(unknown_migrations)}. It was written by a newer version." + ) + _verify_schema_objects(conn) missing = set(REQUIRED_TABLES) - tables if missing: diff --git a/src/quorumgit/work.py b/src/quorumgit/work.py index 8df438b..6f88cb8 100644 --- a/src/quorumgit/work.py +++ b/src/quorumgit/work.py @@ -8,6 +8,8 @@ from __future__ import annotations import math +import posixpath +import re import subprocess from typing import Any @@ -43,6 +45,7 @@ def _lease_seconds(lease_hours: float) -> int: def create_task( conn: Connection, repository: str, title: str, objective: str = "" ) -> int: + begin_immediate(conn) repo = get_repository(conn, repository) row = conn.execute( """ @@ -191,13 +194,7 @@ def live_claims_in_repository( ).fetchall() result: list[dict] = [] for row in rows: - scopes = [ - r[0] - for r in conn.execute( - "SELECT path_glob FROM scopes WHERE claim_id = ? ORDER BY id", - (row[0],), - ).fetchall() - ] + scopes = claim_scopes(conn, row[0]) result.append( { "id": row[0], @@ -210,6 +207,16 @@ def live_claims_in_repository( return result +def claim_scopes(conn: Connection, claim_id: int) -> list[str]: + return [ + r[0] + for r in conn.execute( + "SELECT path_glob FROM scopes WHERE claim_id = ? ORDER BY id", + (claim_id,), + ).fetchall() + ] + + def open_handoff_for_task(conn: Connection, task_id: int) -> dict | None: """An open handoff reserving this task, if any.""" row = conn.execute( @@ -304,6 +311,30 @@ def live_claim_for_branch( # --------------------------------------------------------- scope comparison +def normalize_scope(glob: str) -> str: + """Canonical repository-relative spelling of a scope glob. + + `./src/**`, `src//**`, `/src/**` and `src\\**` all name `src/**`; a scope + naming the whole repository (`.`, `/`) becomes `**`. Never raises, so it is + safe on historical rows; validate_scope() rejects unusable input. + """ + text = glob.strip().replace("\\", "/") + if not text: + return "" + text = posixpath.normpath(text).lstrip("/") + return "**" if text in ("", ".") else text + + +def validate_scope(glob: str) -> str: + """Normalize a newly declared scope, refusing ones outside the repository.""" + scope = normalize_scope(glob) + if not scope: + raise ClaimRefused("Scopes must be non-empty path globs.") + if scope == ".." or scope.startswith("../"): + raise ClaimRefused(f"Scope {glob!r} escapes the repository root.") + return scope + + def _glob_prefix(glob: str) -> str: """Literal path prefix of a glob (up to the first wildcard character).""" for i, ch in enumerate(glob): @@ -313,31 +344,90 @@ def _glob_prefix(glob: str) -> str: def scopes_overlap(a: str, b: str) -> bool: - """Conservative overlap test that errs toward flagging.""" - pa, pb = _glob_prefix(a), _glob_prefix(b) + """Conservative overlap test that errs toward flagging. + + Both sides are normalized and compared case-insensitively, because a + case-insensitive checkout (macOS, Windows) treats `Src/` and `src/` as one + directory; on case-sensitive systems this only adds conservative flags. + """ + pa = _glob_prefix(normalize_scope(a)).casefold() + pb = _glob_prefix(normalize_scope(b)).casefold() return pa.startswith(pb) or pb.startswith(pa) +def _glob_regex(scope: str) -> re.Pattern[str]: + """Compile a normalized scope into a full-path matcher. + + `**` spans directories, `*` and `?` stay within one path segment, and + `[...]` is a character class. A scope without wildcards names a file or a + directory, so it also matches everything beneath it. + """ + out: list[str] = [] + i = 0 + while i < len(scope): + ch = scope[i] + if scope.startswith("**/", i): + out.append("(?:.*/)?") + i += 3 + elif scope.startswith("**", i): + out.append(".*") + i += 2 + elif ch == "*": + out.append("[^/]*") + i += 1 + elif ch == "?": + out.append("[^/]") + i += 1 + elif ch == "[" and (end := scope.find("]", i + 2)) != -1 and ( + scope[i + 1 : end] != "!" + ): + body = scope[i + 1 : end] + negate = body.startswith("!") + body = re.sub(r"([\\^\[\]])", r"\\\1", body[1:] if negate else body) + out.append(f"[{'^' if negate else ''}{body}]") + i = end + 1 + else: + out.append(re.escape(ch)) + i += 1 + pattern = "".join(out) + if not any(c in scope for c in "*?["): + pattern += "(?:/.*)?" + return re.compile(pattern + r"\Z", re.DOTALL) + + +def path_in_scopes(path: str, scopes: list[str]) -> bool: + """True when a repository path is covered by any of the declared scopes.""" + return any(_glob_regex(normalize_scope(s)).match(path) for s in scopes) + + def verify_commit( repo_path: str, commit_oid: str, branch: str | None = None -) -> None: - """Require an existing commit, reachable from the branch when it exists.""" - exists = ( - subprocess.run( - [ - "git", - "-C", - str(repo_path), - "cat-file", - "-e", - f"{commit_oid}^{{commit}}", - ], - capture_output=True, - check=False, - ).returncode - == 0 +) -> str: + """Resolve a commit to its full object ID and verify it as a continuation. + + Accepts anything Git resolves to a commit (full or abbreviated OID, a ref) + and returns the canonical lowercase full OID. The commit must exist in the + registered repository and be reachable from the branch when it exists. + """ + spec = commit_oid.strip() + if not spec or spec.startswith("-"): + raise WorkError(f"Invalid commit: {commit_oid!r}") + resolved = subprocess.run( + [ + "git", + "-C", + str(repo_path), + "rev-parse", + "--verify", + "--quiet", + f"{spec}^{{commit}}", + ], + capture_output=True, + text=True, + check=False, ) - if not exists: + oid = resolved.stdout.strip().lower() + if resolved.returncode != 0 or not re.fullmatch(r"[0-9a-f]{40}|[0-9a-f]{64}", oid): raise WorkError( f"Commit {commit_oid} does not exist in the registered repository." ) @@ -367,7 +457,7 @@ def verify_commit( str(repo_path), "merge-base", "--is-ancestor", - commit_oid, + oid, f"refs/heads/{branch}", ], capture_output=True, @@ -380,6 +470,7 @@ def verify_commit( f"Commit {commit_oid} is not reachable from branch " f"{branch!r} in the registered repository." ) + return oid def classify( @@ -458,6 +549,7 @@ def claim_task( agent_row = get_agent(conn, agent) if not scope_globs: raise ClaimRefused("At least one --scope is required to claim a task.") + scope_globs = list(dict.fromkeys(validate_scope(g) for g in scope_globs)) if not via_handoff: pending = open_handoff_for_task(conn, task_id) @@ -732,7 +824,9 @@ def add_checkpoint( f"Claim {claim_id} belongs to {claim['agent']}, not {agent}." ) task = get_task(conn, claim["task_id"]) - verify_commit(task["repository_path"], commit_oid, branch=claim["branch"]) + commit_oid = verify_commit( + task["repository_path"], commit_oid, branch=claim["branch"] + ) row = conn.execute( """ INSERT INTO checkpoints (claim_id, commit_oid, note, detail) @@ -750,3 +844,12 @@ def add_checkpoint( detail={"commit": commit_oid, "note": note}, ) return row[0] + + +def checkpoint_commit(conn: Connection, checkpoint_id: int) -> str: + row = conn.execute( + "SELECT commit_oid FROM checkpoints WHERE id = ?", (checkpoint_id,) + ).fetchone() + if row is None: + raise WorkError(f"No such checkpoint: {checkpoint_id}") + return row[0] diff --git a/tests/test_approval_authority.py b/tests/test_approval_authority.py index 063b4ce..e1ce7c9 100644 --- a/tests/test_approval_authority.py +++ b/tests/test_approval_authority.py @@ -359,7 +359,10 @@ def test_upgrade_leaves_no_agent_with_authority(tmp_path): conn.commit() conn.close() - assert store.migrate(local) == ["004_approval_authority.sql"] + assert store.migrate(local) == [ + "003_ref_updates.sql", + "004_approval_authority.sql", + ] upgraded = store.connect(local) try: assert registry.get_agent(upgraded, "self-approver")["role"] == "worker" diff --git a/tests/test_authorization_binding.py b/tests/test_authorization_binding.py index a19faaa..97edde9 100644 --- a/tests/test_authorization_binding.py +++ b/tests/test_authorization_binding.py @@ -38,6 +38,7 @@ def _apply_001(cfg: Config): UPGRADE_MIGRATIONS = [ "002_approval_identities.sql", + "003_ref_updates.sql", "004_approval_authority.sql", ] @@ -205,7 +206,8 @@ def test_contract_rejects_001_only_store_until_all_migrations_apply(tmp_path): message = ( "Missing required migrations: " - "['002_approval_identities.sql', '004_approval_authority.sql']" + "['002_approval_identities.sql', '003_ref_updates.sql', " + "'004_approval_authority.sql']" ) with pytest.raises(store.ContractViolation, match=re.escape(message)): store.verify_contract(cfg) diff --git a/tests/test_governance_fixes.py b/tests/test_governance_fixes.py new file mode 100644 index 0000000..7b12d5f --- /dev/null +++ b/tests/test_governance_fixes.py @@ -0,0 +1,319 @@ +"""Regression tests for the second review round. + +1. (Superseded by the repository approval policy tests on main.) +2. A successor claim on another branch still gets a fresh worktree; same-branch + continuation is covered by main's worktree continuation tests. +3. Scope globs are normalized before overlap comparison. +4. Pushes to a claimed branch are held to the claim's declared scopes. +5. Commit specs resolve to full OIDs and user errors never print tracebacks. +6. `handoff create --last-commit` is honored when a worktree exists. +""" + +from __future__ import annotations + +import os +import re +import subprocess +import uuid +from pathlib import Path + +import pytest + +from quorumgit import handoff, registry, trees, work +from tests.conftest import make_git_repo +from tests.test_cli_hub import _cli +from tests.test_gate import _commit, _push, _setup + +GIT_ENV = { + **os.environ, + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@localhost", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@localhost", +} + + +def _git(path: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(path), *args], + check=True, + capture_output=True, + text=True, + env=GIT_ENV, + ).stdout.strip() + + +def _local_setup(conn, tmp_path, agents=("a", "b", "op")): + """A registered working repository (local model) plus named agents.""" + suffix = uuid.uuid4().hex[:8] + repo_path = make_git_repo(tmp_path / f"repo-{suffix}") + repo = f"local-{suffix}" + registry.add_repository(conn, repo, repo_path) + names = [f"{agent}-{suffix}" for agent in agents] + for name in names: + registry.add_agent(conn, name) + task = work.create_task(conn, repo, "work") + conn.commit() + return repo, repo_path, task, names + + +def _claim_id(output: str) -> int: + match = re.search(r"claim (\d+) acquired", output) + assert match is not None, output + return int(match.group(1)) + + +def _worktree(conn, claim_id: int) -> dict: + wt = trees.worktree_for_claim(conn, claim_id) + assert wt is not None, f"claim {claim_id} has no worktree" + return wt + + +def _no_traceback(result) -> None: + assert "Traceback" not in result.stderr, result.stderr + assert result.returncode == 1, (result.stdout, result.stderr) + assert result.stderr.startswith("[quorumgit] ERROR:"), result.stderr + + +# ---------------------------------------------------------------- finding 2 + + +def test_reclaim_on_a_different_branch_creates_a_fresh_worktree( + committed_conn, tmp_path, cfg +): + conn = committed_conn + _repo, _path, task, (a, b, _op) = _local_setup(conn, tmp_path) + first = _cli(cfg, "claim", str(task), "--branch", "feat/old", + "--scope", "src/**", agent=a) + assert first.returncode == 0, first.stderr + old_claim = _claim_id(first.stdout) + conn.execute( + "UPDATE claims SET lease_expires_at = unixepoch() - 10 WHERE id = ?", + (old_claim,), + ) + conn.commit() + + second = _cli(cfg, "claim", str(task), "--branch", "feat/new", + "--scope", "src/**", agent=b) + assert second.returncode == 0, second.stderr + assert "inherited" not in second.stdout + new_claim = _claim_id(second.stdout) + new_wt = trees.worktree_for_claim(conn, new_claim) + assert new_wt is not None and new_wt["branch"] == "feat/new" + assert _worktree(conn, old_claim)["removed_at"] is None + + +# ---------------------------------------------------------------- finding 3 + + +@pytest.mark.parametrize( + ("raw", "normalized"), + [ + ("./src/**", "src/**"), + ("src//api/**", "src/api/**"), + ("/src/**", "src/**"), + ("src\\api\\**", "src/api/**"), + (".", "**"), + ("src/../lib/**", "lib/**"), + ], +) +def test_normalize_scope(raw, normalized): + assert work.normalize_scope(raw) == normalized + + +@pytest.mark.parametrize( + ("a", "b"), + [ + ("./src/**", "src/**"), + ("src\\api\\**", "src/api/handlers.py"), + ("Src/**", "src/app.py"), + ("src/../lib/**", "lib/x.py"), + ], +) +def test_differently_spelled_scopes_overlap(a, b): + assert work.scopes_overlap(a, b) + assert work.scopes_overlap(b, a) + + +def test_scopes_escaping_the_repository_are_refused(conn, tmp_path): + _repo, _path, task, (a, _b, _op) = _local_setup(conn, tmp_path) + with pytest.raises(work.ClaimRefused, match="escapes the repository"): + work.claim_task(conn, task, a, "feat/escape", ["../other/**"]) + + +def test_dot_slash_scope_is_classified_overlapping(conn, tmp_path): + repo, _path, task, (a, b, _op) = _local_setup(conn, tmp_path) + work.claim_task(conn, task, a, "feat/one", ["src/**"]) + other = work.create_task(conn, repo, "second") + with pytest.raises(work.ClaimRefused, match="overlap"): + work.claim_task(conn, other, b, "feat/two", ["./src/**"]) + + claim_id, classification, _ = work.claim_task( + conn, other, b, "feat/two", ["./src/**"], override_overlap=True + ) + assert classification == "OVERLAPPING" + assert work.claim_scopes(conn, claim_id) == ["src/**"] + + +# ---------------------------------------------------------------- finding 4 + + +@pytest.mark.parametrize( + ("scope", "path", "inside"), + [ + ("src/**", "src/a/b.py", True), + ("src/**", "srcx/a.py", False), + ("src/*.py", "src/a.py", True), + ("src/*.py", "src/a/b.py", False), + ("docs", "docs/guide.md", True), + ("docs", "docs", True), + ("docs", "docs2/x", False), + ("**/*.md", "README.md", True), + ("**/*.md", "a/b/c.md", True), + ("src/[ab].py", "src/a.py", True), + ("src/[!ab].py", "src/a.py", False), + ("**", "anything/at/all", True), + ], +) +def test_path_in_scopes(scope, path, inside): + assert work.path_in_scopes(path, [scope]) is inside + + +def test_hook_holds_claimed_branch_to_its_scopes(committed_conn, tmp_path, cfg): + conn = committed_conn + repo_name, _hub, clone, a, b = _setup(conn, tmp_path) + task = work.create_task(conn, repo_name, "scoped work") + work.claim_task(conn, task, a, branch="feat/scoped", scope_globs=["src/**"]) + conn.commit() + + _commit(clone, "src/inside.py", branch="feat/scoped") + inside = _push(clone, a, "feat/scoped", cfg=cfg) + assert inside.returncode == 0, inside.stderr + + _commit(clone, "README.md", branch="feat/scoped") + outside = _push(clone, a, "feat/scoped", cfg=cfg) + assert outside.returncode != 0 + assert "outside claim" in outside.stderr + assert "README.md" in outside.stderr + + # Undo the out-of-scope commit; merging another branch's accepted work in + # does not attribute that branch's files to the claim holder. + _git(clone, "reset", "--hard", "HEAD~1") + _commit(clone, "docs/elsewhere.md", branch="feat/base") + base = _push(clone, b, "feat/base", cfg=cfg) + assert base.returncode == 0, base.stderr + _git(clone, "checkout", "feat/scoped") + _git(clone, "merge", "--no-edit", "feat/base") + merged = _push(clone, a, "feat/scoped", cfg=cfg) + assert merged.returncode == 0, merged.stderr + + +def test_hook_scope_check_covers_new_claimed_branches(committed_conn, tmp_path, cfg): + conn = committed_conn + repo_name, _hub, clone, a, _b = _setup(conn, tmp_path) + task = work.create_task(conn, repo_name, "new branch") + work.claim_task(conn, task, a, branch="feat/fresh", scope_globs=["src/**"]) + conn.commit() + + _commit(clone, "docs/outside.md", branch="feat/fresh") + rejected = _push(clone, a, "feat/fresh", cfg=cfg) + assert rejected.returncode != 0 + assert "docs/outside.md" in rejected.stderr + + +# ---------------------------------------------------------------- finding 5 + + +def test_checkpoint_accepts_abbreviated_commit(committed_conn, tmp_path, cfg): + conn = committed_conn + _repo, repo_path, task, (a, _b, _op) = _local_setup(conn, tmp_path) + claimed = _cli(cfg, "claim", str(task), "--branch", "feat/short", + "--scope", "src/**", "--no-worktree", agent=a) + assert claimed.returncode == 0, claimed.stderr + claim_id = _claim_id(claimed.stdout) + full = _git(repo_path, "rev-parse", "main") + + for spec in (full[:9], full.upper(), "main"): + result = _cli(cfg, "checkpoint", str(claim_id), "--commit", spec, agent=a) + assert result.returncode == 0, result.stderr + assert full in result.stdout + + bogus = _cli(cfg, "checkpoint", str(claim_id), "--commit", "nope", agent=a) + _no_traceback(bogus) + assert "does not exist" in bogus.stderr + + +def test_duplicate_registration_is_a_clean_error(committed_conn, tmp_path, cfg): + conn = committed_conn + _repo, _path, _task, (a, _b, _op) = _local_setup(conn, tmp_path) + result = _cli(cfg, "agent", "add", a) + _no_traceback(result) + assert "already registered" in result.stderr + + +def test_duplicate_protected_ref_is_deduplicated(committed_conn, tmp_path): + conn = committed_conn + repo_path = make_git_repo(tmp_path / "dup-refs") + name = f"dup-{uuid.uuid4().hex[:8]}" + registry.add_repository( + conn, name, repo_path, + protected_refs=["refs/heads/main", "refs/heads/main"], + ) + assert registry.get_repository(conn, name)["protected_refs"] == ["refs/heads/main"] + + +@pytest.mark.parametrize("payload", ["{bad", "[1, 2]"]) +def test_invalid_operation_json_is_a_clean_error(cfg, initialized_store, payload): + result = _cli(cfg, "approve", "hash", payload) + _no_traceback(result) + assert "Operation" in result.stderr + + +# ---------------------------------------------------------------- finding 6 + + +def test_explicit_last_commit_wins_over_worktree_head(committed_conn, tmp_path, cfg): + conn = committed_conn + _repo, _path, task, (a, _b, _op) = _local_setup(conn, tmp_path) + claimed = _cli(cfg, "claim", str(task), "--branch", "feat/hand", + "--scope", "src/**", agent=a) + assert claimed.returncode == 0, claimed.stderr + claim_id = _claim_id(claimed.stdout) + wt_path = Path(_worktree(conn, claim_id)["path"]) + base = _git(wt_path, "rev-parse", "HEAD") + (wt_path / "src" / "more.py").write_text("x\n") + _git(wt_path, "add", "-A") + _git(wt_path, "commit", "-m", "later work") + assert _git(wt_path, "rev-parse", "HEAD") != base + + created = _cli(cfg, "handoff", "create", str(claim_id), "--completed", "c", + "--remaining", "r", "--last-commit", base[:10], agent=a) + assert created.returncode == 0, created.stderr + assert base in created.stdout + match = re.search(r"handoff (\d+) created", created.stdout) + assert match is not None, created.stdout + hid = int(match.group(1)) + assert handoff.get_handoff(conn, hid)["record"]["last_commit"] == base + + +def test_handoff_after_worktree_removal_uses_last_commit( + committed_conn, tmp_path, cfg +): + conn = committed_conn + _repo, repo_path, task, (a, _b, _op) = _local_setup(conn, tmp_path) + claimed = _cli(cfg, "claim", str(task), "--branch", "feat/gone", + "--scope", "src/**", agent=a) + assert claimed.returncode == 0, claimed.stderr + claim_id = _claim_id(claimed.stdout) + trees.remove_worktree(conn, claim_id, a) + conn.commit() + + missing = _cli(cfg, "handoff", "create", str(claim_id), "--completed", "c", + "--remaining", "r", agent=a) + assert missing.returncode == 1 + assert "--last-commit" in missing.stderr + + head = _git(repo_path, "rev-parse", "feat/gone") + created = _cli(cfg, "handoff", "create", str(claim_id), "--completed", "c", + "--remaining", "r", "--last-commit", head, agent=a) + assert created.returncode == 0, created.stderr diff --git a/tests/test_transaction_boundary.py b/tests/test_transaction_boundary.py new file mode 100644 index 0000000..08ef36b --- /dev/null +++ b/tests/test_transaction_boundary.py @@ -0,0 +1,652 @@ +"""The push authorization boundary and the store's schema contract. + +pre-receive validates and records each ref update but spends nothing. The +reference-transaction hook re-validates it and consumes any approval at the +`prepared` stage, while Git holds the ref locks and can still abort, then +records whether Git committed or aborted. verify_contract() checks every +schema object the governance rules depend on, not only table names. +""" + +from __future__ import annotations + +import os +import shlex +import subprocess +import sys +from pathlib import Path + +import pytest + +from quorumgit import gate, registry, store, work +from quorumgit.config import Config +from tests.conftest import approve +from tests.test_gate import _commit, _push, _setup + + +def _rev(git_args: list[str], ref: str) -> str: + return subprocess.run( + ["git", *git_args, "rev-parse", ref], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +def _approve_main(conn, repo_name: str, hub: Path, clone: Path) -> tuple[dict, dict]: + """Approve the clone's pending update of main: requested, then an operator votes.""" + op = { + "type": "protected_ref_update", + "repository": repo_name, + "refname": "refs/heads/main", + "oldrev": _rev(["--git-dir", str(hub)], "refs/heads/main"), + "newrev": _rev(["-C", str(clone)], "HEAD"), + } + approval = approve(conn, op, requested_by="requester") + conn.commit() + return op, approval + + +def _install_update_hook(hub: Path, python_body: str) -> Path: + """An extra `update` hook: runs after pre-receive, before the ref moves.""" + script = hub / "hooks" / "extra_update.py" + script.write_text(python_body, encoding="utf-8") + hook = hub / "hooks" / "update" + hook.write_text( + "#!/bin/sh\n" + f"exec {shlex.quote(sys.executable)} {shlex.quote(str(script))} \"$@\"\n", + encoding="utf-8", + ) + hook.chmod(0o755) + return hook + + +def _ref_update_statuses(conn, refname: str, newrev: str) -> list[str]: + conn.commit() + return [ + row[0] + for row in conn.execute( + "SELECT status FROM ref_updates WHERE refname = ? AND newrev = ? " + "ORDER BY id", + (refname, newrev), + ).fetchall() + ] + + +# ----------------------------------------------------- transaction boundary + + +def test_approval_survives_an_update_git_refuses_after_pre_receive( + committed_conn, tmp_path, cfg +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "src/approved.py") + op, approval = _approve_main(conn, repo_name, hub, clone) + + refusing = _install_update_hook(hub, "import sys\nsys.exit(1)\n") + refused = _push(clone, a, "main", cfg=cfg) + assert refused.returncode != 0 + assert _rev(["--git-dir", str(hub)], "refs/heads/main") == op["oldrev"] + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + assert _ref_update_statuses(conn, "refs/heads/main", op["newrev"]) == ["validated"] + + refusing.unlink() + accepted = _push(clone, a, "main", cfg=cfg) + assert accepted.returncode == 0, accepted.stderr + assert _rev(["--git-dir", str(hub)], "refs/heads/main") == op["newrev"] + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + assert _ref_update_statuses(conn, "refs/heads/main", op["newrev"]) == [ + "validated", + "committed", + ] + + +def test_ownership_change_after_pre_receive_aborts_the_update( + committed_conn, tmp_path, cfg +): + conn = committed_conn + repo_name, hub, clone, a, b = _setup(conn, tmp_path) + task = work.create_task(conn, repo_name, "contested") + claim, _, _ = work.claim_task(conn, task, a, branch="feat/race", scope_globs=["src/**"]) + conn.commit() + _commit(clone, "src/first.py", branch="feat/race") + first = _push(clone, a, "feat/race", cfg=cfg) + assert first.returncode == 0, first.stderr + before = _rev(["--git-dir", str(hub)], "refs/heads/feat/race") + + # Between pre-receive and the ref update, a's claim is released and b + # claims the branch. pre-receive saw a as owner; the update must not land. + _install_update_hook( + hub, + "from quorumgit import config, store, work\n" + "cfg = config.load()\n" + "conn = store.connect(cfg)\n" + f"work.release_claim(conn, {claim}, agent={a!r})\n" + f"work.claim_task(conn, {task}, {b!r}, branch='feat/race', scope_globs=['src/**'])\n" + "conn.commit()\n" + "conn.close()\n", + ) + _commit(clone, "src/second.py", branch="feat/race") + raced = _push(clone, a, "feat/race", cfg=cfg) + assert raced.returncode != 0 + assert _rev(["--git-dir", str(hub)], "refs/heads/feat/race") == before + newrev = _rev(["-C", str(clone)], "HEAD") + assert _ref_update_statuses(conn, "refs/heads/feat/race", newrev) == ["aborted"] + rejected = conn.execute( + "SELECT count(*) FROM audit_events WHERE event_type = 'gate.update_prepared' " + "AND json_extract(detail, '$.newrev') = ?", + (newrev,), + ).fetchone() + assert rejected is not None and rejected[0] == 0 + + +def test_aborted_transaction_restores_the_consumed_approval( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "src/restore.py") + # Make the new commit's objects available in the hub without moving main. + staged = _push(clone, a, "HEAD:refs/heads/staging", cfg=cfg) + assert staged.returncode == 0, staged.stderr + op, approval = _approve_main(conn, repo_name, hub, clone) + + # receive-pack runs hooks from inside the repository with GIT_DIR=".". + monkeypatch.chdir(hub) + monkeypatch.setenv("GIT_DIR", ".") + monkeypatch.setenv("QUORUMGIT_AGENT", a) + line = [f"{op['oldrev']} {op['newrev']} refs/heads/main\n"] + assert gate.run_pre_receive(conn, repo_name, line) == 0 + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 0 + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + assert gate.run_reference_transaction(conn, repo_name, "aborted", line) == 0 + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + assert _ref_update_statuses(conn, "refs/heads/main", op["newrev"]) == ["aborted"] + restored = conn.execute( + "SELECT count(*) FROM audit_events WHERE event_type = 'approval.restored' " + "AND entity_id = ?", + (approval["id"],), + ).fetchone() + assert restored is not None and restored[0] == 1 + + +def test_abort_does_not_restore_over_a_newer_live_approval( + committed_conn, tmp_path, cfg, monkeypatch +): + """Restoring must never leave two live approvals for one operation.""" + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "src/conflict.py") + staged = _push(clone, a, "HEAD:refs/heads/staging", cfg=cfg) + assert staged.returncode == 0, staged.stderr + op, first = _approve_main(conn, repo_name, hub, clone) + + monkeypatch.chdir(hub) + monkeypatch.setenv("GIT_DIR", ".") + monkeypatch.setenv("QUORUMGIT_AGENT", a) + line = [f"{op['oldrev']} {op['newrev']} refs/heads/main\n"] + assert gate.run_pre_receive(conn, repo_name, line) == 0 + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 0 + assert gate.get_approval_by_id(conn, first["id"])["status"] == "consumed" + + # While the transaction is in flight the same operation is requested + # again; the consumed instance is not live, so this opens a new one. + second = gate.request_approval(conn, op, requested_by="requester") + conn.commit() + assert second["id"] != first["id"] and second["status"] == "pending" + + assert gate.run_reference_transaction(conn, repo_name, "aborted", line) == 0 + assert gate.get_approval_by_id(conn, first["id"])["status"] == "consumed" + assert gate.get_approval_by_id(conn, second["id"])["status"] == "pending" + live = conn.execute( + "SELECT count(*) FROM approvals WHERE operation_hash = ? " + "AND status IN ('pending', 'approved')", + (gate.operation_hash(op),), + ).fetchone() + assert live is not None and live[0] == 1 + assert _ref_update_statuses(conn, "refs/heads/main", op["newrev"]) == ["aborted"] + events = { + row[0] + for row in conn.execute( + "SELECT event_type FROM audit_events WHERE entity = 'approval' " + "AND entity_id = ?", + (first["id"],), + ).fetchall() + } + assert "approval.restore_skipped" in events + assert "approval.restored" not in events + + +def test_prepared_rejects_an_update_pre_receive_did_not_validate( + committed_conn, tmp_path, monkeypatch +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "docs/one.md", branch="feat/swap") + _commit(clone, "docs/two.md", branch="feat/swap") + validated = _rev(["-C", str(clone)], "HEAD") + swapped = _rev(["-C", str(clone)], "HEAD~1") + + monkeypatch.chdir(hub) + monkeypatch.setenv("GIT_DIR", ".") + monkeypatch.setenv("QUORUMGIT_AGENT", a) + zero = "0" * 40 + conn.execute( + "INSERT INTO ref_updates (repository_id, refname, oldrev, newrev, " + "pusher_agent_id) VALUES (?, 'refs/heads/feat/swap', ?, ?, ?)", + ( + registry.get_repository(conn, repo_name)["id"], + zero, + validated, + registry.get_agent(conn, a)["id"], + ), + ) + conn.commit() + line = [f"{zero} {swapped} refs/heads/feat/swap\n"] + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 1 + + +def test_push_requires_the_reference_transaction_hook(committed_conn, tmp_path, cfg): + conn = committed_conn + _repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + (hub / "hooks" / "reference-transaction").unlink() + _commit(clone, "docs/free.md", branch="feat/nohook") + rejected = _push(clone, a, "feat/nohook", cfg=cfg) + assert rejected.returncode != 0 + assert "reference-transaction hook is missing" in rejected.stderr + + +def test_local_ref_maintenance_in_the_hub_passes_through( + committed_conn, tmp_path, cfg +): + _repo_name, hub, _clone, _a, _b = _setup(committed_conn, tmp_path) + head = _rev(["--git-dir", str(hub)], "refs/heads/main") + for data_dir in (cfg.data_dir, tmp_path / "no-store-here"): + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(data_dir)} + env.pop("QUORUMGIT_AGENT", None) + env["PATH"] = str(Path(sys.executable).parent) + os.pathsep + env["PATH"] + result = subprocess.run( + ["git", "--git-dir", str(hub), "update-ref", + f"refs/heads/local-{data_dir.name}", head], + capture_output=True, + text=True, + env=env, + check=False, + ) + assert result.returncode == 0, result.stderr + + +def test_install_refuses_foreign_reference_transaction_hook_before_writing( + conn, tmp_path +): + from tests.test_hook_integrity import _hub_with_clone + + hub, _clone = _hub_with_clone(tmp_path, "foreign-rt") + repo_name = f"foreign-rt-{tmp_path.name[-8:]}" + registry.add_repository(conn, repo_name, hub) + foreign = hub / "hooks" / "reference-transaction" + foreign.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + foreign.chmod(0o755) + + with pytest.raises(gate.GateError, match="not owned by QuorumGit"): + gate.install_hook(conn, repo_name) + assert not (hub / "hooks" / "pre-receive").exists() + assert foreign.read_text(encoding="utf-8") == "#!/bin/sh\nexit 0\n" + + +def test_install_writes_both_hooks(conn, tmp_path): + from tests.test_hook_integrity import _hub_with_clone + + hub, _clone = _hub_with_clone(tmp_path, "both-hooks") + repo_name = f"both-{tmp_path.name[-8:]}" + registry.add_repository(conn, repo_name, hub) + gate.install_hook(conn, repo_name) + rt = (hub / "hooks" / "reference-transaction").read_text(encoding="utf-8") + assert rt == gate._reference_transaction_script(repo_name) + assert gate.REFERENCE_TRANSACTION_MARKER in rt + + +# ------------------------------------------------------------ schema contract + + +@pytest.fixture() +def fresh_store(tmp_path) -> Config: + cfg = Config(data_dir=tmp_path / "contract", agent=None) + store.migrate(cfg) + store.verify_contract(cfg) + return cfg + + +def _tamper(cfg: Config, *statements: str) -> None: + conn = store.open_connection(cfg) + try: + for statement in statements: + conn.execute(statement) + conn.commit() + finally: + conn.close() + + +@pytest.mark.parametrize( + "trigger", + ["audit_events_no_delete", "audit_events_no_update", "votes_require_registered_voter"], +) +def test_contract_rejects_a_missing_governance_trigger(fresh_store, trigger): + _tamper(fresh_store, f"DROP TRIGGER {trigger}") + with pytest.raises(store.ContractViolation, match=f"missing trigger {trigger}"): + store.verify_contract(fresh_store) + with pytest.raises(store.ContractViolation): + store.connect(fresh_store) + + +def test_contract_rejects_a_weakened_unique_index(fresh_store): + _tamper( + fresh_store, + "DROP INDEX claims_one_active_per_task", + "CREATE INDEX claims_one_active_per_task ON claims(task_id) " + "WHERE released_at IS NULL", + ) + with pytest.raises( + store.ContractViolation, match="altered index claims_one_active_per_task" + ): + store.verify_contract(fresh_store) + + +def test_contract_rejects_an_unexpected_trigger(fresh_store): + _tamper( + fresh_store, + "CREATE TRIGGER auto_approve AFTER INSERT ON approvals BEGIN " + "UPDATE approvals SET status = 'approved' WHERE id = NEW.id; END", + ) + with pytest.raises(store.ContractViolation, match="unexpected trigger auto_approve"): + store.verify_contract(fresh_store) + + +def test_contract_rejects_a_store_from_a_newer_version(fresh_store): + _tamper(fresh_store, "INSERT INTO schema_migrations (version) VALUES ('999_future.sql')") + with pytest.raises(store.ContractViolation, match="newer version"): + store.verify_contract(fresh_store) + + +def test_contract_tolerates_formatting_only_schema_differences(fresh_store): + _tamper( + fresh_store, + "DROP INDEX ref_updates_in_flight", + "CREATE INDEX ref_updates_in_flight ON ref_updates ( repository_id,refname , " + "newrev ) WHERE status IN ('validated','prepared')", + ) + store.verify_contract(fresh_store) + + +def test_identity_lookup_ignores_the_hooks_git_dir(tmp_path, monkeypatch): + from tests.conftest import make_git_repo + + hub = make_git_repo(tmp_path / "hook-repo") + other = make_git_repo(tmp_path / "other-repo") + monkeypatch.setenv("GIT_DIR", str(hub / ".git")) + assert registry.git_common_dir(other) == (other / ".git").resolve() + + +# ----------------------------------------------------- doctor: stuck updates + + +def _stuck_prepared_update(conn, tmp_path, cfg, monkeypatch): + """A protected update that reached `prepared` but never heard its outcome.""" + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "src/stuck.py") + staged = _push(clone, a, "HEAD:refs/heads/staging", cfg=cfg) + assert staged.returncode == 0, staged.stderr + op, approval = _approve_main(conn, repo_name, hub, clone) + + with monkeypatch.context() as scoped: + scoped.chdir(hub) + scoped.setenv("GIT_DIR", ".") + scoped.setenv("QUORUMGIT_AGENT", a) + line = [f"{op['oldrev']} {op['newrev']} refs/heads/main\n"] + assert gate.run_pre_receive(conn, repo_name, line) == 0 + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 0 + update_id = conn.execute( + "SELECT id FROM ref_updates WHERE status = 'prepared' AND newrev = ?", + (op["newrev"],), + ).fetchone()[0] + conn.execute( + "UPDATE ref_updates SET created_at = created_at - ? WHERE id = ?", + (gate.PREPARED_STUCK_AFTER_SECONDS + 60, update_id), + ) + conn.commit() + return hub, op, approval, update_id + + +def _finding(conn, update_id: int, repair: bool = False) -> dict: + findings = [ + f for f in gate.doctor_ref_updates(conn, repair=repair) + if f["ref_update_id"] == update_id + ] + assert len(findings) == 1, findings + return findings[0] + + +def _status(conn, update_id: int) -> str: + return conn.execute( + "SELECT status FROM ref_updates WHERE id = ?", (update_id,) + ).fetchone()[0] + + +def _move_ref(hub: Path, tmp_path: Path, refname: str, value: str) -> None: + # No store at this data dir, so QuorumGit's hook treats this as local + # maintenance and records nothing. + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(tmp_path / "no-store")} + env.pop("QUORUMGIT_AGENT", None) + env["PATH"] = str(Path(sys.executable).parent) + os.pathsep + env["PATH"] + subprocess.run( + ["git", "--git-dir", str(hub), "update-ref", refname, value], + check=True, + capture_output=True, + env=env, + ) + + +def test_doctor_reconciles_a_stuck_update_git_committed( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + hub, op, approval, update_id = _stuck_prepared_update(conn, tmp_path, cfg, monkeypatch) + _move_ref(hub, tmp_path, "refs/heads/main", op["newrev"]) + + detected = _finding(conn, update_id) + assert detected["outcome"] == "committed" and not detected["repaired"] + assert _status(conn, update_id) == "prepared" + + repaired = _finding(conn, update_id, repair=True) + conn.commit() + assert repaired["repaired"] and "error" not in repaired + assert _status(conn, update_id) == "committed" + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + +def test_doctor_reconciles_a_stuck_update_git_aborted( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + _hub, _op, approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + repaired = _finding(conn, update_id, repair=True) + conn.commit() + assert repaired["outcome"] == "aborted" and repaired["repaired"] + assert _status(conn, update_id) == "aborted" + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + events = conn.execute( + "SELECT count(*) FROM audit_events WHERE entity = 'ref_update' " + "AND entity_id = ? AND event_type = 'gate.update_reconciled_aborted'", + (update_id,), + ).fetchone() + assert events is not None and events[0] == 1 + + +def test_doctor_refuses_to_guess_when_the_ref_has_moved_on( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + hub, op, approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + # A later commit that is neither the update's old nor its new value. + pusher = conn.execute( + "SELECT a.name FROM ref_updates u JOIN agents a ON a.id = u.pusher_agent_id " + "WHERE u.id = ?", + (update_id,), + ).fetchone()[0] + clone = tmp_path / "clone" + _commit(clone, "src/later.py") + later = _push(clone, pusher, "HEAD:refs/heads/later", cfg=cfg) + assert later.returncode == 0, later.stderr + elsewhere = _rev(["-C", str(clone)], "HEAD") + assert elsewhere not in (op["oldrev"], op["newrev"]) + _move_ref(hub, tmp_path, "refs/heads/main", elsewhere) + + finding = _finding(conn, update_id, repair=True) + conn.commit() + assert not finding["repaired"] and "moved" in finding["error"] + assert _status(conn, update_id) == "prepared" + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + +def test_doctor_leaves_a_locked_ref_alone(committed_conn, tmp_path, cfg, monkeypatch): + conn = committed_conn + hub, _op, _approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + lock = hub / "refs" / "heads" / "main.lock" + lock.parent.mkdir(parents=True, exist_ok=True) + lock.write_text("", encoding="utf-8") + + finding = _finding(conn, update_id, repair=True) + conn.commit() + assert not finding["repaired"] and "locked" in finding["error"] + assert _status(conn, update_id) == "prepared" + + +def test_doctor_ignores_a_recent_prepared_update( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + _hub, _op, _approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + conn.execute( + "UPDATE ref_updates SET created_at = unixepoch() WHERE id = ?", (update_id,) + ) + conn.commit() + assert all( + f["ref_update_id"] != update_id for f in gate.doctor_ref_updates(conn) + ) + + +def test_doctor_cli_reports_stuck_updates(committed_conn, tmp_path, cfg, monkeypatch): + from tests.test_cli_hub import _cli + + conn = committed_conn + _hub, _op, _approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + result = _cli(cfg, "doctor") + assert result.returncode == 1 + assert f"ref update {update_id} " in result.stdout + assert "stuck_prepared (git aborted) — detected" in result.stdout + + +# ------------------------------------------- review: fail-closed transactions + + +@pytest.mark.skipif(os.name == "nt", reason="Windows has no executable bit") +def test_push_requires_an_executable_transaction_hook(committed_conn, tmp_path, cfg): + conn = committed_conn + _repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + (hub / "hooks" / "reference-transaction").chmod(0o644) + _commit(clone, "docs/noexec.md", branch="feat/noexec") + rejected = _push(clone, a, "feat/noexec", cfg=cfg) + assert rejected.returncode != 0 + assert "not executable" in rejected.stderr + + +def test_identified_transaction_needs_a_fresh_validation( + committed_conn, tmp_path, monkeypatch +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "docs/slow.md", branch="feat/slow") + newrev = _rev(["-C", str(clone)], "HEAD") + zero = "0" * 40 + conn.execute( + "INSERT INTO ref_updates (repository_id, refname, oldrev, newrev, " + "pusher_agent_id, created_at) VALUES (?, 'refs/heads/feat/slow', ?, ?, ?, " + "unixepoch() - ?)", + ( + registry.get_repository(conn, repo_name)["id"], + zero, + newrev, + registry.get_agent(conn, a)["id"], + gate.VALIDATION_TTL_SECONDS + 60, + ), + ) + conn.commit() + monkeypatch.chdir(hub) + monkeypatch.setenv("GIT_DIR", ".") + line = [f"{zero} {newrev} refs/heads/feat/slow\n"] + + # A push delayed past the validation window is rejected, not waved through. + monkeypatch.setenv("QUORUMGIT_AGENT", a) + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 1 + other = [f"{zero} {newrev} refs/heads/feat/never-validated\n"] + assert gate.run_reference_transaction(conn, repo_name, "prepared", other) == 1 + + # Only unidentified transactions are local maintenance. + monkeypatch.delenv("QUORUMGIT_AGENT") + assert gate.run_reference_transaction(conn, repo_name, "prepared", other) == 0 + + +def test_identified_local_ref_change_in_the_hub_is_rejected( + committed_conn, tmp_path, cfg +): + _repo_name, hub, _clone, a, _b = _setup(committed_conn, tmp_path) + head = _rev(["--git-dir", str(hub)], "refs/heads/main") + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(cfg.data_dir), "QUORUMGIT_AGENT": a} + env["PATH"] = str(Path(sys.executable).parent) + os.pathsep + env["PATH"] + result = subprocess.run( + ["git", "--git-dir", str(hub), "update-ref", "refs/heads/sneaky", head], + capture_output=True, + text=True, + env=env, + check=False, + ) + assert result.returncode != 0 + assert "no current pre-receive validation" in result.stderr + + +@pytest.mark.parametrize( + ("agent", "state", "code"), + [("someone", "prepared", 1), (None, "prepared", 0), ("someone", "committed", 0)], +) +def test_missing_store_fails_closed_only_for_identified_prepare( + tmp_path, agent, state, code +): + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(tmp_path / "vanished")} + env.pop("QUORUMGIT_AGENT", None) + if agent: + env["QUORUMGIT_AGENT"] = agent + result = subprocess.run( + [sys.executable, "-m", "quorumgit", "hook", "reference-transaction", + "--repo", "any", state], + input=f"{'0' * 40} {'1' * 40} refs/heads/x\n", + capture_output=True, + text=True, + env=env, + check=False, + ) + assert result.returncode == code, result.stderr