From c9007ff2ed40a8eedc8fe1b25e7f03f2bea2a272 Mon Sep 17 00:00:00 2001 From: davidrimshnick Date: Sat, 21 Feb 2026 11:33:35 -0500 Subject: [PATCH] fix(happy-cli): pass positional prompt arguments correctly to Claude CLI Two fixes: 1. Remove incorrect arg-pairing heuristic in index.ts that consumed the next argument after any unknown flag if it didn't start with "-". This broke positional prompts like: happy "/review URL" 2. Move claudeArgs after --settings in claudeLocal.ts so positional prompts end up at the end of the argv array where Claude CLI expects them. Fixes #663 Co-Authored-By: Claude Opus 4.6 --- .../happy-cli/src/claude/claudeLocal.test.ts | 23 +++++++++++++++++++ packages/happy-cli/src/claude/claudeLocal.ts | 11 +++++---- packages/happy-cli/src/index.ts | 9 ++++---- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/packages/happy-cli/src/claude/claudeLocal.test.ts b/packages/happy-cli/src/claude/claudeLocal.test.ts index bf9aa4398..cce1fbf31 100644 --- a/packages/happy-cli/src/claude/claudeLocal.test.ts +++ b/packages/happy-cli/src/claude/claudeLocal.test.ts @@ -283,6 +283,29 @@ describe('claudeLocal --continue handling', () => { expect(mockSandboxCleanup).toHaveBeenCalledTimes(1); }); + it('should place claudeArgs after --settings so positional prompts are last (regression #663)', async () => { + mockClaudeFindLastSession.mockReturnValue(null); + + await claudeLocal({ + abort: new AbortController().signal, + sessionId: null, + path: '/tmp', + onSessionFound, + claudeArgs: ['/review https://example.com/pr/123'], + hookSettingsPath: '/tmp/hook-settings.json', + }); + + const spawnArgs: string[] = mockSpawn.mock.calls[0][1]; + + const settingsIdx = spawnArgs.indexOf('--settings'); + const promptIdx = spawnArgs.indexOf('/review https://example.com/pr/123'); + + expect(settingsIdx).toBeGreaterThan(-1); + expect(promptIdx).toBeGreaterThan(-1); + // Positional prompt must come AFTER --settings and its value + expect(promptIdx).toBeGreaterThan(settingsIdx + 1); + }); + it('should continue without sandbox when initialization fails', async () => { mockInitializeSandbox.mockRejectedValue(new Error('sandbox failed')); diff --git a/packages/happy-cli/src/claude/claudeLocal.ts b/packages/happy-cli/src/claude/claudeLocal.ts index 45d370989..b8064d073 100644 --- a/packages/happy-cli/src/claude/claudeLocal.ts +++ b/packages/happy-cli/src/claude/claudeLocal.ts @@ -219,17 +219,18 @@ export async function claudeLocal(opts: { args.push('--allowedTools', opts.allowedTools.join(',')); } - // Add custom Claude arguments - if (opts.claudeArgs) { - args.push(...opts.claudeArgs) - } - // Add hook settings for session tracking (when available) if (opts.hookSettingsPath) { args.push('--settings', opts.hookSettingsPath); logger.debug(`[ClaudeLocal] Using hook settings: ${opts.hookSettingsPath}`); } + // Add custom Claude arguments last so positional prompts + // (e.g. happy "/review URL") end up at the end of the argv + if (opts.claudeArgs) { + args.push(...opts.claudeArgs) + } + if (!claudeCliPath || !existsSync(claudeCliPath)) { throw new Error('Claude local launcher not found. Please ensure HAPPY_PROJECT_ROOT is set correctly for development.'); } diff --git a/packages/happy-cli/src/index.ts b/packages/happy-cli/src/index.ts index ca3c03192..b5c4b7448 100644 --- a/packages/happy-cli/src/index.ts +++ b/packages/happy-cli/src/index.ts @@ -603,12 +603,11 @@ ${chalk.bold('To clean up runaway processes:')} Use ${chalk.cyan('happy doctor c console.warn(chalk.yellow(` To configure Claude, edit ~/.claude/settings.json instead.`)) // Don't pass through to claudeArgs } else { - // Pass unknown arguments through to claude + // Pass unknown arguments through to claude as-is. + // Don't try to pair with the next arg — the shell already + // handles quoting, and the heuristic breaks positional + // arguments like: happy "/review URL" unknownArgs.push(arg) - // Check if this arg expects a value (simplified check for common patterns) - if (i + 1 < args.length && !args[i + 1].startsWith('-')) { - unknownArgs.push(args[++i]) - } } }