Repository navigation
Conversation
|
@Horacehxw is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR bundles three mostly-unrelated improvements: (1) three Claude Code hook scripts + a setup guide that add tab auto-naming and The Swift/test/CI changes are clean and well-targeted. The shell scripts work correctly in the happy path, but have a few issues worth addressing before publication:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CC as Claude Code
participant UPS as cmux-rename-sync.sh<br/>(UserPromptSubmit)
participant SS as cmux-session-start.sh<br/>(SessionStart)
participant ST as cmux-tab-sync.sh<br/>(Stop)
participant cmux as cmux CLI
CC->>SS: SessionStart event (JSON with cwd)
SS->>cmux: rename-workspace(basename(cwd))
CC->>UPS: UserPromptSubmit event (JSON with transcript_path)
UPS->>UPS: tail -500 transcript | grep custom-title
alt /rename was used
UPS->>cmux: list-workspaces (get current name)
UPS->>cmux: rename-workspace(customTitle)
end
CC->>ST: Stop event (JSON with transcript_path)
ST->>ST: readlines() → first 3 user msgs + last 5 msgs
ST->>ST: tokenize + keyword frequency → label (2–4 words)
ST->>cmux: rename-tab --surface $CMUX_SURFACE_ID label
Reviews (1): Last reviewed commit: "docs: add Claude Code hooks for tab auto..." | Re-trigger Greptile |
|
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:
📝 WalkthroughWalkthroughThis PR introduces three distinct changes: refactors UI-test launch stabilization in AppDelegate from single-shot logic to a DEBUG-only retry loop with failure recovery; extends TerminalWindowPortal.bind() with an additional deferred external-geometry synchronization pass to reconcile post-bind ancestor layout shifts; and adds comprehensive documentation and three example Bash hook scripts for Claude Code integration with cmux. Changes
Sequence DiagramsequenceDiagram
participant Test as Test Suite
participant AppDelegate
participant NSApp as NSApp/Windows
participant RunApp as NSRunningApplication
participant Callback as Retry Callback
Test->>AppDelegate: trigger UI-test launch
AppDelegate->>AppDelegate: stabilizeUITestLaunchWindowAndForeground()
loop Retry Loop (up to 20 attempts)
AppDelegate->>NSApp: check windows present & visible
AppDelegate->>NSApp: check key window non-nil
AppDelegate->>RunApp: check isActive
alt All conditions met
AppDelegate->>AppDelegate: activateUITestAppIfNeeded()
AppDelegate->>RunApp: activate(macOS-specific options)
AppDelegate->>NSApp: frontOrderWindows
AppDelegate->>NSApp: register/close fallback window
AppDelegate->>Callback: cancel retry loop
else Conditions not met
Callback->>AppDelegate: schedule retry at +0.25s
end
end
AppDelegate->>Test: stabilization complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #!/bin/bash | ||
| # Hook: Immediately sync /rename to cmux workspace name | ||
| # Triggered on: UserPromptSubmit | ||
| # Lightweight — no Python, no summarization. Just tail + grep. |
There was a problem hiding this comment.
The comment claims # Lightweight — no Python, no summarization. Just tail + grep., but the script invokes python3 twice: once on line 13 to extract the transcript path from JSON, and again on lines 19–22 to parse the customTitle field from a JSONL line. This is a factual error that will mislead users troubleshooting in environments where python3 is unavailable.
| # Lightweight — no Python, no summarization. Just tail + grep. | |
| # Lightweight — extracts custom-title via tail + grep + python3. No LLM summarization. |
There was a problem hiding this comment.
Fixed in 6c4a74a — comment updated to "Lightweight — minimal Python for JSON parsing only. Just tail + grep."
There was a problem hiding this comment.
Fixed in 6c4a74a — the comment now reads 'Lightweight — minimal Python for JSON parsing only. Just tail + grep.' Python is used only for the two JSON field extractions.
| # --- Read transcript efficiently: only parse what we need --- | ||
| lines = [] | ||
| with open(TRANSCRIPT, 'r') as f: | ||
| lines = f.readlines() |
There was a problem hiding this comment.
The comment says # --- Read transcript efficiently: only parse what we need ---, and the file header and PR description both describe the approach as "head+tail, not full scan". However, f.readlines() loads the entire transcript into memory. For a large long-running session (thousands of lines, potentially megabytes), this contradicts the stated design goal.
Only lines[:300] and lines[-200:] slices are ever consumed. A minimal fix is to avoid loading lines that will never be used:
# Read only the lines we need: first 300 + last 200
with open(TRANSCRIPT, 'r') as f:
first_chunk = [next(f, None) for _ in range(300)]
first_chunk = [l for l in first_chunk if l is not None]
# For the tail, we still need to seek; read all but limit to last 200
with open(TRANSCRIPT, 'r') as f:
lines = f.readlines()
tail_chunk = lines[-200:]Or, more simply, continue using f.readlines() but remove the misleading "efficiently (head+tail, not full scan)" comments so expectations are set correctly.
There was a problem hiding this comment.
Fixed in 6c4a74a — replaced f.readlines() with a subprocess call using head -300 + tail -200 for true partial reading. The comment was updated to "Reads transcript JSONL (first 300 + last 200 lines, not full parse)".
There was a problem hiding this comment.
Fixed in 6c4a74a — the Python block now uses subprocess.run(['head', '-300', TRANSCRIPT]) and subprocess.run(['tail', '-200', TRANSCRIPT]) instead of loading the whole file. Only the first 300 + last 200 lines are parsed. The header comment at line 4 reflects this: 'Reads transcript JSONL (first 300 + last 200 lines, not full parse)'.
| command -v cmux &>/dev/null || exit 0 | ||
|
|
||
| # Extract transcript path | ||
| TRANSCRIPT=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('transcript_path',''))" 2>/dev/null) |
There was a problem hiding this comment.
set -e + bare python3 one-liner can block UserPromptSubmit
All three scripts enable set -e at the top. The TRANSCRIPT=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('transcript_path',''))" 2>/dev/null) one-liner has no try/except around json.load(). If the hook input is unexpectedly malformed — or if python3 is not on PATH — the command substitution returns a non-zero exit code, which set -e converts into a script-level exit with a non-zero status.
For a UserPromptSubmit hook, a non-zero exit status is interpreted by Claude Code as a hook failure and can block prompt submission entirely, making the Claude Code session unusable until the hook is removed. Adding a fallback keeps the script safe:
| TRANSCRIPT=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('transcript_path',''))" 2>/dev/null) | |
| TRANSCRIPT=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('transcript_path',''))" 2>/dev/null) || true |
The same pattern applies to cmux-tab-sync.sh line 13 and cmux-session-start.sh line 14.
There was a problem hiding this comment.
Fixed in 6c4a74a — added || true to all python3 command substitutions in cmux-rename-sync.sh, cmux-tab-sync.sh, and cmux-session-start.sh so a missing python3 or malformed input can't block UserPromptSubmit hooks.
There was a problem hiding this comment.
Fixed in 6c4a74a — the transcript extraction on line 13 now has || true at the end of the assignment, so a missing python3 or malformed input silently produces an empty string and the early-exit guard on line 14 ([ -z "$TRANSCRIPT" ] && exit 0) handles the rest without aborting.
| - **First 3 user messages** — captures the session's original goal | ||
| - **Last 5 messages** (user + assistant) — captures the current focus | ||
|
|
||
| Extracts the most frequent meaningful keywords (2-5 words, mixed Chinese/English), and calls `cmux rename-tab --surface $CMUX_SURFACE_ID`. |
There was a problem hiding this comment.
The doc says the hook extracts a "2-5 keyword summary" and the summarize() function in cmux-tab-sync.sh iterates for n in (4, 3, 2) — so the maximum generated label is 4 words, not 5. The PR description also says "2-5 keyword summary". The doc and implementation should agree.
| Extracts the most frequent meaningful keywords (2-5 words, mixed Chinese/English), and calls `cmux rename-tab --surface $CMUX_SURFACE_ID`. | |
| Extracts the most frequent meaningful keywords (2–4 words, mixed Chinese/English), and calls `cmux rename-tab --surface $CMUX_SURFACE_ID`. |
There was a problem hiding this comment.
Fixed in 6c4a74a — the implementation was updated to iterate (5, 4, 3, 2) so the max label is now 5 words, matching the documentation's "2-5 words" claim.
There was a problem hiding this comment.
Fixed in 6c4a74a — the loop was changed from (4, 3, 2) to (5, 4, 3, 2) and the docs at line 90 already say '2-5 words'. The implementation now matches the documentation.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/examples/cmux-rename-sync.sh">
<violation number="1" location="docs/examples/cmux-rename-sync.sh:13">
P1: Because this script runs with `set -e`, a JSON parse failure or missing `python3` will abort the hook at this line before the guard checks run. Make transcript extraction non-fatal so unexpected input does not fail the hook event.</violation>
</file>
<file name="docs/examples/cmux-tab-sync.sh">
<violation number="1" location="docs/examples/cmux-tab-sync.sh:106">
P3: This loads the entire transcript into memory even though only the first ~300 and last ~200 lines are consumed. Use a bounded head/tail read strategy here to avoid full-file memory and I/O cost on long sessions.</violation>
</file>
<file name="docs/examples/cmux-session-start.sh">
<violation number="1" location="docs/examples/cmux-session-start.sh:14">
P2: With `set -e` enabled, this JSON extraction can terminate the script on malformed input or missing `python3` before it reaches the no-op checks. Treat the parse step as best-effort so the hook fails closed and exits cleanly.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
docs/claude-code-hooks-integration.md (1)
107-107: Add language specifier to fenced code block.Per markdownlint, fenced code blocks should have a language specified. For ASCII diagrams,
textorplaintextworks.📝 Suggested fix
-``` +```text Workspace name (sidebar):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/claude-code-hooks-integration.md` at line 107, The fenced code block containing the ASCII diagram line "Workspace name (sidebar):" is missing a language specifier; update the opening fence to include a language (e.g., change the opening ``` to ```text or ```plaintext) so the block follows markdownlint rules for fenced code blocks and helps render plain text diagrams correctly.docs/examples/cmux-tab-sync.sh (1)
104-106: Reads entire file into memory before slicing.The comment at line 4 claims "head+tail, not full scan," but
f.readlines()loads the entire transcript into memory. For large transcripts, consider streaming reads or using shellhead/tailbefore passing to Python.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/cmux-tab-sync.sh` around lines 104 - 106, The current code opens TRANSCRIPT and calls f.readlines(), which loads the whole file into memory (contradicting the "head+tail" intent); change the reading logic to stream the file instead of using f.readlines(): iterate over f (the file object) to capture only the first N lines for the head and use collections.deque(maxlen=M) or itertools.islice to retain only the last M lines for the tail, then combine those into the lines variable; alternatively, invoke shell head/tail before Python if preferred—update occurrences around TRANSCRIPT, the open(...) as f block, and the lines assignment to implement this streaming approach.docs/examples/cmux-rename-sync.sh (1)
28-28: Fragile parsing ofcmux list-workspacesoutput.The sed pattern depends on double-space delimiters and the exact
[selected]marker format. If the output format changes, this may fail silently (resulting in unnecessary but harmless rename calls). Consider documenting the expected format or using a more robust parsing approach if available (e.g., a JSON output mode).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/claude-code-hooks-integration.md`:
- Line 90: The docs claim "2-5 words" but the implementation in cmux-tab-sync.sh
(the loop over word lengths producing 4,3,2) only generates 2-4 words; update
the documentation phrase in docs/claude-code-hooks-integration.md to "2-4 words"
to match the script (or alternatively change the cmux-tab-sync.sh loop to
include 5 if you intend to support up to 5 words); reference the
cmux-tab-sync.sh loop that iterates through (4,3,2) and the sentence mentioning
"2-5 words" to keep them consistent.
In `@docs/examples/cmux-rename-sync.sh`:
- Line 4: The header comment "# Lightweight — no Python, no summarization. Just
tail + grep." is inaccurate because the script invokes python3 for JSON parsing
(see uses around the json parsing calls); update that comment to accurately
reflect the implementation (for example, mention that the script is lightweight
but does use Python for JSON parsing and still performs no summarization) so
readers aren't misled by the header.
In `@docs/examples/cmux-tab-sync.sh`:
- Around line 60-68: The tokenize function currently strips URLs and code but
doesn't redact sensitive spans; update tokenize to first apply an unanchored set
of sensitiveSpanPatterns (e.g., UUIDs, email addresses, access tokens/long hex
or bearer-like tokens, filesystem paths, ENV_VAR=... assignments, long numeric
sequences) across the combined body+subtitle before any other cleaning,
replacing matches with a neutral placeholder like "<REDACTED>"; then continue
the existing URL/code removal and tokenization logic. Ensure you reference the
tokenize function and the sensitiveSpanPatterns variable/name so the redaction
step runs prior to the subsequent re.sub calls and token extraction.
In `@Sources/AppDelegate.swift`:
- Around line 2425-2438: The helper moveUITestWindowToTargetDisplayIfNeeded()
currently performs its own 20-step asyncAfter retry scheduling, and
stabilizeUITestLaunchWindowAndForeground(attempt:) wraps it with another 20-step
retry, causing nested overlapping retries and diagnostic overwrites; fix by
adding a scheduleRetry (Bool) parameter or internal check to
moveUITestWindowToTargetDisplayIfNeeded() so its DispatchQueue.main.asyncAfter
retry branches only run when scheduleRetry is true, then call
moveUITestWindowToTargetDisplayIfNeeded(scheduleRetry: false) from
stabilizeUITestLaunchWindowAndForeground(attempt:) and let only
stabilizeUITestLaunchWindowAndForeground perform the outer scheduling, keeping
writeUITestDiagnosticsIfNeeded(stage:) behavior unchanged.
- Around line 2447-2451: The macOS 14+ branch in activateUITestAppIfNeeded()
drops .activateIgnoringOtherApps causing the app to remain backgrounded; change
the NSRunningApplication.current.activate call so both .activateAllWindows and
.activateIgnoringOtherApps are passed on macOS 14+ as well (i.e., use options:
[.activateAllWindows, .activateIgnoringOtherApps] instead of only
.activateAllWindows) so the isActive check can succeed reliably.
---
Nitpick comments:
In `@docs/claude-code-hooks-integration.md`:
- Line 107: The fenced code block containing the ASCII diagram line "Workspace
name (sidebar):" is missing a language specifier; update the opening fence to
include a language (e.g., change the opening ``` to ```text or ```plaintext) so
the block follows markdownlint rules for fenced code blocks and helps render
plain text diagrams correctly.
In `@docs/examples/cmux-tab-sync.sh`:
- Around line 104-106: The current code opens TRANSCRIPT and calls
f.readlines(), which loads the whole file into memory (contradicting the
"head+tail" intent); change the reading logic to stream the file instead of
using f.readlines(): iterate over f (the file object) to capture only the first
N lines for the head and use collections.deque(maxlen=M) or itertools.islice to
retain only the last M lines for the tail, then combine those into the lines
variable; alternatively, invoke shell head/tail before Python if
preferred—update occurrences around TRANSCRIPT, the open(...) as f block, and
the lines assignment to implement this streaming approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c742e1ec-cc3c-475b-a214-6ab73a89d3d6
📒 Files selected for processing (8)
.github/workflows/ci.ymlSources/AppDelegate.swiftSources/TerminalWindowPortal.swiftcmuxTests/TerminalAndGhosttyTests.swiftdocs/claude-code-hooks-integration.mddocs/examples/cmux-rename-sync.shdocs/examples/cmux-session-start.shdocs/examples/cmux-tab-sync.sh
| if #available(macOS 14.0, *) { | ||
| NSRunningApplication.current.activate(options: [.activateAllWindows]) | ||
| } else { | ||
| NSRunningApplication.current.activate(options: [.activateAllWindows, .activateIgnoringOtherApps]) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is NSRunningApplication.ActivationOptions.activateIgnoringOtherAppsstill available and intended forNSRunningApplication.current.activate(options:)on macOS 14+, and is.activateAllWindows alone sufficient to bring a background app to the foreground?
💡 Result:
Yes, NSRunningApplication.ActivationOptions.activateIgnoringOtherApps (which is .activateIgnoringOtherApps in Swift) is still available and intended for use with NSRunningApplication.current.activate(options:) on macOS 14+ (Sonoma) and later. It is part of NSApplication.ActivationOptions, explicitly documented as available for NSRunningApplication.activate(options:) in current Apple documentation (copyright 2026). While the old NSApplication.activate(ignoringOtherApps:) method is deprecated as of macOS 14, the options-based API using activateIgnoringOtherApps remains supported, as evidenced by developer usage in macOS Sonoma contexts and bindings like .NET for macOS 26.0 and Rust objc2-app-kit noting it as deprecated only in a warning sense but functional. No, .activateAllWindows alone is not sufficient to reliably bring a background app to the foreground, especially if another app is currently active. By default (no options or without .activateIgnoringOtherApps), activation deactivates the calling app first and only activates the target if no other app becomes active in the interim, preventing focus stealing. .activateAllWindows only controls which windows come forward (all vs. main/key), not whether the app reliably becomes active/foreground over others. Sources confirm that for reliable foreground activation of background apps (e.g., from status bar/menu bar apps), .activateIgnoringOtherApps is needed alongside it, as in Stack Overflow solutions for Sonoma where plain activation fails inconsistently.
Citations:
- 1: https://developer.apple.com/documentation/appkit/nsrunningapplication/activate(options:)
- 2: https://developer.apple.com/documentation/appkit/nsrunningapplication?changes=_4
- 3: https://developer.apple.com/documentation/appkit/nsapplication/activationoptions/activateignoringotherapps
- 4: https://developer.apple.com/documentation/appkit/nsrunningapplication
- 5: https://developer.apple.com/documentation/appkit/nsapplication?changes=latest_minor
- 6: https://stackoverflow.com/questions/77242593/macos-sonoma-activate-menubar-only-app-when-ui-appears
- 7: https://stackoverflow.com/questions/37558709/why-does-nsrunningapplication-activatewithoptions-return-no
- 8: https://docs.rs/objc2-app-kit/latest/objc2_app_kit/struct.NSApplicationActivationOptions.html
- 9: https://learn.microsoft.com/en-us/dotnet/api/appkit.nsapplicationactivationoptions
🏁 Script executed:
sed -n '2420,2460p' Sources/AppDelegate.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 2041
Fix macOS 14+ activation to reliably bring app to foreground.
The activateUITestAppIfNeeded() function at lines 2447–2451 drops .activateIgnoringOtherApps on macOS 14+, leaving the app backgrounded when another process owns focus. This causes the isActive check at line 2435 to fail repeatedly, exhausting the 20-attempt retry budget in the stabilization loop.
.activateAllWindows alone does not guarantee foreground activation; .activateIgnoringOtherApps is still available and required on macOS 14+ for reliable foreground placement, especially in shared-VM test environments.
Suggested fix
- if `#available`(macOS 14.0, *) {
- NSRunningApplication.current.activate(options: [.activateAllWindows])
- } else {
- NSRunningApplication.current.activate(options: [.activateAllWindows, .activateIgnoringOtherApps])
- }
+ _ = NSRunningApplication.current.activate(
+ options: [.activateAllWindows, .activateIgnoringOtherApps]
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if #available(macOS 14.0, *) { | |
| NSRunningApplication.current.activate(options: [.activateAllWindows]) | |
| } else { | |
| NSRunningApplication.current.activate(options: [.activateAllWindows, .activateIgnoringOtherApps]) | |
| } | |
| _ = NSRunningApplication.current.activate( | |
| options: [.activateAllWindows, .activateIgnoringOtherApps] | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 2447 - 2451, The macOS 14+ branch in
activateUITestAppIfNeeded() drops .activateIgnoringOtherApps causing the app to
remain backgrounded; change the NSRunningApplication.current.activate call so
both .activateAllWindows and .activateIgnoringOtherApps are passed on macOS 14+
as well (i.e., use options: [.activateAllWindows, .activateIgnoringOtherApps]
instead of only .activateAllWindows) so the isActive check can succeed reliably.
There was a problem hiding this comment.
Same as above — Sources/AppDelegate.swift is not changed in this docs PR. This is a pre-existing issue. Tracking separately.
There was a problem hiding this comment.
Regarding NSRunningApplication.current.activate(options:) deprecation on macOS 14+: noted. The activateIgnoringOtherApps option is deprecated but still functional. Will update to use the modern activation API (activate(from: NSRunningApplication?)) in a follow-up.
There was a problem hiding this comment.
Rate Limit Exceeded
@Horacehxw have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 56 seconds before sending another message.
There was a problem hiding this comment.
No code change needed — the activateUITestAppIfNeeded() helper already uses if #available(macOS 14.0, *) { activate(options: [.activateAllWindows]) } (without activateIgnoringOtherApps) on macOS 14+, with the deprecated form only as a fallback for macOS 13 and earlier. So the API usage is already correct per the deprecation guidance.
There was a problem hiding this comment.
@Horacehxw Got it — thanks for clarifying. I’ll leave this as-is for this docs PR and keep your follow-up note about moving to the modern activation API in mind for future reviews.
✏️ Learnings added
Learnt from: Horacehxw
Repo: manaflow-ai/cmux PR: 1980
File: Sources/AppDelegate.swift:2454-2458
Timestamp: 2026-04-06T09:33:37.199Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — In activateUITestAppIfNeeded() for macOS 14+, prefer NSRunningApplication.current.activate(options: [.activateAllWindows]) without .activateIgnoringOtherApps; the deprecated option is only used on older macOS. A future follow-up may migrate to the modern activation API.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Horacehxw
Repo: manaflow-ai/cmux PR: 2043
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-04-06T09:32:23.623Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — UI test fallback window pattern: during XCTest launch stabilization, the app may force-create a fallback main window when NSApp.windows is empty. The window is tracked via `private weak var fallbackUITestWindow: NSWindow?` and, upon the first real WindowGroup registration in `registerMainWindow(...)`, if a second main window exists and the registering window differs, the fallback is closed and the reference cleared to avoid two main windows during tests.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2124
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-25T08:05:26.034Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — Pattern for “New Window” geometry seed:
In AppDelegate.createMainWindow(initialWorkingDirectory:sessionWindowSnapshot:), compute the existingFrame using preferredMainWindowContextForWorkspaceCreation(debugSource: "createMainWindow.initialGeometry") and then resolvedWindow(for:) rather than relying on NSApp.keyWindow or the first registered mainWindowContext. Rationale: ensures the new window inherits size from the intended main-terminal window even when an auxiliary window is key, and keeps behavior consistent with showOpenFolderPanel().
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 2505
File: Sources/AppDelegate.swift:4910-4918
Timestamp: 2026-04-06T02:02:42.414Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — Keyboard repair pattern: In AppDelegate.repairFocusedTerminalKeyboardRoutingIfNeeded(window:event:), determine whether to repair focus by asking the focused terminal’s hosted view if the current first responder already matches that panel’s preferred keyboard target. Implemented via hostedView.responderMatchesPreferredKeyboardFocus(responder) inside responderNeedsFocusedTerminalKeyRepair(_:in:hostedView:). This covers same-window drift to a different Ghostty surface without comparing workspace/panel IDs. Verified by test cmuxTests/AppDelegateShortcutRoutingTests.swift::testWindowSendEventRepairsVisibleSameWindowResponderDriftForFocusedTerminalTyping.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2034
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-25T07:14:56.211Z
Learning: Repo: manaflow-ai/cmux — Sources/AppDelegate.swift — Pattern for “Open Folder”: In AppDelegate.showOpenFolderPanel(), seed NSOpenPanel.directoryURL using preferredMainWindowContextForWorkspaceCreation(debugSource: …) rather than NSApp.keyWindow, and on selection delegate to openWorkspaceForExternalDirectory(workingDirectory:…, debugSource: …). Rationale: handles auxiliary-key-window cases, ensures shouldBringToFront = true, and unifies menu/shortcut behavior with a consistent fallback to createMainWindow when workspace creation returns nil.
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/AppDelegate.swift:2864-2866
Timestamp: 2026-04-04T19:35:02.567Z
Learning: Repo: manaflow-ai/cmux — Sources/AppDelegate.swift — Quit path design: applicationShouldTerminate(_) performs a precautionary save without setting isTerminatingApp (async refresh/write); applicationWillTerminate(_) sets isTerminatingApp = true and then calls saveSessionSnapshot(...), which runs refreshForegroundProcessCacheSync() (2s timeout) and persists synchronously to capture fresh foreground process data.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T00:55:26.574Z
Learning: Applies to **/*.swift : To add a debug toggle or visual option: create an `NSWindowController` subclass with a `shared` singleton, add it to the "Debug Windows" menu in `Sources/cmuxApp.swift`, and add a SwiftUI view with `AppStorage` bindings for live changes
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T00:55:26.574Z
Learning: Applies to **/*.swift : Socket/CLI commands must not steal macOS app focus (no app activation/window raising side effects). Only explicit focus-intent commands may mutate in-app focus/selection (`window.focus`, `workspace.select/next/previous/last`, `surface.focus`, `pane.focus/last`, browser focus commands, and v1 focus equivalents)
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T04:48:00.216Z
Learning: Applies to **/*.swift : Socket/CLI commands must not steal macOS app focus (no app activation/window raising side effects). Only explicit focus-intent commands may mutate in-app focus/selection (`window.focus`, `workspace.select/next/previous/last`, `surface.focus`, `pane.focus/last`, browser focus commands, and v1 focus equivalents).
Learnt from: andrekat
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-29T11:05:01.662Z
Learning: In manaflow-ai/cmux PR `#1773`: `setActiveProfileName(_:)` on `TabManager` (Sources/TabManager.swift) must call `updateWindowTitleForSelectedTab()` explicitly — the save and delete paths do not go through `selectedTabId.didSet`, so without the explicit call the AppKit window title is left stale with the old profile prefix. This was confirmed fixed in commit be9eaa26.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-05T03:20:48.079Z
Learning: Applies to **/*.swift : Socket/CLI commands must not steal macOS app focus; only explicit focus-intent commands may mutate in-app focus/selection
Learnt from: johnhanks1
Repo: manaflow-ai/cmux PR: 1556
File: Sources/AppDelegate.swift:7878-7888
Timestamp: 2026-03-17T01:11:15.590Z
Learning: Repo: manaflow-ai/cmux — Sources/AppDelegate.swift — In AppDelegate.handleCustomShortcut(event:), when detecting an external keyboard input-source change (via KeyboardLayout.id), the skip branch must not update lastKeyboardInputSourceId; it should only log and return false. Update lastKeyboardInputSourceId only after passing the guard into normal shortcut handling. Rationale: prevents same-event re-entry (e.g., via handleBrowserSurfaceKeyEquivalent from NSWindow.performKeyEquivalent) from bypassing the skip and inadvertently processing app shortcuts.
Learnt from: SuperManfred
Repo: manaflow-ai/cmux PR: 803
File: Sources/Panels/BrowserPopupWindowController.swift:36-38
Timestamp: 2026-03-04T05:11:56.373Z
Learning: In Sources/Panels/BrowserPopupWindowController.swift and Sources/Panels/BrowserPanel.swift, `webView.isInspectable = true` (guarded by `#available(macOS 13.3, *)`) is intentionally enabled in all builds — not just DEBUG — because cmux is a developer tool and full Web Inspector access is desired in production builds as well. Do not flag this as a security concern.
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 1909
File: Sources/AppDelegate.swift:1955-1970
Timestamp: 2026-03-21T07:13:41.796Z
Learning: Repo: manaflow-ai/cmux — AppDelegate.registerMainWindow ownership pattern: the primary window registers from ContentView.onAppear and passes the SwiftUI-owned FileBrowserDrawerState; secondary windows created via AppDelegate.createMainWindow construct and pass their own FileBrowserDrawerState. This mirrors SidebarState and avoids re-registration mismatches.
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/ContentView.swift:3194-3196
Timestamp: 2026-03-04T14:05:48.668Z
Learning: In manaflow-ai/cmux (PR `#819`), Sources/ContentView.swift: The command palette’s external window labels intentionally use the global window index from the full orderedSummaries (index + 1), matching the Window menu in AppDelegate. Do not reindex after filtering out the current window to avoid mismatches (“Window 2” for an external window is expected).
Learnt from: apollow
Repo: manaflow-ai/cmux PR: 1089
File: CLI/cmux.swift:462-499
Timestamp: 2026-03-09T02:08:54.956Z
Learning: Repo: manaflow-ai/cmux
PR: `#1089`
File: CLI/cmux.swift
Component: ClaudeHookTagExtractor.extractTags(subtitle:body:)
Learning: For Claude Code session tag extraction, pre-redact sensitive spans (UUIDs, emails, access tokens, filesystem paths, ENV_VAR=..., long numerics) across the combined body+subtitle using unanchored sensitiveSpanPatterns before tokenization. Then tokenize and still filter each token with anchored sensitivePatterns. Rationale: prevents PII/path fragments from slipping into searchable tags after delimiter splitting.
Learnt from: Horacehxw
Repo: manaflow-ai/cmux PR: 2043
File: Resources/bin/claude:0-0
Timestamp: 2026-04-03T07:49:14.139Z
Learning: Repo: manaflow-ai/cmux — In Resources/bin/claude, SELF_DIR is validated with a broad regex `[\"\\' $\`!;|&<>()*?\[\]]` before interpolating it into HOOKS_JSON command strings. Paths that match (unsafe chars including spaces and shell metacharacters) cause the wrapper to fall back to BASE_HOOKS_JSON and emit a warning. This guard-and-fallback is the intentional design; do not suggest adding shell quoting inside the JSON command strings — the guard approach is preferred.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2398
File: CLI/cmux.swift:0-0
Timestamp: 2026-04-01T09:50:23.728Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift within CMUXCLI.buildInteractiveRemoteShellScript(...), never export CMUX_TAB_ID from the workspace UUID. CMUX_TAB_ID must be surface-scoped: only set it when a surface ID is available (map CMUX_TAB_ID to CMUX_SURFACE_ID). Rationale: tab-action/rename-tab resolve CMUX_TAB_ID before CMUX_SURFACE_ID; workspace-scoped values misroute or fail.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2514
File: CLI/cmux.swift:9691-9711
Timestamp: 2026-04-01T22:58:26.254Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift (runClaudeTeams), custom Claude path resolution now trims whitespace and rejects paths that point to the cmux wrapper using isCmuxClaudeWrapper(), before falling back to PATH/bundled. The Resources/bin/claude wrapper also resolves the real path and compares against itself, requiring -f/-x to avoid recursion/self-reference.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2525
File: Sources/GhosttyTerminalView.swift:481-513
Timestamp: 2026-04-02T10:13:39.235Z
Learning: Repo: manaflow-ai/cmux — In Sources/GhosttyTerminalView.swift, terminal file-link resolution trims trailing unmatched closing delimiters “) ] } >” only when they are dangling (more closers than openers), preserving wrapped tokens like “(file:///tmp/a.png)”. Implemented via terminalFileLinkTrailingClosingDelimiters and count comparison inside trimTrailingTerminalFileLinkPunctuation(_:) and exercised by a regression test (PR `#2525`, commit 3f5c5b6d).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-05T21:26:10.684Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift, applySessionPanelMetadata() must gate listeningPorts restoration on the per-panel snapshot.terminal?.isRemoteBacked flag (not workspace-wide remoteTerminalStartupCommand()), so that local panels are always eligible for port restore regardless of current SSH state. Fixed in commit 4d0fd871 (PR `#2545`).
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form `#RRGGBB` (no alpha, i.e., do not allow `#RRGGBBAA`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:3491-3493
Timestamp: 2026-03-04T14:06:12.296Z
Learning: In PRs affecting this repository, limit the scope of localization (i18n) changes to Japanese translations for the file Sources/AppDelegate.swift. Do not include UX enhancements (e.g., preferring workspace.customTitle in workspaceDisplayName() or altering move-target labels) in this PR. Open a separate follow-up issue to address any UX-related changes to avoid scope creep and keep localization review focused.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: CLI/cmux.swift:1958-1960
Timestamp: 2026-03-25T00:33:00.675Z
Learning: Repo: manaflow-ai/cmux — CLI/cmux.swift set-workspace-color intentionally accepts both "#RRGGBB" and "RRGGBB" for ergonomics, matching server-side WorkspaceTabColorSettings.normalizedHex/TabManager normalization. Do not suggest enforcing a mandatory leading '#'; at most, suggest clarifying help/usage text.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2514
File: Sources/GhosttyTerminalView.swift:3759-3761
Timestamp: 2026-04-01T22:57:41.165Z
Learning: Repo: manaflow-ai/cmux — In Sources/cmuxApp.swift, ClaudeCodeIntegrationSettings.customClaudePath(defaults:) trims surrounding whitespace and returns nil for empty/whitespace-only values; callers (e.g., TerminalSurface.createSurface(for:)) can safely set CMUX_CUSTOM_CLAUDE_PATH without additional trimming.
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: jt-hsiao
Repo: manaflow-ai/cmux PR: 1423
File: Sources/AppDelegate.swift:11220-11226
Timestamp: 2026-03-14T07:05:52.379Z
Learning: In Sources/AppDelegate.swift (within the manaflow-ai/cmux repo), ensure that NSWindow.cmux_performKeyEquivalent forwards to firstResponderWebView.performKeyEquivalent and returns its Bool unconditionally. This prevents re-entry into SwiftUI’s performKeyEquivalent path that can swallow keys when WKWebView has focus, and aligns with the Command-key routing chain implemented in Sources/Panels/CmuxWebView.swift: (1) route to NSApp.mainMenu.performKeyEquivalent when allowed, (2) fall back to AppDelegate.shared?.handleBrowserSurfaceKeyEquivalent(event) for non-menu shortcuts, (3) fall back to super.performKeyEquivalent. Non-Command keys should still call super directly.
Learnt from: jt-hsiao
Repo: manaflow-ai/cmux PR: 1423
File: Sources/AppDelegate.swift:11220-11226
Timestamp: 2026-03-14T07:05:52.379Z
Learning: In Sources/AppDelegate.swift for the manaflow-ai/cmux repository, Cmd+` (command-backtick) should not be routed directly to the main menu. Do not implement a window/main-menu bypass for this key. Tests will assert this behavior, so ensure routing logic respects this exclusion and that there is no path that bypasses the main menu for Cmd+`. If similar key-command bypass considerations exist in other files, apply the same explicit exclusion only where applicable.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/GhosttyTerminalView.swift:3891-3903
Timestamp: 2026-04-02T21:37:21.463Z
Learning: In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags (e.g., hasStartupCommand/hasInitialInput), (2) byte counts, and (3) the relevant surface id (so issues can be correlated without exposing the underlying strings).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:35:54.082Z
Learning: In this repo’s keyboard shortcut implementation, ensure `KeyboardShortcutSettings.setShortcut(...)` does nothing (no-op) when `KeyboardShortcutSettings.isManagedBySettingsFile(action)` returns `true` (i.e., the shortcut is managed via `settings.json`). This prevents writing back overrides into `UserDefaults` and keeps `settings.json` as the source of truth.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 2505
File: Sources/AppDelegate.swift:5636-5642
Timestamp: 2026-04-06T07:18:38.685Z
Learning: In the AppDelegate.swift keyDown focus-repair path (and related surface keyboard-focus matching like GhosttySurfaceScrollView.swift), do not dereference NSTextView.delegate when it is unsafe-unretained. Instead, resolve the field-editor ownership via cmuxFieldEditorOwnerView(_). For keyboard-focus matching, prefer superview/nextResponder traversal or hostedView.responderMatchesPreferredKeyboardFocus(...), and ensure any matches are performed using the owned responder chain rather than reading an unsafe delegate pointer.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/examples/cmux-tab-sync.sh (2)
10-11: Consider validatingCMUX_SURFACE_IDsince it's used for the rename operation.The script checks
CMUX_WORKSPACE_IDbut usesCMUX_SURFACE_IDat line 158. While both are set together by cmux (perGhosttyTerminalView.swift:3330-3331), adding a guard forCMUX_SURFACE_IDmakes the script more self-documenting and defensive.🛡️ Suggested fix
[ -z "$CMUX_WORKSPACE_ID" ] && exit 0 +[ -z "$CMUX_SURFACE_ID" ] && exit 0 command -v cmux &>/dev/null || exit 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/cmux-tab-sync.sh` around lines 10 - 11, Add a defensive check for CMUX_SURFACE_ID alongside the existing CMUX_WORKSPACE_ID check: ensure the script verifies that CMUX_SURFACE_ID is non-empty (similar to the existing [ -z "$CMUX_WORKSPACE_ID" ] && exit 0 guard) before proceeding, and exit early if it is unset, since CMUX_SURFACE_ID is later used for the rename operation; update the initial conditional that checks command -v cmux to also validate CMUX_SURFACE_ID to make the script self-documenting and prevent downstream failures.
64-64: File path pattern only matches absolute paths.The regex
(/[a-zA-Z0-9._-]+){2,}requires paths to start with/. Relative paths like./config/secrets.jsonor multi-segment paths likesrc/auth/tokens.pywon't be redacted and could leak into tab names.Based on learnings: "pre-redact sensitive spans... filesystem paths" should cover both absolute and relative paths.
🔒 Suggested fix to also catch relative paths
- text = re.sub(r'(/[a-zA-Z0-9._-]+){2,}', '', text) # file paths + text = re.sub(r'(?:\.{0,2}/)?(?:[a-zA-Z0-9._-]+/){1,}[a-zA-Z0-9._-]+', '', text) # file paths (absolute and relative)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/cmux-tab-sync.sh` at line 64, The current redaction only matches absolute paths in the line with text = re.sub(r'(/[a-zA-Z0-9._-]+){2,}', '', text); update the regex to also match relative paths (e.g., ./, ../, or bare multi-segment paths) by allowing an optional leading ./ or ../ or / and requiring at least two path segments, for example using a pattern like r'(?:(?:\./|\.\./|/)?[A-Za-z0-9._-]+(?:/[A-Za-z0-9._-]+){1,})' (apply this in the same text = re.sub call) so both absolute and relative multi-segment filesystem paths are redacted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/examples/cmux-tab-sync.sh`:
- Around line 10-11: Add a defensive check for CMUX_SURFACE_ID alongside the
existing CMUX_WORKSPACE_ID check: ensure the script verifies that
CMUX_SURFACE_ID is non-empty (similar to the existing [ -z "$CMUX_WORKSPACE_ID"
] && exit 0 guard) before proceeding, and exit early if it is unset, since
CMUX_SURFACE_ID is later used for the rename operation; update the initial
conditional that checks command -v cmux to also validate CMUX_SURFACE_ID to make
the script self-documenting and prevent downstream failures.
- Line 64: The current redaction only matches absolute paths in the line with
text = re.sub(r'(/[a-zA-Z0-9._-]+){2,}', '', text); update the regex to also
match relative paths (e.g., ./, ../, or bare multi-segment paths) by allowing an
optional leading ./ or ../ or / and requiring at least two path segments, for
example using a pattern like
r'(?:(?:\./|\.\./|/)?[A-Za-z0-9._-]+(?:/[A-Za-z0-9._-]+){1,})' (apply this in
the same text = re.sub call) so both absolute and relative multi-segment
filesystem paths are redacted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb3bf2e0-3ab8-428a-8f86-7d340c73c346
📒 Files selected for processing (1)
docs/examples/cmux-tab-sync.sh
There was a problem hiding this comment.
Pull request overview
This PR adds documentation + example Claude Code hook scripts for improved cmux tab/workspace naming, and also includes fixes to improve terminal portal geometry syncing during restore and to stabilize UI test app activation (plus CI retry logic for a known flake).
Changes:
- Add docs and example hook scripts to auto-name tabs from transcript summaries and sync
/renameto thecmuxworkspace name. - Fix restore-time portal geometry issues by scheduling an additional external geometry sync after binding, and add a regression test.
- Improve UI test launch/foreground stabilization in-app, and add a CI retry for a known “Running Background” activation failure.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
Sources/TerminalWindowPortal.swift |
Queues an extra external geometry sync after bind to catch delayed restore/layout shifts. |
Sources/AppDelegate.swift |
Adds retry-based UI test window/foreground stabilization to reduce launch flakes. |
cmuxTests/TerminalAndGhosttyTests.swift |
Adds a lifecycle regression test covering queued layout shifts after bind. |
.github/workflows/ci.yml |
Retries the display-resolution UI test once on a specific activation flake signature. |
docs/claude-code-hooks-integration.md |
Documents hook setup and the naming/sync architecture. |
docs/examples/cmux-tab-sync.sh |
Stop hook: transcript → keyword summary → cmux rename-tab. |
docs/examples/cmux-rename-sync.sh |
UserPromptSubmit hook: /rename → cmux rename-workspace. |
docs/examples/cmux-session-start.sh |
SessionStart hook: reset workspace name to cwd basename. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Session/window restore can queue additional ancestor layout shifts (sidebar width, | ||
| // split positions) after the initial bind tick. Queue a later external sync so the | ||
| // portal catches that settled geometry instead of staying at the seeded frame. | ||
| scheduleExternalGeometrySynchronize() |
There was a problem hiding this comment.
PR title/summary indicate a docs-only change, but this PR also includes functional Swift code changes (portal geometry sync, UI test launch stabilization) and CI workflow tweaks. Please update the PR title and/or top-level description so reviewers and release notes reflect the non-doc behavior changes.
| @@ -0,0 +1,159 @@ | |||
| #!/bin/bash | |||
| # Hook: Sync Claude Code session focus to cmux tab/workspace name | |||
There was a problem hiding this comment.
The file header says this hook syncs the session focus to the "cmux tab/workspace name", but the script only calls cmux rename-tab (workspace renaming is handled elsewhere). Update the comment to avoid misleading readers about what this hook changes.
| # Hook: Sync Claude Code session focus to cmux tab/workspace name | |
| # Hook: Sync Claude Code session focus to cmux tab name |
| TRANSCRIPT=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('transcript_path',''))" 2>/dev/null || true) | ||
| [ -z "$TRANSCRIPT" ] && exit 0 | ||
| [ -f "$TRANSCRIPT" ] || exit 0 | ||
|
|
||
| LABEL=$(_TRANSCRIPT="$TRANSCRIPT" python3 -c " | ||
| import json, re, os, sys, collections | ||
|
|
There was a problem hiding this comment.
LABEL=$( ... python3 -c ... ) isn't guarded. With set -e, if python3 is missing or that command exits non-zero, the hook will exit non-zero and may disrupt Claude Code's hook pipeline. Consider adding command -v python3 >/dev/null || exit 0 and/or appending || true to the label extraction so the hook always no-ops safely.
| # Fast: grep last custom-title from transcript tail | ||
| TITLE=$(tail -500 "$TRANSCRIPT" | grep '"custom-title"' | tail -1 | python3 -c " | ||
| import sys,json | ||
| try: | ||
| print(json.loads(sys.stdin.readline()).get('customTitle','')) | ||
| except: pass | ||
| " 2>/dev/null) |
There was a problem hiding this comment.
This script invokes python3 in the TITLE pipeline without guarding it. Because the script runs with set -e, a missing python3 (or a non-zero exit) will cause the hook to exit non-zero rather than silently no-oping. Add a command -v python3 >/dev/null || exit 0 check and/or make the TITLE extraction tolerant (e.g., ... 2>/dev/null || true).
Address two CodeRabbit review issues (PR manaflow-ai#1980): 1. Replace didRequestFallbackUITestWindow bool with fallbackUITestWindow: NSWindow? Store a weak reference to the force-created window so registerMainWindow can close it when a real WindowGroup window appears, preventing two main windows. 2. Only call moveUITestWindowToTargetDisplayIfNeeded() on attempt == 0 The helper already schedules its own 20-step retry; calling it on every outer loop pass fanned out into overlapping timers that raced each other.
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
…anaflow-ai#1973) * test: cover queued restore-time terminal portal shift * fix: resync terminal portal after restore-time bind
Add documentation and example hook scripts that enhance cmux's Claude Code integration with: - Auto-generated tab names from session transcript (Stop hook) - Instant /rename → workspace name sync (UserPromptSubmit hook) - Workspace name reset on new session (SessionStart hook) These hooks complement cmux's native claude-hook integration by adding concise, keyword-based tab summaries and ensuring /rename changes propagate to cmux immediately.
- Add `|| true` to all python3 command substitutions under `set -e` to prevent hook failures from blocking UserPromptSubmit - Fix misleading "no Python" comment in cmux-rename-sync.sh - Replace f.readlines() with head/tail subprocess for true partial transcript reading (only first 300 + last 200 lines) - Fix word-count cap: 2-5 words (was 2-4), matching documentation
…ction Before tokenizing transcript content for tab naming, strip: - UUIDs (session IDs, request IDs) - Email addresses - File paths (2+ segments) - ENV_VAR=value assignments - Long numeric sequences (6+ digits) This prevents sensitive data from leaking into visible tab names.
Address two CodeRabbit review issues (PR manaflow-ai#1980): 1. Replace didRequestFallbackUITestWindow bool with fallbackUITestWindow: NSWindow? Store a weak reference to the force-created window so registerMainWindow can close it when a real WindowGroup window appears, preventing two main windows. 2. Only call moveUITestWindowToTargetDisplayIfNeeded() on attempt == 0 The helper already schedules its own 20-step retry; calling it on every outer loop pass fanned out into overlapping timers that raced each other.
ea72ddc to
ea32878
Compare
|
Thanks for the review. A few notes: P1 / P2 (set -e + JSON parse failure): Both P3 (transcript memory): The latest version of Also rebased the branch on |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/claude-code-hooks-integration.md (1)
107-115: Consider adding a language specifier to the fenced code block.The architecture diagram would benefit from an explicit language identifier (e.g.,
textorplaintext) to satisfy markdownlint and improve rendering consistency across viewers.📋 Suggested fix
-``` +```text Workspace name (sidebar):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/claude-code-hooks-integration.md` around lines 107 - 115, The fenced code block containing "Workspace name (sidebar):" and "Tab name (tab bar):" lacks a language specifier; update that block (the triple-backtick that opens the architecture diagram) to include a plain-text language identifier such as text or plaintext so markdownlint passes and rendering is consistent (e.g., change ``` to ```text at the start of the block that contains the listed priorities and tab rules).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/claude-code-hooks-integration.md`:
- Around line 107-115: The fenced code block containing "Workspace name
(sidebar):" and "Tab name (tab bar):" lacks a language specifier; update that
block (the triple-backtick that opens the architecture diagram) to include a
plain-text language identifier such as text or plaintext so markdownlint passes
and rendering is consistent (e.g., change ``` to ```text at the start of the
block that contains the listed priorities and tab rules).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6abf04a-5f7b-4ed6-9a0c-0809443d6182
📒 Files selected for processing (7)
Sources/AppDelegate.swiftSources/TerminalWindowPortal.swiftcmuxTests/TerminalAndGhosttyTests.swiftdocs/claude-code-hooks-integration.mddocs/examples/cmux-rename-sync.shdocs/examples/cmux-session-start.shdocs/examples/cmux-tab-sync.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/examples/cmux-session-start.sh
- docs/examples/cmux-rename-sync.sh
- docs/examples/cmux-tab-sync.sh
teamleaderleo
added
area: docs
Summary
Stophook reads session transcript (first 3 user msgs + last 5 msgs), extracts 2-5 keyword summary, sets tab name viacmux rename-tab— gives each tab a distinct focus label instead of mirroring the workspace nameUserPromptSubmithook instantly propagates Claude Code/renameto cmux workspace nameSessionStarthook clears stale workspace names when starting a new sessionMotivation
cmux's native
claude-hookintegration handles workspace naming well via OSC 2, but tab names default to mirroring the workspace name. In multi-tab workspaces (Claude session + helper terminals), this makes tabs indistinguishable. These hooks solve that by auto-generating concise tab summaries from the conversation transcript.Files added
docs/claude-code-hooks-integration.mddocs/examples/cmux-tab-sync.shdocs/examples/cmux-rename-sync.shdocs/examples/cmux-session-start.shHow it works
All hooks are guarded by
[ -z "$CMUX_WORKSPACE_ID" ] && exit 0— they no-op outside cmux.Test plan
~/.claude/hooks/and register in~/.claude/settings.json/rename foo— verify workspace name changes to "foo" immediatelySummary by cubic
Adds Claude Code hook docs and example scripts to auto-name tabs (with privacy-safe tokenization) and instantly sync
/renametocmuxworkspaces. Also resyncs portal geometry after restore-time bind and stabilizes UI test launch by using a fallback window and avoiding overlapping timers.New Features
docs/claude-code-hooks-integration.mdand example hooks:cmux-tab-sync.sh(Stop): summarizes transcript to rename the tab.cmux-rename-sync.sh(UserPromptSubmit): syncs/renameto workspace name.cmux-session-start.sh(SessionStart): resets workspace name on new session.cmux.head/tail, guardpython3with|| true, cap tab labels to 2–5 words, and redact UUIDs/emails/paths/ENV assignments/long numbers before tokenization.Bug Fixes
Written for commit ea32878. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests