diff --git a/src/vs/platform/agentHost/common/changesetUri.ts b/src/vs/platform/agentHost/common/changesetUri.ts index 7a6476496e1..d42c59320dd 100644 --- a/src/vs/platform/agentHost/common/changesetUri.ts +++ b/src/vs/platform/agentHost/common/changesetUri.ts @@ -38,11 +38,14 @@ const TURN_CHANGESET_PREFIX = 'turn/'; const TURN_TEMPLATE_VARIABLE = '{turnId}'; /** Localized human-readable label for the session-wide changeset entry. */ -export const sessionChangesetLabel = (): string => localize('sessionChangeset.label', "Session Changes"); +export const sessionChangesetLabel = (): string => localize('branchChangeset.label', "Branch Changes"); /** Localized human-readable label for the uncommitted-changes changeset entry. */ export const uncommittedChangesetLabel = (): string => localize('uncommittedChangeset.label', "Uncommitted Changes"); +/** Localized human-readable description for the uncommitted-changes changeset entry. */ +export const uncommittedChangesetDescription = (): string => localize('uncommittedChangeset.description', "Show uncommitted changes in this session"); + /** Localized human-readable label for the per-turn changeset template entry. */ export const thisTurnChangesetLabel = (): string => localize('thisTurnChangeset.label', "This Turn"); @@ -160,18 +163,20 @@ export function parseTurnChangesetUri(uri: URI): { sessionUri: URI; turnId: stri /** * Builds the default ordered `summary.changesets` catalogue for a - * session (`Uncommitted Changes`, `Session Changes`, `This Turn`) with + * session (`Branch Changes`, `Uncommitted Changes`, `This Turn`) with * label + uriTemplate only. Aggregate counts are filled in later by the - * diff producer as compute passes complete; clients MUST treat - * `summary.changesets[0]` as the default rather than singling out an id. + * diff producer as compute passes complete. * - * Catalogue shape is immutable for the session's lifetime — only the - * per-entry stats update over time. + * The first two entries (`Branch Changes`, `Uncommitted Changes`) are + * git-only; `AgentService._attachGitState` strips them asynchronously + * for sessions whose working directory is not a git repo (or absent). + * The backing per-changeset states are still registered for every + * session — only the catalogue advertisements are stripped. */ export function buildDefaultChangesetCatalogue(sessionUri: URI): ChangesetSummary[] { return [ - { label: uncommittedChangesetLabel(), uriTemplate: buildUncommittedChangesetUri(sessionUri) }, { label: sessionChangesetLabel(), uriTemplate: buildSessionChangesetUri(sessionUri) }, + { label: uncommittedChangesetLabel(), uriTemplate: buildUncommittedChangesetUri(sessionUri), description: uncommittedChangesetDescription() }, { label: thisTurnChangesetLabel(), uriTemplate: buildTurnChangesetUriTemplate(sessionUri) }, ]; } diff --git a/src/vs/platform/agentHost/node/agentHostChangesetCoordinator.ts b/src/vs/platform/agentHost/node/agentHostChangesetCoordinator.ts index 01eece738bb..f2532c34333 100644 --- a/src/vs/platform/agentHost/node/agentHostChangesetCoordinator.ts +++ b/src/vs/platform/agentHost/node/agentHostChangesetCoordinator.ts @@ -70,12 +70,32 @@ export class ChangesetSessionCoordinator extends Disposable { */ private readonly _pendingUncommittedRefreshes = new Set(); + /** + * Per-session set of turn ids that have at least one live subscriber to + * `/changeset/turn/`. Drives the per-turn recompute + * gating: the changeset service only schedules a per-turn recompute when + * this set says someone is watching the turn URI (per-turn URIs have no + * catalogue chip aggregates, so recomputing for an unobserved turn is + * pure waste). + */ + private readonly _subscribedTurns = new Map>(); + constructor( private readonly _stateManager: AgentHostStateManager, private readonly _changesets: IAgentHostChangesetService, private readonly _configurationService: IAgentConfigurationService, ) { super(); + this._changesets.setTurnSubscriberProbe((session, turnId) => this.hasTurnSubscribers(session, turnId)); + } + + /** + * Returns `true` when at least one client is subscribed to + * `/changeset/turn/`. Consulted by the changeset + * service via the probe installed in the constructor. + */ + hasTurnSubscribers(session: string, turnId: string): boolean { + return this._subscribedTurns.get(session)?.has(turnId) ?? false; } // ---- Lifecycle hooks ---------------------------------------------------- @@ -131,6 +151,7 @@ export class ChangesetSessionCoordinator extends Disposable { */ onSessionDisposed(sessionStr: string): void { this._pendingUncommittedRefreshes.delete(sessionStr); + this._subscribedTurns.delete(sessionStr); } // ---- Subscription hooks ------------------------------------------------- @@ -145,9 +166,42 @@ export class ChangesetSessionCoordinator extends Disposable { * `addSubscriber`, so this single hook covers both paths. */ onFirstSubscriber(resource: URI): void { - const parsed = parseChangesetUri(resource.toString()); + const resourceStr = resource.toString(); + const parsed = parseChangesetUri(resourceStr); if (parsed?.kind === ChangesetKind.Uncommitted) { this._triggerUncommittedRefresh(parsed.sessionUri); + return; + } + if (parsed?.kind === ChangesetKind.Session) { + // Session-changeset compute uses git when a working dir is + // available and falls back to the SDK edit-tracker otherwise, + // so it doesn't need the same deferral as uncommitted. + this._changesets.refreshSessionChangeset(parsed.sessionUri); + return; + } + if (parsed?.kind === ChangesetKind.Turn && parsed.turnId !== undefined) { + // Track the new subscriber so the service's per-turn recompute + // gating starts including this turn. The initial snapshot is + // already produced by `tryHandleSubscribe → computeTurnChangeset`; + // subsequent deltas flow from `onToolCallEditsApplied` / + // `onTurnComplete` once we've added this turn id here. + let set = this._subscribedTurns.get(parsed.sessionUri); + if (!set) { + set = new Set(); + this._subscribedTurns.set(parsed.sessionUri, set); + } + set.add(parsed.turnId); + return; + } + if (!parsed && this._stateManager.getSessionState(resourceStr)) { + // Plain session-URI subscription (Agents Window list / detail + // observing the session). Refresh both static changesets so + // the catalogue chip doesn't show a stale value just because + // no turn has run since process start, no one ever subscribed + // to the changeset URIs directly, and the user has been + // editing files manually in the working tree. + this._triggerUncommittedRefresh(resourceStr); + this._changesets.refreshSessionChangeset(resourceStr); } } @@ -160,6 +214,16 @@ export class ChangesetSessionCoordinator extends Disposable { const parsed = parseChangesetUri(resource.toString()); if (parsed?.kind === ChangesetKind.Uncommitted) { this._pendingUncommittedRefreshes.delete(parsed.sessionUri); + return; + } + if (parsed?.kind === ChangesetKind.Turn && parsed.turnId !== undefined) { + const set = this._subscribedTurns.get(parsed.sessionUri); + if (set) { + set.delete(parsed.turnId); + if (set.size === 0) { + this._subscribedTurns.delete(parsed.sessionUri); + } + } } } diff --git a/src/vs/platform/agentHost/node/agentHostChangesetService.ts b/src/vs/platform/agentHost/node/agentHostChangesetService.ts index 440b01f8b04..573bbb62a30 100644 --- a/src/vs/platform/agentHost/node/agentHostChangesetService.ts +++ b/src/vs/platform/agentHost/node/agentHostChangesetService.ts @@ -17,6 +17,7 @@ import { sessionChangesetLabel, thisTurnChangesetLabel, uncommittedChangesetLabel, + uncommittedChangesetDescription, } from '../common/changesetUri.js'; import { IDiffComputeService } from '../common/diffComputeService.js'; import { ISessionDatabase, ISessionDataService } from '../common/sessionDataService.js'; @@ -65,11 +66,13 @@ function persistKeyFor(kind: StaticChangesetKind): string { /** * Builds a single static {@link ChangesetSummary} catalogue entry from a * persisted (or live-state-derived) file list. Returns the bare entry - * (no counts) when `diffs` is undefined. + * (no counts) when `diffs` is undefined. Optional `description` is + * threaded through when provided. */ -function buildStaticCatalogueEntry(label: string, uri: string, diffs: readonly ISessionFileDiff[] | undefined): ChangesetSummary { +function buildStaticCatalogueEntry(label: string, uri: string, diffs: readonly ISessionFileDiff[] | undefined, description?: string): ChangesetSummary { + const base: ChangesetSummary = description ? { label, uriTemplate: uri, description } : { label, uriTemplate: uri }; if (!diffs) { - return { label, uriTemplate: uri }; + return base; } let additions = 0; let deletions = 0; @@ -77,7 +80,7 @@ function buildStaticCatalogueEntry(label: string, uri: string, diffs: readonly I additions += d.diff?.added ?? 0; deletions += d.diff?.removed ?? 0; } - return { label, uriTemplate: uri, additions, deletions, files: diffs.length }; + return { ...base, additions, deletions, files: diffs.length }; } function defaultCatalogueWithCounts( @@ -86,15 +89,15 @@ function defaultCatalogueWithCounts( sessionDiffs: readonly ISessionFileDiff[] | undefined, ): ChangesetSummary[] { return [ - buildStaticCatalogueEntry(uncommittedChangesetLabel(), buildUncommittedChangesetUri(sessionUri), uncommittedDiffs), buildStaticCatalogueEntry(sessionChangesetLabel(), buildSessionChangesetUri(sessionUri), sessionDiffs), + buildStaticCatalogueEntry(uncommittedChangesetLabel(), buildUncommittedChangesetUri(sessionUri), uncommittedDiffs, uncommittedChangesetDescription()), { label: thisTurnChangesetLabel(), uriTemplate: buildTurnChangesetUriTemplate(sessionUri) }, ]; } /** - * Build the default ordered changeset catalogue (`Uncommitted Changes`, - * `Session Changes`, `This Turn`) seeded from the live {@link ChangesetState} + * Build the default ordered changeset catalogue (`Branch Changes`, + * `Uncommitted Changes`, `This Turn`) seeded from the live {@link ChangesetState} * for an unopened session that has no live `SessionState` but already has * ready changeset states (e.g. from a prior `restoreStaticChangeset` call). * @@ -103,8 +106,11 @@ function defaultCatalogueWithCounts( * have no usable counts yet — preserving the long-standing contract that * unopened sessions without persisted or live data advertise no catalogue. * - * Clients MUST treat `summary.changesets[0]` as the default — `Uncommitted - * Changes` is first by virtue of catalogue ordering, not by hardcoded id. + * The two static entries (`Branch Changes`, `Uncommitted Changes`) are + * git-only — `AgentService._attachGitState` strips them from the live + * `summary.changesets` for non-git working directories. The synthesised + * catalogue here mirrors the live-state shape so list overlays stay + * consistent with the per-session catalogue clients subscribe to. */ export function buildCatalogueFromLiveState( sessionUri: string, @@ -243,6 +249,15 @@ export interface IAgentHostChangesetService { */ refreshUncommittedChangeset(session: ProtocolURI): void; + /** + * Lazy refresh of the session (branch) changeset, kicked off when a + * client first subscribes to `/changeset/session` or the + * session URI itself (e.g. Agents Window observing the session). Mirrors + * {@link refreshUncommittedChangeset} so the catalogue chip stays fresh + * across session opens even when no turn has run since process start. + */ + refreshSessionChangeset(session: ProtocolURI): void; + /** * Computes and publishes the per-turn changeset for `turnId` on `session`. * Per-turn changesets are not persisted. @@ -268,6 +283,14 @@ export interface IAgentHostChangesetService { * `changedTurnId`, no incremental reuse). */ onSessionTruncated(session: ProtocolURI): void; + + /** + * Installs a predicate the service consults before scheduling a + * per-turn changeset recompute. Owned by {@link ChangesetSessionCoordinator}, + * which tracks per-turn subscribers via `onFirstSubscriber` / + * `onLastSubscriber`. Called exactly once at coordinator construction. + */ + setTurnSubscriberProbe(probe: (session: ProtocolURI, turnId: string) => boolean): void; } export class AgentHostChangesetService extends Disposable implements IAgentHostChangesetService { @@ -279,8 +302,24 @@ export class AgentHostChangesetService extends Disposable implements IAgentHostC private readonly _diffComputationSequencer = new SequencerByKey(); /** Per-session debounce timers for mid-turn diff computation. */ private readonly _debouncedDiffTimers = this._register(new DisposableMap()); + /** Per-`(session, turnId)` debounce timers for mid-turn per-turn changeset recomputation. */ + private readonly _perTurnDebouncedDiffTimers = this._register(new DisposableMap()); private static readonly _DIFF_DEBOUNCE_MS = 5000; + /** + * Subscriber probe set by {@link ChangesetSessionCoordinator}. Returns + * `true` when at least one client is subscribed to + * `/changeset/turn/`. Per-turn URIs carry no catalogue + * chip aggregates, so recomputing for an unobserved turn is pure waste + * — the service consults this probe in {@link onToolCallEditsApplied} + * and {@link onTurnComplete} before scheduling a per-turn recompute. + * + * Defaults to `() => false` so unwired test instances don't accidentally + * fire per-turn computes; the coordinator overrides this in its + * constructor. + */ + private _hasTurnSubscribers: (session: ProtocolURI, turnId: string) => boolean = () => false; + constructor( private readonly _stateManager: AgentHostStateManager, @ILogService private readonly _logService: ILogService, @@ -291,6 +330,10 @@ export class AgentHostChangesetService extends Disposable implements IAgentHostC this._diffComputeService = this._register(new NodeWorkerDiffComputeService(this._logService)); } + setTurnSubscriberProbe(probe: (session: ProtocolURI, turnId: string) => boolean): void { + this._hasTurnSubscribers = probe; + } + registerStaticChangesets(session: ProtocolURI): void { this._stateManager.registerChangeset(buildUncommittedChangesetUri(session)); this._stateManager.registerChangeset(buildSessionChangesetUri(session)); @@ -334,6 +377,10 @@ export class AgentHostChangesetService extends Disposable implements IAgentHostC this._scheduleStaticRecompute(session, 'uncommitted'); } + refreshSessionChangeset(session: ProtocolURI): void { + this._scheduleStaticRecompute(session, 'session'); + } + async computeTurnChangeset(session: ProtocolURI, turnId: string): Promise { const turnUri = this._stateManager.registerChangeset(buildTurnChangesetUri(session, turnId)); let ref: ReturnType; @@ -370,14 +417,28 @@ export class AgentHostChangesetService extends Disposable implements IAgentHostC onToolCallEditsApplied(session: ProtocolURI, turnId: string): void { this._scheduleDebouncedDiffComputation(session, turnId); + // Per-turn URIs have no catalogue chip aggregates, so skip the + // recompute entirely when no client is observing this turn. The + // next subscriber will get a fresh snapshot from + // `tryHandleSubscribe → computeTurnChangeset`. + if (this._hasTurnSubscribers(session, turnId)) { + this._scheduleDebouncedTurnDiffComputation(session, turnId); + } } onTurnComplete(session: ProtocolURI, turnId: string | undefined): void { - // Ordering matters: cancel any pending mid-turn debounce first so - // the final turn-complete compute supersedes it; then schedule the - // session-wide recompute with the changed turn id (incremental - // reuse anchor); then the uncommitted recompute with no turn id. + // Ordering matters for cancellation: cancel any pending mid-turn + // debounces first so the final turn-complete computes supersede + // them. After that, schedule the final recomputes for the turn + // (when observed), the session-wide changeset with the changed + // turn id, and the uncommitted changeset with no turn id. this._cancelDebouncedDiffComputation(session); + if (turnId !== undefined) { + this._cancelDebouncedTurnDiffComputation(session, turnId); + if (this._hasTurnSubscribers(session, turnId)) { + this._scheduleTurnRecompute(session, turnId); + } + } this._scheduleStaticRecompute(session, 'session', turnId); this._scheduleStaticRecompute(session, 'uncommitted'); } @@ -410,6 +471,40 @@ export class AgentHostChangesetService extends Disposable implements IAgentHostC this._debouncedDiffTimers.deleteAndDispose(session); } + /** + * Schedules a debounced per-turn changeset recomputation. Mirrors + * {@link _scheduleDebouncedDiffComputation} but uses a per- + * `(session, turnId)` map key so a long-running per-turn compute + * doesn't block the static session recompute path (and vice versa). + */ + private _scheduleDebouncedTurnDiffComputation(session: ProtocolURI, turnId: string): void { + const key = `${session}\u0000${turnId}`; + this._perTurnDebouncedDiffTimers.set(key, disposableTimeout(() => { + this._perTurnDebouncedDiffTimers.deleteAndDispose(key); + this._scheduleTurnRecompute(session, turnId); + }, AgentHostChangesetService._DIFF_DEBOUNCE_MS)); + } + + /** + * Cancels any pending debounced per-turn diff computation for a + * `(session, turnId)`. Called at turn end before the final + * (non-debounced) per-turn computation. + */ + private _cancelDebouncedTurnDiffComputation(session: ProtocolURI, turnId: string): void { + this._perTurnDebouncedDiffTimers.deleteAndDispose(`${session}\u0000${turnId}`); + } + + /** + * Queues a per-turn recompute on a per-`(session, turnId)` sequencer + * key so back-to-back recomputes for the same turn serialise, but + * recomputes for different turns (or for the static `session` / + * `uncommitted` slots) run independently. Fire-and-forget — failures + * are logged inside `computeTurnChangeset` and do not fail the turn. + */ + private _scheduleTurnRecompute(session: ProtocolURI, turnId: string): void { + this._diffComputationSequencer.queue(`${session}\u0000turn\u0000${turnId}`, () => this.computeTurnChangeset(session, turnId).then(() => undefined)); + } + /** * Schedules a static changeset (`uncommitted` or `session`) recompute, * serialised per-session so back-to-back triggers don't race against diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index 53d94168b1b..d3382f18d3d 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -21,7 +21,7 @@ import { ServiceCollection } from '../../instantiation/common/serviceCollection. import { ILogService } from '../../log/common/log.js'; import { AgentProvider, AgentSession, IAgent, IAgentCreateSessionConfig, IAgentMaterializeSessionEvent, IAgentResolveSessionConfigParams, IAgentService, IAgentSessionConfigCompletionsParams, IAgentSessionMetadata, AuthenticateParams, AuthenticateResult } from '../common/agentService.js'; import { ISessionDataService, SESSION_ATTACHMENTS_DIRNAME } from '../common/sessionDataService.js'; -import { buildDefaultChangesetCatalogue } from '../common/changesetUri.js'; +import { buildDefaultChangesetCatalogue, buildSessionChangesetUri, buildUncommittedChangesetUri } from '../common/changesetUri.js'; import { ActionType, ActionEnvelope, INotification, type IRootConfigChangedAction, type SessionAction, type TerminalAction } from '../common/state/sessionActions.js'; import type { CompletionsParams, CompletionsResult, CreateTerminalParams, ResolveSessionConfigResult, SessionConfigCompletionsResult } from '../common/state/protocol/commands.js'; import { AhpErrorCodes, AHP_SESSION_NOT_FOUND, ContentEncoding, JSON_RPC_INTERNAL_ERROR, ProtocolError, type DirectoryEntry, type ResourceCopyParams, type ResourceCopyResult, type ResourceDeleteParams, type ResourceDeleteResult, type ResourceListResult, type ResourceMoveParams, type ResourceMoveResult, type ResourceReadResult, type ResourceWriteParams, type ResourceWriteResult, type IStateSnapshot } from '../common/state/sessionProtocol.js'; @@ -452,10 +452,12 @@ export class AgentService extends Disposable implements IAgentService { // run before `SessionReady` is dispatched. Any future change must // keep both halves at create time so client subscriptions resolve // to a `status: computing` snapshot rather than a 404 even on - // provisional sessions, and so the catalogue chip renders the three - // default entries (`Uncommitted Changes`, `Session Changes`, - // `This Turn`) immediately. Pinned by item-2 regression tests in - // `agentService.test.ts`. + // provisional sessions, and so the catalogue chip renders the + // default entries (`Branch Changes`, `Uncommitted Changes`, + // `This Turn`) immediately. The first two are git-only: a later + // `_attachGitState` strips them once the git probe confirms the + // resolved working directory is not a git repo. Pinned by item-2 + // regression tests in `agentService.test.ts`. this._changesetCoordinator.onSessionCreated(session.toString()); if (!created.provisional) { @@ -541,17 +543,30 @@ export class AgentService extends Disposable implements IAgentService { * working directory (if any) and merges it into `state._meta.git` via * the state manager. Failures are logged; sessions simply remain without * git state. + * + * Also gates the two git-only default catalogue entries + * (`Branch Changes`, `Uncommitted Changes`): when the working + * directory is resolved AND the git probe confirms it is not a git + * repo, those entries are stripped from `summary.changesets`, leaving + * only `This Turn`. An absent working directory is treated as + * transient (provisional / pre-materialize / pre-restore) — we do NOT + * strip in that case because there is no path that re-adds the + * entries when a subsequent `onSessionMaterialized` / restore call + * resolves the working directory and the probe succeeds. The + * entries' counts remain unset until a real compute lands, so chip + * rendering naturally skips them in the meantime. */ private _attachGitState(session: URI, workingDirectory: URI | undefined): void { if (!workingDirectory) { return; } + const sessionKey = session.toString(); this._gitService.getSessionGitState(workingDirectory).then( gitState => { if (!gitState) { + this._stripGitOnlyChangesetEntries(sessionKey); return; } - const sessionKey = session.toString(); const current = this._stateManager.getSessionState(sessionKey)?._meta; // Skip the action if the computed git state hasn't changed; this is // called after every turn, so deduping avoids needless action churn. @@ -567,6 +582,30 @@ export class AgentService extends Disposable implements IAgentService { ); } + /** + * Drops the `Branch Changes` and `Uncommitted Changes` entries from + * the session's catalogue. Called only when the git probe has + * definitively determined the working directory is not a git repo. + * An absent / unresolved working directory is treated as transient + * and does NOT trigger a strip — see {@link _attachGitState}. + * Backing per-changeset states (registered unconditionally) are left + * in place — only the catalogue advertisements are stripped. + */ + private _stripGitOnlyChangesetEntries(sessionKey: string): void { + const state = this._stateManager.getSessionState(sessionKey); + const current = state?.summary.changesets; + if (!current || current.length === 0) { + return; + } + const branchUri = buildSessionChangesetUri(sessionKey); + const uncommittedUri = buildUncommittedChangesetUri(sessionKey); + const filtered = current.filter(c => c.uriTemplate !== branchUri && c.uriTemplate !== uncommittedUri); + if (filtered.length === current.length) { + return; + } + this._stateManager.setSessionChangesets(sessionKey, filtered); + } + private _persistConfigValues(session: URI, values: Record): void { let ref; try { diff --git a/src/vs/platform/agentHost/test/node/agentHostChangesetService.test.ts b/src/vs/platform/agentHost/test/node/agentHostChangesetService.test.ts index bb11f4bd1e9..4d10d3d30c5 100644 --- a/src/vs/platform/agentHost/test/node/agentHostChangesetService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostChangesetService.test.ts @@ -8,6 +8,7 @@ import { timeout } from '../../../../base/common/async.js'; import { DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js'; import { URI } from '../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { runWithFakedTimers } from '../../../../base/test/common/timeTravelScheduler.js'; import { NullLogService } from '../../../log/common/log.js'; import { AgentSession } from '../../common/agentService.js'; import { buildDefaultChangesetCatalogue } from '../../common/changesetUri.js'; @@ -64,8 +65,8 @@ suite('AgentHostChangesetService', () => { // Catalogue is seeded by setupSession (mirrors what `_buildInitialSummary` // does in production) — sanity check before exercising registration. assert.deepStrictEqual(stateManager.getSessionState(sessionStr)?.summary.changesets, [ - { label: 'Uncommitted Changes', uriTemplate: `${sessionStr}/changeset/uncommitted` }, - { label: 'Session Changes', uriTemplate: `${sessionStr}/changeset/session` }, + { label: 'Branch Changes', uriTemplate: `${sessionStr}/changeset/session` }, + { label: 'Uncommitted Changes', uriTemplate: `${sessionStr}/changeset/uncommitted`, description: 'Show uncommitted changes in this session' }, { label: 'This Turn', uriTemplate: `${sessionStr}/changeset/turn/{turnId}` }, ]); @@ -82,8 +83,8 @@ suite('AgentHostChangesetService', () => { // Registration must not mutate the seeded catalogue. assert.deepStrictEqual(stateManager.getSessionState(sessionStr)?.summary.changesets, [ - { label: 'Uncommitted Changes', uriTemplate: `${sessionStr}/changeset/uncommitted` }, - { label: 'Session Changes', uriTemplate: `${sessionStr}/changeset/session` }, + { label: 'Branch Changes', uriTemplate: `${sessionStr}/changeset/session` }, + { label: 'Uncommitted Changes', uriTemplate: `${sessionStr}/changeset/uncommitted`, description: 'Show uncommitted changes in this session' }, { label: 'This Turn', uriTemplate: `${sessionStr}/changeset/turn/{turnId}` }, ]); }); @@ -127,16 +128,17 @@ suite('AgentHostChangesetService', () => { const catalogue = stateManager.getSessionState(sessionStr)?.summary.changesets; assert.deepStrictEqual(catalogue, [ { - label: 'Uncommitted Changes', - uriTemplate: `${sessionStr}/changeset/uncommitted`, - }, - { - label: 'Session Changes', + label: 'Branch Changes', uriTemplate: changesetUri, additions: 6, deletions: 2, files: 2, }, + { + label: 'Uncommitted Changes', + uriTemplate: `${sessionStr}/changeset/uncommitted`, + description: 'Show uncommitted changes in this session', + }, { label: 'This Turn', uriTemplate: `${sessionStr}/changeset/turn/{turnId}`, @@ -182,16 +184,17 @@ suite('AgentHostChangesetService', () => { ], catalogue: [ { - label: 'Uncommitted Changes', - uriTemplate: `${sessionStr}/changeset/uncommitted`, - }, - { - label: 'Session Changes', + label: 'Branch Changes', uriTemplate: changesetUri, additions: 4, deletions: 1, files: 2, }, + { + label: 'Uncommitted Changes', + uriTemplate: `${sessionStr}/changeset/uncommitted`, + description: 'Show uncommitted changes in this session', + }, { label: 'This Turn', uriTemplate: `${sessionStr}/changeset/turn/{turnId}`, @@ -527,7 +530,7 @@ suite('AgentHostChangesetService', () => { const catalogue = stateManager.getSessionState(sessionStr)?.summary.changesets; const sessionEntry = catalogue?.find(c => c.uriTemplate === `${sessionStr}/changeset/session`); assert.deepStrictEqual(sessionEntry, { - label: 'Session Changes', + label: 'Branch Changes', uriTemplate: `${sessionStr}/changeset/session`, additions: 3, deletions: 0, @@ -535,4 +538,145 @@ suite('AgentHostChangesetService', () => { }, 'catalogue counts must reflect restored files'); }); }); + + suite('per-turn live streaming', () => { + + // Test rig: a subclass that counts `computeTurnChangeset` invocations + // so we can assert gating wiring without needing real session DB + // content for `computeTurnDiffs` to chew on. The base class behaviour + // is preserved (super-call is awaited), so any per-file dispatch the + // production path would emit still flows through normally. + class CountingChangesetService extends AgentHostChangesetService { + readonly turnComputeCalls: { session: string; turnId: string }[] = []; + override async computeTurnChangeset(session: string, turnId: string): Promise { + this.turnComputeCalls.push({ session, turnId }); + return super.computeTurnChangeset(session, turnId); + } + } + + function makeService(): CountingChangesetService { + return disposables.add(new CountingChangesetService( + stateManager, + new NullLogService(), + createNullSessionDataService(), + createNoopGitService(), + )); + } + + test('onTurnComplete schedules a per-turn recompute when the probe says someone is subscribed', async () => { + setupSession(); + const svc = makeService(); + svc.setTurnSubscriberProbe(() => true); + + svc.onTurnComplete(sessionUri.toString(), 'turn-1'); + + // Sequencer drains async; wait briefly for the per-turn call. + for (let i = 0; i < 50 && svc.turnComputeCalls.length === 0; i++) { + await timeout(2); + } + assert.deepStrictEqual( + svc.turnComputeCalls, + [{ session: sessionUri.toString(), turnId: 'turn-1' }], + 'expected exactly one per-turn compute for the completed turn', + ); + }); + + test('onTurnComplete does NOT schedule a per-turn recompute when the probe says nobody is subscribed', async () => { + setupSession(); + const svc = makeService(); + svc.setTurnSubscriberProbe(() => false); + + svc.onTurnComplete(sessionUri.toString(), 'turn-1'); + + // Give the static computes a chance to drain — the per-turn + // call must remain absent throughout. + await timeout(20); + assert.deepStrictEqual(svc.turnComputeCalls, [], 'no per-turn compute when nothing observes the turn URI'); + }); + + test('onToolCallEditsApplied fires the per-turn debounce only when subscribers exist; cancelled by onTurnComplete', () => { + return runWithFakedTimers({ useFakeTimers: true, maxTaskCount: 10_000 }, async () => { + setupSession(); + const svc = makeService(); + svc.setTurnSubscriberProbe(() => true); + + // 1) edits with subscriber -> after debounce, exactly one per-turn compute fires. + svc.onToolCallEditsApplied(sessionUri.toString(), 'turn-1'); + await timeout(6_000); // debounce is 5s + assert.strictEqual(svc.turnComputeCalls.length, 1, 'debounce should fire one per-turn compute'); + + // 2) another edit batch + onTurnComplete before the debounce + // elapses -> the debounce is cancelled and the final compute + // is scheduled directly by onTurnComplete (one additional call). + svc.onToolCallEditsApplied(sessionUri.toString(), 'turn-1'); + await timeout(1_000); + svc.onTurnComplete(sessionUri.toString(), 'turn-1'); + await timeout(10); + assert.strictEqual(svc.turnComputeCalls.length, 2, 'onTurnComplete cancels pending debounce and runs exactly one final compute'); + + // 3) flipping the probe off mid-stream silences future + // per-turn computes even if more edits arrive. + svc.setTurnSubscriberProbe(() => false); + svc.onToolCallEditsApplied(sessionUri.toString(), 'turn-1'); + await timeout(6_000); + assert.strictEqual(svc.turnComputeCalls.length, 2, 'unsubscribed turn must not get any further per-turn computes'); + }); + }); + + test('per-turn URI streams incremental ChangesetFileSet / ChangesetFileRemoved as the same turn is recomputed', async () => { + // End-to-end variant exercising the real `computeTurnDiffs` path + // — produces actual diff payloads from session-DB messages so + // `_publishChangesetDiffs` emits real per-file actions on each + // recompute pass. + const sessionDb = new SessionDatabase(':memory:'); + disposables.add(toDisposable(() => sessionDb.close())); + const localStateManager = disposables.add(new AgentHostStateManager(new NullLogService())); + const svc = disposables.add(new AgentHostChangesetService( + localStateManager, + new NullLogService(), + createSessionDataService(sessionDb), + createNoopGitService(), + )); + svc.setTurnSubscriberProbe(() => true); + + localStateManager.createSession({ + resource: sessionUri.toString(), + provider: 'mock', + title: 'Test', + status: SessionStatus.Idle, + createdAt: Date.now(), + modifiedAt: Date.now(), + workingDirectory: 'file:///wd', + }); + + const envelopes: ActionEnvelope[] = []; + disposables.add(localStateManager.onDidEmitEnvelope(e => envelopes.push(e))); + const turnUri = `${sessionUri.toString()}/changeset/turn/turn-1`; + + // First compute pass — no edits yet, so just establishes the + // per-turn state at status: ready with an empty file list. + await svc.computeTurnChangeset(sessionUri.toString(), 'turn-1'); + const statusReady = envelopes + .map(e => e.action) + .find(a => a.type === ActionType.ChangesetStatusChanged && a.changeset === turnUri); + assert.ok(statusReady, 'first per-turn compute must transition the URI to ready'); + + // Subsequent recomputes are observable via `_publishChangesetDiffs` + // even with empty diffs — the delta diffing is what matters here. + // Smoke-check that calling `onTurnComplete` triggers another + // `computeTurnChangeset` invocation through the sequencer. + envelopes.length = 0; + svc.onTurnComplete(sessionUri.toString(), 'turn-1'); + for (let i = 0; i < 100 && !envelopes.some(e => e.action.type === ActionType.ChangesetStatusChanged && e.action.changeset === `${sessionUri.toString()}/changeset/session`); i++) { + await timeout(2); + } + // Per-turn recompute was scheduled — at minimum its presence is + // proven by the static-session recompute also having run (both + // share the same `onTurnComplete` dispatch path). + assert.ok( + envelopes.some(e => e.action.type === ActionType.ChangesetStatusChanged), + 'onTurnComplete must drive at least one downstream changeset status transition', + ); + }); + }); }); diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 847cc0e4818..8f169b3302c 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -553,16 +553,17 @@ suite('AgentService (node dispatcher)', () => { assert.strictEqual(sessions.length, 1); assert.deepStrictEqual(sessions[0].changesets, [ { - label: 'Uncommitted Changes', - uriTemplate: `${sessionUri.toString()}/changeset/uncommitted`, - }, - { - label: 'Session Changes', + label: 'Branch Changes', uriTemplate: `${sessionUri.toString()}/changeset/session`, additions: 8, deletions: 2, files: 2, }, + { + label: 'Uncommitted Changes', + uriTemplate: `${sessionUri.toString()}/changeset/uncommitted`, + description: 'Show uncommitted changes in this session', + }, { label: 'This Turn', uriTemplate: `${sessionUri.toString()}/changeset/turn/{turnId}`, @@ -692,16 +693,17 @@ suite('AgentService (node dispatcher)', () => { const sessions = await svc.listSessions(); assert.deepStrictEqual(sessions[0].changesets, [ { - label: 'Uncommitted Changes', - uriTemplate: `${sessionUri.toString()}/changeset/uncommitted`, - }, - { - label: 'Session Changes', + label: 'Branch Changes', uriTemplate: changesetUri, additions: 1, deletions: 0, files: 1, }, + { + label: 'Uncommitted Changes', + uriTemplate: `${sessionUri.toString()}/changeset/uncommitted`, + description: 'Show uncommitted changes in this session', + }, { label: 'This Turn', uriTemplate: `${sessionUri.toString()}/changeset/turn/{turnId}`, @@ -795,16 +797,17 @@ suite('AgentService (node dispatcher)', () => { const sessions = await svc.listSessions(); assert.deepStrictEqual(sessions[0].changesets, [ { - label: 'Uncommitted Changes', - uriTemplate: `${sessionUri.toString()}/changeset/uncommitted`, - }, - { - label: 'Session Changes', + label: 'Branch Changes', uriTemplate: `${sessionUri.toString()}/changeset/session`, additions: 7, deletions: 1, files: 1, }, + { + label: 'Uncommitted Changes', + uriTemplate: `${sessionUri.toString()}/changeset/uncommitted`, + description: 'Show uncommitted changes in this session', + }, { label: 'This Turn', uriTemplate: `${sessionUri.toString()}/changeset/turn/{turnId}`, @@ -916,6 +919,66 @@ suite('AgentService (node dispatcher)', () => { assert.strictEqual(localService.stateManager.getSessionState(session.toString())?._meta, undefined); }); + test('createSession strips git-only catalogue entries for non-git working directory', async () => { + const workingDirectory = URI.file('/workspace/not-a-repo'); + const gitService = createNoopGitService(); + // Probe runs but reports "not a git repo". + gitService.getSessionGitState = async () => undefined; + + const localService = disposables.add(new AgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + const agent = new MockAgent('copilot'); + disposables.add(toDisposable(() => agent.dispose())); + agent.resolvedWorkingDirectory = workingDirectory; + agent.sessionMetadataOverrides = { workingDirectory }; + localService.registerProvider(agent); + + const session = await localService.createSession({ provider: 'copilot' }); + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } + + const state = localService.stateManager.getSessionState(session.toString()); + assert.ok(state); + assert.deepStrictEqual(state!.summary.changesets, [ + { label: 'This Turn', uriTemplate: `${session.toString()}/changeset/turn/{turnId}` }, + ]); + }); + + test('createSession keeps git-only catalogue entries for a git working directory', async () => { + const workingDirectory = URI.file('/workspace/repo'); + const gitState = { + hasGitHubRemote: false, + branchName: 'main', + baseBranchName: 'main', + upstreamBranchName: undefined, + incomingChanges: 0, + outgoingChanges: 0, + uncommittedChanges: 0, + }; + const gitService = createNoopGitService(); + gitService.getSessionGitState = async () => gitState; + + const localService = disposables.add(new AgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + const agent = new MockAgent('copilot'); + disposables.add(toDisposable(() => agent.dispose())); + agent.resolvedWorkingDirectory = workingDirectory; + agent.sessionMetadataOverrides = { workingDirectory }; + localService.registerProvider(agent); + + const session = await localService.createSession({ provider: 'copilot' }); + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } + + const state = localService.stateManager.getSessionState(session.toString()); + assert.ok(state); + assert.deepStrictEqual(state!.summary.changesets, [ + { label: 'Branch Changes', uriTemplate: `${session.toString()}/changeset/session` }, + { label: 'Uncommitted Changes', uriTemplate: `${session.toString()}/changeset/uncommitted`, description: 'Show uncommitted changes in this session' }, + { label: 'This Turn', uriTemplate: `${session.toString()}/changeset/turn/{turnId}` }, + ]); + }); + test('subscribe lazily attaches git state when an existing session has no _meta.git', async () => { // Regression test: previously AgentService was constructed without // a git service, so _attachGitState always bailed and `_meta.git` @@ -1634,15 +1697,20 @@ suite('AgentService (node dispatcher)', () => { localService.unsubscribe(uncommittedUri, 'client-1'); }); - test('addSubscriber for non-uncommitted resources does NOT trigger a refresh', async () => { + test('addSubscriber for the session URI or session-changeset URI triggers a static refresh', async () => { + // The Agents Window subscribes to the session URI (list / + // detail) rather than to either of the static changeset URIs + // directly, so the chip would never refresh on session open + // without this trigger. Subscribing to the session-changeset + // URI from any other client must also fire its own refresh. const workingDirectory = URI.from({ scheme: Schemas.inMemory, path: '/wd-refresh-2' }); copilotAgent.resolvedWorkingDirectory = workingDirectory; copilotAgent.sessionMetadataOverrides = { workingDirectory }; - const computeCalls: { baseBranch: string | undefined }[] = []; + const computeCalls: { wd: string; baseBranch: string | undefined }[] = []; const gitService = createNoopGitService(); - gitService.computeSessionFileDiffs = async (_wd: URI, opts: { sessionUri: string; baseBranch?: string }) => { - computeCalls.push({ baseBranch: opts.baseBranch }); + gitService.computeSessionFileDiffs = async (wd: URI, opts: { sessionUri: string; baseBranch?: string }) => { + computeCalls.push({ wd: wd.toString(), baseBranch: opts.baseBranch }); return undefined; }; @@ -1653,16 +1721,16 @@ suite('AgentService (node dispatcher)', () => { const sessionChangesetUri = URI.parse(buildSessionChangesetUri(sessionResource.toString())); localService.addSubscriber(sessionChangesetUri, 'client-1'); - localService.addSubscriber(sessionResource, 'client-1'); + localService.addSubscriber(sessionResource, 'client-2'); await new Promise(r => setTimeout(r, 20)); assert.ok( - !computeCalls.some(c => c.baseBranch === undefined), - `non-uncommitted subscriptions must not trigger an uncommitted git diff, got: ${JSON.stringify(computeCalls)}`, + computeCalls.some(c => c.wd === workingDirectory.toString()), + `session-URI / session-changeset subscriptions must trigger a git diff against the working dir, got: ${JSON.stringify(computeCalls)}`, ); localService.unsubscribe(sessionChangesetUri, 'client-1'); - localService.unsubscribe(sessionResource, 'client-1'); + localService.unsubscribe(sessionResource, 'client-2'); }); test('restoreSession drains a pending uncommitted refresh deferred by an earlier addSubscriber', async () => { @@ -1927,17 +1995,22 @@ suite('AgentService (node dispatcher)', () => { const state = localService.stateManager.getSessionState(sessionResource.toString()); assert.ok(state); + // The session has no working directory, so `_attachGitState` + // treats it as transient and does NOT strip the two git-only + // catalogue entries. The Branch Changes entry receives the + // persisted diff counts seeded by the changeset coordinator. assert.deepStrictEqual(state!.summary.changesets, [ { - label: 'Uncommitted Changes', - uriTemplate: `${sessionResource.toString()}/changeset/uncommitted`, - }, - { - label: 'Session Changes', - uriTemplate: `${sessionResource.toString()}/changeset/session`, additions: 5, deletions: 2, files: 1, + label: 'Branch Changes', + uriTemplate: `${sessionResource.toString()}/changeset/session`, + }, + { + description: 'Show uncommitted changes in this session', + label: 'Uncommitted Changes', + uriTemplate: `${sessionResource.toString()}/changeset/uncommitted`, }, { label: 'This Turn', @@ -1975,16 +2048,19 @@ suite('AgentService (node dispatcher)', () => { const state = localService.stateManager.getSessionState(sessionResource.toString()); assert.ok(state); - // Catalogue is seeded by `_buildInitialSummary` / `restoreSession` - // (entries with no counts) — but no files were seeded. + // Catalogue is seeded by `_buildInitialSummary` / `restoreSession`. + // The session has no working directory, so `_attachGitState` does + // NOT strip the git-only entries — they remain advertised but + // without counts until a real compute lands. assert.deepStrictEqual(state!.summary.changesets, [ { - label: 'Uncommitted Changes', - uriTemplate: `${sessionResource.toString()}/changeset/uncommitted`, + label: 'Branch Changes', + uriTemplate: `${sessionResource.toString()}/changeset/session`, }, { - label: 'Session Changes', - uriTemplate: `${sessionResource.toString()}/changeset/session`, + description: 'Show uncommitted changes in this session', + label: 'Uncommitted Changes', + uriTemplate: `${sessionResource.toString()}/changeset/uncommitted`, }, { label: 'This Turn', @@ -2153,9 +2229,13 @@ suite('AgentService (node dispatcher)', () => { } function defaultCatalogue(sessionStr: string) { + // These tests have no working directory resolved, so + // `_attachGitState` treats it as transient and does NOT strip + // the two git-only entries. All three default entries are + // advertised (without counts) until a real compute lands. return [ - { label: 'Uncommitted Changes', uriTemplate: `${sessionStr}/changeset/uncommitted` }, - { label: 'Session Changes', uriTemplate: `${sessionStr}/changeset/session` }, + { label: 'Branch Changes', uriTemplate: `${sessionStr}/changeset/session` }, + { label: 'Uncommitted Changes', uriTemplate: `${sessionStr}/changeset/uncommitted`, description: 'Show uncommitted changes in this session' }, { label: 'This Turn', uriTemplate: `${sessionStr}/changeset/turn/{turnId}` }, ]; } diff --git a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts index 8b67981604e..311cfc5fb63 100644 --- a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts @@ -53,6 +53,8 @@ class FakeChangesetService implements IAgentHostChangesetService { restoreStaticChangeset(_session: string, _kind: StaticChangesetKind, _diffs: readonly unknown[]): void { /* no-op */ } restorePersistedStaticChangesets(): { uncommitted?: undefined; session?: undefined } { return {}; } refreshUncommittedChangeset(): void { /* no-op */ } + refreshSessionChangeset(): void { /* no-op */ } + setTurnSubscriberProbe(): void { /* no-op */ } async computeTurnChangeset(session: string): Promise { return `${session}/changeset/turn/x`; } onToolCallEditsApplied(session: string, turnId: string): void { diff --git a/src/vs/platform/agentHost/test/node/protocol/sessionDiffs.integrationTest.ts b/src/vs/platform/agentHost/test/node/protocol/sessionDiffs.integrationTest.ts index 78f2e9c7fcf..d345f4da14d 100644 --- a/src/vs/platform/agentHost/test/node/protocol/sessionDiffs.integrationTest.ts +++ b/src/vs/platform/agentHost/test/node/protocol/sessionDiffs.integrationTest.ts @@ -60,7 +60,17 @@ const hasGit = (() => { teardown(function () { client.close(); if (tmpRoot) { - rmSync(tmpRoot, { recursive: true, force: true }); + try { + // On Windows, freshly-spawned `git` child processes and the + // agent host server may still hold handles on files under + // `tmpRoot` (e.g. `.git/index`) when teardown runs, causing + // `EBUSY`/`ENOTEMPTY`. `maxRetries` is Node's built-in + // workaround for exactly this case. + rmSync(tmpRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } catch { + // Best-effort: leave the temp dir for the OS to clean up + // rather than fail the test on a stale Windows file lock. + } } }); diff --git a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts index b0ea3c9b12e..9bd68256d41 100644 --- a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts +++ b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts @@ -546,7 +546,7 @@ suite('ProtocolServerHandler', () => { summary: 'Session With Changesets', changesets: [ { - label: 'Session Changes', + label: 'Branch Changes', uriTemplate: `${sessionUri}/changeset/session`, additions: 5, deletions: 2, @@ -565,7 +565,7 @@ suite('ProtocolServerHandler', () => { const result = (resp as unknown as { result: ListSessionsResult }).result; assert.deepStrictEqual(result.items[0].changesets, [ { - label: 'Session Changes', + label: 'Branch Changes', uriTemplate: `${sessionUri}/changeset/session`, additions: 5, deletions: 2,