Skip to content

fix(core): hold session done notifications until background subagents finish - #59258

Closed
neriousy wants to merge 7 commits into
session-idle-hookfrom
session-settled
Closed

neriousy wants to merge 7 commits into
session-idle-hookfrom
session-settled

Conversation

@neriousy

@neriousy neriousy commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Issue for this PR

Stacked on #58508, which adds SessionBackground.outstanding. Merge that first; this PR then retargets to dev.

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Desktop and the TUI play the "done" sound and show "Response ready" on every session.execution.succeeded. That event only means one execution ended. A Session waiting on background subagents ends an execution when its turn ends, and again each time a subagent's result wakes it. So it beeps while the subagents are still working, and again for each one.

This adds an ephemeral session.settled event. SessionExecution publishes it right after succeeded when no background subagent of the Session or its descendants is still running. While one is running, nothing is published. When it finishes, its notice wakes the Session, that execution succeeds with nothing outstanding, and settled follows. Clients don't poll and nothing extra wakes up.

  • Desktop (shell/notifications/notification.tsx) and the TUI notifications plugin now notify "done" on session.settled instead of session.execution.succeeded.
  • Failures still notify right away on session.execution.failed. Failed and interrupted executions never settle.
  • Background shells don't hold it back. Background shells have no timeout by default, so a build can't be told apart from a dev server. Counting shells would mean a Session running bun dev never notifies. A Session waiting on a background build still gets a second notification after the build reports; that is the current behaviour.
  • It's ephemeral because it's derived from the in-memory job registry. Durable history and the unread state are unchanged; unread still follows the projected idle marker.
  • Regenerated the Promise client types (bun run generate).

Not included: packages/app/src/voice/call.ts listens for the deprecated session.idle, which nothing publishes. Voice probably wants every reply rather than settled, so I left it for a separate change.

How did you verify your code works?

  • packages/core/test/session-execution.test.ts:
    • With a background subagent and a background dev-server shell running, the execution succeeds without settled. After the subagent finishes and the Session runs again, settled follows succeeded, even though the shell is still running.
    • Failed and user-interrupted executions don't settle.
  • packages/tui/test/cli/cmd/tui/notifications.test.ts: a turn that ends while a subagent runs doesn't notify; the later settled execution notifies once. Existing tests now emit settled.
  • packages/app/test-browser/server-notification-state.test.ts: the "Response ready" alert now follows session.settled.
  • bun run check passes. The core runner, execution, subagent and bus routing suites pass (299 tests), along with the schema and client test suites.

Visual

A looping animation for the desktop app's "What's new" dialog. GitHub only shows its source.

SVG source
<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round">
  <rect x="16" y="22" width="92" height="28" rx="14" opacity="0.45"/>
  <line x1="32" y1="36" x2="76" y2="36" opacity="0.35"/>
  <circle cx="132" cy="36" r="6" fill="currentColor" fill-opacity="1"><animate attributeName="fill-opacity" values="0;0;1;1" keyTimes="0;0.2;0.25;1" dur="4s" repeatCount="indefinite"/></circle>
  <circle cx="154" cy="36" r="6" fill="currentColor" fill-opacity="1"><animate attributeName="fill-opacity" values="0;0;1;1" keyTimes="0;0.4;0.45;1" dur="4s" repeatCount="indefinite"/></circle>
  <circle cx="176" cy="36" r="6" fill="currentColor" fill-opacity="1"><animate attributeName="fill-opacity" values="0;0;1;1" keyTimes="0;0.6;0.65;1" dur="4s" repeatCount="indefinite"/></circle>
  <g transform="translate(208 34)">
    <g>
      <path d="M-7 5V-1a7 7 0 0 1 14 0v6l2 2h-18z"/>
      <path d="M-2 10a2 2 0 0 0 4 0"/>
      <animate attributeName="opacity" values="0.15;0.15;1;1" keyTimes="0;0.7;0.75;1" dur="4s" repeatCount="indefinite"/>
      <animateTransform attributeName="transform" type="rotate" values="0;0;-14;12;-8;0;0" keyTimes="0;0.75;0.79;0.83;0.87;0.9;1" dur="4s" repeatCount="indefinite"/>
    </g>
  </g>
</svg>

Screenshots / recordings

N/A, notification timing only.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@neriousy

Copy link
Copy Markdown
Member Author

Folded into #58508, which now holds the whole /goal stack (idle hook, settled notifications, /goal, TUI and desktop) in one PR.

@neriousy neriousy closed this Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant