From 87fcdc196677df657d613b0f4f85e645c300b08e Mon Sep 17 00:00:00 2001 From: Ben Villalobos <4691428+benvillalobos@users.noreply.github.com> Date: Tue, 25 Aug 2026 16:37:51 -0700 Subject: [PATCH] automations: revert subscribe callbacks to URI The subscription manager threaded raw channel strings through _subscribe/_unsubscribe to dodge a lossy round-trip on the catalogue channel. Now that the catalogue URI round-trips, revert those callbacks to (resource: URI) to match main. The wire boundary keeps its .toString() serialization in the protocol client. --- .../browser/agentHostProtocolClient.ts | 4 ++-- .../common/state/agentSubscription.ts | 20 +++++++++---------- .../test/common/agentSubscription.test.ts | 20 +++++++++---------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts b/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts index 4768a91ea2c..29559116c92 100644 --- a/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts +++ b/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts @@ -340,8 +340,8 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect this._clientId, () => this.nextClientSeq(), msg => this._logService.warn(`[AgentHostProtocolClient] ${msg}`), - channel => this._subscribeChannel(channel), - channel => this._unsubscribeChannel(channel), + resource => this.subscribe(resource), + resource => this.unsubscribe(resource), )); // Forward action envelopes from the transport to the subscription manager diff --git a/src/vs/platform/agentHost/common/state/agentSubscription.ts b/src/vs/platform/agentHost/common/state/agentSubscription.ts index 8069e6b5809..30b00e73480 100644 --- a/src/vs/platform/agentHost/common/state/agentSubscription.ts +++ b/src/vs/platform/agentHost/common/state/agentSubscription.ts @@ -878,15 +878,15 @@ export class AgentSubscriptionManager extends Disposable { private readonly _clientId: string; private readonly _seqAllocator: () => number; private readonly _log: (msg: string) => void; - private readonly _subscribe: (channel: string) => Promise; - private readonly _unsubscribe: (channel: string) => void; + private readonly _subscribe: (resource: URI) => Promise; + private readonly _unsubscribe: (resource: URI) => void; constructor( clientId: string, seqAllocator: () => number, log: (msg: string) => void, - subscribe: (channel: string) => Promise, - unsubscribe: (channel: string) => void, + subscribe: (resource: URI) => Promise, + unsubscribe: (resource: URI) => void, ) { super(); this._clientId = clientId; @@ -1000,7 +1000,7 @@ export class AgentSubscriptionManager extends Disposable { } } try { - const snapshot = await this._subscribe(resolved.channel); + const snapshot = await this._subscribe(resolved.resource); if (this._subscriptions.get(resolved.resource) === entry) { sub.handleSnapshot(snapshot.state as never, snapshot.fromSeq); } @@ -1039,19 +1039,19 @@ export class AgentSubscriptionManager extends Disposable { } private _disposeSubscriptionEntry(entry: ManagedSubscriptionEntry): void { - this._tryUnsubscribe(entry.channel); + this._tryUnsubscribe(entry.resource); if (entry.sub instanceof SessionStateSubscription || entry.sub instanceof ChatStateSubscription || entry.sub instanceof AnnotationsStateSubscription) { entry.sub.clearPending(); } entry.sub.dispose(); } - private _tryUnsubscribe(channel: string): void { + private _tryUnsubscribe(resource: URI): void { try { - this._unsubscribe(channel); + this._unsubscribe(resource); } catch (error) { const message = error instanceof Error ? error.message : String(error); - this._log(`Failed to unsubscribe ${channel}: ${message}`); + this._log(`Failed to unsubscribe ${resource.toString()}: ${message}`); } } @@ -1248,7 +1248,7 @@ export class AgentSubscriptionManager extends Disposable { override dispose(): void { for (const entry of this._subscriptions.values()) { - this._tryUnsubscribe(entry.channel); + this._tryUnsubscribe(entry.resource); entry.sub.dispose(); } this._subscriptions.clear(); diff --git a/src/vs/platform/agentHost/test/common/agentSubscription.test.ts b/src/vs/platform/agentHost/test/common/agentSubscription.test.ts index eaa0f98c891..fc07774ede1 100644 --- a/src/vs/platform/agentHost/test/common/agentSubscription.test.ts +++ b/src/vs/platform/agentHost/test/common/agentSubscription.test.ts @@ -774,9 +774,9 @@ suite('AgentSubscriptionManager', () => { ensureNoDisposablesAreLeakedInTestSuite(); - function createManager(subscribe: (channel: string) => Promise<{ resource: string; state: SessionState | TerminalState | ChangesetState | AnnotationsState | AutomationCatalogState; fromSeq: number }> = async channel => { - subscribedResources.push(channel); - const key = channel; + function createManager(subscribe: (resource: URI) => Promise<{ resource: string; state: SessionState | TerminalState | ChangesetState | AnnotationsState | AutomationCatalogState; fromSeq: number }> = async resource => { + const key = resource.toString(); + subscribedResources.push(key); if (key.endsWith('/annotations')) { return { resource: key, state: { annotations: [] }, fromSeq: 0 }; } @@ -790,8 +790,8 @@ suite('AgentSubscriptionManager', () => { () => ++seq, noop, subscribe, - channel => { - unsubscribedResources.push(channel); + resource => { + unsubscribedResources.push(resource.toString()); }, )); } @@ -926,9 +926,9 @@ suite('AgentSubscriptionManager', () => { }); test('preserves the exact authority-less automation catalogue channel', async () => { - const mgr = createManager(async channel => { - subscribedResources.push(channel); - return { resource: channel, state: { automations: [] }, fromSeq: 0 }; + const mgr = createManager(async resource => { + subscribedResources.push(resource.toString()); + return { resource: resource.toString(), state: { automations: [] }, fromSeq: 0 }; }); const ref = mgr.getSubscriptionByChannel(StateComponents.AutomationCatalog, AUTOMATION_CATALOG_URI, 'AutomationHolder'); await Event.toPromise(ref.object.onDidChange); @@ -1192,8 +1192,8 @@ suite('AgentSubscriptionManager', () => { }); test('markSubscriptionsMissing preserves exact protocol channels', async () => { - const mgr = createManager(async channel => ({ - resource: channel, + const mgr = createManager(async resource => ({ + resource: resource.toString(), state: { automations: [] }, fromSeq: 0, }));