From 14d11bf062bbca16e334f91e25b6b5dcd1c2c317 Mon Sep 17 00:00:00 2001 From: Godopu Date: Thu, 24 Sep 2026 03:38:10 +0000 Subject: [PATCH] fix(issue-36): a job that cannot be registered after a rebuttal ends the loop with rc 1 Review of f5cadd9 (planner-reviewer-codex-01, R1): the compliance job's registration failure was only logged, so a passing next round ended the real loop process with rc 0. Compliance now returns 1 when it cannot be registered or does not finish, as the correction step already does. Re-adjudication and arbitration also return 1 when their job cannot be registered; a registered job that yields no verdict keeps the designed fail-closed fallback. S4c (stubbed delegate_job, a fixture whose later rounds never pass) is replaced by S3c: a real `python -m lib_py.loop` process whose wrapper runs an executable that simulates the agents, refusing the compliance, re-adjudication or arbitration registration while the next round passes. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib_py/loop/runner.py | 16 +++- tests/test_issue36_prompt_transport.py | 121 +++++++++++++++---------- 2 files changed, 84 insertions(+), 53 deletions(-) diff --git a/.agents/skills/lib_py/loop/runner.py b/.agents/skills/lib_py/loop/runner.py index 6d5bdff..cf9502a 100644 --- a/.agents/skills/lib_py/loop/runner.py +++ b/.agents/skills/lib_py/loop/runner.py @@ -1350,7 +1350,11 @@ class LoopRunner: adj_prompt = f"작업자(Creator)가 귀하의 리뷰 지적 중 일부를 타당하지 않다고 판단하여 반론을 제기했습니다. 반론을 검토하고 재심(re-adjudication)하십시오. 반론이 타당하여 귀하의 지적을 철회한다면 리포트 마지막에 단독 행으로 '[ADJUDICATION: SUSTAINED]' 를, 반론이 타당하지 않아 지적을 유지한다면 '[ADJUDICATION: OVERRULED]' 를 명시하십시오. 반론 내용:\n{rebuttal_text}" adj_jid = self._dispatch(rt, rt_type, "Reviewer", adj_prompt, remote=rt in self._remote_recipients, purpose="adjudication") - if not adj_jid or not self.wait_for_job(adj_jid): + if not adj_jid: + # a job that was never registered is a failure, not a verdict (Issue #36) + log_error(f"Failed to register re-adjudication job for '{rt}'.") + return 1 + if not self.wait_for_job(adj_jid): log_warn(f"Re-adjudication job for '{rt}' failed; objection stands.") overruled_revs.append(rt) continue @@ -1369,7 +1373,10 @@ class LoopRunner: p_type = self.resolve_agent_type(planner_session, state) arb_prompt = f"작업자(Creator)와 리뷰어의 의견이 충돌하여 교착 상태입니다. 계획서 관점에서 어느 쪽이 타당한지 재정(arbitration)하십시오. 리포트 마지막에 단독 행으로 '[ARBITRATION: CREATOR]' 또는 '[ARBITRATION: REVIEWER]' 를 명시하십시오. 리뷰어 지적:\n{feedback_aggregate}\n작업자 반론:\n{rebuttal_text}\n기존 계획서:\n{self.current_plan}" arb_jid = self._dispatch(planner_session, p_type, "Planner", arb_prompt) - if arb_jid and self.wait_for_job(arb_jid): + if not arb_jid: + log_error("Failed to register Planner arbitration job.") + return 1 + if self.wait_for_job(arb_jid): arb_file = self.find_report(arb_jid) if arb_file and parser.has_tag_line(arb_file, "ARBITRATION: CREATOR"): arbitration = "CREATOR" @@ -1383,10 +1390,9 @@ class LoopRunner: rebut_budget = 0 comply_prompt = f"귀하의 반론은 재심 결과 기각되었습니다. 더 이상 반론하지 말고 다음 리뷰 피드백을 그대로 반영하여 코드를 수정하십시오. 피드백:\n{feedback_aggregate}" comp_jid = self._dispatch(self.creator, creator_type, "Worker", comply_prompt) - if comp_jid: - self.wait_for_job(comp_jid) - else: + if not comp_jid or not self.wait_for_job(comp_jid): log_error("Failed to register Creator compliance job.") + return 1 # Promotion if self.plan_mode and self.current_plan and self.plan_job_id: diff --git a/tests/test_issue36_prompt_transport.py b/tests/test_issue36_prompt_transport.py index 388ae58..a208136 100644 --- a/tests/test_issue36_prompt_transport.py +++ b/tests/test_issue36_prompt_transport.py @@ -9,7 +9,7 @@ The prompt now goes by file: runner -> wrapper (--prompt-file) -> registry (--prompt-file). These tests push prompts over the cap through the real bash wrapper, the real registry and a real `python -m lib_py.loop` process; subprocess is replaced only where a test is about the error handling itself -(S4, S4b, S4c), never to prove the transport. +(S4, S4b), never to prove the transport. Every test writes only under tmp_path. """ @@ -297,58 +297,83 @@ def test_s4b_an_exec_error_on_the_busy_retry_is_reported_not_raised(runner, tmp_ # --------------------------------------------------------------------------- -# S4c - a compliance job that cannot be registered is logged +# S3c - a job that cannot be registered after the rebuttal ends the loop with +# rc 1, even when the next review round would pass (real loop process; only +# the agents' outcomes are simulated, by an executable behind the wrapper) # --------------------------------------------------------------------------- -def _drive_to_compliance(lr, r, monkeypatch, comply_jid): - """Self-review PASS, cross review NOT PASS, the correction rebuts, the - reviewer overrules, and the compliance delegation returns comply_jid.""" - stub = Path(r.workspace_root) / "stub" - stub.mkdir(exist_ok=True) - files = {} - - def fake_delegate(session, agent_type, role, prompt): - jid = f"j{len(files)}" - if "재심 결과 기각" in prompt: - return comply_jid - if prompt.startswith(lr.SELF_REVIEW_PROMPT) or lr.SELF_REVIEW_PROMPT in prompt: - body = "ok\n\n[VERDICT: PASS]\n" - elif "re-adjudication" in prompt: - body = "no\n\n[ADJUDICATION: OVERRULED]\n" - elif "[이의제기 채널]" in prompt: - body = "I disagree\n\n[REBUT: r]\n" - elif role == "Reviewer": - body = "line 3 is wrong\n\n[VERDICT: NOT PASS]\n" - else: - body = "done\n" - f = stub / f"{jid}.md" - f.write_text(body, encoding="utf-8") - files[jid] = f - return jid - - monkeypatch.setattr(lr.signal, "signal", lambda *a, **k: None) - monkeypatch.setattr(r, "delegate_job", fake_delegate) - monkeypatch.setattr(r, "wait_for_job", lambda *a, **k: True) - monkeypatch.setattr(r, "find_report", lambda jid: files.get(jid)) - return r.run() +FAKE_AGENT = r""" +import json, sys +from pathlib import Path +args = sys.argv[3:] # argv[1] = kind to refuse, argv[2] = "submit" +opts = dict(zip(args[::2], args[1::2])) +prompt = Path(opts["--prompt-file"]).read_text(encoding="utf-8") +refuse = sys.argv[1] +jobs = Path(opts["--registry-dir"]) +jobs.mkdir(parents=True, exist_ok=True) +kind = ("comply" if "재심 결과 기각" in prompt else + "adjudication" if "re-adjudication" in prompt else + "arbitration" if "재정(arbitration)" in prompt else + "correction" if "[이의제기 채널]" in prompt else + opts["--role"]) +if kind == refuse: + print(kind + " registration refused", file=sys.stderr) + sys.exit(2) +if kind == "adjudication": + report = "no\n\n[ADJUDICATION: OVERRULED]\n" +elif kind == "arbitration": + report = "reviewer is right\n\n[ARBITRATION: REVIEWER]\n" +elif kind == "correction": + report = "I disagree\n\n[REBUT: r]\n" +elif kind == "Reviewer" and "Self-Review" in prompt: + report = "ok\n\n[VERDICT: PASS]\n" +elif kind == "Reviewer": # the first cross review objects, every later one passes + counter = Path.cwd() / "review-count" + n = int(counter.read_text()) + 1 if counter.exists() else 1 + counter.write_text(str(n)) + report = "line 3 is wrong\n\n[VERDICT: NOT PASS]\n" if n == 1 else "ok\n\n[VERDICT: PASS]\n" +elif kind == "Planner": + report = "# plan\n\ndo it\n" +else: + report = "done\n" +jid = "j%d" % len(list(jobs.glob("*.json"))) +(jobs / (jid + ".json")).write_text(json.dumps({"job_id": jid, "status": "completed"})) +dest = jobs / jid / "agent-reports" +dest.mkdir(parents=True) +(dest / "report-final.md").write_text(report, encoding="utf-8") +print("registered job: " + jid) +""" -@pytest.mark.parametrize("comply_jid", [None, "jc"]) -def test_s4c_a_failed_compliance_delegation_is_logged(tmp_path, monkeypatch, capsys, comply_jid): +@pytest.mark.parametrize("refuse, message", [ + ("comply", "Failed to register Creator compliance job."), + ("adjudication", "Failed to register re-adjudication job for 'r'."), + ("arbitration", "Failed to register Planner arbitration job."), +]) +def test_s3c_a_registration_failure_after_a_rebuttal_is_never_a_success(tmp_path, refuse, message): ctl = _repo(tmp_path / "ctl") - for k, v in _loop_env(ctl).items(): - if k in ("WORKSPACE_ROOT", "MAM_JOBS_DIR", "MAM_STATE_JSON"): - monkeypatch.setenv(k, v) - for k in ("MAM_REAL_ROOT", "MAM_LOOP_WORKTREE", "MAM_LOOP_BASE"): - monkeypatch.delenv(k, raising=False) - from lib_py.loop import runner as lr - # two rounds: the correction (and so the rebuttal) runs only between rounds - r = lr.LoopRunner(["--creator", "c", "--task", "t36", "--reviewer", "r", "--max-loop", "2"]) - rc = _drive_to_compliance(lr, r, monkeypatch, comply_jid) - out = capsys.readouterr() - assert "Creator must comply" in out.out + out.err - assert ("Failed to register Creator compliance job" in out.err) == (comply_jid is None) - assert rc == 1 # the review round was not passed either way; the flow is unchanged + agent = ctl / "fake-agent.py" + agent.write_text(FAKE_AGENT, encoding="utf-8") + _fake_wrapper(ctl, f'exec "{sys.executable}" "{agent}" {refuse} "$@"\n') + extra = {} + argv = ["--max-loop", "2"] + if refuse == "arbitration": # arbitration needs --plan and a planner + extra["MAM_STATE_JSON"] = json.dumps({"herdr_sessions": [ + {"name": "c", "agent": "claude", "role": "creator", "status": "running"}, + {"name": "r", "agent": "claude", "role": "reviewer", "status": "running"}, + {"name": "p", "agent": "claude", "role": "planner", "status": "running"}]}) + argv += ["--plan", "--plan-talk", "0"] + else: + plan = ctl / "plan.md" + plan.write_text("# plan\n\ndo it\n", encoding="utf-8") + argv += ["--plan-file", str(plan)] + res = subprocess.run([sys.executable, "-m", "lib_py.loop", "--creator", "c", "--task", "t36", + "--reviewer", "r", *argv], + capture_output=True, text=True, env=_loop_env(ctl, extra), cwd=str(ctl), timeout=120) + assert message in res.stderr, res.stdout + res.stderr + assert res.returncode == 1, res.stdout + res.stderr + assert "100% PASS" not in res.stdout + assert "Traceback" not in res.stderr # ---------------------------------------------------------------------------