feat: restore daemon supervisor and remoteControlServer command#170
Conversation
Reverse-engineer the missing daemon + remoteControlServer implementation by tracing the call chain from existing code: - src/daemon/main.ts: restore from stub to full supervisor (spawn/monitor workers, exponential backoff restart, graceful shutdown) - src/daemon/workerRegistry.ts: restore from stub to worker dispatcher (remoteControl kind → runBridgeHeadless()) - src/commands/remoteControlServer/: new slash command /remote-control-server (alias /rcs) for managing the daemon from REPL - build.ts + scripts/dev.ts: enable DAEMON feature flag Both official CLI 2.1.92 and our codebase had the command registered in commands.ts but the directory and daemon implementation were missing. The bottom layer (runBridgeHeadless in bridgeMain.ts) was already complete.
📝 WalkthroughWalkthroughThis PR introduces daemon and remote control server functionality to the project. It adds a persistent daemon supervisor that manages worker processes, a new remote-control-server command for managing the daemon, worker registry for process lifecycle handling, and enables the DAEMON feature by default in build and dev configurations. Documentation is added describing the implementation path and key modules. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant RCS as RemoteControlServer<br/>Command
participant Supervisor as Daemon<br/>Supervisor Process
participant Worker as Remote Control<br/>Worker Process
participant Bridge as Bridge<br/>Headless
User->>RCS: Invoke remote-control-server
RCS->>RCS: Check prerequisites<br/>(bridge enabled, token)
alt Prerequisites Failed
RCS->>User: Display error message
else Prerequisites Passed
RCS->>Supervisor: Spawn daemon process<br/>(node daemon start)
Supervisor->>Supervisor: Parse CLI args & supervisor options
Supervisor->>Worker: Spawn worker child process
Worker->>Worker: Read env configuration
Worker->>Bridge: Initialize HeadlessBridge
Bridge->>Bridge: Start long-running loop
Worker-->>Supervisor: Emit stdout/stderr
Supervisor-->>RCS: Stream logs
RCS->>User: Show management dialog<br/>(status, logs, actions)
par Daemon Lifecycle
User->>RCS: Stop/Restart action
RCS->>Supervisor: Send SIGTERM
Supervisor->>Worker: Propagate SIGTERM
Worker->>Bridge: Cleanup connections
Worker-->>Supervisor: Process exit
Supervisor->>Supervisor: Exponential backoff restart<br/>(if restart action)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
DEV-LOG.md (1)
12-20: Add language identifier to fenced code block.The fenced code block showing the call chain is missing a language specifier. Consider adding
textorplaintextto satisfy markdown linting rules.📝 Proposed fix
-``` +```text /remote-control-server (slash command) → spawn: claude daemon start → daemonMain() (supervisor,管理 worker 生命周期) → spawn: claude --daemon-worker=remoteControl → runDaemonWorker('remoteControl') → runBridgeHeadless(opts, signal) ← 已有完整实现 → runBridgeLoop() → 接受远程会话</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@DEV-LOG.mdaround lines 12 - 20, The fenced code block in DEV-LOG.md lacks a
language specifier; update the block that documents the call chain (the block
containing runDaemonWorker('remoteControl'), runBridgeHeadless(opts, signal),
and runBridgeLoop()) to add a language tag such as "text" or "plaintext" after
the opening triple backticks so markdown linters accept it (e.g., change ``` to
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/remoteControlServer/remoteControlServer.tsx`:
- Around line 252-270: In stopDaemon(): you're nullifying daemonProcess before
the 10s timeout so the later check (daemonProcess && !daemonProcess.killed)
always short-circuits and SIGKILL is never sent; fix by using the captured pid
directly in the timeout to check/force-kill the process (call process.kill(pid,
0) and process.kill(pid, 'SIGKILL') as needed) or alternatively move the
daemonProcess = null assignment into the process exit handler so daemonProcess
remains valid until the process actually exits (update any references in
stopDaemon and the exit handler accordingly).
---
Nitpick comments:
In `@DEV-LOG.md`:
- Around line 12-20: The fenced code block in DEV-LOG.md lacks a language
specifier; update the block that documents the call chain (the block containing
runDaemonWorker('remoteControl'), runBridgeHeadless(opts, signal), and
runBridgeLoop()) to add a language tag such as "text" or "plaintext" after the
opening triple backticks so markdown linters accept it (e.g., change ``` to
```text).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f898b0d6-4036-4345-bbbc-d3df20de7cf6
📒 Files selected for processing (7)
DEV-LOG.mdbuild.tsscripts/dev.tssrc/commands/remoteControlServer/index.tssrc/commands/remoteControlServer/remoteControlServer.tsxsrc/daemon/main.tssrc/daemon/workerRegistry.ts
| function stopDaemon(): void { | ||
| if (daemonProcess && !daemonProcess.killed) { | ||
| daemonProcess.kill('SIGTERM'); | ||
| // Force kill after 10s grace | ||
| const pid = daemonProcess.pid; | ||
| setTimeout(() => { | ||
| try { | ||
| if (pid) process.kill(pid, 0); // Check if still alive | ||
| if (daemonProcess && !daemonProcess.killed) { | ||
| daemonProcess.kill('SIGKILL'); | ||
| } | ||
| } catch { | ||
| // Process already gone | ||
| } | ||
| }, 10_000); | ||
| } | ||
| daemonProcess = null; | ||
| daemonStatus = 'stopped'; | ||
| } |
There was a problem hiding this comment.
Race condition: SIGKILL will never be sent because daemonProcess is nullified before the timeout fires.
Setting daemonProcess = null at line 268 happens synchronously, but the timeout captures pid at line 256. When the timeout fires, the check at line 260 (daemonProcess && !daemonProcess.killed) will always short-circuit to false since daemonProcess is already null, so SIGKILL is never sent.
🐛 Proposed fix — use `pid` directly for force kill
function stopDaemon(): void {
if (daemonProcess && !daemonProcess.killed) {
daemonProcess.kill('SIGTERM');
// Force kill after 10s grace
const pid = daemonProcess.pid;
+ const proc = daemonProcess;
setTimeout(() => {
try {
if (pid) process.kill(pid, 0); // Check if still alive
- if (daemonProcess && !daemonProcess.killed) {
- daemonProcess.kill('SIGKILL');
+ if (pid && proc && !proc.killed) {
+ process.kill(pid, 'SIGKILL');
}
} catch {
// Process already gone
}
}, 10_000);
}
daemonProcess = null;
daemonStatus = 'stopped';
}Alternatively, move the null assignment inside the exit event handler so daemonProcess remains valid until the process actually exits.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/remoteControlServer/remoteControlServer.tsx` around lines 252 -
270, In stopDaemon(): you're nullifying daemonProcess before the 10s timeout so
the later check (daemonProcess && !daemonProcess.killed) always short-circuits
and SIGKILL is never sent; fix by using the captured pid directly in the timeout
to check/force-kill the process (call process.kill(pid, 0) and process.kill(pid,
'SIGKILL') as needed) or alternatively move the daemonProcess = null assignment
into the process exit handler so daemonProcess remains valid until the process
actually exits (update any references in stopDaemon and the exit handler
accordingly).
Upstream changes: - feat: add Grok (xAI) API adapter with custom model mapping (claude-code-best#152) - feat: restore daemon supervisor and remoteControlServer command (claude-code-best#170) - fix: prevent iTerm2 terminal response sequences from leaking into REPL input (claude-code-best#172) - Revert ink component abstraction (claude-code-best#158 → claude-code-best#175) - Various fixes and cleanups Conflict resolutions: - contributors.svg: kept ours - packages/@ant/ink/*: deleted (upstream reverted ink refactor) - src/utils/earlyInput.ts: kept ours (superior APC sequence handling) - src/main.tsx: accepted upstream (setAppCallbacks now built into App.tsx) - Theme: preserved our brand color customizations (BRAND_COLOR rgb(88,190,255)) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
src/daemon/main.tsfrom stub to full supervisor implementation (spawn/monitor workers, exponential backoff restart, graceful shutdown)src/daemon/workerRegistry.tsfrom stub to worker dispatcher (remoteControlkind →runBridgeHeadless())src/commands/remoteControlServer/— new/remote-control-server(alias/rcs) slash command for managing the daemon from REPLDAEMONfeature flag inbuild.tsandscripts/dev.tsBackground
commands.tsregistersremoteControlServer(dual gate:feature('DAEMON') && feature('BRIDGE_MODE')), but the directory was missing and daemon files were stubs. Official CLI 2.1.92 has the same gap. The bottom layer (runBridgeHeadless()inbridgeMain.ts) was already fully implemented — we reverse-engineered the middle layers by tracing the call chain.Call chain
vs
/remote-control/remote-control/remote-control-server(daemon)--capacityworktreemodeTest plan
bun run build— 490 files, successbun run dev daemon startstarts supervisor/remote-control-serverin REPL shows management dialogSummary by CodeRabbit