From 3ab1122e0fbbff5c6821acb21df7d3d93f52955f Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Wed, 29 Jul 2026 14:01:00 +0200 Subject: [PATCH 1/7] feat(agent-manager): add diff scope selector and base branch picker --- .../agent-manager-diff-scope-selector.md | 5 + ...00000-agent-manager-diff-scope-selector.md | 339 ++++++++++++++++++ .../src/agent-manager/AgentManagerProvider.ts | 44 ++- .../src/agent-manager/diff-scope.ts | 57 +++ .../kilo-vscode/src/agent-manager/types.ts | 32 ++ .../agent-manager/worktree-diff-controller.ts | 193 +++++----- .../kilo-vscode/src/diff/sources/catalog.ts | 13 +- .../kilo-vscode/src/diff/sources/staged.ts | 38 +- .../kilo-vscode/src/diff/sources/unstaged.ts | 38 +- .../kilo-vscode/src/diff/sources/worktree.ts | 61 +++- packages/kilo-vscode/src/diff/types.ts | 21 ++ .../tests/unit/agent-manager-arch.test.ts | 4 +- .../kilo-vscode/tests/unit/diff-scope.test.ts | 52 +++ .../agent-manager/AgentManagerApp.tsx | 171 ++++----- .../webview-ui/agent-manager/DiffPanel.tsx | 11 +- .../agent-manager/agent-manager.css | 9 + .../webview-ui/agent-manager/diff-messages.ts | 74 ++++ .../agent-manager/diff-review-scope.ts | 120 +++++++ .../agent-manager/diff-scope-state.ts | 91 +++++ .../webview-ui/agent-manager/i18n/ar.ts | 1 + .../webview-ui/agent-manager/i18n/br.ts | 1 + .../webview-ui/agent-manager/i18n/bs.ts | 1 + .../webview-ui/agent-manager/i18n/da.ts | 1 + .../webview-ui/agent-manager/i18n/de.ts | 1 + .../webview-ui/agent-manager/i18n/en.ts | 1 + .../webview-ui/agent-manager/i18n/es.ts | 1 + .../webview-ui/agent-manager/i18n/fr.ts | 1 + .../webview-ui/agent-manager/i18n/it.ts | 1 + .../webview-ui/agent-manager/i18n/ja.ts | 1 + .../webview-ui/agent-manager/i18n/ko.ts | 1 + .../webview-ui/agent-manager/i18n/nl.ts | 1 + .../webview-ui/agent-manager/i18n/no.ts | 1 + .../webview-ui/agent-manager/i18n/pl.ts | 1 + .../webview-ui/agent-manager/i18n/ru.ts | 1 + .../webview-ui/agent-manager/i18n/th.ts | 1 + .../webview-ui/agent-manager/i18n/tr.ts | 1 + .../webview-ui/agent-manager/i18n/uk.ts | 1 + .../webview-ui/agent-manager/i18n/zh.ts | 1 + .../webview-ui/agent-manager/i18n/zht.ts | 1 + .../webview-ui/agent-manager/revert-file.ts | 17 +- .../diff-viewer/DiffScopeControls.tsx | 55 +++ .../diff-viewer/FullScreenDiffView.tsx | 5 +- .../src/types/messages/extension-messages.ts | 13 + .../src/types/messages/webview-messages.ts | 21 ++ 44 files changed, 1287 insertions(+), 217 deletions(-) create mode 100644 .changeset/agent-manager-diff-scope-selector.md create mode 100644 .kilo/plans/1784100000000-agent-manager-diff-scope-selector.md create mode 100644 packages/kilo-vscode/src/agent-manager/diff-scope.ts create mode 100644 packages/kilo-vscode/tests/unit/diff-scope.test.ts create mode 100644 packages/kilo-vscode/webview-ui/agent-manager/diff-messages.ts create mode 100644 packages/kilo-vscode/webview-ui/agent-manager/diff-review-scope.ts create mode 100644 packages/kilo-vscode/webview-ui/agent-manager/diff-scope-state.ts create mode 100644 packages/kilo-vscode/webview-ui/diff-viewer/DiffScopeControls.tsx diff --git a/.changeset/agent-manager-diff-scope-selector.md b/.changeset/agent-manager-diff-scope-selector.md new file mode 100644 index 00000000000..341e5ee7684 --- /dev/null +++ b/.changeset/agent-manager-diff-scope-selector.md @@ -0,0 +1,5 @@ +--- +"kilo-code": minor +--- + +Add a scope selector and base branch picker to the Agent Manager diff review. The side panel and full-screen review now let you switch between Branch, Staged, Unstaged, and Session scopes for the selected worktree, and the Branch scope's base branch can be overridden from a picker next to it. Branch stays the default, so existing review behavior is unchanged. diff --git a/.kilo/plans/1784100000000-agent-manager-diff-scope-selector.md b/.kilo/plans/1784100000000-agent-manager-diff-scope-selector.md new file mode 100644 index 00000000000..f827a2c3417 --- /dev/null +++ b/.kilo/plans/1784100000000-agent-manager-diff-scope-selector.md @@ -0,0 +1,339 @@ +# Bring the Changes scope selector and base branch picker into Agent Manager + +## Goal + +The standalone **Changes** editor panel has a scope selector (`GIT: Branch / Staged / Unstaged`, +`SESSION: Session`) plus a base branch picker (`main → origin/main [Default]`). Agent Manager has +two diff surfaces (compact side panel and full-screen review tab) with **no scope selector and no +base picker**: it always shows one fixed scope against one fixed base. + +Make both Agent Manager surfaces scope-aware and base-aware, reusing the existing components and +extension-side sources rather than duplicating them. + +## What exists today + +### Standalone Changes panel + +| Concern | Where | +|---|---| +| Panel host, ephemeral base override | `src/diff/DiffViewerProvider.ts:26-33`, `:101-119`, `:170-182` | +| Source enumeration and construction | `src/diff/sources/catalog.ts:73-115` | +| Scope select | `webview-ui/diff-viewer/DiffPickerHeader.tsx:50-86` | +| Base picker | `webview-ui/diff-viewer/BaseBranchPicker.tsx:77-142` | +| Branch list + auto base + HEAD | `src/diff/sources/catalog.ts:117-138` | +| Renderer | `webview-ui/diff-viewer/FullScreenDiffView.tsx:95` | + +Sources: `worktree.ts` (Branch), `staged.ts`, `unstaged.ts`, `session.ts`, `turn.ts`. Polling, +dedupe, and lazy per-file detail live in `src/diff/SourceController.ts`. + +### Agent Manager + +| Concern | Where | +|---|---| +| Diff controller (wraps the same `SourceController`) | `src/agent-manager/worktree-diff-controller.ts:35-80` | +| Synthetic single source, hardcoded capabilities | `src/agent-manager/worktree-diff-controller.ts:228-242` | +| Fixed base = `origin/` | `src/agent-manager/worktree-diff-controller.ts:217`, `WorktreeStateManager.ts:57-64` | +| `local` pseudo-context uses auto base | `src/agent-manager/worktree-diff-controller.ts:220-222` | +| Side panel | `webview-ui/agent-manager/DiffPanel.tsx:474-524` | +| Full-screen review (shared component) | `webview-ui/agent-manager/AgentManagerApp.tsx:3105-3131` | +| Data store keyed by session id | `webview-ui/agent-manager/AgentManagerApp.tsx:1675-1699` | + +So both systems already share `SourceController`, `local-diff.ts`, `GitOps`, `FullScreenDiffView`, +`FileTree`, `diff-state.ts`, `diff-requests.ts`. The gap is only the *selection* layer. + +## The architectural mismatch to resolve first + +The two systems key diff sources along orthogonal axes: + +- Standalone: keyed by **scope** (`workspace`, `staged`, `unstaged`, `session:`) inside one + fixed directory (`getWorkspaceRoot()`). +- Agent Manager: keyed by **context** (`sessionId` or `local`), which resolves to a directory and + base, with one fixed scope. + +Integration therefore needs a composite key `(context, scope)`: + +``` +ctx = "local" | "" +scope = "branch" | "staged" | "unstaged" | "session" +id = `${ctx}#${scope}` +``` + +`ctx#branch` is the default and reproduces today's behavior exactly. + +### Hard prerequisite: sources must accept an explicit directory + +Three sources resolve the workspace root themselves and cannot currently point at a worktree: + +- `src/diff/sources/worktree.ts:46`, `:60` +- `src/diff/sources/staged.ts:47` +- `src/diff/sources/unstaged.ts:53` +- `src/diff/sources/catalog.ts:118` (`listWorkspaceBranches`) + +`session.ts` is already directory-parameterized (`catalog.ts:111`), so Session scope is nearly free. + +Each source also constructs its own `GitOps` + `OutputChannel` (`worktree.ts:35-37`, +`staged.ts:43-45`). That is acceptable in the standalone panel where sources swap only on scope +change, but Agent Manager swaps sources on **every session selection**. Inject a `log` function and +reuse Agent Manager's shared `GitOps` (`AgentManagerProvider.ts:140-180`) instead of constructing +per source. + +## Is Branch still the right default per worktree? + +Yes, keep `Branch` as the default for every worktree context. Reasons, strongest first: + +1. **It matches what you ship.** Branch is `merge-base(HEAD, origin/) → current working + tree`: committed work, staged, unstaged, and untracked. That is exactly the payload + `Apply to local` builds (`GitOps.buildWorktreePatch`, used at + `worktree-diff-controller.ts:115`) and what a PR from that branch would contain. Reviewing the + same set you apply or push is the whole point of the review tab. +2. **No silent behavior change.** It is what Agent Manager does today, and the sidebar `Nf +N -N` + badge (`GitStatsPoller.ts:191-224`) uses the same base. A different default would make the + badge and the review disagree on first open. +3. **Stable under base movement.** merge-base semantics mean commits landing on `origin/main` + after the worktree branched do not pollute the diff. +4. **Session scope can be legitimately empty.** It depends on snapshots being enabled and degrades + to a `snapshots-disabled` notice (`sources/session.ts:33-66`). A default that can be empty for + configuration reasons is a bad default. + +One correction to the framing: Branch compares against the base **ref** (`origin/main`), not +against the local main checkout's working tree. If your local `main` has unpushed commits or dirty +files, the worktree diff does not account for them. "What changes if I apply this to my current +checkout" is a different question, answered today only by the `Apply to local` conflict check. A +`Local workspace` scope could answer it directly, but it is a non-goal here (see below). + +## What each scope means in Agent Manager + +| Scope | Worktree context | Local context | Value | +|---|---|---|---| +| Branch | `merge-base(HEAD, origin/)` to working tree | `merge-base(HEAD, auto base)` to working tree | Default. The reviewable/shippable set. | +| Staged | index vs `HEAD` inside the worktree | same, workspace root | "What did I stage for the next commit." Read-only. | +| Unstaged | working tree vs index, plus untracked | same | "What is not committed yet." Read-only. | +| Session | snapshot diff for the selected session, `directory = worktree.path` | selected local session | Highest new value: separates *this agent session's* edits from manual edits and setup-script output. | + +Two honest caveats to design around: + +- On a fresh worktree where the agent never commits (the common case), `Branch` ≈ `Staged` + + `Unstaged`, so the selector adds little until a commit exists. It is still worth shipping because + `Session` is valuable immediately and because committing agents are increasingly common. +- A worktree can hold several sessions. Expose only the **currently selected** session's Session + scope; a per-session submenu is a follow-up, not v1. + +## UI design + +### Full-screen review tab + +Put the controls at the head of the existing left toolbar group +(`FullScreenDiffView.tsx:542-571`), before the unified/split radio. Do **not** add a second row: +the standalone panel's separate header row should collapse into this same slot so both hosts render +one identical toolbar. + +``` ++-----------------------------------------------------------------------------------------------+ +| [Branch v] feat/foo -> origin/main [Default] | (Unified|Split) 12 files +340 -88 | | +| ^ scope ^ base picker ^ existing stats | +| Expand all Send 3 to chat [x] | ++-----------------------------------------------------------------------------------------------+ +| tree | diff | +``` + +### Compact side panel + +`DiffPanel`'s header (`DiffPanel.tsx:476-524`) already competes for width with the radio group, +stats, and three icon buttons in a resizable inspector. Add a **second compact row** under the +existing header rather than cramming one row: + +``` ++------------------------------------------+ +| Changes (Unified|Split) 12f +340 -88 | [expand] [fullscreen] [x] +| [Branch v] -> origin/main | ++------------------------------------------+ +``` + +Below a width threshold, drop the `-> origin/main` hint and keep only `[Branch v]`; the full base +picker stays reachable in the full-screen tab. The side panel must at least *display* the active +scope even when narrow, because scope state is shared with the review tab (single +`SourceController`) and an unexplained staged-only file list is confusing. + +### Dropdown + +Reuse `DiffPickerHeader` unchanged: it already renders grouped options with per-option tooltips +from `diffViewer.source..tooltip` (`webview-ui/src/i18n/en.ts:1235-1248`). + +``` ++--------------------------+ +| GIT | +| Branch | tooltip: all changes vs base, incl. local commits +| Staged | +| Unstaged | +| SESSION | +| Session | ++--------------------------+ +``` + +Deliberately **no per-scope file counts** in the dropdown. Counts would require polling every scope +continuously (four git pipelines per tick per worktree), which is not worth it. The Branch row's +"vs origin/main" context is already carried by the adjacent base picker. + +### UX traps to close + +- **Apply to local always applies Branch scope.** It builds its patch from + `remoteRef(worktree)` regardless of what the review shows. When the active scope is not `Branch`, + either label the button "Apply branch changes" or disable it with a tooltip explaining that apply + is branch-scoped. Otherwise users will read "Apply" as "apply what I am looking at". +- **Revert must follow source capabilities.** `staged` and `unstaged` declare + `capabilities.revert: false` (`staged.ts:29`, `unstaged.ts:29`). Agent Manager currently + hardcodes `{ revert: true, comments: true }` (`worktree-diff-controller.ts:233`). Send the real + descriptor capabilities and pass them into the already-existing `canRevert` / `canComment` props + (`FullScreenDiffView.tsx:88-91`). +- **Scope switch should not flash stale files.** Key the webview diff store by the composite key so + switching back to a previously fetched scope is instant and never renders another scope's files. + +## Base branch picker: semantics and the consistency problem + +In the standalone panel the base override is ephemeral and its `Default` means +"auto-resolved tracking or repo default" (`shared/target.ts:4-27`). In Agent Manager, `Default` +should mean **the worktree's recorded parent** (`origin/`), which is a deliberate +recorded value, not a guess. + +The real issue: as soon as a base override exists, the review toolbar and the sidebar badge can +disagree, because `GitStatsPoller` and `Apply to local` both derive from `remoteRef(worktree)`. + +Two coherent options: + +**A. View-local ephemeral override (cheap).** Mirrors the standalone panel. Zero risk to stats, +apply, and PR flows, but the sidebar badge will visibly disagree with the review toolbar the moment +the user overrides. Needs the review header to always show `vs ` so the difference is legible. + +**B. Persisted worktree base (recommended).** Treat the picker as editing the worktree's base: +persist `parentBranch` / `remote` (or a new `diffBase`) in `agent-manager.json` +(`WorktreeStateManager.ts:16-45`), and let stats, review, and apply all read the same value. This +keeps a single base per worktree with no divergence, and it fixes a real existing gap: today a +worktree's base cannot be changed after creation, so branching off the wrong base is unrecoverable +without recreating the worktree. + +Recommendation: ship **A** in the same phase as the scope selector to keep the diff small, then +promote to **B** as its own change, because B needs stats invalidation, apply, and PR paths updated +together and deserves isolated review. If B is chosen up front, do not also keep A: two competing +bases is worse than either. + +## Reuse plan + +The point of this work is to add zero new diff rendering code. + +### Reuse as-is + +- `DiffPickerHeader.tsx` (scope select, grouping, tooltips) +- `BaseBranchPicker.tsx` and `webview-ui/src/components/shared/BranchSelect.tsx` +- `FullScreenDiffView.tsx`, `FileTree.tsx`, `VirtualDiffList.tsx`, `diff-state.ts`, + `diff-requests.ts`, `diff-open-policy.ts` +- `SourceController.ts`, `sources/session.ts`, `local-diff.ts`, `GitOps.ts` +- All `diffViewer.source.*` and `diffViewer.baseBranch.*` i18n keys, already translated + +### New shared pieces (small) + +1. `webview-ui/diff-viewer/DiffScopeControls.tsx` - composes `DiffPickerHeader` + + `BaseBranchPicker`, plus a `compact` flag for the side panel. Consumed by three hosts: + standalone `DiffViewerApp`, Agent Manager review tab, Agent Manager side panel. +2. A leading toolbar slot prop on both renderers: `FullScreenDiffView` (`lead?: JSXElement`, + rendered first inside `am-review-toolbar-left`) and `DiffPanel` (second header row). Once + `FullScreenDiffView` has the slot, move the standalone panel's separate header row into it so + both hosts share one layout. +3. `webview-ui/agent-manager/diff-scope-state.ts` - composite key helpers and per-context scope + signal. Keep this out of `AgentManagerApp.tsx` (already 3191 lines). + +### Extension side + +4. Generalize `PanelContext` so `workspaceRoot` is the resolved diff directory, and make + `DiffSourceCatalog` build sources for that directory. `SourceController.setContext` is already + called by Agent Manager (`worktree-diff-controller.ts:79`, `:188`), so the plumbing exists. +5. `WorktreeDiffController.source()` delegates to `DiffSourceCatalog` instead of returning its + bespoke descriptor. This deletes Agent Manager's synthetic source and gets staged, unstaged, and + session scopes for free. +6. `src/agent-manager/diff-scope.ts` (new, vscode-free): composite id parse/format, scope to source + id mapping, capability lookup. `worktree-diff-controller.ts` is already 332 lines and + `tests/unit/agent-manager-arch.test.ts` enforces `maxLines` caps that must not be raised. + +## Message and state changes + +Inbound (webview to extension), all additive and optional so existing callers keep working: + +| Message | Change | +|---|---| +| `agentManager.requestWorktreeDiff` | add `scope?` | +| `agentManager.startDiffWatch` | add `scope?` | +| `agentManager.requestWorktreeDiffFile` | add `scope?` | +| `agentManager.revertWorktreeFile` | add `scope?` | +| `agentManager.requestDiffBranches` | new, `{ sessionId }` | +| `agentManager.setDiffBaseBranch` | new, `{ sessionId, branch? }` where `undefined` clears | + +No separate "set scope" message: a scope switch is a re-activation via `startDiffWatch` / +`requestWorktreeDiff` with the new scope. + +Outbound: `worktreeDiff`, `worktreeDiffLoading`, `worktreeDiffFile`, `revertWorktreeFileResult` +gain `scope` and the source `capabilities`; new `agentManager.diffBranches` reuses the existing +`WorkspaceBranchesResult` shape (`sources/catalog.ts:22-33`). + +Webview state: `diffDatas` and `diffFileLoading` re-key from `sessionId` to `${ctx}#${scope}`; +`diffSessionKey()` (`AgentManagerApp.tsx:1694-1699`) appends the scope so accordion open state +resets per scope. + +Known breakage to fix during the refactor: `shouldStopForWorktree` passes +`this.controller.currentId` into `shouldStopDiffPolling`, which compares it against orphaned +session ids (`delete-worktree.ts:11-20`). Pass the parsed context id, not the composite id. + +Target resolution ordering: today `ensureTarget` resolves lazily inside `fetch()`. With the catalog +owning source construction, the directory and base must be resolved *before* +`controller.activate()`. `activate` is already awaited by `request` and `start`, so awaiting +`ready()` plus target resolution first is safe, but this is the main refactor risk in the change. + +## Phasing + +1. **Parameterize sources by directory.** Add explicit `dir` / injected `log` / shared `GitOps` to + `worktree.ts`, `staged.ts`, `unstaged.ts`, and `listWorkspaceBranches`. No user-visible change; + standalone panel keeps passing the workspace root. Verify the standalone panel is unaffected. +2. **Composite keying and catalog delegation.** `diff-scope.ts`, controller delegates to the + catalog, messages gain `scope` and `capabilities`, webview re-keys. Still one scope exposed, so + still no visible change. This is the risky phase and should land on its own. +3. **Scope selector UI.** `DiffScopeControls`, toolbar slots in both hosts, standalone header moved + into the shared slot, `canRevert` / `canComment` wired from capabilities, Apply-scope guard. +4. **Base picker in Agent Manager.** `requestDiffBranches` / `setDiffBaseBranch`, ephemeral + override (option A), `Default` labeled as the worktree's recorded parent. +5. **Optional follow-up.** Promote the override to a persisted worktree base (option B) shared with + stats, apply, and PR. + +## Verification + +- `packages/kilo-vscode/`: `bun run typecheck`, `bun run lint`, `bun run test:unit`, `bun run knip` + (new exports must be imported somewhere). +- New unit tests: scope to source id mapping, composite id parse/format, base override resolution + for a worktree directory, and `shouldStopDiffPolling` with composite ids. Existing coverage to + keep green: `tests/unit/local-diff.test.ts`, `tests/unit/agent-manager-arch.test.ts`. +- Visual regression stories for the toolbar in both hosts, per the `vscode-visual-regression` + skill. +- Manual (self-test instance): worktree with a commit plus dirty files, switch Branch / Staged / + Unstaged / Session, confirm counts differ correctly, revert hidden in read-only scopes, side + panel and review tab stay in sync, base override changes the file set, and switching worktrees + resets to Branch. +- Changeset required (user-facing feature). + +## Non-goals + +- A `Local workspace` scope comparing a worktree against the local checkout's working tree. Useful, + but it needs a tree-to-tree diff path that does not exist in `local-diff.ts` and it overlaps with + the `Apply to local` conflict check. +- Per-session Session scope submenu when a worktree holds multiple sessions. +- Turn scope inside Agent Manager. Per-turn changes already open the standalone panel from the + transcript (`VscodeSessionTurn.tsx:179-200`). +- Per-scope file counts in the dropdown. +- Merging the standalone Changes panel into Agent Manager. Phase 3 makes them converge visually; + actually collapsing the two hosts is a separate decision. + +## Open questions + +1. Option A or B for the base picker (view-local override vs persisted worktree base). B is the + better end state; A is the smaller step. +2. Should the scope persist per worktree in `agent-manager.json` alongside `reviewDiffStyle`, or + always reset to Branch on selection change? Resetting is more predictable; persisting is less + repetitive for someone who lives in Session scope. +3. Should the side panel's scope control be interactive or display-only, given how little width it + has? diff --git a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts index bf72af74ab8..2556bd391db 100644 --- a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts +++ b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts @@ -4,10 +4,12 @@ import type { KiloClient, Session } from "@kilocode/sdk/v2/client" import type { KiloConnectionService } from "../services/cli-backend" import { getErrorMessage } from "../kilo-provider-utils" import { resolveLocalDiffTarget } from "../diff/shared/target" +import { DiffSourceCatalog } from "../diff/sources/catalog" import { getDiffMarkdownRender, setDiffMarkdownRender } from "../review-settings" import { isAbsolutePath } from "../path-utils" import { WorktreeManager, type CreateWorktreeResult } from "./WorktreeManager" import { remoteRef, WorktreeStateManager, type Worktree } from "./WorktreeStateManager" +import { composeDiffId, normalizeScope } from "./diff-scope" import { handleSection } from "./section-handler" import { normalizeBaseBranch } from "./base-branch" import { GitStatsPoller, type LocalStats, type WorktreePresenceResult, type WorktreeStats } from "./GitStatsPoller" @@ -71,6 +73,7 @@ export class AgentManagerProvider implements Disposable { private orchestration: AgentManagerOrchestrationBridge private gitOps: GitOps private diffs: WorktreeDiffController + private diffCatalog: DiffSourceCatalog private naming: BranchNamingController private staleWorktreeIds = new Set() private toolRequests = new Set() @@ -148,12 +151,13 @@ export class AgentManagerProvider implements Disposable { log: (msg) => this.log(msg), }) const local = createLocalDiff(this.gitOps, (...args) => this.log(...args)) + this.diffCatalog = new DiffSourceCatalog(this.connectionService) this.diffs = new WorktreeDiffController({ getState: () => this.getStateManager(), getRoot: () => this.getRoot(), getStateReady: () => this.stateReady, + catalog: this.diffCatalog, git: this.gitOps, - localDiff: local.summary, localDiffFile: local.file, post: (msg) => this.postToWebview(msg), log: (...args) => this.log(...args), @@ -679,11 +683,11 @@ export class AgentManagerProvider implements Disposable { private onDiffMessage(m: AgentManagerInMessage): Record | null | undefined { if (m.type === "agentManager.requestWorktreeDiff") { - void this.diffs.request(m.sessionId) + void this.diffs.request(composeDiffId(m.sessionId, normalizeScope(m.scope))) return null } if (m.type === "agentManager.requestWorktreeDiffFile") { - void this.diffs.requestFile(m.sessionId, m.file) + void this.diffs.requestFile(composeDiffId(m.sessionId, normalizeScope(m.scope)), m.file) return null } if (m.type === "agentManager.applyWorktreeDiff") { @@ -691,23 +695,52 @@ export class AgentManagerProvider implements Disposable { return null } if (m.type === "agentManager.revertWorktreeFile") { - void this.diffs.revert(m.sessionId, m.file) + void this.diffs.revert(composeDiffId(m.sessionId, normalizeScope(m.scope)), m.file) return null } if (m.type === "agentManager.startDiffWatch") { - this.diffs.start(m.sessionId) + this.diffs.start(composeDiffId(m.sessionId, normalizeScope(m.scope))) return null } if (m.type === "agentManager.stopDiffWatch") { this.diffs.stop() return null } + if (m.type === "agentManager.requestDiffBranches") { + void this.sendDiffBranches(m.sessionId, m.scope) + return null + } + if (m.type === "agentManager.setDiffBaseBranch") { + void this.diffs.setBase(composeDiffId(m.sessionId, normalizeScope(m.scope)), m.branch).then(() => { + void this.sendDiffBranches(m.sessionId, m.scope) + }) + return null + } if (m.type === "agentManager.openFile") { this.openWorktreeFile(m.sessionId, m.filePath, m.line, m.column) return null } } + private async sendDiffBranches(sessionId: string, scope?: string): Promise { + const id = composeDiffId(sessionId, normalizeScope(scope)) + const result = await this.diffs.branches(id).catch((err) => { + this.log("Failed to list diff branches:", err instanceof Error ? err.message : String(err)) + return undefined + }) + if (!result) return + this.postToWebview({ + type: "agentManager.diffBranches", + sessionId: id, + branches: result.branches, + defaultBranch: result.defaultBranch, + autoBase: result.autoBase, + currentBase: result.currentBase, + isAuto: result.isAuto, + currentBranch: result.currentBranch, + }) + } + private onBridgeMessage(m: AgentManagerInMessage): Record | null | undefined { if (m.type !== "openFile") return undefined @@ -1914,6 +1947,7 @@ export class AgentManagerProvider implements Disposable { this.orchestration.dispose() this.visiblePresence.clear() this.diffs.stop() + this.diffCatalog.dispose() this.naming.dispose() this.statsPoller.stop() this.gitOps.dispose() diff --git a/packages/kilo-vscode/src/agent-manager/diff-scope.ts b/packages/kilo-vscode/src/agent-manager/diff-scope.ts new file mode 100644 index 00000000000..21d2e007935 --- /dev/null +++ b/packages/kilo-vscode/src/agent-manager/diff-scope.ts @@ -0,0 +1,57 @@ +/** + * Composite diff-source keying for Agent Manager. + * + * Agent Manager keys diff sources by *context* (a session 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. + * + * ctx = "local" | "" + * scope = "branch" | "staged" | "unstaged" | "session" + * id = `${ctx}#${scope}` + * + * `ctx#branch` is the default and reproduces the pre-scope behavior exactly. + */ + +export type DiffScope = "branch" | "staged" | "unstaged" | "session" + +export const DEFAULT_DIFF_SCOPE: DiffScope = "branch" + +const SEP = "#" + +export function composeDiffId(ctx: string, scope: DiffScope): string { + return `${ctx}${SEP}${scope}` +} + +/** + * Split a composite id back into context and scope. Tolerates a bare context + * 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 } { + 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 } + return { ctx: id, scope: DEFAULT_DIFF_SCOPE } +} + +export function isDiffScope(value: string): value is DiffScope { + return value === "branch" || value === "staged" || value === "unstaged" || value === "session" +} + +export function normalizeScope(value: unknown): DiffScope { + return typeof value === "string" && isDiffScope(value) ? value : DEFAULT_DIFF_SCOPE +} + +/** + * 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. + */ +export function scopeToSourceId(scope: DiffScope, ctx: string): string { + if (scope === "staged") return "staged" + if (scope === "unstaged") return "unstaged" + if (scope === "session") return `session:${ctx}` + return "workspace" +} diff --git a/packages/kilo-vscode/src/agent-manager/types.ts b/packages/kilo-vscode/src/agent-manager/types.ts index 8950c68d22f..ae72d2cd015 100644 --- a/packages/kilo-vscode/src/agent-manager/types.ts +++ b/packages/kilo-vscode/src/agent-manager/types.ts @@ -286,6 +286,18 @@ interface RevertWorktreeFileResultMessage { message: string } +/** Branch picker data for a context's diff directory. */ +interface DiffBranchesMessage { + type: "agentManager.diffBranches" + sessionId: string + branches: BranchListItem[] + defaultBranch: string + autoBase?: string + currentBase?: string + isAuto: boolean + currentBranch?: string +} + interface PRStatusOutMessage { type: "agentManager.prStatus" worktreeId: string @@ -324,6 +336,7 @@ export type AgentManagerOutMessage = | WorktreeDiffMessage | WorktreeDiffFileMessage | RevertWorktreeFileResultMessage + | DiffBranchesMessage | PRStatusOutMessage | ActionOutMessage | RunStatusMessage @@ -512,6 +525,7 @@ interface ImportFromPRIn { interface RequestWorktreeDiffIn { type: "agentManager.requestWorktreeDiff" sessionId: string + scope?: string } interface ApplyWorktreeDiffIn { @@ -524,11 +538,13 @@ interface RequestWorktreeDiffFileIn { type: "agentManager.requestWorktreeDiffFile" sessionId: string file: string + scope?: string } interface StartDiffWatchIn { type: "agentManager.startDiffWatch" sessionId: string + scope?: string } interface StopDiffWatchIn { @@ -539,6 +555,20 @@ interface RevertWorktreeFileIn { type: "agentManager.revertWorktreeFile" sessionId: string file: string + scope?: string +} + +interface RequestDiffBranchesIn { + type: "agentManager.requestDiffBranches" + sessionId: string + scope?: string +} + +interface SetDiffBaseBranchIn { + type: "agentManager.setDiffBaseBranch" + sessionId: string + scope?: string + branch?: string } interface RefreshPRIn { @@ -803,6 +833,8 @@ export type AgentManagerInMessage = | StartDiffWatchIn | StopDiffWatchIn | RevertWorktreeFileIn + | RequestDiffBranchesIn + | SetDiffBaseBranchIn | RefreshPRIn | OpenPRIn | OpenSessionsIn diff --git a/packages/kilo-vscode/src/agent-manager/worktree-diff-controller.ts b/packages/kilo-vscode/src/agent-manager/worktree-diff-controller.ts index 75d538cc35c..dff233ca5cc 100644 --- a/packages/kilo-vscode/src/agent-manager/worktree-diff-controller.ts +++ b/packages/kilo-vscode/src/agent-manager/worktree-diff-controller.ts @@ -1,12 +1,13 @@ import { SourceController } from "../diff/SourceController" import { resolveLocalDiffTarget } from "../diff/shared/target" import { WorktreeDiffReverter, type StatusResolver } from "../diff/shared/reverter" -import type { DiffFile } from "../diff/types" -import type { DiffSource, DiffSourceDescriptor, DiffSourceFetch } from "../diff/sources/types" +import type { DiffFile, PanelContext } from "../diff/types" +import type { DiffSource } from "../diff/sources/types" +import type { DiffSourceCatalog } from "../diff/sources/catalog" import type { ApplyConflict, GitOps } from "./GitOps" import { shouldStopDiffPolling } from "./delete-worktree" -import { Semaphore } from "./semaphore" import { remoteRef, type ManagedSession, type WorktreeStateManager } from "./WorktreeStateManager" +import { parseDiffId, scopeToSourceId } from "./diff-scope" import type { AgentManagerOutMessage, WorktreeDiffEntry } from "./types" const LOCAL_DIFF_ID = "local" as const @@ -19,14 +20,11 @@ export interface WorktreeDiffControllerContext { getState: () => WorktreeStateManager | undefined getRoot: () => string | undefined getStateReady: () => Promise | undefined - /** - * In-process diff paths deliberately bypass the SDK client to keep git spawns - * out of the Bun `kilo serve` process (see oven-sh/bun#18265). - */ + /** Builds the underlying per-scope diff sources (workspace/staged/unstaged/session). */ + catalog: DiffSourceCatalog + /** Shared git ops, injected into sources so they don't spawn their own channels. */ git: GitOps - /** In-process diff summary (replaces client.worktree.diffSummary). */ - localDiff: (dir: string, base: string) => Promise - /** In-process single-file diff (replaces client.worktree.diffFile). */ + /** In-process single-file diff (replaces client.worktree.diffFile). Used by revert. */ localDiffFile: (dir: string, base: string, file: string) => Promise post: (msg: AgentManagerOutMessage) => void log: (...args: unknown[]) => void @@ -34,13 +32,14 @@ export interface WorktreeDiffControllerContext { export class WorktreeDiffController { private readonly controller: SourceController - private readonly details = new Semaphore(3) private target: Target | undefined private applying: string | undefined + /** Ephemeral per-context base override, keyed by context id. */ + private baseOverrides = new Map() constructor(private readonly ctx: WorktreeDiffControllerContext) { this.controller = new SourceController( - (id) => this.source(id), + (id, ctx) => this.source(id, ctx), () => [], (msg) => this.ctx.post(msg as AgentManagerOutMessage), { @@ -80,7 +79,11 @@ export class WorktreeDiffController { } public shouldStopForWorktree(path: string, sessions: ManagedSession[]): boolean { - return shouldStopDiffPolling(path, sessions, this.target, this.controller.currentId) + // Pass the parsed context id, not the composite id, so the orphaned-session + // check matches real session ids. + const current = this.controller.currentId + const ctxId = current ? parseDiffId(current).ctx : undefined + return shouldStopDiffPolling(path, sessions, this.target, ctxId) } public async apply(worktreeId: string, value?: unknown): Promise { @@ -144,38 +147,38 @@ export class WorktreeDiffController { } } - public async revert(sessionId: string, file: string): Promise { + public async revert(id: string, file: string): Promise { if (!file) return - if (this.controller.currentId !== sessionId) { - const result = await this.revertFile(sessionId, file) - this.postRevertResult(sessionId, file, result) + if (this.controller.currentId !== id) { + const result = await this.revertFile(id, file) + this.postRevertResult(id, file, result) return } await this.controller.revertFile(file) } - public async request(sessionId: string): Promise { - if (this.controller.currentId !== sessionId) { - await this.activate(sessionId, false, true) + public async request(id: string): Promise { + if (this.controller.currentId !== id) { + await this.activate(id, false, true) return } this.target = undefined await this.controller.refresh() } - public async requestFile(sessionId: string, file: string): Promise { + public async requestFile(id: string, file: string): Promise { if (!file) return - if (this.controller.currentId !== sessionId) { - this.ctx.post({ type: "agentManager.worktreeDiffFile", sessionId, file, diff: null }) + if (this.controller.currentId !== id) { + this.ctx.post({ type: "agentManager.worktreeDiffFile", sessionId: id, file, diff: null }) return } await this.controller.requestFile(file) } - public start(sessionId: string): void { - if (this.controller.isPolling && this.controller.currentId === sessionId) return - this.ctx.log(`Starting diff polling for session ${sessionId}`) - void this.activate(sessionId, true, true) + public start(id: string): void { + if (this.controller.isPolling && this.controller.currentId === id) return + this.ctx.log(`Starting diff polling for ${id}`) + void this.activate(id, true, true) } public stop(): void { @@ -183,92 +186,113 @@ export class WorktreeDiffController { this.target = undefined } - private async activate(sessionId: string, poll: boolean, fetch: boolean): Promise { + /** + * Set or clear an ephemeral base override for a context (worktree or local), + * then re-activate the current source so it refetches against the new base. + * Passing undefined clears the override and falls back to the recorded parent. + */ + public async setBase(id: string, branch: string | undefined): Promise { + const { ctx } = parseDiffId(id) + if (branch) this.baseOverrides.set(ctx, branch) + else this.baseOverrides.delete(ctx) this.target = undefined - this.controller.setContext({ workspaceRoot: this.ctx.getRoot() }) - await this.controller.activate(sessionId, { poll, fetch }) + await this.controller.reactivate() } - private async resolve(sessionId: string): Promise<{ directory: string; baseBranch: string } | undefined> { - if (sessionId === LOCAL_DIFF_ID) return await this.resolveLocal() + /** Branch picker data for a context's directory, using any active override. */ + public async branches(id: string) { + await this.ready("stateReady rejected, continuing diff branches resolve:") + const { ctx } = parseDiffId(id) + const target = await this.resolve(ctx) + if (!target) return undefined + return await this.ctx.catalog.listWorkspaceBranches(this.baseOverrides.get(ctx), target.directory) + } + + private async activate(id: string, poll: boolean, fetch: boolean): Promise { + this.target = undefined + await this.ready("stateReady rejected, continuing diff activate:") + const { ctx } = parseDiffId(id) + const resolved = await this.resolve(ctx) + this.target = resolved ? { sessionId: id, ...resolved } : undefined + this.controller.setContext({ + workspaceRoot: this.ctx.getRoot(), + dir: resolved?.directory, + // The resolved base already bakes in any ephemeral override (see + // resolve()), so pass it as the explicit base and leave + // baseBranchOverride unset to avoid double resolution. + baseBranch: resolved?.baseBranch, + // Agent Manager always knows its intended directory (LOCAL resolves to + // the root). Never fall back to the workspace root for an unresolvable + // worktree context — return an empty diff instead. + strictDir: true, + git: this.ctx.git, + log: (...args) => this.ctx.log(...args), + }) + await this.controller.activate(id, { poll, fetch }) + } + + private async resolve(ctxId: string): Promise<{ directory: string; baseBranch: string } | undefined> { + if (ctxId === LOCAL_DIFF_ID) return await this.resolveLocal() const state = this.ctx.getState() if (!state) { - this.ctx.log(`resolveDiffTarget: no state manager for session ${sessionId}`) + this.ctx.log(`resolveDiffTarget: no state manager for context ${ctxId}`) return undefined } - const session = state.getSession(sessionId) + const session = state.getSession(ctxId) if (!session) { this.ctx.log( - `resolveDiffTarget: session ${sessionId} not found in state (${state.getSessions().length} total sessions)`, + `resolveDiffTarget: session ${ctxId} not found in state (${state.getSessions().length} total sessions)`, ) return undefined } if (!session.worktreeId) { - this.ctx.log(`resolveDiffTarget: session ${sessionId} has no worktreeId (local session)`) + this.ctx.log(`resolveDiffTarget: session ${ctxId} has no worktreeId (local session)`) return undefined } const worktree = state.getWorktree(session.worktreeId) if (!worktree) { - this.ctx.log(`resolveDiffTarget: worktree ${session.worktreeId} not found for session ${sessionId}`) + this.ctx.log(`resolveDiffTarget: worktree ${session.worktreeId} not found for session ${ctxId}`) return undefined } - return { directory: worktree.path, baseBranch: remoteRef(worktree) } + const base = this.baseOverrides.get(ctxId) ?? remoteRef(worktree) + return { directory: worktree.path, baseBranch: base } } private async resolveLocal(): Promise<{ directory: string; baseBranch: string } | undefined> { - return await resolveLocalDiffTarget(this.ctx.git, (...args) => this.ctx.log(...args), this.ctx.getRoot()) + const root = this.ctx.getRoot() + if (!root) return undefined + const override = this.baseOverrides.get(LOCAL_DIFF_ID) + if (override) { + return { directory: root, baseBranch: override } + } + return await resolveLocalDiffTarget(this.ctx.git, (...args) => this.ctx.log(...args), root) } private async ready(msg: string): Promise { await this.ctx.getStateReady()?.catch((err) => this.ctx.log(msg, err)) } - private source(sessionId: string): DiffSource { - const descriptor: DiffSourceDescriptor = { - id: sessionId, - type: "workspace", - group: "Git", - capabilities: { revert: true, comments: true }, - } - + /** + * 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. + */ + private source(id: string, panelCtx: PanelContext): DiffSource { + const { ctx, scope } = parseDiffId(id) + const built = this.ctx.catalog.build(scopeToSourceId(scope, ctx), panelCtx) return { - descriptor, - fetch: () => this.fetch(sessionId), - fetchFile: (file) => this.fetchFile(sessionId, file), - revert: (file) => this.revertFile(sessionId, file), + ...built, + descriptor: { ...built.descriptor, id }, } } - private async fetch(sessionId: string): Promise { - await this.ready("stateReady rejected, continuing diff resolve:") - const target = await this.ensureTarget(sessionId) - if (!target) return { diffs: [], stopPolling: true } - - const files = await this.ctx.localDiff(target.directory, target.baseBranch) - this.ctx.log(`Worktree diff returned ${files.length} file(s) for session ${sessionId}`) - return { diffs: files as AgentManagerDiffFile[] } - } - - private async fetchFile(sessionId: string, file: string): Promise { - await this.ready("stateReady rejected, continuing diff detail resolve:") - return this.details.run(async () => { - const target = await this.ensureTarget(sessionId) - if (!target) return null - - try { - return (await this.ctx.localDiffFile(target.directory, target.baseBranch, file)) as AgentManagerDiffFile | null - } catch (error) { - this.ctx.log("Failed to fetch worktree diff file:", error) - return null - } - }) - } - - private async revertFile(sessionId: string, file: string): Promise<{ ok: boolean; message: string }> { + private async revertFile(id: string, file: string): Promise<{ ok: boolean; message: string }> { await this.ready("stateReady rejected, continuing revert resolve:") - const target = await this.resolveTarget(sessionId) + const { ctx } = parseDiffId(id) + const target = await this.resolve(ctx) if (!target) return { ok: false, message: "Could not resolve diff target" } try { @@ -285,19 +309,6 @@ export class WorktreeDiffController { } } - private async ensureTarget(sessionId: string): Promise { - if (this.controller.currentId !== sessionId) return undefined - if (this.target?.sessionId === sessionId) return this.target - return await this.resolveTarget(sessionId) - } - - private async resolveTarget(sessionId: string): Promise { - const target = await this.resolve(sessionId) - if (!target) return undefined - this.target = { sessionId, ...target } - return this.target - } - private postRevertResult(sessionId: string, file: string, result: { ok: boolean; message: string }): void { this.ctx.post({ type: "agentManager.revertWorktreeFileResult", diff --git a/packages/kilo-vscode/src/diff/sources/catalog.ts b/packages/kilo-vscode/src/diff/sources/catalog.ts index fa36aaecf2d..608d4580afc 100644 --- a/packages/kilo-vscode/src/diff/sources/catalog.ts +++ b/packages/kilo-vscode/src/diff/sources/catalog.ts @@ -90,12 +90,13 @@ export class DiffSourceCatalog implements vscode.Disposable { } build(id: string, ctx: PanelContext): DiffSource { + const opts = { dir: () => ctx.dir, strictDir: ctx.strictDir, git: ctx.git, log: ctx.log } if (id === WORKSPACE_SOURCE_ID) { - return createWorktreeDiffSource({ baseBranchOverride: ctx.baseBranchOverride }) + return createWorktreeDiffSource({ ...opts, baseBranchOverride: ctx.baseBranchOverride, baseBranch: ctx.baseBranch }) } - if (id === STAGED_SOURCE_ID) return createStagedDiffSource() - if (id === UNSTAGED_SOURCE_ID) return createUnstagedDiffSource() + if (id === STAGED_SOURCE_ID) return createStagedDiffSource(opts) + if (id === UNSTAGED_SOURCE_ID) return createUnstagedDiffSource(opts) if (id.startsWith(TURN_PREFIX)) { const [sessionId, messageId] = id.slice(TURN_PREFIX.length).split(":") @@ -108,14 +109,14 @@ export class DiffSourceCatalog implements vscode.Disposable { if (id.startsWith(SESSION_PREFIX)) { const sessionId = id.slice(SESSION_PREFIX.length) if (!sessionId) throw new Error(`DiffSourceCatalog.build: empty session id in "${id}"`) - return createSessionDiffSource(sessionId, this.sessionFetch, ctx.workspaceRoot, this.checkSnapshotsEnabled) + return createSessionDiffSource(sessionId, this.sessionFetch, ctx.dir ?? ctx.workspaceRoot, this.checkSnapshotsEnabled) } throw new Error(`DiffSourceCatalog.build: unknown source id "${id}"`) } - async listWorkspaceBranches(override: string | undefined): Promise { - const root = getWorkspaceRoot() + async listWorkspaceBranches(override: string | undefined, dir?: string): Promise { + const root = dir ?? getWorkspaceRoot() if (!root) return undefined const git = this.ensureBranchGit() diff --git a/packages/kilo-vscode/src/diff/sources/staged.ts b/packages/kilo-vscode/src/diff/sources/staged.ts index d21a508902b..8cd250743cb 100644 --- a/packages/kilo-vscode/src/diff/sources/staged.ts +++ b/packages/kilo-vscode/src/diff/sources/staged.ts @@ -34,17 +34,39 @@ function stamp(entry: FileEntry, before: string, after: string): FileEntry { return { ...entry, stamp: `${entry.status}:${before}:${after}` } } +export interface StagedDiffSourceOptions { + /** + * Resolve the directory to diff. Defaults to the VS Code workspace root. + * Agent Manager passes a worktree path so the source diffs inside the + * worktree rather than the main checkout. + */ + dir?: () => string | undefined + /** + * When true, a `dir` that resolves to undefined yields an empty diff rather + * than falling back to the workspace root. + */ + strictDir?: boolean + /** Shared GitOps / log so sources don't each spawn their own channel. */ + git?: GitOps + log?: (...args: unknown[]) => void +} + /** * Diff between the git index and HEAD — what `git diff --cached` would show. * Polls on the standard interval; revert isn't supported (use `git reset` from * a real git client). Read-only view. */ -export function createStagedDiffSource(): DiffSource { - const output = vscode.window.createOutputChannel("Kilo Diff: Staged") - const log = (...args: unknown[]) => appendOutput(output, "StagedDiffSource", ...args) - const git = new GitOps({ log }) +export function createStagedDiffSource(opts: StagedDiffSourceOptions = {}): DiffSource { + const output = opts.git ? undefined : vscode.window.createOutputChannel("Kilo Diff: Staged") + const log = opts.log ?? ((...args: unknown[]) => appendOutput(output!, "StagedDiffSource", ...args)) + const git = opts.git ?? new GitOps({ log }) - const root = (): string | undefined => getWorkspaceRoot() + const root = (): string | undefined => { + const dir = opts.dir?.() + if (dir) return dir + if (opts.strictDir) return undefined + return getWorkspaceRoot() + } const listEntries = async (dir: string): Promise => { const [nameStatus, numstat, raw] = await Promise.all([ @@ -150,8 +172,10 @@ export function createStagedDiffSource(): DiffSource { }, dispose(): void { - git.dispose() - output.dispose() + // Only dispose resources we own (created here). Injected git/log are + // owned by the caller. + if (!opts.git) git.dispose() + output?.dispose() }, } } diff --git a/packages/kilo-vscode/src/diff/sources/unstaged.ts b/packages/kilo-vscode/src/diff/sources/unstaged.ts index b4d0c996161..5fcb03793d2 100644 --- a/packages/kilo-vscode/src/diff/sources/unstaged.ts +++ b/packages/kilo-vscode/src/diff/sources/unstaged.ts @@ -40,17 +40,39 @@ function stamp(entry: FileEntry, before: string, after: string): FileEntry { return { ...entry, stamp: `${entry.status}:${before}:${after}` } } +export interface UnstagedDiffSourceOptions { + /** + * Resolve the directory to diff. Defaults to the VS Code workspace root. + * Agent Manager passes a worktree path so the source diffs inside the + * worktree rather than the main checkout. + */ + dir?: () => string | undefined + /** + * When true, a `dir` that resolves to undefined yields an empty diff rather + * than falling back to the workspace root. + */ + strictDir?: boolean + /** Shared GitOps / log so sources don't each spawn their own channel. */ + git?: GitOps + log?: (...args: unknown[]) => void +} + /** * Diff between the working tree and the index — what `git diff` shows for * tracked files, plus untracked files (treated as fully-added). Read-only; * polls on the standard interval. */ -export function createUnstagedDiffSource(): DiffSource { - const output = vscode.window.createOutputChannel("Kilo Diff: Unstaged") - const log = (...args: unknown[]) => appendOutput(output, "UnstagedDiffSource", ...args) - const git = new GitOps({ log }) +export function createUnstagedDiffSource(opts: UnstagedDiffSourceOptions = {}): DiffSource { + const output = opts.git ? undefined : vscode.window.createOutputChannel("Kilo Diff: Unstaged") + const log = opts.log ?? ((...args: unknown[]) => appendOutput(output!, "UnstagedDiffSource", ...args)) + const git = opts.git ?? new GitOps({ log }) - const root = (): string | undefined => getWorkspaceRoot() + const root = (): string | undefined => { + const dir = opts.dir?.() + if (dir) return dir + if (opts.strictDir) return undefined + return getWorkspaceRoot() + } const listTracked = async (dir: string): Promise => { const [nameStatus, numstat, raw] = await Promise.all([ @@ -192,8 +214,10 @@ export function createUnstagedDiffSource(): DiffSource { }, dispose(): void { - git.dispose() - output.dispose() + // Only dispose resources we own (created here). Injected git/log are + // owned by the caller. + if (!opts.git) git.dispose() + output?.dispose() }, } } diff --git a/packages/kilo-vscode/src/diff/sources/worktree.ts b/packages/kilo-vscode/src/diff/sources/worktree.ts index 1f3d8ee25a4..89e750a1c87 100644 --- a/packages/kilo-vscode/src/diff/sources/worktree.ts +++ b/packages/kilo-vscode/src/diff/sources/worktree.ts @@ -23,6 +23,28 @@ export interface WorktreeDiffSourceOptions { * the current branch — only the comparison target changes. Reset on dispose. */ baseBranchOverride?: string + /** + * Resolve the directory to diff. Defaults to the VS Code workspace root. + * Agent Manager passes a worktree path so the source diffs inside the + * worktree rather than the main checkout. + */ + dir?: () => string | undefined + /** + * When true, a `dir` that resolves to undefined yields an empty diff rather + * than falling back to the workspace root. Prevents an unresolvable + * worktree context from silently diffing the main checkout. + */ + strictDir?: boolean + /** + * Explicit base branch to diff against. When set, the source skips + * auto-resolution (tracking → default) and diffs against this ref directly. + * Agent Manager passes the worktree's recorded parent so a worktree always + * compares against its own base even when the workspace default differs. + */ + baseBranch?: string + /** Shared GitOps / log so sources don't each spawn their own channel. */ + git?: GitOps + log?: (...args: unknown[]) => void } /** @@ -32,9 +54,16 @@ export interface WorktreeDiffSourceOptions { * extension host — no `kilo serve` round-trip. */ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): DiffSource { - const output = vscode.window.createOutputChannel("Kilo Diff: Workspace") - const log = (...args: unknown[]) => appendOutput(output, "WorktreeDiffSource", ...args) - const git = new GitOps({ log }) + const output = opts.git ? undefined : vscode.window.createOutputChannel("Kilo Diff: Workspace") + const log = opts.log ?? ((...args: unknown[]) => appendOutput(output!, "WorktreeDiffSource", ...args)) + const git = opts.git ?? new GitOps({ log }) + + const root = (): string | undefined => { + const dir = opts.dir?.() + if (dir) return dir + if (opts.strictDir) return undefined + return getWorkspaceRoot() + } // Cached between fetches so repeated polling doesn't re-resolve the base // branch every tick. Reset only on dispose (when the source is swapped out). @@ -42,22 +71,32 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): const resolveTarget = async (): Promise => { if (target) return target + if (opts.baseBranch) { + const dir = root() + if (!dir) { + log("Local diff: no directory (explicit base mode)") + return + } + target = { directory: dir, baseBranch: opts.baseBranch } + log(`Local diff: using explicit base=${opts.baseBranch} dir=${dir}`) + return target + } if (opts.baseBranchOverride) { - const root = getWorkspaceRoot() - if (!root) { + const dir = root() + if (!dir) { log("Local diff: no workspace root (override mode)") return } - const resolved = await resolveOverrideRef(git, root, opts.baseBranchOverride, log) + const resolved = await resolveOverrideRef(git, dir, opts.baseBranchOverride, log) if (!resolved) { log(`Local diff: override base="${opts.baseBranchOverride}" could not be resolved, falling back to auto`) } else { - target = { directory: root, baseBranch: resolved } + target = { directory: dir, baseBranch: resolved } log(`Local diff: using override base=${resolved}`) return target } } - target = await resolveLocalDiffTarget(git, log, getWorkspaceRoot()) + target = await resolveLocalDiffTarget(git, log, root()) return target } @@ -109,8 +148,10 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): }, dispose(): void { - git.dispose() - output.dispose() + // Only dispose resources we own (created here). Injected git/log are + // owned by the caller. + if (!opts.git) git.dispose() + output?.dispose() target = undefined }, } diff --git a/packages/kilo-vscode/src/diff/types.ts b/packages/kilo-vscode/src/diff/types.ts index 913d4560a31..3ff72904e13 100644 --- a/packages/kilo-vscode/src/diff/types.ts +++ b/packages/kilo-vscode/src/diff/types.ts @@ -10,6 +10,27 @@ export interface PanelContext { hidePicker?: boolean /** User-picked base branch for the workspace source. Undefined = auto. */ baseBranchOverride?: string + /** + * Explicit directory to diff inside, overriding the workspace root lookup. + * Agent Manager passes a worktree path so its sources operate in the + * worktree rather than the main checkout. + */ + dir?: string + /** + * When true, a source whose `dir` resolves to undefined returns an empty + * diff instead of falling back to the workspace root. Agent Manager sets + * this so an unresolvable worktree context never silently diffs the main + * checkout. + */ + strictDir?: boolean + /** + * Explicit base ref for the workspace source, skipping auto-resolution. + * Agent Manager passes the worktree's recorded parent ref. + */ + baseBranch?: string + /** Shared GitOps / log injected by Agent Manager to avoid per-source channels. */ + git?: import("../agent-manager/GitOps").GitOps + log?: (...args: unknown[]) => void } export type DiffImageError = "too-large" | "unreadable" diff --git a/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts b/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts index 804471f9bb0..10b88670865 100644 --- a/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts +++ b/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts @@ -570,7 +570,9 @@ describe("Agent Manager Provider — onMessage routing", () => { expect(text).toContain("class WorktreeDiffController") expect(text).toContain("buildWorktreePatch") expect(text).toContain("revertFile") - expect(text).toContain("diffSummary") + // Summary/detail diff data comes from the shared DiffSourceCatalog sources + // (workspace/staged/unstaged/session), not a bespoke in-controller pipeline. + expect(text).toContain("catalog.build") expect(text).toContain("shouldStopDiffPolling") expect(providerText).toContain("this.diffs") }) diff --git a/packages/kilo-vscode/tests/unit/diff-scope.test.ts b/packages/kilo-vscode/tests/unit/diff-scope.test.ts new file mode 100644 index 00000000000..7714ae4fa24 --- /dev/null +++ b/packages/kilo-vscode/tests/unit/diff-scope.test.ts @@ -0,0 +1,52 @@ +import { describe, it, expect } from "bun:test" +import { + composeDiffId, + parseDiffId, + isDiffScope, + normalizeScope, + scopeToSourceId, + DEFAULT_DIFF_SCOPE, +} from "../../src/agent-manager/diff-scope" + +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" }) + }) + + it("parses session ids containing no separator as default branch scope", () => { + expect(parseDiffId("ses_abc")).toEqual({ ctx: "ses_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 + // full id as context and falls back to branch. + expect(parseDiffId("ses_a#bogus")).toEqual({ ctx: "ses_a#bogus", scope: DEFAULT_DIFF_SCOPE }) + }) + + it("isDiffScope guards the closed enum", () => { + expect(isDiffScope("branch")).toBe(true) + expect(isDiffScope("staged")).toBe(true) + expect(isDiffScope("unstaged")).toBe(true) + expect(isDiffScope("session")).toBe(true) + expect(isDiffScope("turn")).toBe(false) + expect(isDiffScope("")).toBe(false) + }) + + it("normalizeScope falls back to branch for unknown input", () => { + expect(normalizeScope("staged")).toBe("staged") + expect(normalizeScope("nope")).toBe("branch") + expect(normalizeScope(undefined)).toBe("branch") + expect(normalizeScope(42)).toBe("branch") + }) + + 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", "local")).toBe("workspace") + }) +}) diff --git a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx index ce6f795941b..ac86e26d1a1 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx @@ -20,9 +20,6 @@ import type { AgentManagerMultiVersionProgressMessage, AgentManagerSendInitialMessage, AgentManagerBranchesMessage, - AgentManagerWorktreeDiffMessage, - AgentManagerWorktreeDiffFileMessage, - AgentManagerWorktreeDiffLoadingMessage, AgentManagerApplyWorktreeDiffResultMessage, AgentManagerApplyWorktreeDiffStatus, AgentManagerApplyWorktreeDiffConflict, @@ -156,7 +153,10 @@ import { } from "./section-helpers" import { sectionAwareDetector } from "./section-dnd" import { ConstrainDragXAxis } from "./constrain-drag-x" -import { mergeWorktreeDiffs } from "../diff-viewer/diff-state" +import { DiffScopeControls } from "../diff-viewer/DiffScopeControls" +import { scopeCapabilities } from "./diff-scope-state" +import { createDiffReviewScope } from "./diff-review-scope" +import { handleDiffMessage } from "./diff-messages" import { initialMessage, seedInitialVariant } from "./initial-message" import { createMarkdownRender } from "./review-preferences" import { createSidebarCollapse } from "./sidebar-collapse" @@ -1491,40 +1491,13 @@ const AgentManagerContent: Component = () => { } } - if (msg.type === "agentManager.worktreeDiff") { - const ev = msg as AgentManagerWorktreeDiffMessage - let staleFiles: Set | undefined - setDiffDatas((prev) => { - const existing = prev[ev.sessionId] - const merged = existing - ? mergeWorktreeDiffs(existing, ev.diffs) - : { diffs: ev.diffs, stale: new Set() } - staleFiles = merged.stale - const next = merged.diffs - if (existing && existing.length === next.length && existing.every((old, i) => old === next[i])) return prev - return { ...prev, [ev.sessionId]: next } - }) - if (staleFiles) refreshStaleDiffs(ev.sessionId, staleFiles) - } - - if (msg.type === "agentManager.worktreeDiffFile") { - const ev = msg as AgentManagerWorktreeDiffFileMessage - if (ev.diff) { - setDiffDatas((prev) => { - const existing = prev[ev.sessionId] ?? [] - const next = existing.map((item) => (item.file === ev.diff!.file ? ev.diff! : item)) - return { ...prev, [ev.sessionId]: next } - }) - setDiffFilePending(ev.sessionId, ev.diff.file, false) - return - } - setDiffFilePending(ev.sessionId, ev.file, false) - } - - if (msg.type === "agentManager.worktreeDiffLoading") { - const ev = msg as AgentManagerWorktreeDiffLoadingMessage - setDiffLoading(ev.loading) - } + handleDiffMessage(msg, { + setDiffDatas, + setDiffFilePending, + setDiffLoading, + refreshStaleDiffs, + review, + }) if (msg.type === "agentManager.applyWorktreeDiffResult") { const ev = msg as AgentManagerApplyWorktreeDiffResultMessage @@ -1617,15 +1590,47 @@ const AgentManagerContent: Component = () => { const currentDiffSessionId = createMemo(selectedDiffSessionId) - // Start/stop diff watch when panel opens/closes, review tab opens, or session changes + // Diff scope + base branch state, shared by the side panel and review tab. + const review = createDiffReviewScope({ + ctx: currentDiffSessionId, + panelOpen: diffOpen, + reviewActive, + local: LOCAL, + vscode, + }) + // The composite id (ctx#scope) the extension keys diff data by. + const diffScopeId = review.id + + // Shared scope + base-picker controls for the side panel and review tab. + const diffScopeControls = (compact: boolean) => ( + + ) + + // Start/stop diff watch when panel opens/closes, review tab opens, scope + // changes, or session changes. createEffect(() => { const panel = diffOpen() - const review = reviewActive() + const active = reviewActive() + const scope = review.scope() - if (panel || review) { + if (panel || active) { const id = currentDiffSessionId() if (id) { - vscode.postMessage({ type: "agentManager.startDiffWatch", sessionId: id }) + vscode.postMessage({ type: "agentManager.startDiffWatch", sessionId: id, scope }) return } vscode.postMessage({ type: "agentManager.stopDiffWatch" }) @@ -1670,33 +1675,17 @@ const AgentManagerContent: Component = () => { tabFocus.restore() } - // Data for the review tab: use local diff data for local context, - // current session for selected worktree context, or first available in that worktree. + // Data for the review tab / side panel: keyed by the composite diff id + // (ctx#scope) the extension pushes, so each scope keeps its own file set and + // switching back to a fetched scope is instant. const reviewDiffs = createMemo(() => { const data = diffDatas() - const sel = selection() - const id = session.currentSessionID() - if (sel === LOCAL) return data[LOCAL] ?? [] - if (id && data[id]) { - const current = managedSessions().find((s) => s.id === id) - if (sel && current?.worktreeId === sel) return data[id]! - } - if (!sel) return [] - const ids = managedSessions() - .filter((s) => s.worktreeId === sel) - .map((s) => s.id) - for (const sid of ids) { - if (data[sid]) return data[sid]! - } - return [] + const key = diffScopeId() + if (!key) return [] + return data[key] ?? [] }) - const diffSessionKey = createMemo(() => { - const sel = selection() - if (sel === LOCAL) return `local:${LOCAL}` - if (sel === null) return `session:${session.currentSessionID() ?? ""}` - return `worktree:${sel}` - }) + const diffSessionKey = createMemo(() => diffScopeId() ?? "") const setSharedDiffStyle = (style: "unified" | "split") => { if (reviewDiffStyle() === style) return @@ -1731,29 +1720,39 @@ const AgentManagerContent: Component = () => { } const requestDiffFile = (file: string) => { - const sessionId = currentDiffSessionId() - if (!sessionId) return - if (diffFileLoading()[sessionId]?.[file]) return - setDiffFilePending(sessionId, file, true) - vscode.postMessage({ type: "agentManager.requestWorktreeDiffFile", sessionId, file }) + const id = diffScopeId() + if (!id) return + if (diffFileLoading()[id]?.[file]) return + setDiffFilePending(id, file, true) + vscode.postMessage({ + type: "agentManager.requestWorktreeDiffFile", + sessionId: currentDiffSessionId()!, + file, + scope: review.scope(), + }) } - const refreshStaleDiffs = (sessionId: string, files: Set) => { - const loading = diffFileLoading()[sessionId] ?? {} + const refreshStaleDiffs = (id: string, files: Set) => { + const loading = diffFileLoading()[id] ?? {} for (const file of files) { if (loading[file]) continue - setDiffFilePending(sessionId, file, true) - vscode.postMessage({ type: "agentManager.requestWorktreeDiffFile", sessionId, file }) + setDiffFilePending(id, file, true) + vscode.postMessage({ + type: "agentManager.requestWorktreeDiffFile", + sessionId: currentDiffSessionId()!, + file, + scope: review.scope(), + }) } } const diffFileLoadingForCurrent = createMemo(() => { - const sessionId = currentDiffSessionId() - if (!sessionId) return new Set() - return new Set(Object.keys(diffFileLoading()[sessionId] ?? {})) + const id = diffScopeId() + if (!id) return new Set() + return new Set(Object.keys(diffFileLoading()[id] ?? {})) }) - const revertCtl = createRevertFile(currentDiffSessionId, vscode, showToast, t) + const revertCtl = createRevertFile(diffScopeId, currentDiffSessionId, () => review.scope(), vscode, showToast, t) const handleConfigureSetupScript = () => { vscode.postMessage({ type: "agentManager.configureSetupScript" }) @@ -2755,12 +2754,19 @@ const AgentManagerContent: Component = () => { {t("agentManager.open.button")} - + + ) + return ( - = 2}> + = 2}>