feat(process-manager): add ProcessManager with unified lifecycle, events, and service integrations by DeJeune · Pull Request #13856 · CherryHQ/cherry-studio

DeJeune · 2026-03-27T09:32:27Z

What this PR does

Before this PR:

Each backend service (OvmsManager, OpenClawService) rolled its own process spawn/kill logic — fire-and-forget exec() with no stored PID, PowerShell kill-by-name, pkill -9, etc.
No unified process lifecycle management, status tracking, or centralized shutdown.

After this PR:

Introduces a ProcessManager lifecycle service that provides unified process registration, lifecycle management (start/stop/restart), state tracking, event hub, and graceful shutdown.
ChildProcessHandle wraps crossPlatformSpawn with SIGTERM→SIGKILL graceful shutdown, log forwarding, and state machine (Idle→Running→Stopping→Stopped/Crashed).
UtilityProcessHandle wraps Electron's utilityProcess.fork() with MessagePort IPC.
TaskExecutor provides a composite layer for parallel task dispatch via auto-scaling worker pools.
OvmsManager and OpenClawService are migrated to use ProcessManager for their process lifecycle.

Why we need it and why it was done in this way

Centralized process management provides:

Stored PIDs — no more kill-by-name hacks
State tracking — know if a process is Running, Stopped, or Crashed
Graceful shutdown — SIGTERM with configurable timeout, then SIGKILL
Unified event hub — process:started, process:exited, process:log events
Automatic cleanup — all managed processes are stopped on app quit via lifecycle onStop()

The following tradeoffs were made:

OpenClaw gateway uses skipOnStop: true because it's intentionally detached to survive app exit — ProcessManager tracks it but doesn't kill it on shutdown.
MCPService integration is deferred — the MCP SDK owns StdioClientTransport internally, and wrapping it would be fragile.
OvmsManager's addModel()/stopAddModel() stay as-is (one-shot execAsync with stdout capture) — only the long-running server process uses ProcessManager.

The following alternatives were considered:

Wrapping MCP's StdioClientTransport — rejected due to tight SDK coupling
Making ProcessManager extend EventEmitter directly — kept composition with internal emitter for type safety

Breaking changes

None. This is additive — existing behavior is preserved.

Special notes for your reviewer

ChildProcessHandle and UtilityProcessHandle are the primitive layer; ProcessManager is the orchestration layer; TaskExecutor is the composite layer.
77 new tests covering the process module (types, ChildProcessHandle, UtilityProcessHandle, ProcessManager, TaskExecutor).
OvmsManager integration removes the explicit stopOvms() from will-quit handler — ProcessManager.onStop() handles it during application.shutdown().
OpenClaw's killAllOpenClawProcesses() is kept for stopping since the gateway may be externally started.

Checklist

PR: The PR description is expressive enough and will help future contributors
Code: Write code that humans can understand and Keep it simple
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
Upgrade: Impact of this change on upgrade flows was considered and addressed if required
Documentation: A user-guide update was considered and is present (link) or not required.
Self-review: I have reviewed my own code before requesting review from others

Release note

NONE

Define ProcessState enum, ProcessHandle, UtilityProcessHandle, ProcessLogLine, ProcessManagerEvents, ChildProcessDefinition, UtilityProcessDefinition, and ProcessDefinition union type as the foundation for the ProcessManager system. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…d logging Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…o loggerService - In the error event handler, call onExited(null, null) after setting state to Crashed so ProcessManagerService receives process:exited for spawn failures (e.g. ENOENT). - Forward stdout data to logger.debug and stderr data to logger.warn before invoking the onLog callback, so output appears in Winston logs. - Add test: error event triggers onExited with (null, null). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…and events Adds ProcessManagerService as a lifecycle service extending BaseService. Provides a central registry for managed child processes with a unified event hub (process:started, process:exited, process:log) forwarded from individual ChildProcessHandle callbacks. Graceful shutdown stops all running processes in parallel, catching individual errors so one failure does not block others. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…stry Add barrel export for process module and register ProcessManagerService in the centralized service registry for lifecycle-managed access. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…t IPC Add UtilityProcessHandle class wrapping Electron's utilityProcess.fork() for structured-clone MessagePort communication. Update ProcessManagerService to handle type: 'utility' registration and update barrel exports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

Electron's utilityProcess.fork() takes (modulePath, args?, options?), not (modulePath, options). Fixed the call to pass args as the second parameter and options as the third. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

Add TaskExecutor, a composite layer over ProcessManagerService that manages a pool of utility process workers for parallel task dispatch. Supports worker reuse, auto-scaling up to a max cap, task queuing, idle timeouts, and clean shutdown with rejection of pending tasks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

@Injectable

…ager Shorter name, consistent with the pattern used by other services. Updated class name, @Injectable identifier, file names, imports, exports, and documentation references. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…ldProcessDefinition - Add `detached`, `stdio`, and `skipOnStop` optional fields to `ChildProcessDefinition` in types.ts - Add `skipOnStop: boolean` readonly property to `ProcessHandle` interface - Import `StdioOptions` from `child_process` in types.ts - Pass `detached` and `stdio` through to `crossPlatformSpawn` in `ChildProcessHandle.start()` - Call `child.unref()` when `detached` is true - Add `skipOnStop` getter to `ChildProcessHandle` (reads from def, defaults false) - Add `skipOnStop` getter to `UtilityProcessHandle` (always false) - Filter out `skipOnStop` handles in `ProcessManager.onStop()` - Add `unref: vi.fn()` to `createMockChildProcess` in both test files - Add tests for detached, stdio, skipOnStop options in ChildProcessHandle.test.ts - Add skipOnStop test in ProcessManager.test.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

Replace fire-and-forget exec() in runOvms() with ProcessManager registration so the ovms-server handle is tracked and stopped automatically during application.shutdown(). stopOvms() now delegates to the PM handle when available, falling back to the PowerShell kill for externally-started processes. getOvmsStatus() checks the PM handle before querying PowerShell. Remove the explicit ovmsManager.stopOvms() call from the will-quit handler in index.ts — ProcessManager.onStop() handles cleanup automatically. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

Signed-off-by: suyao <sy20010504@gmail.com> # Conflicts: # src/main/core/application/serviceRegistry.ts # src/main/index.ts

… tests - Wrap crossPlatformSpawn() and utilityProcess.fork() in try/catch so state transitions to Crashed and onExited fires on synchronous errors - Tighten weak toContain assertions in TaskExecutor tests to exact match - Add edge-case tests: spawn throw, fork throw, pid undefined, skipOnStop - Remove trivial types.test.ts (enum string-value checks) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…rface - Delete duplicate `interface UtilityProcessHandle` from types.ts that shadowed the class of the same name and was never referenced - Add explicit `implements ProcessHandle` to the UtilityProcessHandle class for compile-time contract enforcement Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…ype discriminant - Extract common fields (id, args, env, killTimeoutMs) into ProcessOptions base - Rename ChildProcessDefinition → ChildProcessOptions - Rename UtilityProcessDefinition → UtilityProcessOptions - Remove type discriminant field; use structural narrowing ('modulePath' in options) - Simplify ProcessManager.register() callback wiring (deduplicate) - Update all callers and tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

DeJeune

Well-structured process management addition with clean architecture (primitive handles → orchestrator → composite executor). Found 4 issues:

[A1] ChildProcessHandle.stop() can hang if process exits during race window (missing resolve() in kill timeout)
[A7] Double onExited callback when both error and close events fire
[A7] OpenClawService overrides callbacks after start(), creating a race for early exit diagnostics
[C1] String literal 'running' instead of ProcessState.Running enum in OpenClawService

…ack race - ChildProcessHandle.stop(): add resolve() in kill timeout callback to prevent promise hanging when process exits during race window - ChildProcessHandle: add _exited guard to prevent double onExited callback when both error and close events fire - OpenClawService: move onLog/onExited callback overrides before handle.start() to capture early process events - OpenClawService: use ProcessState.Running enum instead of string literal for consistency with OvmsManager Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…k and add _exited guard to UtilityProcessHandle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…down filter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…ate leaks on assertion failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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

…pectedly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

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

- Add taskTimeoutMs option to TaskExecutor for per-task timeout support - Add handleTaskTimeout to reject tasks and free workers when timeout fires - Clear timeout timers on normal response, shutdown, and worker crash - Add public getters (workerCount, pendingCount, queueLength) to TaskExecutor - Fix scheduleIdleTimeout to use finally for unregister (handles stop() failure) - Refactor tests: use direct state assertions, remove redundant/trivial tests - Add task timeout tests (TDD red→green) - Fix unhandled promise rejections across all test cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

- Remove pass-through tests (fork args, spawn env, detached, stdio options) - Remove hardcoded constant tests (skipOnStop) - Remove pure wiring tests (onStarted, onExited callbacks) - Merge duplicate stdout/stderr log tests into single test - Replace dynamic imports with static imports, remove loadModules() - Use vi.hoisted() for UtilityProcessHandle electron mock Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

- Remove trivial initial state tests (already covered by handle tests) - Remove UtilityProcessHandle type test that only checked id/state - Remove Map.get() undefined semantics test - Remove empty onInit test - Remove unregister no-op for unknown id test - Merge duplicate stdout/stderr log event tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

Conflicts: - src/main/index.ts: removed unused `runAsyncFunction` import (v2 deleted it) - src/main/services/OvmsManager.ts: combined lifecycle decorator imports from v2 with application/ProcessState imports from our branch; dropped unused isWin and getCpuName imports Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

…on, reduce duplication - Extract DEFAULT_KILL_TIMEOUT_MS to types.ts (was duplicated in both handles) - Replace Date.now()+Math.random() task ID with crypto.randomUUID() - Use spawnWorker() return value directly instead of re-scanning workers map - Remove redundant vi.mock('@logger') from 4 test files (global setup handles it) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

cherry-ai-bot

Cherry Review · 阻塞

4 blocker · 8 warning · 1 notice
逐条见行内评论。

cherry-ai-bot · 2026-08-23T15:31:38Z

+    return this.def.skipOnStop ?? false
+  }
+
+  async start(): Promise<void> {


必须修复：An in-flight start remains visible as Idle, so concurrent starts or unregister/replacement can create an untracked child process.

inv_d75b471e44106063#c0

cherry-ai-bot · 2026-08-23T15:31:38Z

-    })
+    }
+
+    await handle.start()


必须修复：OpenClaw startup and restart can leave a managed gateway handle registered after failure, causing later registration to fail with a duplicate-id error.

inv_d75b471e44106063#c1

cherry-ai-bot · 2026-08-23T15:31:38Z

+    this.dispatch()
+  }
+
+  private handleTaskTimeout(taskId: string, entry: WorkerEntry): void {


建议修复：Timeout cleanup marks workers idle or terminal before the underlying process has exited, allowing reuse of a worker that may still be executing and suppressing exit notifications.

inv_d75b471e44106063#c2

cherry-ai-bot · 2026-08-23T15:31:38Z

+
+    this.workers.set(workerId, entry)
+
+    await handle.start()


建议修复：A worker start failure can strand queued exec() promises indefinitely.

inv_d75b471e44106063#c3

cherry-ai-bot · 2026-08-23T15:31:38Z

+export { ChildProcessHandle } from './ChildProcessHandle'
+export { ProcessManager } from './ProcessManager'
+export type { TaskExecutorOptions } from './TaskExecutor'
+export { TaskExecutor } from './TaskExecutor'


必须修复：TaskExecutor is exposed as a new public process abstraction without a legitimate production consumer or concrete invariant requiring it.

inv_d75b471e44106063#c4

cherry-ai-bot · 2026-08-23T15:31:38Z

+/**
+ * Process states - shared across all process types
+ */
+enum ProcessState {


建议修复：The English and Chinese process-manager references document public types and examples that do not match the shipped implementation.

inv_d75b471e44106063#c8

cherry-ai-bot · 2026-08-23T15:31:39Z

+
+### Graceful Shutdown Sequence
+
+When `ProcessManager.onStop()` is called (app shutdown):


建议修复：The bilingual shutdown and migration documentation contradicts current ownership and lifecycle behavior.

inv_d75b471e44106063#c9

cherry-ai-bot · 2026-08-23T15:31:39Z

+
+    let proc: Electron.UtilityProcess
+    try {
+      proc = utilityProcess.fork(this.def.modulePath, this.def.args, {


建议修复：Utility workers launched by TaskExecutor can receive an empty environment instead of inheriting the main-process environment.

inv_d75b471e44106063#c10

cherry-ai-bot · 2026-08-23T15:31:39Z

+      this.onStarted?.(child.pid)
+    }
+
+    child.stdout?.on('data', (data: Buffer) => {


待作者确认：Shell output is decoded as UTF-8 rather than with the repository's Windows OEM/GBK decoder, so Chinese Windows output can be garbled.

inv_d75b471e44106063#c11

cherry-ai-bot · 2026-08-23T15:31:39Z

+      const mockCp1 = createMockChildProcess(1111)
+      const mockCp2 = createMockChildProcess(2222)
+
+      mockCp1.kill = vi.fn().mockImplementation(() => {


建议修复：The ProcessManager shutdown test leaves a delayed SIGKILL timer whose callback throws outside the test promise.

inv_d75b471e44106063#c12

0xfullex · 2026-08-25T05:01:43Z

Thank you very much for the extensive work on unified process management.

The main-process lifecycle, service ownership, and process integrations have continued to evolve since this PR was opened. This implementation also has unresolved review blockers and no longer applies cleanly to current main, so it cannot be merged safely as-is.

We are closing the PR as part of the v2 cleanup. The underlying requirement may still be valuable; if you would like to continue, you are very welcome to reopen this PR and redesign/update it against the latest main branch, or open a clean follow-up PR.

Thank you again for your contribution and understanding.

Resolve ProcessManager integration against the current lifecycle, process runner, and documentation architecture. Signed-off-by: suyao <sy20010504@gmail.com>

Signed-off-by: suyao <sy20010504@gmail.com>

DeJeune and others added 13 commits March 27, 2026 01:56

feat(process-manager): implement ChildProcessHandle with lifecycle an…

f9b510a

…d logging Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

Saving uncommitted changes before archiving

9f86b10

feat(openclaw): register OpenClaw gateway with ProcessManager

a6281eb

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

DeJeune changed the base branch from main to v2 March 27, 2026 09:51

DeJeune and others added 4 commits March 27, 2026 18:23

drop superpower docs

c487fb2

Merge remote-tracking branch 'origin/v2' into DeJeune/process-manager

aeb2abd

Signed-off-by: suyao <sy20010504@gmail.com> # Conflicts: # src/main/core/application/serviceRegistry.ts # src/main/index.ts

DeJeune added the v2 Related to the v2 codebase, migration, or release line label Mar 27, 2026

DeJeune commented Mar 27, 2026

View reviewed changes

Comment thread src/main/services/process/ChildProcessHandle.ts

Comment thread src/main/services/process/ChildProcessHandle.ts Outdated

Comment thread src/main/services/OpenClawService.ts

Comment thread src/main/services/OpenClawService.ts Outdated

DeJeune and others added 9 commits March 28, 2026 00:06

fix(process-manager): update state to Stopped in kill-timeout callbac…

ff7fc53

…k and add _exited guard to UtilityProcessHandle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

fix(process-manager): include Stopping-state processes in onStop shut…

38768ac

…down filter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

fix(tests): add afterEach vi.useRealTimers() to prevent fake timer st…

5d8ed58

…ate leaks on assertion failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix(tests): remove redundant afterEach import since vitest globals ar…

192ca8c

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

fix(process-manager): reject in-flight tasks when worker crashes unex…

5cdee0a

…pectedly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>

fix(tests): add on/off mock methods to MockPM for process:exited even…

2919f7c

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

refactor(tests): task-executor-test

95949d3

DeJeune and others added 3 commits March 28, 2026 14:58

DeJeune marked this pull request as ready for review March 28, 2026 07:09

DeJeune requested a review from a team March 28, 2026 07:09

DeJeune added this to the v2.0.0 milestone Apr 18, 2026

Base automatically changed from v2 to main May 29, 2026 02:54

0xfullex self-requested a review as a code owner May 29, 2026 02:54

cherry-ai-bot Bot requested changes Aug 23, 2026

View reviewed changes

0xfullex closed this Aug 25, 2026

DeJeune reopened this Aug 28, 2026

DeJeune added 2 commits August 29, 2026 02:51

chore(process-manager): merge origin/main

db8791d

Resolve ProcessManager integration against the current lifecycle, process runner, and documentation architecture. Signed-off-by: suyao <sy20010504@gmail.com>

fix(process-manager): harden managed process lifecycle

3de57ed

Signed-off-by: suyao <sy20010504@gmail.com>


		### Graceful Shutdown Sequence

		When `ProcessManager.onStop()` is called (app shutdown):

Uh oh!

Conversation

DeJeune commented Mar 27, 2026

What this PR does

Why we need it and why it was done in this way

Breaking changes

Special notes for your reviewer

Checklist

Release note

Uh oh!

DeJeune left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cherry-ai-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

cherry-ai-bot Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

0xfullex commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants