Skip to content

fix(vscode): stop moved active sessions - #11404

Merged
marius-kilocode merged 3 commits into
mainfrom
toothsome-mask
Jun 18, 2026
Merged

fix(vscode): stop moved active sessions#11404
marius-kilocode merged 3 commits into
mainfrom
toothsome-mask

Conversation

@marius-kilocode

@marius-kilocode marius-kilocode commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Agent Manager can change the directory associated with a session while that session is already running. The backend does not move the active runner when this happens: runners are isolated by directory, so a turn started under /repo remains owned by the /repo instance even after Agent Manager maps the same session ID to /repo/.kilo/worktrees/foo.

Stop was sending session.abort only to the session's current mapped directory. In the example above, it asked the worktree instance to abort a runner that still belonged to the repository instance. The abort endpoint treats an absent runner as a successful no-op, so the request appeared successful while the original response continued and the UI remained in its Stop state. Remote mode can make the timing more visible, but it is not the cause.

Regression history

PR #11152 contains two overlapping cancellation fixes:

  • Commit a09dafe61b first added directory-aware ownership tracking, aborted both the active owner and current mapped directory, and added moved-session regression coverage.
  • Commit b23d3dfd75 then added the separate ability to abort prompts during session startup. While replacing the same handleAbort and status-management path, it unintentionally restored single-directory cancellation and removed the moved-session coverage.

The final version of #11152 therefore retained its intended startup-cancellation behavior but lost the protection for already-running sessions that are reassigned between the local workspace and a worktree.

Durable behavior

This change combines both requirements rather than reverting #11152. An instance-scoped abort helper records directories that report a non-idle session and preserves the current owner before Agent Manager changes the session mapping. Stop asks every known owner plus the current mapped directory to abort, attempts all requests even if one fails, and returns the webview to Send only after all known-owner aborts succeed. Session deletion, provider disposal, idle events, and backend-instance disposal clear the corresponding ownership state.

The ownership and multi-directory orchestration live outside KiloProvider; the provider only forwards lifecycle signals and reconciles the successful Stop state.

@kilo-code-bot

kilo-code-bot Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (3 files)
  • packages/kilo-vscode/src/KiloProvider.ts
  • packages/kilo-vscode/src/kilo-provider/abort.ts
  • packages/kilo-vscode/tests/unit/abort.test.ts

Incremental review (commit 09a7e89f5b, refactor "extract session abort tracking"):

The change extracts the abort-tracking state and logic out of KiloProvider into a dedicated SessionAbort class (kilo-provider/abort.ts), replacing the loose activeSessionDirectories map plus preserveDirectory/updateActiveSessionDirectory/resolveAbortDirectories helpers with observe/preserve/stop/dispose/delete/clear methods. KiloProvider now holds a single private readonly aborts = new SessionAbort().

Behavioral equivalence verified across all paths:

  • handleAbort — not-tracked sessions still abort the fallback directory only (no idle post); tracked+success deletes the entry and posts idle; failures log and leave the entry active (no idle post), identical to the prior implementation.
  • server.instance.disposedaborts.dispose(dir) returns the sessions that became idle, and the caller sets sessionStatusMap to idle for exactly those, matching the old per-session loop.
  • session.statusaborts.observe(...) is a direct rename of updateActiveSessionDirectory, including the idle-prune and dedup-via-sameDirectory logic.
  • setSessionDirectory/clearSessionDirectoryaborts.preserve(...) replicates the old preserveDirectory guard (!status || idle || already-tracked).

Lifecycle/leak surface is unchanged or smaller: per-session pruning in handleDeleteSession (aborts.delete), self-pruning when a directory set empties, and full clear on dispose. Set/Map mutation-during-iteration only deletes the current entry, which is safe in JS. No new let/else/any/empty-catch; names follow the single-word guide. Tests exercise the real SessionAbort class against a minimal client stub rather than duplicating logic.

Previous Review Summaries (2 snapshots, latest commit d43df06)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit d43df06)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)
  • packages/kilo-vscode/src/KiloProvider.ts
  • packages/kilo-vscode/src/kilo-provider/abort.ts
  • packages/kilo-vscode/src/session-status.ts
  • packages/kilo-vscode/tests/unit/abort.test.ts
  • packages/kilo-vscode/tests/unit/kilo-provider-load-messages.test.ts

Incremental review (commit d43df066d8, refactor "minimize moved session abort fix"):

  • The refactor simplifies abort handling by dropping the revision tracking system (sessionStatusRevision / sessionStatusRevisions) and collapsing ActiveSessionDirectories from Map<string, Map<string, SessionStatus>> to Map<string, Set<string>> (directories only). This removes state rather than adding it, so the memory-leak surface shrinks. No leftover references to the removed resolveActiveSessionStatus / markStatus / statusChanged helpers remain in src/ or tests/.
  • activeSessionDirectories lifecycle is intact: pruned per-session in handleDeleteSession (KiloProvider.ts:1777), self-pruned when a session's directory set empties in updateActiveSessionDirectory (abort.ts:20), and cleared on dispose (KiloProvider.ts:3651). No new leak pattern.
  • handleAbort (KiloProvider.ts:2776) only posts an idle status when the session was previously tracked (known) and all aborts succeeded; on failure it leaves the webview status as-is (still busy), which is correct since the session remains active. server.instance.disposed (KiloProvider.ts:3225) updates sessionStatusMap for the Save-warning path but no longer proactively posts a sessionStatus message — this is a deliberate behavioral simplification relying on the server's subsequent session.status SSE events, consistent with the removed tests.
  • Changeset stop-moved-agent-sessions.md is present. Tests exercise the real KiloProvider internals and were updated to match the simplified model rather than duplicating logic into mocks.

Previous review (commit f35d9c5)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (6 files)
  • packages/kilo-vscode/src/KiloProvider.ts
  • packages/kilo-vscode/src/kilo-provider/abort.ts
  • packages/kilo-vscode/src/session-status.ts
  • packages/kilo-vscode/tests/unit/abort.test.ts
  • packages/kilo-vscode/tests/unit/kilo-provider-load-messages.test.ts
  • .changeset/stop-moved-agent-sessions.md

Notes:

  • Reviewed specifically for memory leaks per the custom instruction. The new activeSessionDirectories and sessionStatusRevisions maps are cleaned per-session in handleDeleteSession (KiloProvider.ts:1796-1797), cleared on dispose (KiloProvider.ts:3739-3740), and activeSessionDirectories self-prunes its session key once all directory entries go idle (updateActiveSessionDirectory, abort.ts:20). This matches the pre-existing sessionStatusMap lifecycle, so no new leak pattern is introduced.
  • The multi-directory status resolution in resolveActiveSessionStatus (abort.ts:33) relies on Map insertion order — updateActiveSessionDirectory delete+reinserts on each non-idle update so "last" equals most-recently-updated. Subtle but coherent; the revision guards (markStatus/statusChanged) correctly prevent stale HTTP status responses from overwriting newer SSE state.
  • Changeset is present, scoped patch, and written in user-facing imperative voice.
  • Tests exercise the real KiloProvider via internals rather than duplicating logic into mocks.

Reviewed by glm-5.2 · 574,796 tokens

Review guidance: REVIEW.md from base branch main

Comment thread packages/kilo-vscode/src/KiloProvider.ts Outdated
@marius-kilocode
marius-kilocode merged commit ec00d6c into main Jun 18, 2026
23 checks passed
@marius-kilocode
marius-kilocode deleted the toothsome-mask branch June 18, 2026 10:52
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
fix(vscode): stop moved active sessions
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.

2 participants