Fix: detect PowerShell 7+ (pwsh) in the UnsafeLocalCodeExecutor SHELL branch

[AmaadMartin]

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

No issue exists for this report.

2. Or, if no issue exists, describe the change:

Problem:

CodeExecutionLanguage.SHELL ignores PowerShell 7+ (pwsh) when selecting spawn arguments and the script extension.

The SHELL branch of UnsafeLocalCodeExecutor.executeCode picked PowerShell-specific spawn arguments with a substring test against the literal string powershell:

if (this.shellCommandPath.toLowerCase().includes('powershell')) {
  args = ['-NoLogo', '-ExecutionPolicy', 'Bypass', '-File', filePath];
}

PowerShell 7+ ships as pwsh / pwsh.exe, not powershell — the executor's own CodeExecutionLanguage.POWERSHELL branch already acknowledges this (IS_WINDOWS ? 'powershell' : 'pwsh'). So new UnsafeLocalCodeExecutor({shellCommandPath: 'pwsh'}) produced an invocation that cannot work:

• args stayed at the default [filePath], so none of -NoLogo, -ExecutionPolicy Bypass, -File were passed.
• getExtensionForLanguage had the same blind spot, so on non-Windows hosts the script was written as script.sh. PowerShell refuses a -File argument without a .ps1 extension, so fixing the arguments alone would not have been enough.

Observed: spawn('pwsh', ['/tmp/.../script.sh']).

Expected: spawn('pwsh', ['-NoLogo', '-ExecutionPolicy', 'Bypass', '-File', '/tmp/.../script.ps1']) — the same shape the POWERSHELL language branch already produces.

The substring test had a second defect: /usr/local/powershell-helpers/run.sh was misclassified as PowerShell and received PowerShell flags.

Solution:

Replace the substring test with a module-private predicate that matches on the executable name only:

function isPowerShellCommand(commandPath: string): boolean {
  return /^(powershell|pwsh)(\.exe)?$/i.test(path.win32.basename(commandPath));
}

and use it for both the spawn arguments and the script extension. path.win32.basename is used rather than path.basename because it splits on both / and \ on every platform, so a Windows-style path is handled correctly when the tests run on Linux/macOS CI. An exact-name allowlist is deliberate: a substring match is what caused this bug class in the first place, so /opt/pwsh-tools/bin/bash must not match.

Two intentional behavior changes, both fixes of clearly-wrong behavior:

1. A shellCommandPath naming a PowerShell 7+ host now receives PowerShell flags and a .ps1 script instead of a bare positional .sh invocation. The previously produced invocation could not work.
2. Commands that merely contain powershell as a substring but are not a PowerShell host (for example /usr/local/powershell-helpers/run.sh) no longer receive PowerShell flags. That match was accidental and produced a broken invocation.

Existing default behavior (bash off Windows, powershell on Windows, explicit cmd) is bit-for-bit identical. No public API, type, option, export, or dependency change.

Also in this diff, both small and load-bearing:

• import {spawn} from 'child_process' → 'node:child_process'. This was the only bare child_process specifier in the repository; every other file and this file's three sibling imports already use the node: prefix. It is also required for the test's module spy to intercept the same specifier the source imports.
• createTempScriptFile and getExtensionForLanguage took shellCommandPath?: string, but the only call site passes this.shellCommandPath, which the constructor always defaults to a non-empty string. Both are now required, which retires an unreachable shellCommandPath && guard. Both functions are module-private, so this is not an API change.

Explicitly out of scope: cmd detection still uses includes('cmd') and is unchanged byte-for-byte, and -NoProfile is not added. Both are left for a follow-up change; the new tests are written so that inserting -NoProfile later will not require rewriting them.

Testing Plan

Unit Tests:

• I have added or updated unit tests for my change.
• All unit tests pass locally.

The bug is about the argv handed to spawn, which the pre-existing tests could not observe (they assert on the stdout/stderr of real child processes). core/test/code_executors/unsafe_local_code_executor_test.ts now uses vitest's autospy — vi.mock('node:child_process', {spy: true}) — which wraps the real export without replacing its implementation, so all 14 pre-existing tests keep executing real child processes unchanged while the 11 new cases assert only on spawnSpy.mock.calls. That makes them deterministic on ubuntu-latest, windows-latest and macos-latest whether or not pwsh/cmd exists on the runner: a missing binary just resolves through the existing Process error: path after the arguments were already recorded.

New describe('shell command detection') cases:

• PowerShell is detected for pwsh, pwsh.exe, /usr/bin/pwsh, C:\Program Files\PowerShell\7\pwsh.exe, PWSH, and (regression) powershell, powershell.exe — each gets the PowerShell flags, -File immediately before the script path, and a script.ps1 extension.
• No substring misfire for /opt/pwsh-tools/bin/bash and /usr/local/powershell-helpers/run.sh — argv is the bare [filePath].
• cmd and cmd.exe are unaffected — argv is ['/c', <script>].

Commands run locally:

npx vitest run --project unit:core core/test/code_executors/unsafe_local_code_executor_test.ts   # 25 passed
npx vitest run --project unit:core core/test/code_executors core/test/tools/skills               # 193 passed
npm run build        # OK
npm run lint         # OK
npm run format:check # OK
npm run docs:check   # OK (typedoc --treatWarningsAsErrors)
npx secretlint       # OK

The new tests were verified to actually catch the bug: reverting only core/src/code_executors/unsafe_local_code_executor.ts to its pre-fix state fails 8 of the 25 (all 7 PowerShell rows plus the /usr/local/powershell-helpers/run.sh misfire row) and leaves the other 17 passing.

The shell command detection block carries an explicit 30s timeout. CI runners that ship PowerShell really launch it for these cases, and the first launch on a cold runner exceeded vitest's default 5s timeout — which is itself evidence the fixed invocation is accepted by a real PowerShell host.

Manual End-to-End (E2E) Tests:

On a host with PowerShell 7 installed:

import {CodeExecutionLanguage, UnsafeLocalCodeExecutor} from '@google/adk';

const executor = new UnsafeLocalCodeExecutor({shellCommandPath: 'pwsh'});
const result = await executor.executeCode({
  invocationContext,
  codeExecutionInput: {
    code: 'Write-Host "hello from pwsh"',
    language: CodeExecutionLanguage.SHELL,
    inputFiles: [],
  },
});

result.stdout contains hello from pwsh and result.stderr is empty. Before this change the script was written as .sh and handed to pwsh as a bare positional argument, so nothing was executed.

PowerShell 7 was not installed on the machine used for development, so this was verified end-to-end against the built package with no mocks by putting an executable named pwsh on disk that enforces the PowerShell -File contract (it exits 64 unless it receives exactly -NoLogo -ExecutionPolicy Bypass -File <path>.ps1, then runs the script). Real process spawn, real temp-file I/O:

RESULT /tmp/.../bin/pwsh  stdout="hello from /tmp/.../bin/pwsh\n"  stderr=""
RESULT bash               stdout="hello from bash\n"               stderr=""

Against the pre-fix build the same run fails with pwsh: expected '-NoLogo -ExecutionPolicy Bypass -File <script>', got: /tmp/.../script.sh. The bash row confirms the default path is unchanged.

Checklist

• I have read the CONTRIBUTING.md document.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests pass locally with my changes.
• I have manually tested my changes end-to-end.
• Any dependent changes have been merged and published in downstream modules.

Additional context

The POWERSHELL language branch and the SHELL branch now agree on what counts as a PowerShell host, so a pwsh path reaches the same -NoLogo -ExecutionPolicy Bypass -File invocation through either entry point.

[kalenkevich]

why changed to be required?

[AmaadMartin]

Good catch - reverted in 449368a, both createTempScriptFile and getExtensionForLanguage take shellCommandPath?: string again.

The reasoning was that the only call site is executeCode, which always passes this.shellCommandPath, and the constructor defaults that to 'powershell'/'bash' - so the optional made the undefined case unreachable. But that is a pre-existing cleanup and unrelated to this bug fix, so it did not belong in this diff. The new PowerShell check now guards the same way the cmd check on the next line already does:

if (shellCommandPath && isPowerShellCommand(shellCommandPath)) {
  return '.ps1';
}

The diff is now only the pwsh detection fix. Happy to send the signature tightening separately if you want it.