Skip to content

Commit 49ee58b

Browse files
authored
fix: bridge Python environment selection to content creator commands (#2763)
## Bridge Python environment selection to content creator commands ### Problem Content creator commands (`ansible-creator`, `ade`, etc.) were not respecting the Python environment selected in VS Code. When users selected a virtual environment containing these tools, the extension would still attempt to execute them using system Python, resulting in "command not found" errors even when the tools were correctly installed in the selected venv. This affected: - Collection/Project/Role/Devfile/Devcontainer creation panels - Execution Environment panel - Ansible Vault operations - All commands executed via `withInterpreter()` ### Root Cause The `withInterpreter()` helper was a legacy synchronous function that only set `ANSIBLE_FORCE_COLOR` and `PYTHONBREAKPOINT` environment variables. It did not: 1. Resolve the active Python environment 2. Add the venv's `bin/` directory to `PATH` 3. Set `VIRTUAL_ENV` for tools that check it Additionally, `PythonEnvironmentService.getEnvironment()` would return system Python when no workspace scope was provided, even when a workspace-specific venv was selected. ### Solution #### 1. Enhanced `withInterpreter()` to bridge Python environment - Made function async to support environment resolution - Integrated `PythonEnvironmentService` to resolve interpreter path - Added venv `bin/` directory to `PATH` before command execution - Set `VIRTUAL_ENV` environment variable for compatibility #### 2. Workspace scope defaulting Added intelligent scope resolution in `PythonEnvironmentService.getEnvironment()`: - Defaults to first workspace folder when no scope provided - Ensures workspace-specific Python environments are used instead of system Python - Maintains explicit scope behavior when caller provides one #### 3. Configuration middleware improvements - Added `PythonEnvironmentService.resolveInterpreterPath()` to centralize path resolution - Expanded tilde (`~`) and `${workspaceFolder}` in user-configured interpreter paths - Added error handling for configuration fetch and environment resolution failures - Send expanded paths to language server to avoid server-side expansion issues #### 4. Execution Environment webview parity - Added requirements checking to Execution Environment panel (previously missing) - Added `RequirementsBanner` component to show version check results - Aligned behavior with other content creator panels ### Changes **Core Infrastructure:** - `src/features/utils/commandRunner.ts` - Made `withInterpreter()` async with venv PATH support - `src/services/PythonEnvironmentService.ts` - Added scope defaulting and `resolveInterpreterPath()` method **Updated Callers (now await async `withInterpreter()`):** - `src/extension.ts` - ansible-creator and ansible-dev-tools installation commands - `src/features/contentCreator/utils.ts` - `getBinDetail()` for version checks - `src/features/lightspeed/vue/views/ansibleCreatorUtils.ts` - Role, plugin, collection, project creation - `src/features/lightspeed/vue/views/webviewMessageHandlers.ts` - Execution environment init - `src/features/utils/buildExecutionEnvironment.ts` - EE build command - `src/features/vault.ts` - All ansible-vault operations **Configuration Middleware:** - `src/extension.ts` - `makeConfigurationMiddleware()` now expands user paths and handles errors gracefully **UI:** - `src/features/contentCreator/vue/views/createExecutionEnvPanel.ts` - Added requirements check message handler - `webviews/CreateExecutionEnvApp.vue` - Added `RequirementsBanner` and requirements state **Tests:** - `test/unit/utils/commandRunner.test.ts` - New comprehensive unit tests (10 tests) - `test/unit/services/PythonEnvironmentService.test.ts` - Added scope resolution tests (6 new tests) - `test/unit/configurationMiddleware.test.ts` - Added error handling and path expansion tests (3 new tests) ### Testing **Unit Tests:** 56 tests (54 passing, 2 skipped) - `commandRunner.test.ts` - Validates PATH manipulation, environment variable setting, scope handling - `PythonEnvironmentService.test.ts` - Tests scope resolution for single/multi-workspace scenarios - `configurationMiddleware.test.ts` - Validates path expansion and error handling **Integration Tests:** - `utilities.test.ts` - Validates real command execution through `getBinDetail()` ### Migration Notes The changes are backward compatible: - `withInterpreter()` is now async but maintains the same return signature - All callers updated to use `await` - No configuration changes required from users - Workspace scope defaulting is transparent ### Limitations - Multi-workspace setups default to first workspace folder when no scope provided (RFE: add workspace picker for ambiguous cases) - Debug logging added temporarily to aid troubleshooting (can be removed after stabilization) ## Identified Regression In #2757, the resources for devfile and devcontainer were identified as build artifacts, when they need to be packaged into the final .vsix. The fixes for this were incorporated into this change due to urgency.
1 parent c2baa58 commit 49ee58b

19 files changed

Lines changed: 820 additions & 130 deletions

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ CHANGELOG.html
2626
docs/als/settings.md
2727
examples/.vscode/
2828
packages/ansible-language-server/test/fixtures/diagnostics/invalid_yaml.yml
29+
resources/contentCreator/createDevcontainer/.devcontainer
30+
resources/contentCreator/createDevfile/devfile-template.txt
2931
test/testFixtures/.vscode/settings.json
3032
test/testFixtures/diagnostics/yaml/invalid_yaml.yml
3133
test/unit/lightspeed/utils/samples/collections/ansible_collections/community/broken_MANIFEST/MANIFEST.json

Taskfile.base.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ tasks:
3030
generates:
3131
- "{{ .VIRTUAL_ENV }}/pyvenv.cfg"
3232
- "{{ .VIRTUAL_ENV }}/CACHEDIR.TAG"
33-
- out/resources/contentCreator/createDevfile/devfile-template.txt
34-
- out/resources/contentCreator/createDevcontainer/.devcontainer
33+
- resources/contentCreator/createDevfile/devfile-template.txt
34+
- resources/contentCreator/createDevcontainer/.devcontainer
3535
dir: "{{ .TASKFILE_DIR }}"
3636
sources:
3737
- pyproject.toml

src/extension.ts

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ export async function activate(context: ExtensionContext): Promise<void> {
596596
const pythonInterpreter = extSettings.settings.interpreterPath;
597597

598598
// specify the current python interpreter path in the pip installation
599-
const { command, env } = withInterpreter(
599+
const { command, env } = await withInterpreter(
600600
extSettings.settings,
601601
`${pythonInterpreter} -m pip install ansible-creator`,
602602
"--no-input",
@@ -1032,7 +1032,7 @@ export async function activate(context: ExtensionContext): Promise<void> {
10321032
const pythonInterpreter = extSettings.settings.interpreterPath;
10331033

10341034
// specify the current python interpreter path in the pip installation
1035-
const { command, env } = withInterpreter(
1035+
const { command, env } = await withInterpreter(
10361036
extSettings.settings,
10371037
`${pythonInterpreter} -m pip install ansible-dev-tools`,
10381038
"--no-input",
@@ -1447,7 +1447,16 @@ export function makeConfigurationMiddleware(
14471447
) => HandlerResult<LSPAny[], void>,
14481448
): HandlerResult<LSPAny[], void> => {
14491449
return (async (): Promise<LSPAny[]> => {
1450-
const originalResult = await next(params, token);
1450+
let originalResult: LSPAny[] | LSPAny;
1451+
try {
1452+
originalResult = await next(params, token);
1453+
} catch (error) {
1454+
outputChannel.appendLine(
1455+
`[Ansible] Configuration middleware error: ${error}`,
1456+
);
1457+
return [] as unknown as LSPAny[];
1458+
}
1459+
14511460
if (!Array.isArray(originalResult))
14521461
return originalResult as unknown as LSPAny[];
14531462

@@ -1467,18 +1476,49 @@ export function makeConfigurationMiddleware(
14671476
const rawInterpreterPath = pythonConfig?.interpreterPath;
14681477

14691478
if (typeof rawInterpreterPath === "string" && rawInterpreterPath) {
1470-
// User has explicit config, log only on change
1479+
// User has explicit config - use centralized resolver to expand ~ and ${workspaceFolder}
1480+
const scope = scopeUri ? vscode.Uri.parse(scopeUri) : undefined;
1481+
const resolvedUserPath =
1482+
await pythonEnvService.resolveInterpreterPath(
1483+
rawInterpreterPath,
1484+
scope,
1485+
);
1486+
1487+
// Log only on change (compare raw path for deduplication)
14711488
if (lastLoggedUserByScope.get(scopeUri) !== rawInterpreterPath) {
14721489
outputChannel.appendLine(
1473-
`[Ansible] Using user-configured interpreterPath: ${rawInterpreterPath}`,
1490+
`[Ansible] Using user-configured interpreterPath: ${rawInterpreterPath}${resolvedUserPath !== rawInterpreterPath ? ` (resolved: ${resolvedUserPath})` : ""}`,
14741491
);
14751492
lastLoggedUserByScope.set(scopeUri, rawInterpreterPath);
14761493
}
1494+
1495+
// Inject the expanded path if expansion succeeded
1496+
if (resolvedUserPath && resolvedUserPath !== rawInterpreterPath) {
1497+
result[i] = {
1498+
...config,
1499+
python: {
1500+
...pythonConfig,
1501+
interpreterPath: resolvedUserPath,
1502+
},
1503+
};
1504+
}
14771505
continue;
14781506
}
14791507

14801508
const scope = scopeUri ? vscode.Uri.parse(scopeUri) : undefined;
1481-
const resolvedPath = await pythonEnvService.getExecutablePath(scope);
1509+
let resolvedPath: string | undefined;
1510+
try {
1511+
resolvedPath = await pythonEnvService.getExecutablePath(scope);
1512+
} catch (error) {
1513+
// Treat resolution failure as "no path" - log once and leave config unchanged
1514+
if (lastLoggedResolvedByScope.get(scopeUri) !== "") {
1515+
outputChannel.appendLine(
1516+
`[Ansible] Failed to resolve Python environment: ${error}`,
1517+
);
1518+
lastLoggedResolvedByScope.set(scopeUri, "");
1519+
}
1520+
continue;
1521+
}
14821522

14831523
if (resolvedPath) {
14841524
// Log only when path actually changes

src/features/contentCreator/utils.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ export async function getBinDetail(cmd: string, arg: string) {
99
const extSettings = new SettingsManager();
1010
await extSettings.initialize();
1111

12-
const { command, env } = withInterpreter(extSettings.settings, cmd, arg);
12+
const { command, env } = await withInterpreter(
13+
extSettings.settings,
14+
cmd,
15+
arg,
16+
);
1317

1418
try {
1519
const result = cp.execSync(command, {

src/features/contentCreator/vue/views/createExecutionEnvPanel.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
disposePanelResources,
66
createOrRevealPanel,
77
} from "@src/features/contentCreator/vue/views/panelUtils";
8+
import { checkContentCreatorRequirements } from "@src/features/contentCreator/utils";
89

910
export class MainPanel {
1011
public static currentPanel: MainPanel | undefined;
@@ -20,6 +21,20 @@ export class MainPanel {
2021
this._disposables,
2122
() => this.dispose(),
2223
);
24+
25+
this._panel.webview.onDidReceiveMessage(
26+
async (msg) => {
27+
if (msg && msg.type === "request-requirements-status") {
28+
const status = await checkContentCreatorRequirements();
29+
this._panel.webview.postMessage({
30+
type: "requirements-status",
31+
...status,
32+
});
33+
}
34+
},
35+
null,
36+
this._disposables,
37+
);
2338
}
2439

2540
public static render(context: ExtensionContext) {

src/features/lightspeed/vue/views/ansibleCreatorUtils.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export class AnsibleCreatorOperations {
9191
const extSettings = new SettingsManager();
9292
await extSettings.initialize();
9393

94-
const { command, env } = withInterpreter(
94+
const { command, env } = await withInterpreter(
9595
extSettings.settings,
9696
ansibleCreatorAddCommand,
9797
"",
@@ -190,7 +190,7 @@ export class AnsibleCreatorOperations {
190190
const extSettings = new SettingsManager();
191191
await extSettings.initialize();
192192

193-
const { command, env } = withInterpreter(
193+
const { command, env } = await withInterpreter(
194194
extSettings.settings,
195195
ansibleCreatorAddCommand,
196196
"",
@@ -362,7 +362,7 @@ export class AnsibleCreatorOperations {
362362
const extSettings = new SettingsManager();
363363
await extSettings.initialize();
364364

365-
const { command, env } = withInterpreter(
365+
const { command, env } = await withInterpreter(
366366
extSettings.settings,
367367
ansibleCreatorInitCommand,
368368
"",
@@ -415,7 +415,7 @@ export class AnsibleCreatorOperations {
415415

416416
if (exceedADEImVersion) {
417417
adeCommand += " --im=cfg";
418-
const { command, env } = withInterpreter(
418+
const { command, env } = await withInterpreter(
419419
extSettings.settings,
420420
adeCommand,
421421
"",

src/features/lightspeed/vue/views/webviewMessageHandlers.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -742,10 +742,8 @@ export class WebviewMessageHandlers {
742742

743743
const templateSourcePath = vscode.Uri.joinPath(
744744
extensionUri,
745-
"out/resources/contentCreator/createDevcontainer/.devcontainer",
746-
)
747-
.toString()
748-
.replace("file://", "");
745+
"resources/contentCreator/createDevcontainer/.devcontainer",
746+
).fsPath;
749747

750748
await this.scaffoldDevcontainerStructure(
751749
templateSourcePath,
@@ -872,7 +870,7 @@ export class WebviewMessageHandlers {
872870
) {
873871
let devfile: string;
874872
const relativeTemplatePath =
875-
"out/resources/contentCreator/createDevfile/devfile-template.txt";
873+
"resources/contentCreator/createDevfile/devfile-template.txt";
876874

877875
const expandedDestUrl = expandPath(destinationUrl);
878876

@@ -882,9 +880,7 @@ export class WebviewMessageHandlers {
882880
const absoluteTemplatePath = vscode.Uri.joinPath(
883881
extensionUri,
884882
relativeTemplatePath,
885-
)
886-
.toString()
887-
.replace("file://", "");
883+
).fsPath;
888884

889885
try {
890886
const dirPath = path.dirname(expandedDestUrl);
@@ -1104,7 +1100,7 @@ export class WebviewMessageHandlers {
11041100
const extSettings = new SettingsManager();
11051101
await extSettings.initialize();
11061102

1107-
const { command, env } = withInterpreter(
1103+
const { command, env } = await withInterpreter(
11081104
extSettings.settings,
11091105
initEEProjectCommand,
11101106
"",

src/features/utils/buildExecutionEnvironment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export function rightClickEEBuildCommand(commandId: string): vscode.Disposable {
4343
const extSettings = new SettingsManager();
4444
await extSettings.initialize();
4545

46-
const { command, env } = withInterpreter(
46+
const { command, env } = await withInterpreter(
4747
extSettings.settings,
4848
builderCommand,
4949
"",
Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,64 @@
1-
/* stdlib */
2-
31
/* local */
2+
import * as path from "path";
3+
import * as vscode from "vscode";
44
import type { ExtensionSettings } from "@src/interfaces/extensionSettings";
5+
import { PythonEnvironmentService } from "@src/services/PythonEnvironmentService";
56

67
/**
7-
* A helper method to build a command with appropriate environment variables.
8+
* Build a command with the active Python environment's bin directory in PATH.
9+
* Uses Python environment resolution with user config fallback.
10+
* This ensures commands like ansible-creator run from the correct venv.
811
*
9-
* NOTE: This function is maintained for backward compatibility.
10-
* For terminal-based execution, prefer using TerminalService which handles
12+
* NOTE: For terminal-based execution, prefer using TerminalService which handles
1113
* Python environment activation automatically via the Python extension.
1214
*
1315
* @param settings - The extension setting.
1416
* @param runExecutable - The name of the executable to run.
1517
* @param cmdArgs - The arguments to the executable.
16-
* @returns The complete command to be executed with environment.
18+
* @param scope - Optional workspace scope for environment resolution.
19+
* @returns Promise resolving to the command and environment with venv PATH.
1720
*/
18-
export function withInterpreter(
21+
export async function withInterpreter(
1922
settings: ExtensionSettings,
2023
runExecutable: string,
2124
cmdArgs: string,
22-
): { command: string; env: NodeJS.ProcessEnv } {
25+
scope?: import("vscode").Uri,
26+
): Promise<{ command: string; env: NodeJS.ProcessEnv }> {
27+
// Build command string
2328
const command = `${runExecutable} ${cmdArgs}`;
2429

25-
const newEnv = Object.assign({}, process.env, {
30+
// Start with base environment and Ansible-specific vars
31+
const env = Object.assign({}, process.env, {
2632
ANSIBLE_FORCE_COLOR: "0", // ensure output is parsable (no ANSI)
27-
PYTHONBREAKPOINT: "0", // We want to be sure that python debugger is never
28-
// triggered, even if we mistakenly left a breakpoint() there while
29-
// debugging ansible-lint, or another tool we call.
33+
PYTHONBREAKPOINT: "0", // prevent debugger from being triggered
3034
});
3135

32-
return { command: command, env: newEnv } as const;
36+
// Resolve Python environment and add venv bin to PATH
37+
const pythonEnvService = PythonEnvironmentService.getInstance();
38+
39+
// Get interpreter path (respects user config, then Python extension)
40+
const execPath = await pythonEnvService.resolveInterpreterPath(
41+
settings.interpreterPath,
42+
scope,
43+
);
44+
45+
if (execPath) {
46+
// Add venv bin directory to PATH (like TerminalService does)
47+
const binDir = path.dirname(execPath);
48+
const existingPath = env.PATH ?? process.env.PATH ?? "";
49+
env.PATH = `${binDir}${path.delimiter}${existingPath}`;
50+
51+
// Set VIRTUAL_ENV for tools that check it (defensive pattern from TerminalService)
52+
const environment = await pythonEnvService.getEnvironment(scope);
53+
if (environment?.environmentPath) {
54+
const envPath = environment.environmentPath;
55+
if (envPath instanceof vscode.Uri) {
56+
env.VIRTUAL_ENV = envPath.fsPath;
57+
} else if (typeof envPath === "string") {
58+
env.VIRTUAL_ENV = envPath;
59+
}
60+
}
61+
}
62+
63+
return { command, env };
3364
}

src/features/vault.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,15 @@ function displayInvalidConfigError(): void {
5050
export class Vault {
5151
constructor(private extSettings: SettingsManager) {}
5252

53-
private ansibleVaultCmd(args: string): {
53+
private async ansibleVaultCmd(args: string): Promise<{
5454
command: string;
5555
env: NodeJS.ProcessEnv;
56-
} {
57-
return withInterpreter(this.extSettings.settings, "ansible-vault", args);
56+
}> {
57+
return await withInterpreter(
58+
this.extSettings.settings,
59+
"ansible-vault",
60+
args,
61+
);
5862
}
5963

6064
async toggleEncrypt(): Promise<void> {
@@ -272,7 +276,7 @@ export class Vault {
272276
});
273277
}
274278

275-
private encryptText(
279+
private async encryptText(
276280
text: string,
277281
rootPath: string | undefined,
278282
vaultId: string | undefined,
@@ -281,20 +285,20 @@ export class Vault {
281285
if (vaultId) {
282286
args += ` --encrypt-vault-id ${vaultId}`;
283287
}
284-
const { command: cmd, env } = this.ansibleVaultCmd(args);
288+
const { command: cmd, env } = await this.ansibleVaultCmd(args);
285289
return this.pipeTextThroughCmd(text, rootPath, cmd, env);
286290
}
287291

288-
private decryptText(
292+
private async decryptText(
289293
text: string,
290294
rootPath: string | undefined,
291295
): Promise<string> {
292296
const args = "decrypt";
293-
const { command: cmd, env } = this.ansibleVaultCmd(args);
297+
const { command: cmd, env } = await this.ansibleVaultCmd(args);
294298
return this.pipeTextThroughCmd(text, rootPath, cmd, env);
295299
}
296300

297-
private encryptFile(
301+
private async encryptFile(
298302
f: string,
299303
rootPath: string | undefined,
300304
vaultId: string | undefined,
@@ -305,15 +309,15 @@ export class Vault {
305309
"encrypt " +
306310
(vaultId ? `--encrypt-vault-id="${vaultId}" ` : "") +
307311
`"${f}"`;
308-
const { command: cmd, env } = this.ansibleVaultCmd(args);
312+
const { command: cmd, env } = await this.ansibleVaultCmd(args);
309313

310314
return execCwd(cmd, rootPath, env);
311315
}
312316

313-
private decryptFile = (f: string, rootPath: string | undefined) => {
317+
private decryptFile = async (f: string, rootPath: string | undefined) => {
314318
console.log(`Decrypt file: ${f}`);
315319
const args = `decrypt "${f}"`;
316-
const { command: cmd, env } = this.ansibleVaultCmd(args);
320+
const { command: cmd, env } = await this.ansibleVaultCmd(args);
317321

318322
return execCwd(cmd, rootPath, env);
319323
};

0 commit comments

Comments
 (0)