Skip to content

Fix PATH concatenation without separator in environment variable collection #958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rchiodo merged 5 commits into from
Feb 17, 2026
+56 −95
Merged

Fix PATH concatenation without separator in environment variable collection #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2076083
Initial plan
Copilot Feb 17, 2026
9e4098c
Fix PATH separator issue when appending to environment variable colle…
Copilot Feb 17, 2026
db20bde
Remove extra blank lines in test file
Copilot Feb 17, 2026
c5ef7e5
Merge branch 'main' into copilot/fix-debugpy-path-issue
rchiodo Feb 17, 2026
80e4a14
Fix prettier formatting in test file
Copilot Feb 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions package-lock.json
View file
Open in desktop
Copy link

@edvilme edvilme Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be checked in? I believe the changes are only for handling the path and separators?

Copy link
Contributor

@rchiodo rchiodo Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot there's an error here. Package-lock.json shouldn't have changed. Can you revert this change?

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 2 additions & 6 deletions src/extension/noConfigDebugInit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,8 @@ export async function registerNoConfigDebug(
const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts');
const pathSeparator = process.platform === 'win32' ? ';' : ':';

// Check if the current PATH already ends with a path separator to avoid double separators
const currentPath = process.env.PATH || '';
const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator);
const pathValueToAppend = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir;

collection.append('PATH', pathValueToAppend);
// Always prepend separator when appending to PATH since append() concatenates to existing value
collection.append('PATH', `${pathSeparator}${noConfigScriptsDir}`);

const bundledDebugPath = path.join(extPath, 'bundled', 'libs', 'debugpy');
collection.replace('BUNDLED_DEBUGPY_PATH', bundledDebugPath);
Expand Down
109 changes: 25 additions & 84 deletions src/test/unittest/noConfigDebugInit.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ suite('setup for no-config debug scenario', function () {
.setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.callback((key, value) => {
if (key === 'PATH') {
assert(value.includes(noConfigScriptsDir));
const pathSeparator = process.platform === 'win32' ? ';' : ':';
assert(value === `${pathSeparator}${noConfigScriptsDir}`);
}
})
.returns(envVarCollectionAppendStub);
Expand All @@ -88,98 +89,38 @@ suite('setup for no-config debug scenario', function () {
sinon.assert.calledOnce(envVarCollectionAppendStub);
});

test('should not add extra separator when PATH already ends with separator', async () => {
test('should always add separator when appending to PATH', async () => {
const environmentVariableCollectionMock = TypeMoq.Mock.ofType<any>();
envVarCollectionReplaceStub = sinon.stub();
envVarCollectionAppendStub = sinon.stub();

// Simulate a PATH that already ends with a separator to test the fix
// The separator should always be prepended regardless of process.env.PATH
const pathSeparator = process.platform === 'win32' ? ';' : ':';
const originalPath = process.env.PATH;
process.env.PATH = `/some/path${pathSeparator}`;

try {
environmentVariableCollectionMock
.setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns(envVarCollectionReplaceStub);

environmentVariableCollectionMock
.setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.callback((key, value) => {
if (key === 'PATH') {
// Since PATH already ends with separator, we should NOT add another one
assert(value === noConfigScriptsDir);
assert(!value.startsWith(pathSeparator));
}
})
.returns(envVarCollectionAppendStub);

context
.setup((c) => c.environmentVariableCollection)
.returns(() => environmentVariableCollectionMock.object);

setupFileSystemWatchers();

// run init for no config debug
await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath);

// assert that append was called for PATH
sinon.assert.calledOnce(envVarCollectionAppendStub);
} finally {
// Restore original PATH
if (originalPath !== undefined) {
process.env.PATH = originalPath;
} else {
delete process.env.PATH;
}
}
});
environmentVariableCollectionMock
.setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns(envVarCollectionReplaceStub);

test('should add separator when PATH does not end with separator', async () => {
const environmentVariableCollectionMock = TypeMoq.Mock.ofType<any>();
envVarCollectionReplaceStub = sinon.stub();
envVarCollectionAppendStub = sinon.stub();
environmentVariableCollectionMock
.setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.callback((key, value) => {
if (key === 'PATH') {
// Should always add separator when appending
assert(value === `${pathSeparator}${noConfigScriptsDir}`);
assert(value.startsWith(pathSeparator));
}
})
.returns(envVarCollectionAppendStub);

// Simulate a PATH that does NOT end with a separator
const pathSeparator = process.platform === 'win32' ? ';' : ':';
const originalPath = process.env.PATH;
process.env.PATH = '/some/path';
context.setup((c) => c.environmentVariableCollection).returns(() => environmentVariableCollectionMock.object);

try {
environmentVariableCollectionMock
.setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns(envVarCollectionReplaceStub);

environmentVariableCollectionMock
.setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.callback((key, value) => {
if (key === 'PATH') {
// Since PATH does NOT end with separator, we should add one
assert(value === `${pathSeparator}${noConfigScriptsDir}`);
assert(value.startsWith(pathSeparator));
}
})
.returns(envVarCollectionAppendStub);

context
.setup((c) => c.environmentVariableCollection)
.returns(() => environmentVariableCollectionMock.object);

setupFileSystemWatchers();

// run init for no config debug
await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath);

// assert that append was called for PATH
sinon.assert.calledOnce(envVarCollectionAppendStub);
} finally {
// Restore original PATH
if (originalPath !== undefined) {
process.env.PATH = originalPath;
} else {
delete process.env.PATH;
}
}
setupFileSystemWatchers();

// run init for no config debug
await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath);

// assert that append was called for PATH
sinon.assert.calledOnce(envVarCollectionAppendStub);
});

test('should create file system watcher for debuggerAdapterEndpointFolder', async () => {
Expand Down
Loading