Repository navigation
Daemon companions: commands the daemon keeps running for as long as it runs - #370
Conversation
The goal tool sets an agent, or a task it made, a goal: what done looks like and a shell command that exits 0 once it is. The lookout runs the check each time the agent's turn completes; while it fails, the agent gets a turn with its exit code and the end of its output, and its lead hears nothing. The goal ends once the check passes, the budget is spent, two turns it started call no tool, or the agent gives up with blocked, and the lead hears that last turn with how it ended. No judge, nothing in the daemon. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
- A check's output is read as it prints, keeping only its end. - A turn's goal is read fresh as the turn ends, so one a lead set or cleared meanwhile counts. - One check at a time per agent; a turn that ends while one runs is plain news. - A task is the caller's by its creator's id, not its name. - A turn waiting for its check keeps the replay cursor before it, so a restarted lookout runs the check again. - The check runs without the provider keys in ~/.agent/env. - The macOS skip is back on the trigger test, not on note_for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Main does not build since #344 renamed Places::plist to Places::job while #318's restamp still called the old name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
- A goal left by an agent deleted while the lookout was away is dropped when another agent with that name ends a turn. - A goal's next turn is not news while it runs, as its end is. - A goal cleared or set again while its check ran is plain news, even when that check passed. - Listing goals shows a lead its tasks' goals, whoever set them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
# Conflicts: # sonny/src/lookout/mod.rs
…e its check ran ends so Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
…lay runs again Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
…turn is not counted twice Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
The reason an agent gives up for, and a check it set itself, are its own text; the lead reads them in the turn or with the goal tool. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
… turn waits on one Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
…ves a later turn's news Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
…d a lookout that gives way mid-check Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
…-kxnbh9 # Conflicts: # sonny/src/served.rs
…-kxnbh9 # Conflicts: # sonny/src/lookout/mod.rs # sonny/src/lookout/updates.rs
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
…ched or not Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
… to tell Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
…t runs `agent companion add/rm/ls` keeps a command in the store that every daemon on it starts once it listens, restarts with backoff and stops at shutdown. Sonny's lookout and approver now run as companions, so they share the daemon's life cycle whether or not a window is open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21f6720eb5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn spawn(&self) -> std::result::Result<tokio::process::Child, String> { | ||
| std::fs::create_dir_all(self.log.parent().expect("in its folder")) | ||
| .map_err(|e| e.to_string())?; | ||
| let log = std::fs::OpenOptions::new() | ||
| .create(true) |
There was a problem hiding this comment.
Every companion start and restart calls this synchronous filesystem setup directly from Run::keep on the daemon's shared current-thread runtime. Slow filesystem operations therefore suspend all bots and socket handling; an existing FIFO at the log path can make the blocking open stall the entire daemon indefinitely. Perform directory creation and log opening through tokio::fs or a blocking worker before spawning the process. This violates the explicit requirement that file work run off the runtime thread.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8cb07d1. The log folder and file are now made and opened with tokio::fs, on the blocking pool, so a slow disk or a FIFO at the path holds up only that companion's task and not the runtime.
Generated by Claude Code
| if tokio::time::timeout(GRACE, child.wait()).await.is_err() { | ||
| // SAFETY: as above. | ||
| unsafe { libc::kill(group, libc::SIGKILL) }; |
There was a problem hiding this comment.
If a companion starts a descendant that ignores SIGTERM while its leader exits normally on SIGTERM, child.wait() completes immediately and this condition skips SIGKILL entirely. The descendant remains running after removal, replacement, or daemon shutdown, contrary to the process-group lifecycle promised here. Determine whether the group still has surviving processes and enforce the grace-period escalation independently of whether the leader has already been reaped.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8cb07d1. terminate now waits out the grace while any member of the group lives, whether or not the leader has been reaped, and then sends SIGKILL to the group. The new unit test has a leader that exits on SIGTERM and a child that ignores it, and the child is gone by the time the daemon has stopped.
Generated by Claude Code
| pub fn drop_one(&mut self, name: &str) -> bool { | ||
| self.kept | ||
| .remove(name) | ||
| .map(|kept| kept.stop.send(true)) | ||
| .is_some() |
There was a problem hiding this comment.
Removing a companion drops its JoinHandle, detaching termination from the manager. If the client then shuts down the daemon while that companion is still in its five-second termination grace period, Companions::stop cannot await it because it is no longer in kept; runtime teardown can cancel the task before group cleanup finishes. Removing and immediately re-adding the same name also bypasses the sequencing used by keep, allowing the old and new commands to overlap. Retain stopping tasks and await them during shutdown and before starting a replacement under the same name.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8cb07d1. A removed companion's task now waits in stopping. Shutdown awaits those tasks too, and adding the name again starts the new run only once the old one has stopped. The new unit test removes a companion and then stops the daemon.
Generated by Claude Code
| "--store".to_owned(), | ||
| path(&options.store), | ||
| "--socket".to_owned(), | ||
| path(&options.socket), |
There was a problem hiding this comment.
If the same store is later served on a different --socket, its persisted Sonny companions still receive the old socket through these saved arguments. Those explicit arguments override the fresh AGENT_SOCKET supplied by Companions, so the lookout repeatedly connects to the obsolete address and the approver fails to connect, leaving task notifications, Sonny tools, and automatic approvals unavailable until Sonny opens again and replaces the specs. Persist only the subcommand and let these helpers resolve their store and socket from the daemon-provided environment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8cb07d1, which is in bcb379b. Sonny now keeps only the subcommand (lookout, approver). Both helpers take their store and socket from the AGENT_STORE and AGENT_SOCKET the running daemon gives them, so a store served on another socket later still reaches its companions.
Generated by Claude Code
…, a removed one waited for, and Sonny's take store and socket from the daemon Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
…-kxnbh9 # Conflicts: # src/store/db.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcb379b183
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| status.started_ms = Some(epoch_ms()); | ||
| } | ||
| let exit = tokio::select! { | ||
| exit = child.wait() => exited(exit), |
There was a problem hiding this comment.
If a companion starts a background process and then exits or crashes, this branch reaps only the leader and restarts the command without terminating the remaining group. For example, sh -c 'sleep 600 & exit 7' leaves another sleep process behind on every restart. Clearing the PID also loses the information needed to stop those groups during removal or daemon shutdown. Preserve the group ID and clean up surviving descendants before restarting; the corrected SIGTERM shutdown path does not cover natural exits.
Useful? React with 👍 / 👎.
| let log = tokio::fs::OpenOptions::new() | ||
| .create(true) | ||
| .append(true) | ||
| .open(&self.log) | ||
| .await |
There was a problem hiding this comment.
If STORE.companions/NAME.log is a FIFO with no reader, the asynchronous open waits indefinitely. Although this no longer blocks the runtime thread, Run::keep awaits self.spawn() outside the stop selection, so removal cannot finish, replacements wait forever, and Companions::stop() hangs daemon shutdown. Reject nonregular log targets or open them nonblocking, and ensure pending setup responds to stop requests; merely cancelling the async filesystem future would still leave its blocking worker stuck.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 39328ab. The log is now opened with O_NONBLOCK, so a FIFO with no reader fails at once with ENXIO, and anything that isn't a regular file is refused as spawn_failed. A new unit test puts a FIFO at the log path and checks that the companion fails and stop() returns promptly.
Generated by Claude Code
| // What a companion sends is no client's activity. | ||
| let session = match &message { Inbound::Open(id, ..) | Inbound::Request(id, _) | Inbound::Closed(id) => *id }; | ||
| if !peers.get(&session).is_some_and(|pid| service.companions.runs_pid(*pid)) { | ||
| last_activity = std::time::Instant::now(); |
There was a problem hiding this comment.
If a companion keeps one connection open and sends requests more frequently than --idle-exit, excluding its requests from last_activity still does not let the daemon exit. The idle arm at line 1458 constructs a fresh full-duration sleep on every run-loop iteration, so each companion request cancels and restarts that timer before the idle check can run. A companion that polls stats therefore keeps an otherwise idle daemon alive indefinitely, contrary to the new companion-session behavior. Keep the timer across iterations or calculate its deadline from the last qualifying client activity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 39328ab. The idle arm now sleeps until last_activity + idle_exit, so loop wakes from a companion no longer push it back. A session also counts as a companion's when its peer is in the companion's process group, which covers what a companion's script runs. The new runtime test has a companion's child poll stats every 0.3 s on one connection, and the daemon still exits on --idle-exit 2s. I checked that the test fails without the fix.
Generated by Claude Code
|
Correction to my four review replies: the fixes are in 2983abd, not 8cb07d1. The PR head is bcb379b, which also brings in #365 and #361 and moves companions to schema 62. Generated by Claude Code |
…nion's whole group is no client, and a log that is no file fails at once Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39328ab4bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let group = unsafe { libc::getpgid(pid as i32) }; | ||
| self.kept.values().any(|kept| { | ||
| let leader = kept.status.lock().unwrap_or_else(|e| e.into_inner()).pid; | ||
| leader.is_some_and(|leader| leader == pid || leader as i32 == group) |
There was a problem hiding this comment.
The socket request path now calls runs_pid for every message from an established peer, adding a getpgid syscall and a scan that locks companion statuses. However, this commit adds no matched before/after results to docs/DAEMON_MEASUREMENTS.md; the existing measurements cover earlier changes. Record the required lifecycle, completion-burst, admission-burst, and footprint screen against main so the cost of this new shared-runtime hot-path work is checked before merging.
AGENTS.md reference: AGENTS.md:L67-L70
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f0e8a4b by taking the work off the hot path rather than measuring it. Whether a session belongs to a companion (runs_pid, with its getpgid) is now decided once, when the session opens. Each request then costs one HashSet lookup in place of main's unconditional last_activity write, and there is no syscall or lock per message. The Perf workflow runs the matched lifecycle, burst and footprint screen against base on this PR, so its result is the record for this change.
Generated by Claude Code
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGHCyGRHdT5WYDPKVFY
Requested by George · project thread
This is step 1 of the autonomy plan. Sonny is just a window, so whatever agents wait on now shares the daemon's life cycle instead of the window's. George picked a general, open API: the daemon can keep any command running and knows nothing of what that command is. It replaces #333.
What changes
agent companion add NAME [--cwd DIR] [--env K=V]... -- COMMAND [ARGS]...,agent companion rm NAMEandagent companion ls.companionstable, schema 61) and started now. Every later daemon on that store starts it once the daemon listens.changed: false. A different spec under the same name stops the old one and replaces it.rmreturnsremoved: falsewhen there was nothing to remove.ls, andstatsundercompanions, show each companion's spec,running,pid,started_ms,restartsandlast_exit.companionswithadd,removeor neither. A sandboxed bot's commands can't make it, since a companion runs as the daemon does.--envon top, andAGENT_STORE,AGENT_SOCKET,AGENT_BINandAGENT_COMPANION.STORE.companions/NAME.log.--idle-exit. A daemon on stdio runs no companions.sonny-lookout) and approver (approver-auto) as companions instead of starting them detached, so they keep running whether or not a window is open. A companion lookout takes the place of a detached lookout of any build.docs/CLI.md,docs/LOOKOUT.mdandsonny/README.mddescribe this.Tests
tests/test_companions.py:test_a_companion_lives_and_dies_with_the_daemon_and_comes_back_with_the_nexttest_a_companion_that_exits_starts_againtest_a_companion_s_session_keeps_no_daemon_from_going_idletest_a_bad_companion_is_refused_by_nametests/test_acp.pyis updated for the lookout running as a companion.cargo test --workspacepasses, and clippy-D warningsand fmt are clean.tests.run) passes. The one failure,test_delivery's interrupt test, is a timing flake: it passed 3 of 3 times on rerun.No screen changes.
Generated by Claude Code