fix(sessions): restore resume-or-fresh fallback chain for AI tool launches

[agent-era-ai]

Summary

• Restore the <resume> || <fresh> shell-level fallback that commit ca72a85 removed. When the user attaches to an existing worktree, devteam launches the selected AI tool with its resume flag (claude --continue, codex resume --last, gemini --resume latest) — but on tool-switch (e.g. picking claude on a worktree that previously ran codex) the resume form exits with no prior session, leaving the tmux pane empty. The chain falls through to a fresh launch in that case.
• Unify the per-tool launch helpers. Previously there was a separate launchClaudeSessionWithFallback and launchAISessionWithFallback; the only Claude-specific concern (the -n displayName tmux pane title flag) is now an optional parameter on a single helper. The helper is one straight-line build instead of per-tool branching.
• The freshWorktree=true callers (createFeature, recreateImplementWorktree) are unchanged: they still launch the fresh form directly with no chain, since the worktree was just created and there is nothing to resume.

Test plan

• npm run build clean
• npm test — 781/781 pass across 77 suites
• npx tsc -p tsconfig.test.json clean
• Updated existing WorktreeCoreAutoResume assertions to match the chained command shape
• Added regression test: switching to claude on a worktree with lastTool=codex produces the chained launch (the originally-reported failure case)
• Manual: open an existing worktree with claude, observe claude --continue || claude runs and lands at a working prompt whether the resume succeeds or not

🤖 Generated with Claude Code

[claude]

Code Review — PR #236: fix(sessions): restore resume-or-fresh fallback chain

Overview

This PR restores the <resume> || <fresh> shell-level fallback that commit ca72a85 removed and simultaneously consolidates two nearly-identical helpers (launchClaudeSessionWithFallback / launchAISessionWithFallback) into a single unified one. The motivation is sound: when a user switches AI tools on an existing worktree (e.g. codex → claude), the resume form exits with no prior session and the tmux pane was being left empty.

---

Code quality ✅

• The unified helper is cleaner than the previous two-function split. The only Claude-specific concern (-n displayName) is now a properly-typed optional trailing parameter — no conditional branching needed at the call site.
• Using Exclude<AITool, 'none'> in the function signature is the right guard; the AI_TOOLS lookup is now safe without a runtime check.
• shellQuote is applied consistently to all user-controlled string values, preventing tmux command injection.
• The command shape is correct for all three tools and matches the documented forms in the implementation notes.

Potential issues

Minor — error output visibility before fallback runs: When claude --continue (or codex resume --last) exits with a non-zero code, the tool may print an error message to the terminal before the fresh form starts. The notes say the fallback is "intentionally silent", but that only holds if the tool does not write anything to stdout/stderr before exiting. This is pre-existing behaviour (same as the original pre-ca72a85 chain), so not a regression — just worth noting if users report stray error lines in the pane.

Unused launchTool in AIToolService still lacks a fallback: The implementation notes call this out under "Notes for cleanup" — AIToolService.launchTool still launches claude --continue with no chain. Since it has no callers in src/ today this is not a live bug, but it is a trap for the next person who wires it up. A // TODO: apply resume-or-fresh chain here if wired to a call site comment would help prevent a silent re-introduction of the bug.

Performance ✅

No change to refresh rates or hot paths. The command string building is trivial.

Security ✅

All externally-derived strings (displayName, initialPrompt) pass through shellQuote before being embedded in the tmux command. No new surface area introduced.

Test coverage ✅

• Existing assertions are correctly updated to the new chained shape.
• The new 'switching to claude on a worktree previously used with codex' test directly covers the reported failure case — a good regression test.
• The freshWorktree=true path (unaffected by this PR) already has coverage.

---

Summary

This is a clean, well-scoped fix. The refactor to a single helper is a genuine improvement. The only actionable suggestion is adding a TODO comment to the unused AIToolService.launchTool to prevent a future caller from silently reintroducing the no-fallback bug. Everything else looks good to merge.

🤖 Generated with Claude Code

[claude]

Code Review — PR #236: restore resume-or-fresh fallback chain

Overall: ✅ Looks good — clean fix, well-scoped, solid tests.

---

What this does

Restores the <resume> || <fresh> shell fallback that commit ca72a85 dropped, and eliminates the previous launchClaudeSessionWithFallback / launchAISessionWithFallback split. The merge is well-motivated: the only Claude-specific concern (-n displayName) is now an optional trailing parameter, and the unified helper is simpler than what it replaced.

---

Code quality

• Unification is the right call. The prior split of launchClaudeSessionWithFallback vs launchAISessionWithFallback was papering over a trivial difference (one optional flag). A single method with an optional displayName parameter is clearer and easier to keep correct.
• Type narrowing improvement. Changing the signature to Exclude<AITool, 'none'> is a good addition — it pushes the guard up to the call site (selectedTool !== 'none' check at line ~399) and removes the need for the helper to handle the impossible 'none' case.
• freshCmd / resumeCmd locals improve readability over the previous deeply-nested if/else tree.

---

Potential issues

1. displayName is computed and passed unconditionally from the call site, but only applied when tool === 'claude'. This is correct and harmless, but if a future tool gains a similar pane-title flag, adding it to the nameFlag logic inside the helper may surprise reviewers who see displayName always passed in. Consider either passing it only for claude at the call site or adding a brief note in the helper — minor style point.

2. AIToolService.launchTool warning comment — the multi-line JSDoc added to this method is helpful context, but it's longer than the conventions in AGENTS.md recommend. If the method is truly dead code, a // Unused — no callers in src/; see launchAISessionWithFallback in WorktreeCore.ts one-liner would be more idiomatic. The current comment won't cause any bugs, though.

3. gemini prompt arg placement in the chain. For gemini with an initialPrompt, the resume form becomes gemini --resume latest -i 'prompt' and the fresh form becomes gemini -i 'prompt'. The original code placed -i after flagStr (e.g., gemini --resume latest${flagStr} -i ${promptQ}). In the new code promptArg (which embeds -i) is appended after flagStr in both freshCmd and resumeCmd. This is consistent and matches the old shape — just worth a quick sanity-check if Gemini's CLI is sensitive to argument ordering.

---

Tests

• Updating the existing WorktreeCoreAutoResume assertions to expect the ||-chained shape is exactly right.
• The new 'switching to claude on a worktree previously used with codex' regression test directly covers the originally-reported failure case.
• 781/781 pass and clean typecheck noted in the PR description.

---

Summary

The fix is narrow, well-understood, and addresses the root cause. The two minor style points (unconditional displayName pass-through, verbose dead-code comment) are cosmetic. No blockers.

[claude]

Code Review — PR #236: fix(sessions): restore resume-or-fresh fallback chain for AI tool launches

Overview

Restores the shell-level <resume> || <fresh> fallback that was inadvertently dropped in ca72a85, and uses the opportunity to consolidate the two separate launch helpers (launchClaudeSessionWithFallback + launchAISessionWithFallback) into a single unified helper. The fix is narrow and well-scoped; the unification is a clean bonus.

---

Code Quality

Positive:

• The unified helper is significantly cleaner than its predecessors. The old split existed solely because of Claude's -n displayName flag; making that an optional trailing parameter is the right call.
• The three-line command build (freshCmd / resumeCmd / cmd) is easy to read and audit. The freshWorktree ? freshCmd : \${resumeCmd} || ${freshCmd}`` idiom is self-documenting.
• Using Exclude<AITool, 'none'> in the signature is a good type-level guard; it shifts the "none means skip" responsibility to the caller rather than silently no-oping.
• Shell quoting is handled correctly: flagStr is constructed upstream with per-flag shellQuote calls, and displayName / initialPrompt are quoted at the point of interpolation.
• The gemini promptArg ordering (flagStr before -i) matches the old code exactly — the refactor preserved all three tools' argument shapes correctly.

---

Specific Issues

Minor — comment on launchAISessionWithFallback is longer than the new guideline:

AGENTS.md now says "prefer a single line" for warranted comments. The block comment at lines 595–601 is 7 lines covering two distinct concerns. Both concerns are genuinely non-obvious (the agent-switch scenario, and the claude-only -n flag), so some comment is warranted — but condensing to two single-line comments would put it in line with the new convention.

// Non-fresh path uses `<resume> || <fresh>` so an agent switch (e.g. codex → claude) doesn't leave the pane empty.
// displayName is claude-only (-n pane title); other tools ignore it.
private launchAISessionWithFallback(...)

Minor — AIToolService.launchTool comment will rot:

// Unused in src/; the live launch path is WorktreeCore.launchAISessionWithFallback. If wired up again, mirror its `<resume> || <fresh>` chain.

AGENTS.md says "don't reference current task / fix / caller" and the specific method name reference (WorktreeCore.launchAISessionWithFallback) will become stale if the method is renamed or moved. The maintenance warning is useful; the cross-reference is the part that rots. A trimmed version:

// Unused; if revived, build the command as `<resume-form> || <fresh-form>` (see WorktreeCore for the canonical pattern).

Observation — test comments explain the WHAT:

The two inline comments in the new regression test:

// Simulate a worktree that was previously running codex.
// Now the user explicitly picks claude — no fresh-worktree flag, since the worktree directory already exists.

They mostly restate the setup lines that follow them. The test name already says "switching to claude on a worktree previously used with codex"; the comments could be dropped without losing anything.

---

Correctness / Behavior

Traced all three tools through the new command-build logic:

Tool	Resume cmd	Fresh cmd	Chained cmd
claude	claude --continue -n 'feat - proj'	claude -n 'feat - proj'	✓ matches test assertion
codex (with prompt)	codex resume --last <flags> <prompt>	codex <flags> <prompt>	✓ matches old code shape
gemini (with prompt)	gemini --resume latest <flags> -i <prompt>	gemini <flags> -i <prompt>	✓ matches old code shape

The freshWorktree=true path is untouched, consistent with the stated design.

---

Test Coverage

• Three updated/renamed assertions correctly match the new ||-chained shapes.
• The new regression test covers the originally-reported failure case (agent switch codex → claude on an existing worktree) — this is the most important addition.
• 781/781 tests pass, typecheck is clean.

One gap worth noting: there's no test for the freshWorktree=true path producing a command without ||. That path was already tested implicitly before this PR, so it's not a blocker, but a targeted assertion would guard against future regressions in the opposite direction.

---

Security

No concerns. All user-controlled values (displayName, initialPrompt, individual flags) pass through shellQuote before interpolation. The || operator itself is safe in this context — both sides of the chain are built from the same already-quoted strings.

---

Summary

The core logic is correct, the refactor is a genuine improvement, and the test additions are appropriate. The three comments flagged above are minor style nits relative to the new AGENTS.md guideline. No blockers.

🤖 Generated with Claude Code