From ef0d8f4b154371399aae868d456b8d03ace60200 Mon Sep 17 00:00:00 2001 From: Feldt <204218226+Feldt@users.noreply.github.com> Date: Tue, 20 Oct 2026 15:38:48 -0400 Subject: [PATCH] fix: skipFiles ignoring negations of absolute paths and node internals preprocessAuthoredGlobs turns absolute skipFiles entries into file URLs, but it called isAbsolute() on the raw pattern. The leading "!" makes a negated absolute path look relative, so "!${workspaceFolder}/foo/**" stayed a bare path while the positive globs became file:/// URLs, and the negation never matched a script. Convert the glob without its "!" and put the "!" back afterwards. Negated node internals ("!/events") were dropped for the same reason: preprocessNodeInternals only matched entries that start with "/", and preprocessAuthoredGlobs skips any entry with the token. Keep the "!" on the node internals glob, which micromatch applies as a negation, and only use the list when at least one entry has no leading "!", so a lone negation still skips nothing. Co-Authored-By: Claude Opus 5.5 --- src/adapter/scriptSkipper/implementation.ts | 31 +++-- .../scriptSkipper/scriptSkipper.test.ts | 109 ++++++++++++++++++ 2 files changed, 132 insertions(+), 8 deletions(-) create mode 100644 src/adapter/scriptSkipper/scriptSkipper.test.ts diff --git a/src/adapter/scriptSkipper/implementation.ts b/src/adapter/scriptSkipper/implementation.ts index 2c0b4767e..78e17c1ad 100644 --- a/src/adapter/scriptSkipper/implementation.ts +++ b/src/adapter/scriptSkipper/implementation.ts @@ -33,17 +33,27 @@ interface ISharedSkipToggleEvent { } function preprocessNodeInternals(userSkipPatterns: ReadonlyArray): string[] | undefined { - const nodeInternalRegex = /^[\/\\](.*)$/; + const nodeInternalRegex = /^(!?)[\/\\](.*)$/; + let hasPositive = false; const nodeInternalPatterns = userSkipPatterns .map(userPattern => { userPattern = userPattern.trim(); const nodeInternalPattern = nodeInternalRegex.exec(userPattern); - return nodeInternalPattern ? nodeInternalPattern[1] : null; + if (!nodeInternalPattern?.[2]) { + return null; + } + + // keep a leading `!`, micromatch applies it as a negation + const [, negation, glob] = nodeInternalPattern; + hasPositive ||= !negation; + return negation + glob; }) .filter(truthy); - return nodeInternalPatterns.length > 0 ? nodeInternalPatterns : undefined; + // A negation only excludes internals that a positive entry skips. On its + // own, micromatch would match every module the negation does not name. + return hasPositive ? nodeInternalPatterns : undefined; } function preprocessAuthoredGlobs( @@ -52,11 +62,16 @@ function preprocessAuthoredGlobs( ): string[] { const authoredGlobs = userSkipPatterns .filter(pattern => !pattern.includes(nodeInternalsToken)) - .map(pattern => - urlUtils.isAbsolute(pattern) - ? urlUtils.absolutePathToFileUrlWithDetection(spr.rebaseLocalToRemote(pattern)) - : pathUtils.forceForwardSlashes(pattern) - ) + .map(pattern => { + // Convert the glob without its negation, so negated absolute paths + // (`!${workspaceFolder}/foo/**`) also become file URLs. + const negated = pattern.startsWith('!'); + const glob = negated ? pattern.slice(1) : pattern; + const converted = urlUtils.isAbsolute(glob) + ? urlUtils.absolutePathToFileUrlWithDetection(spr.rebaseLocalToRemote(glob)) + : pathUtils.forceForwardSlashes(glob); + return negated ? `!${converted}` : converted; + }) .map(urlUtils.lowerCaseInsensitivePath); return authoredGlobs; diff --git a/src/adapter/scriptSkipper/scriptSkipper.test.ts b/src/adapter/scriptSkipper/scriptSkipper.test.ts new file mode 100644 index 000000000..c37c72d9a --- /dev/null +++ b/src/adapter/scriptSkipper/scriptSkipper.test.ts @@ -0,0 +1,109 @@ +/*--------------------------------------------------------- + * Copyright (C) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------*/ + +import { expect } from 'chai'; +import Cdp from '../../cdp/api'; +import { Logger } from '../../common/logging/logger'; +import { upcastPartial } from '../../common/objUtils'; +import { ISourcePathResolver } from '../../common/sourcePathResolver'; +import { AnyLaunchConfiguration } from '../../configuration'; +import { ITarget } from '../../targets/targets'; +import { ScriptSkipper } from './implementation'; + +describe('ScriptSkipper', () => { + let skipper: ScriptSkipper | undefined; + let blackboxPatterns: string[] = []; + + const createSkipper = (skipFiles: string[]) => { + const cdp = upcastPartial({ + Debugger: upcastPartial({ + setBlackboxPatterns: async ({ patterns }) => { + blackboxPatterns = patterns; + return {}; + }, + }), + Runtime: upcastPartial({ + evaluate: async () => ({ result: { type: 'object', value: ['events', 'fs'] } }), + }), + }); + + skipper = new ScriptSkipper( + upcastPartial({ skipFiles }), + upcastPartial({ rebaseLocalToRemote: p => p }), + Logger.null, + cdp, + upcastPartial({ type: () => 'node', id: () => 'a', parent: () => undefined }), + ); + + return skipper; + }; + + const isBlackboxed = (url: string) => blackboxPatterns.some(p => new RegExp(p, 'i').test(url)); + + afterEach(() => { + skipper?.dispose(); + skipper = undefined; + blackboxPatterns = []; + }); + + const lodash = 'file:///proj/node_modules/lodash/index.js'; + const math = 'file:///proj/node_modules/math/index.js'; + + it('applies relative negations', () => { + const s = createSkipper(['**/node_modules/**', '!**/node_modules/math/**']); + expect(s.isScriptSkipped(lodash)).to.be.true; + expect(s.isScriptSkipped(math)).to.be.false; + expect(isBlackboxed(lodash)).to.be.true; + expect(isBlackboxed(math)).to.be.false; + }); + + it('applies absolute negations', () => { + // e.g. "!${workspaceFolder}/node_modules/math/**" once variables are resolved + const s = createSkipper(['/proj/node_modules/**', '!/proj/node_modules/math/**']); + expect(s.isScriptSkipped(lodash)).to.be.true; + expect(s.isScriptSkipped(math)).to.be.false; + expect(isBlackboxed(lodash)).to.be.true; + expect(isBlackboxed(math)).to.be.false; + }); + + it('applies absolute negations to relative globs', () => { + const s = createSkipper(['**/node_modules/**', '!/proj/node_modules/math/**']); + expect(s.isScriptSkipped(lodash)).to.be.true; + expect(s.isScriptSkipped(math)).to.be.false; + expect(isBlackboxed(lodash)).to.be.true; + expect(isBlackboxed(math)).to.be.false; + }); + + it('applies node internals negations', async () => { + // Node 15+ names internals `node:events`, shown as `/events` + const s = createSkipper(['/**', '!/events']); + await new Promise(r => setImmediate(r)); // let the builtin module list load + expect(s.isScriptSkipped('node:fs')).to.be.true; + expect(s.isScriptSkipped('node:events')).to.be.false; + }); + + it('applies node internals negations to Node 14 module names', async () => { + // before Node 15, internals are named `events.js` + const s = createSkipper(['/**', '!/events.js']); + await new Promise(r => setImmediate(r)); + expect(s.isScriptSkipped('fs.js')).to.be.true; + expect(s.isScriptSkipped('events.js')).to.be.false; + expect(isBlackboxed('fs.js')).to.be.true; + expect(isBlackboxed('events.js')).to.be.false; + }); + + it('ignores node internals negations without a positive glob', async () => { + const s = createSkipper(['!/events']); + await new Promise(r => setImmediate(r)); + expect(s.isScriptSkipped('node:fs')).to.be.false; + expect(s.isScriptSkipped('node:events')).to.be.false; + }); + + it('keeps node internals globs that start with an extglob', async () => { + const s = createSkipper(['/!(events)']); + await new Promise(r => setImmediate(r)); + expect(s.isScriptSkipped('node:fs')).to.be.true; + expect(s.isScriptSkipped('node:events')).to.be.false; + }); +});