Skip to content

Batch approval and prefix generalization for surface resume command alerts - #9028

Merged
azooz2003-bit merged 20 commits into
mainfrom
feat-resume-approval-batch
Jul 31, 2026
Merged

azooz2003-bit merged 20 commits into
mainfrom
feat-resume-approval-batch

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 28, 2026 •

Copy link
Copy Markdown
Collaborator

Restoring many closed agent sessions fired one modal alert per session. Prompt-created approval records covered the full command tokens, including the per-session id (claude --resume <id>), plus exact cwd and environment, so no stored approval ever matched the next session and every session prompted again.

Three changes, all on the existing signed approval-record store (matching and signing semantics untouched):

  1. "Apply to all" checkbox on the "Allow Resume Command?" proposal alert. SurfaceResumeCommandCanonicalizer.generalizedApprovalPrefix(forCommand:) trims the session-specific tail (claude --resume 5f0c… → claude --resume, codex resume 0197… → codex resume, handling cd-guards and env assignments). Checking the box writes the approval record with that generalized prefix, so the first answered alert silences the remaining queued proposals for the same agent in the same cwd/environment.

  2. Run All / Skip All on the "Run Resume Command?" restore alert. A sticky decision (SurfaceResumeRunPromptBatch, MainActor, app-lifetime) answers the remaining restore prompts without showing them. Scope is deliberately until app quit, no timers.

  3. "Don't ask again for this command" checkbox on the restore alert. On Run/Run All it promotes the binding's existing approval record to Auto-Restore, so later relaunches skip the prompt for that record entirely.

All four new strings are localized (en + ja). Tests extended in cmuxTests/SessionPersistenceTests.swift: prefix generalization cases, cross-session record matching with generalized prefixes, cwd rejection, proposal-prompt suppression via generalized .auto records, and sticky batch semantics.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Batch-approve and batch-run resume commands to stop repeated alerts when restoring many sessions. Approvals are folder-scoped, local-only, and shell-safe; prefixes generalize only when the session id is the sole unmatched token. Run prompts support Run All/Skip All and “Don’t ask again,” with decisions scoped to nested restore passes.

  • New Features

    • “Apply to all…” can save a folder-scoped generalized prefix when available; shows the exact quoted prefix.
    • Restore prompt adds Run All/Skip All with a sticky decision limited to the current (nested) restore pass; “Don’t ask again” sets Auto‑Restore on Run/Run All and Manual on Skip; localized in en and ja.
  • Bug Fixes

    • Scope and safety: approvals match only local resumes and only shell‑expansion‑safe commands; reject unsafe env flags/nested env, subshells/backticks/history controls, control operators, unquoted glob/brace/tilde/leading “=”; never generalize if trailing args follow the session id.
    • Records: reuse the same id for identical prefixes; remove subsumed exact records when generalizing while keeping unrelated exact rules; handle nil vs empty env; return nil and avoid writes when the store path is invalid.

Written for commit 8d84339. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added “Run All” and “Skip All” options for grouped resume approval prompts.
    • Added an option to stop asking about a specific command.
    • Resume approvals can now apply to matching commands within the current folder.
    • Added English and Japanese translations for the new prompt options.
  • Bug Fixes

    • Improved recognition of equivalent command formats for saved approval decisions.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Resume approval now supports generalized command-prefix scopes, localized batch prompt actions, and sticky run-all or skip-all decisions. Approval persistence, workspace prompting, project wiring, and session persistence tests were updated.

Changes

Surface resume approval flow

Layer / File(s) Summary
Command-prefix approval scope
Sources/SessionPersistence.swift, Sources/ControlSurfaceResumeTarget.swift, Resources/Localizable.xcstrings, cmuxTests/SessionPersistenceTests.swift
Commands are generalized into approval prefixes, and prompt results persist the selected policy with an optional prefix scope.
Sticky batch prompt decisions
Sources/SurfaceResumeRunPromptBatch.swift, Sources/Workspace.swift, Resources/Localizable.xcstrings, cmux.xcodeproj/project.pbxproj, cmuxTests/SessionPersistenceTests.swift
Resume prompts add Run All and Skip All choices, store sticky decisions, bypass later prompts, and compile the new batch state type.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: austinywang


Important

Pre-merge checks failed

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

❌ Failed checks (5 errors, 1 warning)

Check name Status Explanation Resolution
Cmux Swift Package Boundaries ❌ Error Sources/SessionPersistence.swift adds pure resume-command canonicalization and approval-record logic in the app target, though CmuxWorkspaces already owns shared restore-policy code. Extract SurfaceResumeCommandCanonicalizer and approval-record matching into a small package target (e.g. CmuxWorkspaces or CmuxSurfaceResumeCore), exposing SurfaceResumeCommandCanonicalizer; keep AppKit/app-lifecycle glue in `Sou...
Cmux User-Facing Error Privacy ❌ Error The new alert checkbox title renders generalizedApprovalPrefix, which can include env assignments and vendor command tokens, exposing implementation details in user-facing text. Sanitize or omit the rendered prefix in the alert; use a generic label and keep env/vendor details out of user-visible copy.
Cmux Full Internationalization ❌ Error Resources/Localizable.xcstrings adds 4 new keys with only en/ja, but the catalog already supports 20 locales, so translations are missing. Add translated stringUnit entries for every existing locale in Localizable.xcstrings (or remove unsupported locales) for each new key.
Cmux Architecture Rethink ❌ Error SurfaceResumeRunPromptBatch.shared adds process-global mutable UI state for restore prompts, a side channel that outlives the restore flow and can leak decisions across sessions. Scope batch decisions to a restore-coordinator value passed through shouldRunPromptedSurfaceResume, and reset it at restore completion instead of using a singleton.
Cmux No Ambient Global State ❌ Error PR adds SurfaceResumeRunPromptBatch.shared with mutable stickyDecision runtime state, a new singleton/global state used by Workspace. Move stickyDecision into an injectable app-owned controller and pass it through the prompt path instead of a shared singleton.
Description check ⚠️ Warning The description thoroughly explains the changes and rationale but omits the required Testing, Demo Video, Review Trigger, and Checklist sections. Add the required Testing, Demo Video, Review Trigger, and Checklist sections, including test results, manual verification, and review status.
✅ Passed checks (19 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 PASS: the new singleton is explicitly @MainActor, and production access goes through MainActor.assumeIsolated/on-main UI code; no new Sendable shared mutable type or background UI access.
Cmux Swift Blocking Runtime ✅ Passed The PR adds MainActor-stored sticky state and modal prompts, but no new semaphores/waits/sleeps/locks; only test-only waits appear.
Cmux Browser Automation Off-Main ✅ Passed Full PR diff only changes surface-resume approval/localization/tests; no browser.*, socketWorkerMethods, or processV2Command edits appear.
Cmux Expensive Synchronous Load ✅ Passed PASS: the PR only changes resume-prompt UI and approval canonicalization; no new expensive agent-history load was added to a main-actor or interactive path.
Cmux Cache Substitution Correctness ✅ Passed The only new cache is the transient SurfaceResumeRunPromptBatch UI decision; persistence still reads records from disk, with no fresh-read substitution.
Cmux No Hacky Sleeps ✅ Passed PR only changes Swift and localization files; no non-Swift runtime sleeps/timers were introduced.
Cmux Algorithmic Complexity ✅ Passed New loops only scan single command-token arrays or constant state; no scalable batch rescans or repeated hot-path sorting/filtering were introduced.
Cmux Swift Concurrency ✅ Passed No new legacy async patterns; the diff is AppKit/@mainactor UI state only, with no added background queues, Combine state, completion handlers, or fire-and-forget Tasks.
Cmux Swift @Concurrent ✅ Passed No new @concurrent or nonisolated async work was introduced; the new resume-prompt logic is synchronous and explicitly main-actor-bound.
Cmux Swiftpm Lockfiles ✅ Passed No .gitignore, Package.resolved, or Xcode package-reference changes appear in the diff; only Swift and localization files changed.
Cmux Swift Logging ✅ Passed The patch adds no new print/debugPrint/dump/NSLog/Logger in production Swift; existing Workspace NSLog calls are untouched, and test stdout is allowed.
Cmux Swiftui State Layout ✅ Passed No new SwiftUI state/layout anti-patterns were introduced; changes are AppKit alert/store logic and a plain MainActor singleton, with existing Workspace @Published state untouched.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR only adds NSAlert prompt options and batch state; no new standalone NSWindow/NSPanel/WindowGroup or identifiers, so the auxiliary-window close-shortcut rule isn’t triggered.
Cmux Source Artifacts ✅ Passed All changed paths are intentional source/config/localization/test assets; no logs, caches, temp dirs, build output, or other artifacts were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Changed production sources add only product behavior; no new #if DEBUG/test-guarded seam, debug/test-only accessor, or wrapper for tests.
Title check ✅ Passed The title clearly and concisely summarizes the pull request's primary changes: batch approval and generalized prefixes for surface resume alerts.
✨ 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 feat-resume-approval-batch

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.

@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: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmuxTests/SessionPersistenceTests.swift`:
- Around line 4883-4895: Update
SurfaceResumeCommandCanonicalizer.generalizedApprovalPrefix and its test
expectations so removing the session ID preserves trailing non-session arguments
such as --yolo in the normalized approval scope. Ensure matching does not widen
to every command sharing only codex resume; if the session ID cannot be removed
safely, fail closed instead.

In `@Sources/SessionPersistence.swift`:
- Around line 655-700: Update generalizedApprovalPrefix(forCommand:) to fail
closed unless the entire command matches the supported resume grammar. Reject
shell-composition syntax such as separators, redirections, substitutions, or
opaque trailing tokens, and reject unsupported env flags so commands like env -i
do not produce a prefix. Return nil whenever parsing cannot reliably identify
only the approved executable, supported options, and environment assignments.

In `@Sources/SurfaceResumeRunPromptBatch.swift`:
- Around line 3-17: Make SurfaceResumeRunPromptBatch constructable by removing
its global shared singleton and private-only initialization, while preserving
its app-lifetime sticky-decision behavior. Create and retain one
coordinator-owned instance at app lifecycle scope, then update Workspace prompt
handling to consume that instance’s decision and reset/action methods instead of
SurfaceResumeRunPromptBatch.shared; use the injected owner in tests as well.
Apply the corresponding change at Sources/Workspace.swift lines 2657-2665, with
no direct change needed elsewhere.

In `@Sources/Workspace.swift`:
- Around line 2710-2714: Update the suppression handling around the approval
prompt so it uses the active approval-store context rather than
`SurfaceResumeApprovalStore` defaults. When `alert.suppressionButton?.state ==
.on`, create and validate an auto-approval record if `binding.approvalRecordId`
is absent; otherwise update the existing record in that same store and handle
the update result.
🪄 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: a6408b5a-1f83-4ad2-bdde-bca8577b0573

📥 Commits

Reviewing files that changed from the base of the PR and between fe57d57 and 445ce5a.

📒 Files selected for processing (7)
  • Resources/Localizable.xcstrings
  • Sources/ControlSurfaceResumeTarget.swift
  • Sources/SessionPersistence.swift
  • Sources/SurfaceResumeRunPromptBatch.swift
  • Sources/Workspace.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/SessionPersistenceTests.swift

Comment thread cmuxTests/SessionPersistenceTests.swift
Comment thread Sources/SessionPersistence.swift Outdated
Comment thread Sources/SurfaceResumeRunPromptBatch.swift
Comment thread Sources/Workspace.swift Outdated
azooz2003-bit and others added 17 commits July 29, 2026 19:55
…batch

# Conflicts:
#	Sources/ControlSurfaceResumeTarget.swift
Regressions for the round-four review findings: env -i / env -u / nested
env wrappers must not become the scoped command, and commands with
arguments after the session id (codex resume <id> --yolo) must not
generalize to a wider prefix scope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
generalizedApprovalPrefix now rejects a command whose executable slot is
an option token or another env wrapper (env -i FOO=1 claude ... scoped
approval to bare 'env -i'), and only generalizes when the session id is
the sole unmatched token, so prefix matching can never re-authorize a
session launched with different trailing options.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@azooz2003-bit
azooz2003-bit merged commit 9112fe2 into main Jul 31, 2026
6 checks passed
@azooz2003-bit
azooz2003-bit deleted the feat-resume-approval-batch branch July 31, 2026 00:27
azooz2003-bit added a commit that referenced this pull request Jul 31, 2026
Absorb HQ broadcast baseline: iPhone install queue, phone-plus-simulator
mandate, Xcode 26.3 warning gate fix, resume alert batching (#9028).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant