feat(agent-manager): key diff review by selection with per-session scope (#12709)

This commit is contained in:
Marius
2026-07-30 18:32:54 +02:00
committed by GitHub
parent 166d04e846
commit 5c140b12cf
18 changed files with 314 additions and 158 deletions
@@ -0,0 +1,5 @@
---
"kilo-code": minor
---
Make the Agent Manager diff review follow the sidebar selection instead of a single session. Switching session tabs inside a worktree no longer refetches the Branch, Staged, and Unstaged scopes, the Session scope now swaps to the active session's changes on tab switch, and the Local tab gains the Session scope so sessions running in the workspace can be reviewed on their own. The Session scope shows a notice when snapshots are disabled instead of a blank list, worktrees without an open session now still show their branch diff, and the Apply dialog lists the worktree's changes again.
@@ -781,7 +781,7 @@ export class AgentManagerProvider implements Disposable {
return null
}
if (m.type === "agentManager.requestWorktreeDiffFile") {
void this.diffs.requestFile(composeDiffId(m.sessionId, normalizeScope(m.scope)), m.file)
void this.diffs.requestFile(composeDiffId(m.sessionId, normalizeScope(m.scope), m.diffSessionId), m.file)
return null
}
if (m.type === "agentManager.applyWorktreeDiff") {
@@ -793,7 +793,7 @@ export class AgentManagerProvider implements Disposable {
return null
}
if (m.type === "agentManager.startDiffWatch") {
this.diffs.start(composeDiffId(m.sessionId, normalizeScope(m.scope)))
this.diffs.start(composeDiffId(m.sessionId, normalizeScope(m.scope), m.diffSessionId))
return null
}
if (m.type === "agentManager.stopDiffWatch") {
@@ -1577,18 +1577,19 @@ export class AgentManagerProvider implements Disposable {
}
/** Open a file from a worktree or local session in the VS Code editor.
* Absolute paths (Unix `/…` or Windows `C:\…`) are opened directly.
* Relative paths are resolved against the session's worktree directory
* (or repo root for local sessions) with symlink-traversal protection. */
private openWorktreeFile(sessionId: string, filePath: string, line?: number, column?: number): void {
* Absolute paths are opened directly; relative paths resolve against the
* context's worktree directory (repo root for local) with symlink-traversal
* protection. The id may be a worktree id, session id, or `local`. */
private openWorktreeFile(id: string, filePath: string, line?: number, column?: number): void {
if (isAbsolutePath(filePath)) {
this.host.openFile(filePath, line, column)
return
}
const state = this.getStateManager()
if (!state) return
const session = state.getSession(sessionId)
const base = session?.worktreeId ? state.getWorktree(session.worktreeId)?.path : this.getRoot()
const worktree = state.getWorktree(id)
const session = worktree ? undefined : state.getSession(id)
const base = worktree?.path ?? (session?.worktreeId ? state.getWorktree(session.worktreeId)?.path : this.getRoot())
if (!base) return
// Resolve real paths to prevent symlink traversal and normalize for
// consistent comparison on both Unix and Windows.
@@ -5,16 +5,16 @@ import type { ManagedSession } from "./WorktreeStateManager"
* Determine whether diff polling should stop when a worktree is being removed.
*
* Returns true when the worktree being deleted is currently the diff target
* (either by directory path or because one of its orphaned sessions is the
* active diff session).
* (either by directory path or because the diff context is the worktree
* itself or one of its orphaned sessions).
*/
export function shouldStopDiffPolling(
worktreePath: string,
orphaned: ManagedSession[],
diffTarget: { directory: string } | undefined,
diffSessionId: string | undefined,
diffCtx: string | undefined,
): boolean {
if (diffTarget && normalizePath(diffTarget.directory) === normalizePath(worktreePath)) return true
if (diffSessionId && orphaned.some((s) => s.id === diffSessionId)) return true
if (diffCtx && orphaned.some((s) => s.worktreeId === diffCtx || s.id === diffCtx)) return true
return false
}
@@ -1,14 +1,18 @@
/**
* Composite diff-source keying for Agent Manager.
*
* Agent Manager keys diff sources by *context* (a session id, or the `local`
* Agent Manager keys diff sources by *context* (a worktree id, or the `local`
* workspace pseudo-context) while the standalone Changes viewer keys by
* *scope* (branch / staged / unstaged / session). To expose scopes in Agent
* Manager we compose the two into a single id the SourceController can build.
* The context is the sidebar selection, so it stays stable when the user
* switches between session tabs of the same worktree; only the Session scope
* follows the active session, carried inside the id.
*
* ctx = "local" | "<sessionId>"
* ctx = "local" | "<worktreeId>"
* scope = "branch" | "staged" | "unstaged" | "session"
* id = `${ctx}#${scope}`
* id = `${ctx}#${scope}` (git scopes)
* id = `${ctx}#session:<sid>` (session scope, sid = active session id)
*
* `ctx#branch` is the default and reproduces the pre-scope behavior exactly.
*/
@@ -18,8 +22,10 @@ export type DiffScope = "branch" | "staged" | "unstaged" | "session"
export const DEFAULT_DIFF_SCOPE: DiffScope = "branch"
const SEP = "#"
const SESSION_TOKEN = "session:"
export function composeDiffId(ctx: string, scope: DiffScope): string {
export function composeDiffId(ctx: string, scope: DiffScope, sessionId?: string): string {
if (scope === "session" && sessionId) return `${ctx}${SEP}${SESSION_TOKEN}${sessionId}`
return `${ctx}${SEP}${scope}`
}
@@ -28,11 +34,13 @@ export function composeDiffId(ctx: string, scope: DiffScope): string {
* id (no separator) by assuming the default branch scope, which keeps the
* pre-scope messages working unchanged.
*/
export function parseDiffId(id: string): { ctx: string; scope: DiffScope } {
export function parseDiffId(id: string): { ctx: string; scope: DiffScope; sessionId?: string } {
const idx = id.lastIndexOf(SEP)
if (idx === -1) return { ctx: id, scope: DEFAULT_DIFF_SCOPE }
const scope = id.slice(idx + SEP.length)
if (isDiffScope(scope)) return { ctx: id.slice(0, idx), scope }
const token = id.slice(idx + SEP.length)
const ctx = id.slice(0, idx)
if (token.startsWith(SESSION_TOKEN)) return { ctx, scope: "session", sessionId: token.slice(SESSION_TOKEN.length) }
if (isDiffScope(token)) return { ctx, scope: token }
return { ctx: id, scope: DEFAULT_DIFF_SCOPE }
}
@@ -46,12 +54,13 @@ export function normalizeScope(value: unknown): DiffScope {
/**
* Map a scope to the underlying standalone-viewer source id the catalog knows
* how to build. `branch` maps to the workspace source; `session` is handled
* separately because it needs the session id embedded in the source id.
* how to build. `branch` maps to the workspace source; `session` needs the
* active session id embedded in the source id (the context id is a worktree
* or `local`, not a session).
*/
export function scopeToSourceId(scope: DiffScope, ctx: string): string {
export function scopeToSourceId(scope: DiffScope, ctx: string, sessionId?: string): string {
if (scope === "staged") return "staged"
if (scope === "unstaged") return "unstaged"
if (scope === "session") return `session:${ctx}`
if (scope === "session") return `session:${sessionId ?? ctx}`
return "workspace"
}
@@ -311,6 +311,13 @@ interface WorktreeDiffLoadingMessage {
loading: boolean
}
/** Source-level notice for a diff context (e.g. snapshots disabled). */
interface WorktreeDiffNoticeMessage {
type: "agentManager.worktreeDiffNotice"
sessionId: string
notice?: string
}
interface WorktreeDiffMessage {
type: "agentManager.worktreeDiff"
sessionId: string
@@ -386,6 +393,7 @@ export type AgentManagerOutMessage =
| RepoInfoMessage
| ApplyWorktreeDiffResultMessage
| WorktreeDiffLoadingMessage
| WorktreeDiffNoticeMessage
| WorktreeDiffMessage
| WorktreeDiffFileMessage
| RevertWorktreeFileResultMessage
@@ -665,12 +673,16 @@ interface RequestWorktreeDiffFileIn {
sessionId: string
file: string
scope?: string
/** Active session for the session scope (ctx alone is a worktree/local id). */
diffSessionId?: string
}
interface StartDiffWatchIn {
type: "agentManager.startDiffWatch"
sessionId: string
scope?: string
/** Active session for the session scope (ctx alone is a worktree/local id). */
diffSessionId?: string
}
interface StopDiffWatchIn {
@@ -50,6 +50,11 @@ export class WorktreeDiffController {
sessionId: source.descriptor.id,
loading,
}),
notice: (source, notice) => ({
type: "agentManager.worktreeDiffNotice",
sessionId: source.descriptor.id,
notice,
}),
diffs: (source, diffs) => ({
type: "agentManager.worktreeDiff",
sessionId: source.descriptor.id,
@@ -81,8 +86,8 @@ export class WorktreeDiffController {
}
public shouldStopForWorktree(path: string, sessions: ManagedSession[]): boolean {
// Pass the parsed context id, not the composite id, so the orphaned-session
// check matches real session ids.
// The parsed context id is a worktree id (or `local`), so the
// orphaned-session check matches sessions of the deleted worktree.
const current = this.controller.currentId
const ctxId = current ? parseDiffId(current).ctx : undefined
return shouldStopDiffPolling(path, sessions, this.target, ctxId)
@@ -225,6 +230,9 @@ export class WorktreeDiffController {
const { ctx } = parseDiffId(id)
const resolved = await this.resolve(ctx)
this.target = resolved ? { sessionId: id, ...resolved } : undefined
// Clear any stale source notice up front; sources only push a notice when
// one is active, so a swap away from a noticing source must reset it.
this.ctx.post({ type: "agentManager.worktreeDiffNotice", sessionId: id, notice: undefined })
this.controller.setContext({
workspaceRoot: this.ctx.getRoot(),
dir: resolved?.directory,
@@ -250,21 +258,11 @@ export class WorktreeDiffController {
return undefined
}
const session = state.getSession(ctxId)
if (!session) {
this.ctx.log(
`resolveDiffTarget: session ${ctxId} not found in state (${state.getSessions().length} total sessions)`,
)
return undefined
}
if (!session.worktreeId) {
this.ctx.log(`resolveDiffTarget: session ${ctxId} has no worktreeId (local session)`)
return undefined
}
const worktree = state.getWorktree(session.worktreeId)
// The context is the worktree itself (the sidebar selection), not one of
// its sessions — resolution survives session churn inside the worktree.
const worktree = state.getWorktree(ctxId)
if (!worktree) {
this.ctx.log(`resolveDiffTarget: worktree ${session.worktreeId} not found for session ${ctxId}`)
this.ctx.log(`resolveDiffTarget: worktree ${ctxId} not found`)
return undefined
}
const base = this.baseOverrides.get(ctxId) ?? remoteRef(worktree)
@@ -287,13 +285,14 @@ export class WorktreeDiffController {
/**
* Build the active source for a composite id by delegating to the catalog.
* The composite id (ctx#scope) is preserved as the descriptor id so the
* webview keys diff data by context+scope. Context resolution (dir/base)
* already happened in activate() and is carried by the PanelContext.
* The composite id (`ctx#scope`, or `ctx#session:<sid>` for the session
* scope) is preserved as the descriptor id so the webview keys diff data by
* context+scope. Context resolution (dir/base) already happened in
* activate() and is carried by the PanelContext.
*/
private source(id: string, panelCtx: PanelContext): DiffSource {
const { ctx, scope } = parseDiffId(id)
const built = this.ctx.catalog.build(scopeToSourceId(scope, ctx), panelCtx)
const { ctx, scope, sessionId } = parseDiffId(id)
const built = this.ctx.catalog.build(scopeToSourceId(scope, ctx, sessionId), panelCtx)
return {
...built,
descriptor: { ...built.descriptor, id },
@@ -0,0 +1,32 @@
import { describe, it, expect } from "bun:test"
import { composeDiffId, parseDiffId, scopeDescriptors } from "../../webview-ui/agent-manager/diff-scope-state"
describe("agent-manager webview diff scope descriptors", () => {
it("offers the three git scopes without an active session", () => {
const descriptors = scopeDescriptors("wt_1")
expect(descriptors.map((d) => d.type)).toEqual(["workspace", "staged", "unstaged"])
expect(descriptors.map((d) => d.id)).toEqual(["wt_1#branch", "wt_1#staged", "wt_1#unstaged"])
})
it("adds the session scope with the active session embedded", () => {
const descriptors = scopeDescriptors("wt_1", "ses_abc")
expect(descriptors.map((d) => d.type)).toEqual(["workspace", "staged", "unstaged", "session"])
const session = descriptors[3]!
expect(session.id).toBe("wt_1#session:ses_abc")
expect(session.group).toBe("Session")
expect(session.capabilities.revert).toBe(false)
})
it("embeds the active local session for the local context", () => {
const descriptors = scopeDescriptors("local", "ses_abc")
expect(descriptors[3]!.id).toBe("local#session:ses_abc")
})
it("round-trips the session descriptor id", () => {
expect(parseDiffId(composeDiffId("wt_1", "session", "ses_abc"))).toEqual({
ctx: "wt_1",
scope: "session",
sessionId: "ses_abc",
})
})
})
@@ -11,19 +11,30 @@ import {
describe("diff-scope composite ids", () => {
it("round-trips context and scope", () => {
expect(parseDiffId(composeDiffId("local", "branch"))).toEqual({ ctx: "local", scope: "branch" })
expect(parseDiffId(composeDiffId("ses_abc", "staged"))).toEqual({ ctx: "ses_abc", scope: "staged" })
expect(parseDiffId(composeDiffId("ses_abc", "unstaged"))).toEqual({ ctx: "ses_abc", scope: "unstaged" })
expect(parseDiffId(composeDiffId("ses_abc", "session"))).toEqual({ ctx: "ses_abc", scope: "session" })
expect(parseDiffId(composeDiffId("wt_abc", "staged"))).toEqual({ ctx: "wt_abc", scope: "staged" })
expect(parseDiffId(composeDiffId("wt_abc", "unstaged"))).toEqual({ ctx: "wt_abc", scope: "unstaged" })
expect(parseDiffId(composeDiffId("wt_abc", "session"))).toEqual({ ctx: "wt_abc", scope: "session" })
})
it("parses session ids containing no separator as default branch scope", () => {
expect(parseDiffId("ses_abc")).toEqual({ ctx: "ses_abc", scope: DEFAULT_DIFF_SCOPE })
it("embeds the active session id in the session scope", () => {
const id = composeDiffId("wt_abc", "session", "ses_xyz")
expect(id).toBe("wt_abc#session:ses_xyz")
expect(parseDiffId(id)).toEqual({ ctx: "wt_abc", scope: "session", sessionId: "ses_xyz" })
expect(parseDiffId(composeDiffId("local", "session", "ses_xyz"))).toEqual({
ctx: "local",
scope: "session",
sessionId: "ses_xyz",
})
})
it("parses context ids containing no separator as default branch scope", () => {
expect(parseDiffId("wt_abc")).toEqual({ ctx: "wt_abc", scope: DEFAULT_DIFF_SCOPE })
})
it("treats an unknown trailing segment as part of the context, not a scope", () => {
// A session id that happens to contain '#' but not a valid scope keeps the
// A context id that happens to contain '#' but not a valid scope keeps the
// full id as context and falls back to branch.
expect(parseDiffId("ses_a#bogus")).toEqual({ ctx: "ses_a#bogus", scope: DEFAULT_DIFF_SCOPE })
expect(parseDiffId("wt_a#bogus")).toEqual({ ctx: "wt_a#bogus", scope: DEFAULT_DIFF_SCOPE })
})
it("isDiffScope guards the closed enum", () => {
@@ -31,6 +42,7 @@ describe("diff-scope composite ids", () => {
expect(isDiffScope("staged")).toBe(true)
expect(isDiffScope("unstaged")).toBe(true)
expect(isDiffScope("session")).toBe(true)
expect(isDiffScope("session:ses_xyz")).toBe(false)
expect(isDiffScope("turn")).toBe(false)
expect(isDiffScope("")).toBe(false)
})
@@ -43,10 +55,15 @@ describe("diff-scope composite ids", () => {
})
it("maps scopes to catalog source ids", () => {
expect(scopeToSourceId("branch", "ses_abc")).toBe("workspace")
expect(scopeToSourceId("staged", "ses_abc")).toBe("staged")
expect(scopeToSourceId("unstaged", "ses_abc")).toBe("unstaged")
expect(scopeToSourceId("session", "ses_abc")).toBe("session:ses_abc")
expect(scopeToSourceId("branch", "wt_abc")).toBe("workspace")
expect(scopeToSourceId("staged", "wt_abc")).toBe("staged")
expect(scopeToSourceId("unstaged", "wt_abc")).toBe("unstaged")
expect(scopeToSourceId("session", "wt_abc", "ses_xyz")).toBe("session:ses_xyz")
expect(scopeToSourceId("session", "local", "ses_xyz")).toBe("session:ses_xyz")
expect(scopeToSourceId("branch", "local")).toBe("workspace")
})
it("falls back to the context id for a session scope without a session id", () => {
expect(scopeToSourceId("session", "ses_abc")).toBe("session:ses_abc")
})
})
@@ -9,6 +9,7 @@ import type { WorktreeStateManager } from "../../src/agent-manager/WorktreeState
// Records every PanelContext handed to catalog.build so tests can assert which
// base branch the active source was (re)built with. The controller, scope
// resolution, and SourceController lifecycle under test are all real.
// Contexts are worktree ids (the sidebar selection), not session ids.
function make(onFetch?: (n: number) => Promise<void>) {
const builds: { id: string; ctx: PanelContext }[] = []
let fetches = 0
@@ -57,18 +58,18 @@ async function waitFor(cond: () => boolean): Promise<void> {
describe("WorktreeDiffController.setBase", () => {
it("rebuilds the active source against the overridden base branch", async () => {
const { controller, builds } = make()
controller.start("s1#branch")
controller.start("w1#branch")
await waitFor(() => builds.length === 1)
expect(builds[0]!.ctx.dir).toBe("/wt")
expect(builds[0]!.ctx.baseBranch).toBe("origin/main")
await controller.setBase("s1#branch", "feature-x")
await controller.setBase("w1#branch", "feature-x")
expect(builds.length).toBe(2)
expect(builds[1]!.ctx.dir).toBe("/wt")
expect(builds[1]!.ctx.baseBranch).toBe("feature-x")
// Clearing the override falls back to the recorded parent ref.
await controller.setBase("s1#branch", undefined)
await controller.setBase("w1#branch", undefined)
expect(builds.length).toBe(3)
expect(builds[2]!.ctx.baseBranch).toBe("origin/main")
@@ -78,11 +79,11 @@ describe("WorktreeDiffController.setBase", () => {
it("stores the override without rebuilding when the context isn't active", async () => {
const { controller, builds } = make()
await controller.setBase("s1#branch", "feature-x")
await controller.setBase("w1#branch", "feature-x")
expect(builds.length).toBe(0)
// The next activation of that context resolves the stored override.
controller.start("s1#branch")
controller.start("w1#branch")
await waitFor(() => builds.length === 1)
expect(builds[0]!.ctx.baseBranch).toBe("feature-x")
@@ -99,10 +100,10 @@ describe("WorktreeDiffController.setBase", () => {
if (n === 1) await gate
})
controller.start("s1#branch")
controller.start("w1#branch")
await waitFor(() => builds.length === 1)
const change = controller.setBase("s1#branch", "feature-x")
const change = controller.setBase("w1#branch", "feature-x")
release()
await change
expect(builds.length).toBe(2)
@@ -110,7 +111,7 @@ describe("WorktreeDiffController.setBase", () => {
// Polling survives: start() early-returns for an id that is already
// watched. A downgraded one-shot panel would re-activate and rebuild here.
controller.start("s1#branch")
controller.start("w1#branch")
await tick()
expect(builds.length).toBe(2)
@@ -25,6 +25,7 @@ import type {
AgentManagerWorktreeDiffMessage,
AgentManagerWorktreeDiffFileMessage,
AgentManagerWorktreeDiffLoadingMessage,
AgentManagerWorktreeDiffNoticeMessage,
AgentManagerDiffBranchesMessage,
AgentManagerApplyWorktreeDiffResultMessage,
AgentManagerWorktreeStatsMessage,
@@ -139,7 +140,7 @@ import { DiffPanel } from "./DiffPanel"
import { createRevertFile } from "./revert-file"
import { FullScreenDiffView } from "../diff-viewer/FullScreenDiffView"
import { createApplyToLocal } from "./apply-to-local"
import { createWorktreeDiffs } from "./worktree-diffs"
import { createWorktreeDiffs, wireDiffId } from "./worktree-diffs"
import type { ReviewComment } from "../diff-viewer/review-comments"
import { clearReviewComposer, createReviewComposer } from "../diff-viewer/review-annotations"
import type { SidebarSearchMenuRef } from "./SidebarSearchMenu"
@@ -304,6 +305,7 @@ const AgentManagerContent: Component = () => {
const diffDatas = diffs.diffDatas
const diffLoading = diffs.diffLoading
const setDiffLoading = diffs.setDiffLoading
const diffNotices = diffs.diffNotices
// The diff and terminal panels each remember their own width: a diff
// benefits from half the window, a terminal only needs about a third.
const TERMINAL_MIN_WIDTH = 360
@@ -419,15 +421,6 @@ const AgentManagerContent: Component = () => {
setReviewCommentsByContext((prev) => ({ ...prev, [sel]: comments }))
}
const resolveWorktreeSessionId = (worktreeId: string) => {
const id = session.currentSessionID()
if (id) {
const current = managedSessions().find((entry) => entry.id === id)
if (current?.worktreeId === worktreeId) return id
}
return managedSessions().find((entry) => entry.worktreeId === worktreeId)?.id
}
const apply = createApplyToLocal({
vscode,
dialog,
@@ -437,7 +430,6 @@ const AgentManagerContent: Component = () => {
worktrees,
diffDatas,
diffLoading,
resolveWorktreeSessionId,
track: metrics.track,
})
const openApplyDialog = apply.openApplyDialog
@@ -1408,6 +1400,10 @@ const AgentManagerContent: Component = () => {
diffs.onWorktreeDiffLoading(msg as AgentManagerWorktreeDiffLoadingMessage)
}
if (msg.type === "agentManager.worktreeDiffNotice") {
diffs.onWorktreeDiffNotice(msg as AgentManagerWorktreeDiffNoticeMessage)
}
if (msg.type === "agentManager.diffBranches") {
review.onBranches(msg as AgentManagerDiffBranchesMessage)
}
@@ -1470,28 +1466,33 @@ const AgentManagerContent: Component = () => {
}
})
const selectedDiffSessionId = () => {
const sel = selection()
if (sel === LOCAL) return LOCAL
if (!sel) return undefined
// Diff context = sidebar selection (worktree id or LOCAL), stable across
// session tab switches inside the context so the git scopes don't refetch.
const diffCtx = createMemo(() => selection() ?? undefined)
// Active session within the diff context. The Session scope follows it, so
// switching session tabs swaps only the session diff.
const activeDiffSession = createMemo(() => {
const sel = selection()
if (!sel) return undefined
const current = session.currentSessionID()
if (sel === LOCAL) {
if (current && localSessionIDs().includes(current) && !isPending(current)) return current
return localSessionIDs().find((id) => !isPending(id))
}
if (current) {
const item = managedSessions().find((entry) => entry.id === current)
if (item?.worktreeId === sel) return current
}
return managedSessions().find((entry) => entry.worktreeId === sel)?.id
}
const currentDiffSessionId = createMemo(selectedDiffSessionId)
})
// Diff scope + base branch state, shared by the side panel and review tab.
const review = createDiffReviewScope({
ctx: currentDiffSessionId,
ctx: diffCtx,
session: activeDiffSession,
panelOpen: diffOpen,
reviewActive,
local: LOCAL,
vscode,
})
// The composite id (ctx#scope) the extension keys diff data by.
@@ -1516,21 +1517,15 @@ const AgentManagerContent: Component = () => {
/>
)
// Start/stop diff watch when panel opens/closes, review tab opens, scope
// changes, or session changes.
// Start/stop diff watch when the panel opens/closes, the review tab opens,
// or the composite id (context, scope, active session) changes.
createEffect(() => {
const panel = diffOpen()
const active = reviewActive()
const scope = review.scope()
const id = review.id()
if (panel || active) {
const id = currentDiffSessionId()
if (id) {
vscode.postMessage({ type: "agentManager.startDiffWatch", sessionId: id, scope })
return
}
vscode.postMessage({ type: "agentManager.stopDiffWatch" })
setDiffLoading(false)
if ((panel || active) && id) {
vscode.postMessage({ type: "agentManager.startDiffWatch", ...wireDiffId(id) })
return
}
@@ -1583,6 +1578,14 @@ const AgentManagerContent: Component = () => {
const diffSessionKey = createMemo(() => diffScopeId() ?? "")
// Source-level notice for the active composite id (e.g. snapshots disabled
// for the Session scope), shown as a banner instead of the empty state.
const diffNotice = createMemo(() => {
const key = diffScopeId()
if (!key) return undefined
return diffNotices()[key]
})
const setSharedDiffStyle = (style: "unified" | "split") => {
if (reviewDiffStyle() === style) return
setReviewDiffStyle(style)
@@ -1597,7 +1600,7 @@ const AgentManagerContent: Component = () => {
const diffFileLoadingForCurrent = createMemo(() => diffs.diffFileLoadingFor(diffScopeId))
const revertCtl = createRevertFile(diffScopeId, currentDiffSessionId, () => review.scope(), vscode, showToast, t)
const revertCtl = createRevertFile(diffScopeId, diffCtx, () => review.scope(), vscode, showToast, t)
const handleConfigureSetupScript = () => {
vscode.postMessage({ type: "agentManager.configureSetupScript" })
@@ -2476,8 +2479,9 @@ const AgentManagerContent: Component = () => {
diffs={reviewDiffs()}
loading={diffLoading()}
loadingFiles={diffFileLoadingForCurrent()}
sessionId={currentDiffSessionId()}
sessionId={activeDiffSession()}
sessionKey={diffSessionKey()}
notice={diffNotice()}
lead={diffScopeControls(true)}
canRevert={scopeCapabilities(review.scope()).revert}
diffStyle={reviewDiffStyle()}
@@ -2496,10 +2500,9 @@ const AgentManagerContent: Component = () => {
}
onRequestDiff={requestDiffFile}
onOpenFile={(file, line) => {
const id = currentDiffSessionId()
const id = diffCtx()
if (id)
vscode.postMessage({ type: "agentManager.openFile", sessionId: id, filePath: file, line })
else if (selection() === LOCAL) vscode.postMessage({ type: "openFile", filePath: file, line })
}}
onRevertFile={metrics.use("revert_file", "side_review", revertCtl.revert)}
revertingFiles={revertCtl.reverting()}
@@ -2526,8 +2529,9 @@ const AgentManagerContent: Component = () => {
diffs={reviewDiffs()}
loading={diffLoading()}
loadingFiles={diffFileLoadingForCurrent()}
sessionId={currentDiffSessionId()}
sessionId={activeDiffSession()}
sessionKey={diffSessionKey()}
notice={diffNotice()}
lead={diffScopeControls(false)}
canRevert={scopeCapabilities(review.scope()).revert}
canComment={scopeCapabilities(review.scope()).comments}
@@ -2542,9 +2546,8 @@ const AgentManagerContent: Component = () => {
onMarkdownRenderChange={markdown.update}
onRequestDiff={requestDiffFile}
onOpenFile={(file, line) => {
const id = currentDiffSessionId()
const id = diffCtx()
if (id) vscode.postMessage({ type: "agentManager.openFile", sessionId: id, filePath: file, line })
else if (selection() === LOCAL) vscode.postMessage({ type: "openFile", filePath: file, line })
}}
onRevertFile={metrics.use("revert_file", "fullscreen_review", revertCtl.revert)}
revertingFiles={revertCtl.reverting()}
@@ -64,12 +64,19 @@ import { createDiffRequests } from "../diff-viewer/diff-requests"
// --- Data model ---
/** Well-known diff source notices → i18n keys (mirrors the standalone viewer). */
const DIFF_NOTICE_KEYS: Record<string, string> = {
"snapshots-disabled": "diffViewer.notice.snapshotsDisabled",
}
interface DiffPanelProps {
diffs: WorktreeFileDiff[]
loading: boolean
loadingFiles?: Set<string>
sessionId?: string
sessionKey?: string
/** Well-known source notice kind (e.g. "snapshots-disabled"), shown as a banner. */
notice?: string
diffStyle?: "unified" | "split"
onDiffStyleChange?: (style: "unified" | "split") => void
markdownRender?: boolean
@@ -94,6 +101,11 @@ interface DiffPanelProps {
export const DiffPanel: Component<DiffPanelProps> = (props) => {
const { t } = useLanguage()
const noticeText = () => {
const n = props.notice
if (!n) return ""
return t(DIFF_NOTICE_KEYS[n] ?? n)
}
const vscode = useVSCode()
const server = useServer()
const provider = useProvider()
@@ -537,6 +549,15 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
</div>
</div>
<Show when={noticeText()}>
<div class="diff-viewer-notice" role="status">
<span class="diff-viewer-notice-icon">
<Icon name="warning" size="small" />
</span>
<span class="diff-viewer-notice-text">{noticeText()}</span>
</div>
</Show>
<Show when={props.loading && props.diffs.length === 0}>
<div class="am-diff-loading">
<Spinner />
@@ -544,7 +565,7 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
</div>
</Show>
<Show when={!props.loading && props.diffs.length === 0}>
<Show when={!props.loading && props.diffs.length === 0 && !noticeText()}>
<div class="am-diff-empty">
<span>{t("session.review.noChanges")}</span>
</div>
@@ -14,6 +14,7 @@ import { createEffect, createMemo, createSignal, on, type Accessor } from "solid
import { showToast } from "@kilocode/kilo-ui/toast"
import { groupApplyConflicts } from "./apply-conflicts"
import { ApplyDialog } from "./ApplyDialog"
import { composeDiffId } from "./diff-scope-state"
import type { tracker } from "./telemetry"
import type { useDialog } from "@kilocode/kilo-ui/context/dialog"
import type { useLanguage } from "../src/context/language"
@@ -37,7 +38,6 @@ interface ApplyToLocalOptions {
worktrees: Accessor<{ id: string }[]>
diffDatas: Accessor<Record<string, WorktreeFileDiff[]>>
diffLoading: Accessor<boolean>
resolveWorktreeSessionId: (worktreeId: string) => string | undefined
/** Telemetry: metrics.track(name, surface, data). */
track: ReturnType<typeof tracker>["track"]
}
@@ -62,19 +62,18 @@ export function createApplyToLocal(opts: ApplyToLocalOptions) {
return state.status === "checking" || state.status === "applying"
})
const applyTargetSessionId = createMemo(() => {
// Apply diffs come from the branch-scoped diff data of the target worktree
// (keyed by `worktreeId#branch`, matching the review surfaces).
const applyDiffKey = createMemo(() => {
const target = applyTarget()
if (!target) return undefined
return opts.resolveWorktreeSessionId(target)
return composeDiffId(target, "branch")
})
const applyDiffs = createMemo(() => {
const target = applyTarget()
if (!target) return [] as WorktreeFileDiff[]
const data = diffDatas()
const current = applyTargetSessionId()
if (current && data[current]) return data[current]!
return [] as WorktreeFileDiff[]
const key = applyDiffKey()
if (!key) return [] as WorktreeFileDiff[]
return diffDatas()[key] ?? ([] as WorktreeFileDiff[])
})
const applyStateForTarget = createMemo(() => {
@@ -178,8 +177,7 @@ export function createApplyToLocal(opts: ApplyToLocalOptions) {
setApplyTarget(sel)
setApplySelectionTouched(false)
setApplySelectedFiles([])
const sid = opts.resolveWorktreeSessionId(sel)
if (sid) vscode.postMessage({ type: "agentManager.requestWorktreeDiff", sessionId: sid })
vscode.postMessage({ type: "agentManager.requestWorktreeDiff", sessionId: sel })
setApplySelectedFiles(applyDiffs().map((diff) => diff.file))
@@ -9,28 +9,34 @@
import { createEffect, createMemo, createSignal, type Accessor } from "solid-js"
import type { BranchInfo } from "../src/types/messages"
import { createDiffScope, isDiffScope, scopeDescriptors, type DiffScope } from "./diff-scope-state"
import { composeDiffId, createDiffScope, isDiffScope, scopeDescriptors } from "./diff-scope-state"
interface VsCode {
postMessage(msg: unknown): void
}
export interface DiffReviewScopeOptions {
/** Current diff context (worktree session id or the LOCAL pseudo-id). */
/** Current diff context (worktree id or the LOCAL pseudo-id). */
ctx: Accessor<string | undefined>
/** Active session inside the context; the Session scope follows it. */
session: Accessor<string | undefined>
/** Whether the diff side panel is open. */
panelOpen: Accessor<boolean>
/** Whether the full-screen review tab is active. */
reviewActive: Accessor<boolean>
/** The id that marks the local pseudo-context (omits the Session scope). */
local: string
vscode: VsCode
}
export function createDiffReviewScope(opts: DiffReviewScopeOptions) {
const scope = createDiffScope(opts.ctx)
// The composite id (ctx#scope) the extension keys diff data by.
const id = createMemo(() => scope.id())
// The composite id (ctx#scope, or ctx#session:<sid>) the extension keys
// diff data by. Rebuilds when the active session changes while the Session
// scope is active, so a session tab switch refetches that session's diff.
const id = createMemo(() => {
const ctx = opts.ctx()
if (!ctx) return undefined
return composeDiffId(ctx, scope.scope(), scope.scope() === "session" ? opts.session() : undefined)
})
// Branch picker state for the active context (Branch scope only).
const [branches, setBranches] = createSignal<BranchInfo[]>([])
@@ -41,12 +47,20 @@ export function createDiffReviewScope(opts: DiffReviewScopeOptions) {
const [isAuto, setIsAuto] = createSignal(true)
const [currentBranch, setCurrentBranch] = createSignal<string | undefined>(undefined)
// Scope descriptors for the current context. The `local` pseudo-context and
// contexts without a real session omit the Session scope.
// Scope descriptors for the current context. The Session scope only exists
// when the context has an active session to diff.
const descriptors = createMemo(() => {
const ctx = opts.ctx()
if (!ctx) return []
return scopeDescriptors(ctx, ctx !== opts.local)
return scopeDescriptors(ctx, opts.session())
})
// Fall back to Branch when the active session disappears (tab closed,
// session deleted) while the Session scope is selected.
createEffect(() => {
const ctx = opts.ctx()
if (!ctx) return
if (scope.scope() === "session" && !opts.session()) scope.setScope("branch")
})
const isBranch = () => scope.scope() === "branch"
@@ -55,6 +69,10 @@ export function createDiffReviewScope(opts: DiffReviewScopeOptions) {
const ctx = opts.ctx()
if (!ctx) return
const value = next.slice(ctx.length + 1)
if (value.startsWith("session")) {
scope.setScope("session")
return
}
scope.setScope(isDiffScope(value) ? value : "branch")
}
@@ -1,11 +1,13 @@
/**
* Webview-side diff scope state for Agent Manager.
*
* Mirrors the extension's composite diff id (`ctx#scope`, see
* `src/agent-manager/diff-scope.ts`) and builds the fixed scope descriptor
* list shown in the scope selector. Agent Manager always offers the same four
* scopes per context, so the descriptors are computed client-side rather than
* pushed from the extension.
* Mirrors the extension's composite diff id (`ctx#scope`, or `ctx#session:<sid>`
* for the session scope — see `src/agent-manager/diff-scope.ts`) and builds the
* fixed scope descriptor list shown in the scope selector. The context is the
* sidebar selection (a worktree id or the `local` pseudo-context), so it stays
* stable across session tab switches; only the Session scope follows the active
* session. Agent Manager always offers the same four scopes per context, so the
* descriptors are computed client-side rather than pushed from the extension.
*/
import { createMemo, createSignal, type Accessor } from "solid-js"
@@ -16,15 +18,20 @@ export type DiffScope = "branch" | "staged" | "unstaged" | "session"
export const DEFAULT_DIFF_SCOPE: DiffScope = "branch"
const SEP = "#"
const SESSION_TOKEN = "session:"
export function composeDiffId(ctx: string, scope: DiffScope): string {
export function composeDiffId(ctx: string, scope: DiffScope, sessionId?: string): string {
if (scope === "session" && sessionId) return `${ctx}${SEP}${SESSION_TOKEN}${sessionId}`
return `${ctx}${SEP}${scope}`
}
export function parseDiffId(id: string): { ctx: string; scope: DiffScope } {
export function parseDiffId(id: string): { ctx: string; scope: DiffScope; sessionId?: string } {
const idx = id.lastIndexOf(SEP)
const scope = id.slice(idx + SEP.length)
if (idx !== -1 && isDiffScope(scope)) return { ctx: id.slice(0, idx), scope }
if (idx === -1) return { ctx: id, scope: DEFAULT_DIFF_SCOPE }
const token = id.slice(idx + SEP.length)
const ctx = id.slice(0, idx)
if (token.startsWith(SESSION_TOKEN)) return { ctx, scope: "session", sessionId: token.slice(SESSION_TOKEN.length) }
if (isDiffScope(token)) return { ctx, scope: token }
return { ctx: id, scope: DEFAULT_DIFF_SCOPE }
}
@@ -35,10 +42,11 @@ export function isDiffScope(value: string): value is DiffScope {
/**
* The fixed scope descriptors for a context. `workspace` maps to the Branch
* scope to reuse the existing i18n keys (`diffViewer.source.workspace.*`).
* Session scope is only meaningful for a real session context, so it is
* omitted for the `local` pseudo-context and for contexts without a session.
* Session scope is only meaningful when the context has an active session, so
* it is omitted while a context has none (e.g. an empty worktree or the local
* context with no open session).
*/
export function scopeDescriptors(ctx: string, hasSession: boolean): DiffSourceDescriptor[] {
export function scopeDescriptors(ctx: string, sessionId?: string): DiffSourceDescriptor[] {
const out: DiffSourceDescriptor[] = [
{
id: composeDiffId(ctx, "branch"),
@@ -54,9 +62,9 @@ export function scopeDescriptors(ctx: string, hasSession: boolean): DiffSourceDe
capabilities: { revert: false, comments: true },
},
]
if (hasSession) {
if (sessionId) {
out.push({
id: composeDiffId(ctx, "session"),
id: composeDiffId(ctx, "session", sessionId),
type: "session",
group: "Session",
capabilities: { revert: false, comments: true },
@@ -76,7 +84,8 @@ export function scopeCapabilities(scope: DiffScope): { revert: boolean; comments
/**
* Per-context scope selection. Keeps the last-picked scope per context id so
* switching between worktrees restores each worktree's scope, while a brand
* new context defaults to Branch.
* new context defaults to Branch. The context is the sidebar selection, so the
* picked scope survives session tab switches inside the context.
*/
export function createDiffScope(currentCtx: Accessor<string | undefined>) {
const [scopes, setScopes] = createSignal<Record<string, DiffScope>>({})
@@ -87,17 +96,11 @@ export function createDiffScope(currentCtx: Accessor<string | undefined>) {
return scopes()[ctx] ?? DEFAULT_DIFF_SCOPE
})
const id = createMemo(() => {
const ctx = currentCtx()
if (!ctx) return undefined
return composeDiffId(ctx, scope())
})
const setScope = (next: DiffScope) => {
const ctx = currentCtx()
if (!ctx) return
setScopes((prev) => ({ ...prev, [ctx]: next }))
}
return { scope, id, setScope }
return { scope, setScope }
}
@@ -13,7 +13,7 @@ interface Toast {
export function createRevertFile(
diffScopeId: Accessor<string | undefined>,
currentDiffSessionId: Accessor<string | undefined>,
ctx: Accessor<string | undefined>,
scope: Accessor<string>,
vscode: VsCode,
showToast: (t: Toast) => void,
@@ -29,14 +29,14 @@ export function createRevertFile(
function revert(file: string) {
const id = diffScopeId()
const sessionId = currentDiffSessionId()
if (!id || !sessionId) return
const context = ctx()
if (!id || !context) return
setFiles((prev) => {
const set = new Set(prev[id] ?? [])
set.add(file)
return { ...prev, [id]: set }
})
vscode.postMessage({ type: "agentManager.revertWorktreeFile", sessionId, file, scope: scope() })
vscode.postMessage({ type: "agentManager.revertWorktreeFile", sessionId: context, file, scope: scope() })
}
function onResult(ev: AgentManagerRevertWorktreeFileResultMessage) {
@@ -15,22 +15,24 @@ import type {
AgentManagerWorktreeDiffFileMessage,
AgentManagerWorktreeDiffLoadingMessage,
AgentManagerWorktreeDiffMessage,
AgentManagerWorktreeDiffNoticeMessage,
WorktreeFileDiff,
} from "../src/types/messages"
/**
* Decompose a composite diff id (`ctx#scope`) into the wire fields the
* extension expects. Bare ids (no scope separator) parse to the default
* branch scope.
* Decompose a composite diff id (`ctx#scope`, or `ctx#session:<sid>`) into the
* wire fields the extension expects. Bare ids (no scope separator) parse to
* the default branch scope.
*/
function wire(id: string) {
const { ctx, scope } = parseDiffId(id)
return { sessionId: ctx, scope }
export function wireDiffId(id: string) {
const { ctx, scope, sessionId } = parseDiffId(id)
return { sessionId: ctx, scope, diffSessionId: sessionId }
}
export function createWorktreeDiffs(vscode: ReturnType<typeof useVSCode>) {
const [diffDatas, setDiffDatas] = createSignal<Record<string, WorktreeFileDiff[]>>({})
const [diffLoading, setDiffLoading] = createSignal(false)
const [diffNotices, setDiffNotices] = createSignal<Record<string, string | undefined>>({})
const [diffFileLoading, setDiffFileLoading] = createSignal<Record<string, Record<string, true>>>({})
const setDiffFilePending = (sessionId: string, file: string, value: boolean) => {
@@ -63,7 +65,7 @@ export function createWorktreeDiffs(vscode: ReturnType<typeof useVSCode>) {
const requestDiffFile = (id: string, file: string) => {
if (diffFileLoading()[id]?.[file]) return
setDiffFilePending(id, file, true)
vscode.postMessage({ type: "agentManager.requestWorktreeDiffFile", file, ...wire(id) })
vscode.postMessage({ type: "agentManager.requestWorktreeDiffFile", file, ...wireDiffId(id) })
}
/** Files the backend flagged as stale in a merged update need a fresh fetch. */
@@ -72,7 +74,7 @@ export function createWorktreeDiffs(vscode: ReturnType<typeof useVSCode>) {
for (const file of files) {
if (loading[file]) continue
setDiffFilePending(id, file, true)
vscode.postMessage({ type: "agentManager.requestWorktreeDiffFile", file, ...wire(id) })
vscode.postMessage({ type: "agentManager.requestWorktreeDiffFile", file, ...wireDiffId(id) })
}
}
@@ -115,15 +117,21 @@ export function createWorktreeDiffs(vscode: ReturnType<typeof useVSCode>) {
setDiffLoading(ev.loading)
}
const onWorktreeDiffNotice = (ev: AgentManagerWorktreeDiffNoticeMessage) => {
setDiffNotices((prev) => ({ ...prev, [ev.sessionId]: ev.notice }))
}
return {
diffDatas,
diffLoading,
setDiffLoading,
diffNotices,
requestDiffFile,
refreshStaleDiffs,
diffFileLoadingFor,
onWorktreeDiff,
onWorktreeDiffFile,
onWorktreeDiffLoading,
onWorktreeDiffNotice,
}
}
@@ -65,12 +65,19 @@ import { createDiffRequests } from "./diff-requests"
type DiffStyle = "unified" | "split"
/** Well-known diff source notices → i18n keys (mirrors the standalone viewer). */
const DIFF_NOTICE_KEYS: Record<string, string> = {
"snapshots-disabled": "diffViewer.notice.snapshotsDisabled",
}
interface FullScreenDiffViewProps {
diffs: WorktreeFileDiff[]
loading: boolean
loadingFiles?: Set<string>
sessionId?: string
sessionKey?: string
/** Well-known source notice kind (e.g. "snapshots-disabled"), shown as a banner. */
notice?: string
comments: ReviewComment[]
onCommentsChange: (comments: ReviewComment[]) => void
composer?: ReviewComposer
@@ -96,6 +103,11 @@ interface FullScreenDiffViewProps {
export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) => {
const { t } = useLanguage()
const noticeText = () => {
const n = props.notice
if (!n) return ""
return t(DIFF_NOTICE_KEYS[n] ?? n)
}
const vscode = useVSCode()
const server = useServer()
const provider = useProvider()
@@ -615,6 +627,15 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
/>
</div>
<div class="am-review-diff" ref={setScroller}>
<Show when={noticeText()}>
<div class="diff-viewer-notice" role="status">
<span class="diff-viewer-notice-icon">
<Icon name="warning" size="small" />
</span>
<span class="diff-viewer-notice-text">{noticeText()}</span>
</div>
</Show>
<Show when={props.loading && props.diffs.length === 0}>
<div class="am-diff-loading">
<Spinner />
@@ -622,7 +643,7 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
</div>
</Show>
<Show when={!props.loading && props.diffs.length === 0}>
<Show when={!props.loading && props.diffs.length === 0 && !noticeText()}>
<div class="am-diff-empty">
<span>{t("session.review.noChanges")}</span>
</div>
@@ -949,6 +949,13 @@ export interface AgentManagerWorktreeDiffLoadingMessage {
loading: boolean
}
// Agent Manager: Source-level diff notice (extension → webview)
export interface AgentManagerWorktreeDiffNoticeMessage {
type: "agentManager.worktreeDiffNotice"
sessionId: string
notice?: DiffViewerNotice
}
export interface AgentManagerApplyWorktreeDiffResultMessage {
type: "agentManager.applyWorktreeDiffResult"
worktreeId: string
@@ -1355,6 +1362,7 @@ export type ExtensionMessage =
| AgentManagerWorktreeDiffMessage
| AgentManagerWorktreeDiffFileMessage
| AgentManagerWorktreeDiffLoadingMessage
| AgentManagerWorktreeDiffNoticeMessage
| AgentManagerApplyWorktreeDiffResultMessage
| AgentManagerRevertWorktreeFileResultMessage
| AgentManagerDiffBranchesMessage