Skip to content

fix(multi-review): clean stale git lock files#89

Closed
Svtter wants to merge 1 commit into
mainfrom
test/multi-review-ci
Closed

fix(multi-review): clean stale git lock files#89
Svtter wants to merge 1 commit into
mainfrom
test/multi-review-ci

Conversation

@Svtter

@Svtter Svtter commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Clean stale .git/*.lock files before spawning opencode CLI processes to prevent shallow.lock race conditions between parallel reviewers

Context

PR #87 和之前的 runs 中,multi-review CI 连续 3 次因 shallow.lock 竞态失败。并行 reviewer 在同一 git 仓库中同时执行 git fetch 导致锁文件冲突。

Test plan

  • multi-review CI 通过(无 shallow.lock 错误)
  • 其他 CI checks 正常通过

Remove stale .git/*.lock files before spawning opencode CLI processes
to prevent shallow.lock race conditions between parallel reviewers.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

有条件合并

该 PR 通过在 _run_opencode() 中调用 opencode 前清理 .git/*.lock 文件,意图解决并行 reviewer 之间的 shallow.lock 竞态问题。改进方向正确,但实现存在一些需要修复的问题。

阻塞项

  1. glob.glob(".git/*.lock") 使用相对路径 — CWD 不一定是仓库根目录。如果 CI 环境的工作目录发生变化,glob 将找不到 .git 目录,锁清理失效。应使用 git rev-parse --git-dirPath(working_directory) / ".git" 解析绝对路径。

  2. 可能删除其他 reviewer 正在使用的锁文件 — 在 Linux 上,os.remove 可以删除另一个进程已打开的文件(unlink 操作不会被 fd 阻止)。如果 reviewer A 的 opencode 进程正在执行 git fetch 并持有 shallow.lock,reviewer B 的清理操作会删除该锁,导致 reviewer A 的 git 操作在锁被底层删除后产生未定义行为,可能损坏 .git/shallow 状态。建议添加时间判断(如 os.path.getmtime 检查文件是否超过一定时间),确保只清理真正 stale(如 > 60 秒)的锁文件。

建议项

  1. 更彻底的解决方案是为每个 reviewer 创建独立的 git worktree 或临时 clone,从根源上避免共享 .git 目录的锁竞争
  2. 可考虑使用 os.path.islink 判断 .git 是否是一个 gitlink 文件(submodule 场景),增加兼容性

New%20session%20-%202026-05-21T03%3A41%3A05.270Z
opencode session  |  github run

@github-actions

Copy link
Copy Markdown

发现遗漏

PR 在 _run_opencode 中添加了清理 .git/*.lock 的逻辑,但未完整实现其声称的目标。

CRITICAL

并行 reviewer 间竞态条件未解决 — PR 描述的问题是"并行 reviewer 在同一 git 仓库中同时执行 git fetch 导致锁文件冲突",清理 stale lock 仅能移除之前运行遗留的锁文件,无法阻止当前运行的多个 reviewer 同时操作 git 时创建新锁(例如 reviewer A 清理后立即执行 git fetch 创建 shallow.lock,reviewer B 同时执行 git fetch 依然会冲突)。需要额外的序列化机制(如文件锁互斥)或独立 git 工作副本才能根除竞态。相关描述见 PR body 第二段。应在 run_reviewerrun-multi-review.py:226)或 _main 中 Reviewer 并发循环之前增加同步机制。

MEDIUM

清理路径缺少对工作目录的适配glob.glob(".git/*.lock") 固定使用 Python 进程的当前工作目录(CWD),而 MULTI_REVIEW_WORKING_DIRECTORY 输入(action.yml:37-40)允许配置 opencode CLI 的工作目录。如果 opencode CLI 根据 OPENCODE_WORKING_DIRECTORY 内部 cd 到子目录后再执行 git 操作,锁清理与 git 操作就可能指向不同目录。虽然标准 CI 环境下(CWD = repo root)此问题不触发,但配置了 working-directory 输入时存在隐式不一致。应在 _run_opencode 中根据 OPENCODE_WORKING_DIRECTORY 解析正确的 .git 路径。

New%20session%20-%202026-05-21T03%3A41%3A05.363Z
opencode session  |  github run

@github-actions

Copy link
Copy Markdown

架构有疑虑

架构分析摘要

该 PR 包含两个变更:(1) 在 _run_opencode() 中增加 stale git lock 清理;(2) 在根目录新增 opencode.json 配置文件。

阻塞项:无

建议项

  1. scope creep:opencode.json 与 PR 主题无关
    PR 标题为 "clean stale git lock files",但 24/32 行变更(75%)来自新增 opencode.json(配置 reasoningEffort、thinking、git permission)。这是跨关注点的问题,应拆分为独立 PR,或在 PR 描述中说明理由。

  2. 并发防护放置位置不一致

    • run_reviewer() (L244):XDG 缓存隔离
    • _run_opencode() (L195-200):git lock 清理
      同属并发竞态防护,但分散在两个不同层级。第二笔 commit message 本身也指出应移入 run_reviewer()。建议统一放置以保持架构一致性。
  3. 工作目录路径假设
    glob.glob(".git/*.lock") 硬编码为 CWD 的相对路径。若 MULTI_REVIEW_WORKING_DIRECTORY 被设置,opencode 进程的 git 操作目录与 CWD 不同,锁清理将定位到错误目录。建议使用 working_directory 拼接 .git/*.lock 路径。

New%20session%20-%202026-05-21T03%3A42%3A00.323Z
opencode session  |  github run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant