Conversation
📝 WalkthroughWalkthroughAuto-naming now enforces transcript thresholds, launches detached workers directly, and triggers supported naming during message processing and turn completion. Integration coverage verifies that a Pi prompt applies the expected workspace title. ChangesAuto-naming workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PiPromptSubmit
participant CMUXCLI
participant DetachedWorker
participant AgentHookMockServer
PiPromptSubmit->>CMUXCLI: submit prompt
CMUXCLI->>DetachedWorker: start supported auto-naming pass
DetachedWorker->>AgentHookMockServer: send workspace.set_auto_title
AgentHookMockServer-->>DetachedWorker: return configured title result
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings, 1 inconclusive)
✅ Passed checks (21 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLI/CMUXCLI`+AutoNamingHooks.swift:
- Around line 121-132: Update the executable-path selection logic around
ProcessInfo.arguments.first and
normalizedHookValue(env["CMUX_BUNDLED_CLI_PATH"]) to require each resolved path
to be a non-directory regular file in addition to passing the executable check.
Preserve the existing priority order, and fall back to cmux when neither
candidate qualifies.
In `@cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift`:
- Around line 80-116: Make the async regression test around runAgentHook
deterministic and verify non-blocking behavior: replace the pi mock’s fixed
sleep with a test-controlled release signal, invoke the hook, and assert it
returns before releasing the signal. Then release the mock summarizer and retain
the existing wait for workspace.set_auto_title, using the repository’s
established Swift completion-signal utilities instead of wall-clock sleeps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5066aa8f-68b3-431e-b476-98888a3a0701
📒 Files selected for processing (6)
CLI/CMUXCLI+AutoNaming.swiftCLI/CMUXCLI+AutoNamingGenericHooks.swiftCLI/CMUXCLI+AutoNamingHooks.swiftCLI/cmux.swiftcmuxTests/AutoNamingHookPayloadAdapterTests.swiftcmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
Greptile SummaryThis PR advances workspace auto-naming for Pi (and similar message-backed agents) to trigger on prompt submission rather than waiting for the agent to stop, matching Amp's existing behavior. It also hardens the detached-worker spawn path by replacing the shell-mediated
Confidence Score: 5/5Safe to merge; the new prompt-submit naming path is well-guarded and the regression test reliably verifies end-to-end behavior through the Pi summarizer gate. The core behavior change — triggering workspace naming on prompt submission for message-backed agents — is correctly implemented and covered by a solid FIFO-gated integration test. The direct Process.run() approach is more robust than the previous shell-mediated backgrounding, and Foundation's NSTask retains the child internally until exit. The two design trade-offs carried forward from the prior round (unnecessary worker spawns when auto-naming is disabled, and the 1-message/2-message line-equivalent floor collision) were already flagged and are both cosmetic/minor in practice. No files require special attention beyond the design trade-offs noted in prior review rounds. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant H as Hook Process
participant W1 as AutoName Worker (prompt)
participant W2 as AutoName Worker (stop)
participant Pi as Pi Summarizer
participant S as cmux Socket
U->>H: prompt-submit hook (Pi agent)
H->>H: usesHookMessageCacheForAutoNaming?
H->>W1: spawnDetachedAgentAutoNameIfSupported()
Note over H,W1: process.run() — non-blocking
H-->>U: hook returns immediately
W1->>S: workspace.set_auto_title probe
S-->>W1: "enabled=true, user_owned=false"
W1->>Pi: invoke Pi summarizer
Pi-->>W1: Java Workspace
W1->>S: workspace.set_auto_title (title)
U->>H: stop hook (turn complete)
H->>W2: spawnDetachedAgentAutoNameIfSupported()
Note over H,W2: process.run() — non-blocking
H-->>U: hook returns immediately
W2->>S: workspace.set_auto_title probe
S-->>W2: "enabled=true, user_owned=false"
W2->>Pi: invoke Pi summarizer (full transcript)
Pi-->>W2: refined title
W2->>S: workspace.set_auto_title (refined title)
Reviews (2): Last reviewed commit: "fixup! Add first-prompt auto-naming regr..." | Re-trigger Greptile |
84543e7 to
3f9588a
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLI/CMUXCLI`+AutoNamingHooks.swift:
- Around line 153-155: Update the detached worker launch in
spawnDetachedAgentAutoName to enforce a whole-worker deadline, owner/lifetime
handling, and termination escalation through SIGKILL cleanup. Keep direct
process execution, but reuse runProcess if it already provides this timeout and
cleanup behavior, ensuring stalls before the child’s llmTimeout cannot leave the
auto-name process running indefinitely.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e38e6aee-4a8e-443c-8b68-ede305a5e018
📒 Files selected for processing (6)
CLI/CMUXCLI+AutoNaming.swiftCLI/CMUXCLI+AutoNamingGenericHooks.swiftCLI/CMUXCLI+AutoNamingHooks.swiftCLI/cmux.swiftcmuxTests/AutoNamingHookPayloadAdapterTests.swiftcmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
| do { | ||
| // Launch the bounded worker directly; the short-lived hook process must not own its lifetime. | ||
| try process.run() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)CLI/CMUXCLI\+AutoNamingHooks\.swift$|AutoNaming|auto-name|llmTimeout|summarize' || true
echo
echo "Target file outline:"
ast-grep outline CLI/CMUXCLI+AutoNamingHooks.swift --view expanded || true
echo
echo "Target file around relevant lines:"
if [ -f CLI/CMUXCLI+AutoNamingHooks.swift ]; then
nl -ba CLI/CMUXCLI+AutoNamingHooks.swift | sed -n '1,240p'
fi
echo
echo "Search timeout/termination wrapper refs:"
rg -n "llmTimeout|summarize|auto-name|die|SIGKILL|process\.run|Process|run\(\)" CLI CodeRabbit Inc . --glob '!*.{png,jpg,jpeg,webp,gif,svg,aar,swiftinterface,xcstrings}' | head -n 250Repository: manaflow-ai/cmux
Length of output: 1212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Install available line-number alternatives and inspect file:"
for f in CLI/CMUXCLI+AutoNamingHooks.swift CLI/CMUXCLI+.AutoNamingHooks.swift; do
[ ${f%% *:-.} = "$f" ] || continue
done
wc -l CLI/CMUXCLI+AutoNamingHooks.swift
awk '{printf "%6d\t%s\n", NR, $0}' CLI/CMUXCLI+AutoNamingHooks.swift | sed -n '1,260p'
echo
echo "Search timeout/termination references:"
rg -n "llmTimeout|summarize|auto-name|SIGKILL|terminate\(|die|process\.run\(\)|Process\.init|run\(\)" -S . --glob '!*.{png,jpg,jpeg,webp,gif,svg,aar,swiftinterface,xcstrings}' || true
echo
echo "Diff around changed file if present:"
git diff -- CLI/CMUXCLI+AutoNamingHooks.swift | sed -n '1,240p' || trueRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Limited source inspection for AutoNamingHooks and related auto-name files:"
for f in CLI/CMUXCLI+AutoNamingHooks.swift CLI/CMUXCLI+AutoNamingDispatch.swift CLI/CMUXCLI+AutoNamingGenericHooks.swift CLI/CMUXCLI+AutoNamingSummarizers.swift CLI/CMUXCLI+AutoNaming.swift; do
if [ -f "$f" ]; then
echo "===== $f: $(wc -l < "$f") lines ====="
fi
done
echo "===== CLI/CMUXCLI+AutoNamingHooks.swift lines 1-240 ====="
python3 - <<'PY'
from pathlib import Path
p=Path('CLI/CMUXCLI+AutoNamingHooks.swift')
if p.exists():
for i,line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 240:
print(f"{i:5d}\t{line}")
PY
echo "===== CLI/CMUXCLI+AutoNamingDispatch.swift lines 1-260 ====="
python3 - <<'PY'
from pathlib import Path
p=Path('CLI/CMUXCLI+AutoNamingDispatch.swift')
if p.exists():
for i,line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 260:
print(f"{i:5d}\t{line}")
PY
echo "===== Search timeout/termination/cleanup refs excluding broad file types ====="
rg -n "llmTimeout|summarize|\.run\(\)|Process|SIGKILL|SIGTERM|terminate\(|kill|timeout|AutoNamingHooks|spawnDetachedAgentAutoName|runClaudeAutoNameHook|runCodexAutoNameHook" CLI --glob '*.swift' || true
echo "===== Git diff stats and hunks for renamed/deleted hook files ====="
git diff --stat -- '*AutoNamingHook*' '*AutoNamingDispatch*' || true
git diff -- '*AutoNamingHook*' '*AutoNamingDispatch*' | sed -n '1,260p' || trueRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== CLI/CMUXCLI+AutoNamingGenericHooks.swift lines 1-220 ====="
python3 - <<'PY'
from pathlib import Path
p=Path('CLI/CMUXCLI+AutoNamingGenericHooks.swift')
if p.exists():
for i,line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 220:
print(f"{i:5d}\t{line}")
PY
echo "===== CLI/CMUXCLI+AutoNamingSummarizers.swift lines 1-220 ====="
python3 - <<'PY'
from pathlib import Path
p=Path('CLI/CMUXCLI+AutoNamingSummarizers.swift')
if p.exists():
for i,line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 220:
print(f"{i:5d}\t{line}")
PY
echo "===== CLI/CMUXCLI+AutoNaming.swift lines 1-260 ====="
python3 - <<'PY'
from pathlib import Path
p=Path('CLI/CMUXCLI+AutoNaming.swift')
if p.exists():
for i,line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 260:
print(f"{i:5d}\t{line}")
PY
echo "===== CLI/CMUXCLI+Process.swift lines 290-500 ====="
python3 - <<'PY'
from pathlib import Path
p=Path('CLI/CMUXCLI+Process.swift')
if p.exists():
for i,line in enumerate(p.read_text().splitlines(), 1):
if 290 <= i <= 500:
print(f"{i:5d}\t{line}")
PY
echo "===== Direct invocation helper usages ====="
rg -n "runAutoNamingSummarizer|runProcessData|runProcess\\(|cliRunProcess\\(|timeout:|process\\.run\\(|spawnDetachedAgentAutoName\\(|AgentHook|auto-name" CLI --glob '*.swift' | sed -n '1,220p'Repository: manaflow-ai/cmux
Length of output: 50372
Add a process owner/deadline to the detached worker or apply runProcess with an existing timeout.
process.run() leaves spawnDetachedAgentAutoName with no owner or deadline. llmTimeout only covers the summary step inside the child; if the socket read, session-store check, or title apply stalls before then, the hook can leave an unbounded auto-name process. Keep direct execution, but preserve a whole-worker deadline, termination sequence, and SIGKILL cleanup in one of these entry points.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLI/CMUXCLI`+AutoNamingHooks.swift around lines 153 - 155, Update the
detached worker launch in spawnDetachedAgentAutoName to enforce a whole-worker
deadline, owner/lifetime handling, and termination escalation through SIGKILL
cleanup. Keep direct process execution, but reuse runProcess if it already
provides this timeout and cleanup behavior, ensuring stalls before the child’s
llmTimeout cannot leave the auto-name process running indefinitely.
|
Thank you for this, @mrohan-sq! Naming Pi workspaces after the first prompt is a great idea. One thing: this drops the cheap "is auto-naming on?" check, so every prompt and turn end now starts a naming worker even when auto-naming is off. Keeping that check up front and it's in good shape :) |
|
Thanks for this! Before we can land it, please comment |
|
I have read the CLA Document v2.2 and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Context
Amp updates its workspace title when the first prompt starts. Pi should match that timing while generating its semantic title asynchronously, so prompt processing never waits. Pi workspaces can otherwise retain directory-derived titles because detached naming does not reliably start.
Summary
Dependencies
None.
Summary by CodeRabbit