fix(core): align shell tool description with configured shell

[kkhomej33-netizen]

Summary

• What changed: Updated the shell tool guidance so it describes the same shell wrapper and syntax rules that command execution actually uses.
• Why it changed: Windows command execution can use cmd.exe, Windows PowerShell, PowerShell 7, or Git Bash/MSYS depending on the environment, but the previous tool guidance always described Windows as cmd.exe and included Bash-only syntax in all cases.
• Reviewer focus: Please check that the generated guidance stays accurate for cmd.exe, Windows PowerShell, PowerShell 7, Bash, and Git Bash/MSYS environments without changing command execution behavior.

Validation

• Commands run:

  cd packages/core
  npx vitest run src/tools/shell.test.ts
  npm run typecheck
  npm run build
  git diff --check

• Prompts / inputs used: N/A
• Expected result: The focused shell tool tests pass, TypeScript type checking passes, the core package builds, and the diff has no whitespace errors.
• Observed result: All listed commands completed successfully.
• Quickest reviewer verification path:

  cd packages/core
  npx vitest run src/tools/shell.test.ts

• Evidence (output, logs, screenshots, video, JSON, before/after, etc.):

  ✓ src/tools/shell.test.ts (182 tests)
  Test Files  1 passed (1)
  Tests  182 passed (182)
  
  > @qwen-code/qwen-code-core@0.15.11 typecheck
  > tsc --noEmit
  
  > @qwen-code/qwen-code-core@0.15.11 build
  > node ../../scripts/build_package.js
  Successfully copied files.
  
  git diff --check
  # no output
  

Scope / Risk

• Main risk or tradeoff: This changes model-facing instructions only. The main risk is wording that could overfit one shell variant, especially around PowerShell command sequencing.
• Not covered / not validated: Manual execution on native Windows, Linux, Docker, Podman, and Seatbelt was not performed.
• Breaking changes / migration notes: None.

Testing Matrix

	🍏	🪟	🐧
npm run	✅	⚠️	⚠️
npx	✅	⚠️	⚠️
Docker	N/A	N/A	N/A
Podman	N/A	N/A	N/A
Seatbelt	N/A	N/A	N/A

Testing matrix notes:

• macOS validation covered the focused unit test, typecheck, and core package build.
• Windows and Linux were covered by mocked platform/environment unit tests, not by manual native runs.
• Docker, Podman, and Seatbelt are not involved in this model-facing shell tool description change.

Linked Issues / Bugs

None.

[wenshao]

No issues found. LGTM! ✅ — glm-5.1 via Qwen Code /review

[wenshao]

[Suggestion] Missing test coverage: the getDescription tests cover Git Bash detection via MSYSTEM=MINGW64 but do not cover the TERM-based Git Bash path (TERM containing msys or cygwin, without MSYSTEM set). getShellConfiguration() handles this case but the shell description layer has no corresponding test, leaving a regression gap for shell-aware description generation.

[wenshao]

[Suggestion] processGroupNote still uses os.platform() === 'win32' (platform-based) instead of being shell-aware like the rest of the description. When Windows runs Git Bash (shell='bash'), the description says "The active shell is Bash" with Bash quoting guidance, but the process group note (kill -- -PGID) is suppressed because isWindows=true. The test explicitly asserts this behavior (.not.toContain('Command process group can be terminated')), but it creates an inconsistency: the model gets Bash instructions for quoting/sequencing but misses the process group management note.

If Git Bash on Windows genuinely does not support kill -- -PGID, consider adding a comment explaining why the platform check is intentional here while the rest of the function is shell-aware. If it does support it, change the check to shellConfiguration.shell === 'bash'.

Suggested change

	const isWindows = os.platform() === 'win32';
	// Git Bash on Windows does not support POSIX process group signals,
	// so we gate process group notes on platform, not shell type.
	const processGroupNote = isWindows
	? ''
	: '\n - Command is executed as a subprocess that leads its own process group. Command process group can be terminated as `kill -- -PGID` or signaled as `kill -s SIGNAL -- -PGID`.';

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

Code Review Summary

Overall: This is a well-structured PR that cleanly replaces hardcoded cmd.exe descriptions with dynamic shell detection. The helper function decomposition (getExecutableBasename, getShellDisplayName, getShellExecutionWrapper, etc.) is clean and the test coverage is thorough (182/182 passing). Build and lint pass.

Deterministic analysis: tsc = 0, eslint = 0 findings.

---

Findings

#	File	Finding	Confidence	Level
1	shell.ts	Redundant path.basename() wrapper in getExecutableBasename	High	Suggestion
2	shell.test.ts	3 new test cases missing toMatchSnapshot() (existing tests have them)	High	Suggestion

Note: The processGroupNote using os.platform() (already commented inline) is intentionally correct — process groups are an OS capability, not a shell feature. Test assertions confirm this.

---

Reviewed 3 changed files (+248/-47). 9 review agents + verification + reverse audit.

Review by Qwen Code — model: mimo-v2.5-pro

[wenshao]

[Suggestion] path.win32.basename() already handles both \ and / separators, so the outer path.basename() is redundant — path.win32.basename alone returns the correct basename on all platforms.

// Current:
return path.basename(path.win32.basename(executable));

// Simplified:
return path.win32.basename(executable);

This is a minor clarity improvement; the double-call is functionally harmless but may confuse future readers into thinking it handles an edge case that doesn't exist.

Confidence: High

[wenshao]

[Suggestion] The two existing getDescription tests ('should return the windows description' and 'should return the non-windows description') both include toMatchSnapshot() assertions, but the three new tests (should describe PowerShell…, should describe pwsh…, should describe bash when Windows is running in Git Bash) only use toContain()/not.toContain() without snapshot assertions.

Adding expect(shellTool.description).toMatchSnapshot() to each would:

1. Keep the test pattern consistent
2. Catch unexpected wording regressions that toContain might miss
3. Create a reviewable snapshot diff showing the exact description for each shell

Confidence: High

[wenshao]

Local tmux verification transcript, for the record.

1. PR-listed npx vitest run src/tools/shell.test.ts

$ cd /tmp/pr4170/packages/core && npx vitest run --no-coverage src/tools/shell.test.ts

 RUN  v3.2.4 /tmp/pr4170/packages/core

 ✓ src/tools/shell.test.ts (182 tests) 515ms

 Test Files  1 passed (1)
      Tests  182 passed (182)
   Duration  2.90s

Matches the PR body's claimed 182 tests exactly.

2. npm run typecheck

$ npm run typecheck

> @qwen-code/qwen-code-core@0.15.11 typecheck
> tsc --noEmit

(clean, exit 0)

3. npm run build

$ npm run build

> @qwen-code/qwen-code-core@0.15.11 build
> node ../../scripts/build_package.js

Successfully copied files.

4. git diff --check

$ cd /tmp/pr4170 && git diff --check $(git merge-base main HEAD)..HEAD
(clean, exit 0)

Final state at merge

Check	Result
mergeable	MERGEABLE
mergeStateStatus	CLEAN
reviewDecision	APPROVED
Lint / CodeQL / Test mac · ubuntu · windows	all SUCCESS
Unresolved Critical comments	0
Unresolved Suggestion comments	3 (non-blocking — processGroupNote is still platform-based instead of shell-aware, path.basename(path.win32.basename(...)) is a redundant double-call, and the 3 new tests don't carry snapshot assertions; worth a follow-up but not a merge blocker)

Design notes

The new helpers split the previously hardcoded cmd.exe / bash -c description into:

• getShellDisplayName — ShellType → user-facing executable name (cmd.exe / powershell.exe / pwsh.exe / bash) with an exhaustive never switch
• getShellQuotingGuidance(shell) — three distinct rule sets (bash ANSI-C / heredoc, PowerShell here-string / single-quote doubling, cmd.exe ^ escaping)
• getShellCommandSequencingGuidance({executable, shell}) — four branches, importantly distinguishing pwsh.exe (PS 7+, supports &&) from powershell.exe (Windows PowerShell 5.x, requires $LASTEXITCODE flow control)

That pwsh-vs-powershell distinction is the one detail a more naive refactor would have missed and the part the model would actually trip over.

Environment

• Linux x86_64, Node v20.19.2 (project engines require ≥22; only the project's own doctorChecks Node-version assertion reflects this in CI's matrix)
• PR head 05018f7dd against main merge-base 1c529e4f0
• Worktree at /tmp/pr4170 with node_modules and core/dist symlinked from a parent install