feat: 管道状态持久化 + 交互式卡片按钮 (Phase C)

[lishuceo]

Summary

• 新增 PipelineStore (SQLite) 持久化管道状态，支持服务崩溃后恢复并通知用户
• /dev 命令改为先发送确认卡片（含成本预估），用户点击「确认执行」后才开始管道，新增「中止」「取消」「重试」交互按钮
• 新增 Pipeline Runner 模块统一管理管道生命周期，会话锁延迟到确认时获取，避免阻塞聊天

Test plan

• npm test — 200 tests 全部通过（含新增 store/runner/orchestrator abort 测试）
• npm run typecheck — TypeScript 类型检查通过
• 发送 /dev <简单任务> → 看到确认卡片（含「确认执行」和「取消」按钮）
• 点击「取消」→ 卡片更新为已取消状态
• 再次 /dev <任务> → 确认 → 看到管道进度卡片（含「中止」按钮）
• 管道完成/失败 → 失败时看到「重试」按钮
• 管道执行中重启服务 → 启动后被中断管道的卡片更新为中断状态（含重试按钮）

🤖 Generated with Claude Code

[claude]

Claude finished @lishuceo's task in 2m 16s —— View job

---

PR Review: Pipeline State Persistence + Interactive Card Buttons (Phase C)

• Check for previous review comments
• Gather PR diff and context
• Review all changed files (11 files, +1412/-157)
• Post inline comments for issues found
• Post summary

---

⚠️ Issues Found

Overall assessment: Well-structured PR with clean separation of concerns. The PipelineStore (SQLite persistence), PipelineRunner (lifecycle management), and interactive card buttons are solid additions. The test coverage is thorough (store CRUD, CAS atomicity, orchestrator abort/retry/cost-tracking). However, there are a few issues worth addressing before merging:

Severity	Issue	File	Confidence
🔴 Critical	No user authorization on card actions — any user in the chat can confirm/cancel/abort/retry another user's pipeline	src/feishu/event-handler.ts:88	85
🟡 Warning	Abort status overwrite — abortPipeline() sets status to 'aborted', but startPipeline()'s completion handler overwrites it to 'failed'	src/pipeline/runner.ts:271-285	80
🟡 Warning	TOCTOU between get() and tryStart() CAS — redundant status check in handlePipelineConfirm could become inconsistent	src/pipeline/runner.ts:108-119	82
🟡 Warning	Pipeline ID collision risk — 2 bytes of randomness (65K range) with ms-precision timestamp	src/pipeline/store.ts:229-231	78
🔴 Info	Module-level SQLite singleton — pipelineStore opens DB at import time; fragile for tests and startup error handling	src/pipeline/store.ts:233	90

What looks good

• CAS double-click prevention via tryStart() — correct SQLite-level atomicity
• Graceful shutdown in src/index.ts — marks running pipelines as interrupted and closes the DB
• Crash recovery — recoverInterruptedPipelines() correctly marks running→interrupted on startup and notifies users
• Session lock deferred to confirm — avoids blocking chat while waiting for user confirmation
• Comprehensive test suite — store tests cover CRUD, CAS, expiration; orchestrator tests cover happy path, retry logic, cost accumulation, abort, and callback resilience
• Card builder functions are well-organized with proper phase state rendering

[claude]

Claude finished @lishuceo's task in 2m 37s —— View job

---

PR Review: Pipeline State Persistence + Interactive Card Buttons (Phase C) — Follow-up

• Check previous review comments and resolve fixed issues
• Read current diff and all 11 changed files
• Review fix commit (db6bd75) for correctness and new issues
• Post summary

---

Previous Issues: 3 of 5 Fixed

The fix commit db6bd75 addressed 3 of the 5 issues from the previous review:

Issue	Status	Details
Card action authorization	✅ Fixed	Added operatorId extraction and validation (event-handler.ts:86-98)
Abort status overwrite	✅ Fixed	abortPipeline() no longer writes status; startPipeline() checks orchestrator.isAborted() (runner.ts:199-201)
Pipeline ID collision risk	✅ Fixed	randomBytes(2) → randomBytes(4) (store.ts:230)
TOCTOU race condition	Acknowledged	Author correctly noted the early-exit is an optimization; tryStart() CAS is the sole source of truth
Module-level singleton	Acknowledged	Follows established project pattern; config validates before pipeline code executes

All 3 fixed threads have been resolved. The test (store.test.ts:283) was also updated to match the new 8-hex-char format.

New Review: No New Issues Found

The fix commit is clean. The isAborted() method added to PipelineOrchestrator is minimal and correct. The auth check implementation is sound — the record && operatorId && record.userId !== operatorId guard correctly rejects mismatched users while allowing actions to proceed if operator info is unavailable (graceful degradation).

✅ Approved

Well-structured PR with thorough test coverage (store CRUD, CAS atomicity, orchestrator abort/retry/cost tracking, callback resilience). All previously identified issues are resolved or acknowledged.

[claude]

Claude finished @lishuceo's task in 2m 32s —— View job

---

PR Review: Pipeline State Persistence + Interactive Card Buttons (Phase C) — Re-review

• Check previous review comments and resolve fixed issues
• Gather current diff and read all changed files
• Review latest commit (f85c4f9) for correctness and new issues
• Post summary

---

Latest Commit: f85c4f9 — "fix: 修复 deep review 发现的关键问题"

Reviewed all 7 files changed in this commit (+83/-88 lines). The changes address real issues and improve code quality:

Change	Assessment
Separate pipeline DB — pipelineStore now uses config.db.pipelineDbPath instead of sharing sessionDbPath	✅ Correct. Prevents table namespace collisions between sessions and pipelines
ensureThread extraction — Deduplicated thread creation into src/feishu/thread-utils.ts, shared by event-handler.ts and runner.ts	✅ Clean refactor. Eliminates the duplicated ensureThreadForPipeline
Auth check hardened — Changed from permissive (skip if no operatorId) to fail-closed (reject if no operatorId)	✅ Correct security posture. Lines 94-98 of event-handler.ts
CAS moved to handlePipelineConfirm — tryStart() now runs synchronously before returning the progress card, with startPipeline doing a defensive fallback check	✅ Correct. Prevents users from seeing a stuck progress card on CAS failure
Error card update wrapped in try-catch — runner.ts:227-236	✅ Prevents cascading failures when Feishu API is unreachable
Test mocks updated — Config mocks include pipelineDbPath, thread-utils.js mock added	✅ Consistent with code changes

Previous Issues Status

All 5 issues from the original review are now resolved or acknowledged across the two fix commits:

Issue	Final Status
Card action authorization	✅ Fixed (db6bd75) + hardened to fail-closed (f85c4f9)
Abort status overwrite	✅ Fixed (db6bd75)
Pipeline ID collision risk	✅ Fixed (db6bd75)
TOCTOU race condition	✅ Acknowledged — CAS is sole source of truth; improved in f85c4f9 by moving CAS earlier
Module-level singleton	✅ Acknowledged — follows project pattern; now uses separate DB path

New Issues Found: None

The latest commit is clean. I traced through the CAS flow (handlePipelineConfirm → startPipeline), abort status propagation (orchestrator.isAborted() → 'aborted' status in store), and error handling paths — all are correct. The ensureThread extraction is a clean refactor with no behavioral changes.

Minor style note (not blocking): In runner.ts:37, the import { ensureThread } is placed after the interface definition rather than with the other imports at the top of the file. This doesn't affect functionality.

✅ Approved

Well-structured PR with comprehensive fixes across 3 commits. The pipeline persistence, interactive card buttons, crash recovery, and auth controls are all solid. Test coverage is thorough (store CRUD, CAS atomicity, orchestrator abort/retry/cost tracking, callback resilience).