fix: review action no longer fails on opencode session-share push denial (#129)

[Svtter]

Fixes #129.

Problem

The review action advertises contents: read as its documented permission, but the job still exits 1 when opencode's built-in 'share session' git push step is denied by the runner's token scope. The review comment itself was already posted via the API before the push attempt, so the failure is non-fatal — it just shouldn't be propagated.

Fix

run-github-opencode.py now detects the specific push-denied markers in the captured log and converts the non-zero exit to 0 with a ::warning:::

• Write access to repository not granted
• Command failed with code 128: git push
• fatal: unable to access '...': The requested URL returned error: 403

This is consistent with the existing cleanup-error-comments logic, which already auto-deletes the error comment that opencode posts in this scenario — the wrapper was already acknowledging this class of failure as expected, it just wasn't reflecting that in its exit code.

Tests

• test_push_denied_treated_as_success: 403 push in single-model path → exit 0
• test_push_denied_with_fallback_skips_fallback: 403 push in primary → exit 0, fallback not consulted
• test_unrelated_error_not_treated_as_push_denied: a genuine non-push error (e.g. deadline exceeded) still propagates

The fake opencode fixture gained FAKE_OPENCODE_PUSH_DENIED_MODELS to exercise the new path.

Files

• github-run-opencode/run-github-opencode.py — detection + exit 0 logic
• tests/fixtures/fake-installer.sh — FAKE_OPENCODE_PUSH_DENIED_MODELS
• tests/test_all.py — 3 new tests
• CHANGELOG.md — entry under [Unreleased]

[github-actions]

最终决策：可合并 / CAN MERGE

无阻塞项，所有 reviewer 建议均为非阻塞改进。以下为去重整合后的问题。

阻塞项：无

建议项：

1. run_single() 改用 PIPE 捕获导致丢失实时流式、stderr 混入 stdout（已确认 — quality、performance、architecture 一致指出）
   原先实时透传子进程输出的行为改为全量缓冲后一次性写入，不仅失去实时性（长耗时场景用户看不到中间输出，CI 可能因无日志提前 kill），还将 stderr 与 stdout 合并。建议使用 Popen + 边读边写的方式，在保持实时输出的同时检测错误模式；或仅在 returncode != 0 时读取输出用于判别。

2. _is_push_denied_failure() 无法排除 push-denied 与其他真实错误共存（已确认 — security、architecture 一致指出）
   只要输出中包含任一 push-denied 模式即返回 True，若子进程同时输出了其他真实错误（如超时、模型调用失败），真实错误会被掩盖，作业标记为成功。建议确认输出中不含其他非预期 error/fatal 模式，或推动 opencode 侧提供语义化退出码。

3. _is_push_denied_failure docstring 与实际行为不一致（quality）
   docstring 声称该函数检查"唯一有意义的失败"，但实现仅做模式存在判断，未验证唯一性。建议修正 docstring。

4. PUSH_DENIED_PATTERNS 硬编码 opencode 内部错误文案（architecture）
   若 opencode 后续版本变更错误消息，action 将静默失效（该报错的场景变成成功退出）。建议由 opencode 提供结构化退出码或输出标记（如环境变量）替代文本匹配。

---

📋 各 Reviewer 详细审查结果

quality

可合并 / CAN MERGE

该 PR 正确实现了对 opencode session-share git push 因 token 权限不足（如 contents: read）而 403 失败时的优雅处理。整体代码质量良好，逻辑正确，测试覆盖充分。

阻塞项：无

建议项：

1. _is_push_denied_failure 的 docstring 声称检查的是"唯一有意义的失败"（"the only meaningful failure"），但实现只判断模式是否存在（而非唯一性）。建议修正 docstring 使其与实际行为一致，或者加强逻辑确保确实没有其他错误。
2. run_single 中将子进程 stderr 重定向到 stdout（stderr=subprocess.STDOUT），导致原本流向 stderr 的日志合并到 stdout 输出。建议考虑分别捕获 stdout 和 stderr，在检测 push-denied 模式时合并检查，但写入父进程时保持原有 stdout/stderr 分离，以保留 GitHub Actions 中 stderr 的自然标注行为。
3. run_single 改为捕获输出后统一写入（sys.stdout.write(result.stdout)），不再实时流式输出。对于长耗时模型运行，用户可能在执行期间看不到任何输出。可考虑改用逐行读取或定期 flush 的方式保持实时性。

security

存在风险 / AT RISK

该 PR 本身目的明确——在 opencode session-share 的 git push 因 token 权限不足（contents: read）被拒时，不阻塞 CI 流程。代码质量良好，没有注入、凭证泄露、认证绕过等明显漏洞。但对检查逻辑有一个低风险建议。

详细分析

已确认无风险项

• 命令注入：subprocess.run 使用列表传参（无 shell=True），不存在 shell 注入。
• 敏感信息泄露：没有记录 token、密钥等敏感信息。
• 认证/授权：正确处理了权限不足的场景，不会静默绕过认证。
• 依赖安全：未引入新的不安全依赖。

关注项

阻塞项：无

建议项：

1. _is_push_denied_failure 可能掩盖真正错误（逻辑不完全严谨）
   函数仅检查输出中是否包含 push denied 模式串。如果子进程同时输出了真实错误（如超时、段错误）和 push denied 信息，函数仍会返回 True，导致真实错误被掩盖，作业被标记为成功。
   建议改为检查输出中是否只包含 push denied 类的错误，或者至少确认没有其他非预期的 error/fatal 模式。当前 test_unrelated_error_not_treated_as_push_denied 未覆盖"混合输出"场景。

2. stdout 内容拼入 stderr warning 消息时无截断保护

   "::warning::opencode session-share 'git push' was denied (e.g. contents:read). "

   这一行本身安全，但如果未来版本将 stdout 内容拼入 warning 消息时，若 stdout 包含恶意构造的内容（如换行注入），可能破坏 GitHub Actions 的日志解析。当前代码未直接拼接 stdout，仅为参考建议。

performance

性能有疑虑 / CONCERNS

本次 PR 新增了 run_single 函数中对子进程 stdout/stderr 的捕获（stdout=subprocess.PIPE, stderr=subprocess.STDOUT），并最终通过 sys.stdout.write(result.stdout) 写入。此前未捕获时，输出直接透传终端，支持实时流式输出；现在整个输出会被完整缓冲在 Python 进程中再一次性写出。若模型生成长篇 review（数万行文本），内存峰值会增大。此外，输出不再实时流式，用户需等待全量完成才看到内容。

其他变更（模块级编译的正则匹配、run_model 中已有的捕获模式、测试用例）均无明显性能问题。

阻塞项：无

建议项：

1. run_single 中捕获整个子进程输出到内存再写入，对大规模 review 场景增加内存开销。可考虑改用逐行/分块读取的方式降低峰值内存，或仅在需要检查 _is_push_denied_failure 时才捕获（即仅 returncode != 0 时读取输出），正常路径继续透传。
2. 捕获输出导致实时流式丢失（原先是直接继承父进程 stdout/stderr），如需保持用户体验，可考虑使用 subprocess.PIPE 的同时通过独立线程实时转发输出。

architecture

架构有疑虑 / CONCERNS

此 PR 的功能意图（push-denied 时不报错退出）是合理的，修改范围也相对集中，但存在如下问题：

阻塞项：无

虽然 run_single() 从直通 stdout/stderr 改为 PIPE 捕获后集中写入会丢失实时流式输出，但在 CI 上下文中是可接受的副作用，不构成阻塞。

建议项：

1. run_single() 的行为变更影响所有调用者：原先 run_single() 允许子进程的 stdout/stderr 实时透传，现在改为 stdout=PIPE, stderr=STDOUT、进程结束后才统一写出。这不仅改变了输出时机（丢失实时性），也将 stderr 混入 stdout。如果存在长时间无输出的场景（如 timeout 触发前），CI 环境可能因无日志输出而提前 kill。建议考虑用 subprocess.Popen + 边读边写的方式在保留实时输出的同时检查错误模式，或至少将 run_single() 和 run_model() 的 push-denied 逻辑提取为公共 wrapper 避免重复。

2. 模式判断未排除共存的其他错误：_is_push_denied_failure() 只要在 stdout 中匹配到任一 push-denied 模式就返回 True，但没有验证这是否是唯一的失败原因。如果 opencode 同时报告了 push-denied 和其他真实错误（如模型调用失败），当前逻辑仍会返回 0 吞掉错误。建议检测到 push-denied 后进一步确认输出中不含其他非预期错误码/信息，或者由 opencode 侧提供更明确的退出码区分。

3. _is_push_denied_failure() 依赖的字符串硬编码：PUSH_DENIED_PATTERNS 中的错误消息来自 opencode 内部实现，是字符串硬匹配。如果 opencode 后续版本改变了这些错误文案，action 将静默失效（该报错的场景变成失败）。建议推动 opencode 侧提供语义化的退出码或输出标记（如 OPENCODE_PUSH_DENIED=1），而不是依赖 grep 日志文本。