Repository navigation
Conversation
Captured scrollback ends at the prompt line where the cursor sat, with no trailing newline, so a bare replay glued the freshly-restored live prompt onto the end of the old prompt line. Guarantee a single trailing newline in normalizedScrollback so the live shell's first prompt starts on its own line. Applied at replay time, so it fixes both bash and zsh and even scrollback saved by older builds. Co-Authored-By: Claude <noreply@anthropic.com>
|
@grantland is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
To use Codex here, create a Codex account and connect to github. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR ensures truncated, ANSI-safe scrollback ends with a trailing newline before replay to avoid prompt concatenation, and adds documentation describing cmux’s automatic shell integration for bash and zsh, its failure modes, and troubleshooting steps. ChangesSession Persistence and Restoration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let safe = ansiSafeReplayText(truncated) | ||
| // The captured scrollback ends at the prompt line where the cursor sat, | ||
| // which has no trailing newline. A bare replay (`cat`) would then glue the | ||
| // freshly-restored live prompt onto the end of that old prompt line | ||
| // ("…$ …$"). Guarantee a trailing newline so the live shell's first prompt | ||
| // starts on its own line (https://github.com/manaflow-ai/cmux/issues/2823). | ||
| return safe.hasSuffix("\n") ? safe : safe + "\n" |
There was a problem hiding this comment.
ansiSafeReplayText runs on scrollback that already ends in " ", it appends ansiReset after that newline (because hasSuffix(ansiReset) is false), yielding … \u{001B}[0m. The newline check then sees hasSuffix(" ") as false and appends another " ", producing … \u{001B}[0m . On replay this writes an extra blank line between the restored prompt and the live one — the opposite cosmetic artifact from the original glue bug. The case is rare (the PR notes captured scrollback typically has no trailing newline), but adding the newline before calling ansiSafeReplayText avoids the interaction entirely: the reset wraps the content and the newline is always the final character.
| let safe = ansiSafeReplayText(truncated) | |
| // The captured scrollback ends at the prompt line where the cursor sat, | |
| // which has no trailing newline. A bare replay (`cat`) would then glue the | |
| // freshly-restored live prompt onto the end of that old prompt line | |
| // ("…$ …$"). Guarantee a trailing newline so the live shell's first prompt | |
| // starts on its own line (https://github.com/manaflow-ai/cmux/issues/2823). | |
| return safe.hasSuffix("\n") ? safe : safe + "\n" | |
| // The captured scrollback ends at the prompt line where the cursor sat, | |
| // which has no trailing newline. A bare replay (`cat`) would then glue the | |
| // freshly-restored live prompt onto the end of that old prompt line | |
| // ("…$ …$"). Guarantee a trailing newline so the live shell's first prompt | |
| // starts on its own line (https://github.com/manaflow-ai/cmux/issues/2823). | |
| let newlineTerminated = truncated.hasSuffix("\n") ? truncated : truncated + "\n" | |
| return ansiSafeReplayText(newlineTerminated) |
…w-ai#5853 review) When the captured scrollback both contains ANSI and already ended in a newline, the previous two-step approach (wrap, then append \n) produced …\n␛[0m\n — an extra blank line between the restored content and the live prompt, the inverse of the original glue bug. Append the newline before wrapping so the cursor-neutral ANSI reset trails it and exactly one newline is ever emitted. Co-Authored-By: Claude <noreply@anthropic.com>
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
@grantland I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,260 of the 240,000 allowed lines of code this month. Reviews resume on 1 July 2026 (in 9 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
To use Codex here, create a Codex account and connect to github. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get the PR diff
gh pr diff 5853 --repo manaflow-ai/cmux
echo "=== PR FILES ==="
gh pr view 5853 --repo manaflow-ai/cmux --json files --jq '.files[].path'Length of output: 7093
Sources/SessionPersistence.swiftThe core fix is clean and correct. Logic is sound: One minor edge case worth acknowledging: if // Optional: make the intent explicit
let withTrailingNewline: String
if truncated.isEmpty {
return nil // or ansiSafeReplayText(truncated) — preserve existing behaviour
} else {
withTrailingNewline = truncated.hasSuffix("\n") ? truncated : truncated + "\n"
}
return ansiSafeReplayText(withTrailingNewline)Not blocking — just worth a conscious decision either way. docs/shell-integration.mdWell-structured and fills a real gap. A couple of observations: 1. It's listed as one of the zsh-specific ways to break the integration, but it almost certainly applies to both shells. Bash users who stumble on this may not realise the override flag exists. Consider moving it to a shared note or the troubleshooting section. 2. Bash append example ordering PROMPT_COMMAND="history -a${PROMPT_COMMAND:+; $PROMPT_COMMAND}"This places PROMPT_COMMAND="${PROMPT_COMMAND:+$PROMPT_COMMAND; }history -a"Either works; the current example is just slightly non-idiomatic and might confuse readers who try to reason about ordering. 3. Troubleshooting function check is necessary but not sufficient
Overall: the fix is minimal and well-motivated, the comment explains the non-obvious ordering constraint clearly, and the documentation covers the problem space thoroughly. The items above are all minor — nothing blocking. |
2 participants
Fixes #2823 (the scrollback-restore portion).
Summary
What changed?
SessionScrollbackReplayStore.normalizedScrollbacknow guarantees asingle trailing newline on the replayed scrollback. Captured scrollback ends
at the prompt line where the cursor sat, which has no trailing newline, so a
bare replay (
cat) glued the freshly-restored live prompt onto the end of theold prompt line (
…$ …$). The newline is added when the replay file is writtenat terminal spawn, so it applies to both bash and zsh and even to scrollback
saved by older builds.
docs/shell-integration.mdexplaining how cmux's shellintegration loads (bash via an exported
PROMPT_COMMANDbootstrap; zsh viaZDOTDIR), how a dotfile that assignsPROMPT_COMMANDsilently wipes it out(breaking new-tab cwd inheritance, scrollback restore, and shell-state
reporting), the append / direct-source fixes, the analogous (rarer) zsh
hook-array risk, and a troubleshooting check.
Why?
the root cause for affected bash users turned out to be a dotfile overwriting
PROMPT_COMMAND, which prevents cmux's shell integration from ever loading —so scrollback was never persisted and never replayed. Once the integration
loads, restore works, which surfaced a separate, long-latent cosmetic bug: the
restored prompt and the live prompt rendered on the same line. This PR fixes
that cosmetic bug and documents the integration-loading pitfall so others can
self-diagnose the underlying issue.
Testing
CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag cmux-2823)and exercised the real save → quit → relaunch → replay path on macOS.
the same line (
…$ …$).below it.
_cmux_restore_scrollback_onceuses a direct/bin/cat -- "$path"(not$(cat)), so the trailing newline written bywriteReplayFileis preserved.type -t _cmux_restore_scrollback_oncereturnsfunctionand scrollbackrestores; with
PROMPT_COMMANDoverwritten it does not.Demo Video
For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).
Review Trigger (Copy/Paste as PR comment)
text @codex review @coderabbitai review @greptile-apps review @cubic-dev-ai review Checklist
\non the private helper would be a shape test, not behavior (per the repo's test-quality policy). Happy to add a runtime seam in a follow-up if desired.docs/shell-integration.md.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes the glued prompt after scrollback restore by guaranteeing a single trailing newline and adding it before ANSI wrapping, so the live prompt starts on its own line without extra blank lines. Adds
docs/shell-integration.mdexplaining how integration loads (bash viaPROMPT_COMMANDbootstrap; zsh viaZDOTDIR), common clobbers, and troubleshooting for #2823.Written for commit 04e7377. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Documentation