feat(agent-manager): open notification Show in Agent Manager for its sessions - #13932
Conversation
…sessions Attention notifications always routed Show through the sidebar, which pulled Agent Manager sessions out of the panel where the user manages them. Sessions Agent Manager owns now activate their project, open the panel, and select their worktree and session tab, scrolling to the latest message. Sessions it does not own, and sessions whose worktree is gone, keep the sidebar behavior. Reveal orchestration lives in a VS Code-free helper, and openWorktreeFile path resolution moved to worktree-file-path.ts, to keep AgentManagerProvider under its maxLines cap.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (9 files)
Fix these issues in Kilo Cloud Reviewed by grok-4.6 · Input: 208.1K · Output: 34.1K · Cached: 2.6M Review guidance: REVIEW.md from base branch |
…cal sessions Show re-activated the project on every reveal, resetting PR, stats, and busy-session state even when the panel already showed that project. Activation now runs only when the project actually changes. Sessions in Agent Manager's Local tabs are owned by Agent Manager, so they now reveal there too. Ownership requires a session persisted in agent-manager.json: the panel also tracks live sidebar sessions listed from the project root, and those must keep the sidebar.
|
Both findings were valid — addressed in 74740a4. WARNING — always re-activates the current project ( Correct, and this was the more serious of the two. Activation (and the catalog push) now runs only when the target project differs from the active one: if (contexts.active()?.id !== target.projectId) {
contexts.activate(target.projectId)
deps.activate(target.context)
deps.projects()
}Covered by a new test asserting nothing is activated and only SUGGESTION — Local Agent Manager sessions fall back to the sidebar ( Agreed that these are owned by Agent Manager, so they now reveal there. The reveal message's One caveat worth calling out, because it makes the naive version of this fix unsafe: The worktree path is unchanged and still accepts live-but-unpersisted sessions, since a session in a worktree directory belongs to Agent Manager either way. Verification: |
|
@marius-kilocode Here you have the PR specifically for the agent manager navigation using VS Code notifications. Maybe it should have been part of that larger PR after all, didn't seem to be too complicated to get it right. But I am not a frequent user of the Agent Manager UI, so I leave it up to you to decide whether this is good or not. |
|
Very cool, I will give it a spin soon @sylwester-liljegren |
|
The change looks correct, I wrote down some thoughts on how we would be able to iterate further on native notification actions: #14010 Thanks |
|
Glad that you liked it, @marius-kilocode ! |
Issue
Fixes #13922
Context
Attention notifications (added in #13771) always opened their Show action through the sidebar. When the notification came from an Agent Manager session, that pulled the session out of the panel the user is actually working in and into the sidebar. The sidebar is only the right destination for sessions Agent Manager does not own.
Implementation
AttentionService'sshowcallback now asks Agent Manager first.AgentManagerProvider.revealSession()returnsfalsewhen Agent Manager does not own the session, and the existing sidebar path runs unchanged.Ownership is resolved host-side from
getSessionDirectories(). The session's directory is matched back to its owning project context and worktree, and the reveal is refused unless the worktree still exists and still matches that directory. That makes the stale cases (worktree removed, session moved back to Local, directory no longer owned) fall back to the sidebar instead of selecting the wrong thing.Ordering matters in the host: the project is activated and the project catalog is published before the reveal message is posted. The webview rejects messages for a project other than its applied one, so without the catalog push the reveal could be dropped when the notification came from a non-active project.
The webview side reuses
focusManagedSession, which already selects the worktree, clears review/terminal state, and selects the session tab. It gained ascrollToBottomflag so the reveal path passesselectSession(id, { scrollToBottom: true })— the same one-shot requestMessageListconsumes for the sidebar in #13771, so a later switch back still restores the saved scroll position.Two notes for reviewers:
reveal-session.tsrather than the provider becauseAgentManagerProvider.tsis at its enforcedmaxLinescap.openWorktreeFilemoved unchanged intoworktree-file-path.ts. This is the only refactor in the PR and is behavior-preserving; it is covered by new tests, since that symlink-traversal logic previously had none.Screenshots / Video
agent_manager_navigation_with_notifications.1.mp4
How to Test
Manual/local verification
bun run extension:isolatedand confirmed a worktree agent that asks a question raises a notification whose Show focuses Agent Manager, selects that worktree and session tab, and scrolls to the latest messageReviewer test steps
Blocked checks and substitute verification
bun run test:unit(full extension suite) could not complete in the available time budget; substitute verification was the targeted suitesagent-manager-reveal-session,worktree-file-path,agent-manager-arch,attention,agent-manager-focus,agent-manager-focus-panel,agent-manager-provider-lifecycle,agent-manager-session-restore,agent-manager-selection-actions,project-message-ownership,project-local-navigation,session-scroll-bottom, andpresence-registration-contract, all passingbun run check-kilocode-changecannot run in this Windows PowerShell environment (the script is shell-specific); verified instead by searching the package forkilocode_change, which returns no matches — and markers are not required inpackages/kilo-vscode/src/agent-manager/__tests__/AgentManagerProvider.spec.tsfails (1 pass / 10 fail), but it fails identically on a clean checkout ofmain; verified by stashing this branch's changes and re-running, so it is pre-existing and unrelatedAll checks above were executed by the agent.
bun run typecheck,bun run lint,bun run knip, andbun run formatpass.Checklist
Get in Touch
sylwester-liljegren