Skip to content

Commit 6cd0988

Browse files
committed
fix(agent): fail closed on revision implementer error; refresh wire snapshot
- The wire handshake snapshot (tests_e2e/test_wire_protocol.py) was stale: this PR added the judge-minimum-diff and judge-overengineering-review bundled skills but never regenerated the handshake skills list, leaving CI red on test_initialize_handshake / test_initialize_external_tool_conflict. Regenerated to include both new skills. - ImplementAndJudge: an implementer error on the revision now fails closed to BLOCKED, clearing the prior revision's stale NEEDS_WORK verdict and artifact so they can't leak into the final result (mirrors the judge-error branch). Adds a regression test.
1 parent 2b66c09 commit 6cd0988

3 files changed

Lines changed: 46 additions & 0 deletions

File tree

src/pythinker_code/tools/agent/__init__.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,7 +1450,13 @@ async def __call__(self, params: ImplementAndJudgeParams) -> ToolReturnValue:
14501450
)
14511451
last_implementer_output = self._child_result_output(impl_result)
14521452
if impl_result.is_error:
1453+
# Fail closed: an implementer error on a revision must not let the
1454+
# prior revision's NEEDS_WORK verdict or artifact leak into the
1455+
# final result. Reset to BLOCKED, mirroring the judge-error branch.
14531456
last_implementer_error = impl_result.message
1457+
last_verdict = "BLOCKED"
1458+
last_verdict_raw = None
1459+
last_artifact = None
14541460
break
14551461

14561462
last_artifact = _extract_coding_artifact(last_implementer_output)

tests/core/test_implement_judge_chain.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,26 @@ async def test_chain_needs_work_hits_cap(runtime: Runtime, monkeypatch: pytest.M
374374
assert [c[0] for c in calls].count("implementer") == 2
375375

376376

377+
async def test_chain_revision_implementer_error_resets_to_blocked(
378+
runtime: Runtime, monkeypatch: pytest.MonkeyPatch
379+
) -> None:
380+
"""An implementer error on the revision fails closed to BLOCKED — the prior
381+
revision's NEEDS_WORK verdict and artifact must not leak into the result.
382+
"""
383+
tool, _calls = _make_chain(
384+
runtime,
385+
monkeypatch,
386+
[_ok(_ARTIFACT_OUTPUT), _ok(_JUDGE_NEEDS_WORK), _err("implementer exploded on revision")],
387+
)
388+
with tool_call_context("ImplementAndJudge"):
389+
result = await tool(ImplementAndJudgeParams(brief="do x"))
390+
assert result.is_error is True
391+
assert result.extras is not None and result.extras["verdict"] == "BLOCKED"
392+
assert "implementer exploded on revision" in result.output
393+
# The superseded rev-0 artifact must not be presented as the current one.
394+
assert "coding_artifact: (missing" in result.output
395+
396+
377397
async def test_chain_implementer_error_blocks(
378398
runtime: Runtime, monkeypatch: pytest.MonkeyPatch
379399
) -> None:

tests_e2e/test_wire_protocol.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,16 @@ def test_initialize_handshake(tmp_path) -> None:
155155
"description": "Implement one or more checked-in specs using scout-plan-implement-verify workflow.",
156156
"aliases": [],
157157
},
158+
{
159+
"name": "skill:judge-minimum-diff",
160+
"description": "Reduction-ladder and minimum-diff checks the Pythinker judge subagent applies to every non-trivial diff.",
161+
"aliases": [],
162+
},
163+
{
164+
"name": "skill:judge-overengineering-review",
165+
"description": "Over-engineering review checklist the parent runs before declaring non-trivial code changes done.",
166+
"aliases": [],
167+
},
158168
{
159169
"name": "skill:pr-walkthrough",
160170
"description": "Produce a concise reviewer-friendly walkthrough of a PR or diff, including changed areas, behavior, tests, and risks.",
@@ -370,6 +380,16 @@ def test_initialize_external_tool_conflict(tmp_path) -> None:
370380
"description": "Implement one or more checked-in specs using scout-plan-implement-verify workflow.",
371381
"aliases": [],
372382
},
383+
{
384+
"name": "skill:judge-minimum-diff",
385+
"description": "Reduction-ladder and minimum-diff checks the Pythinker judge subagent applies to every non-trivial diff.",
386+
"aliases": [],
387+
},
388+
{
389+
"name": "skill:judge-overengineering-review",
390+
"description": "Over-engineering review checklist the parent runs before declaring non-trivial code changes done.",
391+
"aliases": [],
392+
},
373393
{
374394
"name": "skill:pr-walkthrough",
375395
"description": "Produce a concise reviewer-friendly walkthrough of a PR or diff, including changed areas, behavior, tests, and risks.",

0 commit comments

Comments
 (0)