fix(jetbrains): stop mode picker from cancelling running sessions - #13591
Conversation
Switching the chat mode picker wrote default_agent to the CLI's global config. The CLI disposes every instance it holds whenever that file changes, which cancels every running turn in every open worktree -- three unrelated sessions died mid-turn from a single mode switch, with no error shown because a server-initiated cancellation and a user Stop both report the same MessageAbortedError. The mode pick now stays client-side: it rides on PromptDto.agent per turn (as it already did) and is remembered in KiloPluginSettings so new sessions still open in the last-picked mode, matching how VS Code and the TUI already handle this. No CLI config write happens. Since the CLI can still legitimately cancel a turn on its own (a settings/provider change disposing instances), the plugin now tells those apart from a user Stop: an unrequested abort shows the reason and offers Retry instead of a silent "Stopped", raises a notification, and is captured in telemetry. The backend synthesizes a session.interrupted event naming the cause when disposal happens while a session is busy.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous Review Summary (commit a68cb8a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a68cb8a)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (23 files)
Reviewed by grok-4.6 · Input: 78.2K · Output: 13.6K · Cached: 530.6K Review guidance: REVIEW.md from base branch |
The activity badge for a cancelled turn never appeared. reportDisposal runs immediately before load(), load() calls activity.start(), and start() fully stopped first -- clearing errors, statuses, and the directory resolver. The chat event flow also replays nothing, so the restarted collector could not re-see the SessionInterrupted it had just missed. The badge branch was dead on the only path that emits the event. start() now detaches the collectors in place and keeps what they recorded; a real teardown still clears everything through stop(). The badge is recorded by a direct activity.interrupt() call ordered with the disposal that caused it, rather than racing a flow emission against the reload that swaps collectors, so the event branch is gone. Both new tests fail without their respective halves of this fix.
|
Addressed the review in 9601737. The bot's single suggestion was correct and worth the catch: the activity-badge branch I had added was dead on the only code path that emits the event. Two changes, both verified load-bearing by reverting each and watching the matching test fail:
Five new tests cover it (four unit in Also corrected the PR description: it now documents the badge behaviour and the caveat I only noticed while writing the test — the badge appears once the cancelled session reports idle, because |
Issue
Fixes #
No tracked issue — found while investigating a user report of three sessions in different worktrees dying mid-turn with no visible error, traced to a mode switch in an unrelated session tab.
Context
Picking a mode in the JetBrains chat prompt (
SessionController.selectAgent) wrotedefault_agentto the CLI's global config viaPATCH /global/config. The CLI treats any change to that file as a reason to dispose every instance it holds, which cancels every running turn in every open worktree — a UI preference in one session tab was silently killing work in progress everywhere else.Nothing told the user this happened: the CLI reports both a user-initiated Stop and a server-initiated cancellation as the same
MessageAbortedError, and the JetBrains UI treated every abort as a deliberate Stop — a muted "Stopped" line, no reason, no Retry, no telemetry.VS Code and the TUI never had this bug: both keep the mode pick entirely client-side and send it per-prompt, never writing it to the CLI's global config.
Implementation
The fix:
selectAgentno longer calls the CLI at all. The picked mode already travels with every prompt (PromptDto.agent); the only thing missing was persisting it locally for new sessions, which now lives inKiloPluginSettings(PropertiesComponent, IDE-local) askilo.session.agent. A new session seeds its mode from that remembered pick, falling back to the CLI'sdefault_agentif the remembered mode no longer exists. This mirrors VS Code'sagentSelectionsstore and the TUI's in-memoryagentStore.Deleted the now-dead RPC path end to end so it can't be reintroduced:
ConfigUpdateDto,KiloSessionRpcApi.updateConfig,KiloSessionRpcApiImpl.updateConfig,KiloSessionService.updateConfig,KiloBackendChatManager.updateConfig,KiloCliDataParser.buildConfigPartial.Visibility for the case that's still real: the CLI can legitimately cancel a turn on its own (a settings/provider/org change disposing instances). Since the abort itself can't say who caused it,
SessionControllernow tracks a localstopRequestedflag, set only on an explicit Stop or on the deliberate abort inside revert/redo, and reset on every new turn/send. An abort that arrives without that flag is promoted toSessionState.Error(with Retry available) instead of the silent "Stopped", and raises a notification andSession Errortelemetry.To let that error name its cause, the backend synthesizes a new
ChatEventDto.SessionInterrupted(sessionID, reason)into the same SSE-derived stream whenglobal.disposed/server.instance.disposedarrives while a session is busy (KiloBackendAppService.reportDisposal, waslogSessionDisposalRisk— previously only logged a warning). This event races the abort it explains (the CLI publishes the cancellation mid-disposal), so the frontend handles both arrival orders and relabels an already-visible cancellation when the reason turns up late.The same disposal also badges the affected rows in the session list, Agent Manager worktree list, and Agents-tab dot. That badge is recorded by a direct
activity.interrupt(...)call rather than off the event stream: the disposal reloads the app in the same breath, the reload restarts the activity collector, and the chat event flow replays nothing, so an emission racing that restart can be dropped.KiloBackendActivityManager.start()also had to stop clearing state on an in-place restart (new privatedetach()), or the reload would erase the badge the disposal had just recorded — thanks @kilo-code-bot for catching that the first version of this branch was dead on exactly that path. Note the badge only becomes visible once the cancelled session reports idle, sincekind()deliberately ranks live work above a past error so a resumed row keeps spinning.A reopened session with a historically-aborted tail still shows a plain "Stopped": after a reload there's no way to know who stopped it, and flagging every past Stop as a failure would be worse than the status quo.
Scope note: this PR is JetBrains-only, per explicit instruction. The CLI-side defect is still there —
PATCH /global/configdisposes every instance with no busy guard, and VS Code's own Settings-save paths (and JetBrains' Settings pages) can still trigger it. The plugin now explains the fallout instead of hiding it, but the guard itself belongs inpackages/opencode/src/server/routes/instance/httpapi/handlers/global.tsand is out of scope here.Screenshots / Video
N/A — no visual layout changed (existing error-card and footer components are reused with new copy); the new balloon uses the existing
KiloNotifications.error(...)chrome. Did not launch a sandboxed IDE to capture it; see "Blocked checks" below.How to Test
Manual/local verification
Ran by the agent, from
packages/kilo-jetbrains/:./gradlew typecheck— clean./gradlew :backend:test :frontend:test— all green, including new tests:SessionCancellationTest(10 cases: unrequested-abort → error not "Stopped", reason arriving before/after the abort, balloon content, telemetry, Retry availability, revert-abort still counts as requested, flag doesn't leak into the next turn, reopened session keeps a historical abort as "Stopped")ConfigSelectionTest— mode switch no longer calls the CLI, remembered mode seeds a new session ahead of the CLI default, CLI default wins when the remembered mode is goneKiloBackendChatManagerTest.interrupt emits one reason event per running sessionKiloBackendAppServiceTest— disposal while a session is busy names that session; disposal with nothing busy stays silent; disposal badges the cancelled session after the reload settles (drives real SSEglobal.disposedthroughMockCliServer)KiloBackendActivityManagerTest— interrupt badges the session, survives the reload that follows a disposal, is cleared by resumed work, and is cleared by a realstop()bun run script/check-md-table-padding.ts,bun run script/check-workflows.ts,bun run script/extract-source-links.tsfrom repo root — clean, no diffBoth halves of the badge fix were confirmed load-bearing by reverting each in turn and watching the matching test fail — restoring
stop()insidestart()breaksinterrupt badge survives the reload that follows a disposal, and dropping theactivity.interrupt(...)call breaksdisposal badges the cancelled session after the reload settles.Reviewer test steps
./gradlew runIdeSplitModefrompackages/kilo-jetbrains/) with two project windows on two different worktrees, or two chat tabs in Agent Manager pointed at different directories.default_agent/global config through Settings, or restart the CLI backend) and confirm the busy session's card shows an explanation with a Retry button and a balloon appears, instead of a plain "Stopped" line.Blocked checks and substitute verification
SessionCancellationTestsuite exercising the exactSessionControllerstate transitions (SessionState.Errorconstruction,notify(...)call,canRetry()) and theKiloBackendAppServiceTest/KiloBackendChatManagerTestsuites exercising the real SSE→event wiring againstMockCliServer.Checklist
Get in Touch