From a5794c8c0d5e22e7e6f0de717747cec967eb236a Mon Sep 17 00:00:00 2001 From: saagpatel Date: Sat, 6 Jun 2026 02:24:35 -0700 Subject: [PATCH] feat(automation): durable proposal queue + approval gate (Arc D phase 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add src/automation_proposals.py: the durable intent queue and real approval gate for bounded automation. A proposal records intent (action type + target + description) and a status; it carries no precomputed payload so the executor (phase 3) derives fresh content at apply time. - AutomationProposal (frozen) with pending/approved/rejected/executed lifecycle. - build_automation_proposals merges candidate proposals into an existing queue, id-deduplicated; operator approvals and rejections are sticky (never reset by re-proposing, and rejected proposals are not resurrected). - approve_proposal / reject_proposal: guarded PENDING-only transitions that raise on unknown id (ProposalNotFoundError) or wrong source status (ProposalApprovalError). - executable_proposals / require_approved: the hard enforcement gate — only APPROVED proposals are ever executable (exact status match). - load_proposals / save_proposals: JSON persistence; missing file -> [], malformed/invalid-status/missing-field/non-dict entries -> ValueError. 25 tests. /code-review applied: fixed reject_proposal writing into approved_at (added rejected_at field) and hardened from_dict/load to honor the documented malformed->ValueError contract. Full suite green, ruff clean. --- src/automation_proposals.py | 258 ++++++++++++++++++++++++ tests/test_automation_proposals.py | 303 +++++++++++++++++++++++++++++ 2 files changed, 561 insertions(+) create mode 100644 src/automation_proposals.py create mode 100644 tests/test_automation_proposals.py diff --git a/src/automation_proposals.py b/src/automation_proposals.py new file mode 100644 index 0000000..e1f2919 --- /dev/null +++ b/src/automation_proposals.py @@ -0,0 +1,258 @@ +"""Durable proposal queue + approval gate for bounded automation (Arc D, phase 2). + +A *proposal* records the INTENT to take a bounded-automation action against a +single repo (open a context-improvement PR, or update catalog seeds) plus a +status. Proposals deliberately carry no precomputed payload — the executor +(phase 3) re-derives fresh content at apply time so nothing goes stale between +proposal and execution. + +The approval gate is a real enforcement boundary: ``executable_proposals`` and +``require_approved`` only ever admit proposals an operator has explicitly +approved. Re-running proposal generation never resets an existing proposal's +status, so operator approvals and rejections are sticky. +""" + +from __future__ import annotations + +import json +from dataclasses import asdict, dataclass, replace +from pathlib import Path +from typing import Any, Iterable + +from src.portfolio_automation import AutomationCandidate + +CONTRACT_VERSION = "automation_proposals_v1" + +ACTION_CONTEXT_PR = "context-pr" +ACTION_CATALOG_SEED = "catalog-seed" +VALID_ACTION_TYPES = frozenset({ACTION_CONTEXT_PR, ACTION_CATALOG_SEED}) + +STATUS_PENDING = "pending" +STATUS_APPROVED = "approved" +STATUS_REJECTED = "rejected" +STATUS_EXECUTED = "executed" +VALID_STATUSES = frozenset({STATUS_PENDING, STATUS_APPROVED, STATUS_REJECTED, STATUS_EXECUTED}) + +_ACTION_DESCRIPTIONS = { + ACTION_CONTEXT_PR: "Open an auto-PR improving the managed context block for {repo}.", + ACTION_CATALOG_SEED: "Apply catalog seed updates for {repo}.", +} + + +class ProposalNotFoundError(KeyError): + """Raised when an operation references a proposal id that does not exist.""" + + +class ProposalApprovalError(RuntimeError): + """Raised when a status transition or execution gate is violated.""" + + +@dataclass(frozen=True) +class AutomationProposal: + proposal_id: str + action_type: str + display_name: str + repo_full_name: str + description: str + status: str = STATUS_PENDING + created_at: str = "" + approved_at: str = "" + approved_by: str = "" + rejected_at: str = "" + executed_at: str = "" + execution_ref: str = "" + + def to_dict(self) -> dict[str, str]: + return asdict(self) + + @classmethod + def from_dict(cls, data: dict[str, Any]) -> AutomationProposal: + if not isinstance(data, dict): + raise ValueError(f"Proposal entry must be an object, got {type(data).__name__}.") + for required in ("proposal_id", "action_type"): + if required not in data: + raise ValueError(f"Proposal entry is missing required field {required!r}.") + status = str(data.get("status", STATUS_PENDING)) + if status not in VALID_STATUSES: + raise ValueError( + f"Proposal {data['proposal_id']!r} has invalid status {status!r}; " + f"expected one of {sorted(VALID_STATUSES)}." + ) + return cls( + proposal_id=str(data["proposal_id"]), + action_type=str(data["action_type"]), + display_name=str(data.get("display_name", "")), + repo_full_name=str(data.get("repo_full_name", "")), + description=str(data.get("description", "")), + status=status, + created_at=str(data.get("created_at", "")), + approved_at=str(data.get("approved_at", "")), + approved_by=str(data.get("approved_by", "")), + rejected_at=str(data.get("rejected_at", "")), + executed_at=str(data.get("executed_at", "")), + execution_ref=str(data.get("execution_ref", "")), + ) + + +def _require_action_type(action_type: str) -> None: + if action_type not in VALID_ACTION_TYPES: + raise ValueError( + f"Unknown automation action type {action_type!r}; " + f"expected one of {sorted(VALID_ACTION_TYPES)}." + ) + + +def make_proposal_id(action_type: str, candidate: AutomationCandidate) -> str: + """Stable id for a (action, repo) pair — slug-preferred for cross-run identity.""" + _require_action_type(action_type) + target = candidate.repo_full_name or candidate.display_name + return f"{action_type}:{target}" + + +def proposal_for_candidate( + candidate: AutomationCandidate, action_type: str, *, created_at: str +) -> AutomationProposal: + """Build a fresh PENDING proposal for one candidate + action type.""" + _require_action_type(action_type) + return AutomationProposal( + proposal_id=make_proposal_id(action_type, candidate), + action_type=action_type, + display_name=candidate.display_name, + repo_full_name=candidate.repo_full_name, + description=_ACTION_DESCRIPTIONS[action_type].format(repo=candidate.display_name), + status=STATUS_PENDING, + created_at=created_at, + ) + + +def build_automation_proposals( + candidates: Iterable[AutomationCandidate], + *, + action_type: str, + created_at: str, + existing: Iterable[AutomationProposal] = (), +) -> list[AutomationProposal]: + """Merge fresh candidate proposals into an existing queue, id-deduplicated. + + Existing proposals are preserved verbatim (status, timestamps, and operator + decisions are sticky). Only candidates without an existing proposal id are + appended as new PENDING entries. Existing-first ordering is preserved; new + entries follow in candidate order. + """ + _require_action_type(action_type) + merged: list[AutomationProposal] = list(existing) + seen = {proposal.proposal_id for proposal in merged} + for candidate in candidates: + proposal = proposal_for_candidate(candidate, action_type, created_at=created_at) + if proposal.proposal_id in seen: + continue + merged.append(proposal) + seen.add(proposal.proposal_id) + return merged + + +# --- persistence ----------------------------------------------------------- + + +def load_proposals(path: Path) -> list[AutomationProposal]: + """Load proposals from ``path``; missing file -> []. Malformed -> ValueError.""" + if not path.exists(): + return [] + try: + data = json.loads(path.read_text()) + except json.JSONDecodeError as error: + raise ValueError(f"Malformed proposals file at {path}: {error}") from error + if not isinstance(data, dict) or not isinstance(data.get("proposals"), list): + raise ValueError(f"Proposals file at {path} is missing a 'proposals' list.") + return [AutomationProposal.from_dict(entry) for entry in data["proposals"]] + + +def save_proposals(path: Path, proposals: Iterable[AutomationProposal]) -> None: + """Persist proposals to ``path`` as JSON, creating parent dirs as needed.""" + path.parent.mkdir(parents=True, exist_ok=True) + payload = { + "contract_version": CONTRACT_VERSION, + "proposals": [proposal.to_dict() for proposal in proposals], + } + path.write_text(json.dumps(payload, indent=2) + "\n") + + +# --- status transitions ---------------------------------------------------- + + +def _transition( + proposals: Iterable[AutomationProposal], + proposal_id: str, + *, + expected_status: str, + **changes: str, +) -> list[AutomationProposal]: + result: list[AutomationProposal] = [] + found = False + for proposal in proposals: + if proposal.proposal_id != proposal_id: + result.append(proposal) + continue + found = True + if proposal.status != expected_status: + raise ProposalApprovalError( + f"Proposal {proposal_id!r} is {proposal.status!r}; " + f"expected {expected_status!r} for this transition." + ) + result.append(replace(proposal, **changes)) + if not found: + raise ProposalNotFoundError(proposal_id) + return result + + +def approve_proposal( + proposals: Iterable[AutomationProposal], + proposal_id: str, + *, + approved_by: str, + approved_at: str, +) -> list[AutomationProposal]: + """Transition a PENDING proposal to APPROVED, stamping operator + timestamp.""" + return _transition( + proposals, + proposal_id, + expected_status=STATUS_PENDING, + status=STATUS_APPROVED, + approved_by=approved_by, + approved_at=approved_at, + ) + + +def reject_proposal( + proposals: Iterable[AutomationProposal], + proposal_id: str, + *, + rejected_at: str, +) -> list[AutomationProposal]: + """Transition a PENDING proposal to REJECTED.""" + return _transition( + proposals, + proposal_id, + expected_status=STATUS_PENDING, + status=STATUS_REJECTED, + rejected_at=rejected_at, + ) + + +# --- enforcement gate ------------------------------------------------------ + + +def executable_proposals( + proposals: Iterable[AutomationProposal], +) -> list[AutomationProposal]: + """Return only the APPROVED proposals — the executor acts on these alone.""" + return [proposal for proposal in proposals if proposal.status == STATUS_APPROVED] + + +def require_approved(proposal: AutomationProposal) -> None: + """Raise unless ``proposal`` is APPROVED. The hard gate before any execution.""" + if proposal.status != STATUS_APPROVED: + raise ProposalApprovalError( + f"Proposal {proposal.proposal_id!r} is {proposal.status!r}; " + "refusing to execute a proposal that is not approved." + ) diff --git a/tests/test_automation_proposals.py b/tests/test_automation_proposals.py new file mode 100644 index 0000000..7672a76 --- /dev/null +++ b/tests/test_automation_proposals.py @@ -0,0 +1,303 @@ +"""Tests for the Arc D phase-2 proposal queue + approval gate. + +Proposals record *intent* (action type + target + description) and a status. +They carry no precomputed payload — the executor (phase 3) derives fresh content +at apply time. The approval gate (`executable_proposals` / `require_approved`) +is a real enforcement check: nothing is executable until an operator approves. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from src.automation_proposals import ( + ACTION_CATALOG_SEED, + ACTION_CONTEXT_PR, + STATUS_APPROVED, + STATUS_EXECUTED, + STATUS_PENDING, + STATUS_REJECTED, + AutomationProposal, + ProposalApprovalError, + ProposalNotFoundError, + approve_proposal, + build_automation_proposals, + executable_proposals, + load_proposals, + make_proposal_id, + proposal_for_candidate, + reject_proposal, + require_approved, + save_proposals, +) +from src.portfolio_automation import AutomationCandidate + +NOW = "2026-04-14T12:00:00+00:00" +LATER = "2026-04-15T09:00:00+00:00" + + +def _candidate( + display_name: str = "Repo", repo_full_name: str = "owner/Repo" +) -> AutomationCandidate: + return AutomationCandidate( + display_name=display_name, + repo_full_name=repo_full_name, + registry_status="active", + path_confidence="high", + context_quality="standard", + ) + + +# --- ids + single-proposal construction ------------------------------------ + + +def test_make_proposal_id_prefers_slug() -> None: + assert make_proposal_id(ACTION_CONTEXT_PR, _candidate()) == "context-pr:owner/Repo" + + +def test_make_proposal_id_falls_back_to_display_name() -> None: + candidate = _candidate(repo_full_name="") + assert make_proposal_id(ACTION_CATALOG_SEED, candidate) == "catalog-seed:Repo" + + +def test_proposal_for_candidate_is_pending_with_metadata() -> None: + proposal = proposal_for_candidate(_candidate(), ACTION_CONTEXT_PR, created_at=NOW) + assert proposal.status == STATUS_PENDING + assert proposal.created_at == NOW + assert proposal.action_type == ACTION_CONTEXT_PR + assert proposal.repo_full_name == "owner/Repo" + assert "Repo" in proposal.description + + +def test_unknown_action_type_is_rejected() -> None: + with pytest.raises(ValueError): + proposal_for_candidate(_candidate(), "delete-everything", created_at=NOW) + + +# --- build/merge ----------------------------------------------------------- + + +def test_build_creates_pending_proposals_for_each_candidate() -> None: + candidates = [_candidate("Alpha", "o/Alpha"), _candidate("Beta", "o/Beta")] + proposals = build_automation_proposals( + candidates, action_type=ACTION_CONTEXT_PR, created_at=NOW + ) + assert [p.proposal_id for p in proposals] == ["context-pr:o/Alpha", "context-pr:o/Beta"] + assert all(p.status == STATUS_PENDING for p in proposals) + + +def test_build_preserves_existing_proposal_status_and_timestamp() -> None: + existing = [ + AutomationProposal( + proposal_id="context-pr:o/Alpha", + action_type=ACTION_CONTEXT_PR, + display_name="Alpha", + repo_full_name="o/Alpha", + description="old", + status=STATUS_APPROVED, + created_at="2026-01-01T00:00:00+00:00", + approved_at="2026-01-02T00:00:00+00:00", + approved_by="operator", + ) + ] + merged = build_automation_proposals( + [_candidate("Alpha", "o/Alpha")], + action_type=ACTION_CONTEXT_PR, + created_at=NOW, + existing=existing, + ) + [alpha] = merged + # Existing approval is NOT reset by re-proposing. + assert alpha.status == STATUS_APPROVED + assert alpha.created_at == "2026-01-01T00:00:00+00:00" + assert alpha.approved_by == "operator" + + +def test_build_does_not_resurrect_rejected_proposal() -> None: + existing = [ + AutomationProposal( + proposal_id="context-pr:o/Alpha", + action_type=ACTION_CONTEXT_PR, + display_name="Alpha", + repo_full_name="o/Alpha", + description="old", + status=STATUS_REJECTED, + created_at="2026-01-01T00:00:00+00:00", + ) + ] + merged = build_automation_proposals( + [_candidate("Alpha", "o/Alpha")], + action_type=ACTION_CONTEXT_PR, + created_at=NOW, + existing=existing, + ) + [alpha] = merged + assert alpha.status == STATUS_REJECTED + + +def test_build_appends_new_candidates_alongside_existing() -> None: + existing = [ + AutomationProposal( + proposal_id="context-pr:o/Alpha", + action_type=ACTION_CONTEXT_PR, + display_name="Alpha", + repo_full_name="o/Alpha", + description="old", + status=STATUS_APPROVED, + created_at="2026-01-01T00:00:00+00:00", + ) + ] + merged = build_automation_proposals( + [_candidate("Alpha", "o/Alpha"), _candidate("Beta", "o/Beta")], + action_type=ACTION_CONTEXT_PR, + created_at=NOW, + existing=existing, + ) + by_id = {p.proposal_id: p for p in merged} + assert set(by_id) == {"context-pr:o/Alpha", "context-pr:o/Beta"} + assert by_id["context-pr:o/Beta"].status == STATUS_PENDING + + +# --- persistence ----------------------------------------------------------- + + +def test_save_then_load_round_trips(tmp_path: Path) -> None: + path = tmp_path / "pending-proposals.json" + proposals = build_automation_proposals( + [_candidate("Alpha", "o/Alpha")], action_type=ACTION_CONTEXT_PR, created_at=NOW + ) + save_proposals(path, proposals) + loaded = load_proposals(path) + assert loaded == proposals + + +def test_load_missing_file_returns_empty(tmp_path: Path) -> None: + assert load_proposals(tmp_path / "nope.json") == [] + + +def test_load_malformed_file_raises(tmp_path: Path) -> None: + path = tmp_path / "bad.json" + path.write_text("{not valid json") + with pytest.raises(ValueError): + load_proposals(path) + + +def test_load_missing_proposals_key_raises(tmp_path: Path) -> None: + path = tmp_path / "noproposals.json" + path.write_text('{"contract_version": "automation_proposals_v1"}') + with pytest.raises(ValueError): + load_proposals(path) + + +def test_load_non_dict_entry_raises(tmp_path: Path) -> None: + path = tmp_path / "nondict.json" + path.write_text('{"proposals": ["not-an-object"]}') + with pytest.raises(ValueError): + load_proposals(path) + + +def test_load_entry_missing_required_field_raises(tmp_path: Path) -> None: + path = tmp_path / "missingfield.json" + path.write_text('{"proposals": [{"action_type": "context-pr"}]}') + with pytest.raises(ValueError): + load_proposals(path) + + +def test_load_entry_with_invalid_status_raises(tmp_path: Path) -> None: + path = tmp_path / "badstatus.json" + path.write_text( + '{"proposals": [{"proposal_id": "x", "action_type": "context-pr", "status": "APPROVED"}]}' + ) + with pytest.raises(ValueError): + load_proposals(path) + + +# --- approval gate --------------------------------------------------------- + + +def test_approve_pending_sets_status_and_metadata() -> None: + proposals = build_automation_proposals( + [_candidate("Alpha", "o/Alpha")], action_type=ACTION_CONTEXT_PR, created_at=NOW + ) + updated = approve_proposal( + proposals, "context-pr:o/Alpha", approved_by="saagar", approved_at=LATER + ) + [alpha] = updated + assert alpha.status == STATUS_APPROVED + assert alpha.approved_by == "saagar" + assert alpha.approved_at == LATER + + +def test_approve_unknown_id_raises() -> None: + with pytest.raises(ProposalNotFoundError): + approve_proposal([], "context-pr:o/Ghost", approved_by="x", approved_at=LATER) + + +def test_approve_non_pending_raises() -> None: + proposals = [ + AutomationProposal( + proposal_id="context-pr:o/Alpha", + action_type=ACTION_CONTEXT_PR, + display_name="Alpha", + repo_full_name="o/Alpha", + description="d", + status=STATUS_EXECUTED, + created_at=NOW, + ) + ] + with pytest.raises(ProposalApprovalError): + approve_proposal(proposals, "context-pr:o/Alpha", approved_by="x", approved_at=LATER) + + +def test_reject_pending_sets_status_and_rejected_at_only() -> None: + proposals = build_automation_proposals( + [_candidate("Alpha", "o/Alpha")], action_type=ACTION_CONTEXT_PR, created_at=NOW + ) + updated = reject_proposal(proposals, "context-pr:o/Alpha", rejected_at=LATER) + assert updated[0].status == STATUS_REJECTED + assert updated[0].rejected_at == LATER + # Rejection must NOT contaminate the approval fields. + assert updated[0].approved_at == "" + assert updated[0].approved_by == "" + + +def test_reject_unknown_id_raises() -> None: + with pytest.raises(ProposalNotFoundError): + reject_proposal([], "context-pr:o/Ghost", rejected_at=LATER) + + +# --- enforcement gate ------------------------------------------------------ + + +def test_executable_proposals_returns_only_approved() -> None: + proposals = [ + AutomationProposal( + "context-pr:a", ACTION_CONTEXT_PR, "A", "o/a", "d", status=STATUS_PENDING + ), + AutomationProposal( + "context-pr:b", ACTION_CONTEXT_PR, "B", "o/b", "d", status=STATUS_APPROVED + ), + AutomationProposal( + "context-pr:c", ACTION_CONTEXT_PR, "C", "o/c", "d", status=STATUS_REJECTED + ), + AutomationProposal( + "context-pr:d", ACTION_CONTEXT_PR, "D", "o/d", "d", status=STATUS_EXECUTED + ), + ] + assert [p.proposal_id for p in executable_proposals(proposals)] == ["context-pr:b"] + + +def test_require_approved_passes_for_approved() -> None: + proposal = AutomationProposal( + "context-pr:b", ACTION_CONTEXT_PR, "B", "o/b", "d", status=STATUS_APPROVED + ) + require_approved(proposal) # must not raise + + +@pytest.mark.parametrize("status", [STATUS_PENDING, STATUS_REJECTED, STATUS_EXECUTED]) +def test_require_approved_blocks_non_approved(status: str) -> None: + proposal = AutomationProposal("context-pr:b", ACTION_CONTEXT_PR, "B", "o/b", "d", status=status) + with pytest.raises(ProposalApprovalError): + require_approved(proposal)