refactor(agent-core-v2): derive session busy from agent activity - #1751
Conversation
… web moon stuck after stop
The web moon occasionally stayed visible after a session's turn stopped.
Root cause chain: the session's activity status was a derived state machine
fed by registered bookings, which could wedge; the web then trusted it.
Redesign around a single source of truth:
- agent-core-v2: IAgentActivityView (Agent scope) is the only activity
model, folded purely from the agent's own event bus: { lifecycle, turn?,
lastTurn?, background[] }. background = live background tasks (any kind:
process/agent/question), seeded from IAgentTaskService and folded from
task.started/terminated. busy = turn != null || background.length > 0.
- Session level stores no activity state: the admission/drain registry
(admitWork/canAccept/hasBusyAgents/SessionCommand/closing/quiesce) is
deleted, the loop no longer reports to the session, agent creation has
no gate, and session.rejected is retired.
- fork is unconditional: a mid-work copy is crash-equivalent, which replay
already normalizes on every restore.
- kap-server derives the wire facts from the agents' activity: Session.busy
and event.session.work_changed aggregate any agent's turn/background;
last_turn_reason passes through the main agent's latest outcome.
SessionEventJournal now chains a follow-up flush when appends arrive
mid-flush, so the final events of a turn are not parked indefinitely.
- protocol/web: Session.busy + last_turn_reason replace the five-value
status enum; the web moon keys off the main agent's turn boundaries,
the sidebar spins while any agent is busy, and the terminated label
reads last_turn_reason.
# Conflicts: # apps/kimi-web/test/event-batcher.test.ts # packages/agent-core-v2/src/activity/activity.ts # packages/kap-server/src/transport/ws/v1/sessionEventBroadcaster.ts
… dead test stubs)
…and the aborted badge The new wire busy fact is approval-blind (turn != null || background), while the retired five-value status substituted awaiting_* for running/aborted while input was pending. Without the client-side adjustment the sidebar spinner kept spinning through approval/question waits and the aborted badge could show while a new turn was awaiting input: - isSessionEffectivelyRunning suppresses the spinner while the session has pending approvals/questions (mirrors awaiting_* not being 'running'). - The aborted badge (desktop + mobile) hides while attention is pending (mirrors awaiting_* superseding 'aborted'). - Snapshot seeding of turnActiveBySession now also requires the snapshot's busy fact, so a stale in-flight tracker entry (turn.ended lost on abrupt disposal) can never relight the moon.
Two symptoms, one root: after sending a prompt the moon spun but the Stop button stayed hidden (nothing covered the optimistic submit window), and after the main turn ended with only background tasks left the chat-area indicators kept spinning (the wire busy fact deliberately includes background tasks, while the retired turn-scoped status went idle). The activity computed (Stop button, composer/page-title spinners, send-vs-queue gating, skill activation) now derives 'running' from inFlight || turnActive — exactly the working moon's condition — instead of the wire busy fact. The sidebar spinner keeps the wire busy (turn or background task), unchanged.
…indicator behavior
…rn-end gate starvation - The chat moon, the sidebar spinner, and the Stop button now share ONE condition: prompt submitted to main turn ended (inFlight || turnActive). Background tasks and subagent turns no longer light any of them (the sidebar previously used the wire busy fact, which deliberately includes them). The aborted badge inherits the same quiet gate (plus the existing attention suppression). - agentEventProjector's turn.ended arm now emits turnActiveChanged FIRST: the onMainTurnEnd side effect gates on seq > lastSeqBySession, and the sibling events (messageUpdated/sessionUsageUpdated) advance that cursor, which starved the gate — the prompt-finish cleanup never ran, so inFlight stuck true and the moon stayed on whenever a turn ended with background work still running (no work_changed(busy:false) fallback in that case).
…ad of stacking a duplicate A retried step restarts its stream from offset 0, but the projector only unlinked the failed attempt's assistant bubble (currentAssistantMsgId = undefined) and let the retried step.started create a fresh one — the partial bubble stayed rendered next to the retry's full stream, so text and tool cards showed twice. Much more visible since the per-step LLM retry budget grew (#1740), which makes retries routine. turn.step.retrying now strips the abandoned bubble's streamed parts (text / thinking / toolUse) via messageUpdated and records it in retryReuseMsgId; the retried step.started refills that same bubble in place. turn.step. interrupted keeps the discard semantics. Verified live against dev:v2: multi-resync + retry sequences no longer duplicate content across bubbles.
…aster
Two concurrent activations of the same session (a WS subscribe racing a
REST snapshot / replay / cursor read) each built their own SessionState:
two agent bus subscriptions, two journal writers on one file, and — once a
re-subscribe landed the connection's target in BOTH states — every delta
fanned out twice. The blind twin state (created mid-turn, its tracker never
saw turn.started) annotated no offset, so the client-side alignDelta dedup
(identity by offset) appended those copies too: streamed text and tool
cards showed twice, interleaved per chunk ('AABBCC').
ensureState/ensureGlobalState now share an in-flight promise per session,
so activation is atomic and exactly one SessionState exists per session.
Includes a regression test driving six concurrent activations.
🦋 Changeset detectedLatest commit: 645fa2f 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: |
![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: e0793d832b
ℹ️ 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".
| if (snapshot.inFlightTurn !== null) return; | ||
| if (snapshot.status !== 'idle' && snapshot.status !== 'aborted') return; | ||
| if (snapshot.busy) return; |
There was a problem hiding this comment.
When a resync snapshot arrives after the main turn has ended but the session is still busy because background/subagent work remains, inFlightTurn is null yet this return skips finishPromptLocal; the same stale-local-state problem also occurs if the snapshot carries a stale inFlightTurn while busy is false because the previous line returns first. Since working still includes rawState.inFlightBySession, the moon and queued prompt drain can stay stuck until all aggregate work ends, which reintroduces the background-work case this refactor is trying to separate from main-turn liveness.
Useful? React with 👍 / 👎.
| if (reason === 'completed' || reason === 'cancelled' || reason === 'failed') { | ||
| this._lastTurnReasonBySession.set(sessionId, reason); | ||
| } |
There was a problem hiding this comment.
In the legacy engine path, turn.ended can carry reason: 'blocked' (for example when a user-prompt hook blocks the turn), and the old status code treated that as an aborted terminal state. After this change, the busy-false event.session.work_changed emitted for that turn keeps the previous/undefined last_turn_reason, so clients no longer get the cancelled/failed fact needed to render the terminal aborted/failed state; the v2 broadcaster maps non-completed/non-cancelled reasons to failed, so this path should do the same.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fccfc0e2e
ℹ️ 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".
| status: s.status, | ||
| busy: isSessionEffectivelyRunning(s), | ||
| time: formatTime(s.updatedAt), | ||
| busy: isMainTurnActive(s.id), |
There was a problem hiding this comment.
When the server reports event.session.work_changed with busy: true for background-only/subagent work, that value is stored on the AppSession as s.busy, but this sidebar view replaces it with isMainTurnActive(s.id). As a result the desktop/mobile session rows render no spinner, and the aborted badge can appear, while aggregate session work is still running. Keep the main-turn helper for the chat moon/Stop path, but pass the server aggregate s.busy into sidebar session rows.
Useful? React with 👍 / 👎.
| delete rawState.queuedBySession[sessionId]; | ||
| delete rawState.promptIdBySession[sessionId]; | ||
| delete rawState.sendingBySession[sessionId]; | ||
| delete rawState.inFlightBySession[sessionId]; |
There was a problem hiding this comment.
This cleanup now clears the optimistic inFlightBySession half but leaves the new authoritative turnActiveBySession entry behind. If a session is archived/not-found while its main turn is active, then restored or otherwise upserted again in the same tab, isMainTurnActive() will keep returning true from the stale map entry even though the restored session is idle. Delete rawState.turnActiveBySession[sessionId] here along with the other per-session liveness state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd21150277
ℹ️ 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".
| loop.cancel(undefined, abortError('Agent removed')); | ||
| await loop.settled(); |
There was a problem hiding this comment.
When an agent is removed while it has a running turn plus one or more queued turns, loop.cancel(undefined, ...) only aborts the active turn; queued turns are cancelled only when a specific turnId is passed. Those jobs remain in pendingTurns, and settled() waits for them to empty, so after the active turn aborts the loop can pump and execute queued prompts during session close/archive instead of shutting down. Please cancel the pending turn ids as well before awaiting settlement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97a0e09278
ℹ️ 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".
| status: s.status, | ||
| busy: isSessionEffectivelyRunning(s), | ||
| time: formatTime(s.updatedAt), | ||
| busy: isMainTurnActive(s.id), |
There was a problem hiding this comment.
For sessions that are already running when the page loads (or that are not among the retained WS subscriptions), turnActiveBySession and inFlightBySession have not been seeded yet, so isMainTurnActive(s.id) returns false even though the REST session row has s.busy === true. The sidebar then renders those running sessions as idle (and can even show the aborted tag from a previous turn) until the user opens the session and a snapshot seeds the local turn flag; keep the server busy fact as a fallback or expose a list-level main-turn flag.
Useful? React with 👍 / 👎.
| <Tooltip :text="t('workspace.awaitingAnswerTitle')"> | ||
| <Badge | ||
| v-if="!renaming && (questionCount > 0 || session.status === 'awaitingQuestion')" | ||
| v-if="!renaming && questionCount > 0" |
There was a problem hiding this comment.
When an unselected session is already awaiting a question after a refresh, questionCount is still 0 because pending questions are only populated by a session snapshot or session-scoped WS events; sessions that have not been opened (or whose subscription was evicted) do not have those counts. The removed session.status === 'awaitingQuestion' fallback was the only list-level signal, so the sidebar no longer shows the required-action badge for those sessions; the same issue applies to the approval badge below.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fc10fac84
ℹ️ 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".
| /** Agent teardown aborts the in-flight compaction via scope disposal — no | ||
| * registry: the service owns its own background work. */ | ||
| override dispose(): void { | ||
| if (this._compacting !== null && !this._compacting.abortController.signal.aborted) { | ||
| this._compacting.abortController.abort(); | ||
| } | ||
| super.dispose(); |
There was a problem hiding this comment.
When manual or auto full compaction runs while no turn is active, this now only aborts it on scope disposal and no longer registers it as background activity. Session busy is derived from IAgentActivityView.state().turn/background, and agentLifecycle.remove now waits only for loop.settled(), so status/snapshots can report idle during compaction and archive/close can dispose the agent before the compaction worker finishes applying the compacted context. Keep compaction visible to the activity view or model it as an agent task so busy/drain semantics still include it.
Useful? React with 👍 / 👎.
| }; | ||
| state.activityByAgent.set(agentId, next); | ||
| if ( | ||
| previous !== undefined && |
There was a problem hiding this comment.
When an agent is created after this broadcaster has activated a session, attachAgents only subscribes it and never inserts an idle fold into activityByAgent. If that agent's first activity update is background-only, such as a detached/restored task without a turn boundary, previous is undefined, so this guard suppresses enqueueWorkChanged; the map becomes busy but emittedBusy remains false, and the terminating update later dedupes as false→false. In that scenario the v1 WS stream/journal/replay never publishes the session busy transition for that background work.
Useful? React with 👍 / 👎.
| status: event.status, | ||
| currentPromptId: event.currentPromptId, | ||
| busy: event.busy, | ||
| mainTurnActive: event.mainTurnActive ?? s.mainTurnActive, |
There was a problem hiding this comment.
When a sessionWorkChanged event arrives with busy: false but omits mainTurnActive (the protocol marks it optional, and processEvent explicitly treats this as an authoritative quiet fallback), this keeps the prior s.mainTurnActive. The active-session turnActive computed and sidebar row both fall back to the session field, so a terminal partial/compat work_changed can clear turnActiveBySession while the stale session object immediately relights the moon/Stop/sidebar spinner.
Useful? React with 👍 / 👎.
| * the smooth typewriter/fade reveal; all other turns render statically. | ||
| */ | ||
| running?: boolean; | ||
| turnActive?: boolean; |
There was a problem hiding this comment.
After renaming ChatPane's liveness prop here, SideChatPanel.vue still renders <ChatPane :running="running" :sending="sending">, so those values no longer reach props.turnActive/props.working and side-chat assistant turns are never marked as streaming. In a BTW side chat while the agent is replying, the panel loses the streaming render behavior; update that caller or keep compatibility aliases for the old prop names.
Useful? React with 👍 / 👎.
# Conflicts: # packages/kap-server/src/routes/sessions.ts # packages/kap-server/src/services/snapshot/snapshotReader.ts # packages/kap-server/src/transport/ws/v1/sessionEventBroadcaster.ts
# Conflicts: # apps/kimi-web/test/agent-event-projector.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5672a9fbd7
ℹ️ 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".
| content: msg.content.map((c) => ({ ...c })), | ||
| status: 'pending', | ||
| }); | ||
| s.retryReuseMsgId = msgId; |
There was a problem hiding this comment.
When turn.step.retrying fires and the user aborts or the turn fails before the retried turn.step.started arrives, this stores the old assistant message id indefinitely. The turn.ended path clears currentAssistantMsgId but not retryReuseMsgId, so the next prompt's first step will reuse the previous turn's emptied bubble instead of creating a new assistant message, causing the new response to render under the wrong prompt. Clear this field on terminal/interrupted turn paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc539545d8
ℹ️ 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".
| } | ||
|
|
||
| private onTurnStarted(turnId: number, origin?: PromptOrigin): void { | ||
| this.turn = new MutableTurn(turnId, origin ?? USER_PROMPT_ORIGIN); |
There was a problem hiding this comment.
When a main turn starts after a prior cancelled/failed turn, lastTurn is left intact here. The v1 broadcaster folds agent.activity.updated.lastTurn into event.session.work_changed.last_turn_reason, so the new running turn (and REST session facts via resolveSessionFacts) keep reporting the previous outcome instead of clearing it until another terminal event arrives. Clear this.lastTurn in onTurnStarted before publishing the activity update.
Useful? React with 👍 / 👎.
| busy: event.busy, | ||
| mainTurnActive: event.mainTurnActive ?? (event.busy ? s.mainTurnActive : false), | ||
| pendingInteraction: event.pendingInteraction ?? s.pendingInteraction, | ||
| lastTurnReason: event.lastTurnReason ?? s.lastTurnReason, |
There was a problem hiding this comment.
For event.session.work_changed, an omitted last_turn_reason is how the new protocol represents “no current outcome” (for example, a main turn.started should clear a previous cancelled/failed outcome). This nullish fallback preserves the stale value instead, so once a session has a failed/cancelled reason the client cannot clear it from a work_changed event and can later show an outdated aborted state. Apply the event value authoritatively instead of falling back to the old session value.
Useful? React with 👍 / 👎.
| private readonly _abortedTurns = new Set<string>(); | ||
| /** MAIN-agent latest turn outcome per session — an orthogonal wire fact | ||
| * clients may present as an "aborted" tag (busy=false + cancelled/failed). */ | ||
| private readonly _lastTurnReasonBySession = new Map<string, 'completed' | 'cancelled' | 'failed'>(); |
There was a problem hiding this comment.
This new cache is populated on cancelled/failed turns but is never cleared when a later turn starts. In the legacy agent-core path, a session that previously failed will keep emitting/returning that last_turn_reason during the next active turn, unlike the old _abortedTurns state which was cleared on new work. Clear _lastTurnReasonBySession on turn.started (or prompt submission) so v1 and v2 expose the same fresh-turn semantics.
Useful? React with 👍 / 👎.
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
The engine retired its sessionActivity service in MoonshotAI#1751 (session busy is now derived from agent activity views), but the facade still called the deleted wire channel and imported the deleted engine module, breaking typecheck and every klient suite at import time. - drop the sessionActivity contract/registry entries and mirror the agentActivityView service instead (agent scope) - compose session status() client-side from the pending interaction lists and each agent's agentActivityView, keeping the retired service's precedence and typing SessionStatus locally in the facade - replace the deleted-channel call in the v2 smoke suite with a sessionInteractionService probe - fix the legacy image-file suite to wait with the harness's waitForSessionBusy
…1768) * feat(klient): contract-driven facade with http/ipc/memory transports - add zod-validated contract sections (global/session/agent) under src/contract and a facade exposing global.*, session(id).*, agent(id).* - select transport once at creation via subpath entries (@moonshot-ai/klient/http|ipc|memory); drop legacy channel/client/ httpChannel/wsChannel/wsKlient/proxy implementations - absorb packages/server-e2e into packages/klient test/e2e suites (dual-backend, legacy v1, v2 wire) and remove the server-e2e workspace - expose model registry and catalog services on kap-server v2 RPC surface * fix(klient): derive session status from agentActivityView The engine retired its sessionActivity service in #1751 (session busy is now derived from agent activity views), but the facade still called the deleted wire channel and imported the deleted engine module, breaking typecheck and every klient suite at import time. - drop the sessionActivity contract/registry entries and mirror the agentActivityView service instead (agent scope) - compose session status() client-side from the pending interaction lists and each agent's agentActivityView, keeping the retired service's precedence and typing SessionStatus locally in the facade - replace the deleted-channel call in the v2 smoke suite with a sessionInteractionService probe - fix the legacy image-file suite to wait with the harness's waitForSessionBusy
…oonshotAI#1768) * feat(klient): contract-driven facade with http/ipc/memory transports - add zod-validated contract sections (global/session/agent) under src/contract and a facade exposing global.*, session(id).*, agent(id).* - select transport once at creation via subpath entries (@moonshot-ai/klient/http|ipc|memory); drop legacy channel/client/ httpChannel/wsChannel/wsKlient/proxy implementations - absorb packages/server-e2e into packages/klient test/e2e suites (dual-backend, legacy v1, v2 wire) and remove the server-e2e workspace - expose model registry and catalog services on kap-server v2 RPC surface * fix(klient): derive session status from agentActivityView The engine retired its sessionActivity service in MoonshotAI#1751 (session busy is now derived from agent activity views), but the facade still called the deleted wire channel and imported the deleted engine module, breaking typecheck and every klient suite at import time. - drop the sessionActivity contract/registry entries and mirror the agentActivityView service instead (agent scope) - compose session status() client-side from the pending interaction lists and each agent's agentActivityView, keeping the retired service's precedence and typing SessionStatus locally in the facade - replace the deleted-channel call in the v2 smoke suite with a sessionInteractionService probe - fix the legacy image-file suite to wait with the harness's waitForSessionBusy
1 participant
Related Issue
No linked issue; this PR addresses the reported session-activity and duplicate-stream regressions described below.
Problem
Session activity was represented by a separate five-value state machine even though the underlying work already belonged to individual agents. The duplicated state could diverge at turn boundaries: background work could keep the chat moon alive after the main turn ended, while the Stop button, sidebar spinner, and aborted badge each interpreted activity differently.
Two independent lifecycle races could also duplicate streamed content. An LLM retry detached the abandoned branch without clearing its partial assistant bubble, and concurrent session activation could create multiple broadcaster states and event-bus subscriptions. Those leaked listeners advanced the same current tracker, so identical deltas received consecutive offsets and reached the client as valid
AABBCCcontent rather than replay duplicates.What changed
busyis now aggregated from agent activity (turn != null || background.length > 0).busyandlast_turn_reason, and added durableevent.session.work_changedupdates across snapshots, routes, projections, and Web reducers.@moonshot-ai/kimi-code.Validation
packages/protocol: 28 test files / 518 tests passed; typecheck passed.packages/agent-core: 222 test files passed (3,803 tests passed, expected failures/skips unchanged); typecheck passed.packages/kap-server: 56 test files / 651 tests passed; typecheck passed.packages/agent-core-v2: typecheck and domain-layer check passed; the three resource-sensitive test files passed in isolation (101 tests).apps/kimi-web: resync regression passed in isolation; typecheck, style baseline check, and production build passed.Checklist
gen-changesetsskill and added the required changeset.gen-docsskill; no independent documentation update is needed because this changes runtime status semantics without adding a user-facing command or configuration surface.