diff --git a/doc/architecture/SPEC/engine/steps/pr.md b/doc/architecture/SPEC/engine/steps/pr.md index d931a2b78..e48700ebc 100644 --- a/doc/architecture/SPEC/engine/steps/pr.md +++ b/doc/architecture/SPEC/engine/steps/pr.md @@ -1,6 +1,6 @@ # engine/steps/pr/ —— 规范 - + `LOC ~1390(6 个文件) · step 库(PR) · refactor-status: ok` @@ -30,7 +30,7 @@ `extract_signature`(由 `utils` 再导出;供测试使用)。 ## 不变量 -- `ci.push` 把全部安全判断委托给 `guard_push`(**C4**)。 +- `ci.push` 对 `pr_review` / `pr_quality` 无条件拒绝推送;其他任务仍委托给 `guard_push`(**C4**)。 - **`pr.fetch_diff`:一个 head 统治一切**(PR2 重构后)。head 由 `_resolve_pr_head` **恰好解析一次**,stale 门、fetch、diff、worktree 全部 从这一个答案推导(`_fetch_at_one_head`)。fetch 走 run 域强制目的 ref @@ -49,6 +49,7 @@ 事件 `expected_head_mismatch`,`contract.build_review_result` 据此上报 stale);且每条"降级回 live checkout"的路径都变成硬 BLOCK —— 钉了快照 还静默评错树,比停下更糟。 +- **`pr.post_review` 始终使用 `event="COMMENT"`**,模型 verdict 不授予批准或阻塞 PR 的权限。 - **`pr.post_review` 只发一条 GitHub review + inline thread**,且先把每条发现的位置 对照已抓取的 diff 校验过 —— **绝不是一串独立评论**。 - `diff_text` 和 `gate_report` 都可以经 state 注入,因此网络之下的每条路径都可离线测试; diff --git a/src/infermatrix_copilot/engine/steps/pr/publish.py b/src/infermatrix_copilot/engine/steps/pr/publish.py index 3550acb98..a6262fa3c 100644 --- a/src/infermatrix_copilot/engine/steps/pr/publish.py +++ b/src/infermatrix_copilot/engine/steps/pr/publish.py @@ -16,7 +16,6 @@ from ...step import FailureKind, StepContext, StepResult, StepSpec from ..review.anchor import diff_index from ..review.anchor import normalize_path as _normalize_path -from ..review.utils import _review_verdict from .._common import gh as _gh from .._common import register_step, step from .._common import repo_path as _repo_path @@ -36,6 +35,9 @@ async def _push(ctx: StepContext) -> StepResult: (the default), it stays a dry run — reports the command it *would* run, never executes. Only with pushes enabled does it run the git command; a non-zero exit returns ESCALATE with the stderr tail.""" + if _task_spec(ctx).get("kind") in {"pr_review", "pr_quality"}: + return StepResult(False, FailureKind.FORBIDDEN, + "Review and quality tasks have comment-only authority") repo = _repo_path(ctx) raw = ctx.state.get("push_policy") policy = raw if isinstance(raw, PushPolicy) else PushPolicy(**(raw or {})) @@ -228,22 +230,8 @@ def render(comment: dict) -> str: def _event_for_review(comments: list[dict], pr_state: str = "", review_text: str = "") -> str: - """Translate the product verdict to a submitted GitHub review event.""" - if comments: - verdict = _review_verdict(comments, pr_state) - else: - match = re.search( - r"\*\*Verdict:\*\*\s*(REQUEST CHANGES|COMMENT|APPROVE)", - review_text or "", re.IGNORECASE) - verdict = match.group(1).upper() if match else _review_verdict([], pr_state) - event = { - "REQUEST CHANGES": "REQUEST_CHANGES", - "COMMENT": "COMMENT", - "APPROVE": "APPROVE", - }.get(verdict, "COMMENT") - if str(pr_state).upper() != "OPEN" and event != "COMMENT": - return "COMMENT" - return event + """A review assessment never changes GitHub approval/blocking state.""" + return "COMMENT" def _review_payload(state: dict, *, current_head: str = "") -> tuple[dict, int]: diff --git a/test/test_pr_steps.py b/test/test_pr_steps.py index 956e6aef9..c154ba2df 100644 --- a/test/test_pr_steps.py +++ b/test/test_pr_steps.py @@ -225,7 +225,7 @@ def test_review_payload_validates_diff_lines_and_preserves_fallbacks(): } payload, downgraded = _review_payload(state) - assert payload["event"] == "REQUEST_CHANGES" + assert payload["event"] == "COMMENT" assert payload["commit_id"] == "a" * 40 assert payload["comments"] == [{ "path": "a.py", @@ -259,9 +259,9 @@ def test_review_payload_downgrades_all_comments_when_head_changed(): @pytest.mark.parametrize( ("comments", "pr_state", "review_text", "event"), [ - ([_finding("a.py", 10, "major")], "OPEN", "", "REQUEST_CHANGES"), + ([_finding("a.py", 10, "major")], "OPEN", "", "COMMENT"), ([_finding("a.py", 10, "minor")], "OPEN", "", "COMMENT"), - ([], "OPEN", "**Verdict:** APPROVE", "APPROVE"), + ([], "OPEN", "**Verdict:** APPROVE", "COMMENT"), ([_finding("a.py", 10, "major")], "MERGED", "", "COMMENT"), ], ) @@ -312,7 +312,7 @@ def fake_gh(args, cwd=None): assert len(captured["payload"]["comments"]) == 1 assert result.outputs["inline"] == 1 assert result.outputs["downgraded"] == 1 - assert result.outputs["event"] == "REQUEST_CHANGES" + assert result.outputs["event"] == "COMMENT" assert result.outputs["url"].endswith("#review") @@ -698,3 +698,17 @@ def test_fetch_diff_matching_expected_head_proceeds(settings, trace, tmp_path, assert result.ok, result.summary assert result.outputs["state_updates"]["pr_head_sha"] == sha + + +@pytest.mark.parametrize("kind", ["pr_review", "pr_quality"]) +def test_review_tasks_cannot_push_even_with_allow_push(registry, settings, trace, tmp_path, monkeypatch, kind): + from infermatrix_copilot.engine.steps.pr import publish + settings.allow_push = True + state = {"repo_path": str(tmp_path), "task_spec": {"kind": kind}, + "push_policy": PushPolicy(allowed=True, remote="origin", branch="feature")} + def unexpected(*args, **kwargs): + raise AssertionError("review task attempted a git command") + monkeypatch.setattr(publish.subprocess, "run", unexpected) + result = asyncio.run(registry.get("ci.push").handler(_ctx(settings, trace, tmp_path, state))) + assert not result.ok + assert "comment-only" in result.summary