From 88dc2bd2fac430a470b620941e8ca0deef325bf6 Mon Sep 17 00:00:00 2001 From: Om Singhal Date: Thu, 5 Nov 2026 14:24:45 -0500 Subject: [PATCH 1/7] Avoid a full workspace search per Python file event The workspaceHasPython watcher called findFiles for every created or deleted marker file, and each call starts its own ripgrep process, so a branch switch that touches a thousand Python files started a thousand workspace searches at once. A created marker file or an opened Python document now sets the context to true without searching. Deletes and workspace folder changes share one rescan, debounced by 500 ms. Only one search runs at a time, a rescan requested meanwhile runs once after it, and a search is cancelled when its result is stale or the extension is disposed. The context is only set when its value changes. --- src/features/views/workspacePythonContext.ts | 119 +++++++-- .../views/workspacePythonContext.unit.test.ts | 248 ++++++++++++++---- 2 files changed, 303 insertions(+), 64 deletions(-) diff --git a/src/features/views/workspacePythonContext.ts b/src/features/views/workspacePythonContext.ts index 4a5cb76cb..406535bbf 100644 --- a/src/features/views/workspacePythonContext.ts +++ b/src/features/views/workspacePythonContext.ts @@ -1,6 +1,7 @@ -import { Disposable, Uri } from 'vscode'; +import { CancellationToken, CancellationTokenSource, Disposable, Uri } from 'vscode'; import { executeCommand } from '../../common/command.api'; import { traceError } from '../../common/logging'; +import { createSimpleDebounce } from '../../common/utils/debounce'; import { createFileSystemWatcher, findFiles, @@ -27,8 +28,11 @@ const EXCLUDE = '**/{node_modules,.git,site-packages}/**'; // `createFileSystemWatcher` has no exclude option, so the same folders are filtered in the handlers. const EXCLUDE_RE = /[\\/](node_modules|\.git|site-packages)[\\/]/; -async function hasPythonContent(): Promise { - const markers = await findFiles(MARKER_GLOB, EXCLUDE, 1); +/** How long deletes and workspace folder changes must settle before the workspace is scanned again. */ +export const RESCAN_DEBOUNCE_MS = 500; + +async function hasPythonContent(token: CancellationToken): Promise { + const markers = await findFiles(MARKER_GLOB, EXCLUDE, 1, token); if (markers.length > 0) { return true; } @@ -39,41 +43,116 @@ async function hasPythonContent(): Promise { /** * Keeps the `python-envs.workspaceHasPython` context key up to date. It controls whether the Python * activity bar icon and its views are shown. + * + * Every `findFiles` call is a full workspace search, and a branch switch can create or delete + * thousands of marker files at once, so events never scan directly: + * - a new marker file or an opened Python document proves the answer is `true` without a scan; + * - deletes and workspace folder changes can turn it `false`, so they share one debounced rescan; + * - only one scan runs at a time, and a rescan requested meanwhile runs once after it. */ export function registerWorkspacePythonContext(disposables: Disposable[]): void { - // Every refresh takes a ticket and only the newest one may write the context key, so a slow older - // scan can never overwrite the result of a newer one. - let latestTicket = 0; + let hasPython: boolean | undefined; + let running: CancellationTokenSource | undefined; + let rescanAfterRunning = false; + // `SimpleDebounce` cannot be cancelled, so a pending rescan is dropped by clearing this flag. + let rescanPending = false; + let disposed = false; + + const setHasPython = async (value: boolean): Promise => { + if (value === hasPython) { + return; + } + hasPython = value; + await executeCommand('setContext', PYTHON_WORKSPACE_KEY, value); + }; - const refresh = async (): Promise => { - const ticket = ++latestTicket; + const scan = async (): Promise => { + if (disposed) { + return; + } + if (running) { + rescanAfterRunning = true; + return; + } + const cts = new CancellationTokenSource(); + running = cts; try { - const hasPython = await hasPythonContent(); - if (ticket === latestTicket) { - await executeCommand('setContext', PYTHON_WORKSPACE_KEY, hasPython); + const result = await hasPythonContent(cts.token); + // A cancelled scan was overtaken by `markHasPython` or by disposal, so its result is stale. + if (!cts.token.isCancellationRequested) { + await setHasPython(result); } } catch (ex) { - traceError('Failed to update the Python workspace context', ex); + if (!cts.token.isCancellationRequested) { + traceError('Failed to update the Python workspace context', ex); + } + } finally { + cts.dispose(); + running = undefined; + } + if (rescanAfterRunning && !disposed) { + rescanAfterRunning = false; + await scan(); + } + }; + + const debouncedRescan = createSimpleDebounce(RESCAN_DEBOUNCE_MS, () => { + if (rescanPending) { + rescanPending = false; + void scan(); + } + }); + + const scheduleRescan = (): void => { + if (!disposed) { + rescanPending = true; + debouncedRescan.trigger(); } }; - const onMarkerChanged = (uri: Uri): void => { + // A new marker file or an opened Python document proves the answer is `true`, whatever a running or + // pending scan would report. + const markHasPython = (): void => { + running?.cancel(); + rescanPending = false; + rescanAfterRunning = false; + setHasPython(true).catch((ex) => traceError('Failed to update the Python workspace context', ex)); + }; + + const onMarkerCreated = (uri: Uri): void => { if (!EXCLUDE_RE.test(uri.fsPath)) { - void refresh(); + markHasPython(); } }; + const onMarkerDeleted = (uri: Uri): void => { + if (EXCLUDE_RE.test(uri.fsPath)) { + return; + } + // A delete can only turn the answer off, so there is nothing to recheck while it is already off, + // unless a running scan may still report the deleted file. + if (hasPython === false && !running) { + return; + } + scheduleRescan(); + }; + const watcher = createFileSystemWatcher(MARKER_GLOB, false, true, false); disposables.push( watcher, - watcher.onDidCreate(onMarkerChanged), - watcher.onDidDelete(onMarkerChanged), - onDidChangeWorkspaceFolders(() => void refresh()), + watcher.onDidCreate(onMarkerCreated), + watcher.onDidDelete(onMarkerDeleted), + onDidChangeWorkspaceFolders(() => scheduleRescan()), onDidOpenTextDocument((doc) => { if (doc.languageId === 'python') { - void refresh(); + markHasPython(); } }), + new Disposable(() => { + disposed = true; + debouncedRescan.dispose(); + running?.cancel(); + }), ); - void refresh(); -} \ No newline at end of file + void scan(); +} diff --git a/src/test/features/views/workspacePythonContext.unit.test.ts b/src/test/features/views/workspacePythonContext.unit.test.ts index d20c6cd8b..3c64b66cf 100644 --- a/src/test/features/views/workspacePythonContext.unit.test.ts +++ b/src/test/features/views/workspacePythonContext.unit.test.ts @@ -8,6 +8,7 @@ import { MARKER_GLOB, PYTHON_WORKSPACE_KEY, registerWorkspacePythonContext, + RESCAN_DEBOUNCE_MS, } from '../../../features/views/workspacePythonContext'; type Listener = (e: T) => void; @@ -26,6 +27,7 @@ function flush(): Promise { const marker = (fsPath: string) => ({ fsPath }) as any; suite('Workspace Python context', () => { + let clock: sinon.SinonFakeTimers; let findFilesStub: sinon.SinonStub; let openDocsStub: sinon.SinonStub; let executeCommandStub: sinon.SinonStub; @@ -45,8 +47,16 @@ suite('Workspace Python context', () => { assert.strictEqual(c.args[1], PYTHON_WORKSPACE_KEY); return c.args[2]; }); + /** Runs the debounced rescan, then lets the scan it starts settle. */ + const settle = async () => { + await clock.tickAsync(RESCAN_DEBOUNCE_MS); + await flush(); + }; + const tokenOfScan = (index: number) => findFilesStub.getCall(index).args[3]; setup(() => { + // Only timers are faked, so `flush` (setImmediate) still lets promises settle. + clock = sinon.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); findFilesStub = sinon.stub(workspaceApis, 'findFiles').resolves([]); openDocsStub = sinon.stub(workspaceApis, 'getOpenTextDocuments').returns([]); executeCommandStub = sinon.stub(commandApis, 'executeCommand').resolves(undefined); @@ -65,6 +75,7 @@ suite('Workspace Python context', () => { }); teardown(() => { + disposables.forEach((d) => d.dispose()); sinon.restore(); }); @@ -92,17 +103,18 @@ suite('Workspace Python context', () => { test('searches for a single hit, excluding dependency folders', async () => { registerWorkspacePythonContext(disposables); await flush(); - const [include, exclude, maxResults] = findFilesStub.firstCall.args; + const [include, exclude, maxResults, token] = findFilesStub.firstCall.args; assert.strictEqual(include, MARKER_GLOB); assert.ok(String(exclude).includes('site-packages')); assert.ok(String(exclude).includes('node_modules')); assert.strictEqual(maxResults, 1); + assert.ok(token, 'the scan should be cancellable'); }); test('registers its watcher and listeners for disposal', () => { registerWorkspacePythonContext(disposables); assert.ok(disposables.includes(watcher)); - assert.strictEqual(disposables.length, 5); + assert.strictEqual(disposables.length, 6); }); }); @@ -120,22 +132,24 @@ suite('Workspace Python context', () => { }); suite('transitions', () => { - test('creating the first marker turns the context on', async () => { + test('creating the first marker turns the context on without a rescan', async () => { registerWorkspacePythonContext(disposables); await flush(); - findFilesStub.resolves([marker('/ws/app/main.py')]); fireCreate('/ws/app/main.py'); - await flush(); + await settle(); assert.deepStrictEqual(contextValues(), [false, true]); + assert.strictEqual(findFilesStub.callCount, 1); }); - test('deleting the last marker turns the context off', async () => { + test('deleting the last marker turns the context off after the debounce', async () => { findFilesStub.resolves([marker('/ws/main.py')]); registerWorkspacePythonContext(disposables); await flush(); findFilesStub.resolves([]); fireDelete('/ws/main.py'); await flush(); + assert.strictEqual(findFilesStub.callCount, 1, 'the rescan should wait for the debounce'); + await settle(); assert.deepStrictEqual(contextValues(), [true, false]); }); @@ -144,26 +158,174 @@ suite('Workspace Python context', () => { await flush(); findFilesStub.resolves([marker('/other/requirements.txt')]); fireFoldersChanged(); - await flush(); + await settle(); assert.deepStrictEqual(contextValues(), [false, true]); }); - test('opening a Python document re-evaluates the context', async () => { + test('opening a Python document turns the context on without a rescan', async () => { registerWorkspacePythonContext(disposables); await flush(); - openDocsStub.returns([{ languageId: 'python' }]); fireDocOpened('python'); - await flush(); + await settle(); assert.deepStrictEqual(contextValues(), [false, true]); + assert.strictEqual(findFilesStub.callCount, 1); }); test('opening a non-Python document does not trigger a rescan', async () => { registerWorkspacePythonContext(disposables); await flush(); fireDocOpened('markdown'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 1); + assert.deepStrictEqual(contextValues(), [false]); + }); + + test('only updates the context when the value changes', async () => { + findFilesStub.resolves([marker('/ws/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + fireDelete('/ws/old.py'); + await settle(); + fireCreate('/ws/new.py'); + fireDocOpened('python'); + await flush(); + assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [true]); + }); + }); + + suite('event bursts', () => { + test('a burst of creates does not scan the workspace', async () => { + registerWorkspacePythonContext(disposables); + await flush(); + for (let i = 0; i < 1000; i++) { + fireCreate(`/ws/pkg/mod${i}.py`); + } + await settle(); + assert.strictEqual(findFilesStub.callCount, 1, 'only the initial scan should have run'); + assert.deepStrictEqual(contextValues(), [false, true]); + }); + + test('a burst of deletes is coalesced into one rescan', async () => { + findFilesStub.resolves([marker('/ws/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + for (let i = 0; i < 1000; i++) { + fireDelete(`/ws/pkg/mod${i}.py`); + } + await clock.tickAsync(RESCAN_DEBOUNCE_MS - 1); + assert.strictEqual(findFilesStub.callCount, 1); + await clock.tickAsync(1); + await flush(); + assert.strictEqual(findFilesStub.callCount, 2); + await settle(); + assert.strictEqual(findFilesStub.callCount, 2); + }); + + test('deletes are ignored while the workspace has no Python', async () => { + registerWorkspacePythonContext(disposables); + await flush(); + for (let i = 0; i < 1000; i++) { + fireDelete(`/ws/pkg/mod${i}.py`); + } + await settle(); + assert.strictEqual(findFilesStub.callCount, 1); + assert.deepStrictEqual(contextValues(), [false]); + }); + + test('a burst of workspace folder changes is coalesced into one rescan', async () => { + registerWorkspacePythonContext(disposables); + await flush(); + for (let i = 0; i < 50; i++) { + fireFoldersChanged(); + } + await settle(); + assert.strictEqual(findFilesStub.callCount, 2); + }); + + test('a create cancels a pending rescan', async () => { + findFilesStub.resolves([marker('/ws/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + fireDelete('/ws/old/main.py'); + fireCreate('/ws/new/main.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 1); + assert.deepStrictEqual(contextValues(), [true]); + }); + }); + + suite('overlapping scans', () => { + test('deletes during a scan rerun it once after it finishes, never concurrently', async () => { + let inFlight = 0; + let maxInFlight = 0; + const pending: { resolve: (value: any[]) => void }[] = []; + findFilesStub.callsFake(() => { + inFlight++; + maxInFlight = Math.max(maxInFlight, inFlight); + const deferred = makeDeferred(); + pending.push(deferred); + return deferred.promise.finally(() => inFlight--); + }); + + registerWorkspacePythonContext(disposables); + pending[0].resolve([marker('/ws/main.py')]); + await flush(); + + fireDelete('/ws/a.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 2, 'the debounced rescan should be running'); + + for (let i = 0; i < 1000; i++) { + fireDelete(`/ws/pkg/mod${i}.py`); + } + await settle(); + assert.strictEqual(findFilesStub.callCount, 2, 'no second scan while one is running'); + + pending[1].resolve([marker('/ws/main.py')]); await flush(); + assert.strictEqual(findFilesStub.callCount, 3, 'exactly one follow up scan'); + pending[2].resolve([]); + await settle(); + + assert.strictEqual(findFilesStub.callCount, 3); + assert.strictEqual(maxInFlight, 1); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + + test('a create during a scan wins over a stale empty result', async () => { + const initial = makeDeferred(); + findFilesStub.returns(initial.promise); + + registerWorkspacePythonContext(disposables); + fireCreate('/ws/main.py'); + await flush(); + assert.deepStrictEqual(contextValues(), [true]); + assert.strictEqual(tokenOfScan(0).isCancellationRequested, true, 'the stale scan should be cancelled'); + + initial.resolve([]); + await settle(); + assert.deepStrictEqual(contextValues(), [true]); assert.strictEqual(findFilesStub.callCount, 1); }); + + test('a delete racing a scan is rechecked even while the context is off', async () => { + const folderScan = makeDeferred(); + registerWorkspacePythonContext(disposables); + await flush(); + findFilesStub.onSecondCall().returns(folderScan.promise); + findFilesStub.onThirdCall().resolves([]); + + fireFoldersChanged(); + await settle(); + fireDelete('/other/main.py'); // removed after the running scan already listed it + folderScan.resolve([marker('/other/main.py')]); + await flush(); + await settle(); + + assert.strictEqual(findFilesStub.callCount, 3); + assert.deepStrictEqual(contextValues(), [false, true, false]); + }); }); suite('exclusions', () => { @@ -173,14 +335,23 @@ suite('Workspace Python context', () => { '/ws/node_modules/tool/setup.py', '/ws/.git/hooks/pre-commit.py', ].forEach((excluded) => { - test(`ignores create and delete events under ${excluded}`, async () => { + test(`ignores create events under ${excluded}`, async () => { registerWorkspacePythonContext(disposables); await flush(); fireCreate(excluded); - fireDelete(excluded); + await settle(); + assert.strictEqual(findFilesStub.callCount, 1, 'only the initial scan should have run'); + assert.deepStrictEqual(contextValues(), [false]); + }); + + test(`ignores delete events under ${excluded}`, async () => { + findFilesStub.resolves([marker('/ws/main.py')]); + registerWorkspacePythonContext(disposables); await flush(); + fireDelete(excluded); + await settle(); assert.strictEqual(findFilesStub.callCount, 1, 'only the initial scan should have run'); - assert.strictEqual(executeCommandStub.callCount, 1); + assert.deepStrictEqual(contextValues(), [true]); }); }); @@ -189,43 +360,33 @@ suite('Workspace Python context', () => { await flush(); fireCreate('/ws/my-site-packages-notes/main.py'); await flush(); - assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [false, true]); }); }); - suite('overlapping refreshes', () => { - test('an older scan that finishes last does not overwrite newer state', async () => { - const older = makeDeferred(); - const newer = makeDeferred(); - findFilesStub.onFirstCall().returns(older.promise); - findFilesStub.onSecondCall().returns(newer.promise); - - registerWorkspacePythonContext(disposables); // older scan: started first, still running - fireDelete('/ws/main.py'); // newer scan: started second - - newer.resolve([]); // newer result (false) arrives first... - await flush(); - older.resolve([marker('/ws/main.py')]); // ...then the stale result (true) arrives + suite('disposal', () => { + test('cancels a pending rescan', async () => { + findFilesStub.resolves([marker('/ws/main.py')]); + registerWorkspacePythonContext(disposables); await flush(); - - assert.deepStrictEqual(contextValues(), [false]); + fireDelete('/ws/main.py'); + disposables.forEach((d) => d.dispose()); + assert.strictEqual(clock.countTimers(), 0); + await settle(); + fireDelete('/ws/other.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 1); }); - test('the newest scan wins when scans finish in order', async () => { - const older = makeDeferred(); - const newer = makeDeferred(); - findFilesStub.onFirstCall().returns(older.promise); - findFilesStub.onSecondCall().returns(newer.promise); - + test('cancels the running scan and ignores its result', async () => { + const scan = makeDeferred(); + findFilesStub.returns(scan.promise); registerWorkspacePythonContext(disposables); - fireCreate('/ws/main.py'); - - older.resolve([]); - await flush(); - newer.resolve([marker('/ws/main.py')]); + disposables.forEach((d) => d.dispose()); + assert.strictEqual(tokenOfScan(0).isCancellationRequested, true); + scan.resolve([marker('/ws/main.py')]); await flush(); - - assert.deepStrictEqual(contextValues(), [true]); + assert.strictEqual(executeCommandStub.callCount, 0); }); }); @@ -253,10 +414,9 @@ suite('Workspace Python context', () => { findFilesStub.onFirstCall().rejects(new Error('boom')); registerWorkspacePythonContext(disposables); await flush(); - findFilesStub.resolves([marker('/ws/main.py')]); fireCreate('/ws/main.py'); await flush(); assert.deepStrictEqual(contextValues(), [true]); }); }); -}); \ No newline at end of file +}); From cecefba9c67abf6d560934ee8b2580adfcd512f5 Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Tue, 10 Nov 2026 16:46:47 -0800 Subject: [PATCH 2/7] Address workspace Python context review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cc0aea3d-0a7e-4799-a32b-3a851a5ad484 --- src/features/views/workspacePythonContext.ts | 126 +++++++++-- .../views/workspacePythonContext.unit.test.ts | 211 +++++++++++++++++- 2 files changed, 309 insertions(+), 28 deletions(-) diff --git a/src/features/views/workspacePythonContext.ts b/src/features/views/workspacePythonContext.ts index 406535bbf..c1c2ccd3f 100644 --- a/src/features/views/workspacePythonContext.ts +++ b/src/features/views/workspacePythonContext.ts @@ -1,7 +1,9 @@ -import { CancellationToken, CancellationTokenSource, Disposable, Uri } from 'vscode'; +import { CancellationToken, CancellationTokenSource, Disposable, FileType, Uri } from 'vscode'; import { executeCommand } from '../../common/command.api'; -import { traceError } from '../../common/logging'; +import { traceError, traceVerbose } from '../../common/logging'; import { createSimpleDebounce } from '../../common/utils/debounce'; +import { normalizePath } from '../../common/utils/pathUtils'; +import { stat } from '../../common/workspace.fs.apis'; import { createFileSystemWatcher, findFiles, @@ -12,6 +14,11 @@ import { export const PYTHON_WORKSPACE_KEY = 'python-envs.workspaceHasPython'; +/** How long deletes and workspace folder changes must settle before the workspace is scanned again. */ +export const RESCAN_DEBOUNCE_MS = 500; +/** A slow workspace search must not block context updates indefinitely. */ +export const SCAN_TIMEOUT_MS = 5000; + /** * Files that identify a workspace as a Python one. * @@ -28,16 +35,17 @@ const EXCLUDE = '**/{node_modules,.git,site-packages}/**'; // `createFileSystemWatcher` has no exclude option, so the same folders are filtered in the handlers. const EXCLUDE_RE = /[\\/](node_modules|\.git|site-packages)[\\/]/; -/** How long deletes and workspace folder changes must settle before the workspace is scanned again. */ -export const RESCAN_DEBOUNCE_MS = 500; +function getMarkerKey(uri: Uri): string { + return JSON.stringify([uri.scheme, uri.authority, normalizePath(uri.fsPath), uri.query, uri.fragment]); +} -async function hasPythonContent(token: CancellationToken): Promise { +async function findPythonContent(token: CancellationToken): Promise<{ hasPython: boolean; marker?: Uri }> { const markers = await findFiles(MARKER_GLOB, EXCLUDE, 1, token); - if (markers.length > 0) { - return true; - } // `findFiles` only searches workspace folders, so also cover a standalone Python file opened without a folder. - return getOpenTextDocuments().some((doc) => doc.languageId === 'python'); + return { + hasPython: markers.length > 0 || getOpenTextDocuments().some((doc) => doc.languageId === 'python'), + marker: markers[0], + }; } /** @@ -57,13 +65,24 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void // `SimpleDebounce` cannot be cancelled, so a pending rescan is dropped by clearing this flag. let rescanPending = false; let disposed = false; + let knownMarker: string | undefined; + let nextCreateId = 0; + const pendingCreates = new Map(); + let deletedWhileScanning = new Set(); const setHasPython = async (value: boolean): Promise => { if (value === hasPython) { return; } hasPython = value; - await executeCommand('setContext', PYTHON_WORKSPACE_KEY, value); + try { + await executeCommand('setContext', PYTHON_WORKSPACE_KEY, value); + } catch (ex) { + if (hasPython === value) { + hasPython = undefined; + } + throw ex; + } }; const scan = async (): Promise => { @@ -76,19 +95,37 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void } const cts = new CancellationTokenSource(); running = cts; + let timedOut = false; + deletedWhileScanning = new Set(); + const timeout = setTimeout(() => { + timedOut = true; + traceVerbose(`Workspace Python marker search exceeded ${SCAN_TIMEOUT_MS} ms; keeping the current context`); + cts.cancel(); + }, SCAN_TIMEOUT_MS); + const cancelTimeout = cts.token.onCancellationRequested(() => clearTimeout(timeout)); try { - const result = await hasPythonContent(cts.token); + const result = await findPythonContent(cts.token); + clearTimeout(timeout); // A cancelled scan was overtaken by `markHasPython` or by disposal, so its result is stale. if (!cts.token.isCancellationRequested) { - await setHasPython(result); + const foundMarker = result.marker ? getMarkerKey(result.marker) : undefined; + if (foundMarker && deletedWhileScanning.has(foundMarker)) { + rescanAfterRunning = true; + } else { + knownMarker = foundMarker; + await setHasPython(result.hasPython); + } } } catch (ex) { - if (!cts.token.isCancellationRequested) { + if (!cts.token.isCancellationRequested && !timedOut) { traceError('Failed to update the Python workspace context', ex); } } finally { + clearTimeout(timeout); + cancelTimeout.dispose(); cts.dispose(); running = undefined; + deletedWhileScanning = new Set(); } if (rescanAfterRunning && !disposed) { rescanAfterRunning = false; @@ -112,26 +149,64 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void // A new marker file or an opened Python document proves the answer is `true`, whatever a running or // pending scan would report. - const markHasPython = (): void => { + const markHasPython = (uri?: Uri): void => { + if (disposed) { + return; + } running?.cancel(); rescanPending = false; rescanAfterRunning = false; + deletedWhileScanning.clear(); + if (uri && !knownMarker) { + knownMarker = getMarkerKey(uri); + } setHasPython(true).catch((ex) => traceError('Failed to update the Python workspace context', ex)); }; const onMarkerCreated = (uri: Uri): void => { - if (!EXCLUDE_RE.test(uri.fsPath)) { - markHasPython(); + if (disposed || EXCLUDE_RE.test(uri.fsPath)) { + return; } + const key = getMarkerKey(uri); + const createId = ++nextCreateId; + pendingCreates.set(key, createId); + void stat(uri).then( + (fileStat) => { + if (pendingCreates.get(key) !== createId) { + return; + } + pendingCreates.delete(key); + if ((fileStat.type & FileType.File) === FileType.File) { + markHasPython(uri); + } + }, + (ex) => { + if (pendingCreates.get(key) === createId) { + pendingCreates.delete(key); + traceError('Failed to inspect a created Python marker', ex); + } + }, + ); }; const onMarkerDeleted = (uri: Uri): void => { if (EXCLUDE_RE.test(uri.fsPath)) { return; } - // A delete can only turn the answer off, so there is nothing to recheck while it is already off, - // unless a running scan may still report the deleted file. - if (hasPython === false && !running) { + const key = getMarkerKey(uri); + pendingCreates.delete(key); + const deletedKnownMarker = key === knownMarker; + if (deletedKnownMarker) { + knownMarker = undefined; + } + if (running) { + deletedWhileScanning.add(key); + if (deletedKnownMarker) { + rescanAfterRunning = true; + } + return; + } + if (!deletedKnownMarker) { return; } scheduleRescan(); @@ -142,7 +217,14 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void watcher, watcher.onDidCreate(onMarkerCreated), watcher.onDidDelete(onMarkerDeleted), - onDidChangeWorkspaceFolders(() => scheduleRescan()), + onDidChangeWorkspaceFolders(() => { + pendingCreates.clear(); + // The marker and running search may belong to a removed folder. + knownMarker = undefined; + rescanAfterRunning = false; + running?.cancel(); + scheduleRescan(); + }), onDidOpenTextDocument((doc) => { if (doc.languageId === 'python') { markHasPython(); @@ -150,9 +232,13 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void }), new Disposable(() => { disposed = true; + pendingCreates.clear(); debouncedRescan.dispose(); running?.cancel(); }), ); + if (getOpenTextDocuments().some((doc) => doc.languageId === 'python')) { + markHasPython(); + } void scan(); } diff --git a/src/test/features/views/workspacePythonContext.unit.test.ts b/src/test/features/views/workspacePythonContext.unit.test.ts index 3c64b66cf..94560b893 100644 --- a/src/test/features/views/workspacePythonContext.unit.test.ts +++ b/src/test/features/views/workspacePythonContext.unit.test.ts @@ -1,14 +1,17 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import * as assert from 'assert'; import * as sinon from 'sinon'; +import { FileType, Uri } from 'vscode'; import * as commandApis from '../../../common/command.api'; import * as logging from '../../../common/logging'; import * as workspaceApis from '../../../common/workspace.apis'; +import * as workspaceFs from '../../../common/workspace.fs.apis'; import { MARKER_GLOB, PYTHON_WORKSPACE_KEY, registerWorkspacePythonContext, RESCAN_DEBOUNCE_MS, + SCAN_TIMEOUT_MS, } from '../../../features/views/workspacePythonContext'; type Listener = (e: T) => void; @@ -24,7 +27,7 @@ function flush(): Promise { return new Promise((resolve) => setImmediate(resolve)); } -const marker = (fsPath: string) => ({ fsPath }) as any; +const marker = (fsPath: string) => Uri.file(fsPath); suite('Workspace Python context', () => { let clock: sinon.SinonFakeTimers; @@ -32,6 +35,7 @@ suite('Workspace Python context', () => { let openDocsStub: sinon.SinonStub; let executeCommandStub: sinon.SinonStub; let traceErrorStub: sinon.SinonStub; + let statStub: sinon.SinonStub; let watcher: { dispose: sinon.SinonStub; onDidCreate: sinon.SinonStub; onDidDelete: sinon.SinonStub }; let foldersStub: sinon.SinonStub; let openStub: sinon.SinonStub; @@ -61,6 +65,14 @@ suite('Workspace Python context', () => { openDocsStub = sinon.stub(workspaceApis, 'getOpenTextDocuments').returns([]); executeCommandStub = sinon.stub(commandApis, 'executeCommand').resolves(undefined); traceErrorStub = sinon.stub(logging, 'traceError'); + statStub = sinon.stub(workspaceFs, 'stat').callsFake((uri: any) => + Promise.resolve({ + type: uri.fsPath.endsWith('tools.py') ? FileType.Directory : FileType.File, + ctime: 0, + mtime: 0, + size: 0, + }), + ); const disposable = { dispose: sinon.stub() }; watcher = { @@ -100,6 +112,18 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true]); }); + test('an open Python document enables the context even if the initial search times out', async () => { + const pending = makeDeferred(); + openDocsStub.returns([{ languageId: 'python' }]); + findFilesStub.returns(pending.promise); + registerWorkspacePythonContext(disposables); + await clock.tickAsync(SCAN_TIMEOUT_MS); + assert.deepStrictEqual(contextValues(), [true]); + assert.strictEqual(tokenOfScan(0).isCancellationRequested, true); + pending.resolve([]); + await flush(); + }); + test('searches for a single hit, excluding dependency folders', async () => { registerWorkspacePythonContext(disposables); await flush(); @@ -141,6 +165,24 @@ suite('Workspace Python context', () => { assert.strictEqual(findFilesStub.callCount, 1); }); + test('creating a marker-named directory does not turn the context on', async () => { + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/tools.py'); + await flush(); + assert.deepStrictEqual(contextValues(), [false]); + assert.strictEqual(findFilesStub.callCount, 1); + }); + + test('a delete after create cancels an unconfirmed file event', async () => { + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/main.py'); + fireDelete('/ws/main.py'); + await flush(); + assert.deepStrictEqual(contextValues(), [false]); + }); + test('deleting the last marker turns the context off after the debounce', async () => { findFilesStub.resolves([marker('/ws/main.py')]); registerWorkspacePythonContext(disposables); @@ -153,6 +195,27 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true, false]); }); + test('deleting a file other than the last scan result does not rescan', async () => { + findFilesStub.resolves([marker('/ws/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + fireDelete('/ws/unrelated.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 1); + assert.deepStrictEqual(contextValues(), [true]); + }); + + test('distinguishes markers with the same path in different URI authorities', async () => { + const first = marker('/ws/main.py').with({ scheme: 'vscode-remote', authority: 'first' }); + const second = first.with({ authority: 'second' }); + findFilesStub.resolves([first]); + registerWorkspacePythonContext(disposables); + await flush(); + (watcher.onDidDelete.firstCall.args[0] as Listener)(second); + await settle(); + assert.strictEqual(findFilesStub.callCount, 1); + }); + test('changing workspace folders re-evaluates the context', async () => { registerWorkspacePythonContext(disposables); await flush(); @@ -162,6 +225,35 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [false, true]); }); + test('a workspace change invalidates pending marker checks', async () => { + const pending = makeDeferred(); + statStub.returns(pending.promise); + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/main.py'); + fireFoldersChanged(); + pending.resolve({ type: FileType.File, ctime: 0, mtime: 0, size: 0 }); + await settle(); + assert.deepStrictEqual(contextValues(), [false]); + assert.strictEqual(findFilesStub.callCount, 2); + }); + + test('a new marker replaces one from a previous workspace', async () => { + findFilesStub.resolves([marker('/old/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + fireFoldersChanged(); + fireCreate('/new/main.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 1); + + findFilesStub.resolves([]); + fireDelete('/new/main.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + test('opening a Python document turns the context on without a rescan', async () => { registerWorkspacePythonContext(disposables); await flush(); @@ -184,7 +276,7 @@ suite('Workspace Python context', () => { findFilesStub.resolves([marker('/ws/main.py')]); registerWorkspacePythonContext(disposables); await flush(); - fireDelete('/ws/old.py'); + fireDelete('/ws/main.py'); await settle(); fireCreate('/ws/new.py'); fireDocOpened('python'); @@ -195,6 +287,24 @@ suite('Workspace Python context', () => { }); suite('event bursts', () => { + test('creating and deleting transient markers preserves the scanned marker', async () => { + findFilesStub.resolves([marker('/ws/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + for (let i = 0; i < 5; i++) { + fireCreate(`/ws/temp${i}.py`); + await flush(); + fireDelete(`/ws/temp${i}.py`); + await settle(); + } + assert.strictEqual(findFilesStub.callCount, 1, 'transient markers should not trigger rescans'); + findFilesStub.resolves([]); + fireDelete('/ws/main.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + test('a burst of creates does not scan the workspace', async () => { registerWorkspacePythonContext(disposables); await flush(); @@ -211,7 +321,7 @@ suite('Workspace Python context', () => { registerWorkspacePythonContext(disposables); await flush(); for (let i = 0; i < 1000; i++) { - fireDelete(`/ws/pkg/mod${i}.py`); + fireDelete(i === 0 ? '/ws/main.py' : `/ws/pkg/mod${i}.py`); } await clock.tickAsync(RESCAN_DEBOUNCE_MS - 1); assert.strictEqual(findFilesStub.callCount, 1); @@ -243,15 +353,21 @@ suite('Workspace Python context', () => { assert.strictEqual(findFilesStub.callCount, 2); }); - test('a create cancels a pending rescan', async () => { + test('a create cancels a pending rescan and replaces the deleted marker', async () => { findFilesStub.resolves([marker('/ws/main.py')]); registerWorkspacePythonContext(disposables); await flush(); - fireDelete('/ws/old/main.py'); + fireDelete('/ws/main.py'); fireCreate('/ws/new/main.py'); await settle(); assert.strictEqual(findFilesStub.callCount, 1); assert.deepStrictEqual(contextValues(), [true]); + + findFilesStub.resolves([]); + fireDelete('/ws/new/main.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [true, false]); }); }); @@ -272,7 +388,7 @@ suite('Workspace Python context', () => { pending[0].resolve([marker('/ws/main.py')]); await flush(); - fireDelete('/ws/a.py'); + fireDelete('/ws/main.py'); await settle(); assert.strictEqual(findFilesStub.callCount, 2, 'the debounced rescan should be running'); @@ -282,7 +398,7 @@ suite('Workspace Python context', () => { await settle(); assert.strictEqual(findFilesStub.callCount, 2, 'no second scan while one is running'); - pending[1].resolve([marker('/ws/main.py')]); + pending[1].resolve([marker('/ws/pkg/mod0.py')]); await flush(); assert.strictEqual(findFilesStub.callCount, 3, 'exactly one follow up scan'); pending[2].resolve([]); @@ -309,6 +425,42 @@ suite('Workspace Python context', () => { assert.strictEqual(findFilesStub.callCount, 1); }); + test('deleting a created marker while its cancelled scan finishes triggers a rescan', async () => { + const initial = makeDeferred(); + findFilesStub.onFirstCall().returns(initial.promise); + findFilesStub.onSecondCall().resolves([]); + registerWorkspacePythonContext(disposables); + fireCreate('/ws/main.py'); + await flush(); + fireDelete('/ws/main.py'); + initial.resolve([]); + await flush(); + assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + + test('a workspace change cancels a scan for the previous workspace', async () => { + const staleScan = makeDeferred(); + findFilesStub.onFirstCall().resolves([marker('/old/main.py')]); + findFilesStub.onSecondCall().returns(staleScan.promise); + findFilesStub.onThirdCall().resolves([]); + registerWorkspacePythonContext(disposables); + await flush(); + + fireFoldersChanged(); + await settle(); + fireFoldersChanged(); + assert.strictEqual(tokenOfScan(1).isCancellationRequested, true); + staleScan.resolve([marker('/old/main.py')]); + await flush(); + fireCreate('/new/main.py'); + await flush(); + fireDelete('/new/main.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 3); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + test('a delete racing a scan is rechecked even while the context is off', async () => { const folderScan = makeDeferred(); registerWorkspacePythonContext(disposables); @@ -324,7 +476,7 @@ suite('Workspace Python context', () => { await settle(); assert.strictEqual(findFilesStub.callCount, 3); - assert.deepStrictEqual(contextValues(), [false, true, false]); + assert.deepStrictEqual(contextValues(), [false]); }); }); @@ -384,10 +536,23 @@ suite('Workspace Python context', () => { registerWorkspacePythonContext(disposables); disposables.forEach((d) => d.dispose()); assert.strictEqual(tokenOfScan(0).isCancellationRequested, true); + assert.strictEqual(clock.countTimers(), 0); scan.resolve([marker('/ws/main.py')]); await flush(); assert.strictEqual(executeCommandStub.callCount, 0); }); + + test('ignores a marker stat that finishes after disposal', async () => { + const pending = makeDeferred(); + statStub.returns(pending.promise); + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/main.py'); + disposables.forEach((d) => d.dispose()); + pending.resolve({ type: FileType.File, ctime: 0, mtime: 0, size: 0 }); + await flush(); + assert.deepStrictEqual(contextValues(), [false]); + }); }); suite('error handling', () => { @@ -410,6 +575,18 @@ suite('Workspace Python context', () => { assert.strictEqual(traceErrorStub.firstCall.args[1], error); }); + test('retries setContext after a failed create update', async () => { + executeCommandStub.onSecondCall().rejects(new Error('setContext failed')); + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/main.py'); + await flush(); + fireCreate('/ws/main.py'); + await flush(); + assert.deepStrictEqual(contextValues(), [false, true, true]); + assert.strictEqual(traceErrorStub.callCount, 1); + }); + test('keeps working after a failed refresh', async () => { findFilesStub.onFirstCall().rejects(new Error('boom')); registerWorkspacePythonContext(disposables); @@ -418,5 +595,23 @@ suite('Workspace Python context', () => { await flush(); assert.deepStrictEqual(contextValues(), [true]); }); + + test('preserves the current context when a search times out', async () => { + const pending = makeDeferred(); + findFilesStub.onFirstCall().resolves([marker('/ws/main.py')]); + findFilesStub.onSecondCall().returns(pending.promise); + registerWorkspacePythonContext(disposables); + await flush(); + + fireFoldersChanged(); + await clock.tickAsync(RESCAN_DEBOUNCE_MS); + await clock.tickAsync(SCAN_TIMEOUT_MS); + await flush(); + + assert.deepStrictEqual(contextValues(), [true]); + assert.strictEqual(tokenOfScan(1).isCancellationRequested, true); + pending.resolve([]); + await flush(); + }); }); }); From 8008ad103f004d5e4c840f9a6c1639b746d300c5 Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Tue, 10 Nov 2026 17:15:31 -0800 Subject: [PATCH 3/7] fix: handle workspace Python marker scan edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cc0aea3d-0a7e-4799-a32b-3a851a5ad484 --- src/features/views/workspacePythonContext.ts | 37 +++++++++++----- .../views/workspacePythonContext.unit.test.ts | 44 ++++++++++++++++--- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/features/views/workspacePythonContext.ts b/src/features/views/workspacePythonContext.ts index c1c2ccd3f..a5cfee62d 100644 --- a/src/features/views/workspacePythonContext.ts +++ b/src/features/views/workspacePythonContext.ts @@ -16,7 +16,7 @@ export const PYTHON_WORKSPACE_KEY = 'python-envs.workspaceHasPython'; /** How long deletes and workspace folder changes must settle before the workspace is scanned again. */ export const RESCAN_DEBOUNCE_MS = 500; -/** A slow workspace search must not block context updates indefinitely. */ +/** A slow rescan must not block context updates after the initial value is known. */ export const SCAN_TIMEOUT_MS = 5000; /** @@ -97,15 +97,26 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void running = cts; let timedOut = false; deletedWhileScanning = new Set(); - const timeout = setTimeout(() => { - timedOut = true; - traceVerbose(`Workspace Python marker search exceeded ${SCAN_TIMEOUT_MS} ms; keeping the current context`); - cts.cancel(); - }, SCAN_TIMEOUT_MS); - const cancelTimeout = cts.token.onCancellationRequested(() => clearTimeout(timeout)); + const timeout = + hasPython === undefined + ? undefined + : setTimeout(() => { + timedOut = true; + traceVerbose( + `Workspace Python marker search exceeded ${SCAN_TIMEOUT_MS} ms; keeping the current context`, + ); + cts.cancel(); + }, SCAN_TIMEOUT_MS); + const cancelTimeout = cts.token.onCancellationRequested(() => { + if (timeout !== undefined) { + clearTimeout(timeout); + } + }); try { const result = await findPythonContent(cts.token); - clearTimeout(timeout); + if (timeout !== undefined) { + clearTimeout(timeout); + } // A cancelled scan was overtaken by `markHasPython` or by disposal, so its result is stale. if (!cts.token.isCancellationRequested) { const foundMarker = result.marker ? getMarkerKey(result.marker) : undefined; @@ -121,7 +132,9 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void traceError('Failed to update the Python workspace context', ex); } } finally { - clearTimeout(timeout); + if (timeout !== undefined) { + clearTimeout(timeout); + } cancelTimeout.dispose(); cts.dispose(); running = undefined; @@ -129,6 +142,7 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void } if (rescanAfterRunning && !disposed) { rescanAfterRunning = false; + rescanPending = false; await scan(); } }; @@ -196,17 +210,18 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void const key = getMarkerKey(uri); pendingCreates.delete(key); const deletedKnownMarker = key === knownMarker; + const markerIsUnknown = knownMarker === undefined && hasPython !== false; if (deletedKnownMarker) { knownMarker = undefined; } if (running) { deletedWhileScanning.add(key); - if (deletedKnownMarker) { + if (deletedKnownMarker || markerIsUnknown) { rescanAfterRunning = true; } return; } - if (!deletedKnownMarker) { + if (!deletedKnownMarker && !markerIsUnknown) { return; } scheduleRescan(); diff --git a/src/test/features/views/workspacePythonContext.unit.test.ts b/src/test/features/views/workspacePythonContext.unit.test.ts index 94560b893..a788f8182 100644 --- a/src/test/features/views/workspacePythonContext.unit.test.ts +++ b/src/test/features/views/workspacePythonContext.unit.test.ts @@ -112,16 +112,17 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true]); }); - test('an open Python document enables the context even if the initial search times out', async () => { + test('does not time out the initial search before establishing the context', async () => { const pending = makeDeferred(); - openDocsStub.returns([{ languageId: 'python' }]); findFilesStub.returns(pending.promise); registerWorkspacePythonContext(disposables); await clock.tickAsync(SCAN_TIMEOUT_MS); - assert.deepStrictEqual(contextValues(), [true]); - assert.strictEqual(tokenOfScan(0).isCancellationRequested, true); - pending.resolve([]); + assert.deepStrictEqual(contextValues(), []); + assert.strictEqual(tokenOfScan(0).isCancellationRequested, false); + pending.resolve([marker('/ws/main.py')]); await flush(); + assert.deepStrictEqual(contextValues(), [true]); + assert.strictEqual(tokenOfScan(0).isCancellationRequested, false); }); test('searches for a single hit, excluding dependency folders', async () => { @@ -205,6 +206,23 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true]); }); + test('deleting a marker rescans when Python is known but no marker is tracked', async () => { + const initial = makeDeferred(); + findFilesStub.onFirstCall().returns(initial.promise); + findFilesStub.onSecondCall().resolves([]); + registerWorkspacePythonContext(disposables); + fireDocOpened('python'); + await flush(); + assert.deepStrictEqual(contextValues(), [true]); + + initial.resolve([]); + await flush(); + fireDelete('/ws/main.py'); + await settle(); + assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + test('distinguishes markers with the same path in different URI authorities', async () => { const first = marker('/ws/main.py').with({ scheme: 'vscode-remote', authority: 'first' }); const second = first.with({ authority: 'second' }); @@ -439,6 +457,22 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true, false]); }); + test('a delete while a cancelled scan finishes triggers a rescan without a known marker', async () => { + const initial = makeDeferred(); + findFilesStub.onFirstCall().returns(initial.promise); + findFilesStub.onSecondCall().resolves([]); + registerWorkspacePythonContext(disposables); + fireDocOpened('python'); + await flush(); + assert.strictEqual(tokenOfScan(0).isCancellationRequested, true); + + fireDelete('/ws/main.py'); + initial.resolve([]); + await flush(); + assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + test('a workspace change cancels a scan for the previous workspace', async () => { const staleScan = makeDeferred(); findFilesStub.onFirstCall().resolves([marker('/old/main.py')]); From ec64043937af2f0a672e0283ffcb9fefda0cc126 Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Wed, 11 Nov 2026 13:45:41 -0800 Subject: [PATCH 4/7] Simplify design --- src/features/views/workspacePythonContext.ts | 159 +++++++----------- .../views/workspacePythonContext.unit.test.ts | 102 ++++------- 2 files changed, 96 insertions(+), 165 deletions(-) diff --git a/src/features/views/workspacePythonContext.ts b/src/features/views/workspacePythonContext.ts index a5cfee62d..26a4d2aa7 100644 --- a/src/features/views/workspacePythonContext.ts +++ b/src/features/views/workspacePythonContext.ts @@ -2,7 +2,6 @@ import { CancellationToken, CancellationTokenSource, Disposable, FileType, Uri } import { executeCommand } from '../../common/command.api'; import { traceError, traceVerbose } from '../../common/logging'; import { createSimpleDebounce } from '../../common/utils/debounce'; -import { normalizePath } from '../../common/utils/pathUtils'; import { stat } from '../../common/workspace.fs.apis'; import { createFileSystemWatcher, @@ -35,17 +34,10 @@ const EXCLUDE = '**/{node_modules,.git,site-packages}/**'; // `createFileSystemWatcher` has no exclude option, so the same folders are filtered in the handlers. const EXCLUDE_RE = /[\\/](node_modules|\.git|site-packages)[\\/]/; -function getMarkerKey(uri: Uri): string { - return JSON.stringify([uri.scheme, uri.authority, normalizePath(uri.fsPath), uri.query, uri.fragment]); -} - -async function findPythonContent(token: CancellationToken): Promise<{ hasPython: boolean; marker?: Uri }> { +async function findPythonContent(token: CancellationToken): Promise { const markers = await findFiles(MARKER_GLOB, EXCLUDE, 1, token); // `findFiles` only searches workspace folders, so also cover a standalone Python file opened without a folder. - return { - hasPython: markers.length > 0 || getOpenTextDocuments().some((doc) => doc.languageId === 'python'), - marker: markers[0], - }; + return markers.length > 0 || getOpenTextDocuments().some((doc) => doc.languageId === 'python'); } /** @@ -54,21 +46,21 @@ async function findPythonContent(token: CancellationToken): Promise<{ hasPython: * * Every `findFiles` call is a full workspace search, and a branch switch can create or delete * thousands of marker files at once, so events never scan directly: - * - a new marker file or an opened Python document proves the answer is `true` without a scan; + * - a created marker that is a file, or an opened Python document, proves the answer is `true`; * - deletes and workspace folder changes can turn it `false`, so they share one debounced rescan; - * - only one scan runs at a time, and a rescan requested meanwhile runs once after it. + * - only one scan runs at a time, and a scan that may have missed a change is rerun once afterwards. */ export function registerWorkspacePythonContext(disposables: Disposable[]): void { let hasPython: boolean | undefined; let running: CancellationTokenSource | undefined; + // Set when something changed during the running scan, so its result may be stale. let rescanAfterRunning = false; // `SimpleDebounce` cannot be cancelled, so a pending rescan is dropped by clearing this flag. let rescanPending = false; let disposed = false; - let knownMarker: string | undefined; - let nextCreateId = 0; - const pendingCreates = new Map(); - let deletedWhileScanning = new Set(); + let nextCheckId = 0; + // Created markers whose `stat` is in flight, so a later delete or folder change can invalidate them. + const pendingChecks = new Map(); const setHasPython = async (value: boolean): Promise => { if (value === hasPython) { @@ -85,6 +77,20 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void } }; + const debouncedRescan = createSimpleDebounce(RESCAN_DEBOUNCE_MS, () => { + if (rescanPending) { + rescanPending = false; + void scan(); + } + }); + + const scheduleRescan = (): void => { + if (!disposed) { + rescanPending = true; + debouncedRescan.trigger(); + } + }; + const scan = async (): Promise => { if (disposed) { return; @@ -95,85 +101,48 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void } const cts = new CancellationTokenSource(); running = cts; - let timedOut = false; - deletedWhileScanning = new Set(); + // Only the initial search must finish; later ones keep the current value if they are too slow. const timeout = hasPython === undefined ? undefined : setTimeout(() => { - timedOut = true; - traceVerbose( - `Workspace Python marker search exceeded ${SCAN_TIMEOUT_MS} ms; keeping the current context`, - ); - cts.cancel(); + if (!cts.token.isCancellationRequested) { + traceVerbose( + `Workspace Python marker search exceeded ${SCAN_TIMEOUT_MS} ms; keeping the current context`, + ); + cts.cancel(); + } }, SCAN_TIMEOUT_MS); - const cancelTimeout = cts.token.onCancellationRequested(() => { - if (timeout !== undefined) { - clearTimeout(timeout); - } - }); try { const result = await findPythonContent(cts.token); - if (timeout !== undefined) { - clearTimeout(timeout); - } - // A cancelled scan was overtaken by `markHasPython` or by disposal, so its result is stale. - if (!cts.token.isCancellationRequested) { - const foundMarker = result.marker ? getMarkerKey(result.marker) : undefined; - if (foundMarker && deletedWhileScanning.has(foundMarker)) { - rescanAfterRunning = true; - } else { - knownMarker = foundMarker; - await setHasPython(result.hasPython); - } + if (!cts.token.isCancellationRequested && !rescanAfterRunning) { + await setHasPython(result); } } catch (ex) { - if (!cts.token.isCancellationRequested && !timedOut) { + if (!cts.token.isCancellationRequested) { traceError('Failed to update the Python workspace context', ex); } } finally { if (timeout !== undefined) { clearTimeout(timeout); } - cancelTimeout.dispose(); cts.dispose(); running = undefined; - deletedWhileScanning = new Set(); } - if (rescanAfterRunning && !disposed) { + if (rescanAfterRunning) { rescanAfterRunning = false; - rescanPending = false; - await scan(); - } - }; - - const debouncedRescan = createSimpleDebounce(RESCAN_DEBOUNCE_MS, () => { - if (rescanPending) { - rescanPending = false; - void scan(); - } - }); - - const scheduleRescan = (): void => { - if (!disposed) { - rescanPending = true; - debouncedRescan.trigger(); + scheduleRescan(); } }; - // A new marker file or an opened Python document proves the answer is `true`, whatever a running or - // pending scan would report. - const markHasPython = (uri?: Uri): void => { + // Overrides any running or pending scan, which could only report a stale answer. + const markHasPython = (): void => { if (disposed) { return; } running?.cancel(); rescanPending = false; rescanAfterRunning = false; - deletedWhileScanning.clear(); - if (uri && !knownMarker) { - knownMarker = getMarkerKey(uri); - } setHasPython(true).catch((ex) => traceError('Failed to update the Python workspace context', ex)); }; @@ -181,22 +150,30 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void if (disposed || EXCLUDE_RE.test(uri.fsPath)) { return; } - const key = getMarkerKey(uri); - const createId = ++nextCreateId; - pendingCreates.set(key, createId); - void stat(uri).then( + // While the answer is settled as `true`, a new marker cannot change it. + if (hasPython === true && !running && !rescanPending) { + return; + } + const key = uri.toString(); + if (pendingChecks.has(key)) { + return; + } + const checkId = ++nextCheckId; + pendingChecks.set(key, checkId); + // The watcher also reports directories that match the glob, such as a `tools.py` folder. + stat(uri).then( (fileStat) => { - if (pendingCreates.get(key) !== createId) { + if (pendingChecks.get(key) !== checkId) { return; } - pendingCreates.delete(key); + pendingChecks.delete(key); if ((fileStat.type & FileType.File) === FileType.File) { - markHasPython(uri); + markHasPython(); } }, (ex) => { - if (pendingCreates.get(key) === createId) { - pendingCreates.delete(key); + if (pendingChecks.get(key) === checkId) { + pendingChecks.delete(key); traceError('Failed to inspect a created Python marker', ex); } }, @@ -204,27 +181,16 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void }; const onMarkerDeleted = (uri: Uri): void => { - if (EXCLUDE_RE.test(uri.fsPath)) { + if (disposed || EXCLUDE_RE.test(uri.fsPath)) { return; } - const key = getMarkerKey(uri); - pendingCreates.delete(key); - const deletedKnownMarker = key === knownMarker; - const markerIsUnknown = knownMarker === undefined && hasPython !== false; - if (deletedKnownMarker) { - knownMarker = undefined; - } + pendingChecks.delete(uri.toString()); if (running) { - deletedWhileScanning.add(key); - if (deletedKnownMarker || markerIsUnknown) { - rescanAfterRunning = true; - } - return; - } - if (!deletedKnownMarker && !markerIsUnknown) { - return; + // The running scan may already have listed the deleted file. + rescanAfterRunning = true; + } else if (hasPython !== false) { + scheduleRescan(); } - scheduleRescan(); }; const watcher = createFileSystemWatcher(MARKER_GLOB, false, true, false); @@ -233,9 +199,8 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void watcher.onDidCreate(onMarkerCreated), watcher.onDidDelete(onMarkerDeleted), onDidChangeWorkspaceFolders(() => { - pendingCreates.clear(); - // The marker and running search may belong to a removed folder. - knownMarker = undefined; + // Pending checks and the running scan may belong to a removed folder. + pendingChecks.clear(); rescanAfterRunning = false; running?.cancel(); scheduleRescan(); @@ -247,7 +212,7 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void }), new Disposable(() => { disposed = true; - pendingCreates.clear(); + pendingChecks.clear(); debouncedRescan.dispose(); running?.cancel(); }), diff --git a/src/test/features/views/workspacePythonContext.unit.test.ts b/src/test/features/views/workspacePythonContext.unit.test.ts index a788f8182..73586a200 100644 --- a/src/test/features/views/workspacePythonContext.unit.test.ts +++ b/src/test/features/views/workspacePythonContext.unit.test.ts @@ -196,42 +196,17 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true, false]); }); - test('deleting a file other than the last scan result does not rescan', async () => { - findFilesStub.resolves([marker('/ws/main.py')]); - registerWorkspacePythonContext(disposables); - await flush(); - fireDelete('/ws/unrelated.py'); - await settle(); - assert.strictEqual(findFilesStub.callCount, 1); - assert.deepStrictEqual(contextValues(), [true]); - }); - - test('deleting a marker rescans when Python is known but no marker is tracked', async () => { - const initial = makeDeferred(); - findFilesStub.onFirstCall().returns(initial.promise); - findFilesStub.onSecondCall().resolves([]); + test('a delete only invalidates the pending check for the same URI', async () => { + const pending = makeDeferred(); + statStub.returns(pending.promise); + const created = marker('/ws/main.py').with({ scheme: 'vscode-remote', authority: 'first' }); registerWorkspacePythonContext(disposables); - fireDocOpened('python'); await flush(); - assert.deepStrictEqual(contextValues(), [true]); - - initial.resolve([]); - await flush(); - fireDelete('/ws/main.py'); - await settle(); - assert.strictEqual(findFilesStub.callCount, 2); - assert.deepStrictEqual(contextValues(), [true, false]); - }); - - test('distinguishes markers with the same path in different URI authorities', async () => { - const first = marker('/ws/main.py').with({ scheme: 'vscode-remote', authority: 'first' }); - const second = first.with({ authority: 'second' }); - findFilesStub.resolves([first]); - registerWorkspacePythonContext(disposables); + (watcher.onDidCreate.firstCall.args[0] as Listener)(created); + (watcher.onDidDelete.firstCall.args[0] as Listener)(created.with({ authority: 'second' })); + pending.resolve({ type: FileType.File, ctime: 0, mtime: 0, size: 0 }); await flush(); - (watcher.onDidDelete.firstCall.args[0] as Listener)(second); - await settle(); - assert.strictEqual(findFilesStub.callCount, 1); + assert.deepStrictEqual(contextValues(), [false, true]); }); test('changing workspace folders re-evaluates the context', async () => { @@ -256,22 +231,6 @@ suite('Workspace Python context', () => { assert.strictEqual(findFilesStub.callCount, 2); }); - test('a new marker replaces one from a previous workspace', async () => { - findFilesStub.resolves([marker('/old/main.py')]); - registerWorkspacePythonContext(disposables); - await flush(); - fireFoldersChanged(); - fireCreate('/new/main.py'); - await settle(); - assert.strictEqual(findFilesStub.callCount, 1); - - findFilesStub.resolves([]); - fireDelete('/new/main.py'); - await settle(); - assert.strictEqual(findFilesStub.callCount, 2); - assert.deepStrictEqual(contextValues(), [true, false]); - }); - test('opening a Python document turns the context on without a rescan', async () => { registerWorkspacePythonContext(disposables); await flush(); @@ -305,32 +264,39 @@ suite('Workspace Python context', () => { }); suite('event bursts', () => { - test('creating and deleting transient markers preserves the scanned marker', async () => { - findFilesStub.resolves([marker('/ws/main.py')]); + test('a burst of creates does not scan the workspace', async () => { registerWorkspacePythonContext(disposables); await flush(); - for (let i = 0; i < 5; i++) { - fireCreate(`/ws/temp${i}.py`); - await flush(); - fireDelete(`/ws/temp${i}.py`); - await settle(); + for (let i = 0; i < 1000; i++) { + fireCreate(`/ws/pkg/mod${i}.py`); } - assert.strictEqual(findFilesStub.callCount, 1, 'transient markers should not trigger rescans'); - findFilesStub.resolves([]); - fireDelete('/ws/main.py'); await settle(); - assert.strictEqual(findFilesStub.callCount, 2); - assert.deepStrictEqual(contextValues(), [true, false]); + assert.strictEqual(findFilesStub.callCount, 1, 'only the initial scan should have run'); + assert.deepStrictEqual(contextValues(), [false, true]); }); - test('a burst of creates does not scan the workspace', async () => { + test('creates are not inspected while the workspace is known to have Python', async () => { + findFilesStub.resolves([marker('/ws/main.py')]); registerWorkspacePythonContext(disposables); await flush(); for (let i = 0; i < 1000; i++) { fireCreate(`/ws/pkg/mod${i}.py`); } await settle(); - assert.strictEqual(findFilesStub.callCount, 1, 'only the initial scan should have run'); + assert.strictEqual(statStub.callCount, 0); + assert.strictEqual(findFilesStub.callCount, 1); + }); + + test('repeated create events for the same marker are inspected once', async () => { + const pending = makeDeferred(); + statStub.returns(pending.promise); + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/main.py'); + fireCreate('/ws/main.py'); + assert.strictEqual(statStub.callCount, 1); + pending.resolve({ type: FileType.File, ctime: 0, mtime: 0, size: 0 }); + await flush(); assert.deepStrictEqual(contextValues(), [false, true]); }); @@ -371,7 +337,7 @@ suite('Workspace Python context', () => { assert.strictEqual(findFilesStub.callCount, 2); }); - test('a create cancels a pending rescan and replaces the deleted marker', async () => { + test('a create cancels a pending rescan', async () => { findFilesStub.resolves([marker('/ws/main.py')]); registerWorkspacePythonContext(disposables); await flush(); @@ -417,7 +383,7 @@ suite('Workspace Python context', () => { assert.strictEqual(findFilesStub.callCount, 2, 'no second scan while one is running'); pending[1].resolve([marker('/ws/pkg/mod0.py')]); - await flush(); + await settle(); assert.strictEqual(findFilesStub.callCount, 3, 'exactly one follow up scan'); pending[2].resolve([]); await settle(); @@ -452,12 +418,12 @@ suite('Workspace Python context', () => { await flush(); fireDelete('/ws/main.py'); initial.resolve([]); - await flush(); + await settle(); assert.strictEqual(findFilesStub.callCount, 2); assert.deepStrictEqual(contextValues(), [true, false]); }); - test('a delete while a cancelled scan finishes triggers a rescan without a known marker', async () => { + test('a delete while a cancelled scan finishes triggers a rescan', async () => { const initial = makeDeferred(); findFilesStub.onFirstCall().returns(initial.promise); findFilesStub.onSecondCall().resolves([]); @@ -468,7 +434,7 @@ suite('Workspace Python context', () => { fireDelete('/ws/main.py'); initial.resolve([]); - await flush(); + await settle(); assert.strictEqual(findFilesStub.callCount, 2); assert.deepStrictEqual(contextValues(), [true, false]); }); From 982a3536b7fc64967c4604daed12ee6c639b4fde Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Wed, 11 Nov 2026 14:25:40 -0800 Subject: [PATCH 5/7] Address feedback --- src/features/views/workspacePythonContext.ts | 126 ++++++++++------ .../views/workspacePythonContext.unit.test.ts | 142 +++++++++++++++--- 2 files changed, 201 insertions(+), 67 deletions(-) diff --git a/src/features/views/workspacePythonContext.ts b/src/features/views/workspacePythonContext.ts index 26a4d2aa7..8db4252d4 100644 --- a/src/features/views/workspacePythonContext.ts +++ b/src/features/views/workspacePythonContext.ts @@ -1,6 +1,6 @@ -import { CancellationToken, CancellationTokenSource, Disposable, FileType, Uri } from 'vscode'; +import { CancellationTokenSource, Disposable, FileType, Uri } from 'vscode'; import { executeCommand } from '../../common/command.api'; -import { traceError, traceVerbose } from '../../common/logging'; +import { traceError } from '../../common/logging'; import { createSimpleDebounce } from '../../common/utils/debounce'; import { stat } from '../../common/workspace.fs.apis'; import { @@ -15,8 +15,6 @@ export const PYTHON_WORKSPACE_KEY = 'python-envs.workspaceHasPython'; /** How long deletes and workspace folder changes must settle before the workspace is scanned again. */ export const RESCAN_DEBOUNCE_MS = 500; -/** A slow rescan must not block context updates after the initial value is known. */ -export const SCAN_TIMEOUT_MS = 5000; /** * Files that identify a workspace as a Python one. @@ -34,10 +32,14 @@ const EXCLUDE = '**/{node_modules,.git,site-packages}/**'; // `createFileSystemWatcher` has no exclude option, so the same folders are filtered in the handlers. const EXCLUDE_RE = /[\\/](node_modules|\.git|site-packages)[\\/]/; -async function findPythonContent(token: CancellationToken): Promise { - const markers = await findFiles(MARKER_GLOB, EXCLUDE, 1, token); - // `findFiles` only searches workspace folders, so also cover a standalone Python file opened without a folder. - return markers.length > 0 || getOpenTextDocuments().some((doc) => doc.languageId === 'python'); +// `findFiles` only searches workspace folders, so also cover a standalone Python file opened without a folder. +function hasOpenPythonDocument(): boolean { + return getOpenTextDocuments().some((doc) => doc.languageId === 'python'); +} + +/** Whether deleting `deleted` also removes `uri`, because it is the same file or one of its parent folders. */ +function isRemovedBy(uri: string, deleted: string): boolean { + return uri === deleted || uri.startsWith(`${deleted}/`); } /** @@ -47,13 +49,24 @@ async function findPythonContent(token: CancellationToken): Promise { * Every `findFiles` call is a full workspace search, and a branch switch can create or delete * thousands of marker files at once, so events never scan directly: * - a created marker that is a file, or an opened Python document, proves the answer is `true`; - * - deletes and workspace folder changes can turn it `false`, so they share one debounced rescan; - * - only one scan runs at a time, and a scan that may have missed a change is rerun once afterwards. + * - one marker known to exist is remembered, so deleting any other marker cannot change the answer; + * - deleting that marker, or a folder containing it, and workspace folder changes share one debounced rescan; + * - only one scan runs at a time, and it is rerun if the marker it found was deleted during the search. + * + * Scans are not timed out: creates and opened Python documents override a slow scan, and workspace + * folder changes cancel it. */ export function registerWorkspacePythonContext(disposables: Disposable[]): void { let hasPython: boolean | undefined; + // The value VS Code last accepted, and whether an update is in flight. + let published: boolean | undefined; + let publishing = false; + // A marker file known to exist. While it does, the answer stays `true`. + let witness: string | undefined; let running: CancellationTokenSource | undefined; - // Set when something changed during the running scan, so its result may be stale. + // Markers deleted during the running scan, which may have returned one of them. + const deletedDuringScan = new Set(); + // Set when a rescan was requested while a scan was running. let rescanAfterRunning = false; // `SimpleDebounce` cannot be cancelled, so a pending rescan is dropped by clearing this flag. let rescanPending = false; @@ -62,18 +75,32 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void // Created markers whose `stat` is in flight, so a later delete or folder change can invalidate them. const pendingChecks = new Map(); - const setHasPython = async (value: boolean): Promise => { + // Sends one update at a time, always with the latest value, so a slow update cannot overwrite a newer one. + const publish = async (): Promise => { + publishing = true; + while (!disposed && hasPython !== undefined && hasPython !== published) { + const value = hasPython; + try { + await executeCommand('setContext', PYTHON_WORKSPACE_KEY, value); + published = value; + } catch (ex) { + traceError('Failed to update the Python workspace context', ex); + // Let the next update retry. + if (hasPython === value) { + hasPython = undefined; + } + } + } + publishing = false; + }; + + const setHasPython = (value: boolean): void => { if (value === hasPython) { return; } hasPython = value; - try { - await executeCommand('setContext', PYTHON_WORKSPACE_KEY, value); - } catch (ex) { - if (hasPython === value) { - hasPython = undefined; - } - throw ex; + if (!publishing) { + void publish(); } }; @@ -101,33 +128,31 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void } const cts = new CancellationTokenSource(); running = cts; - // Only the initial search must finish; later ones keep the current value if they are too slow. - const timeout = - hasPython === undefined - ? undefined - : setTimeout(() => { - if (!cts.token.isCancellationRequested) { - traceVerbose( - `Workspace Python marker search exceeded ${SCAN_TIMEOUT_MS} ms; keeping the current context`, - ); - cts.cancel(); - } - }, SCAN_TIMEOUT_MS); + // The scan finds a new witness; until then, deletes are checked against its result instead. + witness = undefined; + let result: boolean | undefined; try { - const result = await findPythonContent(cts.token); - if (!cts.token.isCancellationRequested && !rescanAfterRunning) { - await setHasPython(result); + const [found] = await findFiles(MARKER_GLOB, EXCLUDE, 1, cts.token); + if (!cts.token.isCancellationRequested) { + const key = found?.toString(); + if (key !== undefined && [...deletedDuringScan].some((deleted) => isRemovedBy(key, deleted))) { + rescanAfterRunning = true; + } else { + witness = key; + result = key !== undefined || hasOpenPythonDocument(); + } } } catch (ex) { if (!cts.token.isCancellationRequested) { - traceError('Failed to update the Python workspace context', ex); + traceError('Failed to search the workspace for Python files', ex); } } finally { - if (timeout !== undefined) { - clearTimeout(timeout); - } cts.dispose(); running = undefined; + deletedDuringScan.clear(); + } + if (result !== undefined) { + setHasPython(result); } if (rescanAfterRunning) { rescanAfterRunning = false; @@ -143,7 +168,7 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void running?.cancel(); rescanPending = false; rescanAfterRunning = false; - setHasPython(true).catch((ex) => traceError('Failed to update the Python workspace context', ex)); + setHasPython(true); }; const onMarkerCreated = (uri: Uri): void => { @@ -168,6 +193,8 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void } pendingChecks.delete(key); if ((fileStat.type & FileType.File) === FileType.File) { + // Keep an existing witness, which is less likely to be a short-lived file. + witness ??= key; markHasPython(); } }, @@ -175,6 +202,8 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void if (pendingChecks.get(key) === checkId) { pendingChecks.delete(key); traceError('Failed to inspect a created Python marker', ex); + // A search still finds the marker if it is a file. + scheduleRescan(); } }, ); @@ -184,10 +213,16 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void if (disposed || EXCLUDE_RE.test(uri.fsPath)) { return; } - pendingChecks.delete(uri.toString()); - if (running) { - // The running scan may already have listed the deleted file. - rescanAfterRunning = true; + const key = uri.toString(); + pendingChecks.delete(key); + // While the witness exists, deleting another marker cannot change the answer. + if (witness !== undefined && !isRemovedBy(witness, key)) { + return; + } + witness = undefined; + if (running && !running.token.isCancellationRequested) { + // The running scan checks its result against this. + deletedDuringScan.add(key); } else if (hasPython !== false) { scheduleRescan(); } @@ -199,7 +234,8 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void watcher.onDidCreate(onMarkerCreated), watcher.onDidDelete(onMarkerDeleted), onDidChangeWorkspaceFolders(() => { - // Pending checks and the running scan may belong to a removed folder. + // The witness, pending checks and running scan may belong to a removed folder. + witness = undefined; pendingChecks.clear(); rescanAfterRunning = false; running?.cancel(); @@ -217,7 +253,7 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void running?.cancel(); }), ); - if (getOpenTextDocuments().some((doc) => doc.languageId === 'python')) { + if (hasOpenPythonDocument()) { markHasPython(); } void scan(); diff --git a/src/test/features/views/workspacePythonContext.unit.test.ts b/src/test/features/views/workspacePythonContext.unit.test.ts index 73586a200..2f0a22d42 100644 --- a/src/test/features/views/workspacePythonContext.unit.test.ts +++ b/src/test/features/views/workspacePythonContext.unit.test.ts @@ -11,7 +11,6 @@ import { PYTHON_WORKSPACE_KEY, registerWorkspacePythonContext, RESCAN_DEBOUNCE_MS, - SCAN_TIMEOUT_MS, } from '../../../features/views/workspacePythonContext'; type Listener = (e: T) => void; @@ -112,17 +111,18 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true]); }); - test('does not time out the initial search before establishing the context', async () => { + test('publishes the initial search despite deletes of other markers', async () => { const pending = makeDeferred(); - findFilesStub.returns(pending.promise); + findFilesStub.onFirstCall().returns(pending.promise); registerWorkspacePythonContext(disposables); - await clock.tickAsync(SCAN_TIMEOUT_MS); - assert.deepStrictEqual(contextValues(), []); - assert.strictEqual(tokenOfScan(0).isCancellationRequested, false); + for (let i = 0; i < 10; i++) { + fireDelete(`/ws/tmp${i}.py`); + await clock.tickAsync(RESCAN_DEBOUNCE_MS); + } pending.resolve([marker('/ws/main.py')]); - await flush(); + await settle(); assert.deepStrictEqual(contextValues(), [true]); - assert.strictEqual(tokenOfScan(0).isCancellationRequested, false); + assert.strictEqual(findFilesStub.callCount, 1); }); test('searches for a single hit, excluding dependency folders', async () => { @@ -196,6 +196,33 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true, false]); }); + test('deleting other markers does not search while the found marker remains', async () => { + findFilesStub.resolves([marker('/ws/main.pyi')]); + registerWorkspacePythonContext(disposables); + await flush(); + for (let i = 0; i < 5; i++) { + fireCreate(`/ws/tmp${i}.py`); + await settle(); + fireDelete(`/ws/tmp${i}.py`); + await settle(); + } + fireDelete('/ws/main.py'); // a prefix of the found marker's path, but neither it nor a parent folder + await settle(); + assert.strictEqual(findFilesStub.callCount, 1); + assert.strictEqual(statStub.callCount, 0); + assert.deepStrictEqual(contextValues(), [true]); + }); + + test('deleting the folder containing the found marker turns the context off', async () => { + findFilesStub.resolves([marker('/ws/tools.py/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + findFilesStub.resolves([]); + fireDelete('/ws/tools.py'); + await settle(); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + test('a delete only invalidates the pending check for the same URI', async () => { const pending = makeDeferred(); statStub.returns(pending.promise); @@ -356,7 +383,7 @@ suite('Workspace Python context', () => { }); suite('overlapping scans', () => { - test('deletes during a scan rerun it once after it finishes, never concurrently', async () => { + test('a result deleted during its scan is rechecked once, never concurrently', async () => { let inFlight = 0; let maxInFlight = 0; const pending: { resolve: (value: any[]) => void }[] = []; @@ -393,6 +420,35 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true, false]); }); + test('an empty result is kept when other markers are deleted during its scan', async () => { + const rescan = makeDeferred(); + findFilesStub.onFirstCall().resolves([marker('/ws/main.py')]); + findFilesStub.onSecondCall().returns(rescan.promise); + registerWorkspacePythonContext(disposables); + await flush(); + fireDelete('/ws/main.py'); + await settle(); + fireDelete('/ws/other.py'); + rescan.resolve([]); + await settle(); + assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + + test('a slow search is never abandoned', async () => { + const slowScan = makeDeferred(); + registerWorkspacePythonContext(disposables); + await flush(); + findFilesStub.onSecondCall().returns(slowScan.promise); + fireFoldersChanged(); + await settle(); + await clock.tickAsync(60_000); + assert.strictEqual(tokenOfScan(1).isCancellationRequested, false); + slowScan.resolve([marker('/other/main.py')]); + await flush(); + assert.deepStrictEqual(contextValues(), [false, true]); + }); + test('a create during a scan wins over a stale empty result', async () => { const initial = makeDeferred(); findFilesStub.returns(initial.promise); @@ -555,6 +611,54 @@ suite('Workspace Python context', () => { }); }); + suite('context updates', () => { + test('a slow update is never applied after a newer one', async () => { + const applied: boolean[] = []; + const firstUpdate = makeDeferred(); + let calls = 0; + executeCommandStub.callsFake(async (_command: string, _key: string, value: boolean) => { + if (++calls === 1) { + await firstUpdate.promise; + } + applied.push(value); + }); + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/main.py'); + await flush(); + firstUpdate.resolve(); + await flush(); + assert.deepStrictEqual(applied, [false, true]); + }); + + test('only the latest value is sent after a slow update', async () => { + const firstUpdate = makeDeferred(); + executeCommandStub.onFirstCall().returns(firstUpdate.promise); + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/main.py'); + await flush(); + fireDelete('/ws/main.py'); + await settle(); + firstUpdate.resolve(); + await flush(); + assert.deepStrictEqual(contextValues(), [false]); + }); + + test('no update is sent after disposal', async () => { + const firstUpdate = makeDeferred(); + executeCommandStub.onFirstCall().returns(firstUpdate.promise); + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/main.py'); + await flush(); + disposables.forEach((d) => d.dispose()); + firstUpdate.resolve(); + await flush(); + assert.deepStrictEqual(contextValues(), [false]); + }); + }); + suite('error handling', () => { test('a failing search is logged and does not reject', async () => { const error = new Error('search failed'); @@ -596,22 +700,16 @@ suite('Workspace Python context', () => { assert.deepStrictEqual(contextValues(), [true]); }); - test('preserves the current context when a search times out', async () => { - const pending = makeDeferred(); - findFilesStub.onFirstCall().resolves([marker('/ws/main.py')]); - findFilesStub.onSecondCall().returns(pending.promise); + test('a marker that cannot be inspected is found by a rescan', async () => { + statStub.rejects(new Error('stat failed')); registerWorkspacePythonContext(disposables); await flush(); - - fireFoldersChanged(); - await clock.tickAsync(RESCAN_DEBOUNCE_MS); - await clock.tickAsync(SCAN_TIMEOUT_MS); - await flush(); - - assert.deepStrictEqual(contextValues(), [true]); - assert.strictEqual(tokenOfScan(1).isCancellationRequested, true); - pending.resolve([]); + findFilesStub.resolves([marker('/ws/main.py')]); + fireCreate('/ws/main.py'); await flush(); + await settle(); + assert.strictEqual(traceErrorStub.callCount, 1); + assert.deepStrictEqual(contextValues(), [false, true]); }); }); }); From 6f6d160556480ca3805db7a6fe5535cfb8600a7f Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Wed, 11 Nov 2026 15:18:02 -0800 Subject: [PATCH 6/7] fix: re-check found marker on deletes and ignore vanished created markers The watcher does not report markers removed with a folder whose name does not match the glob, so the remembered marker could go stale and keep the context true forever. Deleting any other marker now triggers a debounced stat of the remembered marker, which clears it and rescans if it is gone. A created marker that is deleted before it is inspected is now ignored instead of being logged as an error and triggering a full rescan. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6260a269-c45d-4a86-bb05-4fe45eec917f --- src/features/views/workspacePythonContext.ts | 45 ++++++++++++++--- .../views/workspacePythonContext.unit.test.ts | 49 ++++++++++++++++++- 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/features/views/workspacePythonContext.ts b/src/features/views/workspacePythonContext.ts index 8db4252d4..0b7f35ad6 100644 --- a/src/features/views/workspacePythonContext.ts +++ b/src/features/views/workspacePythonContext.ts @@ -2,6 +2,7 @@ import { CancellationTokenSource, Disposable, FileType, Uri } from 'vscode'; import { executeCommand } from '../../common/command.api'; import { traceError } from '../../common/logging'; import { createSimpleDebounce } from '../../common/utils/debounce'; +import { isFileNotFoundError } from '../../common/utils/filesystem'; import { stat } from '../../common/workspace.fs.apis'; import { createFileSystemWatcher, @@ -49,12 +50,15 @@ function isRemovedBy(uri: string, deleted: string): boolean { * Every `findFiles` call is a full workspace search, and a branch switch can create or delete * thousands of marker files at once, so events never scan directly: * - a created marker that is a file, or an opened Python document, proves the answer is `true`; - * - one marker known to exist is remembered, so deleting any other marker cannot change the answer; + * - one marker known to exist is remembered, so deleting any other marker only re-checks that it still exists; * - deleting that marker, or a folder containing it, and workspace folder changes share one debounced rescan; * - only one scan runs at a time, and it is rerun if the marker it found was deleted during the search. * * Scans are not timed out: creates and opened Python documents override a slow scan, and workspace * folder changes cancel it. + * + * The watcher does not report markers removed with a folder whose name does not match the glob, so + * the context can stay `true` until a later marker delete re-checks the remembered marker. */ export function registerWorkspacePythonContext(disposables: Disposable[]): void { let hasPython: boolean | undefined; @@ -118,6 +122,26 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void } }; + // The watcher misses some deletes, such as markers removed with their folder, so the witness is re-checked. + const verifyWitness = async (): Promise => { + const candidate = witness; + if (disposed || candidate === undefined) { + return; + } + let exists = false; + try { + exists = ((await stat(Uri.parse(candidate))).type & FileType.File) === FileType.File; + } catch { + // Treated as missing; the rescan finds the marker if it still exists. + } + if (!exists && !disposed && witness === candidate) { + witness = undefined; + scheduleRescan(); + } + }; + + const debouncedVerifyWitness = createSimpleDebounce(RESCAN_DEBOUNCE_MS, () => void verifyWitness()); + const scan = async (): Promise => { if (disposed) { return; @@ -199,12 +223,17 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void } }, (ex) => { - if (pendingChecks.get(key) === checkId) { - pendingChecks.delete(key); - traceError('Failed to inspect a created Python marker', ex); - // A search still finds the marker if it is a file. - scheduleRescan(); + if (pendingChecks.get(key) !== checkId) { + return; + } + pendingChecks.delete(key); + // A short-lived file, such as a temporary one, cannot change the answer. + if (isFileNotFoundError(ex)) { + return; } + traceError('Failed to inspect a created Python marker', ex); + // A search still finds the marker if it is a file. + scheduleRescan(); }, ); }; @@ -215,8 +244,9 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void } const key = uri.toString(); pendingChecks.delete(key); - // While the witness exists, deleting another marker cannot change the answer. + // While the witness exists, deleting another marker cannot change the answer, so only check that it does. if (witness !== undefined && !isRemovedBy(witness, key)) { + debouncedVerifyWitness.trigger(); return; } witness = undefined; @@ -250,6 +280,7 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void disposed = true; pendingChecks.clear(); debouncedRescan.dispose(); + debouncedVerifyWitness.dispose(); running?.cancel(); }), ); diff --git a/src/test/features/views/workspacePythonContext.unit.test.ts b/src/test/features/views/workspacePythonContext.unit.test.ts index 2f0a22d42..05880fdac 100644 --- a/src/test/features/views/workspacePythonContext.unit.test.ts +++ b/src/test/features/views/workspacePythonContext.unit.test.ts @@ -209,10 +209,30 @@ suite('Workspace Python context', () => { fireDelete('/ws/main.py'); // a prefix of the found marker's path, but neither it nor a parent folder await settle(); assert.strictEqual(findFilesStub.callCount, 1); - assert.strictEqual(statStub.callCount, 0); + const inspected = new Set(statStub.getCalls().map((c) => c.args[0].toString())); + assert.deepStrictEqual( + [...inspected], + [marker('/ws/main.pyi').toString()], + 'only the found marker is re-checked', + ); assert.deepStrictEqual(contextValues(), [true]); }); + test('a found marker removed without a delete event is caught by a later delete', async () => { + findFilesStub.resolves([marker('/ws/scripts/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + // Deleting `scripts` is not reported, because the folder name does not match the glob. + statStub.rejects(Object.assign(new Error('gone'), { code: 'FileNotFound' })); + findFilesStub.resolves([]); + fireDelete('/ws/other.py'); + await settle(); // re-checks the found marker + await settle(); // rescans + assert.strictEqual(findFilesStub.callCount, 2); + assert.strictEqual(traceErrorStub.callCount, 0); + assert.deepStrictEqual(contextValues(), [true, false]); + }); + test('deleting the folder containing the found marker turns the context off', async () => { findFilesStub.resolves([marker('/ws/tools.py/main.py')]); registerWorkspacePythonContext(disposables); @@ -343,6 +363,20 @@ suite('Workspace Python context', () => { assert.strictEqual(findFilesStub.callCount, 2); }); + test('a burst of deletes re-checks the found marker once', async () => { + findFilesStub.resolves([marker('/ws/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + for (let i = 0; i < 1000; i++) { + fireDelete(`/ws/pkg/mod${i}.py`); + } + await settle(); + await settle(); + assert.strictEqual(statStub.callCount, 1); + assert.strictEqual(findFilesStub.callCount, 1); + assert.deepStrictEqual(contextValues(), [true]); + }); + test('deletes are ignored while the workspace has no Python', async () => { registerWorkspacePythonContext(disposables); await flush(); @@ -577,6 +611,7 @@ suite('Workspace Python context', () => { findFilesStub.resolves([marker('/ws/main.py')]); registerWorkspacePythonContext(disposables); await flush(); + fireDelete('/ws/other.py'); fireDelete('/ws/main.py'); disposables.forEach((d) => d.dispose()); assert.strictEqual(clock.countTimers(), 0); @@ -711,5 +746,17 @@ suite('Workspace Python context', () => { assert.strictEqual(traceErrorStub.callCount, 1); assert.deepStrictEqual(contextValues(), [false, true]); }); + + test('a marker deleted before it is inspected is ignored', async () => { + statStub.rejects(Object.assign(new Error('gone'), { code: 'FileNotFound' })); + registerWorkspacePythonContext(disposables); + await flush(); + fireCreate('/ws/tmp.py'); + await flush(); + await settle(); + assert.strictEqual(traceErrorStub.callCount, 0); + assert.strictEqual(findFilesStub.callCount, 1); + assert.deepStrictEqual(contextValues(), [false]); + }); }); }); From 2e994a8ffc6bccaa1ba237bdc897ad84f0613f28 Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Wed, 11 Nov 2026 15:43:36 -0800 Subject: [PATCH 7/7] Nit --- src/features/views/workspacePythonContext.ts | 7 +++++-- .../views/workspacePythonContext.unit.test.ts | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/features/views/workspacePythonContext.ts b/src/features/views/workspacePythonContext.ts index 0b7f35ad6..9d83a9c4b 100644 --- a/src/features/views/workspacePythonContext.ts +++ b/src/features/views/workspacePythonContext.ts @@ -72,7 +72,7 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void const deletedDuringScan = new Set(); // Set when a rescan was requested while a scan was running. let rescanAfterRunning = false; - // `SimpleDebounce` cannot be cancelled, so a pending rescan is dropped by clearing this flag. + // `SimpleDebounce` has no `cancel()`, so a pending rescan is dropped by clearing this flag. let rescanPending = false; let disposed = false; let nextCheckId = 0; @@ -131,7 +131,10 @@ export function registerWorkspacePythonContext(disposables: Disposable[]): void let exists = false; try { exists = ((await stat(Uri.parse(candidate))).type & FileType.File) === FileType.File; - } catch { + } catch (ex) { + if (!isFileNotFoundError(ex)) { + traceError('Failed to inspect the found Python marker', ex); + } // Treated as missing; the rescan finds the marker if it still exists. } if (!exists && !disposed && witness === candidate) { diff --git a/src/test/features/views/workspacePythonContext.unit.test.ts b/src/test/features/views/workspacePythonContext.unit.test.ts index 05880fdac..71eb06b66 100644 --- a/src/test/features/views/workspacePythonContext.unit.test.ts +++ b/src/test/features/views/workspacePythonContext.unit.test.ts @@ -758,5 +758,20 @@ suite('Workspace Python context', () => { assert.strictEqual(findFilesStub.callCount, 1); assert.deepStrictEqual(contextValues(), [false]); }); + + test('a found marker that cannot be re-checked is logged and searched for', async () => { + findFilesStub.resolves([marker('/ws/main.py')]); + registerWorkspacePythonContext(disposables); + await flush(); + const error = new Error('stat failed'); + statStub.rejects(error); + fireDelete('/ws/other.py'); + await settle(); // re-checks the found marker + await settle(); // rescans + assert.ok(traceErrorStub.calledOnce); + assert.strictEqual(traceErrorStub.firstCall.args[1], error); + assert.strictEqual(findFilesStub.callCount, 2); + assert.deepStrictEqual(contextValues(), [true]); + }); }); });