diff --git a/README.md b/README.md index 7d45d18..a9cf1d1 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ 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', '005_content_governance.sql', '006_ref_updates.sql', '007_ref_update_scope_paths.sql', '008_operation_policies.sql'] +migrations applied: ['001_core.sql', '002_approval_identities.sql', '004_approval_authority.sql', '005_content_governance.sql', '006_ref_updates.sql', '007_ref_update_scope_paths.sql', '008_operation_policies.sql', '009_approval_quorum.sql'] contract: ok ``` @@ -261,8 +261,9 @@ Rules that hold no matter what: - **A consumed approval is spent, not blacklisted.** Consumption moves the approval to a terminal `consumed` state (with `consumed_at`) rather than reusing `denied`, and only one *live* (`pending` or `approved`) approval may exist per operation hash. The same operation can therefore be requested and approved again later as a new approval instance — which matters for takeovers, whose payload is stable and legitimately repeatable. What is never possible is one approval authorizing twice. - **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]`. `approve request` no longer accepts `--threshold`. -- **Policy can differ per operation type.** A repository's policy is its default; it may override the threshold, the approving roles, or whether the requester may vote 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. +- **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. +- **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 quorumgit repo policy myproject --operation force_update --threshold 2 --agent lead @@ -297,7 +298,7 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c | `quorumgit repo add [--protected-ref ]… [--protected-path ]…` | Register a repository | | `quorumgit agent add [--role worker\|reviewer\|operator]` | Register an agent identity (non-workers need an operator once one exists) | | `quorumgit agent role ` | Change an agent's role (operator only, after bootstrap) | -| `quorumgit repo policy [--operation [--inherit]] [--threshold ] [--role ]… [--[no-]requester-may-vote]` | Show or change a repository's default approval policy, or its override for one operation type (changes are operator only); also shows protected refs, protected paths, and allowed ref namespaces | +| `quorumgit repo policy [--operation [--inherit]] [--threshold ] [--role ]… [--[no-]requester-may-vote] [--[no-]quorum]` | Show or change a repository's default approval policy, or its override for one operation type (changes are operator only); also shows protected refs, protected paths, and allowed ref namespaces | | `quorumgit repo protect-path [--remove]` | Require an approval for any push changing matching paths (operator only) | | `quorumgit repo allow-ref [--remove]` | Allow pushes to a ref namespace such as `refs/tags/` (operator only) | | `quorumgit task add --repo --title [--objective ]` | Create a task | diff --git a/src/quorumgit/cli.py b/src/quorumgit/cli.py index f00a670..4e4ac64 100644 --- a/src/quorumgit/cli.py +++ b/src/quorumgit/cli.py @@ -149,9 +149,23 @@ def cmd_repo_list(args, cfg) -> int: return 0 -def _describe_policy(policy: dict) -> str: +def _approver_count(agents: list[dict], policy: dict) -> int: + return sum(1 for agent in agents if agent["role"] in policy["roles"]) + + +def _describe_threshold(policy: dict, approvers: int) -> str: + if not policy["quorum"]: + return str(policy["threshold"]) + return ( + "2/3 + 1 of the agents eligible to approve each operation, at least " + f"{policy['threshold']} ({approvers} agent(s) hold an approving role; " + f"{gate.quorum_threshold(approvers)} of {approvers} before exclusions)" + ) + + +def _describe_policy(policy: dict, approvers: int) -> str: return ( - f"threshold {policy['threshold']}; roles " + f"threshold {_describe_threshold(policy, approvers)}; roles " f"{', '.join(policy['roles']) or 'none'}; requester may vote: " f"{'yes' if policy['requester_may_vote'] else 'no'}" ) @@ -163,6 +177,7 @@ def cmd_repo_policy(args, cfg) -> int: args.threshold is not None or args.role or args.requester_may_vote is not None + or args.quorum is not None or args.inherit ) if changing: @@ -173,6 +188,7 @@ def cmd_repo_policy(args, cfg) -> int: threshold=args.threshold, roles=args.role or None, requester_may_vote=args.requester_may_vote, + quorum=args.quorum, operation_type=args.operation, inherit=args.inherit, ) @@ -184,18 +200,28 @@ def cmd_repo_policy(args, cfg) -> int: operation_type: registry.approval_policy(conn, repo["id"], operation_type) for operation_type in registry.OPERATION_TYPES } + agents = registry.list_agents(conn) print(f"repository: {args.name}") if args.operation is not None: state = "override" if args.operation in overridden else "inherits default" - print(f"{args.operation} ({state}): {_describe_policy(effective[args.operation])}") + policy = effective[args.operation] + print( + f"{args.operation} ({state}): " + f"{_describe_policy(policy, _approver_count(agents, policy))}" + ) return 0 - print(f"approval threshold: {default['threshold']}") + approvers = _approver_count(agents, default) + print(f"approval threshold: {_describe_threshold(default, approvers)}") print(f"approving roles: {', '.join(default['roles'])}") print(f"requester may vote: {'yes' if default['requester_may_vote'] else 'no'}") if overridden: print("per-operation overrides:") for operation_type in overridden: - print(f" {operation_type}: {_describe_policy(effective[operation_type])}") + policy = effective[operation_type] + print( + f" {operation_type}: " + f"{_describe_policy(policy, _approver_count(agents, policy))}" + ) else: print("per-operation overrides: none") with store.session(cfg) as conn: @@ -723,6 +749,10 @@ def add(name, fn, configure=None, parent=None): "the current set)"), sp.add_argument("--requester-may-vote", dest="requester_may_vote", action=argparse.BooleanOptionalAction, default=None), + sp.add_argument("--quorum", action=argparse.BooleanOptionalAction, + default=None, + help="require 2/3 + 1 of the eligible approvers " + "(never fewer than --threshold)"), sp.add_argument("--operation", choices=registry.OPERATION_TYPES, help="show or change the override for one operation " "type instead of the repository default"), diff --git a/src/quorumgit/gate.py b/src/quorumgit/gate.py index 8a99846..15afa82 100644 --- a/src/quorumgit/gate.py +++ b/src/quorumgit/gate.py @@ -184,6 +184,41 @@ def _eligible_approvals( ) +def quorum_threshold(eligible_approvers: int) -> int: + """Integer-stable 2/3 + 1 quorum over the eligible approvers.""" + return (2 * eligible_approvers) // 3 + 1 + + +def _eligible_approver_count(conn: Connection, approval: dict, policy: dict) -> int: + """Registered agents who could cast a counting vote on this approval.""" + rows = conn.execute("SELECT id, name, role FROM agents").fetchall() + return sum( + 1 + for agent_id, name, role in rows + if _vote_refusal(approval, policy, {"id": agent_id, "name": name, "role": role}) + is None + ) + + +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. + 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). + """ + if not policy["quorum"]: + return policy["threshold"] + eligible = _eligible_approver_count(conn, approval, policy) + return max(policy["threshold"], quorum_threshold(eligible)) + + def _authorization_refusal( conn: Connection, approval: dict, consumer: dict | None ) -> str | None: @@ -201,10 +236,12 @@ def _authorization_refusal( "carry it out." ) eligible = _eligible_approvals(conn, approval, policy) - if eligible < policy["threshold"]: + required = required_approvals(conn, approval, policy) + if eligible < required: + basis = " (2/3 + 1 of eligible approvers)" if policy["quorum"] else "" return ( f"Approval {approval['id']} has {eligible} eligible approval(s); the " - f"repository policy now requires {policy['threshold']}. Eligible " + f"repository policy now requires {required}{basis}. Eligible " f"agents can vote on approval {approval['id']} to reach it." ) return None @@ -243,7 +280,11 @@ def request_approval( if existing is not None: return _approval_dict(existing) - threshold = approval_policy(conn, repo["id"], operation.get("type"))["threshold"] + threshold = required_approvals( + conn, + {"requested_by_agent_id": requester["id"], "operation": operation}, + approval_policy(conn, repo["id"], operation.get("type")), + ) row = conn.execute( """ INSERT INTO approvals ( @@ -316,7 +357,8 @@ def vote(conn: Connection, approval_id: int, voter: str, approve: bool) -> dict: policy = _policy_for(conn, approval) stale = ( approval["status"] == "approved" - and _eligible_approvals(conn, approval, policy) < policy["threshold"] + and _eligible_approvals(conn, approval, policy) + < required_approvals(conn, approval, policy) ) if approval["status"] == "approved" and not stale: raise GateError(f"Approval {approval_id} is already approved.") @@ -366,7 +408,9 @@ def vote(conn: Connection, approval_id: int, voter: str, approve: bool) -> dict: assert denials is not None if denials[0] > 0: new_status = "denied" - elif _eligible_approvals(conn, approval, policy) >= policy["threshold"]: + elif _eligible_approvals(conn, approval, policy) >= required_approvals( + conn, approval, policy + ): new_status = "approved" else: new_status = "pending" diff --git a/src/quorumgit/migrations/009_approval_quorum.sql b/src/quorumgit/migrations/009_approval_quorum.sql new file mode 100644 index 0000000..2509dcb --- /dev/null +++ b/src/quorumgit/migrations/009_approval_quorum.sql @@ -0,0 +1,16 @@ +-- Optional quorum mode for approval policy. When on, the number of eligible +-- yes votes required is 2/3 + 1 of the agents eligible to approve that +-- particular approval (never less than approval_threshold), instead of the +-- fixed approval_threshold alone. +-- +-- The repository default is on or off; a per-operation override may turn it +-- on or off for one operation type, and NULL there inherits the default. + +ALTER TABLE repositories + ADD COLUMN approval_quorum INTEGER NOT NULL DEFAULT 0 + CHECK (approval_quorum IN (0, 1)); + +-- quorumgit-statement +ALTER TABLE operation_approval_policies + ADD COLUMN approval_quorum INTEGER + CHECK (approval_quorum IS NULL OR approval_quorum IN (0, 1)); diff --git a/src/quorumgit/registry.py b/src/quorumgit/registry.py index 28dab8a..e26f532 100644 --- a/src/quorumgit/registry.py +++ b/src/quorumgit/registry.py @@ -287,8 +287,8 @@ def _validate_operation_type(operation_type: str) -> str: def _default_policy(conn: Connection, repository_id: int) -> dict: row = conn.execute( - "SELECT approval_threshold, requester_may_vote FROM repositories " - "WHERE id = ?", + "SELECT approval_threshold, requester_may_vote, approval_quorum " + "FROM repositories WHERE id = ?", (repository_id,), ).fetchone() if row is None: @@ -305,15 +305,16 @@ def _default_policy(conn: Connection, repository_id: int) -> dict: "threshold": row[0], "requester_may_vote": bool(row[1]), "roles": roles, + "quorum": bool(row[2]), } def operation_policy_overrides(conn: Connection, repository_id: int) -> dict: """Per-operation overrides as stored: None or [] means "inherit".""" overrides: dict[str, dict] = {} - for policy_id, operation_type, threshold, requester_may_vote in conn.execute( - "SELECT id, operation_type, approval_threshold, requester_may_vote " - "FROM operation_approval_policies WHERE repository_id = ? " + for policy_id, operation_type, threshold, requester_may_vote, quorum in conn.execute( + "SELECT id, operation_type, approval_threshold, requester_may_vote, " + "approval_quorum FROM operation_approval_policies WHERE repository_id = ? " "ORDER BY operation_type", (repository_id,), ).fetchall(): @@ -322,6 +323,7 @@ def operation_policy_overrides(conn: Connection, repository_id: int) -> dict: "requester_may_vote": ( None if requester_may_vote is None else bool(requester_may_vote) ), + "quorum": None if quorum is None else bool(quorum), "roles": [ r[0] for r in conn.execute( @@ -356,6 +358,9 @@ def approval_policy( else override["requester_may_vote"] ), "roles": override["roles"] or policy["roles"], + "quorum": ( + policy["quorum"] if override["quorum"] is None else override["quorum"] + ), } @@ -374,6 +379,7 @@ def set_approval_policy( threshold: int | None = None, roles: list[str] | None = None, requester_may_vote: bool | None = None, + quorum: bool | None = None, operation_type: str | None = None, inherit: bool = False, ) -> dict: @@ -394,7 +400,9 @@ def set_approval_policy( if inherit: raise RegistryError("--inherit applies only to an --operation override.") before = _default_policy(conn, repo["id"]) - _set_default_policy(conn, repo["id"], threshold, wanted, requester_may_vote) + _set_default_policy( + conn, repo["id"], threshold, wanted, requester_may_vote, quorum + ) after = _default_policy(conn, repo["id"]) detail: dict = {"before": before, "after": after} else: @@ -403,7 +411,7 @@ def set_approval_policy( before = overrides.get(operation_type) _set_operation_override( conn, repo["id"], operation_type, threshold, wanted, - requester_may_vote, inherit, + requester_may_vote, quorum, inherit, ) after = operation_policy_overrides(conn, repo["id"]).get(operation_type) detail = {"operation_type": operation_type, "before": before, "after": after} @@ -425,6 +433,7 @@ def _set_default_policy( threshold: int | None, roles: list[str] | None, requester_may_vote: bool | None, + quorum: bool | None, ) -> None: if threshold is not None: conn.execute( @@ -447,6 +456,11 @@ def _set_default_policy( "UPDATE repositories SET requester_may_vote = ? WHERE id = ?", (1 if requester_may_vote else 0, repository_id), ) + if quorum is not None: + conn.execute( + "UPDATE repositories SET approval_quorum = ? WHERE id = ?", + (1 if quorum else 0, repository_id), + ) def _set_operation_override( @@ -456,13 +470,19 @@ def _set_operation_override( threshold: int | None, roles: list[str] | None, requester_may_vote: bool | None, + quorum: bool | None, inherit: bool, ) -> None: if inherit: - if threshold is not None or roles is not None or requester_may_vote is not None: + if ( + threshold is not None + or roles is not None + or requester_may_vote is not None + or quorum is not None + ): raise RegistryError( "--inherit removes the override; it cannot be combined with " - "--threshold, --role, or --requester-may-vote." + "--threshold, --role, --requester-may-vote, or --quorum." ) conn.execute( "DELETE FROM operation_approval_policies " @@ -494,6 +514,12 @@ def _set_operation_override( "WHERE id = ?", (1 if requester_may_vote else 0, policy_id), ) + if quorum is not None: + conn.execute( + "UPDATE operation_approval_policies SET approval_quorum = ? " + "WHERE id = ?", + (1 if quorum else 0, policy_id), + ) if roles is not None: conn.execute( "DELETE FROM operation_approval_roles WHERE policy_id = ?", (policy_id,) diff --git a/tests/test_approval_authority.py b/tests/test_approval_authority.py index d7a74f7..c3387db 100644 --- a/tests/test_approval_authority.py +++ b/tests/test_approval_authority.py @@ -365,6 +365,7 @@ def test_upgrade_leaves_no_agent_with_authority(tmp_path): "006_ref_updates.sql", "007_ref_update_scope_paths.sql", "008_operation_policies.sql", + "009_approval_quorum.sql", ] upgraded = store.connect(local) try: @@ -374,6 +375,7 @@ def test_upgrade_leaves_no_agent_with_authority(tmp_path): "threshold": 1, "requester_may_vote": False, "roles": ["operator"], + "quorum": False, } backfilled = gate.get_approval_by_id(upgraded, approval_id) assert backfilled["repository_id"] == repo["id"] diff --git a/tests/test_approval_quorum.py b/tests/test_approval_quorum.py new file mode 100644 index 0000000..aaa7214 --- /dev/null +++ b/tests/test_approval_quorum.py @@ -0,0 +1,312 @@ +"""Quorum mode: 2/3 + 1 of the agents eligible to approve an operation. + +Roles are global, so these tests use an isolated store where the number of +agents holding each role is known exactly. +""" + +from __future__ import annotations + +import os +import subprocess +import sys + +import pytest + +from quorumgit import gate, registry, store +from quorumgit.config import Config +from tests.conftest import register_repo + + +@pytest.fixture() +def quorum_store(tmp_path): + local = Config(data_dir=tmp_path / "quorum-data", agent=None) + store.migrate(local) + conn = store.connect(local) + try: + yield local, conn + finally: + conn.close() + store.destroy(local) + + +def _operators(conn, *names: str) -> None: + """Register operators; the first is bootstrapped, the rest added by it.""" + registry.add_agent(conn, names[0]) + registry.set_agent_role(conn, names[0], "operator", actor=None) + for name in names[1:]: + registry.add_agent(conn, name, role="operator", actor=names[0]) + + +def _op(repo: str, n: int = 0) -> dict: + return {"type": "protected_ref_update", "repository": repo, "n": n} + + +@pytest.mark.parametrize( + ("eligible", "required"), + [(0, 1), (1, 1), (2, 2), (3, 3), (4, 3), (5, 4), (6, 5), (7, 5), (9, 7)], +) +def test_quorum_threshold_is_two_thirds_plus_one(eligible, required): + assert gate.quorum_threshold(eligible) == required + + +def test_quorum_counts_eligible_approvers(quorum_store, tmp_path): + _local, conn = quorum_store + _operators(conn, "a", "b", "c", "d") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q1") + registry.set_approval_policy(conn, repo, actor="a", quorum=True) + + # Four operators may vote on a worker's request: 3 of 4 are needed. + approval = gate.request_approval(conn, _op(repo), "worker") + assert approval["threshold"] == 3 + assert gate.vote(conn, approval["id"], "a", True)["status"] == "pending" + assert gate.vote(conn, approval["id"], "b", True)["status"] == "pending" + assert gate.vote(conn, approval["id"], "c", True)["status"] == "approved" + gate.consume_approval(conn, approval["id"], _op(repo), agent="worker") + + +def test_requester_exclusion_shrinks_the_eligible_set(quorum_store, tmp_path): + """An operator's own request is decided by the others: 2 of 2, not 3 of 3.""" + _local, conn = quorum_store + _operators(conn, "a", "b", "c") + registry.add_agent(conn, "pusher") + repo = register_repo(conn, tmp_path / "q2") + registry.set_approval_policy(conn, repo, actor="a", quorum=True) + + approval = gate.request_approval(conn, _op(repo), "a") + 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(repo), agent="pusher") + + +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. + + Five operators are eligible on a worker's request, so 4 are needed. With 3 + yes votes the approval is pending for everyone, including an operator who + did not vote; it becomes usable exactly when it becomes approved. + """ + _local, conn = quorum_store + _operators(conn, "a", "b", "c", "d", "e") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q-consumer") + registry.set_approval_policy(conn, repo, actor="a", quorum=True) + + op = _op(repo) + approval = gate.request_approval(conn, op, "worker") + assert approval["threshold"] == 4 + for voter in ("a", "b", "c"): + approval = gate.vote(conn, approval["id"], voter, True) + assert approval["status"] == "pending" + assert gate.approved_instance(conn, op, consumer="d") is None + with pytest.raises(gate.GateError, match="not consumable"): + gate.consume_approval(conn, approval["id"], op, agent="d") + + assert gate.vote(conn, approval["id"], "e", True)["status"] == "approved" + assert gate.approved_instance(conn, op, consumer="d") is not None + gate.consume_approval(conn, approval["id"], op, agent="d") + + +def test_fixed_threshold_is_a_floor(quorum_store, tmp_path): + _local, conn = quorum_store + _operators(conn, "a", "b", "c", "d") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q3") + registry.set_approval_policy(conn, repo, actor="a", quorum=True, threshold=4) + approval = gate.request_approval(conn, _op(repo), "worker") + assert approval["threshold"] == 4 + + +def test_new_approver_raises_quorum_for_unused_approvals(quorum_store, tmp_path): + _local, conn = quorum_store + _operators(conn, "a", "b") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q4") + registry.set_approval_policy(conn, repo, actor="a", quorum=True) + + op = _op(repo) + approval = gate.request_approval(conn, op, "worker") + gate.vote(conn, approval["id"], "a", True) + assert gate.vote(conn, approval["id"], "b", True)["status"] == "approved" + + # Two more operators: 2 of 4 no longer meets 2/3 + 1 (3). + registry.add_agent(conn, "c", role="operator", actor="a") + registry.add_agent(conn, "d", role="operator", actor="a") + assert not gate.is_approved(conn, op) + with pytest.raises(gate.GateError, match=r"now requires 3 \(2/3 \+ 1"): + gate.consume_approval(conn, approval["id"], op, agent="worker") + + +def test_quorum_respects_approving_roles(quorum_store, tmp_path): + _local, conn = quorum_store + _operators(conn, "a", "b") + registry.add_agent(conn, "r1", role="reviewer", actor="a") + registry.add_agent(conn, "r2", role="reviewer", actor="a") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q5") + registry.set_approval_policy( + conn, repo, actor="a", quorum=True, roles=["operator", "reviewer"] + ) + # Operators and reviewers both count: 4 eligible, so 3 are needed. + approval = gate.request_approval(conn, _op(repo), "worker") + assert approval["threshold"] == 3 + + registry.set_approval_policy(conn, repo, actor="a", roles=["reviewer"]) + other = gate.request_approval(conn, _op(repo, 1), "worker") + assert other["threshold"] == 2 + + +def test_quorum_off_keeps_fixed_threshold(quorum_store, tmp_path): + _local, conn = quorum_store + _operators(conn, "a", "b", "c", "d") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q6") + approval = gate.request_approval(conn, _op(repo), "worker") + assert approval["threshold"] == 1 + assert gate.vote(conn, approval["id"], "a", True)["status"] == "approved" + + +def test_quorum_change_is_audited_and_operator_only(quorum_store, tmp_path): + _local, conn = quorum_store + _operators(conn, "a") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q7") + conn.commit() + with pytest.raises(registry.RegistryError, match="requires an operator"): + registry.set_approval_policy(conn, repo, actor="worker", quorum=True) + conn.rollback() + registry.set_approval_policy(conn, repo, actor="a", quorum=True) + row = conn.execute( + "SELECT detail FROM audit_events WHERE event_type = " + "'repository.policy_changed' ORDER BY id DESC LIMIT 1" + ).fetchone() + detail = store.json_loads(row[0], {}) + assert detail["before"]["quorum"] is False + assert detail["after"]["quorum"] is True + + +def test_cli_quorum_policy(quorum_store, tmp_path): + local, conn = quorum_store + _operators(conn, "a", "b", "c") + repo = register_repo(conn, tmp_path / "q8") + conn.commit() + + def cli(*args): + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(local.data_dir)} + env.pop("QUORUMGIT_AGENT", None) + return subprocess.run( + [sys.executable, "-m", "quorumgit", *args], + capture_output=True, text=True, env=env, check=False, + ) + + shown = cli("repo", "policy", repo, "--quorum", "--agent", "a") + assert shown.returncode == 0, shown.stderr + assert "2/3 + 1" in shown.stdout + assert "3 agent(s) hold an approving role" in shown.stdout + + off = cli("repo", "policy", repo, "--no-quorum", "--agent", "a") + assert "approval threshold: 1" in off.stdout + + +def _typed_op(repo: str, op_type: str, n: int = 0) -> dict: + return {"type": op_type, "repository": repo, "n": n} + + +def test_override_turns_quorum_on_for_one_operation_type(quorum_store, tmp_path): + _local, conn = quorum_store + _operators(conn, "a", "b", "c", "d") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q-override-on") + repo_id = registry.get_repository(conn, repo)["id"] + registry.set_approval_policy( + conn, repo, actor="a", operation_type="force_update", quorum=True + ) + + assert registry.approval_policy(conn, repo_id)["quorum"] is False + assert registry.approval_policy(conn, repo_id, "force_update")["quorum"] is True + forced = gate.request_approval(conn, _typed_op(repo, "force_update"), "worker") + assert forced["threshold"] == 3 + plain = gate.request_approval( + conn, _typed_op(repo, "protected_ref_update"), "worker" + ) + assert plain["threshold"] == 1 + + +def test_override_turns_quorum_off_and_unset_inherits(quorum_store, tmp_path): + _local, conn = quorum_store + _operators(conn, "a", "b", "c", "d") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q-override-off") + repo_id = registry.get_repository(conn, repo)["id"] + registry.set_approval_policy(conn, repo, actor="a", quorum=True) + registry.set_approval_policy( + conn, repo, actor="a", operation_type="out_of_scope_push", quorum=False + ) + # An override that sets only the threshold keeps inheriting quorum mode. + registry.set_approval_policy( + conn, repo, actor="a", operation_type="ref_delete", threshold=2 + ) + + overrides = registry.operation_policy_overrides(conn, repo_id) + assert overrides["out_of_scope_push"]["quorum"] is False + assert overrides["ref_delete"]["quorum"] is None + unscoped = gate.request_approval( + conn, _typed_op(repo, "out_of_scope_push"), "worker" + ) + assert unscoped["threshold"] == 1 + delete = gate.request_approval(conn, _typed_op(repo, "ref_delete"), "worker") + assert delete["threshold"] == 3 + + with pytest.raises(registry.RegistryError, match="--quorum"): + registry.set_approval_policy( + conn, repo, actor="a", operation_type="ref_delete", + inherit=True, quorum=True, + ) + + +def test_stale_quorum_approval_reopens_for_the_missing_votes(quorum_store, tmp_path): + """New approvers raise quorum; the next eligible vote reopens and decides it.""" + _local, conn = quorum_store + _operators(conn, "a", "b") + registry.add_agent(conn, "worker") + repo = register_repo(conn, tmp_path / "q-reopen") + registry.set_approval_policy(conn, repo, actor="a", quorum=True) + + op = _op(repo) + approval = gate.request_approval(conn, op, "worker") + gate.vote(conn, approval["id"], "a", True) + assert gate.vote(conn, approval["id"], "b", True)["status"] == "approved" + + # With four operators, 2 of 4 is below 2/3 + 1 (3): the approval is stale. + registry.add_agent(conn, "c", role="operator", actor="a") + registry.add_agent(conn, "d", role="operator", actor="a") + assert gate.vote(conn, approval["id"], "c", True)["status"] == "approved" + gate.consume_approval(conn, approval["id"], op, agent="worker") + + +def test_cli_shows_quorum_on_an_override(quorum_store, tmp_path): + local, conn = quorum_store + _operators(conn, "a", "b", "c") + repo = register_repo(conn, tmp_path / "q-cli-override") + conn.commit() + + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(local.data_dir)} + env.pop("QUORUMGIT_AGENT", None) + + def cli(*args: str) -> subprocess.CompletedProcess: + result = subprocess.run( + [sys.executable, "-m", "quorumgit", *args], + env=env, capture_output=True, text=True, check=False, + ) + assert result.returncode == 0, result.stderr + return result + + shown = cli( + "repo", "policy", repo, "--operation", "force_update", "--quorum", + "--agent", "a", + ) + assert "force_update (override): threshold 2/3 + 1" in shown.stdout + listing = cli("repo", "policy", repo) + assert "approval threshold: 1" in listing.stdout + assert " force_update: threshold 2/3 + 1" in listing.stdout diff --git a/tests/test_authorization_binding.py b/tests/test_authorization_binding.py index ac5b173..4c07238 100644 --- a/tests/test_authorization_binding.py +++ b/tests/test_authorization_binding.py @@ -43,6 +43,7 @@ def _apply_001(cfg: Config): "006_ref_updates.sql", "007_ref_update_scope_paths.sql", "008_operation_policies.sql", + "009_approval_quorum.sql", ] @@ -207,13 +208,7 @@ def test_contract_rejects_001_only_store_until_all_migrations_apply(tmp_path): conn.commit() conn.close() - message = ( - "Missing required migrations: " - "['002_approval_identities.sql', '004_approval_authority.sql', " - "'005_content_governance.sql', " - "'006_ref_updates.sql', '007_ref_update_scope_paths.sql', " - "'008_operation_policies.sql']" - ) + message = f"Missing required migrations: {UPGRADE_MIGRATIONS}" with pytest.raises(store.ContractViolation, match=re.escape(message)): store.verify_contract(cfg) with pytest.raises(store.ContractViolation, match=re.escape(message)): diff --git a/tests/test_operation_policies.py b/tests/test_operation_policies.py index d21da52..1316d60 100644 --- a/tests/test_operation_policies.py +++ b/tests/test_operation_policies.py @@ -162,6 +162,7 @@ def test_unset_override_fields_inherit_and_inherit_removes(people, approval_repo "threshold": 3, "requester_may_vote": False, "roles": ["reviewer"], + "quorum": False, } effective = registry.set_approval_policy(