diff --git a/README.md b/README.md index 9d3885d..ad1deff 100644 --- a/README.md +++ b/README.md @@ -204,7 +204,7 @@ push → identify pusher → branch reservations → allowed ref namespace ``` - **Allowed ref namespaces.** Every repository accepts pushes to `refs/heads/` only. Tags, notes, and custom refs are refused until an operator opts in: `quorumgit repo allow-ref myproject refs/tags/ --agent lead` (`--remove` withdraws it). Branch claims, reservations, and scopes apply to `refs/heads/`; protected refs and protected paths apply to every allowed namespace. -One push can need several approvals — a direct push to protected `main` that edits a protected path needs both `protected_ref_update` and `protected_path_update`. They are all-or-nothing: the hook lists every missing approval at once, and consumes none unless all are usable. +One push can need several approvals — a direct push to protected `main` that edits a protected path needs both `protected_ref_update` and `protected_path_update`. Ref-level rules compose too: force-updating a protected ref needs both `protected_ref_update` and `force_update`, and deleting one needs both `protected_ref_update` and `ref_delete`. This preserves stricter per-operation policies for destructive updates instead of letting the protected-ref rule mask them. Required approvals are all-or-nothing: the hook lists every missing approval at once, and consumes none unless all are usable. ### Planning a push: `approve prepare` @@ -262,7 +262,7 @@ Rules that hold no matter what: - **Nobody authorizes themselves.** Only agents whose role the repository's policy names may vote, in either direction. The requester may not vote unless the policy says so, the agent a takeover would benefit may not vote on it, and an agent that voted to approve an operation can never be the one that carries it out — pushes and takeovers by an approver are rejected, and a database trigger refuses the consumption even for writers that bypass the CLI. - **Authority is checked when it is used, not only when it is granted.** Consumption re-derives eligible votes from current roles and policy, so raising the threshold or demoting an approver invalidates approvals that no longer meet it. Such an approval is reopened by the next vote on it, so eligible agents can bring it up to the new requirement instead of requesting it again. - **The repository owns the policy.** New repositories require one vote from an `operator`, and the requester may not vote. Change it with `quorumgit repo policy [--threshold ] [--role ]… [--requester-may-vote | --no-requester-may-vote] [--quorum | --no-quorum]`. `approve request` no longer accepts `--threshold`. -- **Quorum mode scales the threshold with the approvers.** With `--quorum` (on the default or on one operation type's override), an operation needs **2/3 + 1 of the agents eligible to approve it** — agents holding an approving role, minus the requester (unless the policy lets it vote) and a takeover's beneficiary — and never fewer than `--threshold`. For example, with four operators a worker's request needs 3 of the 4, while an operator's own request is decided by the other three and needs all 3 of them. Because eligibility is counted when the approval is used, adding approvers raises the bar for approvals not yet used, just as raising `--threshold` does. +- **Quorum mode scales the threshold with the approvers without consuming every executor.** With `--quorum` (on the default or on one operation type's override), an operation normally needs **2/3 + 1 of the agents eligible to approve it** — agents holding an approving role, minus the requester when requester voting is disabled and minus a takeover's beneficiary — and never fewer than `--threshold`. When the requester is itself eligible to vote, quorum mode leaves one eligible agent outside the required yes-vote set so the separation-of-duties rule cannot make the approval executable by nobody; an explicit fixed `--threshold` can still require unanimity. For example, with four operators a worker's request needs 3 of the 4, while with three operators and requester voting enabled an operator's own request needs 2, leaving one non-approver to carry it out. Because eligibility is counted when the approval is used, adding approvers raises the bar for approvals not yet used, just as raising `--threshold` does. - **Policy can differ per operation type.** A repository's policy is its default; it may override the threshold, the approving roles, whether the requester may vote, or quorum mode for any of `protected_ref_update`, `force_update`, `ref_delete`, `out_of_scope_push`, `protected_path_update`, and `lease_takeover`. Fields an override leaves unset keep following the default, and `--inherit` removes the override. The effective policy is enforced by the same database triggers, and re-derived at consumption like any other policy change. ```bash diff --git a/src/quorumgit/gate.py b/src/quorumgit/gate.py index c2024e1..983caf1 100644 --- a/src/quorumgit/gate.py +++ b/src/quorumgit/gate.py @@ -204,19 +204,31 @@ def required_approvals(conn: Connection, approval: dict, policy: dict) -> int: """Eligible yes votes this approval needs under the current policy. With quorum mode on, the requirement is 2/3 + 1 of the agents eligible to - vote on this particular approval — excluding the requester (unless the - policy lets it vote) and a takeover's beneficiary — and never less than - the policy's fixed threshold. Counting only eligible agents keeps those - exclusions from making approval impossible while other approvers exist. + vote on this particular approval and never less than the policy's fixed + threshold. When the requester is itself eligible to vote, quorum mode + leaves one eligible agent outside the required yes-vote set so approving + the operation cannot consume every possible requester/executor under the + separation-of-duties rule. An explicit fixed threshold may still require + unanimity. + The same requirement decides votes and is re-checked at use time, so an - approval's status never disagrees with whether it can be used; who may - use it is a separate rule (an approver can never carry out its own - approval). + approval's status never disagrees with whether it can be used; an agent + that voted yes still may never carry the approval out. """ if not policy["quorum"]: return policy["threshold"] eligible = _eligible_approver_count(conn, approval, policy) - return max(policy["threshold"], quorum_threshold(eligible)) + quorum = quorum_threshold(eligible) + requester_id = approval.get("requested_by_agent_id") + if requester_id is not None and eligible > 1: + row = conn.execute( + "SELECT id, name, role FROM agents WHERE id = ?", (requester_id,) + ).fetchone() + if row is not None: + requester = {"id": row[0], "name": row[1], "role": row[2]} + if _vote_refusal(approval, policy, requester) is None: + quorum = min(quorum, eligible - 1) + return max(policy["threshold"], quorum) def _authorization_refusal( @@ -606,7 +618,7 @@ def governed_operations( ) if refname in repo["protected_refs"]: operations.append({"type": "protected_ref_update", **base}) - elif deletion: + if deletion: operations.append({"type": "ref_delete", **base}) elif forced: operations.append({"type": "force_update", **base}) diff --git a/tests/test_approval_quorum.py b/tests/test_approval_quorum.py index aaa7214..9b0349b 100644 --- a/tests/test_approval_quorum.py +++ b/tests/test_approval_quorum.py @@ -80,6 +80,27 @@ def test_requester_exclusion_shrinks_the_eligible_set(quorum_store, tmp_path): gate.consume_approval(conn, approval["id"], _op(repo), agent="pusher") +def test_requester_quorum_leaves_an_executor(quorum_store, tmp_path): + """Requester voting cannot make quorum consume every possible executor.""" + _local, conn = quorum_store + _operators(conn, "a", "b", "c") + repo = register_repo(conn, tmp_path / "q-requester-executor") + registry.set_approval_policy( + conn, repo, actor="a", quorum=True, requester_may_vote=True + ) + + op = _op(repo) + approval = gate.request_approval(conn, op, "a") + # Raw 2/3 + 1 over three eligible operators is three, but requiring all + # three would leave no non-approver able to consume the approval. Quorum + # mode therefore leaves one eligible requester/executor outside the + # required yes-vote set unless a fixed threshold explicitly says otherwise. + assert approval["threshold"] == 2 + gate.vote(conn, approval["id"], "b", True) + assert gate.vote(conn, approval["id"], "c", True)["status"] == "approved" + gate.consume_approval(conn, approval["id"], op, agent="a") + + def test_status_and_usability_use_the_same_requirement(quorum_store, tmp_path): """A non-voting operator carrying out the operation does not lower quorum. diff --git a/tests/test_content_governance.py b/tests/test_content_governance.py index 269b39a..804a882 100644 --- a/tests/test_content_governance.py +++ b/tests/test_content_governance.py @@ -499,11 +499,30 @@ def test_prepare_reports_refusals_without_writing(committed_conn, tmp_path, cfg) deletion = gate.prepare_push(conn, repo_name, "main", clone, None) assert [e["operation"]["type"] for e in deletion["operations"]] == [ - "protected_ref_update" + "protected_ref_update", + "ref_delete", ] assert deletion["paths"] == [] +def test_protected_ref_force_update_needs_both_policies( + committed_conn, tmp_path, cfg +): + conn = committed_conn + repo_name, _hub, clone, _a, _b = _setup(conn, tmp_path) + + # Build an unrelated commit so replacing protected main is a force update. + _git(clone, "checkout", "--orphan", "rewrite") + _git(clone, "rm", "-rf", ".") + _edit(clone, "replacement.txt", message="replacement") + + plan = gate.prepare_push(conn, repo_name, "main", clone) + assert [e["operation"]["type"] for e in plan["operations"]] == [ + "protected_ref_update", + "force_update", + ] + + # ------------------------------------------------- review regressions (#23)