fix(appbuilder): getSpawnEnv should return correct env on windows

packages/core/src/shared/env/resolveEnv.ts

@@ -139,29 +139,29 @@ export async function mergeResolvedShellPath(env: IProcessEnvironment): Promise<
  * - we hit a timeout of `MAX_SHELL_RESOLVE_TIME`
  * - any other error from spawning a shell to figure out the environment
  */
-export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise<typeof process.env> {
+export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise<typeof process.env | undefined> {
     if (!env) {
         env = process.env
     }
     // Skip if forceResolveEnv is set to false
     if (DevSettings.instance._isSet('forceResolveEnv') && !DevSettings.instance.get('forceResolveEnv', false)) {
         getLogger().debug('resolveShellEnv(): skipped (forceResolveEnv)')
 
-        return {}
+        return undefined
     }
 
     // Skip on windows
     else if (process.platform === 'win32') {
         getLogger().debug('resolveShellEnv(): skipped (Windows)')
 
-        return {}
+        return undefined
     }
 
     // Skip if running from CLI already and forceResolveEnv is not true
     else if (isLaunchedFromCli(env) && !DevSettings.instance.get('forceResolveEnv', false)) {
         getLogger().info('resolveShellEnv(): skipped (VSCODE_CLI is set)')
 
-        return {}
+        return undefined
     }
     // Otherwise resolve (macOS, Linux)
     else {
@@ -181,10 +181,15 @@ export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise<ty
 
                 // Resolve shell env and handle errors
                 try {
-                    resolve(await doResolveUnixShellEnv(timeout))
+                    const shellEnv = await doResolveUnixShellEnv(timeout)
+                    if (shellEnv && Object.keys(shellEnv).length > 0) {
+                        resolve(shellEnv)
+                    } else {
+                        return undefined
+                    }
                 } catch {
                     // failed resolve should not affect other feature.
-                    resolve({})
+                    return undefined
                 }
             })
         }

packages/core/src/shared/sam/localLambdaRunner.ts

@@ -212,10 +212,7 @@ async function invokeLambdaHandler(
         return config
             .samLocalInvokeCommand!.invoke({
                 options: {
-                    env: await getSpawnEnv({
-                        ...process.env,
-                        ...env,
-                    }),
+                    env: env,
                 },
                 command: samCommand,
                 args: samArgs,
@@ -236,7 +233,7 @@ async function invokeLambdaHandler(
             templatePath: config.templatePath,
             eventPath: config.eventPayloadFile,
             environmentVariablePath: config.envFile,
-            environmentVariables: await getSpawnEnv(env),
+            environmentVariables: env,
             invoker: config.samLocalInvokeCommand!,
             dockerNetwork: config.sam?.dockerNetwork,
             debugPort: debugPort,
@@ -290,15 +287,16 @@ export async function runLambdaFunction(
         getLogger().info(localize('AWS.output.sam.local.startRun', 'Preparing to run locally: {0}', config.handlerName))
     }
 
-    const envVars = {
+    const envVars = await getSpawnEnv({
+        ...process.env,
         ...(config.aws?.region ? { AWS_DEFAULT_REGION: config.aws.region } : {}),
-    }
+    })
 
     const settings = SamCliSettings.instance
     const timer = new Timeout(settings.getLocalInvokeTimeout())
 
     // TODO: refactor things to not mutate the config
-    config.templatePath = await buildLambdaHandler(timer, await getSpawnEnv(envVars), config, settings)
+    config.templatePath = await buildLambdaHandler(timer, envVars, config, settings)
 
     await onAfterBuild()
     timer.refresh()
@@ -315,12 +313,13 @@ export async function runLambdaFunction(
 
     // SAM CLI and any API requests are executed in parallel
     // A failure from either is a failure for the whole invocation
-    const [process] = await Promise.all([invokeLambdaHandler(timer, envVars, config, settings), apiRequest]).catch(
-        (err) => {
-            timer.cancel()
-            throw err
-        }
-    )
+    const [processInvoker] = await Promise.all([
+        invokeLambdaHandler(timer, envVars, config, settings),
+        apiRequest,
+    ]).catch((err) => {
+        timer.cancel()
+        throw err
+    })
 
     if (config.noDebug) {
         return config
@@ -329,7 +328,7 @@ export async function runLambdaFunction(
     const terminationListener = vscode.debug.onDidTerminateDebugSession((session) => {
         const config = session.configuration as SamLaunchRequestArgs
         if (config.invokeTarget?.target === 'api') {
-            stopApi(process, config)
+            stopApi(processInvoker, config)
         }
     })
 

packages/core/src/test/shared/env/resolveEnv.test.ts

@@ -0,0 +1,71 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import assert from 'assert'
+import * as resolveEnv from '../../../shared/env/resolveEnv'
+import sinon from 'sinon'
+import path from 'path'
+
+describe('resolveEnv', async function () {
+    let sandbox: sinon.SinonSandbox
+    beforeEach(function () {
+        sandbox = sinon.createSandbox()
+    })
+
+    afterEach(function () {
+        sandbox.restore()
+    })
+
+    // a copy of resolveEnv.mergeResolvedShellPath for stubbing getResolvedShellEnv
+    // mergeResolvedShellPath is calling getResolvedShellEnv within the same file
+    // thus we need a copy to stub getResolvedShellEnv correctly
+    const testMergeResolveShellPath = async function mergeResolvedShellPath(
+        env: resolveEnv.IProcessEnvironment
+    ): Promise<typeof process.env> {
+        const shellEnv = await resolveEnv.getResolvedShellEnv(env)
+        // resolve failed or doesn't need to resolve
+        if (!shellEnv) {
+            return env
+        }
+        try {
+            const envPaths: string[] = env.PATH ? env.PATH.split(path.delimiter) : []
+            const resolvedPaths: string[] = shellEnv.PATH ? shellEnv.PATH.split(path.delimiter) : []
+            const envReturn = { ...env }
+            // merge, dedup, join
+            envReturn.PATH = [...new Set(envPaths.concat(resolvedPaths))].join(path.delimiter)
+
+            return envReturn
+        } catch (err) {
+            return env
+        }
+    }
+
+    describe('windows', async function () {
+        beforeEach(function () {
+            sandbox.stub(process, 'platform').value('win32')
+        })
+
+        it('mergeResolvedShellPath should not change path on windows', async function () {
+            const env = await resolveEnv.mergeResolvedShellPath(process.env)
+            assert(env.PATH)
+            assert.strictEqual(env, process.env)
+        })
+    })
+
+    describe('unix', async function () {
+        const originalEnv = { ...process.env }
+        // skip mac test on windows
+        if (process.platform !== 'win32') {
+            it('mergeResolvedShellPath should get path on mac/linux', async function () {
+                sandbox.stub(process.env, 'PATH').value('')
+                // stub the resolve Env logic cause this is platform sensitive.
+                sandbox.stub(resolveEnv, 'getResolvedShellEnv').resolves(originalEnv)
+                const env = await testMergeResolveShellPath(process.env)
+                assert(env.PATH)
+                assert.notEqual(env, process.env)
+            })
+        }
+    })
+})

packages/toolkit/.changes/next-release/Bug Fix-268ceb41-7bec-4b57-b195-d72f19d00faa.json

@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "System Path parsing should ignore Windows and only parse Mac/Linux system, Sam Local Invoke should get correct system Path on windows"
+}