Skip to content

feat: standalone review-context HTTP server + multi-review integration#271

Merged
Svtter merged 3 commits into
mainfrom
feat/review-context-server
Jun 24, 2026
Merged

feat: standalone review-context HTTP server + multi-review integration#271
Svtter merged 3 commits into
mainfrom
feat/review-context-server

Conversation

@Svtter

@Svtter Svtter commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds review context persistence to multi-review with two backends:

  1. An independent HTTP cache server for self-hosted/internal runners.
  2. GitHub Actions cache fallback for GitHub-hosted runners (uses a per-run key + restore-keys).

Changes

  • New server review-context-server/:
    • Filesystem-backed HTTP API: GET/PUT/DELETE /context/:owner/:repo/:pr
    • Optional bearer-token auth
    • Docker + systemd deployment docs
  • multi-review integration:
    • New inputs: context-cache-url, context-cache-token
    • context-cache.ts prefers HTTP server when configured; falls back to local filesystem
    • XDG_CACHE_HOME is now always passed through
    • When no context-cache-url is set, uses actions/cache with a per-run key so the most recent context for the same PR is restored
  • Tests:
    • Added HTTP backend tests
    • Existing filesystem tests preserved and passing

Behavior matrix

context-cache-url Backend used Works on
set HTTP server Any runner that can reach the server
empty GitHub Actions cache GitHub-hosted runners out of the box

Test

cd multi-review
npm run build
npm test

All 78 tests pass.

Deployment

See review-context-server/README.md for Docker/systemd instructions.

… 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
@Svitter Svitter added feat New feature or enhancement review:p1 Major review findings setup Setup and installation tool Internal tooling and utilities triaged Issue has been triaged labels Jun 24, 2026
@Svitter

Svitter commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

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:

  1. should-fix: loadReviewContext / saveReviewContext HTTP fallback is asymmetric. On HTTP error (non-404), both functions short-circuit instead of falling back to filesystem. A transient 500 or network timeout causes silent context loss on save and context reset on load (context-cache.ts:190, 228).

  2. should-fix: No CHANGELOG.md entry under [Unreleased]. Both review-context-server (new) and multi-review (modified inputs) should be listed.

  3. should-fix: readBody in the server has no size limit — unbounded request body can exhaust memory (server.ts:37).

  4. should-fix: HTTP backend tests don't cover error paths (500, auth failure, network unreachable). These would surface the fallback bug above.

  5. nit: Server uses synchronous node:fs calls in the async handler — tolerable for low traffic but blocks concurrent requests.

  6. nit: /health leaks DATA_DIR to unauthenticated clients.

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.

@github-actions

Copy link
Copy Markdown

🚫 不可合并 / CANNOT MERGE

该 PR 新增了 HTTP 缓存服务端组件,但存在严重的安全漏洞(路径遍历)和性能/质量问题,必须在修复后重新审查。

🔴 阻塞项 / Blocking Issues (3)

  • 路径遍历漏洞(已确认 - 安全)review-context-server/src/server.ts:46-56parseContextPath 未对 ownerrepopr 参数中的 .. 做校验,攻击者可通过构造恶意 URL 实现任意文件读写(OWASP A1:2021)。
  • 无请求体大小限制(已确认 - 质量 + 安全)server.tsreadBody 无缓冲区上限或 content-length 校验,恶意客户端可发送超大 payload 导致 OOM(DoS 攻击)。
  • 同步 FS 操作阻塞事件循环(已确认 - 质量 + 性能)readFileSync / writeFileSync / unlinkSync 等同步 API 在每个请求处理中阻塞事件循环,并发场景下吞吐严重下降。需改用 fs.promises 异步版本。

🟡 警告项 / Warnings (8)

  • 丢失 malformed context 的警告日志(质量):重构后不再记录 "Ignoring malformed or stale review context" 日志,不利于排查缓存损坏或版本不兼容问题。
  • 无优雅关闭处理(质量):未监听 SIGTERM/SIGINT,容器环境中正在写入的请求可能被中断。
  • 认证默认关闭且无强制机制(安全):AUTH_TOKEN 默认空字符串时 isAuthorized 始终返回 true,生产部署若忘记设置则完全开放。
  • HTTP 明文传输风险(安全):context-cache-url 未强制要求 https://,Bearer token 和上下文数据可被中间人窃取。
  • 错误信息泄露内部路径(安全):/health 返回 dataDir 绝对路径;warn 日志包含完整 URL,可能泄露内部拓扑信息。
  • 缓存后端缺少抽象接口(架构):loadReviewContext / saveReviewContext 通过 if (url) 硬编码路由到后端实现,未使用策略模式,违反开闭原则。
  • URL 路径模式硬编码在两端(架构):/context/:owner/:repo/:pr 在客户端和服务端重复硬编码,未抽象为共享常量。
  • HTTP 后端测试缺失异常路径覆盖(测试):未覆盖服务端 500、网络异常、非法 JSON、鉴权边界等场景。

🟢 建议项 / Suggestions (10)

  • readBody error 事件处理(已确认 - 质量 + 安全):req.on("error", reject) 可能在 end 后触发,导致未捕获的 Promise rejection,建议检查 req.destroyed 或直接 reject 固定错误。
  • XDG_CACHE_HOME 测试隔离(质量):建议在 HTTP 后端测试的 before 中将 XDG_CACHE_HOME 设为隔离目录并确保 after 恢复原始值。
  • isSafePathComponent 复用(安全):可复用 repo 中已有的安全路径校验函数对 owner/repo/pr 做校验。
  • 强制 HTTPS scheme(安全):context-cache-url 可选强制 https:// 或输出告警。
  • 启动时警告空 AUTH_TOKEN(安全):非开发模式下可输出显式警告或拒绝启动。
  • getRepo() 重复调用(性能):loadReviewContext 内外各调用一次,可作为参数传递避免重复读取环境变量。
  • JSON.stringify 格式化开销(性能):HTTP 传输时可改用无空格格式,节省序列化体积和 CPU。
  • 客户端增加内存缓存(性能):同一 CI run 内避免对同一 PR 重复发起 HTTP 请求。
  • content-type 校验(架构):httpGetContext 在调用 res.json() 前可校验响应 Content-Type 增强健壮性。
  • review-context-server 添加测试(测试):新增 server.test.ts 覆盖 GET/PUT/DELETE//health/鉴权/非法请求路径等场景。
💰 Review Cost — $0.0000
Role Cost (USD) Input Output Reasoning Cache Read Cache Write
quality $0.0000 17,944 719 3,721 0 0
security $0.0000 17,953 867 782 0 0
performance $0.0000 17,956 457 1,025 0 0
architecture $0.0000 17,957 467 1,553 0 0
regression-test $0.0000 18,394 468 4,141 0 0
test-value $0.0000 25,484 599 4,339 0 0
coordinator $0.0000 11,280 1,028 1,314 0 0
Total $0.0000 126,968 4,605 16,875 0 0
📋 各 Reviewer 详细审查结果
quality

有条件合并 / CONDITIONAL MERGE

PR 整体质量不错,结构清晰,前后端分离设计合理。但存在以下问题:

阻塞项

  1. server.ts PUT 端点没有请求体大小限制 (review-context-server/src/server.ts:68-72) — readBody 无条件拼接所有 chunk,未检查 content-length 或设置缓冲区上限。恶意或误配置的客户端可发送超大 payload 导致 OOM。建议添加 content-length 校验(如拒绝超过几 MB 的请求)或在拼接循环中设置最大字节数阈值。

警告项

  1. 丢失 malformed context 的警告日志 — 重构前 loadReviewContextvalidateLoadedContext 返回 null 时会输出 "Ignoring malformed or stale review context: ${path}"。重构后的 fsGetContexthttpGetContext 直接返回 validateLoadedContext 的结果,不再记录该警告。建议恢复日志以便排查缓存的损坏或版本不兼容问题。

  2. server.ts 同步 FS 操作阻塞事件循环readFileSync / writeFileSync / unlinkSync 用于每个请求。对于低并发 CI 场景问题不大,但仍建议改用 fs.promises 异步版本。

  3. server.ts 无优雅关闭处理 — 未监听 SIGTERM/SIGINT,容器环境(Docker/K8s)中正在写入的请求可能被中断。建议添加 process.on('SIGTERM', ...) 关闭 server 并等待 in-flight 请求完成。

建议项

  1. server.ts:86 readBodyreq.on("error", reject) 会因外层 try/catch 捕获,但 req 的 error 事件也可能在 end 之后触发,导致未处理的 Promise rejection。建议在 Promise reject 之前检查 req.destroyed 或直接改为 req.on("error", () => reject(new Error("request aborted")))

  2. 测试中 before/afterXDG_CACHE_HOME 的清理较为脆弱——删除环境变量后未在 finally 中恢复(依赖 after 钩子)。建议在 HTTP 后端测试的 before 中将 XDG_CACHE_HOME 临时设为隔离目录并确保 after 恢复原始值,避免测试间副作用。

security

高危漏洞 / CRITICAL

该 PR 引入了新的 HTTP 缓存服务端,但存在严重的安全漏洞,必须阻止合并。

阻塞项

  1. 路径遍历漏洞(严重)review-context-server/src/server.ts:46-56parseContextPath 函数使用正则 ([^/]+) 捕获 URL 中的 ownerrepopr,但未对 .. 做任何校验。这些值直接传入 getContextPath 拼接到 join(DATA_DIR, owner, repo, "${pr}.json")。攻击者可通过 URL 如 /context/../../etc/passwd/x/y 实现任意文件写入或读取。例如传入 owner=.. 可遍历到 DATA_DIR 之外。这是 OWASP 路径遍历(A1:2021-Broken Access Control)的经典案例。

  2. 无请求体大小限制(高危)review-context-server/src/server.ts:45-52readBody 函数无任何大小限制,攻击者可发送超大 PUT 请求体导致服务器内存耗尽(DoS 攻击,OWASP A6:2021-Security Logging and Monitoring Failures / DoS)。

  3. 认证默认关闭且无强制机制(中危)review-context-server/src/server.ts:84-87AUTH_TOKEN 默认空字符串时 isAuthorized 始终返回 true。生产部署若忘记设置 AUTH_TOKEN,服务器完全开放。虽 README 有说明,但无运行时强制校验。

警告项

  1. HTTP 明文传输风险context-cache-url 输入和 MULTI_REVIEW_CONTEXT_CACHE_URL 环境变量未做 URL scheme 校验,允许使用 http:// 而非 https://。Bearer token 和上下文数据在 HTTP 明文传输中可被中间人窃取。

  2. 错误信息泄露内部路径review-context-server/src/server.ts:103-104/health 接口返回 dataDir 绝对路径;multi-review/src/context-cache.tshttpGetContext/httpPutContext 的 warn 日志中包含了完整 URL(含 owner/repo/pr),这些可能泄露内部拓扑信息。

  3. DELETE 接口无认证检查review-context-server/src/server.ts:94-99 中 DELETE 方法在 isAuthorized 之后,正确受到认证保护;但需确认是否所有上下文端点(GET/PUT/DELETE)都需要认证——当前实现一致,无问题。

  4. 业务数据无加密存储:JSON 文件以 mode: 0o600 写入,但内容本身未加密。若 DATA_DIR 所在磁盘被未授权访问,Review context 中的会话数据将明文暴露。

建议项

  1. parseContextPath 后对 ownerrepopr 增加 .. 和空值校验,建议使用 isSafePathComponent 类似的函数(该 repo 已有实现可复用)。
  2. readBody 中添加明确的请求体大小上限(如 10MB),过大的请求直接返回 413 Payload Too Large
  3. context-cache-url 强制要求 https:// scheme,或在 action 输入层提供选项并默认告警。
  4. 在服务器启动时,若 AUTH_TOKEN 未设置且非开发模式,输出显式警告日志或直接拒绝启动。
  5. readBody 中的 req.on("error", reject) 若不处理可能触发未捕获的 Promise reject,应考虑安全兜底。
performance

性能问题严重 / CRITICAL

此 PR 引入了 HTTP 远程缓存后端和独立的缓存服务器,功能设计合理,但服务器存在严重的同步 I/O 阻塞问题。

阻塞项:

  1. review-context-server/src/server.ts 的所有文件操作全部使用 同步 APIreadFileSyncwriteFileSyncexistsSyncmkdirSyncunlinkSync)。每个请求处理都会阻塞事件循环,在并发场景下(如多个 PR 同时触发 review),请求会被串行化处理,可能导致整体吞吐严重下降甚至超时。必须改用 fs.promises 的异步版本。

警告项:

  1. getContextCacheDir() 中对同一 raw 值调用了两次 resolve(raw),第二次赋值给 expectedRoot 是冗余计算:getContextCacheDirgetFilesystemPath 都被多次调用的场景下会累积额外开销。

  2. saveReviewContextgetRepo() 被重复调用:loadReviewContextvalidateLoadedContext 内部已经调用一次,外层又显式调用一次。若考虑性能优化,可将 repo 作为参数传递,避免重复从环境变量读取。

建议项:

  1. JSON.stringify(context, null, 2) 使用格式化输出增加了序列化体积和 CPU 开销,HTTP 传输时可改用 JSON.stringify(context)(无空格),由服务端持久化时再格式化;或两端均不格式化以节省 I/O 带宽。

  2. 服务端和客户端均无内存缓存。如果同一 CI run 内对同一 PR 多次调用 loadReviewContext,会重复发起 HTTP 请求或文件读取。可考虑在客户端做简单的内存缓存(如 Map<prNumber, context>)。

  3. getContextCacheUrl 中的 replace(/\/+$/, "") 可在配置时预处理,无需每次调用都执行字符串操作。

architecture

架构有疑虑 / CONCERNS

该 PR 为 multi-review 引入了 HTTP 缓存后端与独立的 review-context-server 组件,整体方向合理,但在抽象层面存在非阻塞性的架构问题。

阻塞项:无

警告项:

  1. 缓存后端缺少抽象接口。loadReviewContext / saveReviewContext 内部通过 if (url) 硬编码路由到 HTTP 或文件系统实现,未使用接口/策略模式。若未来需添加第三种后端(如 Redis、S3),需修改此路由函数,违反开闭原则。
  2. 客户端与服务端的 URL 路径模式 /context/:owner/:repo/:pr 硬编码在两端(context-cache.tsserver.ts),未抽象为共享常量或协议。两端耦合在 URL scheme 层面,若 URL 结构变更需同步修改双方。

建议项:

  1. 建议将 review-context-serverpackage.json"@types/node" 依赖改为非 devDependency 或确认运行时依赖正确分离。当前仅 devDeps 有类型包,生产依赖为空,是无状态纯 HTTP 服务,虽可运行但类型检查需安装 devDeps。
  2. httpGetContext 调用 res.json() 前未校验响应 Content-Type,若服务端返回非 JSON 的 200 响应会静默解析失败。建议增加 content-type 检查或 try-catch 增强健壮性。
  3. saveReviewContext 中的 if (!newSessions.length) return; 提前返回的写法略偏防御式风格,与原代码风格一致可接受。但 HTTP 后端在 HTTP 请求失败时静默吞掉错误,与文件系统后端的错误处理风格一致,建议统一考虑是否在关键路径上重试或告警。
regression-test

无需回归测试

PR 类型:NEW_FEATURE

该 PR 的核心是为 multi-review 添加 HTTP 缓存后端支持(包括新增的 review-context-server 独立服务包),同时对原有文件系统后端进行了不影响外部行为的重构(将 getContextPath 拆分为 getContextKey + getFilesystemPath,提取公共 API 调用入口 loadReviewContext/saveReviewContext 进行后端路由分发)。现有公共 API 签名(loadReviewContextsaveReviewContextgetContextCacheDirtrimSessions)未变更,现有测试继续覆盖文件系统行为。

新功能部分已在 context-cache.test.ts 中增加了完整的 HTTP 后端测试套件:

  1. HTTP GET 返回 404 时返回 null
  2. HTTP 读写完整往返
  3. HTTP 追加会话
  4. URL 未设置时回退到文件系统

阻塞项:无
警告项:无
建议项:无

test-value

发现低价值测试 / LOW-VALUE TESTS FOUND

CRITICAL

无 / None

MEDIUM

无 / None

LOW

  1. multi-review/src/context-cache.test.ts:212-343 — HTTP backend 测试仅覆盖了基本流程(空状态、增删改查、回退到文件系统),缺失多种异常场景覆盖:

    • HTTP 服务返回 500 等非 404 错误码(httpGetContext 第 95-96 行、httpPutContext 第 122-124 行的 error handling 路径未测试)
    • HTTP 服务返回非法 JSON 内容
    • 网络连接失败(fetch 抛异常,catch 块行 101-104 / 行 127-129 未覆盖)
    • 未设置 context-cache-token 的鉴权行为
    • getContextCacheUrl()getContextCacheToken() 返回值的边界情况(如空字符串、带尾部斜杠的 URL)
  2. review-context-server/src/server.ts — 新引入的 HTTP 服务端代码完全无测试覆盖,没有 /healthGETPUTDELETE、鉴权、非法 JSON body、文件不存在等路径的测试。

阻塞项 / Blocking Issues

阻塞项:无 / Blocking Issues: None

警告项 / Warnings

HTTP 后端测试缺失异常/错误处理路径覆盖:服务端 500、网络异常、错误 JSON、鉴权边界。建议补充不阻塞。

建议项 / Suggestions

建议为 review-context-server 新增测试文件(server.test.ts),覆盖 GETPUTDELETE/health、鉴权失败、非法请求路径、非法 JSON body 等场景。

… 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.
@github-actions

Copy link
Copy Markdown

🚫 不可合并 / CANNOT MERGE

该 PR 存在多个由 安全(CRITICAL)性能(CRITICAL) 审查确认的严重阻塞问题,必须在修复后方可合并。


🔴 阻塞项 / Blocking Issues (7)

  • 已确认 路径穿越漏洞 (Path Traversal)review-context-server/src/server.ts:22getContextPath 使用 path.join 拼接用户输入的 owner/repo/pr 参数,未校验参数是否包含 ..,攻击者可构造请求 /context/../etc/passwd/x 读取、覆盖或删除服务器任意文件,影响 GET/PUT/DELETE 三个方法。建议在 parseContextPathgetContextPath 中拒绝包含 .. 的路径组件,或使用 path.resolve 后验证路径仍在 DATA_DIR 之下。([security], [architecture])
  • 已确认 无请求体大小限制 (OOM/DoS 风险)readBody() 函数(server.ts:44-51)无上限累积 chunk,恶意大 payload 可耗尽服务端内存或磁盘。建议限制 body 大小(如 1MB-10MB)并在超限时直接拒绝连接。([performance CRITICAL], [security], [architecture], [quality])
  • 已确认 服务端使用同步文件 I/O 阻塞事件循环server.ts 全部使用 readFileSync/writeFileSync/existsSync/unlinkSync 等同步 API。Node.js 单线程模型下,多个 CI 运行器并发请求时请求将被串行化,延迟线性增长。应改用 fs/promises 异步 API。([performance CRITICAL], [quality])
  • AUTH_TOKEN 为空时认证完全可绕过isAuthorizedAUTH_TOKEN 为空(!AUTH_TOKEN 为 true)时直接返回 trueserver.ts:35)。安全配置不应默认开放所有权限。建议默认要求 AUTH_TOKEN 并在未设置时输出 warning 日志。([security])
  • 无 TLS 加密传输 — 服务端使用 node:httpcontext-cache-token 和 review context 数据以明文传输,存在中间人窃听风险。建议在文档中要求反向代理提供 TLS 终结,或考虑支持 HTTPS。([security])
  • 并发写入无原子性保护(数据竞态) — HTTP 后端的 saveReviewContext 执行 GET 读取 → 内存合并 → PUT 覆写的 read-modify-write 模式。同一 PR 的多个 CI 运行器同时执行时,后一个 PUT 会覆盖前一个的增量变更,导致 session 丢失。建议使用版本号或 If-Match 头避免并发覆盖,或在文档中明确说明 HTTP 后端不支持并发写入同一 PR。([performance CRITICAL])
  • 服务端 URL 解析未处理 query parametersparseContextPath 使用正则 /^\/context\/([^/]+)\/([^/]+)\/([^/]+)$/,URL 含 query string(如 /context/owner/repo/123?t=1)时将匹配失败返回 404。建议在匹配前使用 url.split("?")[0] 剥离 query string。([quality])

🟡 警告项 / Warnings (8)

  • fsGetContext 移除了对损坏缓存文件的诊断日志 — 旧代码在 validateLoadedContext 返回 null 时输出 console.warn,新代码直接返回 null,丢失调试信息。建议恢复该日志。([quality], [regression-test])
  • 服务端无请求/连接超时设置createServer 未设置 requestTimeoutheadersTimeout 等参数;客户端 httpGetContextfetch 也未设置 AbortSignal。慢连接可能长期占用资源。([performance], [quality])
  • 无速率限制 — 服务端无任何速率限制,攻击者可遍历路径、暴力破解 token 或大量写入消耗磁盘。([security])
  • 无内存缓存层 / 无 ETag 支持 — 每次 GET 都走磁盘完整读取和反序列化,不支持 304 Not Modified。([performance])
  • 无 HTTP 压缩 — 服务端和客户端均未启用 Content-Encoding(gzip/brotli),可能浪费网络带宽。([performance])
  • getFilesystemPath 参数类型包含 null 但假设非空ReturnType<typeof getContextKey> 包含 null,但 getFilesystemPathfsGetContextfsPutContext 内部直接访问 key.owner 等属性,类型系统层面存在隐患。([quality])
  • DATA_DIR 依赖相对路径DATA_DIR 默认值 ./data 取决于启动目录,建议强制转为绝对路径避免与工作目录耦合。([architecture])
  • fsGetContext 中存在 TOCTOU 模式 — 先 existsSyncreadFile,中间存在文件被删除/修改的窗口期。建议直接 readFile 捕获 ENOENT 错误。([performance])

🟢 建议项 / Suggestions (7)

  • PUT 操作使用临时文件 + rename 原子写入 — 避免半写文件被 GET 读到。([performance])
  • 服务端添加健康检查 /health 文档说明 — 代码已正确实现 AUTH_TOKEN 白名单,建议在 README 中明确。([architecture])
  • 服务端显式设置 Content-Type: application/json; charset=utf-8 — 更规范。([architecture])
  • 文件系统后端的 getContextCacheDir 可标记为 @internal — 该函数仅在测试中内部使用。([architecture])
  • 补充异常/边界场景测试 — HTTP 后端 5xx、网络不可达、401 未授权和 HTTP PUT 失败路径缺少测试覆盖;新增 HTTP server 无任何配套测试文件。([regression-test], [test-value])
  • 考虑将 GitHub Actions cache step 的条件设定改为参数传递 — 减少 workflow 解析开销(非关键)。([performance])
  • 客户端 MULTI_REVIEW_CONTEXT_CACHE_URL 输入建议增加 URL scheme 白名单 — 如仅允许 http://https://。([security])
💰 Review Cost — $0.0000
Role Cost (USD) Input Output Reasoning Cache Read Cache Write
quality $0.0000 18,175 648 3,995 0 0
security $0.0000 18,184 912 2,391 0 0
performance $0.0000 18,187 968 1,859 0 0
architecture $0.0000 18,188 456 881 0 0
regression-test $0.0000 18,625 970 5,092 0 0
test-value $0.0000 28,157 552 3,666 0 0
coordinator $0.0000 12,137 1,386 1,311 0 0
Total $0.0000 131,653 5,892 19,195 0 0
📋 各 Reviewer 详细审查结果
quality

有条件合并 / CONDITIONAL MERGE

此 PR 整体代码质量良好,结构清晰,测试覆盖完善。HTTP 缓存后端的设计合理,与服务端实现分离干净。

阻塞项

  1. 服务端 URL 解析未处理 query parametersparseContextPath 使用正则 /^\/context\/([^/]+)\/([^/]+)\/([^/]+)$/,若 URL 包含 query string(如 /context/owner/repo/123?t=1),将匹配失败返回 404。虽然当前客户端构造的 URL 不含 query,但若部署在反向代理(如 AWS ALB)后可能被附加 query,导致缓存静默失效。建议在匹配前使用 url.split("?")[0] 剥离 query string。

警告项

  1. fsGetContext 移除了对损坏缓存文件的诊断日志:旧代码在 validateLoadedContext 返回 null 时会输出 "Ignoring malformed or stale review context: ${path}",新代码直接返回 null,丢失了有助于调试的日志信息。建议恢复该警告日志。
  2. getFilesystemPath 参数类型为 T | null 但假设非空ReturnType<typeof getContextKey> 包含 null,但 getFilesystemPathfsGetContextfsPutContext 内部直接访问 key.owner 等属性,类型系统层面存在隐患(尽管调用方已做非空检查)。

建议项

  1. httpGetContext 缺乏超时机制fetch 调用未设置 AbortSignal,若缓存服务挂起可能导致 Action 等待。建议增加合理超时(如 10s)。
  2. 服务端使用同步文件操作readFileSync 等同步调用在并发场景下会阻塞事件循环。考虑到此服务预期负载较低,可作为后续优化项。
  3. 服务端缺乏请求体大小限制readBody 无上限累积数据,恶意大请求可能导致 OOM。建议限制 body 大小(如 10MB)。
security

高危漏洞 / CRITICAL

该 PR 新增了一个独立 HTTP 缓存服务器 review-context-server,存在 路径穿越漏洞(Path Traversal),可导致服务器上任意文件的读取、写入和删除。


阻塞项

  1. 路径穿越漏洞(Path Traversal)- review-context-server 服务端

    • 位置: review-context-server/src/server.ts:29-31parseContextPath 使用正则 ^\/context\/([^/]+)\/([^/]+)\/([^/]+)$ 解析 URL 路径参数,[^/]+ 可以匹配 ..
    • 而第 22 行 getContextPath 使用 path.join(DATA_DIR, owner, repo, "${pr}.json"),其中 path.join 会解析 .. 组件
    • 攻击者可以通过请求 /context/../some/sensitive/file(owner=.., repo=some, pr=sensitive)将路径解析到 DATA_DIR 之外
    • 该漏洞影响 GET、PUT、DELETE 三个方法,可实现服务器任意文件的读取、覆盖写入和删除
    • 测试代码 review-context-server/src/server.test.ts(虽未包含在 PR 中)和现有测试均未覆盖路径穿越防护
  2. 认证完全可绕过(AUTH_TOKEN 为空时)

    • 位置: review-context-server/src/server.ts:35-38isAuthorizedAUTH_TOKEN 为空时直接返回 true!AUTH_TOKEN 为 true)
    • 如果运维人员忘记设置 AUTH_TOKEN,任何能访问该端口的请求都可以读写任意 context 数据
    • 安全配置不应默认开放所有权限
  3. 无 TLS 加密传输

    • 服务端使用 node:http 而非 node:httpscontext-cache-token 和所有 review context 数据以明文传输
    • 这在同一内网/容器网络中仍然存在被中间人窃听的风险

警告项

  1. 无请求体大小限制 - DoS 风险

    • PUT 方法的 readBody 函数(第 44-51 行)没有限制 body 大小,攻击者可以发送超大 payload 耗尽服务器内存
    • 文件系统写入也未做文件大小限制,可能导致磁盘耗尽
  2. 无速率限制 - 暴力攻击/滥用风险

    • 服务端没有任何速率限制,攻击者可以快速遍历路径、暴力破解 token(如果有设置)或大量写入消耗磁盘
  3. DELETE 方法无验证直接删除文件

    • 同路径穿越问题,DELETE 方法也可删除外部任意文件(配合路径穿越)

建议项

  1. 对 URL 路径参数进行路径穿越检查:在 parseContextPathgetContextPath 中拒绝包含 .. 的路径组件,或使用 path.resolve 并验证路径仍在 DATA_DIR 之下
  2. 对 PUT 请求的 body 大小设置上限(如 1MB
  3. 添加速率限制中间件
  4. 建议默认要求 AUTH_TOKEN 并给予明确启动警告(当未设置时输出 warning 日志)
  5. 考虑支持 HTTPS,或至少在文档中建议反向代理提供 TLS 终结
  6. 客户端侧 MULTI_REVIEW_CONTEXT_CACHE_URL 输入无格式校验,建议增加 URL scheme 白名单(如仅允许 http://https://
performance

性能问题严重 / CRITICAL

本 PR 新增基于 HTTP 的外部缓存服务器(review-context-server)以及对 context-cache.ts 的大幅重构。主要性能问题集中在新增的 HTTP 服务端实现上。

阻塞项(Blocking Issues)

  1. 服务端使用同步文件 I/O 阻塞事件循环review-context-server/src/server.ts 全部使用 readFileSync/writeFileSync/existsSync/unlinkSync 等同步 API。Node.js 是单线程事件循环模型,同步 I/O 会在整个操作期间阻塞所有其他请求的处理。若多个 CI 运行器并发请求同一缓存服务(例如多个 PR 同时被 review),请求将被串行化,吞吐量急剧下降,延迟线性增长。应当改用 fs/promises 的异步 API。

  2. 无请求体大小限制,存在 OOM 风险readBody() 直接将所有 chunk 拼接到内存字符串中,未设置 maxBodySize 上限。Review context 可能包含大量 session 和 message 数据,恶意或异常的 large payload 可耗尽服务端内存。应在 req.on('data') 中累计字节计数,超过阈值后直接拒绝并关闭连接。

  3. 并发写入无原子性保护,存在数据竞态saveReviewContext 在 HTTP 后端下执行的是「GET 读取 - 内存合并 - PUT 覆写」的 read-modify-write 模式。若同一 PR 的两个 CI 运行器几乎同时执行,后一个 PUT 可能覆盖前一个的增量变更,导致 session 丢失。而原 filesystem 后端通过 github.run_id 保证了每个运行使用独立 key,不存在此问题。新加的 HTTP 后端共享同一 key 且无锁/版本控制。注意:这既是数据一致性问题,也导致已完成的 review 工作被丢弃,造成计算资源浪费。

警告项(Warnings)

  1. 服务端无内存缓存层 — 每次 GET /context 都走磁盘读取完整 JSON 文件并重新反序列化。对于重复访问同一 PR 的场景(如多轮 review),可以引入简单的 LRU 内存缓存减少重复 I/O。当前实现也不支持 ETag/If-None-Match,无法返回 304 减少带宽。

  2. 无 HTTP 压缩 — Review context JSON 可能体积较大(含多条 session 的完整对话消息),但服务端和客户端均未启用 Content-Encoding(gzip/brotli)。在同一集群内,虽然延迟影响有限,但会浪费网络带宽和传输时间。

  3. 服务端无请求/连接超时设置createServer 使用默认参数,未设置 requestTimeoutheadersTimeoutkeepAliveTimeout。慢客户端或异常连接可能长期占用文件描述符和连接资源,导致资源泄漏。

  4. 客户端 fsGetContext 中 TOCTOU 模式 — 先 existsSyncreadFile,中间毫秒级窗口内文件可能被删除/修改。虽然在当前 CI 单消费者场景下概率极低,但跨文件系统或 NFS 场景下不可靠。推荐直接 readFile 捕获 ENOENT 错误做分支。

建议项(Suggestions)

  1. 服务端可为 PUT 操作使用临时文件 + rename 的原子写入模式,避免半写文件被 GET 读到(当前可能因为同进程事件循环而概率较低,但仍是隐患)。

  2. saveReviewContext 可考虑在 HTTP 后端使用版本号或 If-Match 头避免并发覆盖,或在文档中明确说明 HTTP 后端不支持并发写入同一 PR 的场景。

  3. GitHub Actions cache step 的 context-cache-url 条件 (${{ inputs.context-cache-url == '' }}) 若为空字符串则使用内置 cache,语义正确,但考虑将 cache step 的部分改为设置参数而非完整步骤,以减少 workflow 解析开销(非关键)。

architecture

架构有疑虑 / CONCERNS

本次 PR 为 multi-review action 添加了可选的 HTTP cache 后端,并新增了独立的 review-context-server。整体设计合理:HTTP 后端与已有的文件系统后端共享同一套公共接口(loadReviewContext/saveReviewContext),客户端无需感知后端选择,抽象层次清晰。模块放置合理,新服务器作为独立可部署服务放在顶层目录而非嵌套在 action 内。改动集中在 context-cache.ts 及其测试和 action YAML 中,未出现散弹式修改。

阻塞项:无

警告项:

  1. review-context-server/src/server.ts 存在路径遍历漏洞 — getContextPath 直接将用户输入的 owner/repo/pr 拼接为文件路径,未校验是否包含 ../,攻击者可构造如 ../../../etc/passwd 的参数读取或写入任意文件
  2. 服务器没有对 PUT 请求 body 做大小限制,可能被大 payload 耗尽磁盘或内存

建议项:

  1. review-context-server.gitignore 只忽略了顶层 data/,但 DATA_DIR 默认值是 ./data,相对路径取决于启动目录。建议在代码中强制 DATA_DIR 为绝对路径,避免与工作目录耦合
  2. 服务器 GET/PUT 中未显式设置 Content-Type: application/json 时的 charset(utf-8 通常由默认编码涵盖,但显式指定更规范)
  3. 文件系统后端中 getContextCacheDir 被重新暴露为 export(原实现也在同位置 export),但仅在测试中内部使用,可考虑标记为 @internal 或仅测试导入
  4. 服务器缺少健康检查 /healthAUTH_TOKEN 开启时的白名单注释说明(代码已正确实现,建议在 README 中明确补充)
regression-test

缺少回归测试 / MISSING REGRESSION TESTS

PR 类型:BEHAVIOR_CHANGE(重构 context-cache.ts 核心缓存逻辑,新增 HTTP/文件系统双后端分发机制)

分析总结:
PR 重构了 context-cache.ts,将原本单一的文件系统缓存拆分为 HTTP 和文件系统双后端,loadReviewContextsaveReviewContext 根据 MULTI_REVIEW_CONTEXT_CACHE_URL 环境变量决定使用哪个后端。测试文件已覆盖文件系统后端的常规操作和 HTTP 后端的正常流程(roundtrip、append、404 返回 null),但仍存在以下缺口:

  1. HTTP 失败时无文件系统回退的验证缺失

    • 什么变更:当 MULTI_REVIEW_CONTEXT_CACHE_URL 已设置但 HTTP 请求失败(500、网络错误、invalid JSON、validateLoadedContext 返回 null)时,loadReviewContext 返回 null,不会回退到文件系统。
    • 为什么需要:这是新分发机制的核心行为契约,现有测试仅覆盖了 HTTP 404 场景和「URL 未设置时走文件系统」的场景,但缺少反向验证——URL 已设置且 HTTP 失败时不会偷偷走文件系统。
    • 建议测试:设置 HTTP URL 指向一个返回 500 或不可达的地址,验证 loadReviewContext 返回 null 且文件系统上不存在新创建的文件。
    • 严重程度:MEDIUM
  2. saveReviewContext HTTP 失败路径无测试

    • 什么变更:saveReviewContext 在 HTTP URL 已设置时调用 httpPutContext,失败仅 console.warn 不抛异常。
    • 为什么需要:虽然 save 是 fire-and-forget 设计,但行为变更后没有测试覆盖 HTTP PUT 失败(500、网络错误)时的静默处理逻辑。
    • 建议测试:设置 HTTP URL 指向一个返回 500 或不可达的地址,验证 saveReviewContext 不抛异常,且控制台输出警告信息。
    • 严重程度:LOW
  3. validateLoadedContext 失败时警告日志消失

    • 什么变更:重构后的 fsGetContext 中,validateLoadedContext 返回 null 时不再打印 console.warn("Ignoring malformed or stale review context: ${path}"),旧代码有此日志。
    • 为什么需要:调试辅助信息的退化,虽不影响功能但可能影响问题排查。
    • 建议测试:写一个内容损坏的上下文文件,验证 loadReviewContext 返回 null(现有测试已覆盖),同时验证是否输出了适当的警告日志。
    • 严重程度:LOW

阻塞项:无
警告项:

  • HTTP 失败时不回退文件系统的行为契约缺少测试验证(MEDIUM)
    建议项:
  • saveReviewContext HTTP PUT 失败路径的静默处理缺少测试(LOW)
  • validateLoadedContext 失败时警告日志消失缺少测试(LOW)
test-value

发现低价值测试 / LOW-VALUE TESTS FOUND

  • CRITICAL:无
  • MEDIUM:无
  • LOW:
    1. multi-review/src/context-cache.test.ts:295-298 — HTTP 测试"loadReviewContext returns null when server has no context"仅覆盖了 404 场景,缺少对服务器 5xx 错误、网络不可达、连接拒绝等异常路径的覆盖;httpGetContextcatch 分支返回 null 的行为未测试
    2. multi-review/src/context-cache.test.ts:300-326 — HTTP 圆通和追加测试仅覆盖正路径,缺少对 HTTP PUT 失败的测试(httpPutContext!res.ok 时仅 log warning 并 return),也缺少对 HTTP 401 未授权场景的覆盖(mock server 始终要求 auth,但所有测试都提供了正确 token,未测试 token 错误/缺失时客户端是否正确 fallback)
    3. multi-review/src/context-cache.test.ts:328-341 — 回退文件系统测试缺少对 context-cache-url 设置但 HTTP 请求失败时是否应回退到文件系统的场景覆盖(当前实现不会回退,返回 null,但未测试)
    4. multi-review/src/context-cache.test.ts:300-317 — HTTP 圆通测试未断言 version 字段(FS 后端圆通测试在第 64 行有 assert.strictEqual(loaded!.version, 1),HTTP 版本遗漏该断言)
    5. review-context-server/src/server.ts:1-105 — 新增的 HTTP server 没有任何配套测试文件

阻塞项:None
警告项:None
建议项: 上述 LOW 列出的异常/边界场景覆盖缺失建议补充;HTTP server 单元测试建议添加。

- Update multi-review/package.json and package-lock.json to 4.1.0
- Add CHANGELOG entry for review context persistence feature
@github-actions

Copy link
Copy Markdown

🚫 不可合并 / CANNOT MERGE

安全审查发现路径穿越高危漏洞,且跨 reviewer 确认该缺陷存在于新引入的 review-context-server 中。多个阻塞项必须在合并前修复。


🔴 阻塞项 / Blocking Issues (4)

  • 路径穿越漏洞(CRITICAL)review-context-server/src/server.tsparseContextPath 将 URL 参数直接拼接至 join(DATA_DIR, owner, repo, pr),未对 owner/repo/pr 做任何路径穿越校验。攻击者可利用 ../ 读取或写入 DATA_DIR 之外任意文件。已确认(security + architecture)
  • HTTP 客户端请求未设置超时httpGetContext / httpPutContextfetch 调用无 AbortSignal 保护,缓存服务器无响应时 Action 可能挂起至 6 小时超时。建议设置 10-30 秒超时。已确认(quality + performance)
  • 请求体大小无限制review-context-server/src/server.tsreadBody 函数未限制累积字节数,恶意客户端可耗尽服务器内存。建议增加 maxBodySize(如 10MB),超限返回 413 Payload Too Large已确认(security + quality)
  • 无速率限制 — 所有端点(含 401 响应)均无速率限制,可被用于暴力破解 bearer token 或 DoS 攻击。security reviewer 专属发现

🟡 警告项 / Warnings (5)

  • 服务器使用同步文件 I/OreadFileSync/writeFileSync/mkdirSync/unlinkSync/existsSync 在并发请求时会阻塞事件循环,降低吞吐量。建议改用 fs.promises 异步 API。已确认(quality + performance)
  • 路径安全检查代码失效getContextCacheDir()resolved.startsWith(expectedRoot + "/")resolvedexpectedRoot 值完全相同永远为真,属于死代码。已确认(quality + security + performance)
  • 明文 HTTP 传输敏感数据 — 文档示例使用 http://,bearer token 和 context 数据在网络中明文传输。生产部署应使用 HTTPS。security reviewer 专属
  • 客户端混用同步/异步 I/OfsGetContextexistsSync(同步)与 readFile(异步)混用,同步调用短暂阻塞事件循环。建议统一为 fs.promises.statperformance reviewer 专属
  • HTTP 后端异常路径测试缺失 — 服务端 500 错误、网络不可达、PUT 失败、非法 JSON 响应等错误处理路径未被测试覆盖。test-value reviewer 专属

🟢 建议项 / Suggestions (5)

  • 增加 CORS 头以支持未来浏览器端调试需求。
  • PORT 环境变量进行数字校验,避免 parseInt 返回 NaN 导致运行时错误。
  • PUT 请求 JSON.parse 失败时单独 catch 并返回 400 Bad Request,避免外层 catch 误报 500 错误日志。
  • 文档中明确建议生产部署使用 HTTPS 反向代理(如 nginx TLS termination)。
  • getContextCacheUrl() / getContextCacheToken() 可惰性缓存环境变量读取结果,避免重复解析。
💰 Review Cost — $0.0000
Role Cost (USD) Input Output Reasoning Cache Read Cache Write
quality $0.0000 18,518 454 2,389 0 0
security $0.0000 18,527 851 2,554 0 0
performance $0.0000 18,530 516 1,871 0 0
architecture $0.0000 18,531 338 684 0 0
regression-test $0.0000 18,968 354 2,845 0 0
test-value $0.0000 18,658 346 4,427 0 0
coordinator $0.0000 10,737 821 1,108 0 0
Total $0.0000 122,469 3,680 15,878 0 0
📋 各 Reviewer 详细审查结果
quality

有条件合并 / CONDITIONAL MERGE

本次 PR 新增了 HTTP cache server 后端与 filesystem 后端双轨持久化机制,整体设计合理、测试覆盖完整。但存在以下问题需要在合并前修复:

阻塞项

  1. HTTP 客户端请求(httpGetContext / httpPutContext)未设置超时,若缓存服务器无响应会导致 Action 挂起直至 job 超时(默认 6 小时)。应使用 AbortSignal.timeout()AbortController 设置合理超时(如 10-30 秒)。

警告项

  1. getContextCacheDir() 中的路径安全检查是死代码:resolvedexpectedRoot 均由 resolve(raw) 计算,永远相等,条件永远不成立。此问题在重构时未被修复。
  2. review-context-server 的请求处理器使用了同步 I/O(readFileSync / writeFileSync / existsSync),在并发场景下会阻塞事件循环。而客户端已使用异步 API,建议服务端保持一致。
  3. readBody 未限制请求体大小,恶意或异常客户端可耗尽服务端内存。

建议项

  1. 服务端缺少 CORS 头,若未来有浏览器端调试需求会受限。
  2. 服务端 PORT 环境变量若传入非数字值会导致 parseInt 返回 NaNserver.listen(NaN) 抛出运行时错误,建议增加校验。
  3. 服务端 PUT 请求对 JSON.parse(body) 的异常未单独处理,当前会落入外层 catch 返回 500,行为正确但外层错误日志会误报,建议在 readBody 后立即 try/catch 并返回 400。
security

高危漏洞 / CRITICAL

该 PR 引入了一个独立的 HTTP 缓存服务器 review-context-server,允许通过 URL 路径参数操作文件系统中的 JSON 文件。服务器端未对 URL 路径参数(owner、repo、pr)进行任何路径穿越校验,导致严重安全漏洞。

阻塞项

  1. 路径穿越漏洞 - review-context-server/src/server.ts:58-63parseContextPath 函数使用正则 ^\/context\/([^/]+)\/([^/]+)\/([^/]+)$ 捕获的三个路径段直接传递给 join(DATA_DIR, owner, repo, pr) 参与文件路径构建。发送 /context/../<sensitive-path>/x/y 的请求可将 owner 捕获为 ..join() 会解析到 DATA_DIR 之外的目录,导致任意文件读(GET)、写(PUT)和删除(DELETE)。这直接对应 OWASP A03:2021 - Injection(Path Traversal),攻击者可利用该漏洞读取服务器上的敏感文件(如 /etc/passwd、GitHub Token 文件)或写入恶意内容。

  2. 无请求体大小限制 - review-context-server/src/server.ts:37-47readBody 函数无 maxBodySize 限制,攻击者可发送极大规模 PUT 请求体以耗尽服务器内存,构成 DoS 攻击向量。

  3. 无速率限制 - review-context-server/src/server.ts:所有端点(包括认证失败的 401 请求)均无速率限制,可被用于暴力破解 bearer token 或发起 DoS 攻击。

警告项

  1. 明文 HTTP 传输敏感数据 - review-context-server/README.md:68-75:文档示例使用 http:// 协议,context-cache-token(bearer token)及 review context 数据均在网络中明文传输。若服务器部署在非 localhost 环境,token 和上下文数据可被中间人攻击截获。

  2. 路径校验逻辑无效 - multi-review/src/context-cache.ts:134-138getContextCacheDirresolved.startsWith(expectedRoot + "/") 检查恒为 true(resolved === expectedRoot),实为无效校验。虽然当前不构成直接漏洞(owner/repo 已被上游验证),但维护了无意义的防御代码,属于安全盲区。

建议项

  1. 建议在服务器端对 owner、repo、pr 参数增加与客户端一致的 isSafePathComponent 校验(拒绝包含 ..\0 或非 [\w./-] 字符的输入),并将路径参数交由 resolve() 后检查是否仍在 DATA_DIR 范围内。
  2. 建议在 readBody 中增加 maxBodySize 限制(例如 10MB),超限时直接返回 413 Payload Too Large
  3. 建议文档中明确提示生产部署应使用 HTTPS,并提供反向代理(如 nginx TLS termination)的参考配置。
performance

性能有疑虑 / CONCERNS

该 PR 引入了可选的 HTTP 缓存后端和独立缓存服务器用于持久化 review context,整体改动对算法效率和资源利用影响不大(数据量小、单次网络操作),但存在若干值得关注的问题。

阻塞项:无

警告项:

  1. 缓存服务器全部使用同步文件 I/Oreview-context-server/src/server.ts 中的 readFileSyncwriteFileSyncmkdirSyncunlinkSyncexistsSync 都会阻塞 Node.js 事件循环。当多个 runner 同时请求(如多 PR 并发触发)时,请求将串行排队,吞吐量下降。应考虑改用 fs.promises 异步 API。

  2. 客户端 fsGetContext 混用同步/异步 I/Omulti-review/src/context-cache.ts 中先用 existsSync(同步)判断文件是否存在,再用 readFile(异步)读取内容。同步调用会短暂阻塞事件循环,建议统一使用 fs.promises.statfs.promises.access 替代。

建议项:

  1. HTTP 客户端未设置请求超时httpGetContexthttpPutContextfetch 调用没有 AbortController/signal 超时保护。若缓存服务器无响应,请求可能一直挂起,导致 CI 步骤超时。建议加一个合理的超时(如 5-10 秒)。

  2. 服务器 readBody 无大小限制review-context-server/src/server.tsreadBody 将所有数据累加在内存中,若意外收到超大请求体可能导致 OOM。建议增加 req.on('data') 的累积字节计数并拒绝超出阈值的数据。

  3. getContextCacheDir 安全校验失效resolvedexpectedRoot 为完全相同的值(都调用了 resolve(raw)),导致路径穿越校验形同虚设。建议用 resolve(homedir()) 或固定已知路径作为基准来比较。此问题不影响性能,但影响安全性。

architecture

架构合理 / SOUND

该 PR 整体架构合理,设计清晰:

  • 新增 review-context-server/ 作为独立顶层模块,与 multi-review/ 无直接依赖,降低了耦合
  • context-cache.ts 内通过策略模式分离 HTTP 后端和文件系统后端,公共 API 签名保持不变,向后兼容
  • 模块放置得当:独立 HTTP Server 单独成目录,HTTP 客户端逻辑放在 action 内部,职责清晰
  • actions/cache@v5 基于 run_id/run_attempt 的 per-run key 方案巧妙解决了 GitHub Cache 不可变的问题

阻塞项:无

警告项:

  1. review-context-server/src/server.ts:54 — 服务器端 getContextPath 未对 ownerrepopr 做路径遍历校验,虽然 client 端 (context-cache.ts) 在发出请求前已做 isSafePathComponent 校验,但服务器作为独立服务应独立进行路径安全检查,防止恶意请求读取/写入 DATA_DIR 之外的文件

建议项:

  1. 服务器端可考虑使用异步文件操作 API(如 readFile/writeFile)而非同步版本,以更好支持并发请求场景
  2. context-cache.ts 中的 getContextCacheUrl()getContextCacheToken() 每次调用都重新读环境变量,可考虑惰性缓存避免重复解析
regression-test

无需回归测试 / NO REGRESSION TESTS NEEDED

PR 类型: NEW_FEATURE

分析
此 PR 主要新增了 HTTP 缓存服务器后端(review-context-server/)和 GitHub Actions cache 步骤,用于跨多轮 review 持久化上下文。同时对已有的 filesystem 后端进行了重构(getContextPathgetContextKey + getFilesystemPath),但行为保持一致,路径计算结果等价。

已有测试文件 context-cache.test.ts 完整保留了 filesystem 后端的全部用例(空 sessions 提前返回、追加、校验、trim、格式化等),并新增了 HTTP 后端的完整测试(空上下文返回 null、读写 roundtrip、追加合并、无 HTTP URL 时回退 filesystem)。

review-context-server/ 是全新组件,按规则不要求为全新功能提供回归测试。

阻塞项:无

警告项:无

建议项:无

test-value

发现低价值测试 / LOW-VALUE TESTS FOUND

  • CRITICAL:无

  • MEDIUM:无

  • LOW

    1. multi-review/src/context-cache.test.ts:208-343 — HTTP 后端测试缺少服务端异常(500)、网络错误(fetch 抛异常)、HTTP PUT 非 200 响应、服务端返回非法 JSON、空 sessions 数组等错误处理路径覆盖,这些路径在源码中均有独立的 console.warn 或 return null 逻辑,但未被任何测试覆盖
  • 阻塞项:无

  • 警告项

    1. multi-review/src/context-cache.test.ts:208-343 — HTTP 服务端错误(500)、网络不可达、PUT 失败、非法 JSON 响应、空 sessions 等异常路径未被测试覆盖,建议补充
  • 建议项:无

@Svtter Svtter merged commit a1ad51e into main Jun 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat New feature or enhancement review:p1 Major review findings setup Setup and installation tool Internal tooling and utilities triaged Issue has been triaged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants