diff --git a/src/panopticon/workflows/github_dependabot.py b/src/panopticon/workflows/github_dependabot.py index ac305014..9ad382d1 100644 --- a/src/panopticon/workflows/github_dependabot.py +++ b/src/panopticon/workflows/github_dependabot.py @@ -53,6 +53,21 @@ ), ) +#: MERGING responsibility specific to the dependency-bump lifecycle: the bump PR is approved (via +#: `approve-dependabot-pr`) so branch protection's required-review gate is satisfied and the bump +#: can land. Gated as an explicit, dashboard-visible promise rather than folded into the shared +#: `babysit-merge` skill. Dependabot authored the PR, so the agent's token — a different identity — +#: may approve it; this is scoped to this workflow (on the self/peer-reviewed lifecycles the agent +#: is effectively the author and must not self-approve). Module scope for the same reason as +#: UPGRADE_EVALUATED: the nested `Merging` body can't see the enclosing class's namespace. +PR_APPROVED = Responsibility( + key="pr-approved", + description=( + "The Dependabot bump PR is approved (`gh pr review --approve`, via `approve-dependabot-pr`) " + "so branch protection's required-review gate is satisfied and the bump can land." + ), +) + class GithubDependabot(GithubForgeWorkflow): """The github-dependabot lifecycle: a Dependabot dependency-bump PR (the task memo is the @@ -71,6 +86,9 @@ class GithubDependabot(GithubForgeWorkflow): #: Re-export of the module-level evaluation responsibility (see :data:`UPGRADE_EVALUATED`). UPGRADE_EVALUATED: ClassVar[Responsibility] = UPGRADE_EVALUATED + #: Re-export of the module-level MERGING approval responsibility (see :data:`PR_APPROVED`). + PR_APPROVED: ClassVar[Responsibility] = PR_APPROVED + class Planning(InitialState): label = "PLANNING" description = ( @@ -126,16 +144,28 @@ class Iterating(State): class Merging(State): label = "MERGING" - description = "Add the PR to the merge queue. If the PR exits the merge queue, re-add it." + description = ( + "First run `approve-dependabot-pr` to approve the bump PR (Dependabot authored it, so " + "the agent's token is a different identity and may approve it) — this satisfies branch " + "protection's required review so the bump can land. Then add the PR to the merge queue " + "with `babysit-merge`; if the PR exits the merge queue, re-add it." + ) advanced_by = Actor.AGENT # background: the agent shepherds the merge and advances itself - responsibilities = (Responsibility(key="pr-merged", description="The PR is merged."),) + responsibilities = ( + # The bump PR is approved so branch protection's required-review gate is satisfied. + # Gated here (not folded into `babysit-merge`) so the approval is an explicit, dashboard- + # visible promise the agent must resolve — not a line of prose it could skim past. + PR_APPROVED, + Responsibility(key="pr-merged", description="The PR is merged."), + ) transitions = (Complete,) # the happy path; `advance` derives → COMPLETE initial = Planning def skills(self) -> Sequence[Skill]: """Swap the inherited ``open-pr`` (the PR already exists) for ``checkout-dependabot-pr``, - and reuse the inherited ``babysit-ci`` / ``babysit-merge`` verbatim.""" + add the dependabot-only ``approve-dependabot-pr`` (MERGING's ``pr-approved`` gate), and + reuse the inherited ``babysit-ci`` / ``babysit-merge`` verbatim.""" forge = {skill.name: skill for skill in super().skills()} return ( Skill( @@ -156,6 +186,20 @@ def skills(self) -> Sequence[Skill]: "4. Call the `set_url` MCP tool with the PR URL so the dashboard's `p` hotkey " "opens it and the `url-recorded` responsibility can be resolved.", ), + Skill( + "approve-dependabot-pr", + "Approve the Dependabot bump PR so branch protection's required review is met.", + "Run this **once at the start of MERGING**, before `babysit-merge` queues the PR — " + "an approving review is what satisfies branch protection so the bump can land.\n" + "Dependabot is the PR author and the agent's token is a *different* identity, so it " + "may approve the PR (this is not a self-approval, and is scoped to this " + "dependency-bump workflow).\n" + "1. Read the PR URL from the task memo (or the recorded task URL).\n" + "2. Approve it: `gh pr review --approve` (run in `/workspace`). If it reports " + "the PR is already approved by you, that's fine — treat it as done.\n" + "3. Resolve the `pr-approved` responsibility (`resolve_responsibility`, MET), then " + "run `babysit-merge` to shepherd the PR through the merge queue.", + ), forge["babysit-ci"], forge["babysit-merge"], ) diff --git a/tests/workflows/test_github_dependabot.py b/tests/workflows/test_github_dependabot.py index 00ccc199..e2b1de2c 100644 --- a/tests/workflows/test_github_dependabot.py +++ b/tests/workflows/test_github_dependabot.py @@ -100,8 +100,13 @@ def test_iterating_responsibilities_target_the_dependabot_pr() -> None: assert "Dependabot" in by_key["committed-pushed"].description -def test_merging_responsibility() -> None: - assert {r.key for r in WF.responsibilities("MERGING")} == {"pr-merged"} +def test_merging_gates_approval_and_merge() -> None: + # MERGING carries the shared `pr-merged` plus the dependabot-specific `pr-approved` gate — the + # approval is an explicit, gated promise, so the agent cannot advance to COMPLETE without it. + by_key = {r.key: r for r in WF.responsibilities("MERGING")} + assert set(by_key) == {"pr-approved", "pr-merged"} + approval = by_key["pr-approved"].description.lower() + assert "approve" in approval and "branch protection" in approval # -- skills + forge plumbing -------------------------------------------------------- @@ -109,7 +114,12 @@ def test_merging_responsibility() -> None: def test_skills_swap_open_pr_for_checkout_dependabot_pr() -> None: skills = {s.name: s for s in WF.skills()} - assert set(skills) == {"checkout-dependabot-pr", "babysit-ci", "babysit-merge"} + assert set(skills) == { + "checkout-dependabot-pr", + "approve-dependabot-pr", + "babysit-ci", + "babysit-merge", + } assert "open-pr" not in skills # nothing to open — the PR already exists checkout = skills["checkout-dependabot-pr"] assert checkout.description and checkout.instructions # a functional spec, not a stub @@ -119,12 +129,32 @@ def test_skills_swap_open_pr_for_checkout_dependabot_pr() -> None: def test_babysit_skills_are_reused_verbatim_from_the_forge_base() -> None: + # The merge machinery stays shared: approval is a separate `approve-dependabot-pr` skill + + # gated `pr-approved` responsibility, so `babysit-merge` itself is unchanged (no fork/drift). base = {s.name: s for s in GithubForgeWorkflow().skills()} ours = {s.name: s for s in WF.skills()} for name in ("babysit-ci", "babysit-merge"): assert ours[name].instructions == base[name].instructions # not re-authored +def test_approve_skill_approves_the_pr_and_resolves_the_gate() -> None: + # Approval is a dependabot-only skill that runs at the start of MERGING, before `babysit-merge`. + skills = {s.name: s for s in WF.skills()} + approve = skills["approve-dependabot-pr"] + assert approve.description and approve.instructions # a functional spec, not a stub + assert "gh pr review" in approve.instructions and "--approve" in approve.instructions + assert "pr-approved" in approve.instructions # resolves the MERGING gate + assert "babysit-merge" in approve.instructions # then hands off to the merge shepherd + + +def test_forge_base_does_not_approve_prs() -> None: + # Approval is scoped to dependabot: the shared base (and thus the self/peer-reviewed + # lifecycles, where the agent is effectively the PR author) must NOT approve any PR. + base_instructions = "\n".join(s.instructions for s in GithubForgeWorkflow().skills()) + assert "gh pr review" not in base_instructions + assert "--approve" not in base_instructions + + def test_inherits_the_gh_tool_and_image_layer() -> None: assert "gh" in {t.name for t in WF.tools()} # named in the agent's system prompt assert "gh" in WF.image_layer() # forge skills need gh layered onto the base image