fix: gate task_shell_start/task_shell_wait behind allow_shell (#2303)

[cy2311]

Closes #2303

The allow_shell security gate blocked exec_shell in Agent mode by default but left task_shell_start and task_shell_wait unconditionally registered, bypassing the gate.

Move TaskShellStartTool and TaskShellWaitTool from with_runtime_task_tools() into with_shell_tools() so they share the same allow_shell gate as exec_shell and the other shell tools.

Before

Tool	Agent (default)
exec_shell	blocked
task_shell_start	always allowed

After

Tool	Agent (default)
exec_shell	blocked
task_shell_start	blocked (unless allow_shell=true)

Greptile Summary

This PR adds a Windows .bat launcher for CodeWhale that prefers Windows Terminal (wt) when available and falls back to a plain cmd invocation. The .bat file is registered in the ASSET_MATRIX and its SHA-256 hash is included in the checksum manifest.

• artifacts.js: adds "codewhale.bat" as a third element to the Windows x64 asset array, but allAssetNames() only reads pair[0] and pair[1], so the .bat entry is silently excluded from all downstream asset-name lists.
• prepare-local-release-assets.js: generates the .bat content, but set NO_ANIMATIONS=1 is placed after each process launch — after the binary has already exited in the else branch, and in the wrong (parent) environment in the wt branch — so the variable never reaches the running executable.
• The PR title and description describe a completely different change (gating shell tools behind allow_shell), which raises concerns about whether the correct commits are present.

Confidence Score: 2/5

Not safe to merge: the bat launcher's NO_ANIMATIONS env var never reaches the running process, the .bat file is missing from all asset-name lists, and the PR description describes an entirely different change.

Three real defects are present in just two files. The env var ordering bug means the feature shipped in this PR (Windows Terminal launcher with animation suppression) is broken by construction. The allAssetNames omission means downstream npm install and CI publishing steps will silently skip codewhale.bat. And the description/diff mismatch raises doubt about whether the intended security fix is even present in this branch.

Both changed files need attention: artifacts.js for the asset-list omission and prepare-local-release-assets.js for the env-var ordering and the description mismatch.

Important Files Changed

Filename	Overview
scripts/release/prepare-local-release-assets.js	Adds a Windows .bat launcher generation block with two bugs: NO_ANIMATIONS=1 is set after the executable runs (making it useless), and the wt branch sets it in the parent process rather than the new terminal session.
npm/codewhale/scripts/artifacts.js	Adds codewhale.bat as a third element to the Windows x64 asset array, but allAssetNames() only reads pair[0] and pair[1], so the .bat file is silently omitted from all asset name lists.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[codewhale.bat invoked] --> B{where wt}
    B -->|found ERRORLEVEL=0| C["wt --title CodeWhale cmd /k codewhale-windows-x64.exe"]
    C --> D["set NO_ANIMATIONS=1 (parent process — never seen by exe)"]
    B -->|not found| E["codewhale-windows-x64.exe (runs synchronously)"]
    E --> F["set NO_ANIMATIONS=1 (exe already exited — no effect)"]

    style D fill:#f88,stroke:#c00
    style F fill:#f88,stroke:#c00

Loading

Comments Outside Diff (1)

1. npm/codewhale/scripts/artifacts.js, line 111-119 (link)

   allAssetNames() silently drops codewhale.bat. The function pushes only pair[0] and pair[1] from each platform array; codewhale.bat is now pair[2] for the Windows x64 entry and is never emitted. Any code that relies on allAssetNames() or allReleaseAssetNames() (e.g. npm install download scripts, CI publishing pipelines) will not know this artifact exists.

   

_{Reviews (1): Last reviewed commit: "fix: Windows .bat launcher with correct ..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

[gemini-code-assist]

Code Review

This pull request introduces a Windows batch launcher (codewhale.bat) that prefers Windows Terminal. However, the review identifies two critical issues: first, adding the batch file to the asset matrix causes it to be ignored by several helper functions, preventing it from being properly downloaded or handled during installation; second, the generated batch file does not correctly pass environment variables to the executable (setting NO_ANIMATIONS=1 after execution) and fails to forward command-line arguments.

[gemini-code-assist]

Adding "codewhale.bat" as a third element in the win32.x64 array will cause it to be ignored by several helper functions in this file:

1. allAssetNames() (lines 111-119) explicitly pushes only pair[0] and pair[1], meaning codewhale.bat won't be included in the list of all assets.
2. detectBinaryNames() (lines 26-53) only returns codewhale: pair[0] and tui: pair[1].
3. Consequently, npm/codewhale/scripts/install.js (which relies on detectBinaryNames()) will not download or handle codewhale.bat on Windows.

Consider updating allAssetNames() to dynamically push all elements of the array (e.g., names.push(...pair)), and updating the installation script to download and place the .bat file correctly.

[gemini-code-assist]

There are two issues with the generated batch file content:

1. Environment Variable Inheritance: set NO_ANIMATIONS=1 is executed after the executable is launched in both branches. Since wt is launched asynchronously and the direct executable blocks until exit, the executable never actually runs with NO_ANIMATIONS=1 set in its environment. Placing it at the top of the script allows it to be inherited correctly.
2. Argument Forwarding: The batch file does not forward command-line arguments (like --help or other flags) to the underlying executable. Adding %* to the execution commands fixes this.

    const batContent = [
      "@echo off",
      "set NO_ANIMATIONS=1",
      "where wt >nul 2>nul",
      'if "%ERRORLEVEL%"=="0" (',
      '    wt --title CodeWhale cmd /k "%~dp0codewhale-windows-x64.exe" %*',
      ") else (",
      '    "%~dp0codewhale-windows-x64.exe" %*',
      ")",
      "",
    ].join("\r\n");

[greptile-apps]

NO_ANIMATIONS=1 is set after the process runs, so it never takes effect. In the else branch the env var is assigned only after the binary has already exited. In the wt branch, wt returns immediately (async), so set NO_ANIMATIONS=1 runs in the parent cmd session whose environment is never inherited by the new Windows Terminal window. The env var needs to be placed before each launch path, and for the wt path it must be embedded in the child command string.

Suggested change

	"@echo off",
	"where wt >nul 2>nul",
	'if "%ERRORLEVEL%"=="0" (',
	' wt --title CodeWhale cmd /k "%~dp0codewhale-windows-x64.exe"',
	" set NO_ANIMATIONS=1",
	") else (",
	' "%~dp0codewhale-windows-x64.exe"',
	" set NO_ANIMATIONS=1",
	")",
	"@echo off",
	"where wt >nul 2>nul",
	'if "%ERRORLEVEL%"=="0" (',
	' wt --title CodeWhale cmd /k "set NO_ANIMATIONS=1 && %~dp0codewhale-windows-x64.exe"',
	") else (",
	" set NO_ANIMATIONS=1",
	' "%~dp0codewhale-windows-x64.exe"',
	")",

[greptile-apps]

PR description does not match the actual diff. The title and description describe gating task_shell_start/task_shell_wait behind allow_shell, but the changes here (and in artifacts.js) are entirely about adding a Windows .bat launcher. This may indicate the wrong branch was merged into this PR, or the description was copy-pasted from a different issue. Worth verifying the correct changes are included before merging.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!