Repository navigation
Wait for shell prompt before typing 'cmux welcome' (fixes #1900) - #4682
juliankmazo wants to merge 8 commits into
Conversation
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Notification.Name.panelShellActivityStateDidChange and PanelShellActivityNotificationKey; posts notifications on actual panel state transitions; adds Workspace.sendTextOnNextPromptIdle(one-shot fast-path + observer cleanup); updates welcome-send call sites; adds tests and project wiring. ChangesShell Activity Notifications and Prompt-Idle Waiting
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Workspace
participant NotificationCenter
participant TerminalPanel
Caller->>Workspace: sendTextOnNextPromptIdle("cmux welcome\n")
alt focused panel already .promptIdle
Workspace->>TerminalPanel: sendText("cmux welcome\n")
else wait for first .promptIdle
Workspace->>NotificationCenter: observe panelShellActivityStateDidChange (workspace scope)
NotificationCenter->>Workspace: notify .promptIdle for focused panel
Workspace->>TerminalPanel: sendText("cmux welcome\n")
Workspace->>NotificationCenter: remove observer
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (16 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 SummaryThis PR fixes the welcome-banner race condition (#1900) by replacing a blind 0.5 s
Confidence Score: 4/5Safe to merge with one follow-up: The core fix is sound and well-tested. The one concern is that Sources/Workspace.swift — Important Files Changed
Sequence DiagramsequenceDiagram
participant TM as TabManager.addWorkspace
participant WS as Workspace
participant NC as NotificationCenter
participant TC as TerminalController (shell hook)
TM->>WS: sendTextOnNextPromptIdle("cmux welcome\n")
note over WS: fast path: focusedPanel not .promptIdle yet<br/>register [weak self] observer token
WS->>NC: addObserver(panelShellActivityStateDidChange)
WS-->>TM: (observer pending)
TC->>WS: updatePanelShellActivityState(panelId, .promptIdle)
note over WS: guard previousState != state (dedup)
WS->>NC: post panelShellActivityStateDidChange (workspaceId, panelId, .promptIdle)
NC->>WS: observer fires synchronously (queue: nil)
WS->>WS: MainActor.assumeIsolated → performPromptIdleSend
WS->>WS: panel.sendText("cmux welcome\n")
WS->>NC: removeObserver(token)
WS->>WS: "pendingPromptIdleObservers.removeAll { $0 === token }"
Reviews (7): Last reviewed commit: "Test: use queue: nil for idempotency obs..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| DispatchQueue.main.asyncAfter(deadline: .now() + 8.0) { | ||
| Task { @MainActor in | ||
| if let readyObserver, !resolved { | ||
| NotificationCenter.default.removeObserver(readyObserver) | ||
| } | ||
| if !resolved { | ||
| panelsCancellable?.cancel() | ||
| guard !resolved else { return } | ||
| if let terminalPanel = workspace.focusedTerminalPanel, | ||
| terminalPanel.surface.surface != nil { | ||
| resolved = true | ||
| cleanup() | ||
| performSend(terminalPanel) | ||
| } else { | ||
| cleanup() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
DispatchQueue.main.asyncAfter(deadline: .now() + 8.0) is newly introduced timing-based synchronization in the completely rewritten sendWelcomeWhenReady. The allowed-case carve-out in swift-blocking-runtime for existing code the PR "does not introduce or worsen" does not apply here — the function body is entirely new. If the shell is at a running command when the 8-second wall-clock fires (e.g., user kicked off brew install before cmux's text landed), cmux welcome\n is injected unconditionally into that command's stdin. The real signal — panelShellActivityStateDidChange — is already wired; the fallback should instead model "no shell integration" as a distinct, documented state in Workspace so the send can be deferred cleanly without a bare timer.
Rule Used: Flag new blocking or timing-based synchronization ... (source)
| ), | ||
| terminalPanel.surface.surface != nil { | ||
| send(panel: terminalPanel, mode: "timeoutFallback") | ||
| NSLog("Command send: shell prompt not reported within \(fallbackDeadline)s, sending anyway") |
There was a problem hiding this comment.
NSLog bypasses unified logging
NSLog("Command send: shell prompt not reported within \(fallbackDeadline)s, sending anyway") is new production-path logging not guarded by #if DEBUG. The swift-logging rule flags NSLog in app/runtime Swift. The preferred shape is a nonisolated private let logger = Logger(subsystem: Logging.subsystem, category: "SendText") with logger.warning(...) so the message routes through the unified logging system and can be filtered in Console.
Rule Used: Flag production Swift diagnostics that bypass unif... (source)
juliankmazo
left a comment
juliankmazo
left a comment
There was a problem hiding this comment.
Blocking from my side, even though GitHub will only let me post this as a comment on my own PR. The notification signal is a good direction, but the current fallback can still type into an active shell-init prompt, and the new regression test has a likely async assertion race.
| preferredPanelId: preferredPanelId | ||
| ), | ||
| terminalPanel.surface.surface != nil { | ||
| send(panel: terminalPanel, mode: "timeoutFallback") |
There was a problem hiding this comment.
This fallback can recreate the same stdin race for users who do have shell integration but are still blocked in a shell-init prompt after 8s. In that case .promptIdle has not arrived because precmd has not run yet, but we still type cmux welcome\n into whatever is reading stdin, including the oh-my-zsh update prompt. That means the PR does not fully fix the shell-integration case; it just moves the blind timer from 0.5s to 8s. The fallback needs to distinguish “no shell integration signal is possible” from “shell integration exists but the prompt is not idle yet”, or avoid sending while a non-idle state is known.
| XCTAssertEqual(notificationCount, 0) | ||
|
|
||
| // Real transition — must post exactly once. | ||
| workspace.updatePanelShellActivityState(panelId: panelId, state: .commandRunning) |
There was a problem hiding this comment.
This assertion is likely racy. The observer was registered with queue: .main, so delivery is queued asynchronously; asserting notificationCount == 1 immediately after posting can fail on CI even if the production post is correct. Existing notification-style tests in this repo usually use queue: nil for synchronous assertions or an expectation. Please make this deterministic before relying on this as the red/green regression proof.
| @@ -2620,32 +2620,43 @@ class TabManager: ObservableObject { | |||
|
|
|||
| @MainActor | |||
| private func sendWelcomeWhenReady(to workspace: Workspace) { | |||
There was a problem hiding this comment.
This duplicates the same observer/resolved/fallback state machine now living in AppDelegate.sendTextWhenReady(waitForShellPromptIdle:). Per the shared behavior policy, the prompt-idle wait should have one implementation that both entrypoints use. The two copies already differ in details and will be easy to regress independently when the timeout/state behavior changes.
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
| shellActivityObserver = NotificationCenter.default.addObserver( | ||
| forName: .panelShellActivityStateDidChange, | ||
| object: nil, | ||
| queue: .main | ||
| ) { note in | ||
| guard let noteWorkspaceId = note.userInfo?[PanelShellActivityNotificationKey.workspaceId] as? UUID, | ||
| noteWorkspaceId == workspaceId, | ||
| let state = note.userInfo?[PanelShellActivityNotificationKey.state] as? PanelShellActivityState, | ||
| state == .promptIdle | ||
| else { return } | ||
| Task { @MainActor in attempt() } | ||
| } | ||
| } |
There was a problem hiding this comment.
shellActivityObserver is only deregistered inside attempt(), which only runs when promptIdle fires. For users without cmux shell integration, .promptIdle never fires, so the observer is never removed from NotificationCenter. attempt() is a nested function that implicitly captures self (the Workspace instance) via focusedTerminalPanel and panelShellActivityStates. This gives NotificationCenter a strong reference chain to the workspace, preventing deallocation for the session's lifetime — effectively a workspace-level retain cycle.
Additionally, the PR description and test plan both promise "banner still appears via the 8s fallback" for users without shell integration; the implementation has no fallback at all. Every new workspace created for a user without shell integration will silently skip the welcome banner forever, which is a regression from the pre-PR 3s timeout-based send.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/PanelShellActivityNotificationTests.swift`:
- Around line 96-108: The test uses RunLoop.current.run(until: ...) which is
timing-sensitive; replace those ad-hoc sleeps with deterministic main-queue
drains using XCTestExpectation so assertions don't flake. Introduce and call a
small helper (e.g. drainMainQueue or waitForMainQueue) that creates an
XCTestExpectation, dispatches a no-op to DispatchQueue.main.async (or
OperationQueue.main.addOperation) and fulfills the expectation, then calls
wait(for:timeout:); replace each RunLoop.current.run(...) call in
PanelShellActivityNotificationTests with that helper so
workspace.updatePanelShellActivityState and the Task<MainActor> notification
observer reliably run before asserting sendCount.
In `@Sources/AppDelegate.swift`:
- Around line 7614-7621: The welcome send currently relies on
workspace.sendTextOnNextPromptIdle and only sets WelcomeSettings.shownKey inside
the beforeSend closure, so when the panel never reaches
PanelShellActivityState.promptIdle the welcome is silently skipped; update the
flow so sendTextOnNextPromptIdle reports whether the text was actually sent
(e.g., change its completion to include a Bool sent flag or an Error), and when
it reports "not sent" surface a user-facing hint (UI banner/alert or a clear log
message) directing users to run "cmux hooks setup" so they know shell
integration is missing; keep setting WelcomeSettings.shownKey only when sent.
In `@Sources/TabManager.swift`:
- Around line 2610-2613: The welcome send currently only uses
newWorkspace.sendTextOnNextPromptIdle which waits forever if shell integration
is disabled, so retain the previous timeout fallback: update
sendTextOnNextPromptIdle (or add a workspace-specific helper like
sendWelcomeOnNextPromptIdleWithTimeout) to attempt the promptIdle path but also
schedule a fallback send after the grace period; ensure the callback that sets
UserDefaults.standard.set(true, forKey: WelcomeSettings.shownKey) runs in both
the promptIdle success and the timeout fallback branches so the shownKey is
flipped regardless of shell-integration state (targets: autoWelcomeIfNeeded,
select, WelcomeSettings.shownKey, newWorkspace.sendTextOnNextPromptIdle).
In `@Sources/Workspace.swift`:
- Around line 10765-10807: sendTextOnNextPromptIdle currently registers a
NotificationCenter observer (shellActivityObserver) whose closure captures the
nested attempt() and thus the Workspace strongly; if .promptIdle never arrives
the observer is never removed and Workspace is retained. Fix by making the
notification handler capture self weak (e.g., [weak self]) and avoid referencing
attempt() directly from the closure (call a weak-self-safe helper or inline the
minimal check), and ensure the observer token is stored on the Workspace (e.g.,
a property like pendingShellActivityObserver) so it can be removed in a
deterministic lifecycle point (on cancellation or Workspace.deinit) — always
call cleanup() when removing the observer and clear the stored token. Ensure
shellActivityObserver is nil'd after removal to avoid leaks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f3d706f-d680-45ac-8c2d-d51f71f9a8b9
📒 Files selected for processing (4)
Sources/AppDelegate.swiftSources/TabManager.swiftSources/Workspace.swiftcmuxTests/PanelShellActivityNotificationTests.swift
|
Thanks — both addressed in the follow-up commits:
Also fixed an observer leak Greptile flagged on the same pass: tokens are tracked on |
|
Thanks for the assertive review. Addressing each of the four findings: 1. Test uses 2. 3. If reviewers want the auto-banner to land for users without shell integration, the right plumbing is to write the welcome content to the terminal as output (bypassing the PTY input race entirely) rather than typing — but that needs a new Ghostty surface API and is out of scope for the bug fix. 4. Observer leak in Note: the |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cmuxTests/PanelShellActivityNotificationTests.swift (1)
67-83:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse synchronous observer delivery in the idempotency assertion path.
With
queue: .main, delivery is enqueued, so the assertion on Line 82 can race and flake. Usequeue: nil(or wait via expectation) before asserting.Proposed minimal fix
let observer = NotificationCenter.default.addObserver( forName: .panelShellActivityStateDidChange, object: nil, - queue: .main + queue: nil ) { _ in notificationCount += 1 }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmuxTests/PanelShellActivityNotificationTests.swift` around lines 67 - 83, The observer registered with NotificationCenter.default.addObserver(forName: .panelShellActivityStateDidChange, object: nil, queue: .main) enqueues delivery and can race with the XCTAssertEqual after workspace.updatePanelShellActivityState; change the observer registration to use queue: nil (synchronous delivery) for this test path—or alternatively wait on an XCTestExpectation—so that the idempotency assertion after calling workspace.updatePanelShellActivityState(panelId:panelId, state:.promptIdle) and the subsequent real transition are deterministic; update the addObserver call where notificationCount is incremented and keep the defer removal as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@cmuxTests/PanelShellActivityNotificationTests.swift`:
- Around line 67-83: The observer registered with
NotificationCenter.default.addObserver(forName:
.panelShellActivityStateDidChange, object: nil, queue: .main) enqueues delivery
and can race with the XCTAssertEqual after
workspace.updatePanelShellActivityState; change the observer registration to use
queue: nil (synchronous delivery) for this test path—or alternatively wait on an
XCTestExpectation—so that the idempotency assertion after calling
workspace.updatePanelShellActivityState(panelId:panelId, state:.promptIdle) and
the subsequent real transition are deterministic; update the addObserver call
where notificationCount is incremented and keep the defer removal as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1a3dc21c-b2a0-46bb-859d-e5f90090ce8d
📒 Files selected for processing (3)
Sources/TerminalController.swiftSources/Workspace.swiftcmuxTests/PanelShellActivityNotificationTests.swift
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…anaflow-ai#1900) Regression test for the dropped-first-char welcome bug: the welcome command was typed into the user's shell on a 0.5s timer that races shell init. With oh-my-zsh's auto-update prompt eating stdin, the 'c' of 'cmux welcome' is consumed by the prompt and the rest hits the shell as 'mux welcome' / 'command not found: mux'. Fix path is to wait for shell-integration-reported promptIdle before typing. To make that wait observable, the test asserts that Workspace.updatePanelShellActivityState posts a notification with workspaceId, panelId, and the new state. This commit only adds the declaration and the test; CI is expected to go red until the follow-up commit adds the post site.
cmux's first-launch welcome was injected by typing 'cmux welcome\n' into the workspace's PTY 0.5s after the Ghostty surface became ready. That delay races shell init: oh-my-zsh's 'Would you like to update?' prompt reads stdin during sourcing, so it eats the 'c' and the rest of the line arrives at the real shell prompt as 'mux welcome', giving the user 'zsh: command not found: mux' on every fresh install. Wait for the shell to report an actual prompt before sending. cmux's shell integration already drives panelShellActivityStates[panelId] through promptIdle/commandRunning via Workspace.updatePanelShellActivityState; this commit makes that update post panelShellActivityStateDidChange so sendTextWhenReady can subscribe. - AppDelegate.sendTextWhenReady gains waitForShellPromptIdle, which gates the immediate-send branch on promptIdle, attaches a state-change observer, extends the readiness deadline to 8s, and SENDS at timeout (instead of just cleaning up) so the welcome banner still appears for users without cmux shell integration installed. - sendWelcomeCommandWhenReady opts into the new gating. - TabManager.sendWelcomeWhenReady (the fallback path used when AppDelegate.shared is nil at workspace creation) mirrors the same promptIdle wait + 8s send-on-timeout. Refs manaflow-ai#1900
… timeout Greptile P1 and CodeRabbit pre-merge checks flagged duplicated coordination logic in AppDelegate.sendTextWhenReady and TabManager.sendWelcomeWhenReady, plus the new 8s DispatchQueue.asyncAfter fallback (banned by the project's swift-blocking-runtime rule on latency-sensitive paths) and an NSLog without a DEBUG guard. Move ownership of the wait-for-prompt state machine into Workspace itself: - Workspace.sendTextOnNextPromptIdle(_:beforeSend:) is the single owner. It observes only .panelShellActivityStateDidChange filtered to this workspace, fires beforeSend + sendText exactly once on the first promptIdle, and cleans up its observer. - AppDelegate.sendWelcomeCommandWhenReady is now a one-liner that calls into the workspace. - TabManager auto-welcome branch (formerly forked into AppDelegate or sendWelcomeWhenReady) calls workspace.sendTextOnNextPromptIdle directly. - AppDelegate.sendTextWhenReady reverts to its original signature: the waitForShellPromptIdle parameter, shell-activity observer, and 8s asyncAfter fallback are removed. - TabManager.sendWelcomeWhenReady is deleted. No more time-based fallback. If shell integration is not installed the panel never reaches .promptIdle and the welcome banner is skipped rather than typed into an unknown stdin reader — that's the architectural invariant the rules require, and is also the safe choice for users who might otherwise still hit the omz race with a longer timer. Test now covers the full one-shot promptIdle semantics. NSLog change reverted; no new logging is added by this PR. Net diff against main is 118 insertions / 176 deletions — the fix removes more code than it adds. Refs manaflow-ai#1900
…n test
Greptile's re-review (3/5) flagged two issues on the previous commit:
1. Observer leak. The NotificationCenter observer registered in
sendTextOnNextPromptIdle was only ever removed inside the success
path. For workspaces that never saw .promptIdle (no shell integration,
or workspace closed before integration fired) the observer stayed
registered, and the implicit self capture through the nested
attempt() function kept the Workspace alive indefinitely. The block
itself also leaked.
Fix:
- Capture self weakly so NotificationCenter holding the block never
retains the Workspace.
- Persist outstanding observer tokens on Workspace
(pendingPromptIdleObservers) and sweep them in deinit so the block
is torn out of NotificationCenter as soon as the Workspace goes
away. A new XCTest covers the deinit sweep with a weak ref plus an
autoreleasepool.
2. Test was racy. The previous test relied on
RunLoop.current.run(until: Date().addingTimeInterval(0.05)) after
each updatePanelShellActivityState to give a Task @mainactor hop
time to land. On slow CI this would flake.
Fix:
- Register the observer with queue: nil so notifications dispatch
synchronously on the posting thread
(Workspace.updatePanelShellActivityState is @mainactor, so the
post is already on main).
- Use MainActor.assumeIsolated to call back into MainActor-isolated
send logic without an async hop.
- The test now asserts immediately after each state-change call —
no runloop spins, no flakes. A comment in the test pins this
invariant so a future refactor that re-introduces an async hop
is caught.
Refactored attempt() into a standalone @mainactor performPromptIdleSend
helper so both the fast path (sync) and the observer path call into
the same checked send routine.
Refs manaflow-ai#1900
CodeRabbit's CHANGES_REQUESTED review on the prior commit flagged that the auto-welcome path only flipped `WelcomeSettings.shownKey` inside the `beforeSend` closure passed to `sendTextOnNextPromptIdle`. For users without cmux shell integration installed `.promptIdle` never fires, so: 1. The key was never flipped. 2. Every subsequent workspace creation re-entered the auto-welcome branch and registered another never-fired observer. 3. With the deinit-sweep in place those observers don't leak the workspace, but they're still wasted work and the user-visible semantic — "first-launch onboarding has been attempted" — was wrong. Fix the semantic: flip the shown flag synchronously the first time we attempt to type the welcome. The welcome banner is one-shot onboarding; one attempt per fresh install is correct regardless of whether the banner actually rendered. Users who later install shell integration won't see the auto-banner, but they can run `cmux welcome` from the CLI at any time. Also drop the now-dead `markShownOnSend` parameter on `AppDelegate.sendWelcomeCommandWhenReady` — the only remaining caller (`openWelcomeWorkspace`, triggered by the user) always used `markShownOnSend: false`, so the parameter was vestigial. Refs manaflow-ai#1900
The only remaining pre-merge warning on the last revision was 'Docstring Coverage 30.77%' against the 80% threshold. Adds doc comments to the functions and userInfo-key enum introduced by this PR: - Workspace.updatePanelShellActivityState - Workspace.performPromptIdleSend - PanelShellActivityNotificationKey (enum + each constant) - testStateChangePostsNotificationWithWorkspaceAndPanelIds - testDuplicateStateDoesNotPostNotification Pure documentation, no behavior change.
Doc-coverage tooling counts only /// docstrings. The notification name already had a comment explaining its purpose; this just promotes it to the form the doc tool recognizes.
CodeRabbit's re-review noted that testDuplicateStateDoesNotPostNotification's observer was still registered with queue: .main, so the post-then-assert sequence could race on slow CI. Switch to queue: nil (synchronous delivery on the posting thread, which is @mainactor) to match the other tests and the production observer in Workspace.sendTextOnNextPromptIdle.
745bd9d to
e8a8b37
Compare
|
Thank you for the fix! Current main has moved the welcome and shell prompt handling forward, superseding this implementation :) |
2 participants
Summary
The first-launch welcome banner is injected by typing
cmux welcome\ninto the new workspace's PTY 0.5s after the Ghostty surface becomes ready. That blind delay races shell init. With oh-my-zsh's auto-update prompt reading stdin during sourcing, thecofcmux welcomeis consumed by theWould you like to update? [Y/n]prompt, and the rest of the line lands on the real shell prompt asmux welcome, leaving every fresh oh-my-zsh user with:This is the canonical instance of the bigger pattern called out in #1900: cmux types commands into a shell that isn't actually at a prompt yet.
Approach
Workspaceowns the wait-for-prompt state machine. cmux's shell integration already drivespanelShellActivityStates[panelId]through.promptIdle/.commandRunningviaWorkspace.updatePanelShellActivityState; this PR wires that into a single owner:Workspace.updatePanelShellActivityStateposts.panelShellActivityStateDidChangeon every real state change (deduplicated; userInfo carries workspaceId, panelId, state).Workspace.sendTextOnNextPromptIdle(_:beforeSend:)is the single coordination point. It checks the synchronous fast path, otherwise registers a[weak self]observer that firesbeforeSend+sendTextexactly once when.promptIdleis first observed for the focused panel.Workspaceand swept indeinit, so workspaces without shell integration installed (or workspaces closed before integration fires) do not leak their observer block.AppDelegate.sendWelcomeCommandWhenReadyis a one-line delegate to the workspace.TabManager's auto-welcome branch calls the workspace directly. The oldTabManager.sendWelcomeWhenReady(with its blindasyncAfter(0.5)) is deleted.No timing fallback. If cmux shell integration is not installed the panel never reaches
.promptIdleand the welcome banner is skipped. That's the architectural invariant the project'sswift-blocking-runtime.mdrule asks for — any time-based fallback would still race shell init, which is the bug we're fixing. Users without shell integration get a clean prompt instead ofmux welcome/command not found: mux; the better fix for them is to surfacecmux hooks setupmore prominently (#4515) rather than to keep typing into a shell whose state we can't verify.Net change
More code is deleted than added. No new
DispatchQueue.asyncAfter, no newNSLog, no duplicate coordination logic.Regression test
Per
CLAUDE.md's two-commit regression policy, this PR is structured as:PanelShellActivityNotificationTestsasserting the new notification is posted with the expected userInfo and not posted for duplicate state. CI should go red on this commit alone (no post site exists).Workspace.swift,AppDelegate.swift,TabManager.swift. CI should go green.Later commits in the branch address reviewer feedback (single-owner refactor, observer leak fix, synchronous test delivery to remove flake risk).
Tests cover:
testStateChangePostsNotificationWithWorkspaceAndPanelIds).testDuplicateStateDoesNotPostNotification).sendTextOnNextPromptIdlewaits, fires exactly once on first.promptIdle, and is one-shot through later state bounces (testSendTextOnNextPromptIdleFiresExactlyOnceAtFirstPromptIdle)..promptIdlenever fires (testSendTextOnNextPromptIdleDoesNotLeakWorkspaceWhenPromptIdleNeverFires)../scripts/lint-pbxproj-test-wiring.shpasses locally.Test plan
cmuxTestsrunsPanelShellActivityNotificationTestsand the rest of the suite passescmux hooks setup). Banner appears under the prompt, nocommand not found: mux, even with oh-my-zsh's auto-update prompt waiting on stdin~/.zshrcthat does not run oh-my-zsh and no shell integration — welcome banner is skipped (intentional; better than typing into an unverified prompt)Notes for reviewers
Workspace.sendTextOnNextPromptIdle) was done in response to Greptile's P1 (duplicate coordination logic) and CodeRabbit's architectural-rethink pre-merge check.ghostty_surface_*.@concurrentfinding is a false positive:workspacePullRequestAuthHeaderValuewas added by Austin Wang ine39f5e02c8(April 2026) and is unmodified by this PR.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests