feat(daemon): zero daemon — worker pool + session supervisor over a local control socket#203
Merged
Conversation
…er a local control socket Adds internal/daemon + `zero daemon start|stop|status|run|attach`: a long-running local service that supervises a pool of headless `zero exec` workers and routes multiple agent sessions to them over an owner-only Unix-domain control socket. Additive — interactive and one-shot exec are unchanged. Reuses zero's building blocks: - internal/background: ConfigureChildProcessGroup (process-group setup) + TerminateProcess (cross-platform terminate) for worker kill/drain. The pool does its own os/exec and uses background only for those helpers. - internal/streamjson: the worker speaks stream-json on stdout; the daemon forwards those lines to clients. - internal/cli exec: a worker is `zero exec --output-format stream-json`. Pieces (reimplemented in Go from a reference daemon): - protocol.go: framed control codec — 4-byte BE length + 1 kind byte, 1 MiB cap, JSON control messages, version negotiation. - pool.go + backoff.go: bounded worker pool — at-least-once dispatch with exponential backoff+jitter retry capped by MaxAttempts, EXIT_TEMPFAIL(75)/ EXIT_PERMANENT(76) handling, graceful drain + kill-timeout. zero's exec is one-shot, so workers are on-demand (one session per worker = the lease), not warm long-lived slots (a persistent worker mode is a follow-up). - session.go: SessionManager — sessionID->worker routing, lease (one active request per worker), queue when the pool is full, per-session output buffer for attach + metrics. - server.go + lock.go + socket.go + paths.go: single-instance PID-file lock with stale-lock (dead-PID) recovery, status file, owner-only socket, accept loop; cleans up lock/socket/status on exit. - launcher.go: production worker launcher. client.go: control-socket client. Security model: - Control socket is LOCAL-ONLY (AF_UNIX loopback file, never a TCP port) and owner-only: socket 0600 + parent dir 0700 on POSIX (Windows relies on the per-user profile ACL; a dedicated pipe ACL is a follow-up). - Workers stay sandboxed: the launcher SCRUBS the sandbox re-entrancy markers (ZERO_SANDBOXED / ZERO_SANDBOX_BACKEND) from each worker's env so the worker re-establishes its OWN sandbox instead of inheriting a pass-through plan, while propagating the rest of the policy config/env. The daemon never bypasses the sandbox. - Control frames over the 1 MiB cap are rejected before allocation. Robustness: a crashed worker never takes down the daemon (backoff+jitter retry, MaxAttempts cap, EXIT_PERMANENT stops retry); drain gives a grace window then force-kills stragglers; a stale lock from an unclean exit is reclaimed. Out of scope (follow-up): remote session-linking; warm pre-spawned persistent workers; Windows named-pipe ACL. Tests: protocol round-trip + oversize/unknown-kind rejection; backoff bounds; pool spawn/retry-backoff/permanent/cap/queue/drain; session lease/route/queue/ attach; lock single-instance + stale recovery; launcher env-scrub (incl. an end-to-end check that the spawned worker process receives scrubbed env); server e2e start->run->stream-json->status->attach->stop with a fake worker; CLI usage/validation/not-running paths. Live-verified start/status/stop (0600 socket, 0700 dir, file cleanup). Platform files behind explicit //go:build tags.
Contributor
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR introduces a complete background daemon subsystem for the Changeszero daemon subsystem
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as zero daemon run
participant Server as Daemon Server
participant SessionMgr as SessionManager
participant Pool as Worker Pool
participant Worker as zero exec
User->>CLI: zero daemon run --session s1 --prompt "..."
CLI->>Server: Dial Unix socket
CLI->>Server: CtrlHello (version)
Server->>CLI: CtrlHelloOK (negotiated)
CLI->>Server: CtrlRun (session, cwd, prompt, args)
Server->>SessionMgr: Start(WorkerSpec)
SessionMgr->>Pool: Run(ctx, spec, sink)
Pool->>Worker: launch subprocess
Worker-->>Pool: stdout lines (stream-json)
Pool-->>SessionMgr: Line via Sink
SessionMgr-->>Server: live channel
Server->>CLI: CtrlData (line)
Worker-->>Pool: exit 0
Pool-->>SessionMgr: finish(done, 0, nil)
Server->>CLI: CtrlEnd
CLI->>User: print output
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
internal/daemon/session.go (1)
178-225: 💤 Low valueSessions accumulate indefinitely in the
sessionsmap.Finished sessions are never removed from
m.sessions. For a long-running daemon handling many sessions, this unbounded growth could become a memory concern. Consider adding a pruning mechanism (TTL, max-sessions cap, or explicit cleanup) if the daemon is expected to handle a high session volume over extended uptime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/daemon/session.go` around lines 178 - 225, The SessionManager accumulates finished sessions indefinitely in the sessions map because completed sessions are never removed. In the Start method, after sess.finish(code, err) is called in the background goroutine, add cleanup logic to remove the session from m.sessions map. Lock m.mu before removing the session from the map to avoid concurrent access issues. This ensures memory is properly released when sessions complete, preventing unbounded growth in long-running daemons.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/cli/app.go`:
- Around line 243-244: The writeHelp function does not include the newly added
"daemon" command in its help output, making it undiscoverable to users. Locate
the writeHelp function and add "daemon" to the list of commands that are
displayed when users run the help command, ensuring it is formatted consistently
with the other command entries already present in the help text.
In `@internal/cli/daemon.go`:
- Around line 160-163: The functions runDaemonStop, runDaemonStatus, and
runDaemonAttach currently ignore unexpected extra arguments passed by the user,
which can mask mistakes like typos. Add validation after the help check in each
function to verify that no extra arguments remain; if unexpected arguments are
present, return writeDaemonUsage with the appropriate exit code instead of
continuing execution. This should be applied at: runDaemonStop function (lines
160-163), runDaemonStatus function (lines 181-184), and runDaemonAttach function
(lines 273-282).
In `@internal/daemon/lock.go`:
- Around line 32-45: The `fmt.Fprintf` call in the `acquireLock` function
ignores its error return value, which means if writing the PID fails (e.g., due
to disk full), the lock file will be left incomplete or empty. This causes
`readPidFile` to fail parsing it, incorrectly treating the stale lock as
unclaimed and breaking the single-instance guarantee. Capture the error from
`fmt.Fprintf` instead of discarding it with an underscore, and if an error
occurs, close the file and remove the lock file before returning the error,
similar to how you handle the `f.Close()` error.
---
Nitpick comments:
In `@internal/daemon/session.go`:
- Around line 178-225: The SessionManager accumulates finished sessions
indefinitely in the sessions map because completed sessions are never removed.
In the Start method, after sess.finish(code, err) is called in the background
goroutine, add cleanup logic to remove the session from m.sessions map. Lock
m.mu before removing the session from the map to avoid concurrent access issues.
This ensures memory is properly released when sessions complete, preventing
unbounded growth in long-running daemons.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e00286c7-8bf7-47b1-b535-edf00bc0c69c
📒 Files selected for processing (25)
internal/cli/app.gointernal/cli/daemon.gointernal/cli/daemon_test.gointernal/daemon/backoff.gointernal/daemon/backoff_test.gointernal/daemon/client.gointernal/daemon/launcher.gointernal/daemon/launcher_test.gointernal/daemon/lock.gointernal/daemon/lock_posix.gointernal/daemon/lock_test.gointernal/daemon/lock_windows.gointernal/daemon/paths.gointernal/daemon/pool.gointernal/daemon/pool_test.gointernal/daemon/protocol.gointernal/daemon/protocol_test.gointernal/daemon/server.gointernal/daemon/server_test.gointernal/daemon/session.gointernal/daemon/session_test.gointernal/daemon/socket.gointernal/daemon/socket_posix.gointernal/daemon/socket_windows.gointernal/daemon/status.go
…e error, bounded sessions - cli/app.go: list `daemon` in `zero --help` so the command is discoverable. - cli/daemon: `stop`/`status` reject extra arguments; `attach` rejects more than one session id (fail fast on typos instead of silently ignoring tokens). - daemon/lock: check the PID fmt.Fprintf error — a partial/failed write would leave a malformed lock file that another process reads as stale and wrongly reclaims, breaking the single-instance guarantee; on write failure remove the file and return the error. - daemon/session: bound the registry with MaxSessions (default 256) — evict the oldest FINISHED sessions once past the cap so a long-running daemon doesn't accumulate sessions without limit; running/queued sessions are never evicted, so attach-after-finish for recent sessions is preserved. Tests: CLI extra-arg rejection; finished-over-cap eviction + running-never-evicted. Gates: gofmt, vet, build, test (incl -race), staticcheck (no new), govulncheck (0), deadcode (no new), GOOS=linux+windows cross-compile.
Collaborator
Author
|
Also addressed the session-retention nitpick in |
Vasanthdev2004
approved these changes
Jun 14, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Verdict: approve. I do not see merge blockers.
What I checked:
- daemon control plane: framed protocol, handshake, max-frame bound, single-instance lock, stale-lock recovery, status, and shutdown path
- worker/session lifecycle: bounded pool, retries/tempfail/permanent failure handling, attach history, bounded retained sessions, and drain/kill behavior
- sandbox invariant: worker launcher scrubs sandbox reentrancy markers while preserving policy env, so daemon workers re-enter sandbox instead of silently bypassing it
- D comparison: D's daemon path is mostly an in-process/IPC adapter. This PR's local socket supervisor is heavier, but appropriate for Go-native Zero. Compared with D, Zero is more protective around explicit locking, framed protocol, owner-local runtime files, and sandbox re-entry.
Validation:
git diff --checkgo test ./internal/daemon -count=1go test ./internal/cli -run TestDaemon -count=1go vet ./internal/daemon ./internal/cligo run ./cmd/zero-release buildgo run ./cmd/zero-release smoke
Non-blocking follow-up:
- Consider hardening
secureSocketParentto chmod or refuse an existing POSIX runtime directory that is not owner-only.MkdirAll(..., 0700)protects newly-created dirs, but does not repair pre-existing permissive dirs. The socket chmod and default per-user locations make this acceptable for this PR, so I would not block merge on it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
internal/daemon+ thezero daemon start|stop|status|run|attachsubcommands: a long-running local service that supervises a pool of headlesszero execworkers and routes multiple agent sessions to them over an owner-only local Unix-domain control socket. Fully additive — interactive and one-shot exec are unchanged.Reuses zero's building blocks
internal/backgroundConfigureChildProcessGroup(process-group setup) +TerminateProcess(cross-platform terminate) for worker kill/drain. The pool does its ownos/execand usesbackgroundonly for these helpers.internal/streamjsoninternal/cliexeczero exec --output-format stream-json.Pieces (reimplemented in Go from a reference daemon)
protocol.goprotocol.jspool.go+backoff.goworker-manager.jsMaxAttempts;EXIT_TEMPFAIL(75)/EXIT_PERMANENT(76); graceful drain + kill-timeoutsession.gosession-manager.js/lease.js/roster.jsserver.go+lock.go+socket.go+paths.gosupervisor.js/status.jslauncher.go/client.gospawner.js/attach.jsDesign note: zero's
execis one-shot (one prompt → run → exit), so workers are on-demand (one session per worker — the lease is inherent), not warm long-lived slots. Warm/pre-spawned persistent workers would need a persistent worker mode inexec→ follow-up.Security model
0600+ parent dir0700on POSIX (verified live:srw-------/drwx------). On Windows the per-user profile dir ACL applies; a dedicated pipe ACL is a follow-up.ZERO_SANDBOXED/ZERO_SANDBOX_BACKEND) from each worker's env so the worker re-establishes its own sandbox instead of inheriting a pass-through plan, while propagating the rest of the policy config/env. The daemon never bypasses the sandbox.Robustness
A crashed worker never takes down the daemon (backoff+jitter retry,
MaxAttemptscap,EXIT_PERMANENTstops retry). Drain gives a grace window then force-kills stragglers. A stale lock from an unclean exit is reclaimed (dead-PID check,x/syson Windows).Out of scope (follow-up)
remote session-linking; warm pre-spawned persistent workers; Windows named-pipe + DACL.
Tests
protocol round-trip + oversize/unknown-kind rejection; backoff bounds; pool spawn/retry-backoff/permanent/cap/queue/drain; session lease/route/queue/attach; lock single-instance + stale recovery; launcher env-scrub end-to-end (asserts the spawned worker process actually receives scrubbed env); server e2e start→run→stream-json→status→attach→stop with a fake worker; CLI usage/validation/not-running. Live-verified start/status/stop.
Gates
gofmt -l .empty ·go vet·go build·go test ./...(incl.-raceoninternal/daemon) ·staticcheckno new findings ·govulncheck0 ·deadcode -test=falseno new unreachable funcs · platform files behind explicit//go:buildtags · cross-compiledGOOS=linuxandGOOS=windows.Summary by CodeRabbit
Release Notes
New Features
daemonmode to the CLI withstart,stop,status,run, andattachsubcommands.Tests