Repository navigation
Sonny tool host: a pause stops what Sonny's calls run, and starts none until work resumes - #675
Conversation
…t was not recorded The host dropped a call from its active map whatever came of handing in its result, so when the daemon did not take the result (a lost connection, a store that could not write), the next sweep found the call still waiting and ran it again: a second write, message or task. A call answered is now kept with its result until the daemon takes it or says the call wants none (answered already, out of time, its turn over), handed in again every two seconds, and never run again meanwhile. The host's own state is in memory: a host that restarts between running a call and recording it still runs it again, which the execution records of the next release cover. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
…e until work resumes A pause parks every turn and stops the daemon's own processes, but the tool host heard only of calls started and turns finished: an exec script ran on after a pause and could make further MCP calls, ones that need no approval among them. The host now hears the pause: what each call still runs is stopped, so a script makes no call after it (one already made may have had effects, which the daemon's daemon_paused result tells its agent), and no waiting call starts while it holds, also for a host that starts or connects again while paused. Resuming lets them start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
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: 3d96a46509
ℹ️ 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".
…and it in again A call that ended after a sweep, whose result was then lost, waited for the 30 s sweep. Its task now wakes the serving loop. The declared-tools row records the guarantee and its limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
…rk is paused The served connection's greeting could miss a resume the event connection never carried, holding every call. The host now starts held, takes the state from the event connection's greeting at each connect and from its events after. Brings in the outbox branch's review fixes; the pull-cord row records the change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1333cf9a9
ℹ️ 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".
A pause (or a turn's end) that stopped a call while the waiting calls were listed left that call on the listing with no job to say it ran, so the sweep started it again after the resume. The host counts its stops, and a sweep whose listing a stop came during admits nothing from it and looks again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
Every turn's end counted, so on a busy store a sweep could keep finding its listing stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
…, not this one Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ef543b786
ℹ️ 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".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
A sweep admitted none from a listing read across a pause only when the pause stopped a running call. With nothing running yet, a call named on an earlier page could start after the resume though the daemon had already answered it `daemon_paused`. Each pause now counts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
# Conflicts: # sonny/FEATURES.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c77857a51
ℹ️ 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".
| pub fn connected(&self, client: &Client) { | ||
| pause(&self.serving, !client.paused().is_null()); | ||
| self.nudge.notify_one(); |
There was a problem hiding this comment.
If the event-reading connection disconnects while the separate serving connection remains healthy, tool_host::serve leaves calls running throughout its reconnect delay. A pause during that interval is therefore not enforced; if work also resumes before reconnection, this greeting reports unpaused and never aborts those calls at all. Pause/resume notices are non-durable broadcasts (src/server/mod.rs:2032–2039), so replaying the saved cursor cannot recover the missed stop, and scripts can continue external effects after the daemon has finalized their answers as daemon_paused. Suspend admission and cancel running calls when the event connection is lost, rather than relying solely on its next greeting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real: the daemon drops a lagging follower's socket while the serving connection stays up. Fixed in bd81270. The host's event loop now calls Server::disconnected() when its stream ends or follow is refused. That holds the calls the way the start does: what they run stops, each is answered events_lost, and nothing starts until the next greeting says work goes on. The test losing_the_events_stops_what_calls_run_until_a_greeting_says_go_on covers it.
Generated by Claude Code
| Job::Running(job) => { | ||
| job.abort(); | ||
| false |
There was a problem hiding this comment.
When a hosted call exceeds the daemon's short answer hold (at most 250 ms), Turn::await_answer parks its turn through suspend_answer, and Service::complete removes it from self.active. The pause command cancels only active tasks, so it never runs suspend_halted for that parked turn: its tool remains executing with an unanswered answers row. Removing the host job here therefore causes a fresh sweep after resume to execute the same call again, potentially repeating external effects performed before cancellation. Unlike the previously fixed cross-pause listing race, this happens with a listing obtained entirely after resume; the generation check cannot prevent it. Record a terminal cancellation answer or have the daemon close parked hosted calls before forgetting their jobs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real: Command::Pause cancels only active turns, so a turn parked on a hosted call keeps its answers row open. Fixed in bd81270. A call the host stops is no longer forgotten. It becomes Job::Answered with daemon_paused and the reason, and is handed in like any result: a parked turn is told, and an active turn's (already answered by the daemon) is let go on call_already_answered. A job that finishes after it was stopped doesn't overwrite that answer. The test a_call_a_pause_stops_is_answered_so_it_does_not_run_again_after lists the call until it's answered, and checks it ran once.
Generated by Claude Code
…ops calls too A call the host stopped for a pause was forgotten, so a turn the daemon had parked on it (past the answer hold) listed it again after the resume and it ran twice. Now a stopped call keeps an answer saying why (`daemon_paused`) and is handed in like any result. Losing the connection that reads the events stops the calls too (`events_lost`) and holds until the next greeting, since a pause while it is down reaches no event. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
`a_view_that_opens_reads_where_the_windows_sit_now` sent a job that closes and opens windows, then read the places kept from before it. The worker thread could run that job and refresh the kept places first, so the test failed now and then (seen on #675 and #680). A job ahead of it now holds the worker until the read is done, as the naming test beside it does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE (cherry picked from commit f38b3c0)
|
The Generated by Claude Code |
`a_view_that_opens_reads_where_the_windows_sit_now` sent a job that closes and opens windows, then read the places kept from before it. The worker thread could run that job and refresh the kept places first, so the test failed now and then (seen on #675 and #680). A job ahead of it now holds the worker until the read is done, as the naming test beside it does. Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE Co-authored-by: Claude <noreply@anthropic.com>
Requested by George · project thread
Part of #669 (Wave 1, 0.8.1), item R1. Stacked on the no-rerun PR (base
claude/wave1-outbox), since both change the host's active map. I'll retarget it to main once that one merges. Sonny only, no protocol or schema change.The cause
A pause parks every turn, answers its unanswered calls
daemon_paused, and stops the daemon's own processes. Sonny's tool host only listened fortool_startedandturn_finished, and cancelled jobs only when a turn ended. So anexecscript kept running after a pause and could make more MCP calls, including ones that need no approval. A host that started while paused would also start pending work.The fix
Calls) now holds apausedflag next to the jobs, under one lock.paused, every running job is aborted. Dropping the answer stops the script and any MCP call it has in flight, so it makes no call after the pause. While the pause holds, the sweep starts nothing. Onresumed, the flag clears and the host sweeps again.client.paused()), so a host that starts or reconnects while paused starts nothing.daemon_pausedresult already tells the agent that ("the pause stopped this call before its result was recorded, so it may have had effects; nothing has run it again"), so the outcome is reported as unknown, not as not-run.Tests
served::a_pause_stops_what_calls_run_and_starts_none_until_work_resumes: a script that reads slowly and then writes is paused during the read, and the write never happens. While paused, a waiting call doesn't start; after resume it does. Fails on main (the write lands).Generated by Claude Code