diff --git a/packages/happy-cli/src/claude/claudeLocal.test.ts b/packages/happy-cli/src/claude/claudeLocal.test.ts index bf9aa4398..7d7e5a393 100644 --- a/packages/happy-cli/src/claude/claudeLocal.test.ts +++ b/packages/happy-cli/src/claude/claudeLocal.test.ts @@ -248,6 +248,48 @@ describe('claudeLocal --continue handling', () => { expect(spawnArgs).toContain('-r'); }); + it('should not leak --continue when sessionId is already set (retry scenario)', async () => { + mockClaudeFindLastSession.mockReturnValue(null); + + // Simulate retry: sessionId is already set from a previous hook notification + await claudeLocal({ + abort: new AbortController().signal, + sessionId: 'existing-session-from-hook', + path: '/tmp', + onSessionFound, + claudeArgs: ['--continue'] + }); + + const spawnArgs = mockSpawn.mock.calls[0][1]; + + // --continue must be stripped to prevent conflict with --resume + expect(spawnArgs).not.toContain('--continue'); + + // Should resume the existing session + expect(spawnArgs).toContain('--resume'); + expect(spawnArgs).toContain('existing-session-from-hook'); + + // Should NOT have --session-id (which would conflict with --resume) + expect(spawnArgs).not.toContain('--session-id'); + }); + + it('should not leak -c when sessionId is already set (retry scenario)', async () => { + mockClaudeFindLastSession.mockReturnValue(null); + + await claudeLocal({ + abort: new AbortController().signal, + sessionId: 'existing-session-from-hook', + path: '/tmp', + onSessionFound, + claudeArgs: ['-c'] + }); + + const spawnArgs = mockSpawn.mock.calls[0][1]; + expect(spawnArgs).not.toContain('-c'); + expect(spawnArgs).toContain('--resume'); + expect(spawnArgs).toContain('existing-session-from-hook'); + }); + it('should initialize sandbox, wrap command, and cleanup on exit', async () => { await claudeLocal({ abort: new AbortController().signal, diff --git a/packages/happy-cli/src/claude/claudeLocal.ts b/packages/happy-cli/src/claude/claudeLocal.ts index 45d370989..a19665fc6 100644 --- a/packages/happy-cli/src/claude/claudeLocal.ts +++ b/packages/happy-cli/src/claude/claudeLocal.ts @@ -52,11 +52,6 @@ export async function claudeLocal(opts: { const projectDir = getProjectPath(opts.path); mkdirSync(projectDir, { recursive: true }); - // Check if claudeArgs contains --continue or --resume (user passed these flags) - const hasContinueFlag = opts.claudeArgs?.includes('--continue'); - const hasResumeFlag = opts.claudeArgs?.includes('--resume'); - const hasUserSessionControl = hasContinueFlag || hasResumeFlag; - // Determine if we have an existing session to resume // Session ID will always be provided by hook (SessionStart) when Claude starts let startFrom = opts.sessionId; @@ -64,6 +59,11 @@ export async function claudeLocal(opts: { // Handle session-related flags from claudeArgs to ensure transparent behavior // We intercept these flags to use happy-cli's session storage rather than Claude's default // + // IMPORTANT: Always extract ALL session flags from claudeArgs unconditionally. + // If we only extract when startFrom is null, flags leak through on retry + // (when session.sessionId is already set from a previous hook notification), + // causing Claude to receive conflicting flags like --resume AND --continue. + // // Supported patterns: // --continue / -c : Resume last session in current directory // --resume / -r : Resume last session (picker in Claude, but we handle) @@ -98,35 +98,39 @@ export async function claudeLocal(opts: { return { found: false }; }; - // 1. Check for --session-id (explicit new session with specific ID) + // Always extract session-related flags from claudeArgs to prevent them from leaking + // through alongside the session args we construct ourselves. This is critical on retry + // when session.sessionId is already set - without unconditional extraction, flags like + // --continue would pass through alongside our --resume , causing Claude to error + // with "--session-id can only be used with --continue or --resume if --fork-session + // is also specified". + // + // Note: Bare --resume / -r (without value) is NOT extracted - it passes through to + // Claude for the interactive session picker. + + // 1. Extract --session-id (explicit new session with specific ID) const sessionIdFlag = extractFlag(['--session-id'], true); + + // 2. Extract --resume / -r (only with value - bare --resume passes through for picker) + const resumeFlag = extractFlag(['--resume', '-r'], true); + + // 3. Extract --continue / -c (always extract to prevent conflict with --resume we may add) + const continueFlag = extractFlag(['--continue', '-c'], false); + + // Track if user passed session control flags (for logging) + const hasUserSessionControl = continueFlag.found || resumeFlag.found; + + // Now use extracted flags to determine startFrom (only if not already set from opts.sessionId) if (sessionIdFlag.found && sessionIdFlag.value) { startFrom = null; // Force new session mode, will use this ID below logger.debug(`[ClaudeLocal] Using explicit --session-id: ${sessionIdFlag.value}`); } - // 2. Check for --resume / -r (resume specific session) - if (!startFrom && !sessionIdFlag.value) { - const resumeFlag = extractFlag(['--resume', '-r'], true); - if (resumeFlag.found) { - if (resumeFlag.value) { - startFrom = resumeFlag.value; - logger.debug(`[ClaudeLocal] Using provided session ID from --resume: ${startFrom}`); - } else { - // --resume without value: find last session - const lastSession = claudeFindLastSession(opts.path); - if (lastSession) { - startFrom = lastSession; - logger.debug(`[ClaudeLocal] --resume: Found last session: ${lastSession}`); - } - } - } - } - - // 3. Check for --continue / -c (resume last session) if (!startFrom && !sessionIdFlag.value) { - const continueFlag = extractFlag(['--continue', '-c'], false); - if (continueFlag.found) { + if (resumeFlag.found && resumeFlag.value) { + startFrom = resumeFlag.value; + logger.debug(`[ClaudeLocal] Using provided session ID from --resume: ${startFrom}`); + } else if (continueFlag.found) { const lastSession = claudeFindLastSession(opts.path); if (lastSession) { startFrom = lastSession; @@ -163,7 +167,7 @@ export async function claudeLocal(opts: { if (startFrom) { logger.debug(`[ClaudeLocal] Will resume existing session: ${startFrom}`); } else if (hasUserSessionControl) { - logger.debug(`[ClaudeLocal] User passed ${hasContinueFlag ? '--continue' : '--resume'} flag, session ID will be determined by hook`); + logger.debug(`[ClaudeLocal] User passed ${continueFlag.found ? '--continue' : '--resume'} flag, session ID will be determined by hook`); } else { logger.debug(`[ClaudeLocal] Fresh start, session ID will be provided by hook`); } diff --git a/packages/happy-cli/src/claude/claudeLocalLauncher.ts b/packages/happy-cli/src/claude/claudeLocalLauncher.ts index 9a5638e23..a761080e1 100644 --- a/packages/happy-cli/src/claude/claudeLocalLauncher.ts +++ b/packages/happy-cli/src/claude/claudeLocalLauncher.ts @@ -136,6 +136,9 @@ export async function claudeLocalLauncher(session: Session): Promise { if (!this.claudeArgs) return; - + const filteredArgs: string[] = []; for (let i = 0; i < this.claudeArgs.length; i++) { const arg = this.claudeArgs[i]; - - if (arg === '--continue') { - logger.debug('[Session] Consumed --continue flag'); + + if (arg === '--continue' || arg === '-c') { + logger.debug(`[Session] Consumed ${arg} flag`); continue; } - - if (arg === '--resume') { - // Check if next arg looks like a UUID (contains dashes and alphanumeric) + + if (arg === '--resume' || arg === '-r') { + // Check if next arg looks like a value (not another flag) if (i + 1 < this.claudeArgs.length) { const nextArg = this.claudeArgs[i + 1]; - // Simple UUID pattern check - contains dashes and is not another flag - if (!nextArg.startsWith('-') && nextArg.includes('-')) { - // Skip both --resume and the UUID - i++; // Skip the UUID - logger.debug(`[Session] Consumed --resume flag with session ID: ${nextArg}`); + if (!nextArg.startsWith('-')) { + // Skip both flag and value + i++; + logger.debug(`[Session] Consumed ${arg} flag with session ID: ${nextArg}`); } else { - // Just --resume without UUID - logger.debug('[Session] Consumed --resume flag (no session ID)'); + logger.debug(`[Session] Consumed ${arg} flag (no session ID)`); } } else { - // --resume at the end of args - logger.debug('[Session] Consumed --resume flag (no session ID)'); + logger.debug(`[Session] Consumed ${arg} flag (no session ID)`); + } + continue; + } + + if (arg === '--session-id') { + // Skip --session-id and its value + if (i + 1 < this.claudeArgs.length && !this.claudeArgs[i + 1].startsWith('-')) { + i++; + logger.debug(`[Session] Consumed --session-id flag with value`); + } else { + logger.debug(`[Session] Consumed --session-id flag (no value)`); } continue; } - + filteredArgs.push(arg); } - + this.claudeArgs = filteredArgs.length > 0 ? filteredArgs : undefined; logger.debug(`[Session] Consumed one-time flags, remaining args:`, this.claudeArgs); }