From 5b5c314a6a6b307e0f7ceeaf3ef87f6d21827984 Mon Sep 17 00:00:00 2001 From: Paul Wang Date: Fri, 4 Sep 2026 10:54:50 -0700 Subject: [PATCH 1/3] Fix synced customization disposal races Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/syncedCustomizationBundler.ts | 59 +++++++++++++++++-- .../syncedCustomizationBundler.test.ts | 48 ++++++++++++++- 2 files changed, 101 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/syncedCustomizationBundler.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/syncedCustomizationBundler.ts index c4fde0679fb09..334139dc504b5 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/syncedCustomizationBundler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/syncedCustomizationBundler.ts @@ -3,9 +3,10 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Limiter } from '../../../../../../base/common/async.js'; +import { Limiter, SequencerByKey } from '../../../../../../base/common/async.js'; import { VSBuffer } from '../../../../../../base/common/buffer.js'; -import { Disposable } from '../../../../../../base/common/lifecycle.js'; +import { CancellationError, isCancellationError } from '../../../../../../base/common/errors.js'; +import { Disposable, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; import { equals } from '../../../../../../base/common/objects.js'; import { ResourceMap } from '../../../../../../base/common/map.js'; import { basename, dirname, extUri } from '../../../../../../base/common/resources.js'; @@ -29,6 +30,7 @@ export { SYNCED_CUSTOMIZATION_SCHEME }; const DISPLAY_NAME = 'VS Code Synced Data'; const FILE_OPERATION_CONCURRENCY = 10; const SKILL_DIRECTORY_IGNORE = new IgnoreFile('.git\nnode_modules\n', '/', undefined, true); +const bundleSequencer = new SequencerByKey(); const MANIFEST_CONTENT = JSON.stringify({ name: DISPLAY_NAME, @@ -61,6 +63,9 @@ async function collectDirectoryFiles(fileService: IFileService, logService: ILog try { return await queueFileOperation(() => fileService.stat(child.resource)); } catch (error) { + if (isCancellationError(error)) { + throw error; + } logService.trace('[SyncedCustomizationBundler] Failed to stat skill resource', child.resource.toString(), error); return undefined; } @@ -154,10 +159,11 @@ interface IBundleResult { */ export class SyncedCustomizationBundler extends Disposable { - private readonly _fileOperationLimiter = this._register(new Limiter(FILE_OPERATION_CONCURRENCY)); + private readonly _fileOperationLimiter = this._register(new MutableDisposable>()); private readonly _authority: string; private _lastNonce: string | undefined; private _lastRef: IBundleResult | undefined; + private _isDisposed = false; /** Maps a synced (destination) URI string back to its original source location. Rebuilt on every {@link bundle}. */ private _originByDest = new ResourceMap(); @@ -168,6 +174,7 @@ export class SyncedCustomizationBundler extends Disposable { @ILogService private readonly _logService: ILogService, ) { super(); + this._fileOperationLimiter.value = new Limiter(FILE_OPERATION_CONCURRENCY); this._authority = authority; agentHostFileSystemService.ensureSyncedCustomizationProvider(); } @@ -182,7 +189,18 @@ export class SyncedCustomizationBundler extends Disposable { } private _queueFileOperation(operation: () => Promise): Promise { - return this._fileOperationLimiter.queue(operation) as Promise; + this._throwIfDisposed(); + const limiter = this._fileOperationLimiter.value; + if (!limiter) { + throw new CancellationError(); + } + return limiter.queue(operation) as Promise; + } + + private _throwIfDisposed(): void { + if (this._isDisposed) { + throw new CancellationError(); + } } /** @@ -195,6 +213,12 @@ export class SyncedCustomizationBundler extends Disposable { * @returns The bundle result, or `undefined` if there is nothing to sync. */ async bundle(files: readonly ISyncableFile[], mcpServers: readonly ISyncableMcpServer[] = []): Promise { + this._throwIfDisposed(); + return bundleSequencer.queue(this._authority, () => this._bundle(files, mcpServers)); + } + + private async _bundle(files: readonly ISyncableFile[], mcpServers: readonly ISyncableMcpServer[]): Promise { + this._throwIfDisposed(); const syncable = files.filter(f => pluginDirForType(f.type) !== undefined); if (syncable.length === 0 && mcpServers.length === 0) { return undefined; @@ -246,6 +270,7 @@ export class SyncedCustomizationBundler extends Disposable { addEntry(file, source, URI.joinPath(this._rootUri, dir, fileName), `${dir}/${fileName}`); } })); + this._throwIfDisposed(); // Write MCP servers into `.mcp.json`. The agent host's Open Plugin // adapter reads this file relative to the plugin root. Servers are @@ -278,6 +303,7 @@ export class SyncedCustomizationBundler extends Disposable { // Stable nonce: sort so file ordering doesn't matter. hashParts.sort(); const nonce = String(hash(hashParts.join('\n'))); + this._throwIfDisposed(); // Nothing changed since the last successful bundle — reuse it and skip // reading file contents and rewriting the in-memory plugin tree. @@ -298,7 +324,7 @@ export class SyncedCustomizationBundler extends Disposable { destUri: entry.destUri, content: (await this._queueFileOperation(() => this._fileService.readFile(entry.sourceUri))).value, }))); - this._originByDest = originByDest; + this._throwIfDisposed(); // Delete the previous tree for this authority, preserving other authorities try { @@ -308,21 +334,26 @@ export class SyncedCustomizationBundler extends Disposable { } // Write the manifest + this._throwIfDisposed(); const manifestUri = URI.joinPath(this._rootUri, '.plugin', 'plugin.json'); await this._fileService.writeFile(manifestUri, VSBuffer.fromString(MANIFEST_CONTENT)); // Write each source file into the correct plugin directory. for (const entry of fileContents) { + this._throwIfDisposed(); await this._fileService.writeFile(entry.destUri, entry.content); } // Write MCP servers into `.mcp.json`. The agent host's Open Plugin // adapter reads this file relative to the plugin root. if (mcpContent !== undefined) { + this._throwIfDisposed(); const mcpUri = URI.joinPath(this._rootUri, '.mcp.json'); await this._fileService.writeFile(mcpUri, VSBuffer.fromString(mcpContent)); } + this._throwIfDisposed(); + this._originByDest = originByDest; this._lastNonce = nonce; const rootUriString = this._rootUri.toString() as ProtocolURI; @@ -365,4 +396,22 @@ export class SyncedCustomizationBundler extends Disposable { getOrigin(syncedUri: URI): ISyncedCustomizationOrigin | undefined { return this._originByDest.get(syncedUri); } + + override dispose(): void { + if (this._isDisposed) { + return; + } + this._isDisposed = true; + // Keep the limiter alive until file operations queued before disposal have settled. + const limiter = this._fileOperationLimiter.clearAndLeak(); + super.dispose(); + if (!limiter) { + return; + } + if (limiter.size === 0) { + limiter.dispose(); + } else { + void limiter.whenIdle().then(() => limiter.dispose()); + } + } } diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/syncedCustomizationBundler.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/syncedCustomizationBundler.test.ts index 90c0527c69085..e1d4cb62d814a 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/syncedCustomizationBundler.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/syncedCustomizationBundler.test.ts @@ -6,6 +6,7 @@ import assert from 'assert'; import sinon from 'sinon'; import { timeout } from '../../../../../../base/common/async.js'; +import { isCancellationError } from '../../../../../../base/common/errors.js'; import { DisposableStore } from '../../../../../../base/common/lifecycle.js'; import { ResourceSet } from '../../../../../../base/common/map.js'; import { Schemas } from '../../../../../../base/common/network.js'; @@ -27,7 +28,7 @@ class TestInMemoryFileSystemProvider extends InMemoryFileSystemProvider { private readonly symbolicLinks = new ResourceSet(); private readonly statFailures = new ResourceSet(); private statDelay = 0; - private activeStats = 0; + activeStats = 0; maxActiveStats = 0; markSymbolicLink(resource: URI): void { @@ -319,6 +320,51 @@ suite('SyncedCustomizationBundler', () => { assert.strictEqual(memFs.maxActiveStats, 10); }); + test('cancels while queued skill operations drain after disposal', async () => { + const bundler = createBundler(); + const skill = await seedFile('/skills/disposed/SKILL.md', 'skill content'); + for (let index = 0; index < 20; index++) { + await seedFile(`/skills/disposed/references/${index}.md`, `reference ${index}`); + } + memFs.delayStats(20); + + const bundle = bundler.bundle([{ uri: skill, type: PromptsType.skill }]); + while (memFs.maxActiveStats < 10) { + await timeout(0); + } + bundler.dispose(); + + await assert.rejects(bundle, error => isCancellationError(error)); + await assert.rejects(bundler.bundle([{ uri: skill, type: PromptsType.skill }]), error => isCancellationError(error)); + }); + + test('serializes replacement bundles for the same authority', async () => { + const disposedBundler = createBundler('shared-agent'); + const replacementBundler = createBundler('shared-agent'); + const skill = await seedFile('/skills/replaced/SKILL.md', 'old skill content'); + for (let index = 0; index < 20; index++) { + await seedFile(`/skills/replaced/references/${index}.md`, `reference ${index}`); + } + const replacement = await seedFile('/replacement.md', 'replacement content'); + memFs.delayStats(50); + + const disposedBundle = disposedBundler.bundle([{ uri: skill, type: PromptsType.skill }]); + while (memFs.maxActiveStats < 10) { + await timeout(0); + } + disposedBundler.dispose(); + const replacementBundle = replacementBundler.bundle([{ uri: replacement, type: PromptsType.instructions }]); + await timeout(0); + + assert.strictEqual(memFs.activeStats, 10); + await assert.rejects(disposedBundle, error => isCancellationError(error)); + await replacementBundle; + assert.strictEqual( + (await fileService.readFile(URI.from({ scheme: SYNCED_CUSTOMIZATION_SCHEME, path: '/shared-agent/rules/replacement.md' }))).value.toString(), + 'replacement content' + ); + }); + test('skips unreadable nested skill resources', async () => { const bundler = createBundler(); const skill = await seedFile('/skills/unreadable/SKILL.md', 'skill content'); From 06be8469080d56e7c6a191e601b97b8e6df31c17 Mon Sep 17 00:00:00 2001 From: Paul Wang Date: Fri, 4 Sep 2026 15:56:27 -0700 Subject: [PATCH 2/3] Harden cancellation log handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/platform/request/common/request.ts | 8 ++- .../test/common/requestService.test.ts | 38 ++++++++++- .../cloudSandboxAgentHostContribution.ts | 5 +- .../browser/cloudSandboxApiService.ts | 23 +++++-- .../cloudSandboxAgentHostContribution.test.ts | 34 +++++++++- .../browser/cloudSandboxApiService.test.ts | 63 ++++++++++++++++++- .../agentCustomizationContentExpander.ts | 35 ++++++++--- .../agentHost/syncedCustomizationBundler.ts | 62 ++++++++++++------ .../agentCustomizationContentExpander.test.ts | 21 +++++-- .../syncedCustomizationBundler.test.ts | 22 +++++++ 10 files changed, 265 insertions(+), 46 deletions(-) diff --git a/src/vs/platform/request/common/request.ts b/src/vs/platform/request/common/request.ts index 8d4dfc7a5383c..8379a0cae038e 100644 --- a/src/vs/platform/request/common/request.ts +++ b/src/vs/platform/request/common/request.ts @@ -5,7 +5,7 @@ import { streamToBuffer } from '../../../base/common/buffer.js'; import { CancellationToken } from '../../../base/common/cancellation.js'; -import { getErrorMessage } from '../../../base/common/errors.js'; +import { getErrorMessage, isCancellationError } from '../../../base/common/errors.js'; import { Emitter, Event } from '../../../base/common/event.js'; import { Disposable } from '../../../base/common/lifecycle.js'; import { IHeaders, IRequestContext, IRequestOptions } from '../../../base/parts/request/common/request.js'; @@ -110,7 +110,11 @@ export abstract class AbstractRequestService extends Disposable implements IRequ }); return result; } catch (error) { - this.logService.error(`${prefix} - error`, options.type, getErrorMessage(error)); + if (isCancellationError(error)) { + this.logService.trace(`${prefix} - cancelled`, options.type); + } else { + this.logService.error(`${prefix} - error`, options.type, getErrorMessage(error)); + } throw error; } } diff --git a/src/vs/platform/request/test/common/requestService.test.ts b/src/vs/platform/request/test/common/requestService.test.ts index 3760902fb1f9c..1cb89aaa00392 100644 --- a/src/vs/platform/request/test/common/requestService.test.ts +++ b/src/vs/platform/request/test/common/requestService.test.ts @@ -6,15 +6,29 @@ import assert from 'assert'; import { bufferToStream, VSBuffer } from '../../../../base/common/buffer.js'; import { CancellationToken } from '../../../../base/common/cancellation.js'; +import { CancellationError, isCancellationError } from '../../../../base/common/errors.js'; import { IRequestContext, IRequestOptions } from '../../../../base/parts/request/common/request.js'; -import { NullLogService } from '../../../log/common/log.js'; +import { ILogService, NullLogService } from '../../../log/common/log.js'; import { AbstractRequestService, AuthInfo, Credentials, IRequestCompleteEvent, NO_FETCH_TELEMETRY } from '../../common/request.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +class TestLogService extends NullLogService { + readonly traces: string[] = []; + readonly errors: (string | Error)[] = []; + + override trace(message: string, ...args: unknown[]): void { + this.traces.push([message, ...args].join(' ')); + } + + override error(error: string | Error, ..._args: unknown[]): void { + this.errors.push(error); + } +} + class TestRequestService extends AbstractRequestService { - constructor(private readonly handler: (options: IRequestOptions) => Promise) { - super(new NullLogService()); + constructor(private readonly handler: (options: IRequestOptions) => Promise, logService: ILogService = new NullLogService()) { + super(logService); } async request(options: IRequestOptions, token: CancellationToken): Promise { @@ -87,6 +101,24 @@ suite('AbstractRequestService', () => { assert.strictEqual(events.length, 0); }); + test('logs cancellation at trace level', async () => { + const logService = new TestLogService(); + const service = store.add(new TestRequestService(() => Promise.reject(new CancellationError()), logService)); + + await assert.rejects( + () => service.request({ url: 'http://test', callSite: 'test.cancelled' }, CancellationToken.None), + error => isCancellationError(error), + ); + + assert.deepStrictEqual({ + cancelledTraces: logService.traces.filter(message => message.includes(' - cancelled')).length, + errors: logService.errors, + }, { + cancelledTraces: 1, + errors: [], + }); + }); + test('onDidCompleteRequest fires for each request', async () => { const service = store.add(new TestRequestService(() => Promise.resolve(makeResponse(200)))); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts index aecff61bf5893..918dd808809a4 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts @@ -10,7 +10,7 @@ import { CancellationToken, CancellationTokenSource } from '../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../base/common/codicons.js'; -import { CancellationError } from '../../../../../base/common/errors.js'; +import { CancellationError, isCancellationError } from '../../../../../base/common/errors.js'; import { Event } from '../../../../../base/common/event.js'; import { Disposable, DisposableMap, DisposableStore, MutableDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; import { URI } from '../../../../../base/common/uri.js'; @@ -272,6 +272,9 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo try { result = await this._apiService.listSessions(token); } catch (error) { + if (token.isCancellationRequested || isCancellationError(error) || !this._isEnabled()) { + return; + } result = { kind: 'failed', reason: error instanceof Error ? error.message : String(error) }; } if (result.kind === 'failed') { diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxApiService.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxApiService.ts index 4b770d4268050..9b6bf633b6241 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxApiService.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxApiService.ts @@ -6,7 +6,7 @@ import { Limiter, timeout } from '../../../../../base/common/async.js'; import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { toErrorMessage } from '../../../../../base/common/errorMessage.js'; -import { isCancellationError } from '../../../../../base/common/errors.js'; +import { CancellationError, isCancellationError } from '../../../../../base/common/errors.js'; import { Disposable } from '../../../../../base/common/lifecycle.js'; import { CLOUD_SANDBOX_AGENT_SLUG, @@ -210,6 +210,12 @@ export class CloudSandboxApiService extends Disposable implements ICloudSandboxA batch = response.tasks; hasNextPage = hasNextLink(context.res.headers?.['link']); } catch (error) { + if (isCancellationError(error)) { + throw error; + } + if (token.isCancellationRequested) { + throw new CancellationError(); + } if (page === 1) { return { kind: 'failed', reason: `listTasks failed: ${toErrorMessage(error)}` }; } @@ -524,13 +530,18 @@ export class CloudSandboxApiService extends Disposable implements ICloudSandboxA return context; } catch (error) { // A cancelled request was never answered, so it is not a failure worth counting. - if (!isCancellationError(error) && !token.isCancellationRequested) { + const cancellationError = isCancellationError(error) + ? error + : token.isCancellationRequested ? new CancellationError() : undefined; + if (cancellationError) { + this._logService.trace(`${LOG_PREFIX} ${action} -> cancelled after ${Date.now() - started}ms (budget ${timeoutMs}ms)`); + } else { this._telemetry.reportRequest(action, 'networkError'); + // Elapsed at the budget means our own timeout fired; shorter means something else did. + this._logService.trace(`${LOG_PREFIX} ${action} -> failed after ${Date.now() - started}ms (budget ${timeoutMs}ms)`); + this._logService.error(`${LOG_PREFIX} ${requestMethod} ${url} failed: ${toErrorMessage(error)}`); } - // Elapsed at the budget means our own timeout fired; shorter means something else did. - this._logService.trace(`${LOG_PREFIX} ${action} -> failed after ${Date.now() - started}ms (budget ${timeoutMs}ms)`); - this._logService.error(`${LOG_PREFIX} ${requestMethod} ${url} failed: ${toErrorMessage(error)}`); - throw error; + throw cancellationError ?? error; } } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts index d2d3a582abf48..30095f6aba1c0 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts @@ -5,6 +5,7 @@ import assert from 'assert'; import { CancellationToken } from '../../../../../../base/common/cancellation.js'; +import { CancellationError } from '../../../../../../base/common/errors.js'; import { Event } from '../../../../../../base/common/event.js'; import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../../../base/common/lifecycle.js'; import { mock, upcastPartial } from '../../../../../../base/test/common/mock.js'; @@ -124,6 +125,14 @@ class StubSessionsProvidersService extends Disposable { getProviders(): ISessionsProvider[] { return []; } } +class TestLogService extends NullLogService { + readonly warnings: (string | Error)[] = []; + + override warn(message: string | Error, ..._args: unknown[]): void { + this.warnings.push(message); + } +} + /** The single host filter entry every sandbox environment folds into. */ const GITHUB_SANDBOX_GROUP: IAgentHostGroup = { id: 'githubsandbox', @@ -155,8 +164,10 @@ interface ITestHarness { async function createContribution(store: Pick, sessions: readonly ICloudSandboxDiscoveredSession[], options?: { /** Task Mission Control returns from `createSession`, or a rejection. */ readonly createSession?: () => Promise; + readonly listSessions?: (token: CancellationToken) => Promise; /** Whether the sandbox feature settings start on. Defaults to `true`. */ readonly enabled?: boolean; + readonly logService?: ILogService; }): Promise { const discoveryHandlers: (() => Promise)[] = []; const hostGroups: IAgentHostGroup[] = []; @@ -172,7 +183,10 @@ async function createContribution(store: Pick, sessions: } as ITestHarness; instantiationService.stub(ICloudSandboxApiService, new class extends mock() { - override async listSessions(_token: CancellationToken): Promise { + override async listSessions(token: CancellationToken): Promise { + if (options?.listSessions) { + return options.listSessions(token); + } return { kind: 'complete', sessions: harness.discovered }; } override async createSession(request: ICloudSandboxCreateSessionRequest): Promise { @@ -224,7 +238,7 @@ async function createContribution(store: Pick, sessions: }()); instantiationService.stub(INotificationService, new class extends mock() { }()); instantiationService.stub(IChatSessionsService, new class extends mock() { }()); - instantiationService.stub(ILogService, new NullLogService()); + instantiationService.stub(ILogService, options?.logService ?? new NullLogService()); const contribution = store.add(instantiationService.createInstance(TestCloudSandboxContribution)); // The constructor kicks off discovery eagerly; re-running the registered handler awaits it, @@ -314,6 +328,22 @@ suite('CloudSandboxAgentHostContribution', () => { assert.deepStrictEqual([...hostGroups], []); }); + + test('does not warn when discovery is cancelled', async () => { + const logService = new TestLogService(); + const { contribution } = await createContribution(store, [], { + listSessions: async () => { throw new CancellationError(); }, + logService, + }); + + assert.deepStrictEqual({ + providers: [...contribution.stubProviders.keys()], + warnings: logService.warnings, + }, { + providers: [], + warnings: [], + }); + }); }); suite('CloudSandboxAgentHostContribution provisioning', () => { diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxApiService.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxApiService.test.ts index a096d18e4a393..8768ac86c50f1 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxApiService.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxApiService.test.ts @@ -6,7 +6,8 @@ import assert from 'assert'; import { timeout } from '../../../../../../base/common/async.js'; import { bufferToStream, VSBuffer } from '../../../../../../base/common/buffer.js'; -import { CancellationToken } from '../../../../../../base/common/cancellation.js'; +import { CancellationToken, CancellationTokenSource } from '../../../../../../base/common/cancellation.js'; +import { CancellationError, isCancellationError } from '../../../../../../base/common/errors.js'; import { Event } from '../../../../../../base/common/event.js'; import { mock } from '../../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; @@ -47,6 +48,19 @@ interface ITestSetup { readonly concurrency: { max: number; current: number }; } +class TestLogService extends NullLogService { + readonly traces: string[] = []; + readonly errors: (string | Error)[] = []; + + override trace(message: string, ...args: unknown[]): void { + this.traces.push([message, ...args].join(' ')); + } + + override error(error: string | Error, ..._args: unknown[]): void { + this.errors.push(error); + } +} + function createService(store: Pick<{ add(t: T): T }, 'add'>, options: { readonly tasks: readonly unknown[]; /** Repository id -> response, or 'error' to fail the lookup. */ @@ -61,6 +75,8 @@ function createService(store: Pick<{ add(t: T): T readonly retryAfterSeconds?: number; /** Suspend every task-detail response by this many ms, so overlapping fetches are observable. */ readonly taskFetchDelayMs?: number; + readonly requestError?: Error; + readonly logService?: ILogService; }): ITestSetup { const requestedUrls: string[] = []; const concurrency = { max: 0, current: 0 }; @@ -75,6 +91,9 @@ function createService(store: Pick<{ add(t: T): T instantiationService.stub(IRequestService, new class extends mock() { override async request(opts: { url?: string }): Promise { + if (options.requestError) { + throw options.requestError; + } const url = opts.url ?? ''; requestedUrls.push(url); const repoMatch = url.match(/\/repositories\/(\d+)$/); @@ -126,7 +145,7 @@ function createService(store: Pick<{ add(t: T): T override readonly onDidChangeSessions = Event.None; }()); instantiationService.stub(IProductService, { defaultChatAgent: undefined } as unknown as IProductService); - instantiationService.stub(ILogService, new NullLogService()); + instantiationService.stub(ILogService, options.logService ?? new NullLogService()); instantiationService.stub(ICloudSandboxTelemetryService, new class extends mock() { override reportRequest(): void { } }()); @@ -159,6 +178,46 @@ suite('CloudSandboxApiService repository resolution', () => { }); }); + test('propagates task-list cancellation without error logging', async () => { + const logService = new TestLogService(); + const { service } = createService(store, { + tasks: [], + repositories: new Map(), + requestError: new CancellationError(), + logService, + }); + + await assert.rejects(() => service.listSessions(CancellationToken.None), error => isCancellationError(error)); + assert.deepStrictEqual({ + cancelledTraces: logService.traces.filter(message => message.includes(' -> cancelled')).length, + errors: logService.errors, + }, { + cancelledTraces: 1, + errors: [], + }); + }); + + test('normalizes a transport failure after cancellation without error logging', async () => { + const logService = new TestLogService(); + const cancellation = store.add(new CancellationTokenSource()); + cancellation.cancel(); + const { service } = createService(store, { + tasks: [], + repositories: new Map(), + requestError: new Error('transport stopped'), + logService, + }); + + await assert.rejects(() => service.listSessions(cancellation.token), error => isCancellationError(error)); + assert.deepStrictEqual({ + cancelledTraces: logService.traces.filter(message => message.includes(' -> cancelled')).length, + errors: logService.errors, + }, { + cancelledTraces: 1, + errors: [], + }); + }); + test('resolves each repository once across a whole discovery pass', async () => { // Tasks resolve concurrently, so the in-flight promise must be shared, not just the result. const { service, requestedUrls } = createService(store, { diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentCustomizationContentExpander.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentCustomizationContentExpander.ts index 83d98f17bc441..bdc6807c40c18 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentCustomizationContentExpander.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentCustomizationContentExpander.ts @@ -4,11 +4,12 @@ *--------------------------------------------------------------------------------------------*/ import { CancellationToken } from '../../../../../../base/common/cancellation.js'; +import { isCancellationError } from '../../../../../../base/common/errors.js'; import { extname } from '../../../../../../base/common/path.js'; import { joinPath } from '../../../../../../base/common/resources.js'; import { URI } from '../../../../../../base/common/uri.js'; import { parseFrontMatter } from '../../../../../../base/common/yaml.js'; -import { IFileService } from '../../../../../../platform/files/common/files.js'; +import { FileOperationResult, IFileService, toFileOperationResult } from '../../../../../../platform/files/common/files.js'; import { ILogService } from '../../../../../../platform/log/common/log.js'; import { AICustomizationSource } from '../../../common/aiCustomizationWorkspaceService.js'; import { ICustomizationItem } from '../../../common/customizationHarnessService.js'; @@ -42,7 +43,17 @@ export class AgentCustomizationContentExpander { const dirNames = ['agents', 'skills', 'commands', 'rules'] as const; const promptTypes = [PromptsType.agent, PromptsType.skill, PromptsType.prompt, PromptsType.instructions] as const; - const stats = await this.fileService.resolveAll(dirNames.map(name => ({ resource: URI.joinPath(fsRoot, name) }))); + const stats = await Promise.all(dirNames.map(async name => { + const resource = URI.joinPath(fsRoot, name); + try { + return await this.fileService.resolve(resource); + } catch (err) { + if (!isExpectedFileAccessError(err, token)) { + this.logService.trace(`[AgentCustomizationContentExpander] Failed to resolve customization directory ${resource.toString()}: ${err}`); + } + return undefined; + } + })); if (token.isCancellationRequested) { return []; @@ -51,18 +62,20 @@ export class AgentCustomizationContentExpander { for (let i = 0; i < dirNames.length; i++) { const stat = stats[i]; const promptType = promptTypes[i]; - if (!stat.success || !stat.stat?.isDirectory || !stat.stat.children) { + if (!stat?.isDirectory || !stat.children) { continue; } if (promptType === PromptsType.skill) { - children.push(...await this.collectFromSkillDir(stat.stat.children, pluginUri, source, groupKey, isBundleItem, pluginLabel, token)); + children.push(...await this.collectFromSkillDir(stat.children, pluginUri, source, groupKey, isBundleItem, pluginLabel, token)); } else { - children.push(...await this.collectFromRegularDir(stat.stat.children, pluginUri, source, promptType, groupKey, isBundleItem, pluginLabel, token)); + children.push(...await this.collectFromRegularDir(stat.children, pluginUri, source, promptType, groupKey, isBundleItem, pluginLabel, token)); } } children.sort((a, b) => `${a.type}:${a.name}`.localeCompare(`${b.type}:${b.name}`)); } catch (err) { - this.logService.trace(`[AgentCustomizationContentExpander] Failed to expand plugin ${pluginUri.toString()}: ${err}`); + if (!isExpectedFileAccessError(err, token)) { + this.logService.trace(`[AgentCustomizationContentExpander] Failed to expand plugin ${pluginUri.toString()}: ${err}`); + } return []; } return children; @@ -194,12 +207,20 @@ export class AgentCustomizationContentExpander { } return { name: undefined, description: undefined, userInvocable: undefined }; } catch (err) { - this.logService.trace(`[AgentCustomizationContentExpander] Failed to read prompt metadata ${promptFileUri.toString()}: ${err}`); + if (!isExpectedFileAccessError(err, token)) { + this.logService.trace(`[AgentCustomizationContentExpander] Failed to read prompt metadata ${promptFileUri.toString()}: ${err}`); + } return undefined; } } } +function isExpectedFileAccessError(error: Error, token: CancellationToken): boolean { + return token.isCancellationRequested + || isCancellationError(error) + || toFileOperationResult(error) === FileOperationResult.FILE_NOT_FOUND; +} + /** * Strips conventional prompt file extensions so we can show `foo` * for `foo.prompt.md`, `foo.instructions.md`, etc. diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/syncedCustomizationBundler.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/syncedCustomizationBundler.ts index 334139dc504b5..6aea301f22697 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/syncedCustomizationBundler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/syncedCustomizationBundler.ts @@ -6,7 +6,7 @@ import { Limiter, SequencerByKey } from '../../../../../../base/common/async.js'; import { VSBuffer } from '../../../../../../base/common/buffer.js'; import { CancellationError, isCancellationError } from '../../../../../../base/common/errors.js'; -import { Disposable, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; +import { Disposable, IDisposable } from '../../../../../../base/common/lifecycle.js'; import { equals } from '../../../../../../base/common/objects.js'; import { ResourceMap } from '../../../../../../base/common/map.js'; import { basename, dirname, extUri } from '../../../../../../base/common/resources.js'; @@ -57,6 +57,38 @@ function pluginDirForType(type: PromptsType): string | undefined { type QueueFileOperation = (operation: () => Promise) => Promise; +/** Cancels queued operations on disposal while allowing already-started operations to settle. */ +class DrainingFileOperationLimiter implements IDisposable { + + private readonly _limiter = new Limiter(FILE_OPERATION_CONCURRENCY); + private _isDisposed = false; + + queue(operation: () => Promise): Promise { + if (this._isDisposed) { + return Promise.reject(new CancellationError()); + } + return this._limiter.queue(async () => { + if (this._isDisposed) { + throw new CancellationError(); + } + return operation(); + }) as Promise; + } + + dispose(): void { + if (this._isDisposed) { + return; + } + this._isDisposed = true; + void this._disposeWhenIdle(); + } + + private async _disposeWhenIdle(): Promise { + await this._limiter.whenIdle(); + this._limiter.dispose(); + } +} + async function collectDirectoryFiles(fileService: IFileService, logService: ILogService, root: URI, directory: URI, queueFileOperation: QueueFileOperation): Promise { const stat = await queueFileOperation(() => fileService.resolve(directory)); const children = (await Promise.all((stat.children ?? []).map(async child => { @@ -159,7 +191,7 @@ interface IBundleResult { */ export class SyncedCustomizationBundler extends Disposable { - private readonly _fileOperationLimiter = this._register(new MutableDisposable>()); + private readonly _fileOperationLimiter = this._register(new DrainingFileOperationLimiter()); private readonly _authority: string; private _lastNonce: string | undefined; private _lastRef: IBundleResult | undefined; @@ -174,7 +206,6 @@ export class SyncedCustomizationBundler extends Disposable { @ILogService private readonly _logService: ILogService, ) { super(); - this._fileOperationLimiter.value = new Limiter(FILE_OPERATION_CONCURRENCY); this._authority = authority; agentHostFileSystemService.ensureSyncedCustomizationProvider(); } @@ -190,11 +221,7 @@ export class SyncedCustomizationBundler extends Disposable { private _queueFileOperation(operation: () => Promise): Promise { this._throwIfDisposed(); - const limiter = this._fileOperationLimiter.value; - if (!limiter) { - throw new CancellationError(); - } - return limiter.queue(operation) as Promise; + return this._fileOperationLimiter.queue(operation); } private _throwIfDisposed(): void { @@ -214,7 +241,14 @@ export class SyncedCustomizationBundler extends Disposable { */ async bundle(files: readonly ISyncableFile[], mcpServers: readonly ISyncableMcpServer[] = []): Promise { this._throwIfDisposed(); - return bundleSequencer.queue(this._authority, () => this._bundle(files, mcpServers)); + try { + const result = await bundleSequencer.queue(this._authority, () => this._bundle(files, mcpServers)); + this._throwIfDisposed(); + return result; + } catch (error) { + this._throwIfDisposed(); + throw error; + } } private async _bundle(files: readonly ISyncableFile[], mcpServers: readonly ISyncableMcpServer[]): Promise { @@ -402,16 +436,6 @@ export class SyncedCustomizationBundler extends Disposable { return; } this._isDisposed = true; - // Keep the limiter alive until file operations queued before disposal have settled. - const limiter = this._fileOperationLimiter.clearAndLeak(); super.dispose(); - if (!limiter) { - return; - } - if (limiter.size === 0) { - limiter.dispose(); - } else { - void limiter.whenIdle().then(() => limiter.dispose()); - } } } diff --git a/src/vs/workbench/contrib/chat/test/browser/agentHost/agentCustomizationContentExpander.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentHost/agentCustomizationContentExpander.test.ts index 9f923305739dd..add56943a6a08 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentHost/agentCustomizationContentExpander.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentHost/agentCustomizationContentExpander.test.ts @@ -21,6 +21,14 @@ import { CancellationToken } from '../../../../../../base/common/cancellation.js const REMOTE_HOST_GROUP = 'remote-host'; const REMOTE_CLIENT_GROUP = 'remote-client'; +class TestLogService extends NullLogService { + readonly traces: string[] = []; + + override trace(message: string, ...args: unknown[]): void { + this.traces.push([message, ...args].join(' ')); + } +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -79,12 +87,17 @@ suite('AgentCustomizationContentExpander', () => { }, ]); - const expander = new AgentCustomizationContentExpander(fileService, new NullLogService()); + const logService = new TestLogService(); + const expander = new AgentCustomizationContentExpander(fileService, logService); const items = await expand(expander, pluginRoot, REMOTE_HOST_GROUP, false, AICustomizationSources.plugin, CancellationToken.None); - assert.deepStrictEqual(items.map(i => ({ type: i.type, name: i.name, description: i.description })), [ - { type: PromptsType.skill, name: 'Lint', description: 'Runs linting' }, - ]); + assert.deepStrictEqual({ + items: items.map(i => ({ type: i.type, name: i.name, description: i.description })), + traces: logService.traces, + }, { + items: [{ type: PromptsType.skill, name: 'Lint', description: 'Runs linting' }], + traces: [], + }); }); test('uses folder name as fallback when SKILL.md has no name frontmatter', async () => { diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/syncedCustomizationBundler.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/syncedCustomizationBundler.test.ts index e1d4cb62d814a..70ceccea0b40a 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/syncedCustomizationBundler.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/syncedCustomizationBundler.test.ts @@ -30,6 +30,7 @@ class TestInMemoryFileSystemProvider extends InMemoryFileSystemProvider { private statDelay = 0; activeStats = 0; maxActiveStats = 0; + statCalls = 0; markSymbolicLink(resource: URI): void { this.symbolicLinks.add(resource); @@ -44,6 +45,7 @@ class TestInMemoryFileSystemProvider extends InMemoryFileSystemProvider { } override async stat(resource: URI): Promise { + this.statCalls++; this.activeStats++; this.maxActiveStats = Math.max(this.maxActiveStats, this.activeStats); try { @@ -332,10 +334,30 @@ suite('SyncedCustomizationBundler', () => { while (memFs.maxActiveStats < 10) { await timeout(0); } + const statCallsAtDisposal = memFs.statCalls; bundler.dispose(); await assert.rejects(bundle, error => isCancellationError(error)); await assert.rejects(bundler.bundle([{ uri: skill, type: PromptsType.skill }]), error => isCancellationError(error)); + while (memFs.activeStats > 0) { + await timeout(0); + } + assert.strictEqual(memFs.statCalls, statCallsAtDisposal); + }); + + test('normalizes an in-flight provider failure after disposal to cancellation', async () => { + const bundler = createBundler(); + const resource = await seedFile('/test/unavailable.md', 'content'); + memFs.delayStats(20); + memFs.failStat(resource); + + const bundle = bundler.bundle([{ uri: resource, type: PromptsType.instructions }]); + while (memFs.activeStats === 0) { + await timeout(0); + } + bundler.dispose(); + + await assert.rejects(bundle, error => isCancellationError(error)); }); test('serializes replacement bundles for the same authority', async () => { From 7434d145bfeb34f40477422f017df60587833e02 Mon Sep 17 00:00:00 2001 From: Paul Wang Date: Sat, 5 Sep 2026 04:38:40 -0700 Subject: [PATCH 3/3] Update customization harness file mocks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...emoteAgentHostCustomizationHarness.test.ts | 143 +++++++++--------- 1 file changed, 72 insertions(+), 71 deletions(-) diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostCustomizationHarness.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostCustomizationHarness.test.ts index 2c3711e852f4c..dd74806fde9c3 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostCustomizationHarness.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostCustomizationHarness.test.ts @@ -19,7 +19,7 @@ import { sessionReducer } from '../../../../../../platform/agentHost/common/stat import { type IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; import { IFileDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; import { VSBuffer } from '../../../../../../base/common/buffer.js'; -import { IFileService, type IFileContent, type IFileStat, type IFileStatResult } from '../../../../../../platform/files/common/files.js'; +import { FileOperationError, FileOperationResult, IFileService, type IFileContent, type IFileStat, type IFileStatWithMetadata } from '../../../../../../platform/files/common/files.js'; import { PromptsType } from '../../../../../../workbench/contrib/chat/common/promptSyntax/promptTypes.js'; import { NullLogService } from '../../../../../../platform/log/common/log.js'; import { INotificationService } from '../../../../../../platform/notification/common/notification.js'; @@ -44,6 +44,26 @@ class MockPromptsService extends BaseMockPromptsService { } } +/** Throws the file-not-found result used for absent optional plugin directories. */ +function throwFileNotFound(): never { + throw new FileOperationError('File not found', FileOperationResult.FILE_NOT_FOUND); +} + +/** Adds deterministic metadata to a test file stat. */ +function withMetadata(stat: IFileStat): IFileStatWithMetadata { + return { + ...stat, + mtime: stat.mtime ?? 0, + ctime: stat.ctime ?? 0, + etag: stat.etag ?? '', + size: stat.size ?? 0, + readonly: stat.readonly ?? false, + locked: stat.locked ?? false, + executable: stat.executable ?? false, + children: stat.children?.map(withMetadata), + }; +} + class MockAgentConnection extends mock() { private readonly _onDidAction = new Emitter(); @@ -238,7 +258,6 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return false; } - override async resolveAll() { return []; } }; const provider = disposables.add(new AgentCustomizationItemProvider( @@ -298,7 +317,6 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return false; } - override async resolveAll() { return []; } }; const provider = disposables.add(new AgentCustomizationItemProvider( @@ -341,7 +359,6 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return false; } - override async resolveAll() { return []; } }; const provider = disposables.add(new AgentCustomizationItemProvider( @@ -391,37 +408,32 @@ suite('RemoteAgentHostCustomizationHarness', () => { const skillFileUri = URI.parse(`${bundleUri}/skills/my-skill`); const fileService = new class extends mock() { override async canHandleResource() { return true; } - override async resolveAll(resources: { resource: URI }[]): Promise { - return resources.map(r => { - if (r.resource.path.endsWith('/skills')) { - return { - success: true, - stat: { - resource: r.resource, - name: 'skills', - isFile: false, - isDirectory: true, - isSymbolicLink: false, - readonly: false, - mtime: 0, - ctime: 0, - size: 0, - children: [{ - name: 'my-skill', - resource: skillFileUri, - isFile: false, - isDirectory: true, - isSymbolicLink: false, - readonly: false, - mtime: 0, - ctime: 0, - size: 0, - children: [], - }], - }, - } satisfies IFileStatResult; - } - return { success: false, stat: undefined } as unknown as IFileStatResult; + override async resolve(resource: URI): Promise { + if (!resource.path.endsWith('/skills')) { + throwFileNotFound(); + } + return withMetadata({ + resource, + name: 'skills', + isFile: false, + isDirectory: true, + isSymbolicLink: false, + readonly: false, + mtime: 0, + ctime: 0, + size: 0, + children: [{ + name: 'my-skill', + resource: skillFileUri, + isFile: false, + isDirectory: true, + isSymbolicLink: false, + readonly: false, + mtime: 0, + ctime: 0, + size: 0, + children: [], + }], }); } override async readFile(resource: URI): Promise { @@ -476,7 +488,6 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return false; } - override async resolveAll() { return []; } }; const provider = disposables.add(new AgentCustomizationItemProvider( @@ -523,7 +534,6 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return false; } - override async resolveAll() { return []; } }; const provider = disposables.add(new AgentCustomizationItemProvider( @@ -562,7 +572,6 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return false; } - override async resolveAll() { return []; } }; const provider = disposables.add(new AgentCustomizationItemProvider( @@ -645,7 +654,6 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return false; } - override async resolveAll() { return []; } }; const provider = disposables.add(new AgentCustomizationItemProvider( @@ -743,16 +751,11 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return true; } - override async resolveAll(toResolve: { resource: URI }[]): Promise { - return toResolve.map(({ resource }) => { - if (resource.path.endsWith('/skills')) { - return { - success: true, - stat: { name: 'skills', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: skillsDirChildren }, - }; - } - return { success: false }; - }); + override async resolve(resource: URI): Promise { + if (!resource.path.endsWith('/skills')) { + throwFileNotFound(); + } + return withMetadata({ name: 'skills', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: skillsDirChildren }); } override async readFile(resource: URI): Promise { if (resource.path.endsWith('/valid-skill/SKILL.md')) { @@ -808,10 +811,11 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return true; } - override async resolveAll(toResolve: { resource: URI }[]): Promise { - return toResolve.map(({ resource }) => resource.path.endsWith('/rules') - ? { success: true, stat: { name: 'rules', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: rulesDirChildren } } - : { success: false }); + override async resolve(resource: URI): Promise { + if (!resource.path.endsWith('/rules')) { + throwFileNotFound(); + } + return withMetadata({ name: 'rules', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: rulesDirChildren }); } override async readFile(resource: URI): Promise { const content = '---\nname: My Rule\ndescription: A synced rule\n---\n'; @@ -860,10 +864,11 @@ suite('RemoteAgentHostCustomizationHarness', () => { const ruleResource = URI.parse(`${bundleUri}/rules/user-rule.instructions.md`); const fileService = new class extends mock() { override async canHandleResource() { return true; } - override async resolveAll(toResolve: { resource: URI }[]): Promise { - return toResolve.map(({ resource }) => resource.path.endsWith('/rules') - ? { success: true, stat: { name: 'rules', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: [{ name: 'user-rule.instructions.md', resource: ruleResource, isFile: true, isDirectory: false, isSymbolicLink: false, children: undefined }] } } - : { success: false }); + override async resolve(resource: URI): Promise { + if (!resource.path.endsWith('/rules')) { + throwFileNotFound(); + } + return withMetadata({ name: 'rules', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: [{ name: 'user-rule.instructions.md', resource: ruleResource, isFile: true, isDirectory: false, isSymbolicLink: false, children: undefined }] }); } override async readFile(resource: URI): Promise { const content = 'User rule'; @@ -921,10 +926,11 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return true; } - override async resolveAll(toResolve: { resource: URI }[]): Promise { - return toResolve.map(({ resource }) => resource.path.endsWith('/rules') - ? { success: true, stat: { name: 'rules', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: rulesDirChildren } } - : { success: false }); + override async resolve(resource: URI): Promise { + if (!resource.path.endsWith('/rules')) { + throwFileNotFound(); + } + return withMetadata({ name: 'rules', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: rulesDirChildren }); } override async readFile(resource: URI): Promise { const content = '---\nname: My Rule\n---\n'; @@ -972,16 +978,11 @@ suite('RemoteAgentHostCustomizationHarness', () => { const fileService = new class extends mock() { override async canHandleResource() { return true; } - override async resolveAll(toResolve: { resource: URI }[]): Promise { - return toResolve.map(({ resource }) => { - if (resource.path.endsWith('/skills')) { - return { - success: true, - stat: { name: 'skills', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: skillsDirChildren }, - }; - } - return { success: false }; - }); + override async resolve(resource: URI): Promise { + if (!resource.path.endsWith('/skills')) { + throwFileNotFound(); + } + return withMetadata({ name: 'skills', resource, isFile: false, isDirectory: true, isSymbolicLink: false, children: skillsDirChildren }); } override async readFile(resource: URI): Promise { if (resource.path.endsWith('/lint/SKILL.md')) {