External/bench
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'task/16-conflicted-prs-first-class-with-an-agent-path'

Browse Source
This commit is contained in:
istos
2026-07-30 08:08:13 +02:00
parent 580ff879ac 8d08470530
commit 035ee87d4b
9 changed files with 275 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
AGENTS.md
+12 -4
View File
@@ -285,10 +285,18 @@ review and line comment, addresses each point or says why not, commits,
pushes so the PR updates, and appends a `## PR update` section to the task
file. Then **◔ review PR** again, until it settles.
The board polls open PRs of review-stage cards (reviews + CI checks, every
60s — a plain thread in board.py, no agent involved, silent when review/ is
empty) and folds everything into one verdict — any changes-requested review
or failing check wins over any approval. Tool chips (CI, copilot, PR, drive)
The board polls open PRs of review-stage cards (reviews + CI checks +
GitHub's mergeable state, every 60s — a plain thread in board.py, no agent
involved, silent when review/ is empty) and folds everything into one
verdict — any changes-requested review or failing check wins over any
approval. A PR GitHub cannot merge cleanly wears an alarm-coloured
`conflicts` chip and counts as changes-needed-by-you (not a CI failure);
**↻ act on PR** resolves mechanical conflicts by merging main into the
branch — additively, never rebasing or force-pushing — in a dedicated
resolution commit, and refuses semantic ones, naming the collision for a
human to settle. GitHub computes mergeability lazily, so an UNKNOWN
reading keeps the chip's last state rather than flapping.
Tool chips (CI, copilot, PR, drive)
are destinations, not statuses: they live in the card's footer row, never
squeezed into the author row — `CI ✓` (pine), `CI ✕` (terracotta), `◌`
while in flight, with hover actions staying in the status pill's slot. The card wears it in the design
manager/core/adapters/README.md
+6 -2
View File
@@ -40,9 +40,13 @@ side effects its prompt demands, and never a blanket allow-everything
- `work` — implement, test, commit in an isolated worktree. May edit
files, run local git bookkeeping (`git add/commit/status/diff`) and
the project's `AGENT_COMMANDS`. No push.
- `act-pr` — the work stance, plus `git push` (the PR must update) and
- `act-pr` — the work stance, plus `git push` (the PR must update),
reading the PR's reviews and line comments (`gh pr view`, `gh pr
diff`, `gh api`).
diff`, `gh api`), and `git fetch`/`git merge` so a conflicted PR can
be resolved by merging main into the branch. Resolution is additive
only — the branch is public — so `git rebase` and the force-push
spellings must be denied, not merely unlisted (a plain `git push`
allow would otherwise cover them).
- `review` — read-only on the working tree: no edit tools, no commits.
May read a PR (`gh pr view`, `gh pr diff`, read-only git) and post the
verdict (`gh pr review`, `gh pr comment`).
manager/core/adapters/claude/hook_settings.py
+30 -2
View File
@@ -13,7 +13,12 @@ is isolated, the shell is not.
(add/commit/status/diff) + the project's test/check commands.
No push.
act-pr the work stance + `git push` (the PR must update) + reading
the PR's reviews and line comments through gh.
the PR's reviews and line comments through gh + `git fetch`
and `git merge` so a conflicted PR can be resolved by merging
main into the branch. The branch is public, so resolution is
additive only: rebase and the force-push spellings are denied
outright (deny beats allow, catching what the plain
`git push` prefix would otherwise cover).
review read-only (edit tools disallowed in `run`) + reading the PR
it judges + posting the verdict with gh pr review/comment.
@@ -35,10 +40,19 @@ from pathlib import Path
MODE_PREFIXES = {
"work": ["git add", "git commit", "git status", "git diff"],
"act-pr": ["git add", "git commit", "git status", "git diff",
"git fetch", "git merge",
"git push", "gh pr view", "gh pr diff", "gh api"],
"review": ["git status", "git diff", "git log", "git show",
"gh pr view", "gh pr diff", "gh pr review", "gh pr comment"],
}
# History must never rewrite under a public PR: deny the canonical force
# and rebase spellings even though nothing allows them — `git push` alone
# would otherwise cover `git push --force` by prefix. (Prefix rules can't
# catch a flag placed after the refspec; the prompt and the review loop
# guard the exotic spellings.)
MODE_DENY_PREFIXES = {
"act-pr": ["git push --force", "git push -f", "git rebase"],
}
# Which intents run the project's own test/check commands.
MODES_WITH_PROJECT_COMMANDS = {"work", "act-pr"}
@@ -60,6 +74,14 @@ def allow_rules(mode: str, commands: list[str]) -> list[str]:
return rules
def deny_rules(mode: str) -> list[str]:
"""Bash() deny-rules for one intent — deny beats allow."""
rules = []
for prefix in MODE_DENY_PREFIXES.get(mode, []):
rules += [f"Bash({prefix})", f"Bash({prefix}:*)"]
return rules
def settings(mode: str, commands: list[str]) -> dict:
emit = Path(__file__).resolve().parent / "emit.py"
hook = {"type": "command", "command": f'python3 "{emit}"', "timeout": 5}
@@ -71,9 +93,15 @@ def settings(mode: str, commands: list[str]) -> dict:
"PreToolUse": [{"matcher": "Bash", "hooks": [hook]}],
"PostToolUse": [{"matcher": "*", "hooks": [hook]}],
}}
perms = {}
rules = allow_rules(mode, commands)
if rules:
out["permissions"] = {"allow": rules}
perms["allow"] = rules
denies = deny_rules(mode)
if denies:
perms["deny"] = denies
if perms:
out["permissions"] = perms
return out
manager/core/adapters/opencode/permission_config.py
+17 -1
View File
@@ -14,7 +14,11 @@ and never a blanket allow: the worktree is isolated, the shell is not.
work "edit": "allow" + git bookkeeping (add/commit/status/diff) and
the project's test/check commands. No push.
act-pr the work stance + `git push` + reading the PR's reviews and
line comments through gh.
line comments through gh + `git fetch`/`git merge` so a
conflicted PR can be resolved by merging main into the
branch. The branch is public, so resolution is additive
only: rebase and the force-push spellings get explicit deny
rules, placed last so they win over the `git push *` allow.
review "edit": "deny" + reading the PR it judges + posting the
verdict with gh pr review/comment. Everything else denied.
@@ -34,10 +38,20 @@ import sys
MODE_PREFIXES = {
"work": ["git add", "git commit", "git status", "git diff"],
"act-pr": ["git add", "git commit", "git status", "git diff",
"git fetch", "git merge",
"git push", "gh pr view", "gh pr diff", "gh api"],
"review": ["git status", "git diff", "git log", "git show",
"gh pr view", "gh pr diff", "gh pr review", "gh pr comment"],
}
# History must never rewrite under a public PR: deny the force and rebase
# spellings even though nothing allows them — "git push *" would otherwise
# cover them. Globs run over the whole command line, so the flag is caught
# wherever it sits.
MODE_DENY_PATTERNS = {
"act-pr": ["git rebase", "git rebase *",
"git push --force*", "git push * --force*",
"git push -f", "git push -f *", "git push * -f *"],
}
# Which intents run the project's own test/check commands.
MODES_WITH_PROJECT_COMMANDS = {"work", "act-pr"}
@@ -57,6 +71,8 @@ def bash_rules(mode: str, commands: list[str]) -> dict:
for prefix in prefixes:
rules[prefix] = "allow"
rules[f"{prefix} *"] = "allow"
for pattern in MODE_DENY_PATTERNS.get(mode, []):
rules[pattern] = "deny" # last, so it wins over the allows
return rules
manager/core/board.html
+4
View File
@@ -909,6 +909,10 @@ function cardFor(task) {
chips.push({ label: 'CI', glyph: { pass: '✓', fail: '✕', running: '◌' }[prState.ci],
cls: { pass: 'ok', fail: 'bad', running: 'accent' }[prState.ci], title: detail });
}
if (prState && prState.conflicts) {
chips.push({ label: 'conflicts', glyph: '✕', cls: 'bad',
title: 'GitHub cannot merge this into main — ↻ act on PR can attempt a resolution merge' });
}
if (prState && prState.copilot) {
chips.push({ label: 'copilot',
glyph: { asked: '◌', approved: '✓', changes: '✕', commented: '·' }[prState.copilot],
manager/core/github.py
+51 -18
View File
@@ -191,16 +191,21 @@ def _is_copilot(login) -> bool:
return "copilot" in str(login or "").lower()
def _poll_pr(filename: str, url: str) -> None:
number = url.rstrip("/").rsplit("/", 1)[-1]
result = _run([config.GH_BIN, "pr", "view", number,
"--json", "reviews,reviewRequests,statusCheckRollup,state"], timeout=60)
if result.returncode != 0:
return
try:
data = json.loads(result.stdout)
except json.JSONDecodeError:
return
def _conflict_state(data: dict, prev: dict) -> bool | None:
"""GitHub computes mergeability lazily: UNKNOWN means "not computed
yet", never "fine" — keep the previous reading so the chip does not
flap while GitHub thinks."""
mergeable = str(data.get("mergeable") or "").upper()
if mergeable == "CONFLICTING":
return True
if mergeable == "MERGEABLE":
return False
return prev.get("conflicts")
def _fold(data: dict, prev: dict) -> dict:
"""One gh pr-view payload + the previous snapshot → the new snapshot.
Pure fold: fetching, events and broadcasts stay in _poll_pr."""
reviews = data.get("reviews") or []
checks = data.get("statusCheckRollup") or []
changes = any(r.get("state") == "CHANGES_REQUESTED" for r in reviews)
@@ -219,15 +224,18 @@ def _poll_pr(filename: str, url: str) -> None:
elif cop_requested:
copilot = "asked"
else:
copilot = PR_STATE.get(filename, {}).get("copilot")
copilot = "asked" if copilot == "asked" else None
copilot = "asked" if prev.get("copilot") == "asked" else None
states = [_check_state(c) for c in checks]
ci = ("fail" if "fail" in states
else "running" if "running" in states
else "pass" if states else None)
verdict = ("red" if (changes or ci == "fail")
conflicts = _conflict_state(data, prev)
# A conflict is changes-needed-by-you, not a CI failure: it beats any
# approval but leaves the CI chip telling its own story.
verdict = ("red" if (changes or ci == "fail" or conflicts)
else "green" if approved else "pending")
detail_bits = []
if reviews:
@@ -235,14 +243,32 @@ def _poll_pr(filename: str, url: str) -> None:
if ci:
detail_bits.append({"fail": "checks failing", "running": "checks running",
"pass": "checks ok"}[ci])
if conflicts:
detail_bits.append("conflicts with main")
if copilot:
detail_bits.append("copilot " + {"asked": "asked", "approved": "approved",
"changes": "asked for changes",
"commented": "commented"}[copilot])
return {"verdict": verdict, "ci": ci, "copilot": copilot,
"conflicts": conflicts, "detail": " · ".join(detail_bits)}
def _poll_pr(filename: str, url: str) -> None:
number = url.rstrip("/").rsplit("/", 1)[-1]
result = _run([config.GH_BIN, "pr", "view", number,
"--json", "reviews,reviewRequests,statusCheckRollup,state,mergeable"],
timeout=60)
if result.returncode != 0:
return
try:
data = json.loads(result.stdout)
except json.JSONDecodeError:
return
prev = PR_STATE.get(filename, {})
PR_STATE[filename] = {"verdict": verdict, "ci": ci, "copilot": copilot,
"url": url, "detail": " · ".join(detail_bits),
"ts": time.time()}
entry = _fold(data, prev)
entry.update({"url": url, "ts": time.time()})
PR_STATE[filename] = entry
verdict, ci, copilot = entry["verdict"], entry["ci"], entry["copilot"]
if prev.get("copilot") in (None, "asked") and copilot in ("approved", "changes", "commented"):
word = {"approved": "approved it", "changes": "asked for changes",
"commented": "commented"}[copilot]
@@ -250,7 +276,14 @@ def _poll_pr(filename: str, url: str) -> None:
"kind": "agent", "actor": "board", "file": filename,
"summary": f"Copilot reviewed {filename}'s PR and {word}"})
state.broadcast({"type": "board"})
if prev.get("verdict") != verdict and verdict != "pending":
if bool(prev.get("conflicts")) != bool(entry["conflicts"]):
state.record_board_event({
"kind": "agent", "actor": "board", "file": filename,
"summary": (f"{filename}'s PR conflicts with main — ↻ act on PR "
f"can attempt the resolution" if entry["conflicts"]
else f"{filename}'s PR no longer conflicts with main")})
state.broadcast({"type": "board"})
elif prev.get("verdict") != verdict and verdict != "pending":
word = "approved" if verdict == "green" else "changes asked"
state.record_board_event({
"kind": "agent", "actor": "board", "file": filename,
@@ -283,7 +316,7 @@ def poller() -> None:
def public_state() -> dict:
return {f: {k: v.get(k) for k in ("verdict", "ci", "copilot", "detail", "url")}
return {f: {k: v.get(k) for k in ("verdict", "ci", "copilot", "conflicts", "detail", "url")}
for f, v in PR_STATE.items()}
manager/core/prompts/act-pr.md
+17
View File
@@ -14,6 +14,23 @@ Do this properly:
repos/{{owner}}/{{repo}}/pulls/<number>/comments`.
- Address each point in the code. If you disagree with a point, do not
silently ignore it — leave it unchanged and say why in your summary.
- If the PR conflicts with main (check `gh pr view {branch} --json
mergeable` — CONFLICTING means yes), resolve it mechanically:
- `git fetch origin main`, then `git merge origin/main` in this
worktree. Never rebase and never force-push: the branch is public,
so the resolution must be additive.
- Resolve each conflicted file honouring both sides' intent, and run
the project's tests until they pass.
- Put the resolution in its own commit — never folded into other
changes — with a message naming the conflicted files and the choice
made in each.
- Cover the resolution explicitly in your closing report: which files
conflicted and what you chose.
- If both intents cannot hold at once — main has made this branch's
premise false — resolve nothing: run `git merge --abort`, leave the
branch as it was, and state in your report that a human must
decide, naming the specific collision. Guessing at a semantic
conflict is the one forbidden move.
- Follow repo AGENTS.md: layering rules, definition of done. Run the tests
that cover what you changed until they pass.
- Commit in clear, reviewable commits and push the branch (`git push`) so
tests/test_adapter_permissions.py
+43
View File
@@ -53,6 +53,29 @@ class ClaudeAllowRules(unittest.TestCase):
for prefix in ["git push", "gh pr view", "gh pr diff", "gh api"]:
self.assertIn(f"Bash({prefix}:*)", rules)
def test_act_pr_resolves_conflicts_additively(self):
# Conflicted PRs: merging main into the branch is allowed; history
# rewriting is not, and not merely by omission — `git push` alone
# would cover the force spellings, so they are denied outright.
rules = hook_settings.allow_rules("act-pr", COMMANDS)
for prefix in ["git fetch", "git merge"]:
self.assertIn(f"Bash({prefix})", rules)
self.assertIn(f"Bash({prefix}:*)", rules)
joined = " ".join(rules)
self.assertNotIn("git rebase", joined)
self.assertNotIn("--force", joined)
deny = hook_settings.settings("act-pr", COMMANDS)["permissions"]["deny"]
for prefix in ["git push --force", "git push -f", "git rebase"]:
self.assertIn(f"Bash({prefix}:*)", deny)
def test_only_act_pr_may_fetch_and_merge(self):
for mode in ("work", "review"):
joined = " ".join(hook_settings.allow_rules(mode, COMMANDS))
self.assertNotIn("git fetch", joined)
self.assertNotIn("git merge", joined)
self.assertNotIn("deny",
hook_settings.settings(mode, COMMANDS)["permissions"])
def test_review_posts_verdicts_but_writes_nothing_locally(self):
rules = hook_settings.allow_rules("review", COMMANDS)
for prefix in ["gh pr review", "gh pr comment", "gh pr view",
@@ -90,6 +113,26 @@ class OpencodeConfig(unittest.TestCase):
self.assertEqual(bash["git push *"], "allow")
self.assertEqual(bash["gh pr view *"], "allow")
def test_act_pr_resolves_conflicts_additively(self):
bash = permission_config.build_config("act-pr", COMMANDS)["permission"]["bash"]
for prefix in ["git fetch", "git merge"]:
self.assertEqual(bash[prefix], "allow")
self.assertEqual(bash[f"{prefix} *"], "allow")
for pattern in ["git rebase", "git rebase *", "git push --force*",
"git push * --force*", "git push -f", "git push -f *",
"git push * -f *"]:
self.assertEqual(bash[pattern], "deny")
# last match wins: the denies must come after the push allow
keys = list(bash)
self.assertGreater(keys.index("git push --force*"),
keys.index("git push *"))
def test_only_act_pr_may_fetch_and_merge(self):
for mode in ("work", "review"):
bash = permission_config.build_config(mode, COMMANDS)["permission"]["bash"]
for absent in ["git fetch", "git merge", "git rebase"]:
self.assertNotIn(absent, bash) # unlisted = denied by "*"
def test_review_cannot_edit_and_bash_default_denies(self):
config = permission_config.build_config("review", COMMANDS)
self.assertEqual(config["permission"]["edit"], "deny")
tests/test_pr_conflicts.py
+95
View File
@@ -0,0 +1,95 @@
"""Conflicted PRs become card state: the poller folds GitHub's
mergeable field into the PR snapshot, a conflict drops any
approved-green verdict as changes-needed-by-you (not a CI failure),
and GitHub's lazily-computed UNKNOWN keeps the previous reading so the
chip never flaps. The snapshot's `conflicts` key must reach the UI."""
import sys
import unittest
from pathlib import Path
REPO = Path(__file__).resolve().parents[1]
sys.path.insert(0, str(REPO / "manager" / "core"))
import github # noqa: E402
def payload(**overrides):
"""A gh pr-view JSON payload with the fields _fold reads."""
data = {"reviews": [], "reviewRequests": [], "statusCheckRollup": []}
data.update(overrides)
return data
APPROVED = [{"state": "APPROVED"}]
class ConflictFolding(unittest.TestCase):
def test_conflict_drops_green_even_when_approved(self):
entry = github._fold(payload(reviews=APPROVED,
mergeable="CONFLICTING"), {})
self.assertTrue(entry["conflicts"])
self.assertEqual(entry["verdict"], "red")
self.assertIn("conflicts with main", entry["detail"])
def test_conflict_is_not_a_ci_failure(self):
entry = github._fold(payload(mergeable="CONFLICTING"), {})
self.assertIsNone(entry["ci"])
self.assertEqual(entry["verdict"], "red")
def test_mergeable_approved_pr_stays_green(self):
entry = github._fold(payload(reviews=APPROVED,
mergeable="MERGEABLE"), {})
self.assertFalse(entry["conflicts"])
self.assertEqual(entry["verdict"], "green")
def test_unknown_keeps_the_previous_reading_both_ways(self):
# GitHub computes mergeability lazily after a push: UNKNOWN means
# "not yet", never "fine" — the chip must not flap.
still = github._fold(payload(mergeable="UNKNOWN"),
{"conflicts": True})
self.assertTrue(still["conflicts"])
self.assertEqual(still["verdict"], "red")
clean = github._fold(payload(reviews=APPROVED, mergeable="UNKNOWN"),
{"conflicts": False})
self.assertFalse(clean["conflicts"])
self.assertEqual(clean["verdict"], "green")
def test_unknown_on_first_sight_alarms_nobody(self):
entry = github._fold(payload(mergeable="UNKNOWN"), {})
self.assertIsNone(entry["conflicts"])
self.assertEqual(entry["verdict"], "pending")
self.assertNotIn("conflicts", entry["detail"])
def test_resolution_lets_green_return(self):
entry = github._fold(payload(reviews=APPROVED, mergeable="MERGEABLE"),
{"conflicts": True, "verdict": "red"})
self.assertFalse(entry["conflicts"])
self.assertEqual(entry["verdict"], "green")
class SnapshotReachesTheUI(unittest.TestCase):
def test_public_state_carries_conflicts(self):
github.PR_STATE["x.md"] = {"verdict": "red", "ci": None,
"copilot": None, "conflicts": True,
"detail": "conflicts with main",
"url": "u", "ts": 1}
try:
self.assertTrue(github.public_state()["x.md"]["conflicts"])
finally:
github.PR_STATE.pop("x.md", None)
def test_the_card_wears_an_alarm_coloured_chip(self):
html = (REPO / "manager" / "core" / "board.html").read_text(
encoding="utf-8")
self.assertIn("prState.conflicts", html)
self.assertIn("label: 'conflicts', glyph: '✕', cls: 'bad'", html)
def test_poller_asks_github_for_mergeable(self):
source = (REPO / "manager" / "core" / "github.py").read_text(
encoding="utf-8")
self.assertIn("statusCheckRollup,state,mergeable", source)
if __name__ == "__main__":
unittest.main()