Make standalone script template creation reliable

docs/managing-python-projects.md

@@ -75,9 +75,9 @@ Use this to scaffold a brand new Python project with the correct structure and f
    - **Package**: A structured Python package with `pyproject.toml`, tests folder, and package directory
    - **Script**: A simple standalone Python file using PEP 723 inline metadata
 4. Enter a name for your project.
-5. Choose whether to create a virtual environment.
+5. If you're creating a package, choose whether to create a virtual environment.
 
-The extension creates the project structure, adds it to your workspace, and optionally creates a virtual environment.
+The extension creates the project structure and adds it to your workspace. For packages, it can also create a virtual environment.
 
 #### Package template structure
 

files/templates/copilot-instructions-text/script-copilot-instructions.md

@@ -1,7 +1,7 @@
 # Copilot Instructions for <script_name>
 
--   The script `<script_name>` is a Python python project within the workspace.
--   It has inline script metadata (as proposed by PEP 723) that defines the script name, required python version, and dependencies.
+-   The script `<script_name>` is a Python project within the workspace.
+-   It has inline script metadata (as proposed by PEP 723) that defines the required Python version and dependencies.
 -   If imports which require a specific Python version or dependencies are added, keep the inline script metadata up to date.
 -   You need to call the `Get Python Environment Information` tool on the `<script_name>` path to get the Python executable details.
 -   Substitute the Python executable you get from the `Get Python Environment Information` tool anywhere you see `<python>` in these instructions.

files/templates/new723ScriptTemplate/script.py → files/templates/newInlineScriptTemplate/script.py

@@ -1,8 +1,7 @@
 # /// script
-# requires-python = ">=X.XX" TODO: Update this to the minimum Python version you want to support
-# dependencies = [
-#   TODO: Add any dependencies your script requires
-# ]
+# requires-python = ">=3.9"
+# dependencies = []
+# # Add dependency requirement strings to the array above as needed.
 # ///
 
 # TODO: Update the main function to your needs or remove it.

src/common/utils/pathUtils.ts

@@ -65,6 +65,44 @@ export function normalizePath(fsPath: string): string {
     return path1;
 }
 
+/**
+ * Determines whether `candidatePath` is the same as, or nested inside, `parentPath`.
+ *
+ * Both paths are resolved to absolute form and normalized (case-insensitive on
+ * Windows) before comparison, so differences in separators or drive-letter case
+ * do not affect the result.
+ *
+ * @param parentPath The candidate parent (or ancestor) directory.
+ * @param candidatePath The path being tested for containment.
+ * @returns `true` when `candidatePath` equals `parentPath` or is a descendant of it.
+ */
+export function isSameOrParentPath(parentPath: string, candidatePath: string): boolean {
+    const relative = path.relative(
+        normalizePath(path.resolve(parentPath)),
+        normalizePath(path.resolve(candidatePath)),
+    );
+    return (
+        relative === '' ||
+        (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative))
+    );
+}
+
+/**
+ * Determines whether `value` maps to a reserved Windows device name (e.g. `CON`,
+ * `PRN`, `AUX`, `NUL`, `COM1`-`COM9`, `LPT1`-`LPT9`).
+ *
+ * The check inspects the portion of the name before the first dot, since Windows
+ * disallows these names regardless of extension, and only reports `true` on
+ * Windows, where the restriction applies.
+ *
+ * @param value The base file name (without directory) to test.
+ * @returns `true` on Windows when `value` resolves to a reserved device name.
+ */
+export function isWindowsReservedDeviceName(value: string): boolean {
+    const deviceBaseName = value.split('.')[0];
+    return isWindows() && /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i.test(deviceBaseName);
+}
+
 export function getResourceUri(resourcePath: string, root?: string): Uri | undefined {
     try {
         if (!resourcePath) {

src/features/creators/newScriptProject.ts

@@ -1,33 +1,66 @@
 import * as fs from 'fs-extra';
 import * as path from 'path';
-import { commands, l10n, MarkdownString, QuickInputButtons, Uri, window, workspace } from 'vscode';
+import { commands, l10n, MarkdownString, QuickInputButtons, Uri, window, WorkspaceFolder } from 'vscode';
 import { PythonProject, PythonProjectCreator, PythonProjectCreatorOptions } from '../../api';
 import { NEW_PROJECT_TEMPLATES_FOLDER } from '../../common/constants';
 import { traceError } from '../../common/logging';
-import { showInputBoxWithButtons, showTextDocument } from '../../common/window.apis';
+import { isSameOrParentPath, isWindowsReservedDeviceName } from '../../common/utils/pathUtils';
+import { showErrorMessage, showInputBoxWithButtons, showTextDocument } from '../../common/window.apis';
+import { getWorkspaceFolder, getWorkspaceFolders } from '../../common/workspace.apis';
 import { PythonProjectManager } from '../../internal.api';
 import { isCopilotInstalled, manageCopilotInstructionsFile, replaceInFilesAndNames } from './creationHelpers';
 
+function validateScriptFileName(value: string): string | null {
+    const pathSegments = value.split(/[\\/]/);
+    if (pathSegments.length !== 1 || pathSegments.includes('..')) {
+        return l10n.t('Script name must be a file name without path separators or traversal.');
+    }
+    if (!value.endsWith('.py')) {
+        return l10n.t('Script name must end with ".py".');
+    }
+    const baseName = value.replace(/\.py$/, '');
+    if (isWindowsReservedDeviceName(baseName)) {
+        return l10n.t('Script name uses a reserved Windows device name.');
+    }
+    // following PyPI (PEP 508) rules for package names
+    if (!/^([a-z_]|[a-z0-9_][a-z0-9._-]*[a-z0-9_])$/i.test(baseName)) {
+        return l10n.t(
+            'Invalid script name. Use only letters, numbers, underscores, hyphens, or periods. Must start and end with a letter or number.',
+        );
+    }
+    if (/^[-._0-9]$/i.test(baseName)) {
+        return l10n.t('Single-character script names cannot be a number, hyphen, or period.');
+    }
+    return null;
+}
+
+function uriForFileRootInWorkspace(rootPath: string, workspaceFolder: WorkspaceFolder): Uri {
+    const relativeRootPath = path.relative(path.resolve(workspaceFolder.uri.fsPath), path.resolve(rootPath));
+    const pathSegments = relativeRootPath.split(/[\\/]/).filter((segment) => segment.length > 0);
+    return workspaceFolder.uri.with({
+        path: path.posix.join(workspaceFolder.uri.path, ...pathSegments),
+    });
+}
+
 export class NewScriptProject implements PythonProjectCreator {
     public readonly name = l10n.t('newScript');
     public readonly displayName = l10n.t('Script');
-    public readonly description = l10n.t('Creates a new script folder in your current workspace');
+    public readonly description = l10n.t('Creates a new script in your current workspace');
     public readonly tooltip = new MarkdownString(l10n.t('Create a new Python script'));
 
     constructor(private readonly projectManager: PythonProjectManager) {}
 
     async create(options?: PythonProjectCreatorOptions): Promise<PythonProject | Uri | undefined> {
-        // quick create (needs name, will always create venv and copilot instructions)
-        // not quick create
-        // ask for script file name
-        // ask if they want venv
         let scriptFileName = options?.name;
         let createCopilotInstructions: boolean | undefined;
         if (options?.quickCreate === true) {
             // If quickCreate is true, we should not prompt for any input
             if (!scriptFileName) {
                 throw new Error('Script file name is required in quickCreate mode.');
             }
+            if (path.extname(scriptFileName) === '') {
+                scriptFileName = `${scriptFileName}.py`;
+            }
             createCopilotInstructions = true;
         } else {
             //Prompt as quickCreate is false
@@ -37,23 +70,7 @@ export class NewScriptProject implements PythonProjectCreator {
                         prompt: l10n.t('What is the name of the script? (e.g. my_script.py)'),
                         ignoreFocusOut: true,
                         showBackButton: true,
-                        validateInput: (value) => {
-                            // Ensure the filename ends with .py and follows valid naming conventions
-                            if (!value.endsWith('.py')) {
-                                return l10n.t('Script name must end with ".py".');
-                            }
-                            const baseName = value.replace(/\.py$/, '');
-                            // following PyPI (PEP 508) rules for package names
-                            if (!/^([a-z_]|[a-z0-9_][a-z0-9._-]*[a-z0-9_])$/i.test(baseName)) {
-                                return l10n.t(
-                                    'Invalid script name. Use only letters, numbers, underscores, hyphens, or periods. Must start and end with a letter or number.',
-                                );
-                            }
-                            if (/^[-._0-9]$/i.test(baseName)) {
-                                return l10n.t('Single-character script names cannot be a number, hyphen, or period.');
-                            }
-                            return null;
-                        },
+                        validateInput: validateScriptFileName,
                     });
                 } catch (ex) {
                     if (ex === QuickInputButtons.Back) {
@@ -67,64 +84,143 @@ export class NewScriptProject implements PythonProjectCreator {
                     createCopilotInstructions = true;
                 }
             }
+        }
+        const validationError = validateScriptFileName(scriptFileName);
+        if (validationError) {
+            if (options?.quickCreate === true) {
+                throw new Error(validationError);
+            }
+            window.showErrorMessage(validationError);
+            return undefined;
+        }
 
-            // 1. Copy template folder
-            const newScriptTemplateFile = path.join(NEW_PROJECT_TEMPLATES_FOLDER, 'new723ScriptTemplate', 'script.py');
-            if (!(await fs.pathExists(newScriptTemplateFile))) {
-                window.showErrorMessage(l10n.t('Template file does not exist, aborting creation.'));
-                traceError(`Template file not found at: ${newScriptTemplateFile}`);
+        // 1. Copy template file
+        const newScriptTemplateFile = path.join(NEW_PROJECT_TEMPLATES_FOLDER, 'newInlineScriptTemplate', 'script.py');
+        if (!(await fs.pathExists(newScriptTemplateFile))) {
+            window.showErrorMessage(l10n.t('Template file does not exist, aborting creation.'));
+            traceError(`Template file not found at: ${newScriptTemplateFile}`);
+            return undefined;
+        }
+
+        // Check if the destination folder is provided, otherwise use the first workspace folder.
+        let destinationRootUri = options?.rootUri;
+        let workspaceFolders: readonly WorkspaceFolder[] | undefined;
+        if (!destinationRootUri) {
+            workspaceFolders = getWorkspaceFolders();
+            if (!workspaceFolders || workspaceFolders.length === 0) {
+                window.showErrorMessage(l10n.t('No workspace folder is open or provided, aborting creation.'));
                 return undefined;
             }
+            destinationRootUri = workspaceFolders[0].uri;
+        }
 
-            // Check if the destination folder is provided, otherwise use the first workspace folder
-            let destRoot = options?.rootUri.fsPath;
-            if (!destRoot) {
-                const workspaceFolders = workspace.workspaceFolders;
-                if (!workspaceFolders || workspaceFolders.length === 0) {
-                    window.showErrorMessage(l10n.t('No workspace folder is open or provided, aborting creation.'));
-                    return undefined;
-                }
-                destRoot = workspaceFolders[0].uri.fsPath;
-            }
+        const destRoot = destinationRootUri.fsPath;
+        const resolvedDestRoot = path.resolve(destRoot);
+        let workspaceFolder = getWorkspaceFolder(destinationRootUri);
+        if (!workspaceFolder && destinationRootUri.scheme === 'file') {
+            workspaceFolders ??= getWorkspaceFolders();
+            workspaceFolder = workspaceFolders
+                ?.filter((folder) => isSameOrParentPath(folder.uri.fsPath, resolvedDestRoot))
+                .sort((first, second) => second.uri.fsPath.length - first.uri.fsPath.length)[0];
+        }
+        if (!workspaceFolder) {
+            showErrorMessage(l10n.t('Destination folder must be inside an open workspace, aborting creation.'));
+            return undefined;
+        }
 
-            // Check if the destination folder already exists
-            const scriptDestination = path.join(destRoot, scriptFileName);
-            if (await fs.pathExists(scriptDestination)) {
-                window.showErrorMessage(
-                    l10n.t(
-                        'A script file by that name already exists, aborting creation. Please retry with a unique script name given your workspace.',
-                    ),
-                );
-                return undefined;
+        let physicalDestRoot: string;
+        let physicalWorkspaceRoot: string;
+        try {
+            [physicalDestRoot, physicalWorkspaceRoot] = await Promise.all([
+                fs.realpath(resolvedDestRoot),
+                fs.realpath(workspaceFolder.uri.fsPath),
+            ]);
+        } catch (error) {
+            traceError('Failed to resolve the destination or workspace folder:', error);
+            showErrorMessage(l10n.t('Unable to resolve the destination folder inside the open workspace.'));
+            return undefined;
+        }
+        if (!isSameOrParentPath(physicalWorkspaceRoot, physicalDestRoot)) {
+            showErrorMessage(l10n.t('Destination folder must resolve inside the open workspace, aborting creation.'));
+            return undefined;
+        }
+
+        const identityRootUri =
+            destinationRootUri.scheme === 'file' && workspaceFolder.uri.scheme !== 'file'
+                ? uriForFileRootInWorkspace(resolvedDestRoot, workspaceFolder)
+                : destinationRootUri;
+        const scriptDestination = path.resolve(resolvedDestRoot, scriptFileName);
+        const relativeScriptPath = path.relative(resolvedDestRoot, scriptDestination);
+        if (
+            relativeScriptPath === '' ||
+            relativeScriptPath === '..' ||
+            relativeScriptPath.startsWith(`..${path.sep}`) ||
+            path.isAbsolute(relativeScriptPath)
+        ) {
+            const containmentError = l10n.t('Script name must resolve to a file inside the destination folder.');
+            if (options?.quickCreate === true) {
+                throw new Error(containmentError);
             }
-            await fs.copy(newScriptTemplateFile, scriptDestination);
+            window.showErrorMessage(containmentError);
+            return undefined;
+        }
 
-            // 2. Replace 'script_name' in the file using a helper (just script name remove .py)
+        // Check if the destination file already exists
+        if (await fs.pathExists(scriptDestination)) {
+            window.showErrorMessage(
+                l10n.t(
+                    'A script file by that name already exists, aborting creation. Please retry with a unique script name given your workspace.',
+                ),
+            );
+            return undefined;
+        }
+        // Build the project entry up front so copying the template, substituting
+        // the script name, and registering the project share one cleanup boundary:
+        // if any step fails, the partially created script is removed so a retry
+        // starts from a clean state.
+        const createdScript: PythonProject = {
+            name: scriptFileName,
+            uri: identityRootUri.with({
+                path: path.posix.join(identityRootUri.path, scriptFileName),
+            }),
+        };
+        let projectRegistrationAttempted = false;
+        try {
+            await fs.copy(newScriptTemplateFile, scriptDestination);
+            // Replace 'script_name' in the file (script name without the .py suffix).
             await replaceInFilesAndNames(scriptDestination, 'script_name', scriptFileName.replace(/\.py$/, ''));
-
-            // 3. add custom github copilot instructions
-            if (createCopilotInstructions) {
-                const packageInstructionsPath = path.join(
-                    NEW_PROJECT_TEMPLATES_FOLDER,
-                    'copilot-instructions-text',
-                    'script-copilot-instructions.md',
-                );
-                await manageCopilotInstructionsFile(destRoot, packageInstructionsPath, [
-                    { searchValue: '<script_name>', replaceValue: scriptFileName },
-                ]);
+            projectRegistrationAttempted = true;
+            await this.projectManager.add(createdScript);
+        } catch (creationError) {
+            if (projectRegistrationAttempted) {
+                try {
+                    this.projectManager.remove(createdScript);
+                } catch (rollbackError) {
+                    traceError('Failed to remove the new script project after creation failed:', rollbackError);
+                }
             }
+            try {
+                await fs.remove(scriptDestination);
+            } catch (rollbackError) {
+                traceError('Failed to delete the new script after creation failed:', rollbackError);
+            }
+            throw creationError;
+        }
 
-            // Add the created script to the project manager
-            const createdScript: PythonProject | undefined = {
-                name: scriptFileName,
-                uri: Uri.file(scriptDestination),
-            };
-            this.projectManager.add(createdScript);
+        // 3. add custom github copilot instructions
+        if (createCopilotInstructions) {
+            const packageInstructionsPath = path.join(
+                NEW_PROJECT_TEMPLATES_FOLDER,
+                'copilot-instructions-text',
+                'script-copilot-instructions.md',
+            );
+            await manageCopilotInstructionsFile(destRoot, packageInstructionsPath, [
+                { searchValue: '<script_name>', replaceValue: scriptFileName },
+            ]);
+        }
 
-            await showTextDocument(createdScript.uri);
+        await showTextDocument(createdScript.uri);
 
-            return createdScript;
-        }
-        return undefined;
+        return createdScript;
     }
 }

src/features/terminal/utils.ts

@@ -11,6 +11,7 @@ import { VENV_MANAGER_ID } from '../../common/constants';
 import { traceError, traceVerbose } from '../../common/logging';
 import { timeout } from '../../common/utils/asyncUtils';
 import { createSimpleDebounce } from '../../common/utils/debounce';
+import { isSameOrParentPath } from '../../common/utils/pathUtils';
 import { onDidChangeTerminalShellIntegration, onDidWriteTerminalData } from '../../common/window.apis';
 import { getConfiguration, getWorkspaceFolders } from '../../common/workspace.apis';
 import { identifyTerminalShell } from '../common/shellDetector';
@@ -199,11 +200,6 @@ async function getDistinctProjectEnvs(
     return envs;
 }
 
-function isSameOrParentPath(parentPath: string, candidatePath: string): boolean {
-    const relative = path.relative(path.resolve(parentPath), path.resolve(candidatePath));
-    return relative === '' || (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative));
-}
-
 function getProjectForCwd(projects: readonly PythonProject[], cwd: string): PythonProject | undefined {
     return [...projects]
         .filter((project) => isSameOrParentPath(project.uri.fsPath, cwd))