diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 0559b4c..45902e4 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "model-router", - "version": "0.42.1", + "version": "0.42.2", "description": "Route authorized software work through one durable, capacity-aware subscription policy.", "author": { "name": "toolboxmd", diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index ac2f15c..f10e957 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "model-router", - "version": "0.42.1", + "version": "0.42.2", "description": "Route authorized software work through one durable, capacity-aware subscription policy.", "author": { "name": "toolboxmd", diff --git a/.grok-plugin/plugin.json b/.grok-plugin/plugin.json index 0559b4c..45902e4 100644 --- a/.grok-plugin/plugin.json +++ b/.grok-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "model-router", - "version": "0.42.1", + "version": "0.42.2", "description": "Route authorized software work through one durable, capacity-aware subscription policy.", "author": { "name": "toolboxmd", diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e90dd5..831dff8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [0.42.2] - 2026-09-29 + +### Changed + +- A PR merged before completion refuses as pr_merged with its merge commit and blocks once for the planner instead of looping on pr_not_open (#143). + ## [0.42.1] - 2026-09-29 ### Changed diff --git a/RUNNER.md b/RUNNER.md index a018622..84f8323 100644 --- a/RUNNER.md +++ b/RUNNER.md @@ -297,7 +297,9 @@ thread; Grok turns are re-sent by the router. becomes the job's single PR identity: Retry and Escalation update that PR, never open a second. An ordinary job in a workspace without an origin push remote completes without a PR; experiment and replay jobs - need none. Nothing merges the PR. + need none. Nothing merges the PR. A PR that is already merged refuses as + `pr_merged` with its merge commit and blocks the job at once for the + planner, without resuming the dispatcher. **Review turn (#126).** A completion that passes these checks first gets one read-only review turn on the candidate's exact head, in a reviewer diff --git a/VERSION b/VERSION index 339fd08..f386df2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.1 +0.42.2 diff --git a/runner/controller.py b/runner/controller.py index 5adb1de..e04aede 100644 --- a/runner/controller.py +++ b/runner/controller.py @@ -3436,6 +3436,12 @@ def _handle_completion_refusal(state_dir, request_id: str, envelope: dict, if "completion_refused_seq" in st and st.get("completion_refused_seq") == rep_seq_i: _mark_blocked(state_dir, request_id, reason) return {"action": "blocked", "reason": "completion_refused"} + if "completion_refused: pr_merged" in reason: + # A merged PR cannot be reopened by a dispatcher turn; resuming + # would loop on the same refusal (#143). Block once, clearly. + _record_completion_refusal(state_dir, request_id, report, reason) + _mark_blocked(state_dir, request_id, reason) + return {"action": "blocked", "reason": "completion_refused"} _record_completion_refusal(state_dir, request_id, report, reason) job = core.get_job(state_dir, request_id) try: diff --git a/runner/core.py b/runner/core.py index 2411d53..978dc2c 100644 --- a/runner/core.py +++ b/runner/core.py @@ -3172,7 +3172,8 @@ def record_known_pr(state_dir, request_id: str, pr_url: str | None, # seam so deterministic tests stub it without network or auth. The seam # takes (workspace, pr_url) and returns a dict: {"ok": bool, # "state": "OPEN"|..., "is_draft": bool, "head_sha": str|None, -# "repo": "owner/name"|None, "reason": str, "unknown": bool}. ``unknown`` +# "repo": "owner/name"|None, "merge_commit": str|None, "reason": str, +# "unknown": bool}. ``unknown`` # marks an unavailable check (no gh, no auth), which refuses as # unverified rather than inventing an answer. PR_VERIFIER = None @@ -3183,7 +3184,7 @@ def default_pr_verifier(workspace: str | None, pr_url: str) -> dict: try: proc = subprocess.run( ["gh", "pr", "view", pr_url, "--json", - "number,state,isDraft,headRefOid,url,headRepository"], + "number,state,isDraft,headRefOid,url,headRepository,mergeCommit"], capture_output=True, text=True, timeout=30, stdin=subprocess.DEVNULL, cwd=workspace or None) except FileNotFoundError: @@ -3207,12 +3208,15 @@ def default_pr_verifier(workspace: str | None, pr_url: str) -> dict: repo = data.get("headRepository") if isinstance(repo, dict): repo = repo.get("nameWithOwner") + merge = data.get("mergeCommit") + merge = merge.get("oid") if isinstance(merge, dict) else None return {"ok": True, "unknown": False, "state": data.get("state"), "is_draft": data.get("isDraft") is True, "head_sha": data.get("headRefOid") if isinstance(data.get("headRefOid"), str) else None, "repo": repo if isinstance(repo, str) and repo else None, "url": data.get("url") if isinstance(data.get("url"), str) else None, + "merge_commit": merge if isinstance(merge, str) and merge else None, "reason": ""} @@ -3290,6 +3294,14 @@ def verify_pr_for_completion(workspace: str | None, pr_url: str, return (f"completion_refused: pr_unverified ({reason}; cannot confirm " "the PR exists and is open)") return f"completion_refused: pr_check_failed ({reason})" + if str(seen.get("state") or "").upper() == "MERGED": + # The runner never merges; no dispatcher turn can reopen a merged + # PR, so this refusal blocks at once for the planner (#143). + merge = seen.get("merge_commit") + merge = merge[:12] if isinstance(merge, str) and merge else "unknown" + return (f"completion_refused: pr_merged (PR was merged, merge commit " + f"{merge}; the runner never merges and a job must leave its PR " + "open; the planner decides whether the merged outcome is complete)") if str(seen.get("state") or "").upper() != "OPEN": return (f"completion_refused: pr_not_open " f"(state={str(seen.get('state') or 'unknown')[:20]}; only an open PR completes a job)") diff --git a/tests/test_issue143.py b/tests/test_issue143.py new file mode 100644 index 0000000..a5a4d40 --- /dev/null +++ b/tests/test_issue143.py @@ -0,0 +1,132 @@ +"""Issue 143: a PR merged by the job blocks once, clearly, instead of looping. + +The runner never merges a PR (RUNNER.md step 6). When a job's PR is +already merged at completion, no dispatcher turn can restore an open PR, +so resuming the dispatcher with "update the existing open PR" loops on +``pr_not_open``. A merged PR now refuses as ``pr_merged`` naming the +merge commit and blocks the job at once for the planner; a closed, +unmerged PR keeps the existing once-then-block refusal. +""" +import json +import os +import stat +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT)) +from runner import controller, core # noqa: E402 +from tests.fakes import use_fake_t3 # noqa: E402 +from tests import test_issue87 as t87 # noqa: E402 +from tests.test_issue87 import set_controller_state, sql, stub_pr_verifier # noqa: E402 + +PR = "https://github.com/example/repo/pull/7" +MERGE = "1e6bb1d7a0000000000000000000000000000000" + + +def setUpModule(): + t87.setUpModule() + + +class MergedPrRefusal(unittest.TestCase): + def test_merged_pr_refuses_as_pr_merged_with_merge_commit(self): + seen = {"ok": True, "state": "MERGED", "is_draft": False, + "head_sha": "abc123", "repo": "example/repo", + "merge_commit": MERGE} + reason = core.verify_pr_for_completion( + "/tmp", PR, "abc123", json.dumps({"goal": "x"}), + verifier=stub_pr_verifier(seen)) + self.assertIn("completion_refused: pr_merged", reason) + self.assertIn(MERGE[:12], reason) + self.assertNotIn("pr_not_open", reason) + + def test_closed_unmerged_pr_still_refuses_as_not_open(self): + seen = {"ok": True, "state": "CLOSED", "is_draft": False, + "head_sha": "abc123", "repo": "example/repo"} + reason = core.verify_pr_for_completion( + "/tmp", PR, "abc123", json.dumps({"goal": "x"}), + verifier=stub_pr_verifier(seen)) + self.assertIn("pr_not_open", reason) + self.assertNotIn("pr_merged", reason) + + def test_default_verifier_reads_merge_commit(self): + tmp = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) + self.addCleanup(tmp.cleanup) + gh = Path(tmp.name) / "gh" + gh.write_text("#!/bin/sh\n" + "echo '{\"state\":\"MERGED\",\"isDraft\":false," + "\"headRefOid\":\"abc123\",\"url\":\"" + PR + "\"," + "\"mergeCommit\":{\"oid\":\"" + MERGE + "\"}}'\n") + gh.chmod(gh.stat().st_mode | stat.S_IXUSR) + old = os.environ.get("PATH", "") + os.environ["PATH"] = tmp.name + os.pathsep + old + self.addCleanup(os.environ.__setitem__, "PATH", old) + seen = core.default_pr_verifier(None, PR) + self.assertEqual(seen["state"], "MERGED") + self.assertEqual(seen["merge_commit"], MERGE) + + +class MergedPrBlocksWithoutLoop(unittest.TestCase): + _git_ws = t87.CompletionBinding._git_ws + _passing_report = t87.CompletionBinding._passing_report + + def _job(self, rid): + tmp = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) + self.addCleanup(tmp.cleanup) + base = Path(tmp.name) + sd = str(base / "state") + ws = self._git_ws(base, "ws", with_origin=True) + core.submit(sd, rid, {"goal": "x", "proof": "true"}, str(ws), "p", + planner_t3_thread="planner-t3") + self._passing_report(sd, rid, ws) + sql(sd, "UPDATE jobs SET controller_state=json_set(COALESCE(controller_state,'{}')," + "'$.t3_threads.dispatch',json_object('thread_id','sub.planner-t3.disp'," + "'route','luna/max')), status='running' WHERE request_id=?", (rid,)) + fake = use_fake_t3(self, sd, rid, default={"action": "completion", + "output": "STILL DONE"}) + set_controller_state(sd, rid, seq=1, + last_action={"action": "completion", "output": "DONE", + "pr_url": PR}, + last_action_name="completion") + return sd, ws, fake + + def _verifier(self, ws, state, **extra): + saved = core.PR_VERIFIER + self.addCleanup(setattr, core, "PR_VERIFIER", saved) + core.PR_VERIFIER = stub_pr_verifier(dict( + {"ok": True, "state": state, "is_draft": False, + "head_sha": core._workspace_head(str(ws)), "repo": "example/repo"}, + **extra)) + + def test_merged_pr_blocks_at_once_without_resuming_dispatcher(self): + sd, ws, fake = self._job("merged") + self._verifier(ws, "MERGED", merge_commit=MERGE) + commands_before = len(fake.commands) + out = controller.step(sd, "merged") + self.assertEqual(out["action"], "blocked") + job = core.get_job(sd, "merged") + self.assertEqual(job["status"], "blocked") + self.assertIn("pr_merged", job["block_reason"]) + self.assertIn(MERGE[:12], job["block_reason"]) + self.assertNotIn("pr_not_open", job["block_reason"]) + # No resume: the dispatcher is not asked to restore an open PR; + # the planner gets one terminal report naming the merge. + sent = fake.commands[commands_before:] + self.assertFalse([c for c in sent + if c.get("threadId") == "sub.planner-t3.disp"]) + reports = [c for c in sent if c.get("threadId") == "planner-t3"] + self.assertEqual(len(reports), 1) + self.assertIn("pr_merged", reports[0]["message"]["text"]) + self.assertIsNone(core.known_pr_url(sd, "merged")) + + def test_closed_pr_still_resumes_dispatcher_once(self): + sd, ws, _fake = self._job("closed") + self._verifier(ws, "CLOSED") + out = controller.step(sd, "closed") + self.assertEqual(out["action"], "completion-refused-resumed") + + +if __name__ == "__main__": + unittest.main()