Skip to content

fix(tmux): add delay between paste and Enter for auto-respond prompts#274

Merged
dgershman merged 1 commit into
mainfrom
feature/crow-272-auto-detect-prompt-enter
May 15, 2026
Merged

fix(tmux): add delay between paste and Enter for auto-respond prompts#274
dgershman merged 1 commit into
mainfrom
feature/crow-272-auto-detect-prompt-enter

Conversation

@dgershman
Copy link
Copy Markdown
Collaborator

Closes #272

Summary

  • Adds a 50ms delay between paste-buffer and send-keys Enter in TmuxBackend.sendText() to prevent a timing race where the Enter keystroke arrives before the TUI has finished processing the bracketed-paste bracket-end sequence
  • Only applies the delay when content was actually pasted (bare Enter-only sends skip the delay)
  • Adds two integration tests covering the trailing-newline paste+Enter path and the bare-Enter edge case

Context

PR #267 fixed quick-action prompts not submitting by splitting the trailing \n into a separate send-keys Enter after the paste. The same fix covers auto-respond prompts (same code path), but a timing race between the two tmux CLI invocations can cause the Enter to be silently dropped when the terminal has been idle — which is the typical state when auto-respond fires.

Test plan

  • swift build compiles without errors
  • swift test passes (existing + new TmuxBackendTests)
  • Manual: create a tmux-backed session with a PR that has failing CI, verify the auto-respond prompt is submitted automatically

🤖 Generated with Claude Code

…#272)

When auto-respond detects failing CI and sends a prompt to Claude Code,
the paste-buffer and send-keys Enter commands can race: the TUI hasn't
finished processing the bracket-end sequence before Enter arrives,
causing the keystroke to be silently dropped. Add a 50ms delay between
paste and Enter to give the TUI time to process, matching the pattern
that works reliably for quick actions.

🤖 Generated with Claude Code, orchestrated by Crow

Co-Authored-By: Claude <noreply@anthropic.com>
Crow-Session: F9344639-17E5-4B0E-81C0-ED59F8B0872A
Copy link
Copy Markdown
Contributor

@dhilgaertner dhilgaertner left a comment

Choose a reason for hiding this comment

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

Code & Security Review

Critical Issues

None — the fix is small, targeted, and the reasoning in the comment makes the race clear.

Security Review

Strengths:

  • No new user-controllable surface — the added Enter is a fixed tmux key name, not derived from input.
  • Buffer name is still crow-<UUID> and content is delivered via load-buffer from stdin, so the existing injection-safe path is preserved.

Concerns: None.

Code Quality

Yellow — Thread.sleep blocks the main thread
TmuxBackend is @MainActor, so sendText runs on the main thread. Thread.sleep(forTimeInterval: 0.05) at Packages/CrowTerminal/Sources/CrowTerminal/TmuxBackend.swift:281 therefore stalls AppKit for 50ms on every content-bearing send. Call sites today (TerminalRouter.send at Sources/Crow/App/TerminalRouter.swift:24, and the initial-command paste inside registerTerminal at TmuxBackend.swift:183) all run on the main actor.

In the auto-respond case the user is by definition idle, so a 50ms hitch is invisible. The initial-command path during registerTerminal runs amid bigger startup work, so also unlikely to be noticed. It only becomes a real concern if sendText ever ends up on a hot, interactive path (rapid typing forwarded, scripted bursts).

Cheap mitigation if you want it later: make sendText async and await Task.sleep(nanoseconds: 50_000_000) instead — but that ripples through TerminalRouter.send. Not worth blocking on now; the comment at lines 253–257 already documents why the delay exists, which is what makes a future migration easy.

Green — didPaste flag is equivalent to !payload.isEmpty
At TmuxBackend.swift:268-275 the flag only goes true when payload is non-empty, so the guard at line 280 could be written if !payload.isEmpty. The named flag reads slightly better as intent, though — fine either way.

Green — 50ms is a magic number
The value isn't justified empirically in the PR description. If a slower host (CI VM, low-end hardware under load) drops Enter again, the knob is buried in source. A private static let pasteEnterDelay: TimeInterval = 0.05 constant with a one-line comment would make it greppable and tunable. Not worth blocking on.

Tests

  • sendTextWithTrailingNewlineDoesNotThrow and sendTextEmptyWithNewlineDoesNotThrow (TmuxBackendTests.swift:128-162) cover the two new branches end-to-end against a real tmux server. They only assert non-throwing — the actual race they're fixing isn't deterministically testable from the host side, which is acceptable for this kind of timing fix.
  • The newline test adds ~50ms to the integration suite per run. Negligible.

Summary Table

Priority Issue
🟡 Yellow Thread.sleep on @MainActor blocks the UI for 50ms per content-bearing send. Tolerable today given the call sites, worth revisiting if sendText ever lands on an interactive hot path.
🟢 Green Consider extracting 0.05 to a named constant so the tuning knob is greppable.
🟢 Green didPaste could be inlined as !payload.isEmpty.

Recommendation: Approve. The fix correctly addresses the race described in #272, the scope matches the bug, the comment block at lines 245–257 will save the next person from re-discovering the bracketed-paste quirk, and the tests at least keep the new branches from regressing into a crash. The main-thread sleep is a real cost but bounded and contextually acceptable.


🤖 Reviewed by Crow via Claude Code

@dgershman dgershman merged commit 1299ff3 into main May 15, 2026
3 checks passed
@dgershman dgershman deleted the feature/crow-272-auto-detect-prompt-enter branch May 15, 2026 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto-detect failing CI job prompt does not press Enter

2 participants