Skip to content

Sanitize Save Workspace Layout agent commands through the shared sanitizer; fail loudly on un-replayable commands; fix save dialog layout - #7633

Merged
austinywang merged 31 commits into
mainfrom
issue-7631-save-layout-agent-commands
Jul 9, 2026
Merged

austinywang merged 31 commits into
mainfrom
issue-7631-save-layout-agent-commands

Conversation

@austinywang

@austinywang austinywang commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Closes #7631

Problem

Saving a workspace layout with a running Codex agent broke two ways:

  1. Relaunch wedged the shell at quote>. Capture persisted the live foreground argv verbatim — for a cmux-launched codex that is node …/bin/codex --enable hooks --dangerously-bypass-hook-trust -c 'hooks.<Event>=…' … <user flags>, a multi-KB command with nested quote escaping. On replay the command is typed into a freshly spawned shell whose tty is still in canonical mode, and macOS discards canonical input lines beyond MAX_CANON (1024 bytes) mid-token (empirically reproduced: a 3009-byte line delivers ~1019 chars), so the broken quoting left the shell at a quote> continuation prompt.
  2. The save dialog dumped the multi-KB argv into informativeText and its fixed 260 pt accessory clipped the name field and default checkbox.

Claude was sanitized on the same save while codex was not, because capture detects agent kinds from argv[0]'s basename only (node matches nothing) and the shared AgentLaunchSanitizer had no codex hook stripping — the resume path never needed it, since it rebuilds argv from wrapper-recorded pre-injection args, while layout capture reads live post-injection argv.

Fix

  • Shared sanitization, one seam. AgentLaunchSanitizer now strips cmux-injected codex hook arguments (--enable hooks, --dangerously-bypass-hook-trust, and the -c/--config hooks.<Event>=… pairs, in separate-token and =-joined forms) whenever a cmux marker (cmux-codex-hook, present in both the script-file and inline hook command forms) identifies the launch as cmux-wrapped. A user's own hooks config without the marker is untouched. The stripping lives in the shared codex-launch path so capture, resume, and fork all use it; the codex launch/fork helpers move to a new AgentLaunchSanitizerCodexLaunch.swift (the sanitizer file was at its length budget).
  • Node/bun unwrap in capture. Capture unwraps node/bun-hosted known agents to the bare agent name, so the persisted command is codex --dangerously-bypass-approvals-and-sandbox --model gpt-5.5 -c model_reasoning_effort=xhigh and relaunch resolves through the per-surface PATH shim → cmux wrapper → hooks re-injected fresh (same rationale as the AgentResumeArgv wrapper-shim token, Claude resume still drops cmux hooks after #5430: bare claude doesn't resolve to the wrapper inside the $SHELL -lic restore launcher #5639). Claude capture behavior is byte-for-byte unchanged.
  • Loud failure instead of silent truncation. The replay cap is a kernel tty property and cannot be removed, so saving now fails with a localized error alert when a captured command exceeds 1000 UTF-8 bytes (documented against MAX_CANON = 1024), rather than persisting a command that cannot replay.
  • Dialog rework. Command/URL/env disclosure moves from informativeText into a height-capped, scrollable, selectable text area in a new Auto Layout accessory (WorkspaceActionSaveDialogAccessory, patterned on the repo's working NSAlert accessories); the name field and default checkbox render at full width. Full verbatim disclosure is preserved so secret-bearing commands are still seen before they are written to config.

Tests (two-commit red/green)

  • Commit 1 (red): package + app regression tests only — codex argv with cmux-injected hook flags (script-file, inline, and =-joined forms) sanitizes to the user's own flags; user hook config without the marker preserved; resume path shares the stripping; claude unchanged; node-wrapped codex captures as bare codex. Verified locally: compiles pre-fix and fails by assertion (4 tests, 7 issues).
  • Commit 2 (green): the fix plus tests requiring new APIs (unwrap helper edge cases, oversized-command detection).

Verification

  • cd Packages/macOS/CMUXAgentLaunch && swift test: 188 tests pass.
  • python3 scripts/swift_file_length_budget.py: passes; no budget TSV touched (AgentLaunchSanitizer.swift shrank 598 → 519).
  • Localization audit: new user-facing strings are the over-limit alert title/message (dialog.saveWorkspaceLayout.commandTooLongTitle / .commandTooLongMessage); both have en + ja entries in Resources/Localizable.xcstrings; the disclosure headers reuse existing localized keys; rg over changed Swift found no new bare user-facing literals.
  • pbxproj change is the minimal wiring of the new app source file; new tests went into the already-wired CmuxConfigActionSaverTests.swift.

🤖 Generated with Claude Code


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Note

Medium Risk
Changes how workspace layouts persist and replay agent commands (Codex/Claude and node-hosted launches); mistakes could drop hooks or save broken replay strings, though behavior is heavily covered by new tests.

Overview
Save Workspace Layout now runs captured terminal commands through shared agent sanitization so cmux-injected Codex hook argv (and related resume/session noise) is stripped before persistence, while user-owned hook flags without cmux markers stay intact. Codex/Claude logic moves into AgentLaunchSanitizerCodexLaunch.swift, ClaudeLaunchArgumentsPreserver, and JavaScriptRuntimeAgentLaunchUnwrapper so node/bun package entrypoints keep their runtime + script path but get a cleaned agent tail; hook-looking argv is no longer treated as proof the PATH shim launched the process.

Saves are rejected with a localized alert when setup or any captured command exceeds 1000 UTF-8 bytes (pty canonical-line replay limit). The save dialog uses a new scrollable accessory for commands/URLs/env keys instead of dumping multi-KB text into the alert body, with a sizing fix so disclosure text actually renders.

Reviewed by Cursor Bugbot for commit 3288aca. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Fixes Save Workspace Layout by sanitizing captured agent commands, stripping cmux‑injected hook payloads, preserving captured executables (and node/bun runtime+script paths), and blocking saves that can’t be reliably replayed. Also fixes the dialog so disclosure text renders. Closes #7631.

  • Bug Fixes

    • Strip cmux‑injected Codex/Claude hook payloads via the shared sanitizer; keep user hook flags/settings. Do not treat hook argv as identity proof; preserve explicit binaries and entrypoints.
    • For node/bun‑hosted agents, keep the captured runtime and script path; sanitize only the agent tail. Never rewrite project‑local scripts or pinned installs.
    • Codex resume/fork keep the captured executable; sanitize options without dropping user hooks.
    • Block save and show a localized alert if setup or any captured command exceeds 1000 UTF‑8 bytes.
    • Move command/URL/env disclosure into a scrollable accessory and fix text view sizing so content renders.
  • Refactors

    • Extract Codex launch helpers into AgentLaunchSanitizerCodexLaunch.swift.
    • Move Claude argument preservation into ClaudeLaunchArgumentsPreserver; add JavaScriptRuntimeAgentLaunchUnwrapper.

Written for commit 3288aca. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Enhanced “Save Workspace Layout” dialog with richer, read-only details for captured commands, URLs, and environment keys.
  • Bug Fixes
    • Prevents saving when any captured command exceeds the replay limit; shows a dedicated “Command Too Long to Save” alert with a truncated preview.
    • Improves terminal command replay for JavaScript-based launches by unwrapping known runtime wrappers and stripping injected Codex/Claude hook-related arguments/settings, including on resume.
  • Localization
    • Added localized strings for the “Command Too Long to Save” alert.
  • Tests
    • Expanded coverage for command capture, hook sanitization, and runtime unwrapping behavior.

austinywang and others added 2 commits July 8, 2026 10:11
…x hook argv

Saving a workspace layout with a running codex agent persists the live
process argv verbatim, including every cmux-injected hook flag
(--enable hooks --dangerously-bypass-hook-trust -c 'hooks.<Event>=…'),
because codex launches as `node …/bin/codex` and capture only detects
agent kinds from argv[0]'s basename. The shared AgentLaunchSanitizer
also has no codex hook stripping (claude's --settings blob IS
stripped, hence the asymmetry in #7631).

These tests pin the expected behavior and fail without the fix:
- codex argv with cmux-injected hook flags sanitizes to the user's own
  flags only (--dangerously-bypass-approvals-and-sandbox --model
  gpt-5.5 -c model_reasoning_effort=xhigh)
- inline and =-joined injected hook forms are stripped too
- a user's own hooks config without the cmux marker is preserved
- the codex resume path shares the same stripping
- claude sanitization is unchanged
- node-wrapped codex captures as a bare `codex` command

Part of the two-commit red/green regression policy for #7631; the fix
lands in the following commit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… un-replayable commands; rework save dialog

Fixes three problems in Save Workspace Layout (#7631):

1. Codex hook argv captured verbatim. Layout capture reads the live
   foreground argv, which for a cmux-launched codex is
   `node …/bin/codex --enable hooks --dangerously-bypass-hook-trust
   -c 'hooks.<Event>=…' … <user flags>`. Capture detected agent kinds
   only from argv[0]'s basename (`node` matches nothing), and the
   shared AgentLaunchSanitizer had no codex hook stripping (the resume
   path never needed it: it rebuilds argv from wrapper-recorded
   pre-injection args). Now:
   - AgentLaunchSanitizer strips cmux-injected codex hook arguments
     (marker: `cmux-codex-hook`, present in both script-file and
     inline hook forms) in the shared codex-launch path, so capture
     and resume share one sanitization seam. A user's own hooks
     config without the marker is untouched.
   - Capture unwraps node/bun-hosted known agents to the bare agent
     name (`codex …`), so the replayed command resolves through the
     per-surface PATH shim and the cmux wrapper re-injects hooks
     fresh at launch, mirroring the AgentResumeArgv wrapper-shim
     rationale (#5639). Claude capture behavior is unchanged.
   The codex launch/fork helpers move to
   AgentLaunchSanitizerCodexLaunch.swift (AgentLaunchSanitizer.swift
   was at its file-length budget).

2. Replay truncation. The saved command is typed into a freshly
   spawned shell's pty, which is still in canonical mode; macOS caps
   a canonical input line at MAX_CANON (1024 bytes) and silently
   discards the rest mid-token, leaving the shell wedged at `quote>`.
   Verified empirically (a 3009-byte line delivers ~1019 chars).
   There is no persist- or send-side cap to fix, so saving now fails
   loudly: commands over 1000 UTF-8 bytes present a localized error
   alert instead of persisting a command that cannot replay.

3. Dialog styling. The multi-KB command dump lived in the NSAlert
   informativeText and the 260pt fixed-frame accessory clipped the
   name field and default checkbox. Command/URL/env disclosure moves
   to a height-capped scrollable, selectable text area in a new
   Auto Layout accessory (WorkspaceActionSaveDialogAccessory), and
   the informativeText keeps only the base message. Full verbatim
   disclosure is preserved so secret-bearing commands are still seen
   before they are written to config.

New strings are localized (en + ja). The regression tests added in
the previous commit now pass; this commit adds the tests that require
the new APIs (unwrap helper, oversized-command detection).

Closes #7631

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Canceled Canceled Jul 9, 2026 1:17am
cmux-staging Building Building Preview, Comment Jul 9, 2026 1:17am

@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR updates Codex argument sanitization and JavaScript runtime unwrapping, and it adds oversized-command detection plus a warning path in the workspace save dialog.

Changes

Codex/Cmux Launch Sanitization

Layer / File(s) Summary
Codex preservation filters cmux hooks
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizer.swift
Codex fork and plain-codex preservation paths now filter cmux-injected hook arguments first, and the option helpers are module-visible.
Launch sanitizer extension
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerCodexLaunch.swift
Adds Codex hook stripping, fork/session parsing, cmux hook detection, Node/Bun argv unwrapping, and JavaScript runtime helper parsing.
Sanitization tests
Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/CodexHookInjectionStrippingTests.swift
Adds tests for Codex and Claude hook stripping plus Node/Bun runtime argv unwrapping.

Workspace Save Command Handling

Layer / File(s) Summary
Replayable command threshold and detection
Sources/TerminalForegroundCommandCapture.swift, Sources/WorkspaceConfigActionCapture.swift, cmuxTests/CmuxConfigActionSaverTests.swift
Adds the replay-length limit, unwraps known runtime-hosted agent argv before capture, exposes oversized command detection, and covers both behaviors in tests.
Workspace save accessory view
Sources/WorkspaceActionSaveDialogAccessory.swift, cmux.xcodeproj/project.pbxproj
Adds the save-dialog accessory view with name/default controls and disclosure sections, then wires the file into the Xcode project.
Save dialog alert and wiring
Sources/AppDelegate+WorkspaceActionSave.swift, Resources/Localizable.xcstrings, cmuxTests/CmuxConfigActionSaverTests.swift
Checks for oversized commands before showing the save prompt, shows a truncated-preview alert, uses the new accessory controls, and adds localized alert strings.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

Possibly related PRs

  • manaflow-ai/cmux#4237 — Also touches Codex/cmux argv sanitization and runtime unwrapping in AgentLaunchSanitizer.
  • manaflow-ai/cmux#6803 — Shares the Codex argv preservation path used for fork and resume handling.

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors)

Check name Status Explanation Resolution
Cmux User-Facing Error Privacy ❌ Error The new save-layout error inserts a captured command preview into user-visible alert text, which exposes raw argv/payload instead of minimal diagnostics. Remove the command preview from the alert (or redact it) and keep the message generic: the command exceeds the replay limit and must be shortened.
Cmux Full Internationalization ❌ Error New save-workspace xcstrings entries only have en/ja, but Resources/Localizable.xcstrings already supports 20 locales. Add translations for every existing locale in Resources/Localizable.xcstrings for the new save-workspace strings (commandTooLong* and the accessory copy).
✅ Passed checks (23 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed New UI and capture paths stay on @MainActor, and no new Sendable/shared-mutable or background-store isolation issue was introduced.
Cmux Swift Blocking Runtime ✅ Passed The changed Swift code adds sanitization/UI layout only; no semaphores, sleeps, dispatch syncs, locks, or other blocking waits were introduced.
Cmux Browser Automation Off-Main ✅ Passed PR changes CMUX agent launch, terminal capture, and save-dialog files only; no browser.* socket commands or control-policy files are touched.
Cmux Expensive Synchronous Load ✅ Passed PASS — touched Swift files add sanitization/UI only; no new RestorableAgentSessionIndex.load(), transcript/JSONL parsing, or other heavy sync loader on the interactive path.
Cmux Cache Substitution Correctness ✅ Passed Save path still uses fresh sysctl/proc PID reads via CmuxTopProcessSnapshot.allProcesses and commandLineArguments; no new captureCached/opportunistic substitution in the snapshot path.
Cmux No Hacky Sleeps ✅ Passed No changed non-Swift runtime/script code contains sleeps, timers, polling, or waits; the only timeout strings are test payload data.
Cmux Algorithmic Complexity ✅ Passed The new scans are linear over argv or one workspace snapshot; no nested full-collection rescans or repeated sorts/filters in hot scalable paths were introduced.
Cmux Swift Concurrency ✅ Passed Touched production Swift code is synchronous AppKit/string logic; no new DispatchQueue, Combine, Task, or new completion-handler APIs appeared, and tests stayed boundary-only.
Cmux Swift @Concurrent ✅ Passed No touched Swift file adds async/nonisolated/@Concurrent code; the new MainActor UI work is synchronous and intentionally UI-bound.
Cmux Swift File And Package Boundaries ✅ Passed PASS: The new Codex sanitizer file is 332 lines in the CMUXAgentLaunch package, and the app-target additions are focused AppKit/glue code, not oversized or boundary-breaking.
Cmux Swiftpm Lockfiles ✅ Passed Diff only touches Swift sources/tests; no Package.swift, Package.resolved, .gitignore, workflow, or cmux.xcodeproj dependency changes, so the lockfile rule isn’t implicated.
Cmux Swift Logging ✅ Passed No added production Swift logging APIs or file-scoped Logger constants appear in the diff; changes are sanitization/UI/test code only.
Cmux Swiftui State Layout ✅ Passed This commit only changes launcher logic and tests; no SwiftUI views, ObservableObject/@published state, GeometryReader layout, or render-time state mutation are introduced.
Cmux Architecture Rethink ✅ Passed No banned timing/lock/observer patterns appear; the save UI is owned by a single accessory object and argv sanitization stays on one shared path, so the change fits the allowed small-correctness/br...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed Diff only adds an NSView accessory and NSAlert sheets; no standalone NSWindow/NSPanel/NSWindowController/WindowGroup or cmuxAuxiliaryWindowIdentifiers changes.
Cmux Source Artifacts ✅ Passed All changed paths are intentional source/test/config/localization files; no logs, temp dirs, build output, caches, or other artifact paths appear in the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Touched Sources code adds production capture/sanitization helpers only; no new debug/test-only seam names or #if DEBUG accessors were introduced, and widened helpers are used cross-file in prod.
Cmux No Ambient Global State ✅ Passed PASS: The only new file-scope helpers in AgentLaunchSanitizerCodexLaunch.swift are private, and no new mutable globals, singletons, or top-level API were added.
Title check ✅ Passed The title is specific and clearly reflects the main changes: shared sanitization, replay failure on oversized commands, and save dialog layout fixes.
Description check ✅ Passed The description is detailed and covers the problem, fix, testing, and verification, though it omits the demo video, review trigger, and checklist sections.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-7631-save-layout-agent-commands

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes Save Workspace Layout replay for agent commands. The main changes are:

  • Shared sanitizer handling for captured Codex and Claude launch arguments.
  • Codex hook-argument stripping for cmux-injected hook prefixes.
  • Node and bun package-entrypoint handling during command capture.
  • A save-time limit for commands that cannot replay through a fresh shell.
  • A scrollable save-dialog accessory for command, URL, and environment details.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerCodexLaunch.swift Adds Codex-specific launch preservation and cmux hook-prefix stripping.
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/JavaScriptRuntimeAgentLaunchUnwrapper.swift Adds node and bun runtime handling for known agent package entrypoints.
Sources/TerminalForegroundCommandCapture.swift Routes captured foreground commands through runtime unwrapping and shared sanitization.
Sources/WorkspaceConfigActionCapture.swift Rejects saved workspace commands that exceed the replayable shell input limit.
Sources/WorkspaceActionSaveDialogAccessory.swift Adds the scrollable AppKit accessory used by the save dialog.

Reviews (27): Last reviewed commit: "Preserve captured agent executables for ..." | Re-trigger Greptile

Comment on lines +33 to +36
if arg == "--enable", index + 1 < args.count, args[index + 1] == "hooks" {
index += 2
continue
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 User Hooks Lose Enable Flag

When a Codex argv contains both cmux-injected hook config and a user-owned hook config, the cmux marker makes this branch remove --enable hooks globally while the user hook config is preserved. The saved command can then replay with -c hooks... but without enabling hooks, so the user's hook no longer runs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9e079ee: the stripper now removes exactly one instance of --enable hooks and one --dangerously-bypass-hook-trust (the single pair the cmux wrapper splices in alongside its marker configs), so a user's own enable flag and hooks.* config survive and stay enabled on replay. Regression test added: 'Keeps user hook enabling flags when cmux injection is stripped'.

— Claude Code

austinywang and others added 2 commits July 8, 2026 10:23
…modules installs

Two replay-correctness fixes from review of the workspace-action capture path:

- removingCmuxInjectedCodexHookArguments now strips exactly one instance of
  --enable hooks and --dangerously-bypass-hook-trust (the single pair the cmux
  wrapper splices in alongside its marker configs), so a user's own
  --enable hooks and hooks.* config survive and stay enabled on replay.
- unwrappedJavaScriptRuntimeAgentArgv now requires the script to live under a
  node_modules path component before rewriting node/bun argv to a bare agent
  name, so a user's local script named like an agent (node ./tools/claude.js)
  is never persisted as a different program.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up: the node_modules path component was still a heuristic — a
project-local pinned install (node /repo/node_modules/.../codex) launched
directly by the user would be rewritten to whatever bare `codex` resolves to
on replay.

Unwrapping now requires the argv to carry cmux's own injected hook arguments
(codex `-c hooks.*cmux-codex-hook*` configs or claude's injected --settings
markers). That is a deterministic launch-time signal: cmux only injects those
when the user invoked the agent by bare name through the per-surface PATH
shim, so replaying the bare name reproduces that launch exactly. Argv without
the marker — user scripts, explicit paths, project-local installs — is always
preserved verbatim.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerCodexLaunch.swift`:
- Around line 75-97: The argv unwrapping logic in
unwrappedJavaScriptRuntimeAgentArgv is using a non-exhaustive value-list for
Node/Bun option parsing, so unknown value-taking flags can shift script
detection and cause a false negative unwrap. Update the JavaScript runtime
argument inspection path, especially javaScriptRuntimeScriptArgumentIndex and
nodeOptionValueCount, so any unrecognized value-taking option is handled
conservatively as a value-bearing flag rather than misread as the script path;
preserve the current fail-closed behavior of
unwrappedJavaScriptRuntimeAgentArgv, isPackageInstalledScriptPath, and
isKnownAgentExecutableName.
- Around line 160-168: `looksLikeCodexSessionIdentifier` is too permissive
because the `"019"` fast path returns true without any charset checks. Update
this helper to validate the full token using the same identifier-safe rules as
the fallback branch, including the allowed character set and dash requirement,
before returning true. Keep the fix localized in
`AgentLaunchSanitizerCodexLaunch.looksLikeCodexSessionIdentifier`, since
`codexForkCommandSessionIndex` and `dropForkPositionals` depend on it for
correct argv parsing.

In
`@Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/CodexHookInjectionStrippingTests.swift`:
- Around line 1-295: Add a regression test covering the
`looksLikeCodexSessionIdentifier` false positive: in
`CodexHookInjectionStrippingTests`, exercise `AgentResumeArgv.codexForkCommand`
and/or `AgentResumeArgv.builtInKind` with a `fork` positional followed by a
prompt starting with a long non-hex string like “019. some prompt text ...”.
Verify the prompt is preserved as user input rather than being stripped or
treated as a session ID, using the existing `codexResumePreservation`-style
assertions to locate the bug.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 010207af-3024-4b78-95fa-824cfef5058f

📥 Commits

Reviewing files that changed from the base of the PR and between dbd3ff2 and 9e079ee.

📒 Files selected for processing (10)
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizer.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerCodexLaunch.swift
  • Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/CodexHookInjectionStrippingTests.swift
  • Resources/Localizable.xcstrings
  • Sources/AppDelegate+WorkspaceActionSave.swift
  • Sources/TerminalForegroundCommandCapture.swift
  • Sources/WorkspaceActionSaveDialogAccessory.swift
  • Sources/WorkspaceConfigActionCapture.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/CmuxConfigActionSaverTests.swift

…e cli.js

Review follow-up: Claude Code's real npm entrypoint is
.../@anthropic-ai/claude-code/cli.js, whose basename never matches an agent
name, so a cmux-wrapped node-hosted claude kept its injected --settings hook
payload in the captured command (and could trip the replay-length guard).

The cmux-injected marker that already gates unwrapping also identifies which
wrapper injected it, so when the script basename is not an agent name the
marker-derived name (codex/claude) is used. Basename still wins first so a
wrapped agent sharing another agent's hook plumbing unwraps to its own name.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread Sources/WorkspaceActionSaveDialogAccessory.swift
Aziz policy follow-up: pure helpers used only inside
AgentLaunchSanitizerCodexLaunch.swift are now file-scope private functions
instead of private/internal static members, keeping the AgentLaunchSanitizer
extension surface limited to the API its cross-file consumers call
(preservedCodexLaunchArguments, removingCmuxInjectedCodexHookArguments,
unwrappedJavaScriptRuntimeAgentArgv, codexForkCommand). Behavior-identical.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines +40 to +50
if !strippedEnableHooks, arg == "--enable", index + 1 < args.count, args[index + 1] == "hooks" {
strippedEnableHooks = true
index += 2
continue
}
if !strippedEnableHooks, arg == "--enable=hooks" {
strippedEnableHooks = true
index += 1
continue
}
if !strippedHookTrust, arg == "--dangerously-bypass-hook-trust" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 User Flags Still Strip

When a captured Codex argv contains a user --enable=hooks or --dangerously-bypass-hook-trust before the cmux-injected hook block, this loop removes the user flag first because it only checks that some cmux marker exists anywhere in the argv. The later cmux copy is then preserved instead. That can leave a preserved user -c hooks... config without the user-owned enable flag that was meant to activate it on replay. The removal needs to target the injected tuple rather than the first matching flag in the whole command.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rejected as non-actionable: --enable hooks and --dangerously-bypass-hook-trust are idempotent boolean flags, and the strip removes exactly one instance from the argv multiset. When both a user copy and the cmux copy are present, exactly one survives, so the replayed command is semantically identical regardless of which textual instance was dropped. Also, the real wrapper splices its injected args ahead of user args (CMUXCLI.emitCodexWrapperInjectArgs), so the first instance encountered is cmux's own.

— Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Both points land. The idempotency of boolean flags alone makes the "which copy survives" question moot, and if emitCodexWrapperInjectArgs always prepends cmux args before user args, the first-match ordering is an added guarantee on top of that. I'll withdraw the comment.

Shall I remember a rule like: "Don't flag single-instance removal of idempotent boolean flags (e.g. --enable hooks, --dangerously-bypass-hook-trust) when the codebase guarantees injected args are prepended before user args"?

…back

Review follow-up: the marker-derived fallback name could rewrite an unrelated
node script into an agent command if its argv happened to contain hook-looking
contents. The fallback now also requires the script to live inside the marker
agent's own npm package directory (node_modules/@anthropic-ai/claude-code/,
node_modules/@openai/codex/), so unknown scripts fail closed to verbatim
capture while the real cli.js entrypoint keeps unwrapping.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
}

private func isCmuxInjectedCodexHookConfigValue(_ value: String) -> Bool {
value.hasPrefix("hooks.") && value.contains("cmux-codex-hook")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Marker Match Drops User Hooks

This predicate treats any hooks.* config containing cmux-codex-hook as wrapper-injected. If a user hook calls or wraps a script with that name, such as a command under /Users/u/.cmux/hooks/cmux-codex-hook-stop.sh, the sanitizer removes the whole user -c hooks... pair while keeping the rest of the command. The saved layout then replays without the user's hook config. The check needs to identify the wrapper-emitted hook entries, not only the marker substring inside arbitrary user hook config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rejected as intentional: cmux owns the cmux-codex-hook namespace — /Users/u/.cmux/hooks/cmux-codex-hook-.sh are cmux-generated artifacts (CMUXCLI.writeCodexHookScript). A hooks. config pointing at them is cmux's injection (or a re-specification of it), and replay re-injects equivalent hooks fresh through the wrapper, so stripping is the correct behavior. Treating cmux-namespace hook entries as user data would reintroduce the issue-7631 stale-hook persistence this PR fixes.

— Claude Code

Review follow-up (P1): the cmux wrapper execs the resolved absolute agent
binary with its injected hook args, so a native codex/claude install captured
as an absolute path replayed the real binary directly, bypassing the
per-surface PATH shim — hooks were never re-injected and session tracking was
lost. When the pre-sanitization argv carries the cmux wrapper marker, the
saved executable is now the bare basename so replay re-enters the shim.
Marker-less argv (user typed the path themselves) keeps its executable
verbatim. Exposes AgentLaunchSanitizer.containsCmuxWrapperInjectedHookArguments
for the capture path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
austinywang and others added 2 commits July 8, 2026 11:31
Review follow-up (P1): the disclosure NSTextViews were created 1pt tall, and
NSText clamps vertical growth to maxSize, which defaults to the initial frame
size — so the sensitive commands/URLs/env text the dialog exists to disclose
rendered blank inside its scroll view. Apply the standard scrollable-text-view
setup (unbounded maxSize, container size, sizeToFit after assigning the
string) and add a regression test asserting the document views size to their
content.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CmuxConfigActionSaverTests.swift crossed the 500-line untracked budget
threshold; the dialog-accessory disclosure regression test now lives in
WorkspaceActionSaveDialogAccessoryTests.swift, wired into the cmuxTests
target (build file, file reference, group child, sources phase).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
}

private func isCmuxInjectedCodexHookConfigValue(_ value: String) -> Bool {
value.hasPrefix("hooks.") && value.contains("cmux-codex-hook")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 User Hooks Still Strip

This marker check still matches user-owned Codex hook configs whenever the serialized value contains cmux-codex-hook. If a user saves a hook command that wraps or calls a helper with that name, such as a script under /Users/u/.cmux/hooks/cmux-codex-hook-stop.sh, the sanitizer treats that -c hooks... pair as cmux-injected and removes it. The saved layout then replays without the user's hook config. The match needs to identify the wrapper-emitted hook entries instead of matching this substring inside arbitrary user hook config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rejected as intentional — same rationale as the sibling thread: the cmux-codex-hook namespace is cmux-reserved and the wrapper re-injects fresh hooks at replay, so stripping configs that reference cmux's own hook artifacts is correct and loses nothing.

— Claude Code

The hand-wired test file entries were not in normalized order;
scripts/normalize-pbxproj.py output restores the CI pbxproj guard.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
}

private func isCmuxInjectedCodexHookConfigValue(_ value: String) -> Bool {
value.hasPrefix("hooks.") && value.contains("cmux-codex-hook")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 User Hooks Still Strip

This predicate still treats any hooks.* value containing cmux-codex-hook as wrapper-injected. When a user runs Codex with their own hook command at a path like /Users/u/bin/cmux-codex-hook-wrapper.sh, the saved-layout sanitizer enters the stripping path and drops that entire -c hooks... pair. The replayed workspace then starts without the user's hook config. The marker check needs to identify the wrapper-emitted hook entries, not just this substring inside arbitrary user hook values.

Rule Used: Flag correctness-critical detection/identity deriv... (source)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rejected as a conscious tradeoff: a user naming their own script inside cmux's reserved cmux-codex-hook namespace (e.g. /Users/u/bin/cmux-codex-hook-wrapper.sh) is adversarial to the marker contract. A stricter structural match on the exact wrapper-emitted TOML would be fragile across wrapper format changes for an edge case whose worst-case impact is bounded (that hooks.* pair is dropped from the saved layout while replay re-injects cmux hooks via the wrapper).

— Claude Code

@austinywang
austinywang enabled auto-merge (squash) July 8, 2026 21:07
Comment on lines +53 to +54
let preservedTail = preservedCodexLaunchArguments(args: scriptTail, stripCmuxHooks: false) ?? []
return Array(argv.prefix(scriptIndex + 1)) + preservedTail

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Injected hooks persist

This branch keeps the cmux-injected Codex hook tail for package entrypoints. After unwrappedArgv returns node or bun plus the script path, TerminalForegroundCommandCapture classifies the executable as the runtime instead of codex, so the shared sanitizer is skipped. A saved layout for node .../@openai/codex/bin/codex --enable hooks --dangerously-bypass-hook-trust -c hooks.Stop=... can still write the injected hook config into the workspace command, or hit the 1000-byte save guard instead of saving a replayable command. This path needs to strip only the cmux-injected hook prefix while preserving user hook config.

Rule Used: Flag correctness-critical detection/identity deriv... (source)

if let packageAgentName {
switch packageAgentName {
case "codex":
let preservedTail = preservedCodexLaunchArguments(args: scriptTail, stripCmuxHooks: false) ?? []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Codex hooks persist

When capture sees node .../@openai/codex/bin/codex with cmux-injected hook args, this branch keeps the runtime and script path and passes stripCmuxHooks: false. The returned argv still starts with node, so the capture path does not classify it as Codex later and the shared Codex sanitizer never gets another chance to run. A saved layout can still persist the injected --enable hooks and hooks.*cmux-codex-hook* config, or fail the save through the 1000-byte command guard instead of saving a replayable command.

Suggested change
let preservedTail = preservedCodexLaunchArguments(args: scriptTail, stripCmuxHooks: false) ?? []
let preservedTail = preservedCodexLaunchArguments(args: scriptTail, stripCmuxHooks: true) ?? []

Comment on lines +119 to +121
if !sanitizedArgv.isEmpty, routesThroughWrapper {
sanitizedArgv[0] = agentKind
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Preserve explicit Codex paths
When a captured command uses an explicit Codex binary path with cmux hook args, this branch rewrites the saved command to bare codex. A layout saved from /opt/pinned/bin/codex --enable hooks ... --model gpt-5.5 can then restore through whatever codex resolves to on PATH instead of the pinned executable the user launched. The hook args can be stripped from the tail, but they should not change the executable identity unless a stronger wrapper marker proves the process came from the PATH shim.

Suggested change
if !sanitizedArgv.isEmpty, routesThroughWrapper {
sanitizedArgv[0] = agentKind
}
// Hook argv is user-controllable and only proves that cmux hook
// arguments were present, not that the executable came from the
// per-surface PATH shim. Keep explicit captured executables unless
// a stronger wrapper identity marker is available.

Rule Used: Flag correctness-critical detection/identity deriv... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a9eef49. Configure here.

Comment thread Sources/TerminalForegroundCommandCapture.swift Outdated
@austinywang
austinywang disabled auto-merge July 9, 2026 01:04
@austinywang
austinywang merged commit 68deb3d into main Jul 9, 2026
35 checks passed

This branch was successfully deployed

1 active deployment
Preview – cmux — 3288aca5 Deployed Jul 9, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant