Skip to content

fix: query 超时保护 + 进程清理,防止 PM2 重启后残留孤儿进程 - #24

Merged
lishuceo merged 2 commits into
mainfrom
feat/pipeline-phase-d
Feb 18, 2026
Merged

fix: query 超时保护 + 进程清理,防止 PM2 重启后残留孤儿进程#24
lishuceo merged 2 commits into
mainfrom
feat/pipeline-phase-d

Conversation

@lishuceo

Copy link
Copy Markdown
Owner

Summary

  • executor 接入 CLAUDE_TIMEOUT 配置,通过 abortController 强制终止超时 query(默认 300s)
  • 启动时扫描并 kill 残留的 claude 孤儿子进程(PM2 SIGKILL / 服务崩溃场景)
  • shutdown handler 加 3s 延迟等待子进程退出 + 重复信号去重
  • 新增 PM2 ecosystem 配置,kill_timeout 从默认 1600ms 提升到 10s

Test plan

  • 200 个现有测试全部通过
  • 验证 PM2 restart 后无残留 claude 进程
  • 验证 query 超过 CLAUDE_TIMEOUT 后被正确终止

🤖 Generated with Claude Code

- executor: 接入 CLAUDE_TIMEOUT 配置,通过 abortController 强制终止超时 query
- index: 启动时 killOrphanedClaudeProcesses 清理上次残留的 claude 子进程
- index: shutdown handler 加 3s 延迟等待子进程退出 + 重复信号去重
- 新增 ecosystem.config.cjs,PM2 kill_timeout 从默认 1600ms 提升到 10s

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Feb 18, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 1m 33s —— View job


PR Review: Timeout Protection + Process Cleanup

  • Check for previous review comments
  • Read project conventions (CLAUDE.md)
  • Review PR diff and all changed files
  • Post inline comments for issues found
  • Post summary verdict

⚠️ Issues Found

Overall assessment: The PR is well-structured and addresses a real operational pain point (orphan processes after PM2 restarts). The timeout implementation in executor.ts is clean — proper AbortController usage, clearTimeout on both success and error paths, and correct timeout-vs-error differentiation. The shutdown dedup and PM2 ecosystem config are solid additions.

Two moderate issues found:

  • 🟡 Process cleanup too broad (confidence: 82) — killOrphanedClaudeProcesses() kills any process with basename claude, without verifying it's actually orphaned (PPID=1). On a shared server, this could kill legitimate claude processes belonging to other users or services. See inline comment with suggested fix.

  • 🟡 Interval not cleared on shutdown (confidence: 76) — The 30-minute cleanup interval can fire during the 3-second shutdown grace period, calling sessionManager.cleanup() / pipelineStore.cleanExpired() on already-closed resources. See inline comment.

What looks good:

  • Timeout implementation with AbortController + clearTimeout in both paths is correct
  • Shutdown signal deduplication via shuttingDown flag
  • PM2 kill_timeout: 10000 aligns well with the 3s shutdown delay
  • Test mock updated to include timeoutSeconds
  • ESM patterns and .js import extensions followed consistently

Comment thread src/utils/process-cleanup.ts
Comment thread src/index.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Feb 18, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 3m 11s —— View job


PR Re-review: Timeout Protection + Process Cleanup

  • Check previous review comments against current code
  • Resolve fixed issues from previous review
  • Read all changed files in full
  • Review current diff for new issues
  • Post summary verdict

✅ Approved — All Previous Issues Fixed, No New Issues Found

2 previous issues fixed:

  1. Process cleanup now checks PPID=1killOrphanedClaudeProcesses() at src/utils/process-cleanup.ts:37-43 now verifies the process is a genuine orphan (reparented to init) before killing it. Threads resolved.
  2. Interval cleared on shutdowncleanupInterval is stored (src/index.ts:42) and cleared via clearInterval(cleanupInterval) at the start of shutdown() (src/index.ts:57). Thread resolved.

Review of current state:

  • Timeout implementation (src/claude/executor.ts:130-137) — Clean AbortController + setTimeout pattern. clearTimeout(timer) correctly called in both the catch block (line 283) and the success path (line 301). The timedOut flag properly differentiates timeout errors from other failures.
  • Process cleanup (src/utils/process-cleanup.ts) — Safe: pid is parseInt-parsed (line 25) before interpolation into execSync (line 39), eliminating command injection risk. Triple filtering (basename check → PPID=1 check → skip self) is thorough.
  • Shutdown deduplication (src/index.ts:50-68) — shuttingDown flag prevents duplicate signal handling. clearInterval + claudeExecutor.killAll() + 3s grace period is well-structured. PM2 kill_timeout: 10000 in ecosystem.config.cjs gives adequate headroom.
  • Test mock (src/claude/__tests__/executor.test.ts:15) — Updated to include timeoutSeconds: 300, matching the config shape.
  • ESM patterns.js import extensions followed consistently throughout.

No new issues found. The PR is clean and ready to merge.

@lishuceo
lishuceo merged commit 95d0f22 into main Feb 18, 2026
5 checks passed
@lishuceo
lishuceo deleted the feat/pipeline-phase-d branch February 18, 2026 19:38
lishuceo added a commit that referenced this pull request Apr 8, 2026
* fix: query 超时保护 + 进程清理,防止 PM2 重启后残留孤儿进程

- executor: 接入 CLAUDE_TIMEOUT 配置,通过 abortController 强制终止超时 query
- index: 启动时 killOrphanedClaudeProcesses 清理上次残留的 claude 子进程
- index: shutdown handler 加 3s 延迟等待子进程退出 + 重复信号去重
- 新增 ecosystem.config.cjs,PM2 kill_timeout 从默认 1600ms 提升到 10s

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: 仅清理孤儿进程 (PPID=1) + shutdown 时清理定时器

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant