Skip to content

No fallback to bare mode for summary generation - #1026

Merged
acreeger merged 3 commits into
mainfrom
fix/issue-1022__fallback-bare-mode-summary
Jul 9, 2026
Merged

No fallback to bare mode for summary generation#1026
acreeger merged 3 commits into
mainfrom
fix/issue-1022__fallback-bare-mode-summary

Conversation

@acreeger

@acreeger acreeger commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1022

No fallback to bare mode for summary generation

Issue details

My organization recently switched over to Enterprise Accounts and And now the session summary always fails because of the bare flag isn't compatible. In other steps, there is a fallback, but not for the session summary generation.

🗂️  Generating session summary...
🤖 ...⚠️  Failed to generate session summary: Claude CLI error: Command failed with exit code 1: claude --bare --settings 
---
Command:

claude --bare --settings [REDACTED] -p --output-format stream-json --verbose --model sonnet --add-dir [REDACTED] --no-session-persistence --resume [REDACTED]

Relevant output:

{"type":"assistant","content":[{"type":"text","text":"Invalid API key · Fix external API key"}],"error":"authentication_failed"}

{"type":"result","is_error":true,"api_error_status":401,"result":"Invalid API key · Fix external API key","terminal_reason":"completed"}

Environment:

Claude Code version: 2.1.159

Model: claude-sonnet-4-6

This PR was created automatically by iloom.

@acreeger

acreeger commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Analysis: No Fallback to Bare Mode for Session Summary Generation

Root Cause

The bug is a gap in the bare-mode auth-error fallback inside launchClaude() (src/utils/claude.ts). The fallback exists but is bypassed when a session-in-use retry happens first.

How the fallback mechanism works

launchClaude() has a two-attempt retry loop (lines 406-458) that auto-applies --bare mode for headless utility operations (headless: true && noSessionPersistence: true):

  • Attempt 1: Runs with --bare --settings <oauth-config>
  • If auth error detected → Attempt 2: Retries without --bare (uses normal OAuth/keychain auth)

The auth error regex at line 425:

const isAuthError = /not logged in|unauthorized|authentication|invalid api key|Could not resolve credentials/i.test(rawErrorMessage)

This correctly matches the user's error ("authentication_failed" and "Invalid API key").

Why session summary generation bypasses the fallback

The session summary flow (SessionSummaryService.launchSessionSummary(), line 87-114) passes a sessionId to launchClaude. The sequence:

  1. launchClaude auto-applies bare mode (line 170: bare === undefined && headless && noSessionPersistence)
  2. Attempt 1 uses --bare --session-id <uuid> → fails with "Session ID already in use" error
  3. Inner retry (lines 432-454) catches the session-in-use error and retries with --bare --resume <uuid> instead
  4. The --resume retry fails with auth error (authentication_failed)
  5. Bug: The inner retry's catch block (line 449-452) throws immediately — it never returns control to the outer retry loop where the auth error check lives (line 424)
// Line 446-453: Inner retry throws directly, bypassing outer auth-error fallback
try {
    return await runHeadlessSubprocess(resumeArgs, env)
} catch (retryError) {
    // ← Auth error caught here, thrown immediately
    throw new Error(`Claude CLI error: ${redactSettings(retryErrorMessage)}`)
}

Secondary issue: Epic report generation

generateAndPostEpicReport() (line 278) calls launchClaude with headless: true but without noSessionPersistence: true. This means it never triggers bare mode auto-application (line 170 requires both flags), so it works by accident — but it also doesn't benefit from the bare mode performance optimization.

Affected Files

File Lines Issue
src/utils/claude.ts 432-454 Session-in-use inner retry bypasses auth-error fallback loop
src/lib/SessionSummaryService.ts 87-114 launchSessionSummary passes sessionId which triggers session-in-use path
src/lib/SessionSummaryService.ts 278-281 generateAndPostEpicReport missing noSessionPersistence (minor)

How the fix should work

The session-in-use inner retry (lines 432-454) needs to check for auth errors and continue the outer loop (to retry without --bare) instead of throwing immediately. This mirrors the existing auth error check at lines 424-429.

Alternatively, the inner retry could be restructured so auth errors from the --resume path also trigger the bare-mode fallback.

Questions & Assumptions

Question Assumed Answer
Does the user have ANTHROPIC_API_KEY set? No — they use Enterprise OAuth, which is why --bare fails (it requires an API key or valid OAuth token)
Is the session-in-use error expected for summaries? Yes — the session is created by il spin and still exists when il finish generates the summary
Should generateAndPostEpicReport also use bare mode? Yes, for consistency — adding noSessionPersistence: true would enable the optimization

@acreeger

acreeger commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Implementation Plan

Problem Summary

When launchClaude() is invoked with a sessionId (as SessionSummaryService.launchSessionSummary does), the "Session ID already in use" inner retry at claude.ts:432-454 catches errors from the --resume retry and throws immediately — bypassing the outer bare-mode auth-error fallback at claude.ts:424-429. This means session summary generation never falls back to non-bare mode on Enterprise accounts where --bare fails with an auth error.

Additionally, SessionSummaryService.generateAndPostEpicReport (line 278) calls launchClaude directly without noSessionPersistence: true, so bare mode is never auto-applied for epic reports — they won't benefit from the performance optimization and could encounter stale session issues.

Root Cause

The headless retry loop structure (lines 405-458) has the auth-error check at lines 424-429, but the session-in-use handler at lines 432-454 is nested within the same catch block. When a session-in-use error occurs:

  1. The first runHeadlessSubprocess call fails with "Session ID already in use"
  2. The code enters the session-in-use handler and retries with --resume
  3. If the --resume retry fails with an auth error (e.g., Invalid API key), it throws immediately at line 452
  4. This thrown error escapes the for loop entirely — the continue at line 428 is never reached

Phase 1: Fix the session-in-use inner retry to check for auth errors

File: src/utils/claude.ts

Change 1: Lines 446-453 — In the session-in-use --resume inner retry catch block, check for auth errors and continue the outer loop instead of throwing.

Current code:

try {
    return await runHeadlessSubprocess(resumeArgs, env)
} catch (retryError) {
    const retryExecaError = retryError as { stderr?: string; message?: string }
    // eslint-disable-next-line ...
    const retryErrorMessage = retryExecaError.stderr || retryExecaError.message || 'Unknown Claude CLI error'
    throw new Error(`Claude CLI error: ${redactSettings(retryErrorMessage)}`)
}

New code:

try {
    return await runHeadlessSubprocess(resumeArgs, env)
} catch (retryError) {
    const retryExecaError = retryError as { stderr?: string; message?: string }
    // eslint-disable-next-line ...
    const retryErrorMessage = retryExecaError.stderr || retryExecaError.message || 'Unknown Claude CLI error'

    // Check if the --resume retry failed with an auth error that bare-mode fallback can handle
    if (attempt === 1 && bareModeAutoApplied) {
        const isAuthError = /not logged in|unauthorized|authentication|invalid api key|Could not resolve credentials/i.test(retryErrorMessage)
        if (isAuthError) {
            logger.warn('Bare mode failed during --resume retry (likely expired OAuth token), retrying without --bare')
            continue // Retry the outer loop without bare mode
        }
    }

    throw new Error(`Claude CLI error: ${redactSettings(retryErrorMessage)}`)
}

Phase 1 Must-Haves

  • When launchClaude is called with sessionId, headless: true, noSessionPersistence: true, and the first attempt hits a session-in-use error followed by an auth error on the --resume retry, it must continue to attempt 2 (without --bare) instead of throwing
  • When the --resume retry fails with a non-auth error, it must still throw as before
  • When bare was explicitly set (not auto-applied), it must NOT retry even if the --resume retry fails with auth error
  • TypeScript compiles without errors (pnpm compile)

Phase 2: Add noSessionPersistence to epic report launchClaude call

File: src/lib/SessionSummaryService.ts

Change 1: Lines 278-281 — Add noSessionPersistence: true to the epic report's launchClaude call so bare mode is auto-applied consistently.

Current code:

const reportResult = await launchClaude(prompt, {
    headless: true,
    model: summaryModel,
})

New code:

const reportResult = await launchClaude(prompt, {
    headless: true,
    model: summaryModel,
    noSessionPersistence: true,
})

Phase 2 Must-Haves

  • generateAndPostEpicReport passes noSessionPersistence: true to launchClaude
  • TypeScript compiles without errors (pnpm compile)

Phase 3: Add tests

File: src/utils/claude.test.ts

Add tests under the existing bare mode auth failure retry describe block:

Test 1: should retry without --bare when --resume retry fails with auth error (session-in-use + auth fallback)

  • Set up OAuth token for auto-applied bare mode
  • First execa call: reject with "Session ID xxx is already in use"
  • Second execa call (the --resume retry): reject with "Invalid API Key"
  • Third execa call (retry without bare): resolve with success output
  • Assert result is the success output
  • Assert first call had --bare, second had --resume + --bare, third had neither --bare nor --session-id

Test 2: should NOT retry without --bare when --resume retry fails with non-auth error

  • Set up OAuth token for auto-applied bare mode
  • First execa call: reject with "Session ID xxx is already in use"
  • Second execa call (the --resume retry): reject with "Command timed out"
  • Assert launchClaude rejects with "Claude CLI error"
  • Assert only 2 calls to execa (no third retry)

File: src/lib/SessionSummaryService.test.ts

Add test to verify noSessionPersistence is passed in epic reports:

Test 3: should pass noSessionPersistence when generating epic report

  • Verify that launchClaude is called with noSessionPersistence: true in generateAndPostEpicReport

Phase 3 Must-Haves

  • All three new tests pass (pnpm test)
  • Existing tests in claude.test.ts and SessionSummaryService.test.ts still pass
  • No lint errors (pnpm lint)

Execution Plan

Phase 1: Fix session-in-use auth fallback (1 step)
  Run Step 1: Modify src/utils/claude.ts — add auth-error check inside session-in-use --resume catch block

Phase 2: Fix epic report noSessionPersistence (1 step)
  Run Step 2: Modify src/lib/SessionSummaryService.ts — add noSessionPersistence: true to epic report launchClaude call

Phase 3: Add tests (1 step)
  Run Step 3: Add tests to src/utils/claude.test.ts and src/lib/SessionSummaryService.test.ts

Plan generated by iloom workflow

@acreeger

acreeger commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Implementation Complete

Summary

Added bare-mode auth-error fallback to the session-in-use --resume retry path in launchClaude(), fixing the case where Enterprise Account users with OAuth tokens hit authentication failures during session summary generation. Also added missing noSessionPersistence option to epic report generation.

Changes Made

  • src/utils/claude.ts: Added auth-error check inside session-in-use --resume catch block — when bareModeAutoApplied and the retry error is an auth error, continue to outer loop to retry without --bare
  • src/lib/SessionSummaryService.ts: Added noSessionPersistence: true to generateAndPostEpicReport launchClaude call
  • src/utils/claude.test.ts: Added 2 tests covering auth-error fallback and non-auth-error throw scenarios
  • src/lib/SessionSummaryService.test.ts: Updated existing test to expect noSessionPersistence: true

Validation Results

  • ✅ Tests: 5240 passed (2 new) / 1 skipped
  • ✅ Typecheck: Passed
  • ✅ Lint: Passed
  • ✅ Build: Passed
  • ✅ Phase Verifier: GO on all 5 must-haves
  • ✅ Code Review: No issues found

Detailed Changes by File (click to expand)

src/utils/claude.ts

Changes: Auth-error fallback in session-in-use retry path

  • Added check at lines 453-458: when attempt === 1 && bareModeAutoApplied and error matches auth patterns (not logged in, unauthorized, authentication, invalid api key, Could not resolve credentials), continue to outer loop instead of throwing
  • Mirrors existing auth-error fallback pattern at lines 424-429

src/lib/SessionSummaryService.ts

Changes: Epic report missing option

  • Added noSessionPersistence: true to launchClaude options at line 281

src/utils/claude.test.ts

Changes: New test cases

  • Test: session-in-use + auth error triggers bare fallback (3-call sequence verification)
  • Test: session-in-use + non-auth error still throws normally (2-call sequence)

src/lib/SessionSummaryService.test.ts

Changes: Updated existing test expectation

  • Added noSessionPersistence: true to expected launchClaude call args

@acreeger
acreeger force-pushed the fix/issue-1022__fallback-bare-mode-summary branch from f84b4a5 to 5d02238 Compare July 9, 2026 04:21
@acreeger
acreeger marked this pull request as ready for review July 9, 2026 04:49
@acreeger
acreeger merged commit e225ed2 into main Jul 9, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this to Done in iloom-cli Jul 9, 2026
@acreeger

acreeger commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

iloom Session Summary

Key Themes:

  • The bare-mode auth fallback in launchClaude had two separate catch blocks for auth errors, and only one of them implemented the fallback logic.
  • Fixed the session-in-use resume retry path in claude.ts to apply the same fallback behavior as the direct auth-error path.
  • Added noSessionPersistence to the epic report's launchClaude call and added test coverage for the session-in-use fallback scenario.

Session Details (click to expand)

Key Insights

  • launchClaude has two distinct error-handling paths that both need to trigger the bare-mode summary fallback: the direct auth-error catch and the session-in-use resume retry catch. These paths were implemented independently, so a fix applied to one did not automatically cover the other — any future changes to auth-fallback behavior must be mirrored in both locations unless they're refactored into a shared helper.
  • The epic report flow calls launchClaude with its own parameters, so options like noSessionPersistence must be explicitly passed there too; it is not inherited from other call sites.

Decisions Made

  • Applied the same fallback logic to the session-in-use retry catch block that already existed for the direct auth-error catch, rather than introducing a new abstraction, to keep the fix minimal and consistent with the existing pattern.

Challenges Resolved

Lessons Learned

  • When an error-handling behavior is duplicated across multiple catch blocks in the same function, treat each one as independently testable — a fix verified against one path does not guarantee coverage of the other.

Generated with 🤖❤️ by iloom.ai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

No fallback to bare mode for summary generation

1 participant