feat: AI-powered tab & workspace auto-naming for Claude Code#2043
Open
Horacehxw wants to merge 11 commits intomanaflow-ai:mainfrom
Open
feat: AI-powered tab & workspace auto-naming for Claude Code#2043Horacehxw wants to merge 11 commits intomanaflow-ai:mainfrom
Horacehxw wants to merge 11 commits intomanaflow-ai:mainfrom
Conversation
|
@Horacehxw 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 AI-driven cmux tab/workspace naming via three new hook scripts and updates the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClaudeWrapper as Claude Wrapper (shell)
participant SessionHook as SessionStart Hook
participant StopHook as Stop Hook
participant RenameHook as UserPromptSubmit Hook
participant BackgroundClaude as Background Claude
participant cmux
participant FS as /tmp Files
User->>ClaudeWrapper: invoke `claude`
ClaudeWrapper->>SessionHook: run SessionStart hook (absolute path)
SessionHook->>cmux: cmux rename-tab / cmux rename-workspace (maybe)
SessionHook->>FS: write workspace-owner marker
User->>ClaudeWrapper: send messages / conversation
ClaudeWrapper->>StopHook: run Stop hook on turn end
StopHook->>StopHook: extract transcript context & create prompt
StopHook->>BackgroundClaude: spawn detached `claude -p --model haiku`
BackgroundClaude->>FS: write pending AI label and cache
User->>ClaudeWrapper: next user prompt
ClaudeWrapper->>RenameHook: run UserPromptSubmit hook
RenameHook->>FS: read pending AI label / custom-title
RenameHook->>cmux: cmux rename-tab and maybe cmux rename-workspace
RenameHook->>FS: persist cache/markers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 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/ai-tab-naming.md">
<violation number="1" location="docs/ai-tab-naming.md:46">
P2: The docs claim the workspace-owner file resets when all sessions end, but the implementation never removes `/tmp/cmux-ws-owner-{WORKSPACE_ID}`. The session-start hook only creates it if missing and other hooks only read it, so stale owner state can persist across sessions and prevent new owners from taking over workspace naming.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
Greptile SummaryThis PR adds three shell hooks (
The Swift and CI changes are clean and well-tested. Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant W as claude wrapper
participant SN as cmux-session-namer.sh
participant TN as cmux-tab-namer.sh (Stop)
participant RN as cmux-rename-namer.sh
participant BG as Background Process
participant AI as claude -p haiku
participant C as cmux CLI
U->>W: claude (new session)
W->>SN: SessionStart hook
SN->>C: rename-workspace (basename)
SN-->>C: write /tmp/cmux-ws-owner-WS_ID (if new)
Note over U,C: User works, Claude responds...
W->>TN: Stop hook (Turn N)
TN->>TN: Phase 1 — apply pending label (none yet on first stop)
TN->>BG: spawn background (disown)
BG->>AI: claude -p --model haiku (⚠️ recurses through wrapper)
AI-->>BG: "Fix login bug"
BG-->>TN: write /tmp/cmux-tab-pending-SURFACE_ID
W->>TN: Stop hook (Turn N+1)
TN->>TN: Phase 1 — read pending file
TN->>C: rename-tab "Fix login bug"
TN->>C: rename-workspace "Fix login bug" (if owner)
TN->>BG: spawn new background for Turn N+2
U->>W: /rename "My Session"
W->>RN: UserPromptSubmit hook
RN->>RN: grep transcript for custom-title
RN->>C: rename-tab "My Session"
RN->>C: rename-workspace "My Session" (if owner)
Note over TN: Phase 2 skips (HAS_CUSTOM_TITLE > 0)<br/>⚠️ Only if within last 500 lines
Reviews (1): Last reviewed commit: "feat: AI-powered tab & workspace auto-na..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+31
to
+36
| # Set project basename as initial workspace name (overridden by AI summary after first response) | ||
| CWD=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('cwd',''))" 2>/dev/null || true) | ||
| if [ -n "$CWD" ]; then | ||
| BASENAME=$(basename "$CWD") | ||
| cmux rename-workspace --workspace "$CMUX_WORKSPACE_ID" "$BASENAME" 2>/dev/null || true | ||
| fi |
Contributor
There was a problem hiding this comment.
cmux rename-workspace is called on every SessionStart, regardless of whether this surface is the workspace owner. If Session A (the owner) has already acquired an AI-generated workspace name like "Fix login bug", and Session B starts in the same workspace, the workspace name will flash back to the project basename before Session B's first Stop fires.
The rename should be guarded by the same ownership check used elsewhere:
Suggested change
| # Set project basename as initial workspace name (overridden by AI summary after first response) | |
| CWD=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('cwd',''))" 2>/dev/null || true) | |
| if [ -n "$CWD" ]; then | |
| BASENAME=$(basename "$CWD") | |
| cmux rename-workspace --workspace "$CMUX_WORKSPACE_ID" "$BASENAME" 2>/dev/null || true | |
| fi | |
| # Set project basename as initial workspace name (only if this tab owns the workspace) | |
| WS_OWNER_FILE="/tmp/cmux-ws-owner-${CMUX_WORKSPACE_ID}" | |
| WS_OWNER=$(cat "$WS_OWNER_FILE" 2>/dev/null || true) | |
| if [ "$WS_OWNER" = "$CMUX_SURFACE_ID" ] && [ -n "$CWD" ]; then | |
| BASENAME=$(basename "$CWD") | |
| cmux rename-workspace --workspace "$CMUX_WORKSPACE_ID" "$BASENAME" 2>/dev/null || true | |
| fi |
Without this guard, the workspace naming priority documented in the PR (/rename > AI summary > basename) is broken for multi-session workspaces.
Author
There was a problem hiding this comment.
Fixed in 8fb88cf — cmux rename-workspace is now only called when WS_OWNER == CMUX_SURFACE_ID, both in the custom-title sync path (line 70) and the basename-set path (line 87). Non-owner sessions touch only their own tab name.
Comment on lines
+136
to
+145
| # Spawn background: generate AI label → write to pending file for next Stop | ||
| ( | ||
| set +e | ||
| LABEL=$(perl -e 'alarm 15; exec @ARGV' claude -p --model haiku < "$PROMPT_FILE" 2>/dev/null | head -1 | cut -c1-30) | ||
| rm -f "$PROMPT_FILE" 2>/dev/null | ||
| if [ -n "$LABEL" ]; then | ||
| printf '%s\n%s\n%s\n' "$LABEL" "$LINE_COUNT" "$HAS_CUSTOM_TITLE" > "$TAB_PENDING" | ||
| fi | ||
| ) &>/dev/null & | ||
| disown |
Contributor
There was a problem hiding this comment.
claude -p call recurses through the wrapper
The claude command on line 139 resolves to the cmux claude wrapper (same directory is first in $PATH), not the real claude binary. When the wrapper fires for the background claude -p --model haiku, it executes the SessionStart hooks including cmux-session-namer.sh, which:
- Deletes
TAB_CACHEfor$CMUX_SURFACE_ID— erasing the diff-count from the actual session - Unconditionally renames the workspace back to the project basename (see separate comment)
The Stop hook also fires for the inner call, but the short transcript (< 50 lines) prevents another Phase 2 spawn. The real fix is to call the real claude binary directly. The wrapper already has find_real_claude but it's only callable from within the wrapper itself. The simplest approach is to export the real path:
# In Resources/bin/claude (after REAL_CLAUDE is resolved):
export CMUX_REAL_CLAUDE="$REAL_CLAUDE"Then in cmux-tab-namer.sh, use ${CMUX_REAL_CLAUDE:-claude} instead of claude:
LABEL=$(perl -e 'alarm 15; exec @ARGV' "${CMUX_REAL_CLAUDE:-claude}" -p --model haiku < "$PROMPT_FILE" 2>/dev/null | head -1 | cut -c1-30)
Author
There was a problem hiding this comment.
Fixed in 22de633 — the background subshell now explicitly unset CMUX_WORKSPACE_ID CMUX_SURFACE_ID CMUX_TAB_ID before invoking claude -p. Without those env vars the wrapper's IN_CMUX check fails and it passes straight through to the real claude binary without injecting any hooks.
Comment on lines
+63
to
+65
| # Skip if user has /renamed this session | ||
| HAS_CUSTOM_TITLE=$(tail -500 "$TRANSCRIPT" | grep -c '"custom-title"' 2>/dev/null; true) | ||
| [ "${HAS_CUSTOM_TITLE:-0}" -gt 0 ] 2>/dev/null && exit 0 |
Contributor
There was a problem hiding this comment.
/rename detection only covers the last 500 lines
tail -500 may not reach the "custom-title" entry for a long conversation. If a user ran /rename "My Session" early on and the transcript has grown past 500 lines since then, the check evaluates to 0, re-enables Phase 2, and the next Stop will overwrite the user's custom title with a fresh AI summary. This breaks the documented invariant: "Respects /rename custom titles — AI naming stops when user explicitly names a session".
A simple fix is to scan the full transcript (or at least a larger window). A cheap alternative is to check from the beginning as well:
HAS_CUSTOM_TITLE=$(grep -c '"custom-title"' "$TRANSCRIPT" 2>/dev/null || true)Using grep -c directly on the file avoids the pipe and is not meaningfully slower on typical transcript sizes.
Author
There was a problem hiding this comment.
Fixed in b1b0f8b — replaced the tail-500 approach with a per-surface marker file (/tmp/cmux-custom-title-${CMUX_SURFACE_ID}). On SessionStart, cmux-session-namer.sh scans the entire transcript for custom-title and writes the marker. On Stop, cmux-tab-namer.sh checks the marker first (fast, no size limit) before doing the tail-500 scan for titles set in the current session.
Comment on lines
+26
to
+29
| WS_OWNER_FILE="/tmp/cmux-ws-owner-${CMUX_WORKSPACE_ID}" | ||
| if [ ! -f "$WS_OWNER_FILE" ]; then | ||
| echo "$CMUX_SURFACE_ID" > "$WS_OWNER_FILE" | ||
| fi |
Contributor
There was a problem hiding this comment.
/tmp/cmux-ws-owner-${CMUX_WORKSPACE_ID} (and the per-surface tab-cache, tab-pending, tab-prompt files) are written but never removed. The docs state the owner file "resets when all sessions in the workspace end", but there is no hook — SessionEnd only calls cmux claude-hook session-end, not any of the new scripts. Over time these files accumulate in /tmp.
A SessionEnd hook in a new cmux-session-ender.sh (or inline in the existing hook dispatch) could remove the owner file when the last session for a workspace exits and clean up the per-surface files. At minimum, the documentation should be corrected to reflect the current (no-cleanup) behaviour.
Author
There was a problem hiding this comment.
Addressed in be922f6. We intentionally don't clean up the owner file on exit (no reliable exit hook) — instead, stale-owner recovery runs on every SessionStart: cmux list-surfaces --workspace is called to verify the recorded owner is still active, and the file is deleted if not. This covers the case where the original owner tab closes without triggering cleanup.
| # - PreToolUse: clears "Needs input" when Claude resumes after permission grant (async to avoid latency) | ||
| HOOKS_JSON='{"hooks":{"SessionStart":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook session-start","timeout":10}]}],"Stop":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook stop","timeout":10}]}],"SessionEnd":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook session-end","timeout":1}]}],"Notification":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook notification","timeout":10}]}],"UserPromptSubmit":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook prompt-submit","timeout":10}]}],"PreToolUse":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook pre-tool-use","timeout":5,"async":true}]}]}}' | ||
| # - Tab/workspace naming hooks: AI-powered auto-naming from conversation content | ||
| HOOKS_JSON='{"hooks":{"SessionStart":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook session-start","timeout":10},{"type":"command","command":"'"$SELF_DIR"'/cmux-session-namer.sh","timeout":10}]}],"Stop":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook stop","timeout":10},{"type":"command","command":"'"$SELF_DIR"'/cmux-tab-namer.sh","timeout":10}]}],"SessionEnd":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook session-end","timeout":1}]}],"Notification":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook notification","timeout":10}]}],"UserPromptSubmit":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook prompt-submit","timeout":10},{"type":"command","command":"'"$SELF_DIR"'/cmux-rename-namer.sh","timeout":10}]}],"PreToolUse":[{"matcher":"","hooks":[{"type":"command","command":"cmux claude-hook pre-tool-use","timeout":5,"async":true}]}]}}' |
Contributor
There was a problem hiding this comment.
$SELF_DIR injected into JSON without escaping
$SELF_DIR is interpolated directly into the JSON string literal. If the installation path contains a double-quote or backslash (e.g. a path with \" from a symlink target), the resulting JSON would be malformed and --settings would fail silently, disabling all hooks.
Consider JSON-encoding the path before interpolation, or at minimum asserting that the path contains only safe characters:
# Guard against characters that would break the inline JSON
if [[ "$SELF_DIR" =~ [\"\\] ]]; then
echo "cmux: warning: wrapper directory path contains characters unsafe for JSON: $SELF_DIR" >&2
fiFor a proper fix, build the JSON with a tool that handles escaping (e.g. python3 -c "import json, sys; ...") rather than inline string interpolation.
Author
There was a problem hiding this comment.
Fixed in e7c8694 / 4a358ca — $SELF_DIR is now validated against a regex that rejects paths containing ", \, ', space, $, , !, ;, |, &, <, >, (, ), *, ?, [. If the path fails, the wrapper falls back to BASE_HOOKS_JSON(which usescmux` from PATH) and emits a warning to stderr. This way the naming add-on is simply skipped rather than injecting a broken command string.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
Resources/bin/cmux-tab-namer.sh (1)
139-139: Consider adding a comment explaining the Perl alarm timeout.The
perl -e 'alarm 15; exec@argv'idiom is a clever POSIX-compatible timeout, but it's not immediately obvious to readers unfamiliar with this pattern. A brief comment would aid maintainability.📝 Suggested comment
+ # Timeout after 15s using perl alarm (POSIX-compatible, no external deps) LABEL=$(perl -e 'alarm 15; exec `@ARGV`' claude -p --model haiku < "$PROMPT_FILE" 2>/dev/null | head -1 | cut -c1-30)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/bin/cmux-tab-namer.sh` at line 139, Add a short inline comment next to the LABEL assignment that explains the Perl timeout idiom used: perl -e 'alarm 15; exec `@ARGV`' (it runs the given command with a 15s alarm to kill it if it hangs), and note that this is a POSIX-compatible way to implement a simple timeout for the claude invocation in the LABEL=$(...) pipeline so future readers understand why the alarm is used and its effect on the claude -p --model haiku call.Resources/bin/cmux-rename-namer.sh (1)
27-32: Bareexcept: passswallows parsing errors silently.The Python snippet catches all exceptions without logging, making debugging difficult if the transcript format changes or JSON is malformed.
Consider at minimum logging to stderr or being more specific about the exception type.
♻️ Suggested improvement
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 +except Exception: + pass # Malformed JSON or missing field is expected for non-rename entries " 2>/dev/null)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/bin/cmux-rename-namer.sh` around lines 27 - 32, The current assignment to TITLE uses an inlined Python snippet that swallows all exceptions via a bare except in the python3 call; update that snippet to catch specific exceptions (e.g., json.JSONDecodeError, IndexError, KeyError) and emit an error message to stderr instead of silently passing so parsing problems are visible, ensuring the TITLE pipeline still exits cleanly or returns an empty string on error; locate the block that defines TITLE (the tail|grep|python3 pipeline) and replace the bare except with targeted exception handlers that print diagnostic text to stderr (using sys.stderr) and then safely return an empty string.docs/ai-tab-naming.md (1)
63-69: Document cleanup strategy for/tmpfiles.The documentation lists temp files but doesn't mention when they're cleaned up. The scripts don't appear to have cleanup logic for stale files (e.g., when cmux or a workspace closes).
Line 46 states the owner "resets when all sessions in the workspace end," but I don't see corresponding cleanup code in the hook scripts.
Consider adding a note about cleanup expectations or implementing
SessionEndcleanup logic.Would you like me to draft a cleanup hook or document the expected cleanup behavior?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ai-tab-naming.md` around lines 63 - 69, Docs list several /tmp files (e.g., /tmp/cmux-tab-pending-{SURFACE_ID}, /tmp/cmux-tab-cache-{SURFACE_ID}, /tmp/cmux-tab-prompt-{SURFACE_ID}, /tmp/cmux-ws-owner-{WORKSPACE_ID}) but do not state cleanup behavior; add either (A) a short "Cleanup policy" section in docs stating when files are removed and who is responsible, or (B) implement SessionEnd cleanup in the hook scripts: add a SessionEnd handler that deletes /tmp/cmux-tab-*, /tmp/cmux-ws-owner-{WORKSPACE_ID} for a workspace when the last session ends and ensure cmux-ws-owner is reset, and update the docs to reference this SessionEnd behavior; place logic alongside existing hook handlers so it runs reliably when sessions close.Resources/bin/cmux-session-namer.sh (1)
26-29: TOCTOU race in workspace owner registration.The check-then-write pattern (
[ ! -f ... ]; echo ... > file) is not atomic. Two concurrentSessionStarthooks could both pass the existence check and overwrite each other's owner registration.This is low severity since "first session wins" is approximate and the window is small, but you could use a lock or atomic file creation:
♻️ Atomic alternative using exclusive creation
# Register as workspace owner if no owner exists yet (first session wins). # The workspace owner's AI summary controls the workspace name. WS_OWNER_FILE="/tmp/cmux-ws-owner-${CMUX_WORKSPACE_ID}" -if [ ! -f "$WS_OWNER_FILE" ]; then - echo "$CMUX_SURFACE_ID" > "$WS_OWNER_FILE" -fi +# Use set -o noclobber with >| fallback for atomic first-write-wins +(set -o noclobber; echo "$CMUX_SURFACE_ID" > "$WS_OWNER_FILE") 2>/dev/null || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/bin/cmux-session-namer.sh` around lines 26 - 29, The current check-then-write for WS_OWNER_FILE (using CMUX_WORKSPACE_ID and CMUX_SURFACE_ID) is racy; replace the existence check and naive echo with an atomic creation attempt so only the first concurrent SessionStart wins. Update the logic in cmux-session-namer.sh to attempt an exclusive create (for example use atomic file creation via noclobber redirection or an atomic mkdir/ln-based lock) and only write CMUX_SURFACE_ID when that exclusive create succeeds; if the create fails because the file already exists, do nothing..github/workflows/ci.yml (1)
513-520: Use multiline-aware pattern matching for XCTest activation error detection.The
greppattern at line 513 fails to match if XCTest prints"Failed to activate application"and"Running Background"on separate lines. Replace the grep with a multiline-aware regex that reads from the saved log file directly, avoiding line-oriented matching limitations and simplifying the logic by eliminating the redundantOUTPUTvariable assignments.🔧 Fix
set +e run_display_resolution_ui_test | tee /tmp/display-resolution-ui-test-output.txt EXIT_CODE=${PIPESTATUS[0]} - OUTPUT=$(cat /tmp/display-resolution-ui-test-output.txt) set -e - if [ "$EXIT_CODE" -ne 0 ] && echo "$OUTPUT" | grep -q "Failed to activate application.*Running Background"; then + if [ "$EXIT_CODE" -ne 0 ] && perl -0ne 'exit(!/Failed to activate application.*Running Background/s)' /tmp/display-resolution-ui-test-output.txt; then echo "Display resolution UI regression hit launch activation flake, retrying once" pkill -x "cmux DEV" || true sleep 2 set +e run_display_resolution_ui_test | tee /tmp/display-resolution-ui-test-output.txt EXIT_CODE=${PIPESTATUS[0]} - OUTPUT=$(cat /tmp/display-resolution-ui-test-output.txt) set -e fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 513 - 520, The current conditional uses echo "$OUTPUT" | grep which fails when "Failed to activate application" and "Running Background" appear on separate lines; update the retry check to read the saved log file (/tmp/display-resolution-ui-test-output.txt) and use a multiline-aware regex (e.g., pcregrep -M or grep -Pz with a pattern matching the two phrases across lines) to detect the XCTest activation error, change the condition that references EXIT_CODE and the grep to operate against that file, and remove the now-redundant OUTPUT variable assignment and the echo pipe; keep the existing run_display_resolution_ui_test invocation and EXIT_CODE capture but base the retry decision on the multiline log scan instead of line-oriented OUTPUT matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/bin/cmux-tab-namer.sh`:
- Around line 136-145: The background worker calling "claude -p" can trigger
recursive naming hooks because it inherits CMUX_SURFACE_ID and
CMUX_WORKSPACE_ID; modify the subshell that sets LABEL (the block invoking
claude -p and writing to "$TAB_PENDING") to unset CMUX_SURFACE_ID and
CMUX_WORKSPACE_ID (or set CMUX_TAB_NAMER_DISABLED=1) in its environment before
running claude so the wrapper cannot re-inject naming hooks, leaving the rest of
the logic that reads PROMPT_FILE, computes LABEL, and writes "$TAB_PENDING"
unchanged.
In `@Sources/AppDelegate.swift`:
- Around line 2425-2439: The problem is that
stabilizeUITestLaunchWindowAndForeground(attempt:) calls
moveUITestWindowToTargetDisplayIfNeeded() on every retry, which starts its own
20-attempt reschedule and causes overlapping timers; change
stabilizeUITestLaunchWindowAndForeground to only invoke
moveUITestWindowToTargetDisplayIfNeeded when starting the outer chain (e.g.,
when attempt == 0) or gate the call behind a boolean flag so subsequent retries
skip it, keeping the rest of the retry logic (activateUITestAppIfNeeded,
diagnostics write, and the DispatchQueue reschedule) unchanged; reference the
functions stabilizeUITestLaunchWindowAndForeground(attempt:),
moveUITestWindowToTargetDisplayIfNeeded(), and activateUITestAppIfNeeded() when
making the change.
- Line 2081: The code currently only tracks a boolean
didRequestFallbackUITestWindow and creates a forced AppKit window without
keeping a handle; change this to store and manage the actual fallback window (or
its windowId) so it can be closed or reused when the real launch WindowGroup
appears. Add an optional property (e.g., fallbackUITestWindow: NSWindow? or
fallbackUITestWindowId: String?) in AppDelegate, update the helper that
force-creates the window (the code path around the existing
didRequestFallbackUITestWindow usage and the block that creates the AppKit
window) to assign the created window/windowId to that property, and modify the
window-registration path that detects the real launch window to check for and
close/reuse the saved fallback window (or clear the stored id) to avoid leaving
two main windows alive. Ensure all references to didRequestFallbackUITestWindow
are replaced or augmented to use the new stored handle for cleanup/reuse.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 513-520: The current conditional uses echo "$OUTPUT" | grep which
fails when "Failed to activate application" and "Running Background" appear on
separate lines; update the retry check to read the saved log file
(/tmp/display-resolution-ui-test-output.txt) and use a multiline-aware regex
(e.g., pcregrep -M or grep -Pz with a pattern matching the two phrases across
lines) to detect the XCTest activation error, change the condition that
references EXIT_CODE and the grep to operate against that file, and remove the
now-redundant OUTPUT variable assignment and the echo pipe; keep the existing
run_display_resolution_ui_test invocation and EXIT_CODE capture but base the
retry decision on the multiline log scan instead of line-oriented OUTPUT
matching.
In `@docs/ai-tab-naming.md`:
- Around line 63-69: Docs list several /tmp files (e.g.,
/tmp/cmux-tab-pending-{SURFACE_ID}, /tmp/cmux-tab-cache-{SURFACE_ID},
/tmp/cmux-tab-prompt-{SURFACE_ID}, /tmp/cmux-ws-owner-{WORKSPACE_ID}) but do not
state cleanup behavior; add either (A) a short "Cleanup policy" section in docs
stating when files are removed and who is responsible, or (B) implement
SessionEnd cleanup in the hook scripts: add a SessionEnd handler that deletes
/tmp/cmux-tab-*, /tmp/cmux-ws-owner-{WORKSPACE_ID} for a workspace when the last
session ends and ensure cmux-ws-owner is reset, and update the docs to reference
this SessionEnd behavior; place logic alongside existing hook handlers so it
runs reliably when sessions close.
In `@Resources/bin/cmux-rename-namer.sh`:
- Around line 27-32: The current assignment to TITLE uses an inlined Python
snippet that swallows all exceptions via a bare except in the python3 call;
update that snippet to catch specific exceptions (e.g., json.JSONDecodeError,
IndexError, KeyError) and emit an error message to stderr instead of silently
passing so parsing problems are visible, ensuring the TITLE pipeline still exits
cleanly or returns an empty string on error; locate the block that defines TITLE
(the tail|grep|python3 pipeline) and replace the bare except with targeted
exception handlers that print diagnostic text to stderr (using sys.stderr) and
then safely return an empty string.
In `@Resources/bin/cmux-session-namer.sh`:
- Around line 26-29: The current check-then-write for WS_OWNER_FILE (using
CMUX_WORKSPACE_ID and CMUX_SURFACE_ID) is racy; replace the existence check and
naive echo with an atomic creation attempt so only the first concurrent
SessionStart wins. Update the logic in cmux-session-namer.sh to attempt an
exclusive create (for example use atomic file creation via noclobber redirection
or an atomic mkdir/ln-based lock) and only write CMUX_SURFACE_ID when that
exclusive create succeeds; if the create fails because the file already exists,
do nothing.
In `@Resources/bin/cmux-tab-namer.sh`:
- Line 139: Add a short inline comment next to the LABEL assignment that
explains the Perl timeout idiom used: perl -e 'alarm 15; exec `@ARGV`' (it runs
the given command with a 15s alarm to kill it if it hangs), and note that this
is a POSIX-compatible way to implement a simple timeout for the claude
invocation in the LABEL=$(...) pipeline so future readers understand why the
alarm is used and its effect on the claude -p --model haiku call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d161220d-69a5-46eb-a701-711ca5a7562e
📒 Files selected for processing (9)
.github/workflows/ci.ymlResources/bin/claudeResources/bin/cmux-rename-namer.shResources/bin/cmux-session-namer.shResources/bin/cmux-tab-namer.shSources/AppDelegate.swiftSources/TerminalWindowPortal.swiftcmuxTests/TerminalAndGhosttyTests.swiftdocs/ai-tab-naming.md
5 tasks
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Resources/bin/cmux-rename-namer.sh (1)
63-69: Consider edge case: multiple JSON keys containing"custom-title"substring.The grep pattern
'"custom-title"'could match JSON lines where this string appears in values or other contexts, not just as the type field. In practice this is unlikely given the Claude transcript format, but a more precise match could use'"type":"custom-title"'.♻️ More precise grep pattern
-TITLE=$(tail -500 "$TRANSCRIPT" | grep '"custom-title"' | tail -1 | python3 -c " +TITLE=$(tail -500 "$TRANSCRIPT" | grep '"type":"custom-title"' | tail -1 | python3 -c "🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/bin/cmux-rename-namer.sh` around lines 63 - 69, The current TITLE extraction (variable TITLE using tail/grep on TRANSCRIPT) can match any occurrence of "custom-title" in the line; update the grep to match the JSON key/value pair precisely (for example use '"type":"custom-title"' instead of '"custom-title"') so it only selects lines where the type field equals custom-title, keeping the existing tail and python JSON load steps (i.e., modify the grep pattern in the TITLE assignment that reads from "$TRANSCRIPT" to a more specific '"type":"custom-title"' match).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Resources/bin/cmux-rename-namer.sh`:
- Around line 63-69: The current TITLE extraction (variable TITLE using
tail/grep on TRANSCRIPT) can match any occurrence of "custom-title" in the line;
update the grep to match the JSON key/value pair precisely (for example use
'"type":"custom-title"' instead of '"custom-title"') so it only selects lines
where the type field equals custom-title, keeping the existing tail and python
JSON load steps (i.e., modify the grep pattern in the TITLE assignment that
reads from "$TRANSCRIPT" to a more specific '"type":"custom-title"' match).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c55c8f4-78b2-4d18-a87b-a867ce17b567
📒 Files selected for processing (3)
Resources/bin/cmux-rename-namer.shResources/bin/cmux-session-namer.shResources/bin/cmux-tab-namer.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- Resources/bin/cmux-session-namer.sh
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
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/ai-tab-naming.md">
<violation number="1" location="docs/ai-tab-naming.md:47">
P2: Persistent workspace-owner file can become stale and prevent any active session from updating workspace names.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/bin/cmux-session-namer.sh`:
- Around line 25-28: The current WS_OWNER_FILE write is TOCTOU; make claiming
atomic by using an atomic lock+rename pattern: attempt to create a lock
directory with mkdir (e.g. mkdir "$WS_OWNER_FILE.lock") which is atomic; only
the process that succeeds should create a temporary file containing
CMUX_SURFACE_ID (e.g. "$WS_OWNER_FILE.$$"), atomically mv/rename that temp file
to WS_OWNER_FILE, then rmdir the lock; if mkdir fails, do nothing (another
session already claimed). Update the code around WS_OWNER_FILE /
CMUX_WORKSPACE_ID / CMUX_SURFACE_ID to use this mkdir-tempfile-mv-rmdir sequence
so the first SessionStart wins.
- Around line 55-57: The SessionStart hook in cmux-session-namer.sh currently
removes CUSTOM_MARKER and the tab-cache but not the pending label; add removal
of /tmp/cmux-tab-pending-${CMUX_SURFACE_ID} (same pattern as the other rm -f
calls) so stale pending labels are cleared on start, or alternatively add
validation in cmux-rename-namer.sh where UserPromptSubmit applies a pending
label: check the stored session ID (or CMUX_SURFACE_ID) against the current
session before applying the pending file to avoid applying orphaned labels.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f53ff2d3-2460-48f2-a0a1-e525dc0a3229
📒 Files selected for processing (2)
Resources/bin/claudeResources/bin/cmux-session-namer.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- Resources/bin/claude
…anaflow-ai#1973) * test: cover queued restore-time terminal portal shift * fix: resync terminal portal after restore-time bind
Add three shell hooks that automatically name cmux tabs and workspaces based on Claude Code conversation content using AI-generated summaries (via `claude -p --model haiku`). ## Problem When running multiple Claude Code sessions across cmux tabs, all tabs show generic names (directory or command), making it hard to distinguish which tab is working on what task. ## Solution Three hooks injected by the claude wrapper alongside existing lifecycle hooks: - **cmux-session-namer.sh** (SessionStart): Registers workspace owner (first session) and sets project basename as initial workspace name. - **cmux-tab-namer.sh** (Stop): Two-phase AI naming: Phase 1 (foreground): Apply pending label from previous turn via cmux rename-tab/rename-workspace (has socket access). Phase 2 (background): Spawn `claude -p --model haiku` to generate 2-5 word summary in conversation language, write to pending file. - **cmux-rename-namer.sh** (UserPromptSubmit): Sync /rename custom titles to tab name; workspace only if tab is the owner. ## Key design decisions - Two-phase avoids blocking hooks with slow AI calls (~5-10s) and avoids cmux socket errors from detached background processes - Per-surface file keys prevent cross-tab label contamination - Workspace owner model: first session controls workspace name - /rename custom titles take absolute precedence over AI naming - Skip transcripts <50 lines (subagent sessions) - Disable via CMUX_TAB_NAMER_DISABLED=1
Three improvements to the AI tab naming hooks:
1. **Faster tab label apply (UserPromptSubmit instead of Stop)**
Phase 1 (apply pending label) moved from cmux-tab-namer.sh (Stop hook)
to cmux-rename-namer.sh (UserPromptSubmit hook). Tab names now update
the moment the user sends their next message, not when Claude finishes
responding — typically saving the entire response wait time.
2. **Fix: custom-title ignored on resumed long sessions**
When a session with /rename was resumed, cmux-session-namer.sh would
reset the workspace name to basename and clear the cache, then the Stop
hook's tail-500 scan would miss the /rename entry (which may be 1000+
lines back in the transcript), allowing AI summaries to overwrite the
custom title.
Fix: cmux-session-namer.sh now greps the *entire* transcript for
custom-title on SessionStart. If found, it syncs the title immediately
and writes a marker file (/tmp/cmux-custom-title-{SURFACE_ID}) that
suppresses AI summary generation without needing tail-500.
3. **Marker file written on /rename**
cmux-rename-namer.sh writes the same marker file when /rename is used,
so the protection persists even after the /rename entry scrolls beyond
the tail-500 window in very long sessions.
Marker file lifecycle:
Written: SessionStart (found existing custom-title) or /rename issued
Checked: cmux-tab-namer.sh Stop hook (before generating AI summary)
Cleared: SessionStart when no custom-title exists (fresh session)
Address CodeRabbit and cubic review feedback: 1. **Fix recursive hook invocation (CodeRabbit, Major)** The background `claude -p` subprocess inherited CMUX_WORKSPACE_ID, CMUX_SURFACE_ID, and CMUX_TAB_ID, causing the claude wrapper to re-inject naming hooks into the subprocess — potentially clearing the tab cache or writing conflicting pending files. Fix: `unset CMUX_WORKSPACE_ID CMUX_SURFACE_ID CMUX_TAB_ID` inside the subshell before calling `claude -p --model haiku`. 2. **More precise grep pattern for custom-title (CodeRabbit, nitpick)** Changed `grep '"custom-title"'` → `grep '"type":"custom-title"'` across all three hooks to avoid false matches where the string "custom-title" appears in other JSON values. 3. **Correct docs on workspace owner file lifetime (cubic)** The docs stated the owner file "resets when all sessions end" but the code never removes it. Updated to accurately state it persists in /tmp/ until OS clears it, and first-session-wins applies for the lifetime of that file. 4. **Update docs to reflect UserPromptSubmit apply-phase** Section 2 and Section 3 updated to document the current design where label apply happens at UserPromptSubmit (not Stop) and the custom-title marker file mechanism.
Address greptile P1/P2 review findings: 1. **Non-owner sessions unconditionally reset workspace name (greptile P1)** cmux-session-namer.sh called `cmux rename-workspace` on every SessionStart regardless of ownership. If Session A (owner) already had a good AI-generated workspace name, Session B starting in the same workspace would flash the name back to its project basename. Fix: guard the `rename-workspace` call with `[ "$WS_OWNER" = "$CMUX_SURFACE_ID" ]` so only the owning session sets the initial workspace name. 2. **$SELF_DIR injected into JSON without escaping (greptile P2)** The claude wrapper interpolates $SELF_DIR directly into a JSON string literal. A path containing '"' or '\' would produce malformed JSON and silently disable all hooks. Fix: validate $SELF_DIR before building HOOKS_JSON. If the path contains JSON-unsafe characters, emit a clear warning to stderr and fall through to exec the real claude without hooks (safe degradation vs. silent failure).
Horacehxw
force-pushed
the
feat/ai-powered-tab-naming-hooks
branch
from
8727bcf to
8fb88cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Resources/bin/cmux-session-namer.sh (1)
25-28:⚠️ Potential issue | 🟠 MajorMake the workspace-owner claim atomic.
This TOCTOU race condition was previously flagged: two
SessionStarthooks in the same workspace can both observe a missing owner file and the later write wins, breaking the "first session wins" model.Proposed fix using noclobber
WS_OWNER_FILE="/tmp/cmux-ws-owner-${CMUX_WORKSPACE_ID}" -if [ ! -f "$WS_OWNER_FILE" ]; then - echo "$CMUX_SURFACE_ID" > "$WS_OWNER_FILE" -fi +( set -o noclobber + printf '%s\n' "$CMUX_SURFACE_ID" > "$WS_OWNER_FILE" +) 2>/dev/null || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/bin/cmux-session-namer.sh` around lines 25 - 28, Replace the non-atomic check-then-write sequence that uses WS_OWNER_FILE with an atomic create: enable shell noclobber and attempt a single redirect write of CMUX_SURFACE_ID into WS_OWNER_FILE (e.g. run a subshell with set -o noclobber; printf "%s" "$CMUX_SURFACE_ID" > "$WS_OWNER_FILE" 2>/dev/null) and ignore the write failure if the file already exists; this ensures the first writer wins atomically without a TOCTOU window while referencing WS_OWNER_FILE and CMUX_SURFACE_ID.Sources/AppDelegate.swift (1)
2081-2081:⚠️ Potential issue | 🟠 MajorTrack the actual fallback window, not just a flag.
This still only records that a fallback was requested. If the forced AppKit window is created and the delayed
WindowGroupwindow appears afterward, there is no retained window/windowId to close or reuse, so UI tests can end up with two main windows alive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` at line 2081, Replace the boolean didRequestFallbackUITestWindow with a stored reference to the actual fallback window (e.g., a property like fallbackUITestWindow: NSWindow? or fallbackUITestWindowId: Int?) in AppDelegate so you can close or reuse the real window later; update the code paths that create the forced AppKit window to assign that window to fallbackUITestWindow and update the logic that handles the delayed WindowGroup appearance to check for an existing fallbackUITestWindow and close or transfer ownership instead of only checking the flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 2425-2451: The retry stabilizer
stabilizeUITestLaunchWindowAndForeground is never invoked; update
applicationDidFinishLaunching to call stabilizeUITestLaunchWindowAndForeground
(instead of or in addition to the current one-shot launch block) so the
retry/foreground logic runs for delayed WindowGroup materialization—locate
applicationDidFinishLaunching and replace the old one-shot launch completion
path with a call to stabilizeUITestLaunchWindowAndForeground(attempt: 0),
ensuring any existing one-time fallback logic (e.g. openNewMainWindow and
didRequestFallbackUITestWindow) is not duplicated and the new retry loop can
perform moveUITestWindowToTargetDisplayIfNeeded(), activateUITestAppIfNeeded(),
and diagnostics as intended.
---
Duplicate comments:
In `@Resources/bin/cmux-session-namer.sh`:
- Around line 25-28: Replace the non-atomic check-then-write sequence that uses
WS_OWNER_FILE with an atomic create: enable shell noclobber and attempt a single
redirect write of CMUX_SURFACE_ID into WS_OWNER_FILE (e.g. run a subshell with
set -o noclobber; printf "%s" "$CMUX_SURFACE_ID" > "$WS_OWNER_FILE" 2>/dev/null)
and ignore the write failure if the file already exists; this ensures the first
writer wins atomically without a TOCTOU window while referencing WS_OWNER_FILE
and CMUX_SURFACE_ID.
In `@Sources/AppDelegate.swift`:
- Line 2081: Replace the boolean didRequestFallbackUITestWindow with a stored
reference to the actual fallback window (e.g., a property like
fallbackUITestWindow: NSWindow? or fallbackUITestWindowId: Int?) in AppDelegate
so you can close or reuse the real window later; update the code paths that
create the forced AppKit window to assign that window to fallbackUITestWindow
and update the logic that handles the delayed WindowGroup appearance to check
for an existing fallbackUITestWindow and close or transfer ownership instead of
only checking the flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6acf2eed-fda7-4892-b073-4d3e7d88a60e
📒 Files selected for processing (8)
Resources/bin/claudeResources/bin/cmux-rename-namer.shResources/bin/cmux-session-namer.shResources/bin/cmux-tab-namer.shSources/AppDelegate.swiftSources/TerminalWindowPortal.swiftcmuxTests/TerminalAndGhosttyTests.swiftdocs/ai-tab-naming.md
✅ Files skipped from review due to trivial changes (1)
- docs/ai-tab-naming.md
🚧 Files skipped from review as they are similar to previous changes (4)
- Sources/TerminalWindowPortal.swift
- Resources/bin/claude
- cmuxTests/TerminalAndGhosttyTests.swift
- Resources/bin/cmux-rename-namer.sh
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/bin/claude`:
- Around line 89-93: The hook script paths are being inserted into shell command
strings as bare tokens (the code that embeds absolute script paths around line
102), which breaks on spaces/metacharacters and can enable shell injection;
update the code that constructs/executes hook commands to properly quote or
escape path variables (e.g., wrap variables like SELF_DIR, REAL_CLAUDE and any
$SCRIPT_PATH in safe shell quoting or use a quoting helper such as printf '%q'
or exec with argument arrays) so that paths with spaces or special chars are
treated as single arguments and not re-parsed by the shell; keep the existing
JSON-safety check but ensure shell-level quoting is applied wherever command
strings are formed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6e1e00d-a3aa-4f14-acdf-dab6f9c1add5
📒 Files selected for processing (8)
Resources/bin/claudeResources/bin/cmux-rename-namer.shResources/bin/cmux-session-namer.shResources/bin/cmux-tab-namer.shSources/AppDelegate.swiftSources/TerminalWindowPortal.swiftcmuxTests/TerminalAndGhosttyTests.swiftdocs/ai-tab-naming.md
✅ Files skipped from review due to trivial changes (2)
- docs/ai-tab-naming.md
- Resources/bin/cmux-session-namer.sh
🚧 Files skipped from review as they are similar to previous changes (5)
- Sources/TerminalWindowPortal.swift
- cmuxTests/TerminalAndGhosttyTests.swift
- Resources/bin/cmux-rename-namer.sh
- Resources/bin/cmux-tab-namer.sh
- Sources/AppDelegate.swift
Address CodeRabbit actionable findings from latest review pass: 1. **TOCTOU race in workspace owner claim (CodeRabbit Major)** The previous `if [ ! -f ]; then echo > file; fi` pattern is not atomic: two concurrent SessionStart hooks can both observe a missing owner file and the last write wins, silently breaking the "first session wins" model. Fix: wrap the write in a subshell with `set -o noclobber`, which uses O_EXCL under the hood. Only one concurrent write succeeds; the others get EEXIST and silently skip via `|| true`. 2. **Shell metacharacters in hook command paths (CodeRabbit Major)** The previous guard only rejected '"' and '\' (JSON-breaking chars) but missed shell tokenization hazards: spaces, '$', '`', '!', ';', '|', '&', '<', '>', '()'. A path with a space (e.g. "/Applications/cmux test.app/...") would cause the shell to split the command token, silently disabling hooks. Fix: extend the guard regex to cover the full set of JSON/shell-unsafe characters. Behavior is unchanged for the normal case; unsafe paths now get a clear error message and fall through to exec without hooks.
Address cubic P2: workspace owner file persists after the owner session exits, preventing any subsequent session from updating the workspace name. On each SessionStart, before claiming ownership, check if the recorded owner surface is still active in the workspace (via `cmux list-surfaces`). If the owner is gone, delete the stale file so the new session can claim ownership atomically via noclobber. This keeps the "first live session wins" invariant across session lifecycle: - Owner still active → no change, non-owner continues to manage its own tab - Owner exited (stale file) → file cleared, new session claims ownership - Two concurrent new sessions → noclobber ensures exactly one wins
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Resources/bin/claude (1)
86-103:⚠️ Potential issue | 🟠 MajorThe path blacklist is still incomplete, and the fallback disables more than the new feature.
Lines 90-103 still embed
"$SELF_DIR"as a bare command token. Paths containing',*,?,[/]or other shell-active characters pass the current regex, so these naming hooks still break on those installs. When the guard does fire, Line 93 skips the entire--settingspayload, which also drops the pre-existing session/notification hooks instead of only the three new naming hooks.Please switch to a properly shell-quoted + JSON-escaped command string (or a fixed
cmuxsubcommand) so path safety does not depend on a partial blacklist and unsafe paths only disable the naming add-on.If any line below prints
passes guard, the current regex is still letting an unsafe path through:#!/bin/bash pattern='["\\ $`!;|&<>()]' for path in "/tmp/O'Brian/cmux" "/tmp/a*b/cmux" "/tmp/a?b/cmux" "/tmp/[qa]/cmux"; do if printf '%s' "$path" | grep -qE "$pattern"; then echo "blocked: $path" else echo "passes guard: $path" fi done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/bin/claude` around lines 86 - 103, The guard currently misses shell-special characters (e.g. ',*,?,[,]) and on failure disables all hooks by exec-ing REAL_CLAUDE; update the check and the HOOKS_JSON construction so unsafe SELF_DIR paths can't inject JSON/shell content and only the naming hooks are omitted. Specifically: extend the grep pattern to include additional chars (',*,?,[], etc.) or, better, stop interpolating SELF_DIR directly into HOOKS_JSON and instead JSON-escape and shell-quote the command strings (or replace embedded commands with a fixed safe subcommand like "cmux run-namer --name=session-namer" that internally uses SELF_DIR), and change the fallback path to still register existing lifecycle hooks while omitting only cmux-session-namer.sh / cmux-tab-namer.sh / cmux-rename-namer.sh when SELF_DIR is unsafe. Reference: SELF_DIR, HOOKS_JSON, cmux-session-namer.sh, cmux-tab-namer.sh, cmux-rename-namer.sh, REAL_CLAUDE.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/bin/cmux-session-namer.sh`:
- Around line 49-63: CUSTOM_MARKER currently points to /tmp and the script
writes TITLE there with default permissions, exposing conversation text; change
CUSTOM_MARKER to a user-private location (prefer ${TMPDIR:-/tmp}/cmux or a
per-user cmux state directory) and ensure the marker file is created with
restrictive permissions (0600) before writing (use a safe creation method like
creating the directory if needed and setting mode or using a secure temp file)
in the cmux-session-namer.sh logic around the CUSTOM_MARKER/TRANSCRIPT/TITLE
handling so the written title is not world- or group-readable.
- Around line 30-37: The current liveness check can treat CLI failures as "owner
absent" because the pipeline exit status is lost; instead run the cmux
list-surfaces command and capture its output and exit status into a variable
(e.g. SURFACES="$(cmux list-surfaces --workspace "$CMUX_WORKSPACE_ID"
--id-format uuids 2>/dev/null)" and check its exit code), then only run the
exact-match test for STALE_OWNER (use grep -xF or grep -xF -- to match whole
lines against "$STALE_OWNER" against "$SURFACES") and call rm -f
"$WS_OWNER_FILE" only if the list-surfaces command succeeded and the owner is
not found; reference symbols: WS_OWNER_FILE, STALE_OWNER, CMUX_SURFACE_ID,
CMUX_WORKSPACE_ID, cmux list-surfaces, grep -xF, rm -f.
---
Duplicate comments:
In `@Resources/bin/claude`:
- Around line 86-103: The guard currently misses shell-special characters (e.g.
',*,?,[,]) and on failure disables all hooks by exec-ing REAL_CLAUDE; update the
check and the HOOKS_JSON construction so unsafe SELF_DIR paths can't inject
JSON/shell content and only the naming hooks are omitted. Specifically: extend
the grep pattern to include additional chars (',*,?,[], etc.) or, better, stop
interpolating SELF_DIR directly into HOOKS_JSON and instead JSON-escape and
shell-quote the command strings (or replace embedded commands with a fixed safe
subcommand like "cmux run-namer --name=session-namer" that internally uses
SELF_DIR), and change the fallback path to still register existing lifecycle
hooks while omitting only cmux-session-namer.sh / cmux-tab-namer.sh /
cmux-rename-namer.sh when SELF_DIR is unsafe. Reference: SELF_DIR, HOOKS_JSON,
cmux-session-namer.sh, cmux-tab-namer.sh, cmux-rename-namer.sh, REAL_CLAUDE.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 22438691-85ba-461c-bf0d-53a5597d39f3
📒 Files selected for processing (2)
Resources/bin/claudeResources/bin/cmux-session-namer.sh
Address CodeRabbit actionable findings from 2026-03-25 review:
1. **Marker files in /tmp world-readable (CodeRabbit Major)**
CUSTOM_MARKER files were created with default umask (022 → 0644),
exposing conversation-derived rename titles to any local user.
Fix: wrap all marker writes in `(umask 077; printf ... > file)` so
files are created 0600 (owner-only read/write).
2. **Liveness check deletes owner on cmux transient error (CodeRabbit Minor)**
`if ! cmux ... | grep -qF; then rm` treated an empty/failed cmux output
as "owner not present" due to missing pipefail, incorrectly evicting a
valid owner on CLI transport failures.
Fix: capture output into a variable first; only delete the owner file
when the listing command produced non-empty output AND the owner ID is
absent from it.
3. **SELF_DIR guard incomplete, fallback disables all hooks (CodeRabbit Major)**
Previous guard missed shell-special chars: ' * ? [ ] — paths like
"/tmp/O'Brian/cmux" passed the regex. When the guard fired, it also
dropped all lifecycle hooks (session/notification/etc), not just naming.
Fix:
- Extend regex to cover ' * ? [ ] in addition to " \ space $ ` ! ; | & < > ( )
- Extract BASE_HOOKS_JSON (no $SELF_DIR, always safe) separate from
full HOOKS_JSON. Fallback now uses BASE_HOOKS_JSON so existing
lifecycle hooks continue to work; only the naming add-on is skipped.
Author
On SessionStart, clear the pending tab label (/tmp/cmux-tab-pending-*) alongside the custom-title marker and tab-cache. Without this, a pending label written by a prior session's Stop hook but not yet consumed by UserPromptSubmit would survive into the new session and get applied by cmux-rename-namer.sh without any freshness validation, producing a stale tab name from the previous session. Fixes session-namer stale-pending regression flagged in PR review.
There was a problem hiding this comment.
Pull request overview
Adds AI-driven cmux tab/workspace auto-naming hooks to the claude wrapper (with /rename precedence + opt-out), and includes a small portal-geometry sync tweak plus UI-test stabilization/test coverage improvements.
Changes:
- Injects three new cmux hook scripts (SessionStart / Stop / UserPromptSubmit) via
Resources/bin/claudeto generate/apply AI-based names and persist/renameprecedence. - Improves portal geometry synchronization after bind to handle queued ancestor layout shifts; adds a regression test.
- Adds additional UI-test launch stabilization logic (currently not wired in) and documents the AI tab naming feature.
Reviewed changes
Copilot reviewed 4 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Sources/TerminalWindowPortal.swift |
Schedules an additional external geometry sync after binding to catch delayed layout shifts. |
Sources/AppDelegate.swift |
Adds a DEBUG-only UI-test stabilization retry helper (currently unused). |
Resources/bin/claude |
Extends settings JSON injection to include the new AI naming hook scripts with a path-safety guard. |
Resources/bin/cmux-tab-namer.sh |
Stop hook: builds context and spawns background claude -p summary generation into a pending file. |
Resources/bin/cmux-session-namer.sh |
SessionStart hook: establishes workspace owner, syncs resumed custom-title, initializes/clears naming state. |
Resources/bin/cmux-rename-namer.sh |
UserPromptSubmit hook: applies pending AI label then syncs /rename into tab/workspace + marker file. |
docs/ai-tab-naming.md |
Documents how AI naming hooks work and how to disable them. |
cmuxTests/TerminalAndGhosttyTests.swift |
Adds a test ensuring bind queues an external geometry sync for queued layout shifts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2425
to
+2464
| // Retry launch stabilization until a delayed WindowGroup materialization produces | ||
| // a visible key window that XCUITest can bring to the foreground on the shared VM. | ||
| private func stabilizeUITestLaunchWindowAndForeground(attempt: Int = 0) { | ||
| let env = ProcessInfo.processInfo.environment | ||
| guard isRunningUnderXCTest(env) else { return } | ||
|
|
||
| if NSApp.windows.isEmpty, !didRequestFallbackUITestWindow { | ||
| didRequestFallbackUITestWindow = true | ||
| openNewMainWindow(nil) | ||
| } | ||
|
|
||
| moveUITestWindowToTargetDisplayIfNeeded() | ||
| activateUITestAppIfNeeded() | ||
|
|
||
| let hasWindow = !NSApp.windows.isEmpty | ||
| let hasVisibleWindow = NSApp.windows.contains { $0.isVisible } | ||
| let hasKeyWindow = NSApp.keyWindow != nil | ||
| let stage = attempt == 0 ? "afterForceWindow" : "afterForceWindow.retry\(attempt)" | ||
| writeUITestDiagnosticsIfNeeded(stage: stage) | ||
|
|
||
| guard attempt < 20 else { return } | ||
| if !hasWindow || !hasVisibleWindow || !hasKeyWindow || !NSRunningApplication.current.isActive { | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { [weak self] in | ||
| self?.stabilizeUITestLaunchWindowAndForeground(attempt: attempt + 1) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private func activateUITestAppIfNeeded() { | ||
| if let window = NSApp.windows.first { | ||
| window.makeKeyAndOrderFront(nil) | ||
| window.orderFrontRegardless() | ||
| } | ||
| if #available(macOS 14.0, *) { | ||
| NSRunningApplication.current.activate(options: [.activateAllWindows]) | ||
| } else { | ||
| NSRunningApplication.current.activate(options: [.activateAllWindows, .activateIgnoringOtherApps]) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This UI-test stabilization routine is never called (the only references are the function itself and its recursive retry). As-is, didRequestFallbackUITestWindow and the helper functions add dead code/unused state. Either invoke this from the existing UI-test launch path (e.g. after the initial afterForceWindow block) or remove it to keep the debug-only launch logic maintainable.
Suggested change
| // Retry launch stabilization until a delayed WindowGroup materialization produces | |
| // a visible key window that XCUITest can bring to the foreground on the shared VM. | |
| private func stabilizeUITestLaunchWindowAndForeground(attempt: Int = 0) { | |
| let env = ProcessInfo.processInfo.environment | |
| guard isRunningUnderXCTest(env) else { return } | |
| if NSApp.windows.isEmpty, !didRequestFallbackUITestWindow { | |
| didRequestFallbackUITestWindow = true | |
| openNewMainWindow(nil) | |
| } | |
| moveUITestWindowToTargetDisplayIfNeeded() | |
| activateUITestAppIfNeeded() | |
| let hasWindow = !NSApp.windows.isEmpty | |
| let hasVisibleWindow = NSApp.windows.contains { $0.isVisible } | |
| let hasKeyWindow = NSApp.keyWindow != nil | |
| let stage = attempt == 0 ? "afterForceWindow" : "afterForceWindow.retry\(attempt)" | |
| writeUITestDiagnosticsIfNeeded(stage: stage) | |
| guard attempt < 20 else { return } | |
| if !hasWindow || !hasVisibleWindow || !hasKeyWindow || !NSRunningApplication.current.isActive { | |
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { [weak self] in | |
| self?.stabilizeUITestLaunchWindowAndForeground(attempt: attempt + 1) | |
| } | |
| } | |
| } | |
| private func activateUITestAppIfNeeded() { | |
| if let window = NSApp.windows.first { | |
| window.makeKeyAndOrderFront(nil) | |
| window.orderFrontRegardless() | |
| } | |
| if #available(macOS 14.0, *) { | |
| NSRunningApplication.current.activate(options: [.activateAllWindows]) | |
| } else { | |
| NSRunningApplication.current.activate(options: [.activateAllWindows, .activateIgnoringOtherApps]) | |
| } | |
| } |
Comment on lines
+47
to
+48
| Owner is tracked via `/tmp/cmux-ws-owner-{WORKSPACE_ID}`. This file persists in `/tmp/` until the OS clears it (typically on reboot); it is not removed when sessions end. The first session to start in a workspace wins ownership for the lifetime of that file. | ||
|
|
There was a problem hiding this comment.
The docs say the workspace owner file persists until OS cleanup and is "not removed when sessions end", but cmux-session-namer.sh now includes stale-owner recovery that deletes /tmp/cmux-ws-owner-{WORKSPACE_ID} when the recorded surface is no longer active. Please update this section to reflect the new ownership recovery behavior so users understand when ownership can change.
Suggested change
| Owner is tracked via `/tmp/cmux-ws-owner-{WORKSPACE_ID}`. This file persists in `/tmp/` until the OS clears it (typically on reboot); it is not removed when sessions end. The first session to start in a workspace wins ownership for the lifetime of that file. | |
| Owner is tracked via `/tmp/cmux-ws-owner-{WORKSPACE_ID}`. The first active session to start in a workspace becomes the owner and keeps that role while its recorded surface remains active. The owner file normally persists in `/tmp/`, but `cmux-session-namer.sh` performs stale-owner recovery: if the file points to a surface that is no longer active, it deletes the stale file so the next Claude Code session started in that workspace can claim ownership. | |
| This means ownership is usually stable across tabs and sessions, but it can change after the previous owner is no longer active and a later session starts in the same workspace. OS cleanup can still remove the file as well (typically on reboot). |
| All temporary files are in `/tmp/` and scoped by surface/workspace ID: | ||
| - `/tmp/cmux-tab-pending-{SURFACE_ID}` — pending AI label | ||
| - `/tmp/cmux-tab-cache-{SURFACE_ID}` — last applied label + line count | ||
| - `/tmp/cmux-tab-prompt-{SURFACE_ID}` — prompt file for `claude -p` |
There was a problem hiding this comment.
The temp-files list omits the custom-title marker file (/tmp/cmux-custom-title-{SURFACE_ID}) that controls whether AI naming is suppressed for long/resumed sessions. Add it here so users can understand/debug the precedence behavior and clean up temp state if needed.
Suggested change
| - `/tmp/cmux-tab-prompt-{SURFACE_ID}` — prompt file for `claude -p` | |
| - `/tmp/cmux-tab-prompt-{SURFACE_ID}` — prompt file for `claude -p` | |
| - `/tmp/cmux-custom-title-{SURFACE_ID}` — marker that a `/rename` custom title exists, suppressing AI naming for that tab |
… retry timers Address three CodeRabbit review issues on PR manaflow-ai#2043: 1. Wire stabilizeUITestLaunchWindowAndForeground into applicationDidFinishLaunching (was dead code — the old one-shot block is replaced by a call to the stabilizer so the retry loop actually runs for delayed WindowGroup materialization) 2. Only call moveUITestWindowToTargetDisplayIfNeeded() on attempt == 0 (the helper already schedules its own 20-step retry; calling it on every pass of the outer loop fanned out into overlapping timers that raced each other) 3. 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)
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds three shell hooks that auto-name cmux tabs and workspaces from Claude Code conversation content using AI summaries (
claude -p --model haiku). Integrates into the existingResources/bin/claudewrapper alongside current lifecycle hooks./renamecustom titles take absolute precedence — AI naming stops immediatelyCMUX_TAB_NAMER_DISABLED=1Problem
When running multiple Claude Code sessions across cmux tabs, all tabs show generic names (directory basename or shell command). With 5+ concurrent sessions, it's impossible to tell which tab is doing what without manually
/rename-ing each one.Solution
Three hooks injected by the claude wrapper:
cmux-session-namer.shcmux-tab-namer.shcmux-rename-namer.sh/renamecustom titleDesign decisions
Decoupled generate/apply for non-blocking, fast updates
claude -p --model haikutakes ~5-10s and detached background processes lose cmux socket access. The original two-phase design (generate in background, apply at next Stop) has been improved:This means tab names update the moment the user sends their next message — typically the entire Claude response time sooner than the original design.
Custom-title marker file (fix for long/resumed sessions)
The original
tail -500approach to detect/renamefails when a session is resumed and the custom-title entry is more than 500 lines back in the transcript. Fix: a marker file/tmp/cmux-custom-title-{SURFACE_ID}that is independent of transcript length.Workspace owner model
Naming priority
/rename/renamecustom titleFiles changed
Resources/bin/claude— inject naming hooks into the settings JSONResources/bin/cmux-tab-namer.sh— Stop hook: generate AI summary in backgroundResources/bin/cmux-session-namer.sh— SessionStart hook: init + resume custom-title syncResources/bin/cmux-rename-namer.sh— UserPromptSubmit hook: apply pending label + /rename syncdocs/ai-tab-naming.md— documentationTest plan
/rename "custom name"→ tab and workspace (if owner) show custom name, AI stops/rename→ custom title syncs immediately on SessionStartCMUX_TAB_NAMER_DISABLED=1→ no auto-namingSummary by CodeRabbit
New Features
/renamecustom titles and workspace ownership rules.Bug Fixes
Tests
Documentation