fix(server): synthesize session status only from the main agent's turn - #1708
Conversation
A sub-agent runs its own turn on the shared session channel, and the broadcaster synthesized event.session.status_changed from every agent's turn.started/turn.ended. A foreground sub-agent finishing mid-turn thus emitted a bogus idle transition that clients read as 'turn finished' (notifications, sounds, unread dots, queued-message drain), while the real main-agent turn end was swallowed by dedup. Gate the status synthesis on MAIN_AGENT_ID, matching the existing main-only rule in InFlightTurnTracker.
🦋 Changeset detectedLatest commit: 3f9ff00 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55099ddd89
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .then(() => this.dispatch(state, wireEvent, volatile)) | ||
| .catch(() => {}); | ||
| if (event.type === 'turn.ended') { | ||
| if (agentId === MAIN_AGENT_ID && event.type === 'turn.ended') { |
There was a problem hiding this comment.
Preserve idle transition after subagent-only runs
If a client first subscribes or resyncs while only a background/sub-agent turn is active, ensureState still seeds lastStatus from ISessionActivity.status(), which reports running for any active agent. With this new main-agent guard, that sub-agent's later turn.ended is journaled but no status_changed(idle) is enqueued, so v1 clients that received a running snapshot can stay busy and keep subsequent prompts queued until some later main-agent status transition. Please make the initial/snapshot status use the same main-agent-only rule, or still clear a subagent-only running state without firing the premature main-turn completion path.
Useful? React with 👍 / 👎.
…agent Defensive follow-up to the main-only status synthesis: ensureState seeds lastStatus from ISessionActivity, which counts any agent's turn, so a session first activated while only a sub-agent was active would stay running for subscribers until the next main turn. A sub-agent's own lease and loop state are already released when its turn.ended is published, so a session-wide idle activity read cannot be a mid-main-turn foreground sub-agent — emit idle in that case only. Dedup keeps it a no-op everywhere else.
|
Addressed the P2 review on The mechanism in the review checks out (
The follow-up commit takes the review's second suggestion: a sub-agent's On the first suggestion (main-agent-only snapshot/REST status): REST intentionally reports running while detached tasks live, corrected on the wire at main-turn boundaries — that pre-existing divergence is orthogonal to this PR and would be a product-level change. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7918b45fea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const terminalStatus = | ||
| reason === 'cancelled' || reason === 'failed' || reason === 'blocked' ? 'aborted' : 'idle'; | ||
| this.enqueueStatusChanged(state, pendingStatus(state.activity?.status()) ?? terminalStatus); | ||
| } else if (event.type === 'turn.ended' && state.activity?.status() === 'idle') { |
There was a problem hiding this comment.
Preserve aborted status for failed sub-agent endings
When a session is first activated while only a sub-agent is running (the stale running seed case this branch handles), a sub-agent that ends with reason: 'cancelled' or reason: 'failed' now always emits status_changed(idle). That regresses the previous terminal-status mapping for those turn endings: kimi-web treats idle as a successful completion and will mark unread / notify / play the completion sound, while it suppresses those side effects for aborted (apps/kimi-web/src/composables/useKimiWebClient.ts:2528-2556, apps/kimi-web/src/composables/client/useNotification.ts:19-25). Use the turn-end reason here as well, or avoid emitting an idle completion status for non-main failed/cancelled turns.
Useful? React with 👍 / 👎.
This reverts 7918b45. The stale-seed scenario it guarded is effectively unreachable in production: sessions are activated at creation / first prompt and stay active for the process lifetime, and persisted detached tasks are reconciled as lost on restart, never resumed. The guard also raised a new behavioral question of its own (failed/cancelled sub-agent endings would emit a success-style idle). Keep the PR to the minimal main-only status synthesis; the theoretical corner self-corrects on the next REST status pull or main turn.
|
Follow-up on both P2 reviews: commit 3f9ff00 reverts the defensive idle emit (7918b45), so the PR is back to the minimal main-only status synthesis. Reasoning:
Net PR change: main-agent-only |
Related Issue
No linked issue — reported by a user via feedback; the problem is explained below.
Problem
On kimi-web (and any other client of the v1 WS stream), a sub-agent completing its task pushed a "turn finished" browser notification, played the completion sound, and lit the unread dot — while the main turn was still running.
Root cause: in the v2 engine a sub-agent is an agent within the same session and runs its own turn, publishing
turn.started/turn.endedon its own event bus.SessionEventBroadcaster.onAgentEventsynthesizedevent.session.status_changedfrom every agent's turn boundaries without checking the agent id. Sinceevent.session.*events fan out globally, a foreground sub-agent finishing mid-turn broadcast a bogusstatus_changed(idle); clients treat that as the authoritative turn-end signal (onSessionIdlein kimi-web: notification, sound, unread marker, queued-message drain). The real main-agent turn end was then swallowed by status dedup and never notified.What changed
turn.started/turn.ended→event.session.status_changedsynthesis onMAIN_AGENT_ID, matching the existing main-only rule inInFlightTurnTracker("Only main-agent activity is tracked"). Sub-agent turn frames themselves are still broadcast unchanged, so the swarm/task view keeps working.idle. Update two existing tests that encoded the old behavior.@moonshot-ai/kimi-codepatch (kap-server source is bundled into the CLI).Server-side fix at the single fan-out point, so every client (kimi-web, TUI, SDK) is corrected without consumer changes.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.