diff --git a/packages/review-gate/README.md b/packages/review-gate/README.md index 37f9799..b048ce1 100644 --- a/packages/review-gate/README.md +++ b/packages/review-gate/README.md @@ -2,27 +2,25 @@ 常驻轮询守护进程：监控配置仓库的 open PR，对每个新 head sha 调 bridge 的 `zcode_pr_review` 完成审查（git diff + mimosa 深扫 + ZCode 只读复核）， -把带 verdict（pass / concerns）的结果回贴为 PR 评论。同一 head sha 不重复审 -（state 文件去重），失败按指数退避重试。公开、通用，任何 GitHub 仓库可用。 +把带 verdict（pass / concerns / 需人工核对）的结果回贴为 PR 评论。同一 +head sha 不重复审（state 文件去重），失败按指数退避重试。审查前会把 +clone 工作区 **checkout 到被审 head sha 并回读校验**——mimosa 扫的是 +工作区文件，不 checkout 会扫在旧代码上（issue #17）。公开、通用，任何 +GitHub 仓库可用。 ``` -┌─────────────┐ 轮询 open PR ┌──────────────┐ -│ GitHub API │ ◄────────────── │ │ -└──────┬──────┘ │ │ - │ 新 head sha? │ review-gate │ (state 文件去重/退避) - ▼ │ │ - git clone/fetch ──────────────► │ │ - │ └──────┬───────┘ - ▼ │ --call zcode_pr_review -┌─────────────────┐ ▼ -│ zcode-mcp-server│ (git diff + mimosa 深扫 + ZCode 只读复核, -│ (子进程) │ 锁/重试/只读护栏全在 bridge 侧同源复用) -└──────┬──────────┘ - │ 报告 → 解析 P0/P1/P2 → verdict - ▼ -┌─────────────┐ -│ PR 评论 │ ✅ pass / ⚠️ concerns + 完整报告 (details 折叠) -└─────────────┘ +GitHub API ──轮询 open PR──► review-gate (state 文件去重/指数退避) + │ git clone/fetch + ▼ + checkout 到被审 head sha + rev-parse 回读校验 (issue #17) + │ --call zcode_pr_review + ▼ + zcode-mcp-server 子进程 (git diff + mimosa 深扫 + ZCode 只读复核; + 锁/限流重试/只读护栏全在 bridge 侧同源复用) + │ 报告 → 解析 P0/P1/P2 → verdict + ▼ + PR 评论: ✅ pass / ⚠️ concerns / ❓ 需人工核对 + + 完整报告 (details 折叠) ``` ## 前置条件 @@ -166,7 +164,11 @@ zcode。 pass/concerns；解析失败时 fail-safe 为 **concerns**（宁错拦不错放）， 评论里会标注"严重度分布解析失败，请人工核对"。 - **单线程串行**：逐仓逐 PR 串行审查；并发安全靠 bridge mcp-server 侧的 - 跨进程文件锁兜底（多实例同时跑也不会并发打爆 zcode 限流）。 + 跨进程文件锁兜底（多实例同时跑也不会并发打爆 zcode 限流）。**同一 + state 文件（同一部署）只允许一个 gate 实例**：启动时对 + `.lock` 非阻塞 flock，拿不到锁直接退出（exit 2）——checkout + 发生在 bridge 锁之外，第二个实例会在第一个实例 mimosa 扫描中途换掉 + 工作区，静默扫错代码（狗食 review P1-1）。 - **fork PR**：走 `refs/pull/{n}/head` 拉取，无需加 fork 远端； 审查的是 PR head 快照本身。 - token 不落盘：经 git≥2.31 的 `GIT_CONFIG_COUNT/KEY/VALUE` 环境变量逐 diff --git a/packages/review-gate/zcode-review-gate b/packages/review-gate/zcode-review-gate index 21d0f58..7fb4f98 100755 --- a/packages/review-gate/zcode-review-gate +++ b/packages/review-gate/zcode-review-gate @@ -12,6 +12,11 @@ zcode-review-gate — PR 自动审查闸门守护进程 (zcode-open-bridge 第 4 不 import/exec mcp-server 源码, 更不重造这些机制。 - 同一 head sha 不重复审 (state 文件去重); 失败按指数退避重试, 超限 gave_up; head 更新 (force-push / 新 commit) 自动复活重审。 + - 审查前把 clone 工作区 checkout 到被审 head sha 并回读校验 (issue #17): + mimosa 扫的是工作区文件, 不 checkout 会扫在旧代码上 (误报已修/漏报新引)。 + checkout 前先 git clean -fdx 清 untracked 残留, 基线严格一致。 + - 实例互斥: .lock 非阻塞 flock, 同一部署只允许一个实例 — + checkout 在 bridge 锁外, 第二实例会在审查中途换掉工作区 (狗食 P1-1)。 - token 不落盘: 只经 git≥2.31 的 GIT_CONFIG_COUNT/KEY/VALUE 环境变量 逐命令进程内注入 http.extraHeader (env 只对本用户可见, 优于 argv); clone URL / .git/config / state / config 文件里都没有 token。 @@ -23,6 +28,7 @@ zcode-review-gate — PR 自动审查闸门守护进程 (zcode-open-bridge 第 4 import argparse import base64 +import fcntl import json import os import re @@ -410,6 +416,62 @@ def fetch_pr_refs(clone, token, pr_number, base_ref): token=token) +def checkout_review_head(clone, token, head_sha): + """把 clone 工作区切到被审 PR 的确切 head sha, 返回回读到的 HEAD (issue #17)。 + + mimosa 扫描读的是工作区文件而非 git 对象库 — fetch 只更新对象、不动 + 工作区, 不 checkout 会扫在上一轮的旧代码上 (误报已修复的问题 / 漏报 + 新引入的问题)。先 clean 再 checkout: --force 只丢弃 tracked 改动, + untracked 残留会跨轮存活 (狗食 review P2-1), clone 为 gate 专有目录, + --force -d -x 清到严格基线。--detach 挂在 sha 上不动分支引用。 + checkout 后 rev-parse 回读校验, 防浅 clone 缺对象等静默失败让扫描 + 基线名不副实。 + """ + git(["-C", clone, "clean", "--force", "-d", "-x"], token=token) + git(["-C", clone, "checkout", "--force", "--detach", head_sha], + token=token) + actual = git(["-C", clone, "rev-parse", "HEAD"], token=token).strip() + if actual != head_sha: + raise GitError( + f"checkout 校验失败: 请求 {head_sha[:12]}, 工作区实际在 " + f"{actual[:12]} (mimosa 会扫错代码, 拒绝继续)") + return actual + + +def acquire_instance_lock(state_file): + """实例互斥锁 (狗食 review P1-1, PR #18): 同一 state 文件只允许一个实例。 + + checkout 发生在 bridge 侧文件锁之外 — 第二个实例 (如守护进程跑长审查 + 期间手工 --once 调试) 会在第一个实例 mimosa 扫描中途换掉工作区 + (TOCTOU), 静默扫错代码贴错 verdict。非阻塞 flock: 拿不到说明已有实例, + 返回 None (调用方报错退出); 拿到则持有到进程退出 (崩溃时 OS 自动回收)。 + 锁文件固定 .lock — state 路径即部署身份, 不同部署互不干扰。 + """ + path = os.path.expanduser(str(state_file)) + ".lock" + directory = os.path.dirname(path) + if directory: + os.makedirs(directory, exist_ok=True) + fd = os.open(path, os.O_CREAT | os.O_WRONLY, 0o600) + try: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError: + os.close(fd) + log(f"已有 gate 实例持有锁 ({path}), 本实例退出 — 禁止双实例共享 " + "同一 state/clone_root (审查中途换工作区会扫错代码)", "ERROR") + return None + return fd + + +def release_instance_lock(fd): + """释放实例锁 (正常退出路径; 崩溃时 OS 回收, 不依赖此处)。""" + if fd is None: + return + try: + fcntl.flock(fd, fcntl.LOCK_UN) + finally: + os.close(fd) + + # ============================================================ # 审查调用 (经 mcp-server --call 子进程) # ============================================================ @@ -537,7 +599,7 @@ def build_comment_body(verdict, counts, head_sha, report, max_body): f"## ZCode Review Gate：{icon}\n\n" "| 项 | 值 |\n" "|---|---|\n" - f"| head | `{head_sha[:12]}` |\n" + f"| head（mimosa 扫描基线） | `{head_sha[:12]}` |\n" f"| 严重度分布 | {sev} |\n" f"| 结论 | {conclusion} |\n" "\n" @@ -724,6 +786,13 @@ def process_pr(cfg, state, token, owner, repo, clone, info): except GitError as e: mark_failure(entry, f"git fetch 失败: {e}", now, cfg) return + try: + # issue #17: mimosa 扫工作区文件, 必须先把工作区切到被审 head, + # 否则 fetch 后仍停在上轮代码上 (误报已修/漏报新引) + checkout_review_head(clone, token, head_sha) + except GitError as e: + mark_failure(entry, f"git checkout 到被审 head 失败: {e}", now, cfg) + return log(f"{key}: 开始审查 head={head_sha[:12]} " f"base=origin/{info['base_ref']} depth={cfg.review_depth}") @@ -908,20 +977,28 @@ def main(argv=None): return 2 pr_filter = set(args.prs) or None - signal.signal(signal.SIGTERM, _on_signal) - signal.signal(signal.SIGINT, _on_signal) - - if args.once: - run_once(cfg, pr_filter) + # 实例互斥 (狗食 review P1-1): checkout 在 bridge 锁外, 双实例共享 + # state/clone_root 会在审查中途换工作区 — 拿不到锁直接退出 + lock_fd = acquire_instance_lock(cfg.state_file) + if lock_fd is None: + return 2 + try: + signal.signal(signal.SIGTERM, _on_signal) + signal.signal(signal.SIGINT, _on_signal) + + if args.once: + run_once(cfg, pr_filter) + return 0 + + log(f"进入守护循环: repos={cfg.repos} poll_interval={cfg.poll_interval}s " + f"state={cfg.state_file} clone_root={cfg.clone_root}") + while not _SHUTDOWN: + run_once(cfg, pr_filter) + _interruptible_sleep(cfg.poll_interval) + log("优雅退出") return 0 - - log(f"进入守护循环: repos={cfg.repos} poll_interval={cfg.poll_interval}s " - f"state={cfg.state_file} clone_root={cfg.clone_root}") - while not _SHUTDOWN: - run_once(cfg, pr_filter) - _interruptible_sleep(cfg.poll_interval) - log("优雅退出") - return 0 + finally: + release_instance_lock(lock_fd) if __name__ == "__main__": diff --git a/tests/test_review_gate.py b/tests/test_review_gate.py index 766f05b..58214a1 100644 --- a/tests/test_review_gate.py +++ b/tests/test_review_gate.py @@ -18,6 +18,9 @@ - state 原子写 (os.replace 被调) + 损坏恢复 + 往返 - 配置默认值 + env 覆盖 + 下限钳制 - git: token 经 GIT_CONFIG_* env 注入且 argv 无 token / GitError 不带 token + - checkout_review_head: clean+checkout+rev-parse 回读校验 (issue #17: + mimosa 扫工作区文件, 基线必须与被审 head 严格一致) + 实例互斥锁 + (狗食 review P1-1: 双实例退出码 2 不跑审查, 释放后可续跑) - ensure_clone: 半成品重建 / clone 失败清理 / web 宿主推导 - run_review: 坏 JSON / 空报告 / OSError / TimeoutExpired / 超时透传 / stderr 尾部 - mcp_server 解析: PATH 命中 / ~/.local/bin 回退 / 不可执行不用 / 原样兜底 @@ -32,6 +35,7 @@ import base64 import email.message +import fcntl import json import os import re @@ -551,6 +555,63 @@ def fake_run(cmd, *a, **kw): self.assertNotIn("secret-tok", repr(cm.exception)) +# ============================================================ +# checkout_review_head (issue #17: 扫描基线与被审 sha 严格一致) +# ============================================================ +class TestCheckoutHead(_GateCase): + SHA = "a" * 40 + + def _run(self, revparse_sha=None, checkout_rc=0): + calls = [] + + def fake_run(cmd, *a, **kw): + calls.append(list(cmd)) + if "clean" in cmd: + return _CP(returncode=0, stdout="", stderr="") + if "checkout" in cmd: + return _CP(returncode=checkout_rc, stdout="", stderr="boom") + if "rev-parse" in cmd: + return _CP(returncode=0, + stdout=(revparse_sha or self.SHA) + "\n", stderr="") + raise AssertionError(f"意外命令: {cmd}") + + with mock.patch.object(self.mod.subprocess, "run", fake_run): + try: + out = self.mod.checkout_review_head("/tmp/clone", "tok", self.SHA) + except self.mod.GitError: + out = None + return out, calls + + def test_ok_checkout_then_verify(self): + out, calls = self._run() + self.assertEqual(out, self.SHA) + # clean → checkout → rev-parse 回读, 三条命令都带 -C clone + # (clean 清 untracked 残留, 狗食 review P2-1: --force 只管 tracked) + self.assertEqual(calls[0], ["git", "-C", "/tmp/clone", "clean", + "--force", "-d", "-x"]) + self.assertEqual(calls[1], ["git", "-C", "/tmp/clone", "checkout", + "--force", "--detach", self.SHA]) + self.assertEqual(calls[2], ["git", "-C", "/tmp/clone", + "rev-parse", "HEAD"]) + + def test_revparse_mismatch_raises(self): + # checkout 声称成功但 HEAD 不在请求的 sha 上 → GitError 拒绝继续 + # (防浅 clone 缺对象等静默失败让 mimosa 扫错代码) + out, _ = self._run(revparse_sha="b" * 40) + self.assertIsNone(out) + + def test_checkout_failure_raises(self): + out, _ = self._run(checkout_rc=1) + self.assertIsNone(out) + + def test_clean_precedes_checkout(self): + # 狗食 review P2-1: untracked 残留跨轮存活, mimosa 扫工作区文件 + # → clean 必须先于 checkout + _, calls = self._run() + subs = [c[3] for c in calls] + self.assertLess(subs.index("clean"), subs.index("checkout")) + + # ============================================================ # ensure_clone: 自愈 / 清理 / web 宿主推导 # ============================================================ @@ -807,6 +868,13 @@ def _fake_run(self, cmd, *a, **kw): if "clone" in cmd: # 假 clone 也要建出 .git, 否则每轮都重复 clone os.makedirs(os.path.join(cmd[-1], ".git"), exist_ok=True) + if "checkout" in cmd: + # issue #17: 记住工作区切到的 sha, 供 rev-parse HEAD 回读 + self.checked_out = cmd[-1] + if "rev-parse" in cmd and cmd[-1] == "HEAD": + return _CP(returncode=0, + stdout=getattr(self, "checked_out", "") + "\n", + stderr="") return _CP(returncode=0, stdout="", stderr="") if "--call" in cmd: idx = cmd.index("--call") @@ -871,6 +939,15 @@ def test_full_cycle_dedup_and_new_sha(self): self.assertEqual(call_args["depth"], "deep") self.assertTrue(call_args["path"].endswith("octo__hello")) + # issue #17: 审查链路里出现了 checkout --force --detach 到被审 sha + # (process_pr 未接线则 git_calls 里不会有 checkout 命令) + checkout_cmds = [c for c, _ in self.git_calls if "checkout" in c] + self.assertEqual(len(checkout_cmds), 1) + self.assertIn("--force", checkout_cmds[0]) + self.assertIn("--detach", checkout_cmds[0]) + self.assertEqual(checkout_cmds[0][-1], "a" * 40) + self.assertEqual(checkout_cmds[0][-1], self.mcp_calls[0]["head"]) + # 评论体: pass + 严重度分布 + 署名 body = posts[0][2]["body"] self.assertIn("✅ pass", body) @@ -1021,6 +1098,24 @@ def test_pr_filter(self): self.assertIn("octo/hello#6", self._state()) self.assertNotIn("octo/hello#5", self._state()) + def test_instance_lock_blocks_second_run(self): + """狗食 review P1-1 (PR #18): 已有实例持锁 → 第二实例退出码 2 且 + 不跑任何审查; 锁释放后同部署可正常续跑""" + cfg_path = self._write_config() + state = os.path.join(self.tmp, "state.json") + lock_path = state + ".lock" + fd = os.open(lock_path, os.O_CREAT | os.O_WRONLY, 0o600) + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + try: + self.assertEqual(self._run_once(cfg_path), 2) + self.assertEqual(len(self.mcp_calls), 0) # 没跑审查 + self.assertEqual(len(self.api_calls), 0) # 没拉 PR 列表 + finally: + os.close(fd) # 释放锁 + # 释放后同进程再跑 → 正常走完一轮 + self.assertEqual(self._run_once(cfg_path), 0) + self.assertEqual(len(self.mcp_calls), 1) + if __name__ == "__main__": unittest.main()