feat: implement per-turn changeset recompute logic and associated tests (#317256)

* fix: update changeset labels and descriptions to reflect branch changes

* Update tests

* feat: implement per-turn changeset recompute logic and associated tests

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* updates

* Refactor comments in agentService tests to clarify handling of git state in transient sessions

* Improve teardown logic in sessionDiffs integration test to handle Windows file locks

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Don Jayamanne
2026-05-19 13:48:02 +02:00
committed by GitHub
co-authored by Copilot Autofix powered by AI
parent 3d0bb6af4c
commit a45bf445cf
9 changed files with 521 additions and 82 deletions
@@ -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) },
];
}
@@ -70,12 +70,32 @@ export class ChangesetSessionCoordinator extends Disposable {
*/
private readonly _pendingUncommittedRefreshes = new Set<string>();
/**
* Per-session set of turn ids that have at least one live subscriber to
* `<sessionUri>/changeset/turn/<turnId>`. 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<string, Set<string>>();
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
* `<session>/changeset/turn/<turnId>`. 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);
}
}
}
}
@@ -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 `<session>/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<string>();
/** Per-session debounce timers for mid-turn diff computation. */
private readonly _debouncedDiffTimers = this._register(new DisposableMap<string>());
/** Per-`(session, turnId)` debounce timers for mid-turn per-turn changeset recomputation. */
private readonly _perTurnDebouncedDiffTimers = this._register(new DisposableMap<string>());
private static readonly _DIFF_DEBOUNCE_MS = 5000;
/**
* Subscriber probe set by {@link ChangesetSessionCoordinator}. Returns
* `true` when at least one client is subscribed to
* `<session>/changeset/turn/<turnId>`. 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<ProtocolURI> {
const turnUri = this._stateManager.registerChangeset(buildTurnChangesetUri(session, turnId));
let ref: ReturnType<ISessionDataService['openDatabase']>;
@@ -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
+45 -6
View File
@@ -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<string, unknown>): void {
let ref;
try {
@@ -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<string> {
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',
);
});
});
});
@@ -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}` },
];
}
@@ -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<string> { return `${session}/changeset/turn/x`; }
onToolCallEditsApplied(session: string, turnId: string): void {
@@ -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.
}
}
});
@@ -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,