-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
resume: hookless directory-scoped continue bindings for remote agents (#7989) #10049
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4794,8 +4794,27 @@ struct CMUXCLI { | |
| defaultValue: "[cmux] remote session was lost; starting a new shell." | ||
| ) | ||
| cliWriteStderr(Data((notice + "\n").utf8)) | ||
| var respawnArgs = stableAttachArgs.filter { $0 != "--require-existing" } | ||
| // Hookless remote resume (#7989): the app can synthesize a | ||
| // directory-scoped continue command from the daemon's | ||
| // foreground tombstone for the session that just died. Any | ||
| // failure degrades to the plain replacement shell. | ||
| if let resumeCommand = sessionLostResumeRemoteCommand( | ||
| client: client, | ||
| attachArgs: stableAttachArgs | ||
| ) { | ||
| let resumeNotice = String( | ||
| localized: "cli.sshPtyAttach.remoteSessionLostResume", | ||
| defaultValue: "[cmux] resuming the agent that was running in the lost session." | ||
| ) | ||
| cliWriteStderr(Data((resumeNotice + "\n").utf8)) | ||
| respawnArgs = replacingSSHPTYAttachCommandB64( | ||
| in: respawnArgs, | ||
| with: Data(resumeCommand.utf8).base64EncodedString() | ||
| ) | ||
| } | ||
| try runSSHPTYAttach( | ||
| commandArgs: stableAttachArgs.filter { $0 != "--require-existing" }, | ||
| commandArgs: respawnArgs, | ||
| client: client, | ||
| explicitPassword: socketPasswordArg | ||
| ) | ||
|
|
@@ -12801,6 +12820,67 @@ struct CMUXCLI { | |
| return ["workspace_id": workspaceId] | ||
| } | ||
|
|
||
| /// Asks the app for a hookless remote continue command (#7989) after the | ||
| /// wrapper confirmed the persistent session was lost. The daemon's | ||
| /// foreground tombstone supplies the facts; binding precedence, | ||
| /// auto-resume, and approval policy stay in the app. Every failure — | ||
| /// older app, no tombstone, policy says attach plain — returns nil so the | ||
| /// respawn degrades to the existing replacement-shell behavior. | ||
| private func sessionLostResumeRemoteCommand( | ||
| client: SocketClient, | ||
| attachArgs: [String] | ||
| ) -> String? { | ||
| let (workspaceOpt, _) = parseOption(attachArgs, name: "--workspace") | ||
| let (sessionIDOpt, _) = parseOption(attachArgs, name: "--session-id") | ||
| let (attachmentIDOpt, _) = parseOption(attachArgs, name: "--attachment-id") | ||
| let environmentSurfaceID = Self.normalizedEnvValue( | ||
| ProcessInfo.processInfo.environment["CMUX_SURFACE_ID"] | ||
| ) | ||
| let surfaceID = environmentSurfaceID | ||
| ?? Self.normalizedEnvValue(attachmentIDOpt).flatMap { UUID(uuidString: $0) == nil ? nil : $0 } | ||
| guard let workspaceID = Self.normalizedEnvValue(workspaceOpt) | ||
| ?? Self.normalizedEnvValue(ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"]), | ||
| let sessionID = Self.normalizedEnvValue(sessionIDOpt), | ||
| let surfaceID else { | ||
| return nil | ||
| } | ||
| // The app-side handler deliberately outwaits the coordinator's daemon | ||
| // bootstrap (bounded at 20s), so this call must outlast it. | ||
| guard let payload = try? client.sendV2( | ||
| method: "workspace.remote.pty_session_lost_resume", | ||
| params: [ | ||
| "workspace_id": workspaceID, | ||
| "surface_id": surfaceID, | ||
| "session_id": sessionID, | ||
| ], | ||
| responseTimeout: 30 | ||
| ) else { | ||
| return nil | ||
| } | ||
| guard let command = payload["command"] as? String, | ||
| !command.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { | ||
| return nil | ||
| } | ||
| return command | ||
| } | ||
|
Comment on lines
+12823
to
+12865
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win Normalize
This creates two different resolution paths for the same workspace identity within the same retry flow. Today every real caller of Resolve Based on path instructions: 🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
| /// Returns `args` with any `--command-b64 <value>` pair replaced by the | ||
| /// synthesized resume payload. | ||
| private func replacingSSHPTYAttachCommandB64(in args: [String], with base64: String) -> [String] { | ||
| var result: [String] = [] | ||
| var index = 0 | ||
| while index < args.count { | ||
| if args[index] == "--command-b64", index + 1 < args.count { | ||
| index += 2 | ||
| continue | ||
| } | ||
| result.append(args[index]) | ||
| index += 1 | ||
| } | ||
| result.append(contentsOf: ["--command-b64", base64]) | ||
| return result | ||
| } | ||
|
|
||
| private func runSSHPTYAttach(commandArgs: [String], client: SocketClient, explicitPassword: String?) throws { | ||
| let (workspaceOpt, rem0) = parseOption(commandArgs, name: "--workspace") | ||
| let (sessionIDOpt, rem1) = parseOption(rem0, name: "--session-id") | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,6 +32,9 @@ struct ControlCommandExecutionPolicyTests { | |||||||||||||||||||||||||||||
| "feed.push", "browser.download.wait", "system.top", "system.memory", | ||||||||||||||||||||||||||||||
| "workspace.remote.pty_bridge", "workspace.env", "sidebar.custom.reload", | ||||||||||||||||||||||||||||||
| "sidebar.custom.open", | ||||||||||||||||||||||||||||||
| // Tombstone-backed hookless resume outwaits daemon bootstrap and | ||||||||||||||||||||||||||||||
| // queries the tunnel; it must never hold the main actor (#7989). | ||||||||||||||||||||||||||||||
| "workspace.remote.pty_session_lost_resume", | ||||||||||||||||||||||||||||||
|
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win Pin The loop checks only worker routing. It passes if this method becomes Proposed test for method in [
// ...
"workspace.remote.pty_session_lost_resume",
] {
`#expect`(ControlCommandExecutionPolicy(forMethod: method).runsOnSocketWorker, "\(method)")
}
+ `#expect`(
+ ControlCommandExecutionPolicy(
+ forMethod: "workspace.remote.pty_session_lost_resume"
+ ) == .socketWorker(mainThreadCallable: false)
+ )📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| "debug.sidebar.simulate_drag", "debug.mobile.transport.disconnect", | ||||||||||||||||||||||||||||||
| "debug.window.screenshot", "mobile.attach_ticket.create", | ||||||||||||||||||||||||||||||
| "mobile.terminal.set_font", "mobile.task.models.list", | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import Foundation | ||
|
|
||
| /// Synthesizes a hookless, directory-scoped resume binding for a remote agent | ||
| /// (issue #7989, Tier 1). | ||
| /// | ||
| /// A remote host without cmux agent hooks never reports a session checkpoint, | ||
| /// so the strongest honest restore signal is "agent `<kind>` was running in | ||
| /// `<directory>`". This synthesizer turns that pair into a conservative | ||
| /// `cd <dir> && <agent> --continue || <agent>` command bound to the panel's | ||
| /// persistent-SSH PTY, mirroring how `TmuxResumeParser` turns a locally | ||
| /// observed tmux client into a `process-detected` binding. | ||
| /// | ||
| /// Known limitation (accepted for hookless remotes): a directory-scoped | ||
| /// continue command resumes whatever conversation the agent considers most | ||
| /// recent for that directory. When several sessions of the same agent share | ||
| /// one working directory, the wrong conversation can be continued. | ||
| enum RemoteAgentContinueSynthesizer { | ||
| /// `SurfaceResumeBindingSnapshot.source` value for synthesized bindings; | ||
| /// must match `SurfaceResumeBindingSnapshot.isRemoteSynthesized`. | ||
| static let source = "remote-synthesized" | ||
|
|
||
| /// Builds a directory-scoped continue binding, or nil when the agent kind | ||
| /// has no trustworthy sessionless continue invocation or the remote | ||
| /// working directory is unknown. | ||
| static func binding( | ||
| kind: RestorableAgentKind, | ||
| remoteWorkingDirectory: String?, | ||
| remoteContext: SurfaceResumeRemoteContext, | ||
| updatedAt: TimeInterval = Date().timeIntervalSince1970 | ||
| ) -> SurfaceResumeBindingSnapshot? { | ||
| guard let workingDirectory = normalized(remoteWorkingDirectory), | ||
| let continueCommand = directoryScopedContinueCommand(for: kind) else { | ||
| return nil | ||
| } | ||
| // Same cd guard as every other startup command | ||
| // (`TerminalStartupWorkingDirectoryPrefix`): tolerate a deleted saved | ||
| // directory instead of failing before the agent launches. The command | ||
| // runs on the remote host, so the local claude/codex wrapper-resolver | ||
| // tokens are deliberately not used here — mirroring | ||
| // `SurfaceResumeBindingSnapshot.remoteStartupInput()`, which renders | ||
| // remote commands with `repairPortableAgentExecutable: false`. | ||
| let command = TerminalStartupWorkingDirectoryPrefix.prefix( | ||
| continueCommand, | ||
| workingDirectory: workingDirectory | ||
| ) | ||
| return SurfaceResumeBindingSnapshot( | ||
| name: "\(kind.displayName) continue", | ||
| kind: kind.rawValue, | ||
| command: command, | ||
| cwd: workingDirectory, | ||
| source: source, | ||
| autoResume: true, | ||
| launchFlavor: .persistentSSH(remoteContext), | ||
| updatedAt: updatedAt | ||
| ) | ||
| } | ||
|
|
||
| /// Sessionless continue templates. Deliberately conservative: only agents | ||
| /// with a documented directory-level continue invocation are covered; the | ||
| /// per-session commands in `docs/agent-hooks.md` all need a checkpoint id | ||
| /// that a hookless remote cannot provide. The `|| <agent>` fallback starts | ||
| /// a fresh session when the agent has nothing to continue in that | ||
| /// directory, so restore never dead-ends on a continue error. | ||
| private static func directoryScopedContinueCommand( | ||
| for kind: RestorableAgentKind | ||
| ) -> String? { | ||
| switch kind { | ||
| case .claude: | ||
| // Continues the most recent conversation recorded for the current | ||
| // working directory (claude's session store is cwd-keyed). | ||
| return "claude --continue || claude" | ||
| case .codex: | ||
| // Resumes the most recently used codex session. | ||
| return "codex resume --last || codex" | ||
| default: | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| private static func normalized(_ value: String?) -> String? { | ||
| guard let trimmed = value?.trimmingCharacters(in: .whitespacesAndNewlines), | ||
| !trimmed.isEmpty else { | ||
| return nil | ||
| } | ||
| return trimmed | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
cmux Resources/Localizable.xcstrings supported locales policy💡 Result:
In the context of the cmux project by Manaflow (the open-source macOS terminal for coding agents), the policy regarding Resources/Localizable.xcstrings and internationalization (i18n) is one of strict, mandatory full coverage [1][2]. The project enforces a policy of full internationalization for all user-facing text [1][2]. This means that any addition or modification to user-facing text in the Swift codebase must be accompanied by a matching entry in the relevant Localizable.xcstrings catalog, and that entry must include translated values for every locale currently supported by that specific catalog [1][3][2]. Key aspects of this policy include: Enforcement: The project uses automated review-bot rules (specifically configured via CodeRabbit in.coderabbit.yaml) to enforce this policy [2]. Pre-merge checks fail if new or changed strings do not have translated entries for every supported locale in the catalog, or if the Info.plist and string catalogs are not kept in sync with the project's supported language list [1][2]. Catalog-Specific Coverage: 1. macOS Catalog (Resources/Localizable.xcstrings): This catalog supports a broad set of locales (e.g., 20 locales including Arabic, German, Spanish, French, Korean, Japanese, etc.) [3][4]. Updates to this catalog require that every new key added must include translations for all of these locales to prevent regressions where users see English fallback text [3]. 2. iOS Catalog (ios/cmux/Resources/Localizable.xcstrings): This catalog may support a different, often smaller, subset of locales (e.g., English and Japanese), and therefore requires complete coverage only for the locales defined as supported for that specific target [5][3]. Compliance: Developers are expected to keep the locale registry aligned across all surfaces (Swift code, string catalogs, and web messaging) [1]. Intentional removal or addition of a supported locale must be updated consistently across every affected message and catalog file [1]. Exceptions: The policy allows for narrow exceptions, such as for tests, operational documentation not shown to users, developer-only comments, debug-only logs, literal tokens, and pre-existing untranslated strings that are not worsened by a new change [2]. Note: This policy applies to the Manaflow cmux terminal project [6]. It should not be confused with the unrelated Go library also named cmux (github.com/soheilhy/cmux), which is a connection multiplexer [7].
Citations:
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 2280
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 3768
Add translations for all supported locales
Resources/Localizable.xcstringscontainscli.sshPtyAttach.remoteSessionLostResumeonly forenandja, but the catalog supports 20 locales. Add translated entries for the remaining 18 locales.🤖 Prompt for AI Agents
Source: Path instructions