feat: add opencode.db cleanup and migration failure auto-recovery#202
Conversation
|
无遗漏 所有 spec 需求均已完整实现,无遗漏。 分析总结: 对照 Issue #198 的 Requirements 逐项验证: 方案 A — 启动前预防性清理
方案 C — Migration 失败自动恢复
AI review fixes(PR 承诺修复项)
文档
Issue #198 推荐"方案 A + C 结合",实现完全覆盖该推荐。版本变更检测选项是方案 A 中以"或者"列出的可选子项,实现基于文件大小的方案完全满足需求。 |
|
有条件合并 / CONDITIONAL MERGE 综合 quality 和 architecture review 的发现,PR 功能正确、整体质量良好,但存在一个测试断言准确性问题和若干架构建议。 阻塞项 / Blocking Issues
建议项 / Suggestions
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE 此 PR 整体质量较高,实现清晰、测试覆盖良好。有一个小问题建议在合并前修复。 阻塞项:
建议项:
以上均为建议性质, security存在风险 / AT RISK 该 PR 新增了 SQLite 数据库文件的自动清理和迁移恢复功能。整体实现较稳健,但存在若干需要关注的安全问题。 安全分析
阻塞项阻塞项:无 未发现必须阻塞合并的严重漏洞。 建议项
performance性能良好 / GOOD 性能分析摘要本次 PR 新增了 SQLite 数据库清理脚本( 关键分析
阻塞项:无 建议项:无 architecture架构有疑虑 / CONCERNS 架构分析摘要本次 PR 增加了两个功能:运行前清理超大 模块放置:
耦合:
分层问题(核心疑虑):
Shotgun Surgery: 一致性: 阻塞项无。功能正确,不存在架构层面的严重违规。 建议项
|
b969a56 to
d39bc79
Compare
Svitter
left a comment
There was a problem hiding this comment.
Ranked P2 — feature is well-scoped and the design (preventive cleanup + single-shot retry) is sound, but there is one correctness bug that silently disables both features for the typical user, plus a missing CHANGELOG entry.
blocking: Both cleanup-db.sh:16 and run-opencode.sh:149 use ${OPENCODE_DB_PATH:-default} to fall back when the var is unset. However, github-run-opencode/action.yml passes OPENCODE_DB_PATH: ${{ env.OPENCODE_DB_PATH }}, which evaluates to "" when the calling workflow does not set OPENCODE_DB_PATH. An empty string is set but empty, so the :- expansion does not fire — the scripts get db_path="" and silently do nothing. Fix is a one- or two-line guard after the parameter expansion in each script to treat empty as the default path.
should-fix: No ## [Unreleased] entries added to CHANGELOG.md for github-run-opencode, run-opencode, or shared — the three packages touched by this change.
The 8 new tests are well-structured and cover the right branches. The action YAML wiring and the recovery continue logic (not counting against OPENCODE_ATTEMPTS) are correctly implemented.
Thanks for the thorough PR description and for carrying forward the fixes from #200.
|
|
||
| set -uo pipefail | ||
|
|
||
| db_path="${OPENCODE_DB_PATH:-$HOME/.local/share/opencode/opencode.db}" |
There was a problem hiding this comment.
blocking: ${OPENCODE_DB_PATH:-$HOME/.local/share/opencode/opencode.db} falls back only when the variable is unset or null — not when it is set to an empty string.
github-run-opencode/action.yml lines 249 and 286 pass OPENCODE_DB_PATH: ${{ env.OPENCODE_DB_PATH }}, which evaluates to "" for workflows that do not set the var. In that common case db_path becomes "", [[ ! -f "" ]] is true, and the script exits 0 without checking or cleaning the actual default database path.
The same issue also affects run-opencode/run-opencode.sh line 149.
Fix: guard empty db_path with a second default, e.g.:
db_path="${OPENCODE_DB_PATH:-$HOME/.local/share/opencode/opencode.db}"
[[ -z "$db_path" ]] && db_path="$HOME/.local/share/opencode/opencode.db"|
|
||
| # Auto-recover from SQLite migration failures: delete the stale db and retry once. | ||
| if [[ "$migration_recovery_done" == "false" ]] && grep -qim1 "duplicate column name" "$log_file"; then | ||
| db_path="${OPENCODE_DB_PATH:-$HOME/.local/share/opencode/opencode.db}" |
There was a problem hiding this comment.
blocking: Same empty-string fallback issue as cleanup-db.sh:16. When github-run-opencode/action.yml passes OPENCODE_DB_PATH: ${{ env.OPENCODE_DB_PATH }} and the workflow has not set the var, OPENCODE_DB_PATH is "" (set-but-empty). The ${OPENCODE_DB_PATH:-default} expansion does not fire, so db_path="" and rm -f "" is a no-op — the stale db at the real default path is never deleted.
Consistent fix with cleanup-db.sh: check for empty after the parameter expansion.
|
现在我已经获取了所有必要的信息。让我进行全面的对比分析。 分析Issue #198 需求:
PR 自述变更(包括 #200 修复):
所有 spec 功能全部实现,无遗漏。 无遗漏 对照 Issue #198 的每一项需求进行了逐项验证: 方案 A(预防性清理): 方案 C(迁移恢复): 修复项:dead-code 检查移除、GNU stat 优先、 测试:8 个新测试覆盖了无文件、小文件保留、大文件删除、自定义阈值、默认阈值、migration 触发恢复、非 migration 不触发、仅恢复一次等核心场景。 文档:3 个 README 均已补充对应的功能描述。
|
|
有条件合并 / CONDITIONAL MERGE 跨 reviewer 分析后,存在 1 个阻塞项及若干建议项。一个确认项: 阻塞项 / Blocking Issues:
建议项 / Suggestions:
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE PR 整体质量良好,代码清晰,测试覆盖充分。但存在一个需要修复的条件性逻辑缺陷。 阻塞项:
建议项:
security存在风险 / AT RISK 该 PR 引入了通过环境变量 阻塞项 / Blocking Issues: 无 建议项 / Suggestions:
performance性能良好 / GOOD 该 PR 引入的变更均为轻量级 shell 脚本操作,整体性能影响极小。
阻塞项:无 建议项:无 architecture架构合理 / SOUND 该 PR 引入了 opencode 数据库管理功能(预运行清理 + 运行中迁移失败自动恢复),整体架构合理:
阻塞项:无 建议项:
|
d39bc79 to
08ef31c
Compare
|
无遗漏 对照 Issue #198(PR body 自行总结)和 PR 自述变更逐项验证,所有 spec 需求均已完整实现。 方案 A(预防性清理):
方案 C(迁移自动恢复):
#200 修复项:
文档:3 个 README 均已更新对应功能说明 ✅ 已知的边缘情况处理:
|
|
有条件合并 / CONDITIONAL MERGE 所有 reviewer 一致确认无阻塞性缺陷。security 和 architecture 提出了适度担忧,但均未要求阻塞合并。 阻塞项:无 建议项:
📋 各 Reviewer 详细审查结果quality不可合并 / CANNOT MERGE 该 PR 整体思路清晰,新增功能有测试覆盖、文档齐全,代码质量较好。但发现一个逻辑缺陷。 阻塞项
有条件合并 / CONDITIONAL MERGE PR 整体质量高,新增功能有完善测试覆盖,错误处理得当,文档同步更新。以下是建议修改项: 阻塞项:无 建议项
security存在风险 / AT RISK 本次 PR 新增了基于路径删除 阻塞项:无
建议项:
performance性能良好 / GOOD 该 PR 不涉及任何性能敏感代码:
阻塞项:无 architecture架构有疑虑 / CONCERNS 该 PR 引入了 opencode.db 的清理和迁移失败自动恢复机制,整体方向合理,但存在以下架构问题: 阻塞项:无 建议项:
|
08ef31c to
2b641fc
Compare
无遗漏对照 Issue #198 的逐项需求与当前代码进行完整验证: 方案 A(启动前清理)
方案 C(Migration 失败自动恢复)
推荐:方案 A + C 结合:两者均已实现 ✅ #200 修复项(PR 承诺修复)
文档: 所有功能需求均已完整实现,无遗漏。 注: |
|
最终决策:有条件合并 / CONDITIONAL MERGE
阻塞项 / Blocking Issues
建议项 / Suggestions
📋 各 Reviewer 详细审查结果quality可合并 / CAN MERGE 本次 PR 实现了两个关联功能:运行前清理过大的 SQLite 数据库( 阻塞项:无 建议项:
security存在风险 / AT RISK 安全分析摘要此 PR 新增了 SQLite 数据库文件自动清理与迁移失败自动恢复功能。整体设计考虑了基本的安全防护(路径黑名单校验、输入正则校验),但仍存在以下问题: 阻塞项
建议项
performance性能有疑虑 / CONCERNS 性能分析摘要该 PR 新增了 SQLite 数据库清理和迁移错误自动恢复功能。整体设计合理,没有严重的性能回归,但存在几个值得关注的资源利用问题。 阻塞项:无未发现必须阻止合并的性能问题。 建议项
architecture |
2b641fc to
46d61cd
Compare
|
无遗漏 对照 Issue #198 的逐项需求与当前代码完整验证: 方案 A(启动前预防性清理)
方案 C(Migration 失败自动恢复)
推荐:方案 A + C 结合:两者均已实现 ✅ #200 修复项
文档:3 个 README 均已更新 ✅ 所有 Issue #198 列出的功能需求均已完整实现,无遗漏。 |
有条件合并 / CONDITIONAL MERGE总结:PR 方向合理,代码质量良好,测试覆盖充分。存在 2 个跨 reviewer 确认的问题,其中最关键的路径校验缺失问题被安全领域 reviewer 评估为可后续修复(攻击路径难度高),无需阻塞合并。建议合并前优先处理已确认问题。 阻塞项:无建议项已确认(2+ reviewers 同意):
领域特有建议(单 reviewer 提出):
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE 总评本次 PR 整体设计合理,引入了 SQLite 数据库清理和迁移错误自动恢复机制,适用于自托管 runner 上的大型 阻塞项
建议项
security存在风险 / AT RISK 该 PR 引入了数据库清理和迁移恢复功能,整体设计考虑了安全性(路径黑名单校验、输入正则过滤等),但存在两处安全缺陷: 阻塞项:无无需阻塞合并的严重漏洞。 建议项
performance性能良好 / GOOD 性能分析概要本次 PR 的所有变更均为 shell 脚本和测试代码,不涉及数据库查询、内存分配或并发编程模式。所有操作均为文件系统级的轻量级一次性操作。 变更内容分析:
阻塞项:无 建议项:无 architecture架构有疑虑 / CONCERNS 该 PR 引入了数据库清理与迁移故障自动恢复功能,整体方向合理, 阻塞项
阻塞项:无(上述问题均为建议级别,不阻止合并) 建议项
|
46d61cd to
f69ec65
Compare
|
现在我已获取所有必要的信息进行全面分析。 让我逐步验证 PR 描述的所有需求是否已在代码中实现。 方案 A(启动前预防性清理):
方案 C(迁移失败自动恢复):
#200 修复项:
文档: 3 个 README 全部更新 ✅ 其他实现(超出 PR body 描述):
唯一值得注意的点: 无遗漏 对照 Issue #198(PR body 自述的 spec)逐项验证,所有需求均已完整实现,无功能遗漏。
|
有条件合并 / CONDITIONAL MERGEquality 审查者报告了 2 个阻塞项,其中 1 项被 security 审查者交叉确认。修复后即可合并。 阻塞项 / Blocking Issues
建议项 / Suggestions
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE 该 PR 整体质量较高,测试覆盖全面,错误处理也较完善。但存在以下需要修复的问题: 阻塞项
建议项
security存在风险 / AT RISK 安全分析摘要本次 PR 新增了 SQLite 数据库自动清理和迁移故障自动恢复功能。整体上没有严重到需要阻塞合并的高危漏洞,但在防御深度设计上存在一些值得改进的安全缺陷。 阻塞项:无未发现必须阻塞合并的关键安全漏洞。输入值( 建议项
performance性能良好 / GOOD 性能分析摘要: 阻塞项:无 建议项:无 architecture架构有疑虑 / CONCERNS 该 PR 整体架构合理,共享脚本抽取到 阻塞项:无 建议项:
|
f69ec65 to
80b1ec5
Compare
无遗漏对照 PR body 中的 spec 逐项验证,所有需求均已完整实现: 方案 A(预防性清理):
方案 C(迁移恢复):
#200 修复项:
文档: 3 个 README 均已更新 ✅
|
|
最终决策:有条件合并 / CONDITIONAL MERGE quality 审查发现一个需在合并前修复的无限循环 bug。security 标记为 AT RISK 但无阻塞项,其建议与 quality、architecture 的建议合并去重后一并列出。 阻塞项 / Blocking Issues
建议项 / Suggestions
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE 审查概要此 PR 为 阻塞项
建议项
security存在风险 / AT RISK 安全审查总结该 PR 新增了 SQLite 数据库清理和迁移自动恢复功能,整体安全设计较好,未发现严重漏洞。禁止访问系统目录的路径校验、Shell 注入防护、环境变量引用皆已正确处理。 阻塞项:无建议项
performance性能良好 / GOOD 本次 PR 引入的功能是 SQLite 数据库的前置清理和迁移失败自动恢复,均为轻量级 shell 脚本,无新增数据库查询、无复杂算法、无内存分配问题。 分析要点
阻塞项阻塞项:无 建议项建议项:无 architecture架构合理 / SOUND 架构分析摘要本次 PR 引入了 SQLite 数据库清理与迁移恢复功能,整体架构设计合理:
阻塞项:无建议项
|
80b1ec5 to
05d7a9f
Compare
|
现在我已完成所有文件的完整分析。以下是我的裁决: 无遗漏 对照 PR body 中的 spec 逐项验证,所有需求均已完整实现。 方案 A(预防性清理):
方案 C(迁移自动恢复):
#200 修复项:
文档: 3 个 README 均已更新对应功能说明 ✅ 关于 实现范围超出了 spec 描述:额外包含了 |
最终决策:可合并 / CAN MERGE所有 reviewer 均未发现阻塞性问题。整体代码质量良好,安全措施适当,性能影响极低。建议项以非阻塞优化为主。 阻塞项:无建议项已确认(2+ reviewer 提及):
仅单 reviewer 提及:
📋 各 Reviewer 详细审查结果qualityLet me read the full 整体代码质量良好,功能逻辑正确,测试覆盖充分。以下是详细审查意见: 阻塞项:无 建议项:
security安全无虞 / SECURE 本次 PR 未发现安全漏洞。代码实现了 opencode.db 的清理与迁移恢复机制,主要安全控制措施适当: 正向安全措施:
阻塞项:无 建议项:
performance性能有疑虑 / CONCERNS 本次 PR 新增了 SQLite 数据库的清理和迁移恢复逻辑,整体实现轻量,无严重性能问题。主要分析如下: 阻塞项:无 建议项:
以上均为非阻塞性优化建议,整体性能影响极低。 architecture架构有疑虑 / CONCERNS 架构分析摘要该 PR 在 阻塞项无 建议项
|
05d7a9f to
17ef5e6
Compare
|
最终决策:可合并 / CAN MERGE 四个 reviewer 均认为 PR 质量良好、安全无虞、性能无影响、架构设计合理,无任何阻塞项。 阻塞项:无 建议项:
📋 各 Reviewer 详细审查结果quality可合并 / CAN MERGE 该 PR 整体质量良好,引入的 SQLite 数据库清理和迁移错误自动恢复机制设计合理,测试覆盖全面,错误处理路径清晰。 阻塞项:无 建议项:
security安全无虞 / SECURE 该 PR 的变更专注于添加 SQLite 数据库清理和迁移恢复功能。经逐行审查,所有代码在安全方面表现良好: 输入验证与清理: 注入防护:所有 shell 变量均正确引号包裹,无命令注入向量。Perl 调用参数通过 路径遍历: 敏感数据暴露:无 token、密钥或敏感信息被记录。打印的路径信息仅涉及数据库文件路径,属于非敏感信息。 竞态条件:TOCTOU 在 CI 环境(隔离容器/VM)中不可实际利用。 依赖安全性:仅使用标准 POSIX 工具(bash、stat、perl、dirname),无不安全的外部依赖。 阻塞项:无 建议项:无 performance性能良好 / GOOD 该 PR 引入的功能(数据库清理脚本
阻塞项:无 建议项:无 architecture架构合理 / SOUND 该 PR 的架构设计合理,遵循了现有的模式和约定,具体分析如下:
阻塞项:无 建议项:
|
无遗漏对照 PR body 中的 spec 逐项验证,所有需求均已完整实现: 方案 A(启动前预防性清理):
方案 C(迁移失败自动恢复):
#200 修复项:
文档:3 个 README 均已更新对应功能说明 ✅ 实现范围整体超出 spec 描述(额外包含 |
|
最终决策:可合并 / CAN MERGE 所有 reviewer 均无阻塞项。综合来看,PR 质量高、安全措施到位、性能无影响,但架构和代码质量方面有几项值得采纳的建议。 阻塞项:无 建议项:
📋 各 Reviewer 详细审查结果quality可合并 / CAN MERGE 代码质量较高,结构清晰,边界情况和安全性都考虑周全。 阻塞项:无 建议项:
security安全无虞 / SECURE 本次 PR 新增了 阻塞项:无 建议项:无 详细分析:
performance性能良好 / GOOD 该 PR 主要新增了 SQLite DB 清理脚本( 阻塞项:无 建议项:无 architecture架构有疑虑 / CONCERNS 架构分析摘要本次 PR 引入 SQLite 数据库清理与迁移失败自动恢复功能,整体方向合理, 阻塞项:无建议项
|
60d48e1 to
0ff7f93
Compare
|
现在我已获取所有必要信息。让我基于实际的 spec(Issue #198)和代码实现进行全面对比分析。 无遗漏 对照 Issue #198 的逐项需求与当前代码完整验证: 方案 A(workflow 增加清理步骤):
方案 B(独立的 cron workflow): 方案 C(migration 失败自动恢复):
推荐:方案 A + C 结合:两者均已实现 ✅ PR 自述变更(含 #200 修复项):
文档:3 个 README 均已更新:
额外实现超出 spec:
|
|
最终决策:可合并 / CAN MERGE 所有四位 reviewer 均未发现阻塞性问题。代码整体质量高、测试覆盖完善。建议项集中在防御性编程、安全纵深和架构解耦方面。 阻塞项: 无 / Blocking Issues: None 建议项 / Suggestions
📋 各 Reviewer 详细审查结果quality可合并 / CAN MERGE 代码整体质量高,逻辑正确,测试覆盖充分。新的 阻塞项:无 建议项:
security存在风险 / AT RISK 该 PR 引入了 阻塞项:无 建议项:
performance性能良好 / GOOD 性能分析总结本次 PR 新增了 SQLite 数据库清理和迁移失败自动恢复功能。整体性能开销极低,均为一次性 O(1) 操作:
阻塞项:无 建议项:无 architecture架构有疑虑 / CONCERNS 架构分析摘要此 PR 总体架构设计良好,共享脚本 ( 阻塞项:无建议项
|
0ff7f93 to
dc330ab
Compare
|
现在我已获取所有必要信息。以下是我的分析: 无遗漏 对照 Issue #198 的所有需求与当前代码进行完整验证: 方案 A(workflow 增加清理步骤):
方案 B(独立 cron workflow):Issue 明确推荐"方案 A + C 结合",方案 B 是给 latex-agent 仓库的独立替代方案,不在本 PR 范围内 ✅ 方案 C(migration 失败自动恢复):
推荐:方案 A + C 结合 → 两者均已实现 ✅ #200 修复项(PR 承诺修复的全部验证通过):
文档:3 个 README 均已更新对应功能说明 ✅ 初始 review 中 Svitter 发现的空字符串导致清理静默跳过的阻塞问题已在 |
|
CAN MERGE / 可合并 所有四位 reviewer 均未发现阻塞性问题。PR 实现了 SQLite 数据库清理和迁移自动恢复两个特性,模块化设计合理,测试覆盖充分。 阻塞项:无 建议项:
📋 各 Reviewer 详细审查结果quality可合并 / CAN MERGE 本次 PR 实现了两个清晰的特性: 阻塞项:无 建议项:
security存在风险 / AT RISK 该 PR 新增了 SQLite 数据库文件删除功能(DB 超阈值清理 + 迁移失败自动恢复)。核心风险在于文件删除是破坏性操作,需要确保路径验证足够严谨。 安全分析摘要
阻塞项:无未发现必须阻塞合入的严重安全漏洞。 建议项
performance性能有疑虑 / CONCERNS 性能分析摘要该 PR 新增了 SQLite 数据库清理和迁移自动恢复机制。整体设计合理,无严重性能回归,但有一个值得关注的低效实现。 阻塞项:无建议项
architecture架构有疑虑 / CONCERNS 本 PR 新增 SQLite 数据库清理和迁移故障自恢复功能,整体设计模块化程度较高,将路径解析抽象为独立库 ( 阻塞项:无
建议项:
|
dc330ab to
33d0ff8
Compare
|
现在我已获取所有信息。以下是我的分析: 无遗漏 对照 Issue #198 的逐项需求与当前代码完整验证: 方案 A(workflow 增加清理步骤):
方案 B(独立 cron workflow):Issue 明确推荐"方案 A + C 结合",方案 B 是给 latex-agent 仓库的独立方案,不在本 PR 范围内 ✅ 方案 C(migration 失败自动恢复):
#200 修复项(PR 承诺):全部验证通过,并实际使用了 文档:3 个 README 均已更新 ✅ 额外实现超出 spec:
|
CONDITIONAL MERGE / 有条件合并综合 4 位 reviewer 的意见,合并需要先修复以下阻塞项。安全 reviewer 报告了两项防御性安全风险(路径遍历、TOCTOU),质量 reviewer 报告了一项用户行为不一致问题。
阻塞项 / Blocking Issues
建议项 / Suggestions
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE这个 PR 功能设计合理,代码质量较高,自动化测试覆盖完整。但有一个 阻塞项 需要修复。 阻塞项
建议项
security存在风险 / AT RISK 安全分析摘要此 PR 新增了 SQLite 数据库清理和迁移失败自动恢复功能。整体安全设计合理,包含了路径验证、输入检查和格式字符串保护。主要风险来自极端条件下(python3 不可用)的路径遍历绕过可能。 阻塞项
建议项
performance性能有疑虑 / CONCERNS 本次 PR 新增了 SQLite 数据库的清理和迁移恢复逻辑,功能合理,整体性能影响可控。有以下建议项: 阻塞项:无 建议项:
architecture架构合理 / SOUND 该 PR 整体架构正确,遵循了现有的代码组织模式:共享脚本放在 阻塞项:无 建议项:
|
33d0ff8 to
b350b18
Compare
无遗漏对照 PR body 中的 spec 逐项验证,所有需求均已完整实现,无功能遗漏。 方案 A(预防性清理):
方案 C(迁移失败自动恢复):
#200 修复项:
文档: 初始 review 中发现的关键问题(空字符串导致 clean-up 静默跳过)已在 |
|
最终决策 / Final Decision:可合并 / CAN MERGE 本次 PR 设计合理,测试覆盖良好,无阻塞性安全漏洞。跨 reviewer 交叉验证确认了 2 处需优先处理的问题。 阻塞项 / Blocking Issues:无 已确认问题(2+ reviewer 一致同意)
建议项 / Suggestions
📋 各 Reviewer 详细审查结果quality可合并 / CAN MERGE 本次 PR 整体质量较高。新增的 阻塞项:无 建议项:
security存在风险 / AT RISK 本 PR 新增了 SQLite 数据库自动清理和迁移失败恢复机制,整体安全设计较好(路径黑名单、TOCTOU 防守、输入校验),但存在以下需关注的问题: 阻塞项 / Blocking Issues阻塞项:无 建议项 / Suggestions1.
安全影响:该问题让清理功能和迁移恢复对所有绝对路径静默失效(拒绝所有路径,而非拒绝危险路径),用户以为安全机制在生效,实际未运行。建议从 2.
缓解因素: 3.
4. 当 performance性能有疑虑 / CONCERNS 本次 PR 新增的 阻塞项:无 建议项:
architecture架构有疑虑 / CONCERNS 该 PR 新增 SQLite 数据库清理和迁移错误自动恢复功能,整体设计合理,但存在以下架构层面值得关注的问题:
阻断项:无 建议项:
|
b350b18 to
7a54808
Compare
|
无遗漏 对照 PR body 中的 spec 逐项验证,所有需求均已完整实现。 方案 A(预防性清理):
方案 C(迁移自动恢复):
#200 修复项:
文档:
额外实现超出 spec: Svitter 在初始 review 中指出的空字符串导致清理静默跳过的阻塞问题已在 |
有条件合并 / CONDITIONAL MERGE本次 PR 整体方向合理,但存在 1 个阻塞性 bug( 阻塞项 / Blocking Issues
建议项 / Suggestions
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE 汇总此次 PR 的主要变更:新增 阻塞项
建议项
security存在风险 / AT RISK 本次 PR 引入了数据库清理和迁移恢复功能,整体安全设计较为周全(路径黑名单检查、TOCTOU 防护、 阻塞项:无 建议项:
performance性能良好 / GOOD 性能分析总结该 PR 新增了 SQLite 数据库清理和迁移失败自动恢复功能,整体实现轻量高效。Shell 脚本在关键路径上只有 O(1) 的 阻塞项无 建议项
architecture架构有疑虑 / CONCERNS 本次 PR 在 架构分析摘要
阻塞项阻塞项:无 建议项
|
7a54808 to
5425124
Compare
|
分析完毕。所有 spec 需求均已完整实现,无功能遗漏。 |
|
最终决策:有条件合并 / CONDITIONAL MERGE 综合 4 位 reviewer 意见,无安全阻塞项,但 quality reviewer 发现一处明确的功能正确性问题需修复。 阻塞项:
建议项:
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE 优质实现,路径安全校验和 TOCTOU 防御都很到位。但有一个问题需要修复: 阻塞项:
建议项:
security存在风险 / AT RISK 安全分析摘要此 PR 新增了 SQLite 数据库清理和迁移恢复功能,整体安全设计良好,包含多层路径验证和 TOCTOU 防护。但在路径解析的 fallback 分支和迁移恢复的
阻塞项:无未发现必须阻塞合并的高危漏洞。 建议项
performance性能良好 / GOOD 这是一个 CI action 的辅助脚本 PR,主要功能是清理过大的 SQLite 数据库文件,没有引入任何性能风险。 分析要点:
阻塞项:无 建议项:无 architecture架构合理 / SOUND 该 PR 引入的变更在架构上总体合理。新增的 阻塞项:无 建议项:
|
Closes #198 Prevents and recovers from stale opencode.db SQLite issues on self-hosted runners where the database can grow to 160MB+ and cause "duplicate column name" migration errors when opencode upgrades. Scheme A (preventive cleanup): - New shared/cleanup-db.sh checks opencode.db size before each run - Deletes db if it exceeds configurable threshold (default 50MB) - New github-run-opencode input "cleanup-db" (default: true) - GNU stat tried first (action is Linux-only) Scheme C (migration failure auto-recovery): - run-opencode/run-opencode.sh detects "duplicate column name" errors - Auto-deletes the stale db and retries once - Recovery limited to single attempt to avoid infinite loops - grep uses -m1 for early exit Fixes from AI review feedback: - Removed dead-code false check in cleanup step (already guarded by if:) - Swapped stat order: GNU first, BSD fallback - Wired OPENCODE_DB_PATH through action.yml to both cleanup and run steps - Added test coverage: TestCleanupDb (5 tests) + TestMigrationRecovery (3 tests)
5425124 to
d3615b2
Compare
|
基于 Issue #198 和 PR body 的完整分析,我已验证所有代码实现。以下是最终结论: 无遗漏 对照 Issue #198 的逐项需求,所有功能均已完整实现: 方案 A(预防性清理):
方案 C(迁移失败自动恢复):
#200 修复项:全部验证通过。dead-code 移除、GNU stat 优先、 文档:3 个 README 均已更新对应功能说明 ✅ 额外实现超出 spec: |
有条件合并 / CONDITIONAL MERGE本次 PR 整体设计良好,测试覆盖全面,但存在 1 个严重安全漏洞 和 2 个功能阻塞问题 需在合并前修复。 阻塞项 / Blocking Issues
建议项 / Suggestions
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE 本次 PR 整体质量较高,功能设计清晰,测试覆盖全面,文档与实现一致。代码风格统一,安全考量(路径校验、TOCTOU 防护)到位。 阻塞项
建议项
security高危漏洞 / CRITICAL 安全分析总结此 PR 涉及 SQLite 数据库自动清理与迁移故障恢复功能。代码总体安全设计良好,但存在一个严重的安全漏洞:路径验证(forbidden list)可被硬链接(hard link)绕过,导致攻击者可在受保护的系统目录中删除任意文件。 阻塞项
建议项
performance性能良好 / GOOD 该 PR 引入的功能(数据库清理、路径解析、迁移自动恢复)均为一次性操作,在 CI 环境的工作流步骤中执行,不会对 runtime 性能产生可衡量的影响。 阻塞项:无 建议项:无 architecture架构有疑虑 / CONCERNS 该 PR 总体设计合理,新增 阻塞项:无 建议项:
|
















Summary
Closes #198
Prevents and recovers from stale
opencode.dbSQLite database issues on self-hosted runners, where the database can grow to 160MB+ and causeduplicate column namemigration errors when opencode upgrades.Re-lands #200 with all AI review blocking issues resolved.
Changes
Scheme A — Preventive cleanup (before run)
shared/cleanup-db.sh: standalone script that checksopencode.dbsize and deletes it if it exceeds a configurable threshold (default 50MB). GNUstattried first (Linux-only action).github-run-opencodeinputcleanup-db:"true"(default, 50MB threshold), a number like"100"for custom MB threshold, or"false"to disablegithub-run-opencode/action.ymlbetween install and runScheme C — Migration failure auto-recovery (during run)
run-opencode/run-opencode.sh: detectsduplicate column nameerrors in opencode output, auto-deletes the stale database, and retries once. Usesgrep -m1for early exit.AI review fixes (addressing all blocking items from #200)
falsecheck in action.yml cleanup step (the stepif:guard already ensures non-false)stat -c%sfirst, BSDstat -f%zsecond (action is Linux-only)OPENCODE_DB_PATHfromaction.ymlto both cleanup-db and run steps via${{ env.OPENCODE_DB_PATH }}TestCleanupDb(5 tests) +TestMigrationRecovery(3 tests)Docs
github-run-opencode/README.mdwith new input and feature descriptionrun-opencode/README.mdwith migration recovery descriptionshared/README.mdwith cleanup-db.sh documentationTesting
8 new tests added, all passing:
Pre-existing test failures (21+4) are identical to main branch (Python 3.14 type union syntax + missing dogfood workflow).
Files changed
shared/cleanup-db.shgithub-run-opencode/action.ymlcleanup-dbinput + cleanup step + wire OPENCODE_DB_PATHrun-opencode/run-opencode.shtests/test_all.pygithub-run-opencode/README.mdrun-opencode/README.mdshared/README.md