Skip to content

Fix setup script appearing 3 times when starting a worktree#222

Merged
Kitenite merged 1 commit intomainfrom
teal-storm-24
Dec 1, 2025
Merged

Fix setup script appearing 3 times when starting a worktree#222
Kitenite merged 1 commit intomainfrom
teal-storm-24

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Dec 1, 2025

Summary

  • Fix race condition where terminal scrollback and pending events both contained the same output
  • Skip scrollback write when attaching to existing session if pending events already have the data

Test plan

  • Create a new worktree with a setup script configured
  • Verify the setup script command appears only once in the terminal

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed terminal display issue where scrollback content could be duplicated when resuming previous sessions with pending data.

✏️ Tip: You can customize this high-level summary in your review settings.

Race condition caused scrollback and pending events to both contain
the same terminal output, resulting in duplicate display when
attaching to an existing terminal session.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Dec 1, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
website Ready Ready Preview Comment Dec 1, 2025 11:06pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 1, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

The Terminal component's onSuccess handler for the createOrAttach flow is now conditional. Initial scrollback is applied only when the result is new or when pending events do not already contain scrollback data, preventing duplicate scrollback application.

Changes

Cohort / File(s) Summary
Terminal scrollback logic
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
Made onSuccess handler conditional to apply initial scrollback only when result is new or no pending events with scrollback exist, preventing duplication

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single-file, localized logic change affecting conditional scrollback application
  • Requires understanding of scrollback semantics and pending event handling
  • Key area: verify the conditional logic correctly identifies when scrollback should be applied vs. deferred to pending events

Possibly related PRs

  • Fix terminal scrollback #150: Modifies the Terminal component's scrollback initialization — this PR makes scrollback application conditional in the onSuccess handler, while the related PR changes applyInitialScrollback's behavior directly

Poem

🐰 A scrollback so keen, it scrolled once too many,
But now with conditions, no dupes shall be any!
The pending events dance with scrollback so true,
No more double-bouncing—just one crystal view! ✨

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch teal-storm-24

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14e0b66 and 842ec22.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1 hunks)

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.

@Kitenite Kitenite merged commit ec941d1 into main Dec 1, 2025
6 of 7 checks passed
@Kitenite Kitenite deleted the teal-storm-24 branch December 1, 2025 23:16
@coderabbitai coderabbitai Bot mentioned this pull request Dec 4, 2025
4 tasks
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