Skip to content

fix(omni): use --resume for respawn with JSONL-missing fallback#2489

Open
rodriguess-caio wants to merge 12 commits into
automagik-dev:devfrom
rodriguess-caio:fix/omni-spawn-script-path
Open

fix(omni): use --resume for respawn with JSONL-missing fallback#2489
rodriguess-caio wants to merge 12 commits into
automagik-dev:devfrom
rodriguess-caio:fix/omni-spawn-script-path

Conversation

@rodriguess-caio
Copy link
Copy Markdown

@rodriguess-caio rodriguess-caio commented May 27, 2026

Problem

When the Omni bridge respawns a per-chat agent and a prior Claude session id exists, the bridge was emitting --session-id instead of --resume, so Claude never reattached to the existing JSONL transcript. Operator invariant broken: omni-bridged chats should be permanent until explicitly closed.

Additionally, --resume silently fails when the JSONL is missing (cleanup, fresh machine) — Claude exits immediately and the pane returns to the shell with no error output.

Solution

  • Restore buildOmniSpawnParams to emit resume (not sessionId) when a prior Claude session id exists, so buildLaunchCommand generates --resume <id>
  • Add a 3-second liveness check after a --resume launch: if Claude exits immediately (JSONL missing), detect via isPaneProcessRunning and fall back to a fresh --session-id so the inbound message is not lost
  • Extract sendToPane helper to avoid duplicating the env-prefix + script-write + send-keys flow

Test plan

  • bun test src/services/executors/claude-code.test.ts — 45 tests pass
  • Respawn an existing chat: confirm Claude reattaches to prior conversation
  • Delete the JSONL manually and respawn: confirm fallback to fresh session, message not lost

Closes #2486

🤖 Generated with Claude Code

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8553bcf4-95b8-4389-bb14-7e6b0a5399ab

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors how complex tmux commands are spawned by moving the logic to write a temporary launch script (writeTmuxLaunchScript) into a shared utility file (src/lib/tmux-launch-script.ts). It also updates the Claude Code executor to use this script-based launch mechanism to prevent command corruption, and adds unit and smoke tests to validate the fix. The review feedback highlights critical issues where unquoted script paths containing spaces (such as in user home directories) can cause the source command to fail in tmux, and suggests a more robust regular expression for replacing the --resume flag with --session-id.

Comment thread src/services/executors/claude-code.ts Outdated
const cmd = envPrefix ? `${envPrefix} ${launch.command}` : launch.command;
await executeTmux(`send-keys -t '${paneId}' ${shellQuote(cmd)} Enter`);
const scriptPath = writeTmuxLaunchScript(`omni-${chatId}`, cmd);
await executeTmux(`send-keys -t '${paneId}' "source ${scriptPath}" Enter`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If the user's home directory path contains spaces (e.g., /Users/John Doe), the unquoted scriptPath will cause the source command to fail in the tmux pane shell, preventing Claude Code from launching. Wrapping scriptPath in shellQuote ensures the path is safely escaped.

Suggested change
await executeTmux(`send-keys -t '${paneId}' "source ${scriptPath}" Enter`);
await executeTmux("send-keys -t '" + paneId + "' \"source " + shellQuote(scriptPath) + "\" Enter");

Comment thread src/lib/tmux-launch-script.ts Outdated

// Force --session-id instead of --resume so Claude Code creates a fresh
// session rather than failing when the resumed session JSONL is missing.
const safeCommand = fullCommand.replace(/--resume\s+'([^']+)'/, "--session-id '$1'");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current regular expression /--resume\s+'([^']+)'/ assumes that the --resume flag is always wrapped in single quotes. If the command quoting style changes or if it is invoked with double quotes or no quotes (e.g., in manual/test runs), the replacement will silently fail, and the dead pane issue will persist. Using a more robust regex with a replacer function handles all quoting styles (single, double, or none) safely.

Suggested change
const safeCommand = fullCommand.replace(/--resume\s+'([^']+)'/, "--session-id '$1'");
const safeCommand = fullCommand.replace(/--resume\\s+(?:'([^']+)'|\"([^\"]+)\"|(\\S+))/, (_, g1, g2, g3) => "--session-id '" + shellQuote(g1 || g2 || g3) + "'");

const scriptPath = writeTmuxLaunchScript('smoke-2486', nastyCommand);
// Replicate the NEW Genie path:
// executeTmux(`send-keys -t '${paneId}' "source ${scriptPath}" Enter`)
execSync(`tmux -L ${socket2} send-keys -t '${pane2}' "source ${scriptPath}" Enter`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the test is run on a machine where the home directory path contains spaces, the unquoted scriptPath will cause the source command to fail in the tmux pane. Wrapping it in single quotes ensures the path is handled correctly.

Suggested change
execSync(`tmux -L ${socket2} send-keys -t '${pane2}' "source ${scriptPath}" Enter`);
execSync("tmux -L " + shellQuote(socket2) + " send-keys -t '" + shellQuote(pane2) + "' \"source '" + shellQuote(scriptPath) + "'\" Enter");

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 771e1c3c4f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/lib/tmux-launch-script.ts Outdated

// Force --session-id instead of --resume so Claude Code creates a fresh
// session rather than failing when the resumed session JSONL is missing.
const safeCommand = fullCommand.replace(/--resume\s+'([^']+)'/, "--session-id '$1'");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve valid resume launches instead of forcing fresh sessions

When an Omni chat already has existingExecutor.claudeSessionId, buildOmniSpawnParams intentionally emits --resume so the respawned pane reattaches to the existing transcript, but this unconditional replacement changes every such launch into --session-id. In the normal crash/restart path where the JSONL still exists, the pane no longer resumes prior context even though the executor row keeps returning the old session id for future spawns, so permanent Omni chats lose conversation history rather than reattaching.

Useful? React with 👍 / 👎.

rodriguess-caio and others added 3 commits May 27, 2026 11:32
buildOmniSpawnParams now always emits sessionId (never resume), so
buildLaunchCommand produces --session-id <id>. Unlike --resume, this
flag attaches to an existing JSONL transcript when present but gracefully
starts a fresh session with the same id when the transcript is missing
(e.g. after cleanup or on a fresh machine) — preventing hard failures on
respawn. Fix is applied at the source (where the command is built), not
in the transport layer (tmux-launch-script), which is now provider-agnostic.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When respawning a per-chat agent with a prior Claude session id, emit
--resume (not --session-id) so Claude reattaches to the existing JSONL.
Add a 3s liveness check after launch: if --resume silently fails (JSONL
missing), fall back to a fresh --session-id so the inbound message is
not lost.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rodriguess-caio rodriguess-caio changed the title fix(omni): rewrite --resume to --session-id in tmux spawn scripts to prevent dead panes fix(omni): use --resume for respawn with JSONL-missing fallback May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant