diff --git a/.changeset/fix-dep0190-shell-spawn.md b/.changeset/fix-dep0190-shell-spawn.md new file mode 100644 index 000000000..8cb081f75 --- /dev/null +++ b/.changeset/fix-dep0190-shell-spawn.md @@ -0,0 +1,11 @@ +--- +"@bradygaster/squad-cli": patch +--- + +Fix Windows shell spawn issues and gh auth false-negative + +- **DEP0190 fix:** Added `escapeForCmd()`/`escapeArgs()` to properly quote args when `shell: true` is used on Windows. Changed `shell: true` → `shell: IS_WINDOWS`. +- **Capability migration:** Moved monitor-email, monitor-teams, retro, decision-hygiene from inline `buildAgentCommand`/`spawnWithTimeout` to shared `agent-spawn.ts` module. +- **gh auth fix:** Use `gh auth token` instead of `gh auth status` — the latter returns non-zero when any keyring entry is stale, even if the active account works fine. +- **Copilot flag fix:** Use `-p` (correct) instead of `--message` (non-existent) for copilot CLI prompt flag. +- **Default --yolo:** When `--execute` is active and no `copilotFlags`/`agentCmd` are set, default to `--yolo` so copilot doesn't hang waiting for permission prompts. \ No newline at end of file diff --git a/packages/squad-cli/src/cli/commands/preset.ts b/packages/squad-cli/src/cli/commands/preset.ts index a498ad2f6..c2282756c 100644 --- a/packages/squad-cli/src/cli/commands/preset.ts +++ b/packages/squad-cli/src/cli/commands/preset.ts @@ -115,9 +115,12 @@ async function presetInitRemote(): Promise { fatal('GitHub CLI (gh) is required for --remote. Install it: https://cli.github.com'); } - // Check gh auth + // Check gh auth — use `gh auth token` instead of `gh auth status` because + // the latter returns non-zero when any keyring entry is stale, even if the + // active account (e.g. via GH_TOKEN) works fine. try { - execSync('gh auth status', { stdio: 'pipe' }); + const tok = execSync('gh auth token', { encoding: 'utf-8', stdio: 'pipe' }).trim(); + if (!tok) throw new Error('empty token'); } catch { fatal('Not logged in to GitHub CLI. Run: gh auth login'); } diff --git a/packages/squad-cli/src/cli/commands/watch/agent-spawn.ts b/packages/squad-cli/src/cli/commands/watch/agent-spawn.ts index d46adb83b..1843e4f57 100644 --- a/packages/squad-cli/src/cli/commands/watch/agent-spawn.ts +++ b/packages/squad-cli/src/cli/commands/watch/agent-spawn.ts @@ -15,6 +15,36 @@ import type { WatchContext } from './types.js'; /** True when running on Windows — used to gate `shell: true`. */ export const IS_WINDOWS = process.platform === 'win32'; +/** + * Escape an argument for safe use with cmd.exe when `shell: true`. + * + * Node's `execFile` with `shell: true` on Windows concatenates args with + * spaces but does NOT quote them (Node DEP0190). This means multi-word + * prompts get split by cmd.exe and the child process receives garbage argv. + * + * This function wraps any arg containing spaces, quotes, or cmd.exe + * metacharacters in double quotes with internal double quotes escaped. + * + * On non-Windows (shell: false path), args are passed directly to execvp + * without shell interpretation, so no escaping is needed. + */ +export function escapeForCmd(arg: string): string { + // Characters that require quoting in cmd.exe + if (!/[\s"&|<>^%!()]/.test(arg)) return arg; + // Escape internal double quotes by doubling them (cmd.exe convention) + const escaped = arg.replace(/"/g, '""'); + return `"${escaped}"`; +} + +/** + * Escape an array of args for cmd.exe shell invocation. + * Only applies on Windows — returns args unchanged on other platforms. + */ +export function escapeArgs(args: string[]): string[] { + if (!IS_WINDOWS) return args; + return args.map(escapeForCmd); +} + /** * Cached result of copilot CLI detection. * `null` means we haven't checked yet. @@ -73,13 +103,13 @@ export function buildAgentCommand( if (context.agentCmd) { const parts = context.agentCmd.trim().split(/\s+/); const cmd = parts[0]!; - const args = [...parts.slice(1), '--message', prompt]; + const args = [...parts.slice(1), '-p', prompt]; return { cmd, args }; } // Default: detect available copilot CLI at runtime (cached) const { cmd, cmdPrefix } = resolveCopilotCmd(); - const args = [...cmdPrefix, '--message', prompt]; + const args = [...cmdPrefix, '-p', prompt]; if (context.copilotFlags) { args.push(...context.copilotFlags.trim().split(/\s+/)); } @@ -90,7 +120,8 @@ export function buildAgentCommand( * Spawn an agent command with a timeout. * * Uses `shell: true` on Windows so that `.cmd`/`.bat` wrappers and - * PATH resolution work correctly. + * PATH resolution work correctly. Args are escaped via `escapeArgs()` + * to prevent Node DEP0190 and cmd.exe metacharacter injection. */ export function spawnWithTimeout( cmd: string, @@ -98,8 +129,9 @@ export function spawnWithTimeout( cwd: string, timeoutMs: number, ): Promise { + const safeArgs = escapeArgs(args); return new Promise((resolve, reject) => { - execFile(cmd, args, { + execFile(cmd, safeArgs, { cwd, timeout: timeoutMs, maxBuffer: 50 * 1024 * 1024, @@ -130,10 +162,11 @@ export function spawnAgent( cwd: string, timeoutMs: number, ): Promise<{ success: boolean; error?: string }> { + const safeArgs = escapeArgs(args); return new Promise<{ success: boolean; error?: string }>((resolve) => { execFile( cmd, - args, + safeArgs, { cwd, timeout: timeoutMs, diff --git a/packages/squad-cli/src/cli/commands/watch/capabilities/decision-hygiene.ts b/packages/squad-cli/src/cli/commands/watch/capabilities/decision-hygiene.ts index b929c480d..ac98ccb9d 100644 --- a/packages/squad-cli/src/cli/commands/watch/capabilities/decision-hygiene.ts +++ b/packages/squad-cli/src/cli/commands/watch/capabilities/decision-hygiene.ts @@ -3,36 +3,12 @@ */ import path from 'node:path'; -import { execFile } from 'node:child_process'; import { FSStorageProvider } from '@bradygaster/squad-sdk'; import type { WatchCapability, WatchContext, PreflightResult, CapabilityResult } from '../types.js'; -import { withAdditionalMcpConfig } from '../../../core/copilot-invocation.js'; +import { buildAgentCommand, spawnWithTimeout } from '../agent-spawn.js'; const storage = new FSStorageProvider(); -function buildAgentCommand(prompt: string, context: WatchContext): { cmd: string; args: string[] } { - if (context.agentCmd) { - const parts = context.agentCmd.trim().split(/\s+/); - return { cmd: parts[0]!, args: [...parts.slice(1), '-p', prompt] }; - } - const args = ['-p', prompt]; - if (context.copilotFlags) args.push(...context.copilotFlags.trim().split(/\s+/)); - return { cmd: 'copilot', args: withAdditionalMcpConfig('copilot', args, context.teamRoot) }; -} - -function spawnWithTimeout(cmd: string, args: string[], cwd: string, timeoutMs: number): Promise { - return new Promise((resolve, reject) => { - execFile(cmd, args, { cwd, timeout: timeoutMs, maxBuffer: 50 * 1024 * 1024, shell: true }, (err) => { - if (err) { - const execErr = err as Error & { killed?: boolean }; - reject(new Error(execErr.killed ? `Timed out after ${Math.round(timeoutMs / 1000)}s` : execErr.message)); - } else { - resolve(); - } - }); - }); -} - export class DecisionHygieneCapability implements WatchCapability { readonly name = 'decision-hygiene'; readonly description = 'Auto-merge decision inbox when >5 files accumulate'; diff --git a/packages/squad-cli/src/cli/commands/watch/capabilities/monitor-email.ts b/packages/squad-cli/src/cli/commands/watch/capabilities/monitor-email.ts index 24e7eaf57..0452f76fb 100644 --- a/packages/squad-cli/src/cli/commands/watch/capabilities/monitor-email.ts +++ b/packages/squad-cli/src/cli/commands/watch/capabilities/monitor-email.ts @@ -4,30 +4,7 @@ import { execFile } from 'node:child_process'; import type { WatchCapability, WatchContext, PreflightResult, CapabilityResult } from '../types.js'; -import { withAdditionalMcpConfig } from '../../../core/copilot-invocation.js'; - -function buildAgentCommand(prompt: string, context: WatchContext): { cmd: string; args: string[] } { - if (context.agentCmd) { - const parts = context.agentCmd.trim().split(/\s+/); - return { cmd: parts[0]!, args: [...parts.slice(1), '-p', prompt] }; - } - const args = ['-p', prompt]; - if (context.copilotFlags) args.push(...context.copilotFlags.trim().split(/\s+/)); - return { cmd: 'copilot', args: withAdditionalMcpConfig('copilot', args, context.teamRoot) }; -} - -function spawnWithTimeout(cmd: string, args: string[], cwd: string, timeoutMs: number): Promise { - return new Promise((resolve, reject) => { - execFile(cmd, args, { cwd, timeout: timeoutMs, maxBuffer: 50 * 1024 * 1024, shell: true }, (err) => { - if (err) { - const execErr = err as Error & { killed?: boolean }; - reject(new Error(execErr.killed ? `Timed out after ${Math.round(timeoutMs / 1000)}s` : execErr.message)); - } else { - resolve(); - } - }); - }); -} +import { buildAgentCommand, spawnWithTimeout, IS_WINDOWS } from '../agent-spawn.js'; export class MonitorEmailCapability implements WatchCapability { readonly name = 'monitor-email'; @@ -40,7 +17,7 @@ export class MonitorEmailCapability implements WatchCapability { // If using custom agentCmd, skip copilot check if (context.agentCmd) return { ok: true }; return new Promise((resolve) => { - execFile('copilot', ['--version'], { shell: true, timeout: 5000 }, (err) => { + execFile('copilot', ['--version'], { shell: IS_WINDOWS, timeout: 5000 }, (err) => { if (err) { resolve({ ok: false, diff --git a/packages/squad-cli/src/cli/commands/watch/capabilities/monitor-teams.ts b/packages/squad-cli/src/cli/commands/watch/capabilities/monitor-teams.ts index 3989f06b1..89dc2da8d 100644 --- a/packages/squad-cli/src/cli/commands/watch/capabilities/monitor-teams.ts +++ b/packages/squad-cli/src/cli/commands/watch/capabilities/monitor-teams.ts @@ -4,31 +4,7 @@ import { execFile } from 'node:child_process'; import type { WatchCapability, WatchContext, PreflightResult, CapabilityResult } from '../types.js'; -import { withAdditionalMcpConfig } from '../../../core/copilot-invocation.js'; - -/** Build agent command from prompt, respecting --agent-cmd. */ -function buildAgentCommand(prompt: string, context: WatchContext): { cmd: string; args: string[] } { - if (context.agentCmd) { - const parts = context.agentCmd.trim().split(/\s+/); - return { cmd: parts[0]!, args: [...parts.slice(1), '-p', prompt] }; - } - const args = ['-p', prompt]; - if (context.copilotFlags) args.push(...context.copilotFlags.trim().split(/\s+/)); - return { cmd: 'copilot', args: withAdditionalMcpConfig('copilot', args, context.teamRoot) }; -} - -function spawnWithTimeout(cmd: string, args: string[], cwd: string, timeoutMs: number): Promise { - return new Promise((resolve, reject) => { - execFile(cmd, args, { cwd, timeout: timeoutMs, maxBuffer: 50 * 1024 * 1024, shell: true }, (err) => { - if (err) { - const execErr = err as Error & { killed?: boolean }; - reject(new Error(execErr.killed ? `Timed out after ${Math.round(timeoutMs / 1000)}s` : execErr.message)); - } else { - resolve(); - } - }); - }); -} +import { buildAgentCommand, spawnWithTimeout, IS_WINDOWS } from '../agent-spawn.js'; export class MonitorTeamsCapability implements WatchCapability { readonly name = 'monitor-teams'; @@ -41,7 +17,7 @@ export class MonitorTeamsCapability implements WatchCapability { // If using custom agentCmd, skip copilot check if (context.agentCmd) return { ok: true }; return new Promise((resolve) => { - execFile('copilot', ['--version'], { shell: true, timeout: 5000 }, (err) => { + execFile('copilot', ['--version'], { shell: IS_WINDOWS, timeout: 5000 }, (err) => { if (err) { resolve({ ok: false, diff --git a/packages/squad-cli/src/cli/commands/watch/capabilities/retro.ts b/packages/squad-cli/src/cli/commands/watch/capabilities/retro.ts index 160336fb3..72e04a463 100644 --- a/packages/squad-cli/src/cli/commands/watch/capabilities/retro.ts +++ b/packages/squad-cli/src/cli/commands/watch/capabilities/retro.ts @@ -3,36 +3,12 @@ */ import path from 'node:path'; -import { execFile } from 'node:child_process'; import { FSStorageProvider } from '@bradygaster/squad-sdk'; import type { WatchCapability, WatchContext, PreflightResult, CapabilityResult } from '../types.js'; -import { withAdditionalMcpConfig } from '../../../core/copilot-invocation.js'; +import { buildAgentCommand, spawnWithTimeout } from '../agent-spawn.js'; const storage = new FSStorageProvider(); -function buildAgentCommand(prompt: string, context: WatchContext): { cmd: string; args: string[] } { - if (context.agentCmd) { - const parts = context.agentCmd.trim().split(/\s+/); - return { cmd: parts[0]!, args: [...parts.slice(1), '-p', prompt] }; - } - const args = ['-p', prompt]; - if (context.copilotFlags) args.push(...context.copilotFlags.trim().split(/\s+/)); - return { cmd: 'copilot', args: withAdditionalMcpConfig('copilot', args, context.teamRoot) }; -} - -function spawnWithTimeout(cmd: string, args: string[], cwd: string, timeoutMs: number): Promise { - return new Promise((resolve, reject) => { - execFile(cmd, args, { cwd, timeout: timeoutMs, maxBuffer: 50 * 1024 * 1024, shell: true }, (err) => { - if (err) { - const execErr = err as Error & { killed?: boolean }; - reject(new Error(execErr.killed ? `Timed out after ${Math.round(timeoutMs / 1000)}s` : execErr.message)); - } else { - resolve(); - } - }); - }); -} - export class RetroCapability implements WatchCapability { readonly name = 'retro'; readonly description = 'Enforce retrospective checks (Fridays or when missed >7 days)'; diff --git a/packages/squad-cli/src/cli/commands/watch/config.ts b/packages/squad-cli/src/cli/commands/watch/config.ts index 2236c29ab..78e9f092a 100644 --- a/packages/squad-cli/src/cli/commands/watch/config.ts +++ b/packages/squad-cli/src/cli/commands/watch/config.ts @@ -105,6 +105,12 @@ export function loadWatchConfig( stateContext: cliOverrides.stateContext, }; + // Default to --yolo when execute mode is active and no flags were set. + // Copilot CLI hangs in non-interactive (-p) mode without permission flags. + if (merged.execute && !merged.copilotFlags && !merged.agentCmd) { + merged.copilotFlags = '--yolo'; + } + return merged; } diff --git a/packages/squad-cli/src/cli/commands/watch/index.ts b/packages/squad-cli/src/cli/commands/watch/index.ts index 954be8da1..d07026935 100644 --- a/packages/squad-cli/src/cli/commands/watch/index.ts +++ b/packages/squad-cli/src/cli/commands/watch/index.ts @@ -595,7 +595,9 @@ function legacyToConfig(options: WatchOptions): WatchConfig { execute: options.execute ?? false, maxConcurrent: options.maxConcurrent ?? 1, timeout: options.issueTimeoutMinutes ?? 30, - copilotFlags: options.copilotFlags, + // Default to --yolo when execute mode is active — copilot CLI hangs in + // non-interactive mode without permission flags. + copilotFlags: options.copilotFlags ?? (options.execute ? '--yolo' : undefined), agentCmd: options.agentCmd, capabilities, }; diff --git a/packages/squad-cli/src/cli/core/gh-cli.ts b/packages/squad-cli/src/cli/core/gh-cli.ts index 6dfc8c434..0bb948f20 100644 --- a/packages/squad-cli/src/cli/core/gh-cli.ts +++ b/packages/squad-cli/src/cli/core/gh-cli.ts @@ -59,12 +59,17 @@ export async function ghAvailable(): Promise { } /** - * Check if gh CLI is authenticated + * Check if gh CLI is authenticated. + * + * Uses `gh auth token` instead of `gh auth status` because the latter + * returns a non-zero exit code when ANY account in the keyring has an + * invalid token — even if the active account (e.g. via GH_TOKEN) is + * perfectly fine. `gh auth token` only checks the active account. */ export async function ghAuthenticated(): Promise { try { - await execFileAsync('gh', ['auth', 'status']); - return true; + const { stdout } = await execFileAsync('gh', ['auth', 'token']); + return stdout.trim().length > 0; } catch { return false; }