fix: 落地 REVIEW-2026-06-10 P0–P2 — 数据不丢、本地 API 不被劫持、任务不再卡死

[moose-lab]

用户视角:这次更新你会感受到什么

🗄️ 你的数据不会再无声丢失

• 升级 DevLog 后,历史会话与任务的关联完整保留。 旧版数据库迁移有一个隐蔽缺陷:升级时每条会话的 task 关联会被静默清空,会话日志也可能被连带删除。现在迁移走 SQLite 官方重建流程,并在迁移后自动做完整性校验。
• 运行测试数据脚本不会再清空你真实的 Claude Code 历史。 之前 generate-test-data 一执行就 rm -rf ~/.claude/projects(不可恢复)。现在它默认拒绝触碰真实目录,需要显式 --force,且只清理它自己生成的演示项目。

📦 npm 安装版终于能用了

• devlog serve 不再报"需要源码仓库"直接退出——之前发布的二进制里这个命令是完全坏的。
• devlog setup-statusline / setup-tmux 能找到随包发布的脚本了。
• 以后发布的版本不会再带着几周前的旧代码:发布时强制重新构建,不再提交过期的 dist/。

🔒 浏览网页时,你的本地 DevLog 不再是攻击面

DevLog 跑在 localhost 且无鉴权,以前任何你访问的网页都能悄悄调用它的 API:替你创建并执行任务、用你的 gh 凭证推送 PR、在任意目录写出 worktree、甚至拉起 /bin/sh。现在:

• 所有跨源写请求和 DNS rebinding 请求一律 403;curl/脚本等本地工具不受影响。
• worktree 名称/分支、Claude 会话 ID、自定义二进制路径、代理 Base URL 全部在入口校验,路径穿越、git 参数注入、SSRF、任意二进制执行均被堵住。
• 自定义 CLI 路径功能保留:只要文件确实叫 claude/codex 等就照常生效。

⚙️ 长任务、人工确认和任务看板更可靠

• 慢回合不再被反复杀掉重跑。 之前只要 3 分钟没有输出(大型构建、慢模型),看门狗就 SIGKILL 进程并把你的消息重新执行——无限循环,还会重复产生副作用。现在从发送消息起计活动时间,重启最多 2 次,之后如实标记失败并释放任务。
• 空闲会话不再每 3 分钟被无谓重启一次。
• 人工确认(gate)的回答不会再丢。 之前对 codex/gemini 等 CLI,你在界面上点了"继续",答案实际被静默丢弃,界面却显示成功、任务永远卡住。现在答案保证送达(必要时自动换新进程),送不到就如实报错并保留待确认状态,可以重试。
• 看板状态更真实:正常完成且无改动的会话不再被误标为 blocked;DevLog 重启后,卡在"进行中"的孤儿任务会被释放为可重试,而不是永远占着状态。

范围与验证

• 覆盖 review 的 Phase 0–2:全部 9 个 Critical(CR-1~9)+ 6 个 Important(IM-5/6/8/9/19/21/22)。
• 每个修复独立提交,均带回归测试(套件 224 → ~250),每次提交经 pre-commit 强制 typecheck + 全量测试;quality:build(Next + tsup)本地通过;中间件行为已起真实服务用 curl 验证。
• 未包含(review Phase 3/4,后续跟进):统计数字正确性(IM-1/3/24)、发现缓存(IM-23)、前端共享数据层(IM-13/27/28/30)。

🤖 Generated with Claude Code

[claude]

Claude finished @moose-lab's task in 4m 12s —— View job

---

Code Review

• Gather context & read changed files
• Review security changes (request-origin-guard, validation)
• Review DB migration changes
• Review process-manager / watchdog changes
• Review CLI / package-root fixes
• Review control-plane-protocol changes
• Review tests
• Post final findings

---

Findings

• [must-fix] src/proxy.ts is never mounted — CR-6 guard is dead code (src/proxy.ts): No middleware.ts exists anywhere in the project (src/middleware.ts or root middleware.ts), and nothing imports src/proxy.ts outside its own test. The guard implementation and its 8 tests are correct, but the function is never invoked on any real HTTP request. Every API route is still reachable cross-origin. Fix: create src/middleware.ts with export { proxy as default, config } from "@/proxy". Fix this →

• [must-fix] rebuildTable issues BEGIN/COMMIT via raw db.exec() strings (src/core/db.ts:233–239): better-sqlite3's .transaction() API is the safe way to do this — raw BEGIN inside exec() will throw SqliteError: cannot start a transaction within a transaction if the caller is ever inside an implicit transaction (e.g. a future migration wrapper). The current callers are all top-level and don't trigger this, but the pattern is fragile. Also, if the BEGIN/INSERT/DROP/RENAME chain succeeds but the postSql (index creation) throws, the COMMIT has already run, so the ROLLBACK in the catch block would be a no-op while the half-indexed table stays. Fix this →

• [should-fix] resolveGate clears the persisted gate before writeMessage confirms delivery (src/core/process-manager.ts:1358–1359): There is a narrow window between canDeliverGateResponse(sp) at line 1339 and writeMessage(sp, trimmed) at line 1359 where the process could die, leaving the gate cleared in the DB but the response never delivered. The PR comment acknowledges that the gate is only cleared "once delivery is possible", but resolveControlPlaneGate runs unconditionally at line 1358. Consider swapping: call writeMessage first (which handles its own write errors), then clear the gate only if the write succeeded.

• [should-fix] migrateControlPlaneColumns is called twice in getDb() (src/core/db.ts:43,138): Lines 43 and 138 both call migrateControlPlaneColumns. The second call (after all the ALTER TABLE sessions ADD COLUMN blocks) is harmless since the columns already exist, but it's confusing and suggests an integration error during rebasing. The first call (line 43) is correct; remove the second. Fix this →

• [should-fix] onSessionExit falls through to neither branch when !hasChanges && !failedExit (src/core/task-lifecycle.ts:170–178): A clean-exit session with no file changes leaves the task in 'in_progress' forever. The PR description says this fixes the "completed session marked blocked" regression, but the fix for the no-changes + clean-exit case (e.g. a session that ran but made no edits) is not handled — the task stays in_progress rather than transitioning to review or done. Whether this was intentional (relying on the human to decide) should be clarified with a comment.

• [nit] generate-test-data.ts:288 — const content: any[] escapes the type system; use Array<{type: string; text?: string; [key: string]: unknown}> or the existing inferred union type.

• [nit] serve.ts:16 — checks for next.config.ts specifically; a repo using next.config.js would incorrectly get the "requires source repo" error. Low risk given this project is TypeScript-only.

• [nit] src/proxy.ts exports config with a matcher — config.matcher is meaningful only in a Next.js middleware file. Exporting it from a non-middleware module is misleading; move it to the middleware.ts that should be created, or document why it's here.

---

Verdict: REQUEST_CHANGES

The overall PR is high quality — the DB rebuild logic (CR-1/CR-2), watchdog caps (CR-3), gate delivery ordering (CR-4), worktree/branch validation (CR-5), binary path validation (CR-7), VCC session-ID containment (IM-19), and base-URL SSRF filter (IM-21) are all well-reasoned fixes with good test coverage. The test suite additions are meaningful rather than trivially green.

However, CR-6 (CSRF/DNS rebinding protection) — arguably the highest-impact security fix in the PR — is incomplete: the guard is tested in isolation but never wired into the request pipeline. This should be a quick one-liner fix (src/middleware.ts) but needs to land before merging.

[claude]

Claude finished @moose-lab's task in 2m 19s —— View job

---

Code Review

• Read CLAUDE.md for repo instructions
• Check new commits since last review
• Verify must-fix 1: src/middleware.ts mounted (CR-6 guard)
• Verify must-fix 2: rebuildTable transaction fix
• Check should-fix items (gate clearing, duplicate migration call, onSessionExit)
• Post final findings

---

The latest push (537f785) closed two of the five issues from the previous review. Here's the full current state:

What's been fixed ✓

• [should-fix ✓] Duplicate migrateControlPlaneColumns call — removed in 537f785. The second call at the old line 138 is gone; only the call at line 43 remains.

• [should-fix ✓] onSessionExit clean-exit/no-changes case — now intentionally documented with an inline comment in 537f785: the session stays in_progress so the human can decide, which is the correct design. No further action needed.

---

Still open

• [must-fix] src/middleware.ts does not exist — CR-6 guard is still dead code (src/proxy.ts): src/proxy.ts still has no middleware.ts counterpart. No file at src/middleware.ts or the project root. The guard function and its 8 tests are correct, but every API route is still reachable cross-origin. One file, two lines to fix:

  // src/middleware.ts
  export { proxy as default, config } from "@/proxy";

  Fix this →

• [must-fix] rebuildTable uses raw BEGIN/COMMIT inside db.exec() (src/core/db.ts:232–238): The multi-statement string passed to db.exec() at line 232 opens a BEGIN and closes with COMMIT. If any caller is inside a db.transaction() (including future migration wrappers), this will throw SqliteError: cannot start a transaction within a transaction. The catch block's if (db.inTransaction) db.exec("ROLLBACK") is a no-op in that scenario because db.exec() already threw before COMMIT ran, leaving state unclear. Use db.transaction(() => { ... })() instead, which composes correctly with nested callers.

  Fix this →

• [should-fix] resolveGate still clears the persisted gate before writeMessage confirms delivery (src/core/process-manager.ts:1358–1359): resolveControlPlaneGate(db, sessionId) runs at line 1358, clearing the DB record, then writeMessage runs at line 1359. If writeMessage throws (e.g. the stdin pipe is broken), the gate is gone from the DB but the answer was never delivered. Swap the two lines: call this.writeMessage(sp, trimmed) first, then resolveControlPlaneGate only if the write didn't throw.

---

Verdict: REQUEST_CHANGES

The two must-fix items are straightforward one-liners. The security fix (middleware wiring) in particular has zero functional code to write — only the re-export file is missing.

[moose-lab]

Re: review findings — verified each claim before acting. Summary:

[must-fix] "src/proxy.ts is never mounted" — incorrect, verified empirically. proxy.ts is Next.js 16's file convention replacing middleware.ts (running with a middleware.ts file prints "⚠ The middleware file convention is deprecated. Please use proxy instead"). Verification on this branch:

• Production (next build + next start): POST /api/worktrees with Origin: https://evil.example → 403; GET /api/projects with Host: rebind.attacker.example → 403; same-origin GET → 200.
• Dev server: same results.
• Build artifact: .next/server/middleware.js references chunk 52a24538, which contains the guard (grep "Cross-origin API mutations" .next/server/chunks/...52a24538...js matches).

Adding a src/middleware.ts re-export as suggested would reintroduce the deprecated convention alongside the new one. Not changed.

[must-fix] "rebuildTable BEGIN/COMMIT — postSql runs after COMMIT" — misread. postSql is interpolated before COMMIT inside the same exec block, so a failing index creation aborts the transaction and the catch performs ROLLBACK (guarded by db.inTransaction). The nested-transaction concern is hypothetical today (all callers are top-level, and PRAGMA foreign_keys is a no-op inside a transaction, which forces the toggle to live outside any wrapper). Not changed.

[should-fix] resolveGate clears gate before writeMessage returns. Acknowledged residual: the window is between the writability check and the write a few lines later, and a failure there now surfaces as a visible error + idle status (vs. the original bug: silent loss with a success UI). Making writeMessage report delivery is queued as follow-up work rather than expanding this PR.

Fixed in 537f785: duplicate migrateControlPlaneColumns call removed; intentional no-transition for clean-exit/no-changes sessions documented in onSessionExit.

Nits: generate-test-data.ts typing and the next.config.ts existence check are pre-existing and out of scope here; config.matcher lives in proxy.ts because that is the mounted entry file under the Next 16 convention.

🤖 Generated with Claude Code