Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/constants/prompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { TASK_CREATE_TOOL_NAME } from '@claude-code-best/builtin-tools/tools/Tas
import type { Tools } from '../Tool.js'
import type { Command } from '../types/command.js'
import { BASH_TOOL_NAME } from '@claude-code-best/builtin-tools/tools/BashTool/toolName.js'
import { POWERSHELL_TOOL_NAME } from '@claude-code-best/builtin-tools/tools/PowerShellTool/toolName.js'
import {
getCanonicalName,
getMarketingNameForModel,
Expand Down Expand Up @@ -273,8 +274,20 @@ function getUsingYourToolsSection(enabledTools: Set<string>): string {
return [`# Using your tools`, ...prependBullets(items)].join(`\n`)
}

const hasPowerShell = enabledTools.has(POWERSHELL_TOOL_NAME)
const hasBash = enabledTools.has(BASH_TOOL_NAME)
const shellToolGuidance = hasPowerShell
? hasBash
? `On Windows, prefer the ${POWERSHELL_TOOL_NAME} tool for terminal operations (git, npm, docker, builds, tests, system commands). Use ${BASH_TOOL_NAME} only when the user asks for bash/Git Bash or a command is clearly bash-only. Prefer dedicated tools over shell equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content/cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep/Select-String).`
: `Prefer dedicated tools over ${POWERSHELL_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content, ${FILE_EDIT_TOOL_NAME} over (Get-Content) -replace, ${GLOB_TOOL_NAME} over Get-ChildItem -Recurse, ${GREP_TOOL_NAME} over Select-String). Reserve ${POWERSHELL_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`
: `Prefer dedicated tools over ${BASH_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep). Reserve ${BASH_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`

const coreToolsList = hasPowerShell
? `Core tools (Read, Edit, Write, Glob, Grep, ${POWERSHELL_TOOL_NAME}${hasBash ? `, ${BASH_TOOL_NAME}` : ''}, Agent, WebFetch, WebSearch, AskUserQuestion, NotebookEdit, TaskCreate, TaskUpdate, TaskList, TaskGet, TodoWrite, Skill, CronCreate, CronDelete, CronList, Config, LSP, MCPTool) can be called directly as needed. ${shellToolGuidance}`
: `Core tools (Read, Edit, Write, Glob, Grep, Bash, Agent, WebFetch, WebSearch, AskUserQuestion, NotebookEdit, TaskCreate, TaskUpdate, TaskList, TaskGet, TodoWrite, Skill, CronCreate, CronDelete, CronList, Config, LSP, MCPTool) can be called directly as needed. ${shellToolGuidance}`

Comment on lines +277 to +288

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Dynamically include shell tools and avoid hardcoding "Bash".

In the hasPowerShell === false branch, "Bash" is unconditionally hardcoded into coreToolsList, and shellToolGuidance assumes Bash is enabled. If the Bash tool happens to be disabled (hasBash is false), this will confuse the model by providing guidance for a non-existent tool.

Consider refactoring to dynamically build the list of enabled shell tools and selectively append the guidance.

♻️ Proposed refactor
-  const shellToolGuidance = hasPowerShell
-    ? hasBash
-      ? `On Windows, prefer the ${POWERSHELL_TOOL_NAME} tool for terminal operations (git, npm, docker, builds, tests, system commands). Use ${BASH_TOOL_NAME} only when the user asks for bash/Git Bash or a command is clearly bash-only. Prefer dedicated tools over shell equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content/cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep/Select-String).`
-      : `Prefer dedicated tools over ${POWERSHELL_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content, ${FILE_EDIT_TOOL_NAME} over (Get-Content) -replace, ${GLOB_TOOL_NAME} over Get-ChildItem -Recurse, ${GREP_TOOL_NAME} over Select-String). Reserve ${POWERSHELL_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`
-    : `Prefer dedicated tools over ${BASH_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep). Reserve ${BASH_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`
-
-  const coreToolsList = hasPowerShell
-    ? `Core tools (Read, Edit, Write, Glob, Grep, ${POWERSHELL_TOOL_NAME}${hasBash ? `, ${BASH_TOOL_NAME}` : ''}, Agent, WebFetch, WebSearch, AskUserQuestion, NotebookEdit, TaskCreate, TaskUpdate, TaskList, TaskGet, TodoWrite, Skill, CronCreate, CronDelete, CronList, Config, LSP, MCPTool) can be called directly as needed. ${shellToolGuidance}`
-    : `Core tools (Read, Edit, Write, Glob, Grep, Bash, Agent, WebFetch, WebSearch, AskUserQuestion, NotebookEdit, TaskCreate, TaskUpdate, TaskList, TaskGet, TodoWrite, Skill, CronCreate, CronDelete, CronList, Config, LSP, MCPTool) can be called directly as needed. ${shellToolGuidance}`
+  let shellToolGuidance = ''
+  if (hasPowerShell && hasBash) {
+    shellToolGuidance = `On Windows, prefer the ${POWERSHELL_TOOL_NAME} tool for terminal operations (git, npm, docker, builds, tests, system commands). Use ${BASH_TOOL_NAME} only when the user asks for bash/Git Bash or a command is clearly bash-only. Prefer dedicated tools over shell equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content/cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep/Select-String).`
+  } else if (hasPowerShell) {
+    shellToolGuidance = `Prefer dedicated tools over ${POWERSHELL_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content, ${FILE_EDIT_TOOL_NAME} over (Get-Content) -replace, ${GLOB_TOOL_NAME} over Get-ChildItem -Recurse, ${GREP_TOOL_NAME} over Select-String). Reserve ${POWERSHELL_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`
+  } else if (hasBash) {
+    shellToolGuidance = `Prefer dedicated tools over ${BASH_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep). Reserve ${BASH_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`
+  }
+
+  const shellTools = [
+    hasPowerShell ? POWERSHELL_TOOL_NAME : null,
+    hasBash ? BASH_TOOL_NAME : null
+  ].filter(Boolean).join(', ')
+
+  const coreToolsList = `Core tools (Read, Edit, Write, Glob, Grep${shellTools ? `, ${shellTools}` : ''}, Agent, WebFetch, WebSearch, AskUserQuestion, NotebookEdit, TaskCreate, TaskUpdate, TaskList, TaskGet, TodoWrite, Skill, CronCreate, CronDelete, CronList, Config, LSP, MCPTool) can be called directly as needed.${shellToolGuidance ? ' ' + shellToolGuidance : ''}`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const hasPowerShell = enabledTools.has(POWERSHELL_TOOL_NAME)
const hasBash = enabledTools.has(BASH_TOOL_NAME)
const shellToolGuidance = hasPowerShell
? hasBash
? `On Windows, prefer the ${POWERSHELL_TOOL_NAME} tool for terminal operations (git, npm, docker, builds, tests, system commands). Use ${BASH_TOOL_NAME} only when the user asks for bash/Git Bash or a command is clearly bash-only. Prefer dedicated tools over shell equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content/cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep/Select-String).`
: `Prefer dedicated tools over ${POWERSHELL_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content, ${FILE_EDIT_TOOL_NAME} over (Get-Content) -replace, ${GLOB_TOOL_NAME} over Get-ChildItem -Recurse, ${GREP_TOOL_NAME} over Select-String). Reserve ${POWERSHELL_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`
: `Prefer dedicated tools over ${BASH_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep). Reserve ${BASH_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`
const coreToolsList = hasPowerShell
? `Core tools (Read, Edit, Write, Glob, Grep, ${POWERSHELL_TOOL_NAME}${hasBash ? `, ${BASH_TOOL_NAME}` : ''}, Agent, WebFetch, WebSearch, AskUserQuestion, NotebookEdit, TaskCreate, TaskUpdate, TaskList, TaskGet, TodoWrite, Skill, CronCreate, CronDelete, CronList, Config, LSP, MCPTool) can be called directly as needed. ${shellToolGuidance}`
: `Core tools (Read, Edit, Write, Glob, Grep, Bash, Agent, WebFetch, WebSearch, AskUserQuestion, NotebookEdit, TaskCreate, TaskUpdate, TaskList, TaskGet, TodoWrite, Skill, CronCreate, CronDelete, CronList, Config, LSP, MCPTool) can be called directly as needed. ${shellToolGuidance}`
const hasPowerShell = enabledTools.has(POWERSHELL_TOOL_NAME)
const hasBash = enabledTools.has(BASH_TOOL_NAME)
let shellToolGuidance = ''
if (hasPowerShell && hasBash) {
shellToolGuidance = `On Windows, prefer the ${POWERSHELL_TOOL_NAME} tool for terminal operations (git, npm, docker, builds, tests, system commands). Use ${BASH_TOOL_NAME} only when the user asks for bash/Git Bash or a command is clearly bash-only. Prefer dedicated tools over shell equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content/cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep/Select-String).`
} else if (hasPowerShell) {
shellToolGuidance = `Prefer dedicated tools over ${POWERSHELL_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over Get-Content, ${FILE_EDIT_TOOL_NAME} over (Get-Content) -replace, ${GLOB_TOOL_NAME} over Get-ChildItem -Recurse, ${GREP_TOOL_NAME} over Select-String). Reserve ${POWERSHELL_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`
} else if (hasBash) {
shellToolGuidance = `Prefer dedicated tools over ${BASH_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep). Reserve ${BASH_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`
}
const shellTools = [
hasPowerShell ? POWERSHELL_TOOL_NAME : null,
hasBash ? BASH_TOOL_NAME : null
].filter(Boolean).join(', ')
const coreToolsList = `Core tools (Read, Edit, Write, Glob, Grep${shellTools ? `, ${shellTools}` : ''}, Agent, WebFetch, WebSearch, AskUserQuestion, NotebookEdit, TaskCreate, TaskUpdate, TaskList, TaskGet, TodoWrite, Skill, CronCreate, CronDelete, CronList, Config, LSP, MCPTool) can be called directly as needed.${shellToolGuidance ? ' ' + shellToolGuidance : ''}`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/constants/prompts.ts` around lines 277 - 288, Update coreToolsList and
shellToolGuidance to derive shell-tool names from hasPowerShell and hasBash
rather than hardcoding Bash or assuming it is enabled. Ensure the guidance only
references enabled shell tools, while preserving the existing PowerShell/Bash
preference behavior when both are available and providing appropriate
dedicated-tool guidance when neither or only one is enabled.

const items = [
`Core tools (Read, Edit, Write, Glob, Grep, Bash, Agent, WebFetch, WebSearch, AskUserQuestion, NotebookEdit, TaskCreate, TaskUpdate, TaskList, TaskGet, TodoWrite, Skill, CronCreate, CronDelete, CronList, Config, LSP, MCPTool) can be called directly as needed. Prefer dedicated tools over ${BASH_TOOL_NAME} equivalents (e.g., ${FILE_READ_TOOL_NAME} over cat, ${FILE_EDIT_TOOL_NAME} over sed, ${GLOB_TOOL_NAME} over find, ${GREP_TOOL_NAME} over grep). Reserve ${BASH_TOOL_NAME} for shell operations: package installs, test runners, build commands, git operations.`,
coreToolsList,
`Search before saying unknown — when the user references a file, function, or module you have not seen, search with ${GREP_TOOL_NAME}/${GLOB_TOOL_NAME} first.`,
taskToolName
? `Break down and manage your work with the ${taskToolName} tool. Mark each task as completed as soon as you are done.`
Expand Down
7 changes: 4 additions & 3 deletions src/services/tips/tipRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,12 @@ const externalTips: Tip[] = [
{
id: 'powershell-tool-env',
content: async () =>
'Set CLAUDE_CODE_USE_POWERSHELL_TOOL=1 to enable the PowerShell tool (preview)',
cooldownSessions: 10,
'PowerShell is the default shell on Windows. Set CLAUDE_CODE_USE_POWERSHELL_TOOL=0 or defaultShell=bash to prefer Bash/Git Bash instead.',
cooldownSessions: 20,
isRelevant: async () =>
getPlatform() === 'windows' &&
process.env.CLAUDE_CODE_USE_POWERSHELL_TOOL === undefined,
process.env.CLAUDE_CODE_USE_POWERSHELL_TOOL === undefined &&
getSettings_DEPRECATED().defaultShell === undefined,
},
{
id: 'status-line',
Expand Down
4 changes: 2 additions & 2 deletions src/utils/settings/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,13 +460,13 @@ export const SettingsSchema = lazySchema(() =>
.boolean()
.optional()
.describe('Disable all hooks and statusLine execution'),
// Which shell backs input-box `!` (see docs/design/ps-shell-selection.md §4.2)
// Which shell backs input-box `!` (see resolveDefaultShell)
defaultShell: z
.enum(['bash', 'powershell'])
.optional()
.describe(
'Default shell for input-box ! commands. ' +
"Defaults to 'bash' on all platforms (no Windows auto-flip).",
"Defaults to 'powershell' on Windows when the PowerShell tool is enabled, otherwise 'bash'. Set explicitly to override.",
),
// Only run hooks defined in managed settings (managed-settings.json)
allowManagedHooksOnly: z
Expand Down
73 changes: 73 additions & 0 deletions src/utils/shell/__tests__/shellDefaults.windows.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/**
* Windows default shell: PowerShell tool on by default; ! routing prefers it.
*/
import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'

const settingsState: { defaultShell?: 'bash' | 'powershell' } = {}

mock.module('src/utils/settings/settings.js', () => ({
getInitialSettings: () => ({ ...settingsState }),
}))

// Force windows platform for these unit tests regardless of host OS.
mock.module('src/utils/platform.js', () => ({
getPlatform: () => 'windows' as const,
SUPPORTED_PLATFORMS: ['macos', 'wsl'],
}))
Comment on lines +1 to +16

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.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Follow test naming and mocking conventions.

The test file violates two project coding guidelines:

  1. Filename convention: The filename shellDefaults.windows.test.ts does not match the tested modules. As per coding guidelines, place unit tests alongside source modules with the filename pattern <module>.test.ts (e.g., this should be split into shellToolUtils.test.ts and resolveDefaultShell.test.ts).
  2. Mocking pattern: The mock.module() calls use inline factories at the module level. As per coding guidelines, use the use-shared-mock pattern by importing a shared mock from tests/mocks/ and passing it to mock.module() inside a beforeAll block to prevent cross-file mock pollution.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/shell/__tests__/shellDefaults.windows.test.ts` around lines 1 - 16,
Rename and relocate the tests to follow the tested modules’ convention,
splitting them into shellToolUtils.test.ts and resolveDefaultShell.test.ts as
appropriate. Replace the module-level inline mock.module factories with shared
mocks imported from tests/mocks/, and register those mocks inside a beforeAll
block while preserving the existing Windows platform and settings behavior.

Source: Coding guidelines


import { isPowerShellToolEnabled } from '../shellToolUtils.js'
import { resolveDefaultShell } from '../resolveDefaultShell.js'

const ENV_KEY = 'CLAUDE_CODE_USE_POWERSHELL_TOOL'
let savedEnv: string | undefined

beforeEach(() => {
savedEnv = process.env[ENV_KEY]
delete process.env[ENV_KEY]
delete settingsState.defaultShell
})

afterEach(() => {
if (savedEnv === undefined) delete process.env[ENV_KEY]
else process.env[ENV_KEY] = savedEnv
delete settingsState.defaultShell
})

describe('isPowerShellToolEnabled (windows)', () => {
test('enabled by default when env unset', () => {
expect(isPowerShellToolEnabled()).toBe(true)
})

test('disabled when env is falsy', () => {
process.env[ENV_KEY] = '0'
expect(isPowerShellToolEnabled()).toBe(false)
process.env[ENV_KEY] = 'false'
expect(isPowerShellToolEnabled()).toBe(false)
})

test('enabled when env is truthy', () => {
process.env[ENV_KEY] = '1'
expect(isPowerShellToolEnabled()).toBe(true)
})
})

describe('resolveDefaultShell (windows)', () => {
test('defaults to powershell when tool enabled and no settings', () => {
expect(resolveDefaultShell()).toBe('powershell')
})

test('honors settings.defaultShell=bash', () => {
settingsState.defaultShell = 'bash'
expect(resolveDefaultShell()).toBe('bash')
})

test('honors settings.defaultShell=powershell', () => {
settingsState.defaultShell = 'powershell'
expect(resolveDefaultShell()).toBe('powershell')
})

test('falls back to bash when PowerShell tool is disabled', () => {
process.env[ENV_KEY] = '0'
expect(resolveDefaultShell()).toBe('bash')
})
})
23 changes: 17 additions & 6 deletions src/utils/shell/resolveDefaultShell.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,25 @@
import { getInitialSettings } from '../settings/settings.js'
import { getPlatform } from '../platform.js'
import { isPowerShellToolEnabled } from './shellToolUtils.js'

/**
* Resolve the default shell for input-box `!` commands.
* Resolve the default shell for input-box `!` commands and agent preference.
*
* Resolution order (docs/design/ps-shell-selection.md §4.2):
* settings.defaultShell → 'bash'
* Resolution order:
* 1. settings.defaultShell (explicit user/project setting)
* 2. Windows + PowerShell tool enabled → 'powershell'
* 3. otherwise → 'bash'
*
* Platform default is 'bash' everywhere — we do NOT auto-flip Windows to
* PowerShell (would break existing Windows users with bash hooks).
* Opt out of the Windows PowerShell default via settings.defaultShell=bash
* or CLAUDE_CODE_USE_POWERSHELL_TOOL=0 (disables the PowerShell tool entirely).
*/
export function resolveDefaultShell(): 'bash' | 'powershell' {
return getInitialSettings().defaultShell ?? 'bash'
const configured = getInitialSettings().defaultShell
if (configured === 'bash' || configured === 'powershell') {
return configured
}
if (getPlatform() === 'windows' && isPowerShellToolEnabled()) {
return 'powershell'
}
return 'bash'
}
12 changes: 6 additions & 6 deletions src/utils/shell/shellToolUtils.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import { BASH_TOOL_NAME } from '@claude-code-best/builtin-tools/tools/BashTool/toolName.js'
import { POWERSHELL_TOOL_NAME } from '@claude-code-best/builtin-tools/tools/PowerShellTool/toolName.js'
import { isEnvDefinedFalsy, isEnvTruthy } from '../envUtils.js'
import { isEnvDefinedFalsy } from '../envUtils.js'
import { getPlatform } from '../platform.js'

export const SHELL_TOOL_NAMES: string[] = [BASH_TOOL_NAME, POWERSHELL_TOOL_NAME]

/**
* Runtime gate for PowerShellTool. Windows-only (the permission engine uses
* Win32-specific path normalizations). Ant defaults on (opt-out via env=0);
* external defaults off (opt-in via env=1).
* Win32-specific path normalizations).
*
* Default: ON on Windows for all builds (opt-out via
* CLAUDE_CODE_USE_POWERSHELL_TOOL=0 / false / off). Non-Windows always off.
*
* Used by tools.ts (tool-list visibility), processBashCommand (! routing),
* and promptShellExecution (skill frontmatter routing) so the gate is
* consistent across all paths that invoke PowerShellTool.call().
*/
export function isPowerShellToolEnabled(): boolean {
if (getPlatform() !== 'windows') return false
return process.env.USER_TYPE === 'ant'
? !isEnvDefinedFalsy(process.env.CLAUDE_CODE_USE_POWERSHELL_TOOL)
: isEnvTruthy(process.env.CLAUDE_CODE_USE_POWERSHELL_TOOL)
return !isEnvDefinedFalsy(process.env.CLAUDE_CODE_USE_POWERSHELL_TOOL)
}