Skip to content

fix(cli): prevent session flags from leaking through on retry #644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kmizzi wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(cli): prevent session flags from leaking through on retry #644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 42 additions & 0 deletions packages/happy-cli/src/claude/claudeLocal.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
60 changes: 32 additions & 28 deletions packages/happy-cli/src/claude/claudeLocal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,18 @@ 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;

// 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 <id> AND --continue.
//
// Supported patterns:
// --continue / -c : Resume last session in current directory
// --resume / -r : Resume last session (picker in Claude, but we handle)
Expand Down Expand Up @@ -98,35 +98,39 @@ export async function claudeLocal(opts: {
return { found: false };
};

// 1. Check for --session-id <uuid> (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 <id>, 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 <uuid> (explicit new session with specific ID)
const sessionIdFlag = extractFlag(['--session-id'], true);

// 2. Extract --resume <id> / -r <id> (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 <id> / -r <id> (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;
Expand Down Expand Up @@ -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`);
}
Expand Down
3 changes: 3 additions & 0 deletions packages/happy-cli/src/claude/claudeLocalLauncher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ export async function claudeLocalLauncher(session: Session): Promise<LauncherRes
exitReason = { type: 'exit', code: e.exitCode };
break;
}
// Always consume one-time flags on error too, to prevent
// --continue/--resume from leaking through on retry
session.consumeOneTimeFlags();
if (!exitReason) {
session.client.sendSessionEvent({ type: 'message', message: 'Process exited unexpectedly' });
continue;
Expand Down
46 changes: 27 additions & 19 deletions packages/happy-cli/src/claude/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,43 +146,51 @@ export class Session {

/**
* Consume one-time Claude flags from claudeArgs after Claude spawn
* Handles: --resume (with or without session ID), --continue
* Handles: --resume/-r (with or without session ID), --continue/-c, --session-id
*/
consumeOneTimeFlags = (): void => {
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);
}
Expand Down
Loading