From 122efa85e5f7ea59a0d99ac3586adc0cae825172 Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Wed, 13 May 2026 03:34:23 -0700 Subject: [PATCH 1/3] Improve Cloud VM error guidance --- .coderabbit.yaml | 7 + .github/review-bot-rules/README.md | 1 + .../review-bot-rules/user-facing-errors.md | 23 +++ .greptile/rules.md | 9 + CLI/cmux.swift | 195 ++++++++++++++++-- Resources/Localizable.xcstrings | 85 ++++++++ Sources/Cloud/VMClient.swift | 177 +++++++++++++++- Sources/Cloud/VMClientSocketCommands.swift | 16 +- Sources/CloudVMActionLauncher.swift | 27 ++- web/app/api/vm/[id]/exec/route.ts | 25 ++- web/app/api/vm/route.ts | 149 +++++++++---- web/services/vms/errors.ts | 8 + web/services/vms/routeHelpers.ts | 94 ++++++++- web/tests/vm-route-auth.test.ts | 84 ++++++-- 14 files changed, 800 insertions(+), 100 deletions(-) create mode 100644 .github/review-bot-rules/user-facing-errors.md diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 1c968a8499fd..86bae3bc157c 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -25,6 +25,9 @@ reviews: - path: "**/*.{ts,tsx,js,jsx,mjs,cjs,sh,zsh}" instructions: | Apply `.github/review-bot-rules/runtime-no-hacky-sleeps.md` during review. For production runtime, script, and build changes, flag fixed sleeps, timers, delayed dispatch, polling, or wall-clock waits used as synchronization. Pass for tests, pure presentation timing, dedicated cancellation-aware retry/timeout abstractions with tests, and existing delay code not worsened. + - path: "**/*.{swift,ts,tsx,js,jsx,mjs,cjs}" + instructions: | + Apply `.github/review-bot-rules/user-facing-errors.md` during review. For production user-facing errors, alerts, command output, API error bodies, and recovery copy, flag implementation leaks such as upstream vendor names, internal provider names, environment variables, database or migration details, raw upstream messages, internal billing ids, or unredacted payloads. pre_merge_checks: custom_checks: @@ -56,6 +59,10 @@ reviews: mode: error instructions: | For production Swift changes, fail when the diff violates `.github/review-bot-rules/swift-logging.md`: `print`, `debugPrint`, `dump`, or `NSLog` in app/runtime code; ad hoc file/stdout logging for diagnostics; MainActor-coupled file-scoped Logger constants; or logs that expose secrets or personal data. Do not require new logs for new code paths; only check logging that the diff adds or materially changes. Pass for CLI output, tests, debug-only logs, and explicitly sanitized provider diagnostics. + - name: "cmux user-facing error privacy" + mode: error + instructions: | + For production changes, fail when the diff violates `.github/review-bot-rules/user-facing-errors.md`: user-facing errors, alerts, command output, API error bodies, or recovery copy must not expose upstream vendor names, internal provider names, provider-specific flags, templates, snapshots, manifests, environment variables, database or migration details, raw upstream messages, billing item ids, billing customer ids, unrelated team ids, credentials, tokens, headers, private keys, refresh tokens, session ids, or unredacted payload dumps. Pass for tests, docs, operational runbooks, developer-only comments, safe generic terms like billing/team/Cloud VM service, and explicitly advanced help text for user-configured settings. - name: "cmux SwiftUI state layout" mode: error instructions: | diff --git a/.github/review-bot-rules/README.md b/.github/review-bot-rules/README.md index 537709ba0fdc..e82154fae151 100644 --- a/.github/review-bot-rules/README.md +++ b/.github/review-bot-rules/README.md @@ -18,5 +18,6 @@ Current rules: - `swift-file-package-boundaries.md` - `swift-logging.md` - `swiftui-state-layout.md` +- `user-facing-errors.md` Open source repository note: review bots should apply the configuration from the base branch. A PR that edits these rules should not be able to weaken its own review. diff --git a/.github/review-bot-rules/user-facing-errors.md b/.github/review-bot-rules/user-facing-errors.md new file mode 100644 index 000000000000..6c4f18479cc0 --- /dev/null +++ b/.github/review-bot-rules/user-facing-errors.md @@ -0,0 +1,23 @@ +# User-Facing Error Messages + +Flag production changes that add or materially change user-facing errors, alerts, command output, API error bodies, or recovery copy when they expose implementation details. + +Fail when user-facing text includes: + +- Upstream vendor or service names unless the user explicitly configured that vendor in the product UI. +- Internal provider names, provider-specific flags, templates, snapshots, manifests, environment variable names, database or migration details. +- Raw upstream error messages, stack traces, request ids from third-party systems, billing item ids, billing customer ids, or team ids unless the user supplied that exact id in the request. +- Secret material, credentials, tokens, headers, private keys, refresh tokens, session ids, or unredacted payload dumps. + +Expected shape: + +- State what happened in cmux/product terms. +- Give one or two concrete next actions the user can take. +- Put only safe, minimal diagnostics in `details`. +- Keep provider, billing, database, and auth implementation details in sanitized logs or internal telemetry, not in user-visible text. + +Allowed cases: + +- Developer-only comments, tests, docs, and operational runbooks that are not shown to end users. +- Existing public CLI flags or config keys in help text when the user asked for advanced configuration help. +- Generic terms such as "billing", "team", "Cloud VM service", or "Cloud VM state". diff --git a/.greptile/rules.md b/.greptile/rules.md index 64dcdd0eca83..afc1eecfb72e 100644 --- a/.greptile/rules.md +++ b/.greptile/rules.md @@ -15,3 +15,12 @@ Review production Swift and runtime changes for: - Production logging that bypasses unified logging or leaks sensitive data. - SwiftUI state and layout patterns that cause stale state, broad invalidation, or render-time mutation. - Architectural fixes that patch symptoms while leaving bad state representable. +- User-facing errors, alerts, command output, API error bodies, and recovery copy that expose implementation details. + +## User-Facing Error Messages + +For production user-facing errors, alerts, command output, API error bodies, and recovery copy, do not expose implementation details. + +Flag copy that includes upstream vendor or service names, internal provider names, provider-specific flags, templates, snapshots, manifests, environment variable names, database or migration details, raw upstream error messages, billing item ids, billing customer ids, team ids not supplied by the user, credentials, tokens, headers, private keys, refresh tokens, session ids, or unredacted payload dumps. + +Error copy should say what happened in cmux terms, provide concrete user actionables, and keep only safe minimal diagnostics in `details`. Provider, billing, database, and auth implementation details belong in sanitized logs or internal telemetry. diff --git a/CLI/cmux.swift b/CLI/cmux.swift index a88260c17d13..b531cf64a850 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -1788,12 +1788,22 @@ final class SocketClient { if let error = response["error"] as? [String: Any] { let code = (error["code"] as? String) ?? "error" let message = (error["message"] as? String) ?? "Unknown v2 error" - throw CLIError(message: "\(code): \(message)") + throw CLIError(message: formatV2Error(code: code, message: message)) } throw CLIError(message: "v2 request failed") } + private func formatV2Error(code: String, message: String) -> String { + if code == "vm_error" { + return message + } + if message.contains("\n") { + return "\(code):\n\(message)" + } + return "\(code): \(message)" + } + func streamV2( method: String, params: [String: Any] = [:], @@ -1936,7 +1946,12 @@ struct CMUXCLI { } let normalized = trimmed.lowercased() guard normalized == "e2b" || normalized == "freestyle" else { - throw CLIError(message: "vm new: unsupported provider '\(trimmed)'. Expected e2b or freestyle.") + throw CLIError(message: """ + vm new: unsupported Cloud VM provider override '\(trimmed)'. + + Try: + cmux vm new + """) } return normalized } @@ -2675,14 +2690,32 @@ struct CMUXCLI { let detach = hasFlag(rem1, name: "--detach") || hasFlag(rem1, name: "-d") let remaining = rem1.filter { $0 != "--detach" && $0 != "-d" } if let unknown = remaining.first(where: { $0.hasPrefix("--") }) { - throw CLIError(message: "vm new: unknown flag '\(unknown)'. Known flags: --image, --provider, --detach/-d") + throw CLIError(message: """ + vm new: unknown flag '\(unknown)'. + + Known flags: + --image + --provider + --detach, -d + + Try: + cmux vm new + """) } // Stray positional args (e.g. a typo like `cmux vm new myvm`) previously fell // through and still provisioned a VM. That silently costs the user money and // hides the typo. Reject them explicitly. if let extra = remaining.first(where: { !$0.hasPrefix("--") && $0 != "-d" }) { throw CLIError( - message: "vm new: unexpected argument '\(extra)'. vm new takes no positional args; use --image / --provider / --detach." + message: """ + vm new: unexpected argument '\(extra)'. + + `cmux vm new` does not take a VM name or positional arguments. + + Try: + cmux vm new + cmux vm new --detach + """ ) } let normalizedProvider = try Self.normalizedVMProvider(providerOpt) @@ -2733,7 +2766,12 @@ struct CMUXCLI { case "shell", "attach": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux \(command) shell ") + throw CLIError(message: """ + Usage: cmux \(command) shell + + Find an id: + cmux vm ls + """) } let shortId = String(vmId.prefix(8)) try vmOpenShell( @@ -2746,7 +2784,12 @@ struct CMUXCLI { case "rm", "destroy", "delete": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux vm rm ") + throw CLIError(message: """ + Usage: cmux vm rm + + Find an id: + cmux vm ls + """) } _ = try client.sendV2(method: "vm.destroy", params: ["id": vmId], responseTimeout: 60) if jsonOutput { @@ -2757,7 +2800,12 @@ struct CMUXCLI { case "ssh": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux \(command) ssh ") + throw CLIError(message: """ + Usage: cmux \(command) ssh + + Find an id: + cmux vm ls + """) } let shortId = String(vmId.prefix(8)) try vmOpenShell( @@ -2770,7 +2818,12 @@ struct CMUXCLI { case "ssh-info": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux \(command) ssh-info ") + throw CLIError(message: """ + Usage: cmux \(command) ssh-info + + Find an id: + cmux vm ls + """) } try printVMSSHInfo(id: vmId, command: command, client: client, jsonOutput: jsonOutput) @@ -2779,7 +2832,13 @@ struct CMUXCLI { case "exec": guard let vmId = rest.first else { - throw CLIError(message: "Usage: cmux vm exec -- ") + throw CLIError(message: """ + Usage: cmux vm exec -- + + Examples: + cmux vm ls + cmux vm exec -- pwd + """) } var commandArgsForVM: [String] = Array(rest.dropFirst()) // Consume a leading "--" separator if present. @@ -2787,7 +2846,12 @@ struct CMUXCLI { commandArgsForVM.removeFirst() } guard !commandArgsForVM.isEmpty else { - throw CLIError(message: "Usage: cmux vm exec -- ") + throw CLIError(message: """ + Usage: cmux vm exec -- + + Example: + cmux vm exec \(vmId) -- uname -a + """) } // Shell-quote each argv element before joining. Plain-space join previously // dropped quoting so `cmux vm exec -- printf '%s\n' "a b"` reached the @@ -2821,7 +2885,15 @@ struct CMUXCLI { } default: - throw CLIError(message: "Usage: cmux \(command) [args...]") + throw CLIError(message: """ + Usage: cmux \(command) [args...] + + Common commands: + cmux vm ls + cmux vm new + cmux vm ssh + cmux vm rm + """) } case "rpc": @@ -6236,7 +6308,16 @@ struct CMUXCLI { let endpoint = try parseVMPtyWebSocketEndpoint(response) guard endpoint.daemon != nil else { throw CLIError( - message: "vm.attach_info returned a WebSocket PTY without daemon/proxy support. Rebuild the cloud VM image or snapshot with the current cmuxd-remote." + message: """ + This Cloud VM image does not include the cmux daemon proxy needed for interactive attach. + + What to do: + Update cmux, then create a fresh VM with `cmux vm new`. + If this keeps happening, contact support with the VM id. + + Details: + vm.attach_info returned WebSocket PTY without daemon/proxy support. + """ ) } try runVMPtyWebSocketWorkspace( @@ -6281,19 +6362,49 @@ struct CMUXCLI { let cred = response["credential"] as? [String: Any], let kind = cred["kind"] as? String else { - throw CLIError(message: "vm.attach_info returned malformed SSH payload: \(response)") + throw CLIError(message: """ + cmux could not read the SSH attach information for this Cloud VM. + + What to do: + Retry `cmux vm ssh `. + If it keeps failing, recreate the VM with `cmux vm new` and share the details below. + + Details: + \(redactedCloudVMPayloadString(response)) + """) } guard kind == "password" else { if kind == "authorizedKey" { throw CLIError( - message: "authorizedKey credentials aren't supported by `cmux vm shell` yet; received from server." + message: """ + This Cloud VM returned SSH key credentials, but `cmux vm ssh` currently expects password-style gateway credentials. + + What to do: + Update cmux and retry. + If this keeps happening, contact support with the VM id. + """ ) } - throw CLIError(message: "vm.attach_info returned unknown credential kind: \(kind)") + throw CLIError(message: """ + cmux could not use the SSH credential returned for this Cloud VM. + + What to do: + Retry `cmux vm ssh `. + If it keeps failing, recreate the VM with `cmux vm new`. + + Details: + credential kind: \(kind) + """) } guard let token = cred["value"] as? String, !token.isEmpty else { - throw CLIError(message: "vm.attach_info password credential missing `value`") + throw CLIError(message: """ + The Cloud VM SSH gateway did not return a usable password token. + + What to do: + Retry `cmux vm ssh `. + If it keeps failing, recreate the VM with `cmux vm new`. + """) } // Freestyle gateway has a fresh host key per session and we re-mint per attach, @@ -6358,7 +6469,7 @@ struct CMUXCLI { private func runVMSSHAttach(commandArgs: [String], client: SocketClient) throws { let (vmIDOpt, remaining) = parseOption(commandArgs, name: "--id") if let unknown = remaining.first(where: { $0.hasPrefix("--") }) { - throw CLIError(message: "vm ssh-attach: unknown flag '\(unknown)'") + throw CLIError(message: "vm ssh-attach: unknown flag '\(unknown)'. Use `cmux vm ssh-attach --id `.") } guard remaining.isEmpty else { throw CLIError(message: "Usage: cmux vm ssh-attach --id ") @@ -6379,7 +6490,7 @@ struct CMUXCLI { ) let sshArguments = buildSSHCommandArguments(options) guard let launchPath = sshArguments.first else { - throw CLIError(message: "vm ssh-attach: failed to construct ssh command") + throw CLIError(message: "vm ssh-attach could not construct an ssh command. Retry `cmux vm ssh ` from a normal cmux shell.") } client.close() try execInteractiveProgram( @@ -6400,7 +6511,16 @@ struct CMUXCLI { guard let url = response["url"] as? String, let token = response["token"] as? String, let sessionId = response["session_id"] as? String else { - throw CLIError(message: "vm.attach_info websocket endpoint missing url/token/session_id: \(response)") + throw CLIError(message: """ + cmux could not read the WebSocket attach information for this Cloud VM. + + What to do: + Retry `cmux vm ssh `. + If it keeps failing, recreate the VM with `cmux vm new`. + + Details: + \(redactedCloudVMPayloadString(response)) + """) } let headers = parseHeaders(response["headers"]) let expiresAtUnix = (response["expires_at_unix"] as? Int64) @@ -6586,7 +6706,7 @@ struct CMUXCLI { private func runVMPtyAttach(commandArgs: [String], client: SocketClient) throws { let (vmIDOpt, remaining) = parseOption(commandArgs, name: "--id") if let unknown = remaining.first(where: { $0.hasPrefix("--") }) { - throw CLIError(message: "vm-pty-attach: unknown flag '\(unknown)'") + throw CLIError(message: "vm-pty-attach: unknown flag '\(unknown)'. Use `cmux vm-pty-attach --id `.") } guard remaining.isEmpty else { throw CLIError(message: "Usage: cmux vm-pty-attach --id ") @@ -9088,7 +9208,7 @@ struct CMUXCLI { Subcommands: ls List your cloud VMs. - new [--image