Skip to content

Skip directory .env / python.envFile paths when resolving environment variables - #1865

Merged
Eduardo Villalpando Mello (edvilme) merged 4 commits into
mainfrom
copilot/implement-narrow-fix-issue-1857
Oct 14, 2026
Merged

Eduardo Villalpando Mello (edvilme) merged 4 commits into
mainfrom
copilot/implement-narrow-fix-issue-1857

Conversation

Copilot AI commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

If <project>/.env was a directory, for example a virtual environment named .env, getEnvironmentVariables() tried to read it as a file and threw EntryIsADirectory. That broke terminal env injection and Pylance. The same happened when python.envFile pointed at a directory. The cause was the fs-extra pathExists check, which also returns true for directories.

Changes

  • envVariableManager.ts: both pathExists checks are replaced with a private isEnvFile helper, which accepts only regular files:
    async function isEnvFile(filePath: string): Promise<boolean> {
        try {
            return (await fsapi.stat(filePath)).isFile();
        } catch (error) {
            const code = (error as NodeJS.ErrnoException).code;
            if (code === 'ENOENT' || code === 'ENOTDIR') {
                return false;
            }
            throw error;
        }
    }
    • It uses stat, not lstat, so symlinks to regular files still load. Symlinks to directories are skipped.
    • Errors other than ENOENT/ENOTDIR (for example, EACCES) are still raised, not hidden.
    • parseEnvFile is never called on a skipped path.
  • Tests (envVariableManager.unit.test.ts): these call getEnvironmentVariables directly and cover:
    • a directory at either path
    • a valid file next to a directory
    • two regular files loading in the existing order
    • symlinks to a file or a directory, using real temp-dir symlinks that are skipped if the platform can't create them
    • missing paths and ENOTDIR being skipped, and EACCES being raised

Not changed

  • Precedence: the order documented in docs/README.md stays the same: base → python.envFile → project .env → overrides. This PR does not implement the issue's separate request to have python.envFile suppress project .env discovery.
  • Other behavior: path selection, deduplication, merging, and URI overrides are untouched.
  • Race: this is a check before the read, so a path that changes type between the stat and the read can still fail.

Co-authored-by: edvilme <5952839+edvilme@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix environment-variable resolution for virtual environment directory Skip directory .env / python.envFile paths when resolving environment variables Oct 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused implementation correctly addresses directory paths while preserving existing precedence and error behavior.

0 open findings

What changed in this PR

Prevents environment-variable loading from attempting to parse directories as .env files.

Changes:

  • Validates configured and project env paths as regular files.
  • Preserves symlink handling and propagates unexpected filesystem errors.
  • Adds coverage for directories, symlinks, precedence, and error cases.
File Description
src/​features/​execution/​envVariableManager.ts Adds regular-file validation before parsing env files.
src/​test/​features/​execution/​envVariableManager.unit.test.ts Tests file types, symlinks, precedence, and filesystem errors.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: edvilme <5952839+edvilme@users.noreply.github.com>
@heejaechang

Heejae Chang (heejaechang) commented Oct 13, 2026 •

Copy link
Copy Markdown

🔒 Automated review in progress — Heejae Chang (@heejaechang) is auto-reviewing this PR.

@edvilme Eduardo Villalpando Mello (edvilme) added the bug Issue identified by VS Code Team member as probable bug label Oct 13, 2026
@heejaechang

Copy link
Copy Markdown

Result: 🔴 could-not-verify

Verification details

Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Summary: [unavailable] Container verification could not start and local execution was not authorized for this PR HEAD: Could not prepare offline npm dependencies: Could not download semver@7.8.4 from the Microsoft package feed (ReadTimeout).

Test runs: none recorded.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approved via Review Center.

@heejaechang Heejae Chang (heejaechang) added the review-auto:approved Automated review: no blocking findings (approval posted). label Oct 14, 2026

@bschnurr Bill Schnurr (bschnurr) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved via Review Center.

@edvilme
Eduardo Villalpando Mello (edvilme) merged commit 9f1e63d into main Oct 14, 2026
162 of 164 checks passed
@edvilme
Eduardo Villalpando Mello (edvilme) deleted the copilot/implement-narrow-fix-issue-1857 branch October 14, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue identified by VS Code Team member as probable bug review-auto:approved Automated review: no blocking findings (approval posted).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants