Skip to content

Commit 17d13cb

Browse files
eduralphclaude
andcommitted
fix(revert): never close an Onto-branch (mode=stacked) PR — not harness-created
Codex review (#159, P1): an `Onto branch` (#54) contribution records mode="stacked" against a PRE-EXISTING PR the harness only appended a commit to. The OPEN path called _withdraw -> `gh pr close --delete-branch`, which would close AND delete that collaborator's whole PR branch. Gate withdraw to harness-created PRs (new-pr / stacked-pr); for mode=stacked, refuse and tell the human to revert the specific commit by hand. The merged path is unaffected (it opens a NEW revert PR). Adds a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ffb95aa commit 17d13cb

2 files changed

Lines changed: 24 additions & 0 deletions

File tree

template/src/pdca_harness/revert.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ def revert(cfg: Config, issue_id: str, *, dry_run: bool = False, by: str = "",
4848
if pr_state == "MERGED":
4949
return _revert_merged(cfg, d, issue_id, rec, pr_url, dry_run=dry_run, by=by, today=today)
5050
if pr_state == "OPEN":
51+
# ``mode: "stacked"`` (Onto branch, #54) means the harness appended a commit to a
52+
# PRE-EXISTING PR it did NOT create. Withdrawing it would `gh pr close
53+
# --delete-branch` that collaborator's whole PR branch — never do that. (The merged
54+
# path is still safe: it opens a *new* revert PR, leaving the original alone.)
55+
if rec.get("mode") == "stacked":
56+
print(f"revert: {d.name} was published as a commit onto an existing PR "
57+
f"({pr_url}, mode=stacked) the harness did not create — refusing to close "
58+
"it. Revert just that commit on the PR branch by hand.", file=sys.stderr)
59+
return 1
5160
return _withdraw(cfg, d, pr_url, dry_run=dry_run, by=by, today=today)
5261
print(f"revert: {d.name}'s PR is {pr_state} — nothing to revert ({pr_url}).")
5362
return 0

template/tests/test_revert.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,21 @@ def fake_run(cmd, **kw):
111111
self.assertEqual(rec["action"], "withdraw")
112112
self.assertEqual(rec["reverts"], "https://gh/pr/1")
113113

114+
def test_open_stacked_pr_is_refused(self) -> None:
115+
# mode="stacked" (Onto branch #54) = a commit on a PRE-EXISTING PR the harness did
116+
# not create — revert must NOT close/delete it.
117+
d = self._bundle("S")
118+
pj = json.loads((d / "publish.json").read_text(encoding="utf-8"))
119+
pj["mode"] = "stacked"
120+
(d / "publish.json").write_text(json.dumps(pj), encoding="utf-8")
121+
with mock.patch.object(revert, "_pr_state", return_value="OPEN"), \
122+
mock.patch.object(revert.subprocess, "run") as run, \
123+
redirect_stderr(io.StringIO()) as err:
124+
rc = revert.revert(self.cfg, "S")
125+
self.assertEqual(rc, 1)
126+
run.assert_not_called() # never touched the collaborator's PR
127+
self.assertIn("refusing to close", err.getvalue())
128+
114129

115130
if __name__ == "__main__":
116131
unittest.main()

0 commit comments

Comments
 (0)