feat: standalone review-context HTTP server + multi-review integration#271
Conversation
… to use it - Add review-context-server/ with filesystem-backed HTTP API - Refactor multi-review context-cache to prefer HTTP server when configured - Add context-cache-url and context-cache-token inputs to multi-review action - Update tests to cover both filesystem and HTTP backends
|
Rank: P1 — mergeable after a touch. The PR solves a real problem (mutable context store for re-runs) with clean architecture and good test coverage. The fallback semantics and missing CHANGELOG entry are the main blockers. Findings summary:
Open question: Is the HTTP-only-on-success / silent-fallback-to-null-on-error behavior intentional? If the HTTP server is configured, the expectation should be either "HTTP is authoritative" (fail loudly on error) or "HTTP is preferred, filesystem is reliable fallback" (fall through on any failure). The current middle ground — fall through on 404 only — means a misconfigured token or a crashed server silently wipes context. The maintainer should decide which semantics they want. Thank you for the clean implementation and the well-structured README. |
|
🚫 不可合并 / CANNOT MERGE 该 PR 新增了 HTTP 缓存服务端组件,但存在严重的安全漏洞(路径遍历)和性能/质量问题,必须在修复后重新审查。 🔴 阻塞项 / Blocking Issues (3)
🟡 警告项 / Warnings (8)
🟢 建议项 / Suggestions (10)
💰 Review Cost — $0.0000
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE PR 整体质量不错,结构清晰,前后端分离设计合理。但存在以下问题: 阻塞项
警告项
建议项
security高危漏洞 / CRITICAL 该 PR 引入了新的 HTTP 缓存服务端,但存在严重的安全漏洞,必须阻止合并。 阻塞项
警告项
建议项
performance性能问题严重 / CRITICAL 此 PR 引入了 HTTP 远程缓存后端和独立的缓存服务器,功能设计合理,但服务器存在严重的同步 I/O 阻塞问题。 阻塞项:
警告项:
建议项:
architecture架构有疑虑 / CONCERNS 该 PR 为 阻塞项:无 警告项:
建议项:
regression-test无需回归测试 PR 类型:NEW_FEATURE 该 PR 的核心是为 multi-review 添加 HTTP 缓存后端支持(包括新增的 新功能部分已在
阻塞项:无 test-value发现低价值测试 / LOW-VALUE TESTS FOUND CRITICAL无 / None MEDIUM无 / None LOW
阻塞项 / Blocking Issues阻塞项:无 / Blocking Issues: None 警告项 / WarningsHTTP 后端测试缺失异常/错误处理路径覆盖:服务端 500、网络异常、错误 JSON、鉴权边界。建议补充不阻塞。 建议项 / Suggestions建议为 |
… cache server is configured When context-cache-url is empty, persist review contexts via actions/cache using a per-run key and restore-keys so the most recent context is loaded for the same PR. This lets the action work on GitHub-hosted runners without requiring a publicly accessible internal server.
|
🚫 不可合并 / CANNOT MERGE 该 PR 存在多个由 安全(CRITICAL) 和 性能(CRITICAL) 审查确认的严重阻塞问题,必须在修复后方可合并。 🔴 阻塞项 / Blocking Issues (7)
🟡 警告项 / Warnings (8)
🟢 建议项 / Suggestions (7)
💰 Review Cost — $0.0000
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE 此 PR 整体代码质量良好,结构清晰,测试覆盖完善。HTTP 缓存后端的设计合理,与服务端实现分离干净。 阻塞项
警告项
建议项
security高危漏洞 / CRITICAL 该 PR 新增了一个独立 HTTP 缓存服务器 阻塞项
警告项
建议项
performance性能问题严重 / CRITICAL 本 PR 新增基于 HTTP 的外部缓存服务器( 阻塞项(Blocking Issues)
警告项(Warnings)
建议项(Suggestions)
architecture架构有疑虑 / CONCERNS 本次 PR 为 阻塞项:无 警告项:
建议项:
regression-test缺少回归测试 / MISSING REGRESSION TESTS PR 类型:BEHAVIOR_CHANGE(重构 分析总结:
阻塞项:无
test-value发现低价值测试 / LOW-VALUE TESTS FOUND
阻塞项:None |
- Update multi-review/package.json and package-lock.json to 4.1.0 - Add CHANGELOG entry for review context persistence feature
|
🚫 不可合并 / CANNOT MERGE 安全审查发现路径穿越高危漏洞,且跨 reviewer 确认该缺陷存在于新引入的 🔴 阻塞项 / Blocking Issues (4)
🟡 警告项 / Warnings (5)
🟢 建议项 / Suggestions (5)
💰 Review Cost — $0.0000
📋 各 Reviewer 详细审查结果quality有条件合并 / CONDITIONAL MERGE 本次 PR 新增了 HTTP cache server 后端与 filesystem 后端双轨持久化机制,整体设计合理、测试覆盖完整。但存在以下问题需要在合并前修复: 阻塞项
警告项
建议项
security高危漏洞 / CRITICAL 该 PR 引入了一个独立的 HTTP 缓存服务器 阻塞项
警告项
建议项
performance性能有疑虑 / CONCERNS 该 PR 引入了可选的 HTTP 缓存后端和独立缓存服务器用于持久化 review context,整体改动对算法效率和资源利用影响不大(数据量小、单次网络操作),但存在若干值得关注的问题。 阻塞项:无 警告项:
建议项:
architecture架构合理 / SOUND 该 PR 整体架构合理,设计清晰:
阻塞项:无 警告项:
建议项:
regression-test无需回归测试 / NO REGRESSION TESTS NEEDED PR 类型: NEW_FEATURE 分析: 已有测试文件
阻塞项:无 警告项:无 建议项:无 test-value发现低价值测试 / LOW-VALUE TESTS FOUND
|
Summary
Adds review context persistence to
multi-reviewwith two backends:Changes
review-context-server/:GET/PUT/DELETE /context/:owner/:repo/:prcontext-cache-url,context-cache-tokencontext-cache.tsprefers HTTP server when configured; falls back to local filesystemXDG_CACHE_HOMEis now always passed throughcontext-cache-urlis set, usesactions/cachewith a per-run key so the most recent context for the same PR is restoredBehavior matrix
context-cache-urlTest
All 78 tests pass.
Deployment
See
review-context-server/README.mdfor Docker/systemd instructions.