Repository navigation
Improve Cloud VM error guidance - #4094
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughVM API routes, CLI, and client error handling were changed to emit structured, redacted, and localized multi-line error responses with actionable guidance and consistent diagnostics. ChangesVM Error Response System and User Guidance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (12 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
lawrencecchen
force-pushed
the
feat-cloud-vm-actionable-errors
branch
from
May 13, 2026 10:36
f72ef17 to
62fb808
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/cmux.swift`:
- Around line 6370-6379: The error messages currently embed full backend
payloads via jsonString(response) (used inside the CLIError creation), which may
leak SSH tokens/headers/session secrets; replace these direct dumps by passing
the response through a redaction helper (e.g., redactSensitiveFields(response)
or sanitizeForLogs(response)) that strips or masks keys like "token", "value",
"authorization", "headers", cookies, secrets, etc., before calling jsonString;
update both places that call jsonString(response) to use the sanitizer and
ensure the helper is used wherever CLIError is constructed with backend
payloads.
- Around line 2893-2901: The usage string passed into CLIError for the vm
command is misleading because it lists only a subset of supported subcommands;
update the multiline message built where CLIError is thrown (the block that
references the local variable command) to either enumerate every accepted vm
subcommand (e.g., include ssh-info, attach and the delete aliases like
rm/delete/del, plus any others handled in the vm switch) or explicitly mark the
list as non-exhaustive (for example append “(and other subcommands)”); ensure
the updated message uses the same CLIError location and the same command
interpolation so users see an accurate summary of vm subcommands.
In `@Sources/Cloud/VMClient.swift`:
- Around line 129-141: cloudVMDetails(from:) drops the incoming
object["details"] when it is not a dictionary, losing valid non-object guidance;
update the function so that if object["details"] is a [String: Any] it merges as
before, but if it is any other non-nil JSON value (String, Array, Number, Bool)
it assigns that value into details["details"] (without overwriting existing
keys), then proceed to collect keys and use cloudVMIsNull and
cloudVMValueDescription as currently done; keep ignoredKeys and the existing
key-merge behavior for other top-level keys.
In `@Sources/Cloud/VMClientSocketCommands.swift`:
- Line 35: The parser currently accepts whitespace-only values for required
parameters (e.g., id and command); update the parsing/validation logic in
VMClientSocketCommands.swift to trim whitespace and reject empty strings at
parse time (same behavior as idempotency_key). Specifically, where you
extract/validate parameters for vm.destroy (id), vm.exec (id, command),
vm.start/run/etc. (id or command as noted at the diff locations), replace the
simple type check with a trimmed check like if let raw = param as? String, let
val = raw.trimmingCharacters(in: .whitespacesAndNewlines), !val.isEmpty { use
val } else { return v2Error(id: id, code: "invalid_params", message: "...
requires `id`/`command`...") } so whitespace-only inputs are rejected before
downstream processing.
In `@web/app/api/vm/route.ts`:
- Around line 270-279: The response currently exposes raw error text via
previousFailure: err.message in the vmErrorResponse for vm_create_failed; remove
the raw message from the client payload and return only stable, non-sensitive
context (e.g., idempotencyKeySet: !!err.idempotencyKey) and any fixed safe
code/flags. Instead, record the full err.message and stack in server
logs/tracing (e.g., use your logger or tracing span) inside the same error
handler so internal details are preserved for debugging but not sent to clients;
update the vmErrorResponse invocation in this handler (referencing
vmErrorResponse, the vm_create_failed case, and err/idempotencyKey) accordingly.
In `@web/services/vms/routeHelpers.ts`:
- Around line 108-147: The current vmWorkflowErrorResponse sends raw cause
strings from safeErrorMessage(err.cause) back to clients; update the
provider/database/billing branches (within vmWorkflowErrorResponse and checks
isVmProviderOperationError, isVmDatabaseError, isVmBillingError) to stop
returning raw cause text in details (remove
providerMessage/databaseMessage/billingMessage or replace with a generic
non-sensitive note like "See server logs" or a non-sensitive error_id), keep
only safe actionable metadata (provider, operation), and instead record the full
err.cause to server logs/telemetry (use the existing logger/telemetry helper) so
internal details are available for debugging without exposing them in
vmErrorResponse.
🪄 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: 8bd873ea-505d-4c9d-8b58-33712fdf1027
📒 Files selected for processing (10)
CLI/cmux.swiftResources/Localizable.xcstringsSources/Cloud/VMClient.swiftSources/Cloud/VMClientSocketCommands.swiftSources/CloudVMActionLauncher.swiftweb/app/api/vm/[id]/exec/route.tsweb/app/api/vm/route.tsweb/services/vms/errors.tsweb/services/vms/routeHelpers.tsweb/tests/vm-route-auth.test.ts
Contributor
Greptile SummaryThis PR standardizes Cloud VM error handling end-to-end — introducing structured
Confidence Score: 5/5Safe to merge — changes are additive and defensive; the allowlist-filtered The error-format changes are well-contained behind the No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant App as macOS App
participant CLI as cmux CLI
participant Swift as VMClient.swift
participant API as Web API (Next.js)
User->>App: Start Cloud VM
App->>CLI: cmux vm new
CLI->>Swift: VMClient.shared.create(...)
Swift->>API: POST /api/vm
alt Validation error (400)
API-->>Swift: "vmErrorResponse { error, message, action, details }"
Swift-->>CLI: formattedCloudVMHTTPError → "What to do: ..."
CLI-->>App: formatted output (sanitizedCloudVMStartOutput filters)
App-->>User: NSAlert: "What to try" + sanitized Details
else Provider/DB failure (502/503)
API-->>Swift: "vmWorkflowErrorResponse { vm_cloud_service_unavailable }"
Swift-->>CLI: "Cloud VM service could not complete this request."
CLI-->>App: formatted output
App-->>User: NSAlert with localized action string
else Billing error (402/503)
API-->>Swift: "vmErrorResponse { vm_create_credits_insufficient }"
Swift-->>CLI: "This team has no Cloud VM create credits left."
CLI-->>App: formatted output
App-->>User: NSAlert with "What to try" action
else V2 socket error
CLI->>CLI: formatV2Error(code, message, action, safeV2Details)
CLI-->>User: Reason / What to do / Details (allowlisted keys only)
end
Reviews (9): Last reviewed commit: "Narrow Cloud VM sanitizer env var block" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
lawrencecchen
force-pushed
the
feat-cloud-vm-actionable-errors
branch
from
May 13, 2026 10:51
62fb808 to
6d952d4
Compare
lawrencecchen
force-pushed
the
feat-cloud-vm-actionable-errors
branch
2 times, most recently
from
May 13, 2026 11:20
dfc6c87 to
0fbd859
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Cloud/VMClient.swift (1)
268-276:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent generic vs. field-specific error messages in
list().Only the
providerbranch (Line 272) was updated to the generic "missing required fields for item …" wording. The neighboring guards on Lines 269 and 275 still emit field-specific messages (missingidin /api/vm item …,missingimagein /api/vm item …). If the intent of this PR was to stop listing specific missing fields in user-facing errors (per the AI summary and the user-facing-errors rule), the other two guards should be normalized as well — otherwise users see a mix of generic and implementation-flavored messages from the same call site depending on which field happened to be missing.Proposed fix
- guard let id = dict["id"] as? String, !id.isEmpty else { - throw VMClientError.malformedResponse("missing `id` in /api/vm item \(index)") - } - guard let provider = dict["provider"] as? String, !provider.isEmpty else { - throw VMClientError.malformedResponse("Cloud VM list response was missing required fields for item \(index).") - } - guard let image = dict["image"] as? String, !image.isEmpty else { - throw VMClientError.malformedResponse("missing `image` in /api/vm item \(index)") - } + guard let id = dict["id"] as? String, !id.isEmpty else { + throw VMClientError.malformedResponse("Cloud VM list response was missing required fields for item \(index).") + } + guard let provider = dict["provider"] as? String, !provider.isEmpty else { + throw VMClientError.malformedResponse("Cloud VM list response was missing required fields for item \(index).") + } + guard let image = dict["image"] as? String, !image.isEmpty else { + throw VMClientError.malformedResponse("Cloud VM list response was missing required fields for item \(index).") + }🤖 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 `@Sources/Cloud/VMClient.swift` around lines 268 - 276, The guard checks in VMClient.list() are emitting mixed error messages; update the guards that throw VMClientError.malformedResponse for `id` and `image` so they use the same generic message as the `provider` guard (e.g. "Cloud VM list response was missing required fields for item \(index).") instead of field-specific texts, ensuring all three guards (the ones checking `id`, `provider`, and `image`) produce a consistent, non-implementation-specific error.
🤖 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/cmux.swift`:
- Around line 6311-6320: The user-facing error message (the multiline string
assigned to message in the cmux.swift diff) leaks internal implementation
details like vm.attach_info, WebSocket PTY, and daemon/proxy; replace the
"Details" sentence with a product-level, non-technical statement (e.g., "This VM
image lacks the necessary support for interactive attach") and keep the "What to
do" guidance intact, so update the message literal to remove internal symbols
and wording while preserving actionable next steps.
- Around line 12049-12074: The redaction is case-sensitive in
redactedCloudVMPayload(_:) and can miss mixed-case sensitive keys; normalize
keys (e.g., let normalized = pair.key.trimmingCharacters(in:
.whitespacesAndNewlines).lowercased()) before checking against the sensitiveKeys
set and use that normalized key to decide redaction, and also add pattern checks
(e.g., endsWith "token"/"password" or contains
"auth"/"credential"/"secret"/"private") to catch common variants; apply the same
normalization when recursing for dictionaries and arrays so Authorization,
Token, privateKeyPem, etc., are reliably redacted.
In `@Sources/Cloud/VMClient.swift`:
- Around line 184-191: The truncation logic in limitedSingleLine uses Swift
Characters (grapheme clusters) for both counting and slicing (singleLine.count
and index(_:offsetBy:)) which is grapheme-safe but may not match UTF-16/byte
limits or a desired per-message cap; either make the metric explicit or change
it: decide whether you want Character-based truncation (keep current logic and
document that maxLength is a per-field grapheme limit) or switch to UTF-16-based
truncation by using singleLine.utf16.count and slicing via a UTF-16-aware
conversion so both the count check and the truncation use the same UTF-16
metric; also consider adding or enforcing a separate total payload cap outside
limitedSingleLine (e.g., in the caller that builds Details) if you need a
per-message size limit rather than the per-field maxLength.
- Around line 162-178: The cloudVMValueDescription function currently treats
JSON booleans as NSNumbers and prints them as "0"/"1"; update
cloudVMValueDescription to check for CFBoolean-backed NSNumber before the
generic NSNumber branch by using CFBooleanGetTypeID() (or equivalent) to detect
booleans and return "true" or "false" (via limitedSingleLine) when detected;
leave cloudVMIsNull, limitedSingleLine, and the JSONSerialization branch
unchanged and only reorder/add the boolean-detection block so booleans render
correctly in Details output.
In `@Sources/CloudVMActionLauncher.swift`:
- Around line 114-117: The code currently appends raw process output
(limitedOutput) into the user-facing details (sections -> informativeText);
instead, sanitize or replace that output before appending: validate
limitedOutput against a strict allowlist (e.g., only basic safe tokens, short
alphanumeric and punctuation, no URLs/emails/IPs/billing/account IDs) and redact
any matches of provider, billing, auth, DB, or secret patterns, or if content
fails validation replace it with a generic safe message such as "Additional
technical details are available in logs." Update the logic where limitedOutput
is used (the block that builds sections and informativeText) so only the
sanitized/replaced string is appended to sections.
---
Outside diff comments:
In `@Sources/Cloud/VMClient.swift`:
- Around line 268-276: The guard checks in VMClient.list() are emitting mixed
error messages; update the guards that throw VMClientError.malformedResponse for
`id` and `image` so they use the same generic message as the `provider` guard
(e.g. "Cloud VM list response was missing required fields for item \(index).")
instead of field-specific texts, ensuring all three guards (the ones checking
`id`, `provider`, and `image`) produce a consistent, non-implementation-specific
error.
🪄 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: 4c4772c3-84fe-4aa0-beca-bf47b4a679a9
📒 Files selected for processing (14)
.coderabbit.yaml.github/review-bot-rules/README.md.github/review-bot-rules/user-facing-errors.md.greptile/rules.mdCLI/cmux.swiftResources/Localizable.xcstringsSources/Cloud/VMClient.swiftSources/Cloud/VMClientSocketCommands.swiftSources/CloudVMActionLauncher.swiftweb/app/api/vm/[id]/exec/route.tsweb/app/api/vm/route.tsweb/services/vms/errors.tsweb/services/vms/routeHelpers.tsweb/tests/vm-route-auth.test.ts
lawrencecchen
force-pushed
the
feat-cloud-vm-actionable-errors
branch
from
May 13, 2026 11:40
0fbd859 to
b147bcb
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
CLI/cmux.swift (3)
6311-6320:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep attach/SSH errors in product terms, not proxy/auth internals.
These messages still expose internal mechanics in user-facing copy:
cmux daemon proxy,authorizedKey,password-style gateway credentials,credential kind, andpassword token. The recovery steps are good; the problem is the explanatory text should stay at the product level.As per coding guidelines: "Fail changes that expose implementation details, including upstream/vendor/service names ... and auth implementation details ... Expected copy shape: (1) describe what happened in 'cmux/product terms', (2) provide 1–2 concrete next actions, (3) include only safe minimal diagnostics in the
detailssection."Also applies to: 6379-6407
🤖 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/cmux.swift` around lines 6311 - 6320, The user-facing error string in the multiline `message` literal exposes implementation/internal auth terms (e.g., "cmux daemon proxy", "authorizedKey", "password-style gateway credentials", "credential kind", "password token"); rewrite this `message` to use product-level language only: state the high-level problem in cmux/product terms, keep the two recovery actions ("update cmux and recreate VM" and "contact support with VM id"), and replace the Details section with a minimal safe diagnostic (e.g., "Interactive attach is not available for this VM image" and an opaque error code or VM id) without mentioning proxy/auth internals; apply the same change pattern to the similar multiline message at the other block referenced (around lines 6379–6407).
12045-12101:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRedacted payload dumps are still too much for end-user
Details.This helper fixes the raw-secret leak, but it still turns whole backend payloads into user-visible terminal output. That exposes internal field names/state and encourages users to paste backend payloads into bug reports. Keep the full payload in sanitized logs/telemetry instead, and limit CLI
Detailsto a short safe diagnostic.As per coding guidelines: "include only safe minimal diagnostics in the
detailssection—move provider/billing/database/auth specifics to sanitized logs/telemetry."🤖 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/cmux.swift` around lines 12045 - 12101, The CLI currently exposes full redacted backend payloads via redactedCloudVMPayloadString/redactedCloudVMPayload (and uses cloudVMRedactionKeyIsSensitive) which leaks internal state to end-user Details; change the CLI Details path to return a minimal, safe diagnostic string (e.g., "payload redacted; top-level keys: [k1,k2] (N fields)" where the displayed key list is truncated to a small whitelist and filtered through cloudVMRedactionKeyIsSensitive) instead of the full redacted JSON, and send the full redactedCloudVMPayload output only to sanitized logs/telemetry; update redactedCloudVMPayloadString to produce that short summary and ensure callers that render CLI Details use it while logging the full redactedCloudVMPayload elsewhere.
2888-2896:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUsage synopsis still looks exhaustive.
The new “Common commands” block helps, but the first
Usage:line still advertises only a subset of supportedvmsubcommands even though this switch also acceptsattach,ssh-info, and delete aliases. Use<subcommand>there, or explicitly mark the list as non-exhaustive.🤖 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/cmux.swift` around lines 2888 - 2896, The Usage line in the thrown CLIError currently lists a subset of vm subcommands; update the message built where CLIError(message: """ ... """) is created (the multi-line string that begins with "Usage: cmux \(command) <ls|new|shell|rm|exec|ssh> [args...]") to either replace the explicit pipe-list with a generic token like "<subcommand>" or append a clear note that the listed commands are non‑exhaustive (e.g., "common subcommands shown"). Keep the rest of the example lines unchanged.
🤖 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/cmux.swift`:
- Around line 2692-2718: The unknown-flag detection only checks for tokens
starting with "--" so short options like "-x" are treated as positional args;
update the predicate that finds unknown flags (the remaining.first(where: { ...
}) that currently uses $0.hasPrefix("--")) to treat any token beginning with "-"
as a flag while still excluding allowed values (e.g. "-" and "-d") and known
long flags as appropriate; throw the same CLIError for that matched token (the
unknown variable) so unsupported short options are reported via the unknown-flag
branch instead of the unexpected-argument branch.
- Around line 1788-1805: The response error handling in the sendV2 flow only
extracts code and message and then throws CLIError, which discards structured
fields like action/reason/details; update the block that inspects
response["error"] to also extract "action", "reason", and "details" (if present)
and include them in the thrown error so callers can access recovery
guidance—either by passing them into CLIError as dedicated properties or by
enriching the formatted text returned by formatV2Error; also update
formatV2Error (used by the throw) to accept and render action/reason/details in
a compact, user-facing way (e.g., append a "Next steps:" or "Details:" section)
while preserving existing code/message formatting.
In `@Sources/Cloud/VMClient.swift`:
- Around line 129-147: The cloudVMDetails function currently merges arbitrary
keys into details; replace that with an explicit allowlist of safe display keys
(e.g., let displayableKeys: Set =
["code","status","reason","operation","region","instance","message"] or similar
per policy) and only copy keys present in displayableKeys when iterating
nestedDetails and the top-level object (and when adding a non-object
rawDetails). Update the loops that reference ignoredKeys to check
displayableKeys.contains(key) instead (or check both to be extra safe) and
ensure cloudVMValueDescription is only called for allowlisted keys so no
provider/billing/credential/internal fields are ever forwarded to users from
cloudVMDetails.
In `@web/app/api/vm/`[id]/exec/route.ts:
- Around line 43-49: The current request validation in the exec route accepts
whitespace-only commands; update the check around body.command in route.ts to
trim the string before validating so that values like " " are rejected—i.e.,
use body.command.trim() when testing length and type and return the existing
vmErrorResponse({ error: "vm_invalid_command", ... }) when the trimmed length is
zero so whitespace-only commands fail fast; ensure you reference the same
validation block that currently returns vmErrorResponse for invalid commands.
---
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 6311-6320: The user-facing error string in the multiline `message`
literal exposes implementation/internal auth terms (e.g., "cmux daemon proxy",
"authorizedKey", "password-style gateway credentials", "credential kind",
"password token"); rewrite this `message` to use product-level language only:
state the high-level problem in cmux/product terms, keep the two recovery
actions ("update cmux and recreate VM" and "contact support with VM id"), and
replace the Details section with a minimal safe diagnostic (e.g., "Interactive
attach is not available for this VM image" and an opaque error code or VM id)
without mentioning proxy/auth internals; apply the same change pattern to the
similar multiline message at the other block referenced (around lines
6379–6407).
- Around line 12045-12101: The CLI currently exposes full redacted backend
payloads via redactedCloudVMPayloadString/redactedCloudVMPayload (and uses
cloudVMRedactionKeyIsSensitive) which leaks internal state to end-user Details;
change the CLI Details path to return a minimal, safe diagnostic string (e.g.,
"payload redacted; top-level keys: [k1,k2] (N fields)" where the displayed key
list is truncated to a small whitelist and filtered through
cloudVMRedactionKeyIsSensitive) instead of the full redacted JSON, and send the
full redactedCloudVMPayload output only to sanitized logs/telemetry; update
redactedCloudVMPayloadString to produce that short summary and ensure callers
that render CLI Details use it while logging the full redactedCloudVMPayload
elsewhere.
- Around line 2888-2896: The Usage line in the thrown CLIError currently lists a
subset of vm subcommands; update the message built where CLIError(message: """
... """) is created (the multi-line string that begins with "Usage: cmux
\(command) <ls|new|shell|rm|exec|ssh> [args...]") to either replace the explicit
pipe-list with a generic token like "<subcommand>" or append a clear note that
the listed commands are non‑exhaustive (e.g., "common subcommands shown"). Keep
the rest of the example lines unchanged.
🪄 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: 497b53cf-3580-43a0-8318-4b5c2089194f
📒 Files selected for processing (14)
.coderabbit.yaml.github/review-bot-rules/README.md.github/review-bot-rules/user-facing-errors.md.greptile/rules.mdCLI/cmux.swiftResources/Localizable.xcstringsSources/Cloud/VMClient.swiftSources/Cloud/VMClientSocketCommands.swiftSources/CloudVMActionLauncher.swiftweb/app/api/vm/[id]/exec/route.tsweb/app/api/vm/route.tsweb/services/vms/errors.tsweb/services/vms/routeHelpers.tsweb/tests/vm-route-auth.test.ts
lawrencecchen
force-pushed
the
feat-cloud-vm-actionable-errors
branch
from
May 13, 2026 12:01
b147bcb to
1e765cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
Sources/Cloud/VMClient.swift (1)
129-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestrict
Detailsoutput to a safe allowlist before rendering.
cloudVMDetails(from:)currently forwards arbitrary response keys from both top-level JSON and nesteddetailsinto user-visible output. That can re-expose provider/billing/auth/internal fields if backend payloads drift. Please switch this to an explicit safe-key allowlist (or strict denylist + allowlist fallback) before formatting.As per coding guidelines: user-facing errors must not expose implementation details (provider/internal/billing/auth/raw payload identifiers), and should include only safe minimal diagnostics.
🤖 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 `@Sources/Cloud/VMClient.swift` around lines 129 - 147, cloudVMDetails(from:) currently copies arbitrary top-level and nested "details" keys into the user-visible output; change this to use an explicit allowlist of safe keys (e.g., let allowedKeys = Set(["code","status","type","duration","host","region","message"])) and only copy keys present in allowedKeys from nestedDetails and the top-level object into the details dictionary (keep using cloudVMIsNull to filter nulls and cloudVMValueDescription to render values); if no allowed keys are present, optionally include a single sanitized fallback like "details: unavailable" or a generic message instead of exposing raw keys.CLI/cmux.swift (2)
1788-1805:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve structured v2 guidance fields when formatting errors.
sendV2still discardsaction,reason, anddetails, so users lose the backend’s recovery guidance even when it is present.Suggested fix
- 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: formatV2Error(code: code, message: message)) + if let error = response["error"] as? [String: Any] { + let code = (error["code"] as? String) ?? "error" + let message = (error["message"] as? String) ?? "Unknown v2 error" + let action = error["action"] as? String + let reason = error["reason"] as? String + let details = error["details"] as? String + throw CLIError( + message: formatV2Error( + code: code, + message: message, + action: action, + reason: reason, + details: details + ) + ) } @@ - 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)" + private func formatV2Error( + code: String, + message: String, + action: String? = nil, + reason: String? = nil, + details: String? = nil + ) -> String { + var lines: [String] = [code == "vm_error" ? message : "\(code): \(message)"] + if let reason, !reason.isEmpty { + lines += ["", "Reason:", " \(reason)"] + } + if let action, !action.isEmpty { + lines += ["", "What to do:", " \(action)"] + } + if let details, !details.isEmpty { + lines += ["", "Details:", " \(details)"] + } + return lines.joined(separator: "\n") }🤖 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/cmux.swift` around lines 1788 - 1805, The current sendV2 path extracts only error.code and error.message and loses structured guidance (action, reason, details); update sendV2 to extract error["action"], error["reason"], and error["details"] (if present) and pass them into formatV2Error, then throw CLIError with the richer string; change formatV2Error signature (e.g., formatV2Error(code:message:action:reason:details:)) to append the action/reason/details in a readable, newline-separated way (or include them as a JSON snippet) while preserving the existing vm_error and multiline behavior so users retain backend recovery guidance when present.
2692-2708:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUnknown short flags are still misclassified as positional arguments.
Flag checks only match
--..., so unsupported short options like-xfall through to positional/usage errors instead of the unknown-flag path.Suggested fix pattern
- if let unknown = remaining.first(where: { $0.hasPrefix("--") }) { + if let unknown = remaining.first(where: { $0.hasPrefix("-") && $0 != "-" && $0 != "-d" }) {Apply the equivalent predicate in
vm ssh-attachandvm-pty-attachunknown-flag branches as well.Also applies to: 6479-6483, 6716-6720
🤖 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/cmux.swift` around lines 2692 - 2708, The unknown-flag branch currently only checks remaining.first(where: { $0.hasPrefix("--") }) so short flags like "-x" get treated as positional; update that predicate to detect any token that starts with "-" (e.g. hasPrefix("-")) but exclude a lone "-" and allow known short flags (e.g. "-d") so unknown short flags are caught and thrown as CLIError (the variable/branch using remaining.first(where: { ... }) that binds to `unknown`); do the analogous change to the stray-positional check that binds `extra` (so it ignores known short flags but treats any other leading "-" as a flag), and apply the same fix in the vm ssh-attach and vm-pty-attach unknown-flag branches referenced in the review.web/app/api/vm/[id]/exec/route.ts (1)
42-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrim
commandbefore validating to reject whitespace-only values.The validation on Line 43 accepts whitespace-only strings like
" "because it checksbody.command.length === 0without trimming first. Trim the command before the length check so empty/whitespace-only inputs fail withvm_invalid_command.🔧 Suggested fix
const body = rawBody as { command?: unknown; timeoutMs?: unknown }; -if (typeof body.command !== "string" || body.command.length === 0) { +const command = typeof body.command === "string" ? body.command.trim() : ""; +if (command.length === 0) { return vmErrorResponse({ error: "vm_invalid_command", status: 400, message: "`command` is required and must be a non-empty string.", action: "Pass a shell command, for example `cmux vm exec <id> -- uname -a`.", details: { field: "command" }, }); }Then update references to use the trimmed
command:setSpanAttributes(span, { "cmux.vm.id": id, - "cmux.command_length": body.command.length, + "cmux.command_length": command.length, "cmux.timeout_ms": timeoutMs, }); try { const result = await runVmWorkflow(execVm({ userId: user.id, providerVmId: id, - command: body.command, + command, timeoutMs, }));🤖 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 `@web/app/api/vm/`[id]/exec/route.ts around lines 42 - 50, Trim the incoming command before validating and using it: when reading rawBody into body (the variable `body` in route handler) compute a trimmed value (e.g., `const command = String(body.command).trim()`) and then validate that `command` is non-empty; if empty return the existing `vmErrorResponse` with `vm_invalid_command`. Replace subsequent uses of `body.command` in this handler with the trimmed `command` so whitespace-only inputs are rejected and the executed command is the trimmed version.
🤖 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 @.coderabbit.yaml:
- Around line 62-65: Update the "cmux user-facing error privacy" custom check
instructions to explicitly include "stack traces" and "request ids from
third-party systems" in the list of prohibited disclosures so the rule matches
the canonical `.github/review-bot-rules/user-facing-errors.md`; modify the
instructions block under the rule name "cmux user-facing error privacy" to add
those two phrases to the prohibited items alongside the existing entries (e.g.,
after "raw upstream messages"), ensuring the natural language list now includes
both terms.
In `@Sources/CloudVMActionLauncher.swift`:
- Around line 135-214: sanitizedCloudVMStartOutput currently misses local
filesystem paths so error.localizedDescription (used in the launch-failure path)
can leak /Users/<username>/…; update sanitizedCloudVMStartOutput to detect and
redact filesystem paths before returning (e.g., apply a regex for POSIX paths
like /(?:Users|home)/[^/\s]+(?:/[^/\s]+)* and replace matches with "[REDACTED
PATH]" or normalize home to "~") and ensure the same treatment runs when the
launch-failure path passes error.localizedDescription into
sanitizedCloudVMStartOutput; reference the sanitizedCloudVMStartOutput function
and the launch-failure usage of error.localizedDescription to find where to add
the regex detection/replacement and return the redacted/generic details string.
---
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 1788-1805: The current sendV2 path extracts only error.code and
error.message and loses structured guidance (action, reason, details); update
sendV2 to extract error["action"], error["reason"], and error["details"] (if
present) and pass them into formatV2Error, then throw CLIError with the richer
string; change formatV2Error signature (e.g.,
formatV2Error(code:message:action:reason:details:)) to append the
action/reason/details in a readable, newline-separated way (or include them as a
JSON snippet) while preserving the existing vm_error and multiline behavior so
users retain backend recovery guidance when present.
- Around line 2692-2708: The unknown-flag branch currently only checks
remaining.first(where: { $0.hasPrefix("--") }) so short flags like "-x" get
treated as positional; update that predicate to detect any token that starts
with "-" (e.g. hasPrefix("-")) but exclude a lone "-" and allow known short
flags (e.g. "-d") so unknown short flags are caught and thrown as CLIError (the
variable/branch using remaining.first(where: { ... }) that binds to `unknown`);
do the analogous change to the stray-positional check that binds `extra` (so it
ignores known short flags but treats any other leading "-" as a flag), and apply
the same fix in the vm ssh-attach and vm-pty-attach unknown-flag branches
referenced in the review.
In `@Sources/Cloud/VMClient.swift`:
- Around line 129-147: cloudVMDetails(from:) currently copies arbitrary
top-level and nested "details" keys into the user-visible output; change this to
use an explicit allowlist of safe keys (e.g., let allowedKeys =
Set(["code","status","type","duration","host","region","message"])) and only
copy keys present in allowedKeys from nestedDetails and the top-level object
into the details dictionary (keep using cloudVMIsNull to filter nulls and
cloudVMValueDescription to render values); if no allowed keys are present,
optionally include a single sanitized fallback like "details: unavailable" or a
generic message instead of exposing raw keys.
In `@web/app/api/vm/`[id]/exec/route.ts:
- Around line 42-50: Trim the incoming command before validating and using it:
when reading rawBody into body (the variable `body` in route handler) compute a
trimmed value (e.g., `const command = String(body.command).trim()`) and then
validate that `command` is non-empty; if empty return the existing
`vmErrorResponse` with `vm_invalid_command`. Replace subsequent uses of
`body.command` in this handler with the trimmed `command` so whitespace-only
inputs are rejected and the executed command is the trimmed version.
🪄 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: 9a6fa59b-b217-483d-a8ed-3f886381cd13
📒 Files selected for processing (14)
.coderabbit.yaml.github/review-bot-rules/README.md.github/review-bot-rules/user-facing-errors.md.greptile/rules.mdCLI/cmux.swiftResources/Localizable.xcstringsSources/Cloud/VMClient.swiftSources/Cloud/VMClientSocketCommands.swiftSources/CloudVMActionLauncher.swiftweb/app/api/vm/[id]/exec/route.tsweb/app/api/vm/route.tsweb/services/vms/errors.tsweb/services/vms/routeHelpers.tsweb/tests/vm-route-auth.test.ts
lawrencecchen
force-pushed
the
feat-cloud-vm-actionable-errors
branch
from
May 13, 2026 12:46
1e765cf to
f8e091d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Cloud/VMClient.swift (1)
389-389:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSanitize unknown transport value to prevent provider-info leak.
The error message interpolates the
transportvalue from the backend response. If the backend sends a provider-specific identifier (e.g.,"azure-direct","provider-x-ssh"), that value would appear in the user-facing Details section, potentially exposing internal provider names. Use generic phrasing consistent with line 535's pattern.As per coding guidelines: user-facing errors must not include "internal provider names, provider-specific flags, templates, snapshots, manifests, or environment variable names."
🛡️ Proposed fix
default: - throw VMClientError.malformedResponse("attach-endpoint unknown transport: \(String(describing: transport))") + throw VMClientError.malformedResponse("Cloud VM attach response used an unsupported transport type.")🤖 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 `@Sources/Cloud/VMClient.swift` at line 389, The error currently exposes the backend-provided transport value by interpolating transport into VMClientError.malformedResponse for the "attach-endpoint" case; change this to a generic, non-provider-specific message (e.g., "unknown transport" or "unsupported transport type") consistent with the pattern used elsewhere (see other VMClientError.malformedResponse usages) so the backend value is not leaked to users — locate the place constructing VMClientError.malformedResponse("attach-endpoint unknown transport: \(String(describing: transport))") and replace the interpolation with a sanitized generic phrase.
♻️ Duplicate comments (1)
Sources/CloudVMActionLauncher.swift (1)
135-214:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSanitizer still misses local filesystem paths —
error.localizedDescriptioncan surface/Users/<username>/…in Details.The launch-failure path (line 94) forwards
error.localizedDescriptionintosanitizedCloudVMStartOutput. ForProcess.run()failures, that string commonly embeds POSIX paths like/Users/<username>/Library/.../cmux, leaking the local account name. None of the blocked terms or the email/IPv4 heuristics catch this, so the path goes through toinformativeTextverbatim.Add a path/home heuristic alongside the existing checks:
🛡️ Proposed fix to redact filesystem paths
let containsLikelyIPAddress = trimmed.range( of: #"(?<!\d)(?:\d{1,3}\.){3}\d{1,3}(?!\d)"#, options: .regularExpression ) != nil - guard !containsBlockedTerm, !containsLikelyEmail, !containsLikelyIPAddress else { + let containsLikelyFilesystemPath = trimmed.range( + of: #"(?:^|[\s"'(\[])(?:/Users/|/home/|/private/|/var/folders/|~/)"#, + options: .regularExpression + ) != nil + guard !containsBlockedTerm, + !containsLikelyEmail, + !containsLikelyIPAddress, + !containsLikelyFilesystemPath else { return String( localized: "command.cloudVM.failed.details.hidden", defaultValue: "Additional technical details are available in logs." ) }As per coding guidelines (
user-facing-errors.md):detailsmust be "minimal and sanitized" and avoid "unredacted payload dumps."🤖 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 `@Sources/CloudVMActionLauncher.swift` around lines 135 - 214, sanitizedCloudVMStartOutput is missing checks for local filesystem paths (e.g. /Users/<username>/...) so error.localizedDescription can leak usernames; add a path/home heuristic: detect POSIX home paths (like /Users/[^/]+/ or /home/[^/]+/), tilde-prefixed paths (~[/\w.-]*), and generic absolute paths with multiple path components (e.g. regex for /[^ \n]+/[^ \n]+) using String.range(of:options:.regularExpression), combine that result into containsBlockedTerm (or a new containsPath boolean) and return the localized hidden-details string when a path is found; update the sanitizedCloudVMStartOutput function to run these regex checks before returning trimmed.
🤖 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 @.greptile/rules.md:
- Line 24: Update the prohibited-items list entry that begins "Flag copy that
includes upstream vendor or service names..." to explicitly include "stack
traces" and "request ids from third‑party systems" so it matches the canonical
user-facing error rule; edit the rule text in .greptile/rules.md to append those
two items (and a short parenthetical like "unless user supplied the exact id")
to the existing comma-separated list to ensure enforcement aligns with
.github/review-bot-rules/user-facing-errors.md.
In `@web/app/api/vm/route.ts`:
- Around line 156-163: Catch and inspect the parse/validation error instead of a
bare catch: when parsing the request body around JSON.parse and the following
shape checks, call recordSpanError(span, err) with the caught error; if
JSON.parse throws, return vmErrorResponse with a distinct error like
"vm_json_parse_failed" and record the parse error on span; if the parsed value
is null or Array.isArray(body) treat those as shape rejections, record a new
Error describing the shape problem via recordSpanError(span, shapeErr) and
return a different vmErrorResponse (e.g., "vm_invalid_shape" or
"vm_expected_object") with an appropriate message; reference the existing span
variable and vmErrorResponse function and add the shape checks immediately after
parsing so telemetry distinguishes parse failures from shape validation
failures.
In `@web/tests/vm-route-auth.test.ts`:
- Around line 691-695: The leak-detection regex in
expectNoCloudVmImplementationLeaks currently misses provider snapshot ids and
Stack Auth team ids; update the function expectNoCloudVmImplementationLeaks to
extend the negative regex (or add an additional assertion) to also reject
provider snapshot-like patterns (e.g., short hex/alpha-numeric prefixes like
sh-[a-z0-9]{8,24} or similar provider id shapes) and Stack Auth team identifiers
(e.g., team-\d+ or team-[a-z0-9-]+), or alternatively add a positive shape check
that ensures details only contain allowed fields to prevent provider
identifier/metadata leaks.
---
Outside diff comments:
In `@Sources/Cloud/VMClient.swift`:
- Line 389: The error currently exposes the backend-provided transport value by
interpolating transport into VMClientError.malformedResponse for the
"attach-endpoint" case; change this to a generic, non-provider-specific message
(e.g., "unknown transport" or "unsupported transport type") consistent with the
pattern used elsewhere (see other VMClientError.malformedResponse usages) so the
backend value is not leaked to users — locate the place constructing
VMClientError.malformedResponse("attach-endpoint unknown transport:
\(String(describing: transport))") and replace the interpolation with a
sanitized generic phrase.
---
Duplicate comments:
In `@Sources/CloudVMActionLauncher.swift`:
- Around line 135-214: sanitizedCloudVMStartOutput is missing checks for local
filesystem paths (e.g. /Users/<username>/...) so error.localizedDescription can
leak usernames; add a path/home heuristic: detect POSIX home paths (like
/Users/[^/]+/ or /home/[^/]+/), tilde-prefixed paths (~[/\w.-]*), and generic
absolute paths with multiple path components (e.g. regex for /[^ \n]+/[^ \n]+)
using String.range(of:options:.regularExpression), combine that result into
containsBlockedTerm (or a new containsPath boolean) and return the localized
hidden-details string when a path is found; update the
sanitizedCloudVMStartOutput function to run these regex checks before returning
trimmed.
🪄 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: d2b02670-0297-44af-8490-42c10f84bd6e
📒 Files selected for processing (14)
.coderabbit.yaml.github/review-bot-rules/README.md.github/review-bot-rules/user-facing-errors.md.greptile/rules.mdCLI/cmux.swiftResources/Localizable.xcstringsSources/Cloud/VMClient.swiftSources/Cloud/VMClientSocketCommands.swiftSources/CloudVMActionLauncher.swiftweb/app/api/vm/[id]/exec/route.tsweb/app/api/vm/route.tsweb/services/vms/errors.tsweb/services/vms/routeHelpers.tsweb/tests/vm-route-auth.test.ts
lawrencecchen
force-pushed
the
feat-cloud-vm-actionable-errors
branch
from
May 13, 2026 13:17
f8e091d to
c1ddbbe
Compare
lawrencecchen
dismissed
coderabbitai[bot]’s stale review
May 13, 2026 13:46
Stale CodeRabbit review; addressed in c1ddbbe and latest checks are green.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit eeec2dd. Configure here.
lawrencecchen
added a commit
that referenced
this pull request
May 14, 2026
Hotfix release for the 0.64.5 cmux ssh stdin regression. Includes: - #4135 SSH stdin fix (community contribution by @kays0x) - #4154 follow-up Swift interpolation fix in the regression test - #4088 transparent backgrounds for file preview panels - #4083 sidebar reorder visibility - #4094 Cloud VM error guidance - #4120 Pi Vault icon and JSONL titles - f85cc56 command palette settings toggles Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
May 14, 2026
* feat: improve markdown viewer
* fix: address markdown review feedback
* fix: clean up markdown review issues
* fix: address markdown review feedback
* fix: address latest markdown review
* Fix #3807: bring notification CLI to panel parity (#3811)
* Prove notification CLI parity is missing
Add behavior-level coverage for the missing notification socket and CLI actions before implementing them. The tests drive V2 notification action methods, CLI subcommands, extended list fields, and the UI open-notification flow so CI can show the pre-fix gap.
Constraint: Regression tests must be committed before the implementation for issue #3807
Constraint: Local Swift tests are not run in this repo; verification is through CI
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Not-tested: Swift/XCUITest execution; intentionally deferred to CI
* Make notification actions scriptable from the CLI
The notifications panel already owned the behavior for dismissing, marking read, opening, and jumping to unread rows. This wires the socket and CLI to those existing store/AppDelegate paths, extends list output with panel metadata, and documents the new command surface without introducing a second notification action model.
Constraint: Existing Notifications page behavior must remain the source of truth
Constraint: Direct xcodebuild and local Swift/XCUITest runs are forbidden in this workspace
Rejected: Duplicate CLI-side focus or read-state logic | would diverge from AppDelegate.openNotification and TerminalNotificationStore semantics
Confidence: medium
Scope-risk: moderate
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Tested: python3 -m json.tool Resources/Localizable.xcstrings
Not-tested: Swift unit/UI execution; deferred to CI per repo policy
* Make notification CI compile under strict concurrency
The notification parity implementation introduced a shared formatter on a main-actor type and a test helper that crossed actor boundaries through escaping closures. This keeps the socket behavior unchanged while making date formatting local to the main-actor call path and keeping the XCTest socket request helper from capturing actor-isolated state in the background request closure.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Run xcodebuild locally to confirm | user explicitly warned direct xcodebuild can deadlock the machine
Confidence: medium
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Make notification parity actions atomic and exact
Review feedback exposed two behavior risks in the notification parity path: bulk dismissal was implemented as client-side list-and-loop work, and jump-to-unread could report one unread notification while opening a later valid one. The server now owns already-read dismissal as a single V2 action, the jump helper returns the notification it actually opened, and event/list parsing details match those server semantics.
Constraint: Notification action logic must reuse the existing store and AppDelegate paths
Rejected: Split TerminalController into new controllers in this PR | broad refactor is unrelated to issue #3807 and would obscure the parity fix
Confidence: medium
Scope-risk: moderate
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace instructions
* Expose app test symbols to CLI notification coverage
The integration regression exercises real app-backed notification state through the CLI harness, so the test target must import the built app module the same way the socket action tests do.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Verify with xcodebuild locally | user explicitly forbids direct xcodebuild
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Make notification list and dismiss payloads stable
Review caught two small contract hazards in the new notification RPC surface: the dismiss result used mixed JSON types, and a pipe in the appended tab title could shift the legacy list parser. The server now reports dismissals as counts consistently and escapes only the new trailing list field, with the CLI decoding it after structural parsing.
Constraint: The V1 list response remains pipe-delimited for backward compatibility, so only newly appended trailing fields can be encoded without changing old parser behavior
Rejected: Replace list_notifications with JSON-only output | existing V1 parsers depend on the line format
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace policy
* Return async notification socket responses from tests
CircleCI caught a compile-only issue in the async V2 test helper: the continuation result was awaited but not returned from the method that promises a response dictionary. Returning the continuation value restores the helper contract without changing the exercised socket behavior.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Verify with local xcodebuild | user explicitly forbids direct xcodebuild
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Avoid blocking notification CLI integration sockets
The integration regression has to create AppKit-backed notification state on the main actor, but running the CLI subprocess synchronously there can block the socket handler's main-actor store mutations. The test now awaits subprocess work on a background queue and verifies read state through the same CLI list path.
Constraint: Socket notification handlers intentionally hop to the main actor for store and AppDelegate work
Rejected: Run the CLI synchronously from the main actor | it can deadlock the in-process socket server during tests
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace policy
* Harden notification CLI response contracts
The notification parity path now distinguishes new list-notification trailer fields from old pipe-heavy bodies, rejects surface-only mark-read selectors, and returns post-open read state by marking through the shared store path after a successful focus action.
Constraint: list_notifications remains a legacy pipe-delimited protocol, so the new trailer needs its own discriminator while older body parsing keeps joining payload[6...]
Rejected: Add a third list_notifications sentinel field | the issue asked for exactly the two appended fields
Rejected: Rely on AppDelegate delayed mark-read for socket JSON | CLI callers need the open response and subsequent list output to reflect the explicit action
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace policy
* Make CLI integration helper capture explicit self
CircleCI's Swift compile step requires explicit self when the background subprocess closure calls the test helper method. This keeps the deadlock fix intact while satisfying Swift capture rules.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Verify with local xcodebuild | user explicitly forbids direct xcodebuild
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Make CLI presentation flags order-independent
Users naturally put presentation flags next to the command they are inspecting. Normalize --json and --id-format after command selection so docs examples such as cmux list-notifications --json produce JSON while still preserving literal flag-looking values for command options.
Constraint: Do not run reload.sh before CI is green; never run bare xcodebuild.
Rejected: Documentation-only correction | leaves copy-pasted command behavior surprising.
Rejected: Notification-only --json handling | repeats the same parser split for future JSON-capable commands.
Confidence: medium
Scope-risk: moderate
Directive: Keep presentation flag parsing centralized; update commandOptionsWithValues when adding value-taking command options.
Tested: git diff --check
Not-tested: Local XCTest/build per repo policy; CI will verify.
* Fix notification surface selector error message
* Make notification open mark read synchronously
* Fix CLI presentation flag parsing
* Test notification mark-read missing id
* Reject missing notification mark-read ids
* fix: polish markdown viewer dogfood
* Fix shared WebView task manager attribution
Fixes shared WebContent resource attribution in task manager rows and adds regression coverage.
* Prevent display-link crash from terminal portal layout reentry (#3885)
* Pin portal sync deferral during SwiftUI host callbacks
The silent-exit crash path points at SwiftUI/AppKit layout recursion reaching a CATransaction display-link flush. This test locks the intended invariant: geometry callbacks originating from the SwiftUI NSViewRepresentable host must defer portal reconciliation instead of forcing immediate AppKit layout, even while an interactive resize is active.
Constraint: Local tests are intentionally not run in this repository; CI owns regression proof.\nRejected: Assert on source text or unified-log contents | timing-dependent and not executable through the policy seam.\nConfidence: medium\nScope-risk: narrow\nDirective: Keep immediate portal flushing owned by external AppKit resize observers, not SwiftUI host callbacks.\nTested: Not run locally per repository policy and user instruction.\nNot-tested: Full multi-session crash reproduction is timing-dependent and not deterministic.
* Prevent portal layout reentry from terminal host callbacks
The terminal NSViewRepresentable host was allowed to bind into the window portal and synchronously flush AppKit layout from update/layout callbacks. That made SwiftUI's own host layout and the external terminal portal both believe they could drive geometry in the same render turn, matching the NSHostingView reentrant-layout warnings reported before the CATransaction display-link abort.\n\nThe portal now treats SwiftUI host callbacks as state capture only: they register the binding and schedule the portal owner to reconcile geometry after the current render turn. Immediate geometry flushing remains available to the portal's external AppKit resize observers, which are outside SwiftUI body/layout evaluation. The launch path also records clean exits and posts a one-time TerminalNotificationStore breadcrumb when a newer ghostty Breakpad envelope is found after an unclean exit.
Constraint: Local tests and app builds are not run before CI for this branch; verification is delegated to CI, then the required tagged reload.\nRejected: Wrap the CATransaction/display-link exception in @try/@catch | it would hide AppKit's abort symptom without removing the reentrant layout owner split.\nRejected: Keep immediate sync during interactive SwiftUI host callbacks | still allows layoutSubtreeIfNeeded inside the representable layout/update path.\nConfidence: medium\nScope-risk: moderate\nDirective: SwiftUI/AppKit host callbacks must not force terminal portal layout synchronously; add external observer paths for any future immediate resize flushing.\nTested: git diff --check; jq empty Resources/Localizable.xcstrings\nNot-tested: Local XCTest/build/repro per repository and user instructions; 75-minute multi-session crash reproduction is not deterministic.
* Record clean-exit breadcrumb after teardown
The crash breadcrumb should compare Breakpad envelopes against the last successfully completed termination path. Recording the timestamp after teardown avoids marking an exit clean before session persistence, process cleanup, and notification cleanup have finished.
Constraint: This is a follow-up correctness tweak before CI settled.\nRejected: Keep the timestamp at the start of applicationWillTerminate | a crash during termination cleanup could suppress the next-launch breadcrumb.\nConfidence: high\nScope-risk: narrow\nDirective: Only update the clean-exit timestamp after teardown work that must complete for a clean quit.\nTested: git diff --check; jq empty Resources/Localizable.xcstrings\nNot-tested: Local tests/builds per repository and user instructions.
* Defer first portal install from SwiftUI callbacks
A deferred SwiftUI host bind could still create the WindowTerminalPortal for the first time, and the initializer previously installed the host with an immediate layout flush. Threading the same deferral flag through portal creation keeps the invariant complete: representable update/layout callbacks never synchronously flush terminal portal layout, even on first bind.
Constraint: Must preserve immediate install behavior for non-SwiftUI external portal callers
Rejected: Assume portals already exist before host callbacks | first terminal bind in a new window can create one
Confidence: high
Scope-risk: narrow
Directive: Any future portal creation path from SwiftUI callbacks must carry deferred synchronization through initialization
Tested: git diff --check; jq empty Resources/Localizable.xcstrings
Not-tested: Local tests/builds per repository and user instructions
* Move crash breadcrumb work out of launch hot path
The crash breadcrumb scanner is pure Foundation logic, so it now lives in its own source file and performs the crash-directory scan from a detached utility task. App launch only schedules the check once, then posts the existing localized notification after the background scan returns to the main actor. The terminal host geometry policy is also simplified to make the always-deferred invariant explicit at the call site.
Constraint: Local build/tests are intentionally skipped until CI completes per issue instructions
Rejected: Keep synchronous directory enumeration in AppDelegate.configure | startup should not block on crash artifact I/O
Rejected: Preserve the dead immediate portal sync branch | host callbacks are never allowed to force layout synchronously
Confidence: high
Scope-risk: narrow
Directive: Do not reintroduce synchronous crash-directory scans or immediate portal layout flushes from SwiftUI host callbacks
Tested: git diff --check; jq empty Resources/Localizable.xcstrings; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj
Not-tested: Local build/tests per repository and user instructions
* Tighten portal geometry regression seam
* Fix crash breadcrumb actor isolation
* Mark crash breadcrumb async helper nonisolated
* Make crash breadcrumb scan concurrent
* fix: remove inert crash breadcrumb cooldown key
* fix: use renamed crash breadcrumb annotation
* fix: own crash breadcrumb scan task
* Hide sidebar descriptions in title-only mode (#4040)
* Hide sidebar descriptions in title-only mode
* Add sidebar description visibility toggle
* Let sidebar titles use trailing slack
* Float sidebar shortcut hints over rows
* Remove unused sidebar width helper
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add Pi agent icon
PR: https://github.com/manaflow-ai/cmux/pull/4057
* Keep Claude Running after clear (#3631)
* Prove Claude clear hook loses running status
The regression exercises the Claude hook handler directly with a SessionStart payload whose source is clear, then sends a late Stop for the previous session. The current handler does not promote the clear boundary to Running or reject stale Stop mutations, so this test is expected to fail before the lifecycle fix.
Constraint: Tests must exercise the hook handler directly rather than driving the full app.
Confidence: high
Scope-risk: narrow
Directive: Keep this regression on the direct CLI hook path; do not replace it with source-text assertions.
Tested: Not run locally per repository policy.
Not-tested: CI red/green proof pending on GitHub Actions.
* Prove Claude clear should own running status
Add hook-level regression coverage for Claude Code /clear. The tests drive the bundled CLI against a mock cmux socket so CI proves SessionStart(source=clear) must set the visible Running status and a prior session Stop must not clobber that fresh lifecycle.
Constraint: Local tests are intentionally not run; CI owns test execution for this task.
Confidence: high
Scope-risk: narrow
Tested: Not run locally per instruction
Not-tested: Full app UI repro under CI
* Ignore stale Claude hook events after clear
Claude /clear starts a fresh hook lifecycle but the sidebar status key is shared at the workspace level. Persist the active Claude session per workspace, promote clear SessionStart to a visible Running state, and ignore teardown or notification mutations from sessions that are no longer current.
Constraint: Claude Code emits /clear as SessionStart(source=clear).
Rejected: Only set Running on clear SessionStart | late Stop from the previous lifecycle could still clobber the new status.
Confidence: high
Scope-risk: narrow
Directive: Future Claude hook status mutations must respect the active workspace session before touching claude_code.
Tested: Not run locally per instruction
Not-tested: Full app UI after CI
* Prove Claude clear hook loses running status
The regression exercises the Claude hook handler directly with a SessionStart payload whose source is clear, then sends a late Stop for the previous session. The current handler does not promote the clear boundary to Running or reject stale Stop mutations, so this test is expected to fail before the lifecycle fix.
Constraint: Tests must exercise the hook handler directly rather than driving the full app.
Confidence: high
Scope-risk: narrow
Directive: Keep this regression on the direct CLI hook path; do not replace it with source-text assertions.
Tested: Not run locally per repository policy.
Not-tested: CI red/green proof pending on GitHub Actions.
* Keep Claude clear sessions authoritative
Claude /clear arrives as a SessionStart source=clear event, but the hook store only remembered routing data. This change makes the store also own the active session for each workspace, promotes clear starts to Running, and ignores visible Stop/Notification/SessionEnd mutations when their session no longer matches the active workspace session.
Constraint: Claude Code documents SessionStart source=clear for /clear lifecycle boundaries.
Rejected: Only set Running on source=clear | old Stop and SessionEnd events could still clobber the new session afterward.
Confidence: high
Scope-risk: narrow
Directive: Visible Claude hook mutations must pass active-session ownership checks before changing sidebar status or notifications.
Tested: Not run locally per repository policy.
Not-tested: CI red/green proof pending on GitHub Actions.
* Allow Claude sessions to advance turns
A visible hook mutation can only be rejected as stale after the active session boundary is known. Keeping a completed turn id active after Stop made the next prompt in the same Claude session look stale before it could promote its own turn.
The Stop path now refreshes the active session with no turn id once the completed turn has been accepted, preserving stale-session protection across /clear while allowing normal multi-turn prompts to re-enter Running. The regression now exercises that two-turn path before the clear boundary.
Constraint: Do not run local tests; CI owns verification for this branch.
Rejected: Ignore turn id in all current-session checks | would weaken stale turn filtering for late Stop and Notification events.
Confidence: high
Scope-risk: narrow
Directive: Do not persist a completed turn id past Stop without proving the next prompt-submit can promote a new turn.
Tested: git diff --check
Not-tested: Local unit/regression tests per project policy
* Remove duplicate Claude clear helper
The branch integration accidentally kept two isClaudeClearSessionStart definitions in CLI/cmux.swift. The duplicate version referenced a non-existent parsedInput.source property, so Swift would reject the file before CI could exercise the hook lifecycle tests.
Keep the existing implementation that reads source from the parsed hook object and delete only the duplicate helper.
Constraint: Fix review-reported compile blocker without changing lifecycle behavior.
Rejected: Add source to ClaudeHookParsedInput | unnecessary for this compile fix and broader than the failing duplicate.
Confidence: high
Scope-risk: narrow
Directive: Keep exactly one Claude clear source helper unless the parsed input model is intentionally extended.
Tested: git diff --check; rg confirms a single isClaudeClearSessionStart definition and no parsedInput.source reference.
Not-tested: Local tests per project policy
* Restore Claude hook compileability after lifecycle merge
The active-session merge left behind the old source-property helper alongside the newer compact-payload helper. Removing the duplicate keeps source=clear detection on the JSON payload and avoids both the redeclaration and missing-property errors.
Constraint: Do not run local tests; CI owns verification for this branch
Rejected: Reintroduce ClaudeHookParsedInput.source | duplicates state already retained in the compact hook payload
Confidence: high
Scope-risk: narrow
Tested: git diff --check; rg verified a single isClaudeClearSessionStart definition and no parsedInput.source references
Not-tested: Local compile/test execution per task policy
* Normalize Claude active-session cleanup keys
The active-session map is keyed by normalized workspace id, so cleanup after consuming a session must use the same normalized key. Otherwise a record containing incidental whitespace could leave a stale active-session entry behind.
Constraint: Address reviewer-reported lifecycle cleanup edge case without changing visible hook behavior.
Rejected: Store raw workspace ids as active map keys | inconsistent with existing upsert normalization and lookup guards.
Confidence: high
Scope-risk: narrow
Directive: Any activeSessionsByWorkspace lookup should use the normalized workspace key, matching insertion.
Tested: git diff --check
Not-tested: Local tests per project policy
* Harden CI Zig downloads against transient upstream failures
CircleCI and the activation workflow both depend on ziglang.org tarballs during remote macOS setup. A transient 500 from that host failed the debug build before any project code compiled, so the install path now retries downloads and avoids unnecessary Homebrew update/cleanup churn on CircleCI.
Constraint: CI must be made green remotely without running local tests or local xcodebuild.
Rejected: Push an empty commit to rerun CI | would leave the same upstream download flake unchanged.
Confidence: high
Scope-risk: narrow
Directive: Keep Zig installer retries in remote CI setup paths; failures here happen before project build logic runs.
Tested: git diff --check
Not-tested: Local tests and local builds not run per repository/user policy.
* Cover stale Claude session-end lifecycle
Greptile identified that stale SessionEnd exercises a different clear-state path than stale Stop. The existing active-session guard already blocks the mutation, so the regression now drives that hook directly and asserts the active /clear session keeps its status, PID, and notifications intact.
Constraint: Do not run local tests; CI is the verification source for this branch.
Rejected: Add another Swift integration test | the existing Python hook harness already executes the CLI handler through the socket path used by CI.
Confidence: high
Scope-risk: narrow
Tested: Not run locally per repo policy.
Not-tested: Local test execution.
* Stop activation CI from stalling on Zig downloads
The activation workflow was cancelled before build because repeated ziglang.org transfers crawled for the full job timeout. Prefer Homebrew's pinned zig@0.15 bottle on macOS runners, then keep the direct tarball path as a bounded fallback with connection, total-time, and minimum-speed limits.
Constraint: The failed check never reached project build or tests; the only failing surface was CI tool bootstrap.
Rejected: Increase the job timeout | it would hide the bootstrap failure and delay feedback.
Confidence: medium
Scope-risk: narrow
Directive: Keep activation workflow tool bootstrap bounded so performance CI reaches the benchmark or fails quickly.
Tested: git diff --check
Not-tested: Local workflow execution, per repository and user instruction.
* Gate Claude session-end cleanup on the consumed workspace
Greptile noted that session-end computed the current-session guard from the fallback workspace while clearing the consumed session's workspace. Move the guard next to the mutation target so stale cleanup and visible cleanup always evaluate the same workspace identity.
Constraint: This is follow-up review hardening on the existing active-session model.
Rejected: Collapse activeSessionsByWorkspace into sessions in this PR | per-workspace current-session lookup is the intended lifecycle boundary for stale event gating.
Confidence: high
Scope-risk: narrow
Directive: SessionEnd visible cleanup must be gated against the workspace being cleared, not an earlier fallback lookup.
Tested: git diff --check
Not-tested: Local test execution, per repository and user instruction.
* Gate Claude session-end cleanup on consumed workspace
A late SessionEnd can be resolved through fallback surface lookup, so the workspace used to find a record is not always the workspace whose visible state would be cleared. Move the staleness check after consume() and evaluate it against the consumed session's workspace, where the clear_status and notification cleanup actually run.
Constraint: Hook events are delivered by short-lived CLI processes and must tolerate stale or partial Claude payloads
Rejected: Keep the pre-consume fallback workspace guard | it can validate one workspace while clearing another
Confidence: high
Scope-risk: narrow
Tested: Added Swift integration regression for stale SessionEnd fallback cleanup
Not-tested: Local tests not run per repository policy
* Make stale Claude session-end test consume the seeded session
Greptile caught that the Swift regression used an unknown session id, so the hook returned before reaching the intended consumed-session visibility guard. Use the seeded stale session id in the SessionEnd payload so the test exercises consume(), then verifies that stale visible cleanup is blocked.
Constraint: Address high-priority review feedback without running local tests.
Rejected: Leave Python-only coverage | the Swift regression would continue passing for the wrong reason.
Confidence: high
Scope-risk: narrow
Directive: Stale session-end regressions should consume a known stale session before asserting visible cleanup is skipped.
Tested: git diff --check
Not-tested: Local tests per repository and user instruction
* Cover fallback Claude session-end consumption
The stale SessionEnd regression should prove the handler consumed the stale session through fallback lookup before deciding whether visible state may be cleared. Use an unknown late session id and assert the seeded stale session is removed from the store, so the test cannot pass without exercising the consumed-workspace guard.
Constraint: Review feedback flagged the prior Swift regression as able to pass without covering the intended guard
Rejected: Use the stored stale session id directly | that only covers the ordinary mapped-session stale path
Confidence: high
Scope-risk: narrow
Tested: Not run locally per repository policy; CI will run the Swift regression
Not-tested: Local XCTest execution
* Keep Claude clear ownership panel-scoped
The clear SessionStart path now owns the active Claude boundary only for explicit clear events, so late startup/resume SessionStart events from the previous session cannot reclaim the workspace before their stale Stop or SessionEnd arrives. The same clear path now passes the resolved surface id into status updates so split panels receive the Running state in the intended pane.
Constraint: Local test execution is disallowed by repository policy; CI remains the behavioral gate.
Rejected: Let every SessionStart mark active | late non-clear events can overwrite the clear boundary.
Rejected: Drop fail-open isCurrent behavior | transient store errors should not hide legitimate current status updates.
Confidence: medium
Scope-risk: moderate
Directive: Do not let non-clear Claude SessionStart events replace an explicit /clear active boundary without adding event ordering metadata.
Tested: python3 -m py_compile tests/test_claude_hook_clear_running_status.py; git diff --check; conflict marker scan.
Not-tested: Local Swift/unit/UI test execution per repo policy.
* Protect Claude clear state from stale session cleanup
Late hook events can arrive after an explicit /clear boundary. The SessionStart handler now skips PID registration when the event is stale against the active session, and SessionEnd consumption preserves the active session when the incoming turn id is older than the stored active turn.
Constraint: Local test execution is disallowed by repository policy; CI remains the behavioral gate.
Rejected: Clear active ownership before checking currentness | same-session stale SessionEnd would fail open and apply cleanup.
Rejected: Keep prefix-only command assertions | option ordering changes would make the regression test brittle.
Confidence: medium
Scope-risk: moderate
Directive: Do not consume a Claude session or replace its PID from a hook event that is stale relative to active session/turn state.
Tested: python3 -m py_compile tests/test_claude_hook_clear_running_status.py; git diff --check; conflict marker scan.
Not-tested: Local Swift/unit/UI test execution per repo policy.
* Allow new Claude sessions to replace stopped owners
A stopped Claude turn must keep the same session eligible for the next turn, but it must not strand the workspace if that process exits before session-end. Mark stopped ownership as replaceable and let only session activation paths use that escape hatch; clear-session ownership remains non-replaceable so stale pre-clear events stay blocked. Session-end fallback cleanup now checks the consumed session id so missing input session ids do not fail open.\n\nConstraint: /clear SessionStart must continue to suppress stale pre-clear startup, stop, and session-end events\nRejected: Clear active ownership on Stop | breaks same-session multi-turn prompt-submit currentness\nConfidence: medium\nScope-risk: narrow\nTested: git diff --check; conflict-marker scan\nNot-tested: local XCTest per repo policy; CI will run cmux unit regressions
* test: cover codex hooks TOML features section
* fix: harden hook config and auth token cache
* test: cover sign-out browser auth loading cleanup
* fix: cancel browser auth on sign-out
* test: cover codex config read failures
* fix: fail closed on codex config read errors
* test: cover auth sign-out callback race
* fix: discard auth callback after sign-out
* fix: keep claude subcommands out of hook injection
* fix: dismiss stale sparkle update replies
* fix: redact auth logs while preserving observability
* fix: reset update checks and guard sign-out races
* fix: preserve update metadata from driver state
* fix: await auth token assertions before comparing
* fix: address wrapper and auth review feedback
* fix: address session and config review feedback
* ci: retrigger pending checks
* fix: clear stale ime and auth token state
* fix: clean legacy codex hooks config
* test: cover legacy codex hooks markers
* fix: clean empty codex features table
* fix: preserve browser key reentry dispatch
* Fix new-workspace caller window routing (#4042)
* test: cover new workspace caller routing
* fix: route new workspace to caller context
* Add iMessage workspace insertion regression test
* Reset Kitty keyboard mode at shell prompt boundaries (#3870)
* Prove stale Kitty keyboard state leaks CSI-u key bytes
The regression exercises a hosted Ghostty terminal, leaves Kitty keyboard protocol enabled as a crashed TUI would, mirrors the clear-history socket handler clear_screen path, and captures raw PTY stdin bytes for a plain c key. Current behavior should encode CSI-u instead of a single ASCII byte, making the test fail until the protocol state is reset at the shell prompt boundary.
Constraint: Regression must cover PTY input bytes, not source text shape
Confidence: high
Scope-risk: narrow
Directive: Keep this test on the real ghostty_surface_key path; sendText alone bypasses the keyboard encoder
Tested: Manual current-main repro captured c9;1:3uc9;1:3uc9;1:3u after Kitty enable plus clear-history
Not-tested: Local unit execution deferred to CI per requested red/green workflow
* Reset stale Kitty keyboard mode at prompt boundaries
A crashed TUI can leave Ghostty's Kitty keyboard protocol stack pushed after clear-history, causing normal shell input to be encoded as CSI-u. Resetting the stack from the shell prompt hook makes the prompt boundary the ownership point for returning interactive shells to plain byte input.
Constraint: Fix must run through shell integration because Ghostty keeps protocol state inside the terminal surface
Rejected: Reset only in clear-history socket path | stale state also leaks after any crashed TUI returns to prompt
Confidence: high
Scope-risk: narrow
Directive: Keep prompt-boundary reset paired across bash and zsh integrations
Tested: Not run locally per repository testing policy; regression added in prior commit
Not-tested: CI not yet completed
* Clarify Kitty reset fixture failures
The stale keyboard regression now fails at the fixture boundary if the zsh integration does not emit the expected reset bytes, instead of falling through to a misleading PTY byte mismatch.
Constraint: Repository policy forbids local test execution for this PR loop
Confidence: high
Scope-risk: narrow
Tested: Not run locally per repository policy; CI will run the affected XCTest
Not-tested: Local XCTest execution
* chore: retrigger Vercel checks
* Add Kitty reset shell hook coverage
* Reset all terminal keyboard protocols at prompt
* Fix terminal keyboard reset test expectations
* Clarify disabling agent session auto-resume for #3640 (#3991)
* docs: explain disabling agent auto resume
* docs: name auto resume config path
* Fix stale restored agent resume state
* Harden restored agent hook liveness
* refactor: share restored agent normalization
* Honor iMessage workspace ordering and previews
* Add workspace cwd inheritance setting (#3921)
* feat: add workspace cwd inheritance setting
* fix: align workspace cwd setting key naming
* fix: apply workspace cwd setting to detached creation
* fix: expose workspace cwd inheritance setting
* fix: route pane break through detached workspace creation
* fix: keep pane break response pane ids non-null
* fix: distinguish pane break resolution errors
* Approve installed Codex hooks (#4035)
* Approve installed Codex hooks
* Address Codex hook trust review feedback
* Avoid tomllib in Codex hook tests
* Align Codex hook trust test mirror
* Harden Codex hook trust ownership
* Preserve legacy Codex hook cleanup
* Fix workspace unit test after merge
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Revert "Approve installed Codex hooks (#4035)" (#4074)
This reverts commit e4546b7675a4ffa5a41a5429218b60f4e7644e21.
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Fix workspace unit test transfer resume state (#4076)
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Clarify in cmux --help that reload-config covers Ghostty config too (#4060)
* Clarify cmux help that reload-config covers Ghostty config too
`cmux reload-config` reloads BOTH ~/.config/cmux/cmux.json and Ghostty
config (~/.config/ghostty/config) and refreshes terminals in place, but
the help/docs only mentioned cmux.json. Agents (and humans) reading the
help would think they had to restart cmux after editing Ghostty config.
- cmux --help "Agent Help" now tells agents where Ghostty config lives
and that reload-config picks it up live.
- cmux docs settings, cmux settings path, cmux config --help now list
~/.config/ghostty/config as a related (not cmux-owned) location and
describe reload-config's actual scope.
- cmux schema sidebarAppearance.tintOpacity description now notes it's
sidebar-only, and points to Ghostty background-opacity / blur for
terminal transparency.
- skills/cmux/SKILL.md mirrors the wording.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Align ghostty_config JSON shape across CLI surfaces
Cursor Bugbot and Greptile both flagged that `cmux settings path --json`
emitted `ghostty_config` as a string while `cmux docs settings --json`
emitted it as an object {path, note}. An agent reading both outputs with
the same key expectation would have to special-case the type.
Standardize on the object shape with `path` and `note` in both, matching
docsPayload. This is the agent-discoverability path the PR is trying to
make reliable, so making the shape consistent is on-purpose.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Use canonical Ghostty config key background-blur (not background-blur-radius)
CodeRabbit flagged that `background-blur-radius` is outdated. Verified
against the Ghostty submodule at ghostty/src/config/Config.zig: line 70
declares `background-blur-radius` as a compatibilityRenamed alias for
`background-blur`. The current canonical key is `background-blur` and it
accepts the same integer value (e.g. `background-blur = 20`).
Update the two docs that introduced the alias name:
- skills/cmux/SKILL.md
- web/data/cmux.schema.json (sidebarAppearance.tintOpacity description)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add nucleo FFI command palette benchmark
* Open right sidebar tools as panes (#4065)
* Open right sidebar tools as panes
* Remove unreachable sidebar pane commands
* Fix sidebar pane unit test build
* Address sidebar pane review feedback
* Fix vault pane focus tracking
* Address right sidebar pane review followups
* Use modern sidebar pane flash observer
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Save crash diagnostics under cmux state (#4077)
* Save crash diagnostics under cmux state
* fix: key GhosttyKit artifacts by crash path
* fix: pin cmux crash GhosttyKit archive
* fix: mark crash breadcrumb scan concurrent
* fix: keep crash scan compatible with Xcode 16
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Menubar global search P1 (#3908)
* Ship local global search as a remappable menubar flow
Phase 1 needs browser and markdown value without terminal scrollback, so the index owns durable FTS5 upserts while AppDelegate keeps one navigation path from palette rows to focused panels. The global shortcut is wired through the existing shortcut settings instead of a standalone Carbon shim so Settings and cmux.json remain authoritative.
Constraint: Phase 1 excludes Ghostty terminal scrollback capture
Constraint: User required no local test execution and no reload before CI is green
Rejected: Hardcoded GlobalSearchHotkey shim | violates KeyboardShortcutSettings policy
Confidence: medium
Scope-risk: broad
Directive: Keep future terminal capture feeding SearchIndex documents through GlobalSearchCoordinator rather than adding another palette/navigation path
Tested: git diff --check; python3 -m json.tool Resources/Localizable.xcstrings; python3 -m json.tool web/data/cmux.schema.json; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj
Not-tested: Local unit/UI tests and tagged app launch per task constraints
* Remove duplicate search refresh work
Cursor review pointed out that the menubar toggle and palette onAppear both refreshed the live index. Keeping the refresh in the palette lifecycle avoids resetting browser debounce tasks while still indexing each time the palette opens, and removing the unused workspace delete API keeps the storage surface honest.
Constraint: Review feedback came from PR #3908 after the first CI pass started
Rejected: Keep both refresh calls | causes avoidable debounce cancellation and slower browser result availability
Confidence: high
Scope-risk: narrow
Directive: Add workspace-level deletion only when a real workspace teardown caller is wired
Tested: git diff --check; python3 -m json.tool Resources/Localizable.xcstrings; python3 -m json.tool web/data/cmux.schema.json; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj
Not-tested: Local tests/build per task constraints
* Fix global search stale index entries
* Address global search review feedback
* Fix search query token mapping compile error
* Fix global search CI failures
* Fix stale unavailable markdown search entries
* Cancel stale markdown search captures
* Cancel global search refresh on dismiss
* Fix global search review followups
* Address global search post-CI feedback
* Preserve markdown panel title search on read failure
* Address global search lifecycle feedback
* Address global search review lifecycle feedback
* Refine global search capture ownership
* fix: address global search review blockers
* feat: show open panels in global search
* fix: cover right sidebar tool panel in search
* Fix cmuxTests: rename restorableAgentAutoResumePending to restorableAgentResumeState (#4068)
* fix: handle repeated assistant imessage completions
* Open supported files in cmux on cmd-click (#4041)
* Open supported files in cmux on cmd-click
* Add cmd-click file preview verification script
* Reuse right pane for cmd-click file previews
* Keep cmd-click UI test terminal after preview focus
* Accept numeric cmd-click test payload values
* Capture cmd-click UI test window snapshots
* Add file-type-aware external open actions
* Address supported file routing review feedback
* Fix external open menu sendability warnings
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Suppress native proxy icon on cmux main windows (#3973)
* Add proxy icon regression test
* Suppress native proxy icon on cmux windows
* chore: retrigger preview deployments
* Refine native proxy icon suppression
* fix: keep titlebar folder icon aligned
* fix: restore titlebar folder icon leading offset
* test: cover folder icon frame replacement
* fix: resync folder icon on frame replacement
* test: cover folder icon ancestor movement
* fix: track folder icon ancestor movement
* fix: address folder icon review feedback
* test: stabilize folder icon ancestor sync
* fix: handle sidebar tool panels in global search
---------
Co-authored-by: austinpower1258 <austinwang115@gmail.com>
* Close browser panels when pages request window close (#4070)
* Add browser self-close restore regression test
* Close browser panels from WebKit close callbacks
* fix: index right sidebar tool panels as titles
* fix: keep web close callback synchronous
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add notification policy hooks
Merged https://github.com/manaflow-ai/cmux/pull/4054
* Fix sidebar unread badge after re-marking notifications (#4084)
* Add sidebar unread notification regression test
* Keep sidebar notification badge live during menu freeze
* Cover sidebar presentation fallback cases
* Limit Cloud VMs by active provider state (#4046)
* test: cover active vm limit with paused freestyle vms
* fix: enforce cloud vm limits by active state
* fix: parallelize cloud vm status refresh
* chore: update web security dependencies
* fix: handle right sidebar tools in global search
* fix: guard cloud vm status refresh races
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Fix Settings search synonyms (#4082)
* Add settings search synonym regressions
* Fix settings search synonyms
* Add shortcut bindings anchor regression
* Cover clickable PR settings search alias
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add tagged debug CLI helper (#4092)
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add unread defer shortcut (#4086)
* Add unread defer shortcut
* fix: address unread defer feedback
* fix: keep manual unread jump explicit
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Document notification hooks default off
Document that notification hooks are off by default and style the config key in localized docs.
* Approve installed Codex hooks after dogfood (#4075)
* Reapply Codex hook approval changes for dogfood
* Notify on Codex plan input requests
* Handle Codex plan question transcript items
* Address Codex hook review feedback
* Recover malformed Codex hook trust blocks
* Avoid duplicate consecutive cmux hook reinserts
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Document session restore behavior
Adds session restore docs, blog, README updates, and review cleanup.
* Use nucleo for command palette search
* Skip unrestorable Claude startup sessions
PR: https://github.com/manaflow-ai/cmux/pull/4079
* Revert "Suppress native proxy icon on cmux main windows" (#4099)
* Revert "Suppress native proxy icon on cmux main windows (#3973)"
This reverts commit 5048440ff44b4edbe328b65403724ff79476b2e6.
* Fix titlebar proxy icon without detached panel sync
* Apply proxy icon override to fallback config
* Restore key regain redraw invariant
* Tune nucleo palette initialism ranking
* Fix terminal portal resize lag (#4102)
* Fix Korean 2-Set terminal arrows (#4095)
* Add Korean 2-Set arrow IME regression
* Restore Korean 2-Set arrow forwarding
* Restore Zhuyin IME command routing
* Address IME review feedback
* Fix Korean IME regression test compile
* Address IME review follow-ups
* Address Bopomofo preedit review feedback
* Avoid idle Zhuyin key suppression
* Remove dead text input wrapper
* Address IME suppression review feedback
* Fix palette stitched highlight precedence
* Add Codex Teams subagent panes
* Open markdown files in preview panels from cmux open (#4085)
* fix: open markdown files in preview panels
* fix: preserve markdown viewer transparency
* fix: mute markdown open-with header button
* fix: align markdown header controls
* fix: align file header controls
* fix: address markdown review feedback
* test: cover opaque text editor alpha
* fix: align header open fallback
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add task manager sorting and program aggregates (#4066)
* Add task manager sorting and program aggregates
* Add sorting to cmux top output
* Add flat TSV cmux top output
* Add coding agent task manager totals
* Make task manager program totals payload-backed
* Recognize agent launcher process names in task manager
* Fix task manager test build helpers
* Recognize Claude versioned launcher processes
* Show loading state before task manager sample
* Recognize versioned agent process names
* Use agent totals for task manager hierarchy icons
* fix: address task manager review feedback
* fix: address follow-up task manager review
* fix: address final task manager review
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Keep manual unread sticky until terminal interaction (#4104)
* test: cover sticky manual unread state
* fix: keep manual unread sticky until terminal input
* fix: show workspace manual unread pane ring
* fix: sync manual unread badge on focus changes
* fix: stabilize manual unread representative fallback
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: austinpower1258 <austinwang115@gmail.com>
* Bump version to 0.64.5 (#4107)
* Fix Pi Vault icon and JSONL titles (#4120)
* test: cover Pi JSONL content block titles
* fix: parse Pi JSONL text blocks for Vault titles
* fix: align Pi JSONL title role handling
* fix: require typed text blocks for Pi titles
* Improve Cloud VM error guidance (#4094)
* Improve Cloud VM error guidance
* Address final Cloud VM review feedback
* Narrow Cloud VM sanitizer env var block
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Keep selected workspace visible after sidebar reorders
* Add sidebar scroll regression for workspace move to top
* Reveal selected workspace after sidebar reorders
* Handle right sidebar tool panels in global search
* Test workspace move to top visibility only
* Refine sidebar reorder scroll trigger
* Add move-to-top notification regression
* Skip no-op move-to-top notifications
* Assert no-op move-to-top keeps order
* Require matching manager for reorder scroll
* Scroll selected workspace on index shifts
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Use transparent backgrounds for file preview panels (#4088)
* Handle right sidebar tools in global search browse hits
* Use transparent backgrounds for file preview panels
* Cover panel theme background preservation
* Fix PDF file preview open menu
* Make file open menu button compact
* Fix PDF open menu chrome button
* Fix PDF open-with chrome click target
* Address file preview chrome review feedback
* Fix PDF background cache invalidation
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Inherit stdin in backgrounded ssh inside startup wrapper (#4135)
* Add regression test for SSH startup wrapper dropping stdin
After PR #3786 backgrounded the ssh invocation inside the startup
wrapper for signal/reconnect handling, `cmux ssh <host>` sessions
stopped forwarding keystrokes from the surface PTY to the remote
shell. Output flowed back (the remote prompt rendered in cmux UI),
but anything typed never reached zsh on the other side, which sat
blocked in `do_poll` on the remote pts.
Root cause: POSIX sh redirects stdin of an async command (`&`) to
/dev/null when job control is off — the default for the `/bin/sh -c …`
that runs the startup wrapper. Without an explicit `<&0` on the
`&`'d ssh line, the local PTY stdin is silently dropped.
This commit adds the failing regression test. The fix follows in
the next commit so CI can prove the test catches the bug.
* Inherit stdin in backgrounded ssh inside startup wrapper
PR #3786 wrapped the ssh invocation in `while :; do … & wait` to enable
SIGHUP/INT/TERM trap handling and reconnect-on-exit-255. Backgrounding
ssh, however, drops its stdin: POSIX sh redirects fd 0 of any async
command to /dev/null when job control is off, which is the default for
the `/bin/sh -c …` host that runs this wrapper.
As a result, output from the remote still flowed back to the surface
PTY (ssh's stdout/stderr stayed wired) but the user's keystrokes never
reached the remote — they hit ttysNNN on the Mac side, kernel echoed
them locally, but ssh's stdin was /dev/null so nothing was forwarded.
zsh on the far side sat in `do_poll` forever.
Explicit `<&0` on both the `command` line and the `( … )` shell-snippet
form overrides the POSIX default and re-attaches the wrapper's own
stdin to the backgrounded ssh process.
Verified with the regression test added in the previous commit, plus
manual repro on Darwin:
/bin/sh -c 'cat & wait' # cat's fd 0 → /dev/null (bug)
/bin/sh -c 'cat <&0 & wait' # cat's fd 0 → parent stdin (fix)
* Add docs search
Adds localized Pagefind docs search with heading anchors and section-aware results.
* Fix Swift interpolation escape in SSH stdin regression test (#4154)
`testSSHStartupForwardsStdinToBackgroundedSSH` used `\"<empty>\"` inside a
`\(...)` string interpolation. Swift parses `\"` as the end of the outer
literal, breaking compilation of cmuxTests on main:
Cannot find ')' to match opening '(' in string interpolation
Unterminated string literal
That kept `ci/circleci: macos-unit-tests` red on main after #4135 even
though `macos-debug-build` and `macos-release-build` still passed (the
test target was the only thing affected). The runtime fix (`<&0` on the
backgrounded ssh) is unchanged and was verified end-to-end on a cloud Mac.
Inside an interpolation, the string is already a Swift expression and
literal quotes are unescaped; drop the four backslashes.
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add command palette settings toggles
Adds command palette toggles for boolean Settings rows, including iMessage Mode.
* Ensure Rust toolchain is installed for nucleo FFI builds
* Document nucleo FFI thread and ABI assumptions
* Reuse nucleo index in preview search test helper
* Install Rust for activation perf builds
---------
Co-authored-by: Tobi Lutke <tobi@shopify.com>
Co-authored-by: Austin Wang <austinwang115@gmail.com>
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Kevin Peng <48529172+kays0x@users.noreply.github.com>
lawrencecchen
added a commit
that referenced
this pull request
May 15, 2026
* Optimize command palette search
* Reduce command palette typing frame work
* Fix command palette result rendering
* Ignore stale command palette row snapshots
* Use command ids for palette row identity
* Match Zed-style palette result limiting
* Reduce palette preview search frame misses
* Use nucleo for command palette search (#4078)
* feat: improve markdown viewer
* fix: address markdown review feedback
* fix: clean up markdown review issues
* fix: address markdown review feedback
* fix: address latest markdown review
* Fix #3807: bring notification CLI to panel parity (#3811)
* Prove notification CLI parity is missing
Add behavior-level coverage for the missing notification socket and CLI actions before implementing them. The tests drive V2 notification action methods, CLI subcommands, extended list fields, and the UI open-notification flow so CI can show the pre-fix gap.
Constraint: Regression tests must be committed before the implementation for issue #3807
Constraint: Local Swift tests are not run in this repo; verification is through CI
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Not-tested: Swift/XCUITest execution; intentionally deferred to CI
* Make notification actions scriptable from the CLI
The notifications panel already owned the behavior for dismissing, marking read, opening, and jumping to unread rows. This wires the socket and CLI to those existing store/AppDelegate paths, extends list output with panel metadata, and documents the new command surface without introducing a second notification action model.
Constraint: Existing Notifications page behavior must remain the source of truth
Constraint: Direct xcodebuild and local Swift/XCUITest runs are forbidden in this workspace
Rejected: Duplicate CLI-side focus or read-state logic | would diverge from AppDelegate.openNotification and TerminalNotificationStore semantics
Confidence: medium
Scope-risk: moderate
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Tested: python3 -m json.tool Resources/Localizable.xcstrings
Not-tested: Swift unit/UI execution; deferred to CI per repo policy
* Make notification CI compile under strict concurrency
The notification parity implementation introduced a shared formatter on a main-actor type and a test helper that crossed actor boundaries through escaping closures. This keeps the socket behavior unchanged while making date formatting local to the main-actor call path and keeping the XCTest socket request helper from capturing actor-isolated state in the background request closure.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Run xcodebuild locally to confirm | user explicitly warned direct xcodebuild can deadlock the machine
Confidence: medium
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Make notification parity actions atomic and exact
Review feedback exposed two behavior risks in the notification parity path: bulk dismissal was implemented as client-side list-and-loop work, and jump-to-unread could report one unread notification while opening a later valid one. The server now owns already-read dismissal as a single V2 action, the jump helper returns the notification it actually opened, and event/list parsing details match those server semantics.
Constraint: Notification action logic must reuse the existing store and AppDelegate paths
Rejected: Split TerminalController into new controllers in this PR | broad refactor is unrelated to issue #3807 and would obscure the parity fix
Confidence: medium
Scope-risk: moderate
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace instructions
* Expose app test symbols to CLI notification coverage
The integration regression exercises real app-backed notification state through the CLI harness, so the test target must import the built app module the same way the socket action tests do.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Verify with xcodebuild locally | user explicitly forbids direct xcodebuild
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Make notification list and dismiss payloads stable
Review caught two small contract hazards in the new notification RPC surface: the dismiss result used mixed JSON types, and a pipe in the appended tab title could shift the legacy list parser. The server now reports dismissals as counts consistently and escapes only the new trailing list field, with the CLI decoding it after structural parsing.
Constraint: The V1 list response remains pipe-delimited for backward compatibility, so only newly appended trailing fields can be encoded without changing old parser behavior
Rejected: Replace list_notifications with JSON-only output | existing V1 parsers depend on the line format
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace policy
* Return async notification socket responses from tests
CircleCI caught a compile-only issue in the async V2 test helper: the continuation result was awaited but not returned from the method that promises a response dictionary. Returning the continuation value restores the helper contract without changing the exercised socket behavior.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Verify with local xcodebuild | user explicitly forbids direct xcodebuild
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Avoid blocking notification CLI integration sockets
The integration regression has to create AppKit-backed notification state on the main actor, but running the CLI subprocess synchronously there can block the socket handler's main-actor store mutations. The test now awaits subprocess work on a background queue and verifies read state through the same CLI list path.
Constraint: Socket notification handlers intentionally hop to the main actor for store and AppDelegate work
Rejected: Run the CLI synchronously from the main actor | it can deadlock the in-process socket server during tests
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace policy
* Harden notification CLI response contracts
The notification parity path now distinguishes new list-notification trailer fields from old pipe-heavy bodies, rejects surface-only mark-read selectors, and returns post-open read state by marking through the shared store path after a successful focus action.
Constraint: list_notifications remains a legacy pipe-delimited protocol, so the new trailer needs its own discriminator while older body parsing keeps joining payload[6...]
Rejected: Add a third list_notifications sentinel field | the issue asked for exactly the two appended fields
Rejected: Rely on AppDelegate delayed mark-read for socket JSON | CLI callers need the open response and subsequent list output to reflect the explicit action
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace policy
* Make CLI integration helper capture explicit self
CircleCI's Swift compile step requires explicit self when the background subprocess closure calls the test helper method. This keeps the deadlock fix intact while satisfying Swift capture rules.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Verify with local xcodebuild | user explicitly forbids direct xcodebuild
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Make CLI presentation flags order-independent
Users naturally put presentation flags next to the command they are inspecting. Normalize --json and --id-format after command selection so docs examples such as cmux list-notifications --json produce JSON while still preserving literal flag-looking values for command options.
Constraint: Do not run reload.sh before CI is green; never run bare xcodebuild.
Rejected: Documentation-only correction | leaves copy-pasted command behavior surprising.
Rejected: Notification-only --json handling | repeats the same parser split for future JSON-capable commands.
Confidence: medium
Scope-risk: moderate
Directive: Keep presentation flag parsing centralized; update commandOptionsWithValues when adding value-taking command options.
Tested: git diff --check
Not-tested: Local XCTest/build per repo policy; CI will verify.
* Fix notification surface selector error message
* Make notification open mark read synchronously
* Fix CLI presentation flag parsing
* Test notification mark-read missing id
* Reject missing notification mark-read ids
* fix: polish markdown viewer dogfood
* Fix shared WebView task manager attribution
Fixes shared WebContent resource attribution in task manager rows and adds regression coverage.
* Prevent display-link crash from terminal portal layout reentry (#3885)
* Pin portal sync deferral during SwiftUI host callbacks
The silent-exit crash path points at SwiftUI/AppKit layout recursion reaching a CATransaction display-link flush. This test locks the intended invariant: geometry callbacks originating from the SwiftUI NSViewRepresentable host must defer portal reconciliation instead of forcing immediate AppKit layout, even while an interactive resize is active.
Constraint: Local tests are intentionally not run in this repository; CI owns regression proof.\nRejected: Assert on source text or unified-log contents | timing-dependent and not executable through the policy seam.\nConfidence: medium\nScope-risk: narrow\nDirective: Keep immediate portal flushing owned by external AppKit resize observers, not SwiftUI host callbacks.\nTested: Not run locally per repository policy and user instruction.\nNot-tested: Full multi-session crash reproduction is timing-dependent and not deterministic.
* Prevent portal layout reentry from terminal host callbacks
The terminal NSViewRepresentable host was allowed to bind into the window portal and synchronously flush AppKit layout from update/layout callbacks. That made SwiftUI's own host layout and the external terminal portal both believe they could drive geometry in the same render turn, matching the NSHostingView reentrant-layout warnings reported before the CATransaction display-link abort.\n\nThe portal now treats SwiftUI host callbacks as state capture only: they register the binding and schedule the portal owner to reconcile geometry after the current render turn. Immediate geometry flushing remains available to the portal's external AppKit resize observers, which are outside SwiftUI body/layout evaluation. The launch path also records clean exits and posts a one-time TerminalNotificationStore breadcrumb when a newer ghostty Breakpad envelope is found after an unclean exit.
Constraint: Local tests and app builds are not run before CI for this branch; verification is delegated to CI, then the required tagged reload.\nRejected: Wrap the CATransaction/display-link exception in @try/@catch | it would hide AppKit's abort symptom without removing the reentrant layout owner split.\nRejected: Keep immediate sync during interactive SwiftUI host callbacks | still allows layoutSubtreeIfNeeded inside the representable layout/update path.\nConfidence: medium\nScope-risk: moderate\nDirective: SwiftUI/AppKit host callbacks must not force terminal portal layout synchronously; add external observer paths for any future immediate resize flushing.\nTested: git diff --check; jq empty Resources/Localizable.xcstrings\nNot-tested: Local XCTest/build/repro per repository and user instructions; 75-minute multi-session crash reproduction is not deterministic.
* Record clean-exit breadcrumb after teardown
The crash breadcrumb should compare Breakpad envelopes against the last successfully completed termination path. Recording the timestamp after teardown avoids marking an exit clean before session persistence, process cleanup, and notification cleanup have finished.
Constraint: This is a follow-up correctness tweak before CI settled.\nRejected: Keep the timestamp at the start of applicationWillTerminate | a crash during termination cleanup could suppress the next-launch breadcrumb.\nConfidence: high\nScope-risk: narrow\nDirective: Only update the clean-exit timestamp after teardown work that must complete for a clean quit.\nTested: git diff --check; jq empty Resources/Localizable.xcstrings\nNot-tested: Local tests/builds per repository and user instructions.
* Defer first portal install from SwiftUI callbacks
A deferred SwiftUI host bind could still create the WindowTerminalPortal for the first time, and the initializer previously installed the host with an immediate layout flush. Threading the same deferral flag through portal creation keeps the invariant complete: representable update/layout callbacks never synchronously flush terminal portal layout, even on first bind.
Constraint: Must preserve immediate install behavior for non-SwiftUI external portal callers
Rejected: Assume portals already exist before host callbacks | first terminal bind in a new window can create one
Confidence: high
Scope-risk: narrow
Directive: Any future portal creation path from SwiftUI callbacks must carry deferred synchronization through initialization
Tested: git diff --check; jq empty Resources/Localizable.xcstrings
Not-tested: Local tests/builds per repository and user instructions
* Move crash breadcrumb work out of launch hot path
The crash breadcrumb scanner is pure Foundation logic, so it now lives in its own source file and performs the crash-directory scan from a detached utility task. App launch only schedules the check once, then posts the existing localized notification after the background scan returns to the main actor. The terminal host geometry policy is also simplified to make the always-deferred invariant explicit at the call site.
Constraint: Local build/tests are intentionally skipped until CI completes per issue instructions
Rejected: Keep synchronous directory enumeration in AppDelegate.configure | startup should not block on crash artifact I/O
Rejected: Preserve the dead immediate portal sync branch | host callbacks are never allowed to force layout synchronously
Confidence: high
Scope-risk: narrow
Directive: Do not reintroduce synchronous crash-directory scans or immediate portal layout flushes from SwiftUI host callbacks
Tested: git diff --check; jq empty Resources/Localizable.xcstrings; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj
Not-tested: Local build/tests per repository and user instructions
* Tighten portal geometry regression seam
* Fix crash breadcrumb actor isolation
* Mark crash breadcrumb async helper nonisolated
* Make crash breadcrumb scan concurrent
* fix: remove inert crash breadcrumb cooldown key
* fix: use renamed crash breadcrumb annotation
* fix: own crash breadcrumb scan task
* Hide sidebar descriptions in title-only mode (#4040)
* Hide sidebar descriptions in title-only mode
* Add sidebar description visibility toggle
* Let sidebar titles use trailing slack
* Float sidebar shortcut hints over rows
* Remove unused sidebar width helper
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add Pi agent icon
PR: https://github.com/manaflow-ai/cmux/pull/4057
* Keep Claude Running after clear (#3631)
* Prove Claude clear hook loses running status
The regression exercises the Claude hook handler directly with a SessionStart payload whose source is clear, then sends a late Stop for the previous session. The current handler does not promote the clear boundary to Running or reject stale Stop mutations, so this test is expected to fail before the lifecycle fix.
Constraint: Tests must exercise the hook handler directly rather than driving the full app.
Confidence: high
Scope-risk: narrow
Directive: Keep this regression on the direct CLI hook path; do not replace it with source-text assertions.
Tested: Not run locally per repository policy.
Not-tested: CI red/green proof pending on GitHub Actions.
* Prove Claude clear should own running status
Add hook-level regression coverage for Claude Code /clear. The tests drive the bundled CLI against a mock cmux socket so CI proves SessionStart(source=clear) must set the visible Running status and a prior session Stop must not clobber that fresh lifecycle.
Constraint: Local tests are intentionally not run; CI owns test execution for this task.
Confidence: high
Scope-risk: narrow
Tested: Not run locally per instruction
Not-tested: Full app UI repro under CI
* Ignore stale Claude hook events after clear
Claude /clear starts a fresh hook lifecycle but the sidebar status key is shared at the workspace level. Persist the active Claude session per workspace, promote clear SessionStart to a visible Running state, and ignore teardown or notification mutations from sessions that are no longer current.
Constraint: Claude Code emits /clear as SessionStart(source=clear).
Rejected: Only set Running on clear SessionStart | late Stop from the previous lifecycle could still clobber the new status.
Confidence: high
Scope-risk: narrow
Directive: Future Claude hook status mutations must respect the active workspace session before touching claude_code.
Tested: Not run locally per instruction
Not-tested: Full app UI after CI
* Prove Claude clear hook loses running status
The regression exercises the Claude hook handler directly with a SessionStart payload whose source is clear, then sends a late Stop for the previous session. The current handler does not promote the clear boundary to Running or reject stale Stop mutations, so this test is expected to fail before the lifecycle fix.
Constraint: Tests must exercise the hook handler directly rather than driving the full app.
Confidence: high
Scope-risk: narrow
Directive: Keep this regression on the direct CLI hook path; do not replace it with source-text assertions.
Tested: Not run locally per repository policy.
Not-tested: CI red/green proof pending on GitHub Actions.
* Keep Claude clear sessions authoritative
Claude /clear arrives as a SessionStart source=clear event, but the hook store only remembered routing data. This change makes the store also own the active session for each workspace, promotes clear starts to Running, and ignores visible Stop/Notification/SessionEnd mutations when their session no longer matches the active workspace session.
Constraint: Claude Code documents SessionStart source=clear for /clear lifecycle boundaries.
Rejected: Only set Running on source=clear | old Stop and SessionEnd events could still clobber the new session afterward.
Confidence: high
Scope-risk: narrow
Directive: Visible Claude hook mutations must pass active-session ownership checks before changing sidebar status or notifications.
Tested: Not run locally per repository policy.
Not-tested: CI red/green proof pending on GitHub Actions.
* Allow Claude sessions to advance turns
A visible hook mutation can only be rejected as stale after the active session boundary is known. Keeping a completed turn id active after Stop made the next prompt in the same Claude session look stale before it could promote its own turn.
The Stop path now refreshes the active session with no turn id once the completed turn has been accepted, preserving stale-session protection across /clear while allowing normal multi-turn prompts to re-enter Running. The regression now exercises that two-turn path before the clear boundary.
Constraint: Do not run local tests; CI owns verification for this branch.
Rejected: Ignore turn id in all current-session checks | would weaken stale turn filtering for late Stop and Notification events.
Confidence: high
Scope-risk: narrow
Directive: Do not persist a completed turn id past Stop without proving the next prompt-submit can promote a new turn.
Tested: git diff --check
Not-tested: Local unit/regression tests per project policy
* Remove duplicate Claude clear helper
The branch integration accidentally kept two isClaudeClearSessionStart definitions in CLI/cmux.swift. The duplicate version referenced a non-existent parsedInput.source property, so Swift would reject the file before CI could exercise the hook lifecycle tests.
Keep the existing implementation that reads source from the parsed hook object and delete only the duplicate helper.
Constraint: Fix review-reported compile blocker without changing lifecycle behavior.
Rejected: Add source to ClaudeHookParsedInput | unnecessary for this compile fix and broader than the failing duplicate.
Confidence: high
Scope-risk: narrow
Directive: Keep exactly one Claude clear source helper unless the parsed input model is intentionally extended.
Tested: git diff --check; rg confirms a single isClaudeClearSessionStart definition and no parsedInput.source reference.
Not-tested: Local tests per project policy
* Restore Claude hook compileability after lifecycle merge
The active-session merge left behind the old source-property helper alongside the newer compact-payload helper. Removing the duplicate keeps source=clear detection on the JSON payload and avoids both the redeclaration and missing-property errors.
Constraint: Do not run local tests; CI owns verification for this branch
Rejected: Reintroduce ClaudeHookParsedInput.source | duplicates state already retained in the compact hook payload
Confidence: high
Scope-risk: narrow
Tested: git diff --check; rg verified a single isClaudeClearSessionStart definition and no parsedInput.source references
Not-tested: Local compile/test execution per task policy
* Normalize Claude active-session cleanup keys
The active-session map is keyed by normalized workspace id, so cleanup after consuming a session must use the same normalized key. Otherwise a record containing incidental whitespace could leave a stale active-session entry behind.
Constraint: Address reviewer-reported lifecycle cleanup edge case without changing visible hook behavior.
Rejected: Store raw workspace ids as active map keys | inconsistent with existing upsert normalization and lookup guards.
Confidence: high
Scope-risk: narrow
Directive: Any activeSessionsByWorkspace lookup should use the normalized workspace key, matching insertion.
Tested: git diff --check
Not-tested: Local tests per project policy
* Harden CI Zig downloads against transient upstream failures
CircleCI and the activation workflow both depend on ziglang.org tarballs during remote macOS setup. A transient 500 from that host failed the debug build before any project code compiled, so the install path now retries downloads and avoids unnecessary Homebrew update/cleanup churn on CircleCI.
Constraint: CI must be made green remotely without running local tests or local xcodebuild.
Rejected: Push an empty commit to rerun CI | would leave the same upstream download flake unchanged.
Confidence: high
Scope-risk: narrow
Directive: Keep Zig installer retries in remote CI setup paths; failures here happen before project build logic runs.
Tested: git diff --check
Not-tested: Local tests and local builds not run per repository/user policy.
* Cover stale Claude session-end lifecycle
Greptile identified that stale SessionEnd exercises a different clear-state path than stale Stop. The existing active-session guard already blocks the mutation, so the regression now drives that hook directly and asserts the active /clear session keeps its status, PID, and notifications intact.
Constraint: Do not run local tests; CI is the verification source for this branch.
Rejected: Add another Swift integration test | the existing Python hook harness already executes the CLI handler through the socket path used by CI.
Confidence: high
Scope-risk: narrow
Tested: Not run locally per repo policy.
Not-tested: Local test execution.
* Stop activation CI from stalling on Zig downloads
The activation workflow was cancelled before build because repeated ziglang.org transfers crawled for the full job timeout. Prefer Homebrew's pinned zig@0.15 bottle on macOS runners, then keep the direct tarball path as a bounded fallback with connection, total-time, and minimum-speed limits.
Constraint: The failed check never reached project build or tests; the only failing surface was CI tool bootstrap.
Rejected: Increase the job timeout | it would hide the bootstrap failure and delay feedback.
Confidence: medium
Scope-risk: narrow
Directive: Keep activation workflow tool bootstrap bounded so performance CI reaches the benchmark or fails quickly.
Tested: git diff --check
Not-tested: Local workflow execution, per repository and user instruction.
* Gate Claude session-end cleanup on the consumed workspace
Greptile noted that session-end computed the current-session guard from the fallback workspace while clearing the consumed session's workspace. Move the guard next to the mutation target so stale cleanup and visible cleanup always evaluate the same workspace identity.
Constraint: This is follow-up review hardening on the existing active-session model.
Rejected: Collapse activeSessionsByWorkspace into sessions in this PR | per-workspace current-session lookup is the intended lifecycle boundary for stale event gating.
Confidence: high
Scope-risk: narrow
Directive: SessionEnd visible cleanup must be gated against the workspace being cleared, not an earlier fallback lookup.
Tested: git diff --check
Not-tested: Local test execution, per repository and user instruction.
* Gate Claude session-end cleanup on consumed workspace
A late SessionEnd can be resolved through fallback surface lookup, so the workspace used to find a record is not always the workspace whose visible state would be cleared. Move the staleness check after consume() and evaluate it against the consumed session's workspace, where the clear_status and notification cleanup actually run.
Constraint: Hook events are delivered by short-lived CLI processes and must tolerate stale or partial Claude payloads
Rejected: Keep the pre-consume fallback workspace guard | it can validate one workspace while clearing another
Confidence: high
Scope-risk: narrow
Tested: Added Swift integration regression for stale SessionEnd fallback cleanup
Not-tested: Local tests not run per repository policy
* Make stale Claude session-end test consume the seeded session
Greptile caught that the Swift regression used an unknown session id, so the hook returned before reaching the intended consumed-session visibility guard. Use the seeded stale session id in the SessionEnd payload so the test exercises consume(), then verifies that stale visible cleanup is blocked.
Constraint: Address high-priority review feedback without running local tests.
Rejected: Leave Python-only coverage | the Swift regression would continue passing for the wrong reason.
Confidence: high
Scope-risk: narrow
Directive: Stale session-end regressions should consume a known stale session before asserting visible cleanup is skipped.
Tested: git diff --check
Not-tested: Local tests per repository and user instruction
* Cover fallback Claude session-end consumption
The stale SessionEnd regression should prove the handler consumed the stale session through fallback lookup before deciding whether visible state may be cleared. Use an unknown late session id and assert the seeded stale session is removed from the store, so the test cannot pass without exercising the consumed-workspace guard.
Constraint: Review feedback flagged the prior Swift regression as able to pass without covering the intended guard
Rejected: Use the stored stale session id directly | that only covers the ordinary mapped-session stale path
Confidence: high
Scope-risk: narrow
Tested: Not run locally per repository policy; CI will run the Swift regression
Not-tested: Local XCTest execution
* Keep Claude clear ownership panel-scoped
The clear SessionStart path now owns the active Claude boundary only for explicit clear events, so late startup/resume SessionStart events from the previous session cannot reclaim the workspace before their stale Stop or SessionEnd arrives. The same clear path now passes the resolved surface id into status updates so split panels receive the Running state in the intended pane.
Constraint: Local test execution is disallowed by repository policy; CI remains the behavioral gate.
Rejected: Let every SessionStart mark active | late non-clear events can overwrite the clear boundary.
Rejected: Drop fail-open isCurrent behavior | transient store errors should not hide legitimate current status updates.
Confidence: medium
Scope-risk: moderate
Directive: Do not let non-clear Claude SessionStart events replace an explicit /clear active boundary without adding event ordering metadata.
Tested: python3 -m py_compile tests/test_claude_hook_clear_running_status.py; git diff --check; conflict marker scan.
Not-tested: Local Swift/unit/UI test execution per repo policy.
* Protect Claude clear state from stale session cleanup
Late hook events can arrive after an explicit /clear boundary. The SessionStart handler now skips PID registration when the event is stale against the active session, and SessionEnd consumption preserves the active session when the incoming turn id is older than the stored active turn.
Constraint: Local test execution is disallowed by repository policy; CI remains the behavioral gate.
Rejected: Clear active ownership before checking currentness | same-session stale SessionEnd would fail open and apply cleanup.
Rejected: Keep prefix-only command assertions | option ordering changes would make the regression test brittle.
Confidence: medium
Scope-risk: moderate
Directive: Do not consume a Claude session or replace its PID from a hook event that is stale relative to active session/turn state.
Tested: python3 -m py_compile tests/test_claude_hook_clear_running_status.py; git diff --check; conflict marker scan.
Not-tested: Local Swift/unit/UI test execution per repo policy.
* Allow new Claude sessions to replace stopped owners
A stopped Claude turn must keep the same session eligible for the next turn, but it must not strand the workspace if that process exits before session-end. Mark stopped ownership as replaceable and let only session activation paths use that escape hatch; clear-session ownership remains non-replaceable so stale pre-clear events stay blocked. Session-end fallback cleanup now checks the consumed session id so missing input session ids do not fail open.\n\nConstraint: /clear SessionStart must continue to suppress stale pre-clear startup, stop, and session-end events\nRejected: Clear active ownership on Stop | breaks same-session multi-turn prompt-submit currentness\nConfidence: medium\nScope-risk: narrow\nTested: git diff --check; conflict-marker scan\nNot-tested: local XCTest per repo policy; CI will run cmux unit regressions
* test: cover codex hooks TOML features section
* fix: harden hook config and auth token cache
* test: cover sign-out browser auth loading cleanup
* fix: cancel browser auth on sign-out
* test: cover codex config read failures
* fix: fail closed on codex config read errors
* test: cover auth sign-out callback race
* fix: discard auth callback after sign-out
* fix: keep claude subcommands out of hook injection
* fix: dismiss stale sparkle update replies
* fix: redact auth logs while preserving observability
* fix: reset update checks and guard sign-out races
* fix: preserve update metadata from driver state
* fix: await auth token assertions before comparing
* fix: address wrapper and auth review feedback
* fix: address session and config review feedback
* ci: retrigger pending checks
* fix: clear stale ime and auth token state
* fix: clean legacy codex hooks config
* test: cover legacy codex hooks markers
* fix: clean empty codex features table
* fix: preserve browser key reentry dispatch
* Fix new-workspace caller window routing (#4042)
* test: cover new workspace caller routing
* fix: route new workspace to caller context
* Add iMessage workspace insertion regression test
* Reset Kitty keyboard mode at shell prompt boundaries (#3870)
* Prove stale Kitty keyboard state leaks CSI-u key bytes
The regression exercises a hosted Ghostty terminal, leaves Kitty keyboard protocol enabled as a crashed TUI would, mirrors the clear-history socket handler clear_screen path, and captures raw PTY stdin bytes for a plain c key. Current behavior should encode CSI-u instead of a single ASCII byte, making the test fail until the protocol state is reset at the shell prompt boundary.
Constraint: Regression must cover PTY input bytes, not source text shape
Confidence: high
Scope-risk: narrow
Directive: Keep this test on the real ghostty_surface_key path; sendText alone bypasses the keyboard encoder
Tested: Manual current-main repro captured c9;1:3uc9;1:3uc9;1:3u after Kitty enable plus clear-history
Not-tested: Local unit execution deferred to CI per requested red/green workflow
* Reset stale Kitty keyboard mode at prompt boundaries
A crashed TUI can leave Ghostty's Kitty keyboard protocol stack pushed after clear-history, causing normal shell input to be encoded as CSI-u. Resetting the stack from the shell prompt hook makes the prompt boundary the ownership point for returning interactive shells to plain byte input.
Constraint: Fix must run through shell integration because Ghostty keeps protocol state inside the terminal surface
Rejected: Reset only in clear-history socket path | stale state also leaks after any crashed TUI returns to prompt
Confidence: high
Scope-risk: narrow
Directive: Keep prompt-boundary reset paired across bash and zsh integrations
Tested: Not run locally per repository testing policy; regression added in prior commit
Not-tested: CI not yet completed
* Clarify Kitty reset fixture failures
The stale keyboard regression now fails at the fixture boundary if the zsh integration does not emit the expected reset bytes, instead of falling through to a misleading PTY byte mismatch.
Constraint: Repository policy forbids local test execution for this PR loop
Confidence: high
Scope-risk: narrow
Tested: Not run locally per repository policy; CI will run the affected XCTest
Not-tested: Local XCTest execution
* chore: retrigger Vercel checks
* Add Kitty reset shell hook coverage
* Reset all terminal keyboard protocols at prompt
* Fix terminal keyboard reset test expectations
* Clarify disabling agent session auto-resume for #3640 (#3991)
* docs: explain disabling agent auto resume
* docs: name auto resume config path
* Fix stale restored agent resume state
* Harden restored agent hook liveness
* refactor: share restored agent normalization
* Honor iMessage workspace ordering and previews
* Add workspace cwd inheritance setting (#3921)
* feat: add workspace cwd inheritance setting
* fix: align workspace cwd setting key naming
* fix: apply workspace cwd setting to detached creation
* fix: expose workspace cwd inheritance setting
* fix: route pane break through detached workspace creation
* fix: keep pane break response pane ids non-null
* fix: distinguish pane break resolution errors
* Approve installed Codex hooks (#4035)
* Approve installed Codex hooks
* Address Codex hook trust review feedback
* Avoid tomllib in Codex hook tests
* Align Codex hook trust test mirror
* Harden Codex hook trust ownership
* Preserve legacy Codex hook cleanup
* Fix workspace unit test after merge
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Revert "Approve installed Codex hooks (#4035)" (#4074)
This reverts commit e4546b7675a4ffa5a41a5429218b60f4e7644e21.
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Fix workspace unit test transfer resume state (#4076)
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Clarify in cmux --help that reload-config covers Ghostty config too (#4060)
* Clarify cmux help that reload-config covers Ghostty config too
`cmux reload-config` reloads BOTH ~/.config/cmux/cmux.json and Ghostty
config (~/.config/ghostty/config) and refreshes terminals in place, but
the help/docs only mentioned cmux.json. Agents (and humans) reading the
help would think they had to restart cmux after editing Ghostty config.
- cmux --help "Agent Help" now tells agents where Ghostty config lives
and that reload-config picks it up live.
- cmux docs settings, cmux settings path, cmux config --help now list
~/.config/ghostty/config as a related (not cmux-owned) location and
describe reload-config's actual scope.
- cmux schema sidebarAppearance.tintOpacity description now notes it's
sidebar-only, and points to Ghostty background-opacity / blur for
terminal transparency.
- skills/cmux/SKILL.md mirrors the wording.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Align ghostty_config JSON shape across CLI surfaces
Cursor Bugbot and Greptile both flagged that `cmux settings path --json`
emitted `ghostty_config` as a string while `cmux docs settings --json`
emitted it as an object {path, note}. An agent reading both outputs with
the same key expectation would have to special-case the type.
Standardize on the object shape with `path` and `note` in both, matching
docsPayload. This is the agent-discoverability path the PR is trying to
make reliable, so making the shape consistent is on-purpose.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Use canonical Ghostty config key background-blur (not background-blur-radius)
CodeRabbit flagged that `background-blur-radius` is outdated. Verified
against the Ghostty submodule at ghostty/src/config/Config.zig: line 70
declares `background-blur-radius` as a compatibilityRenamed alias for
`background-blur`. The current canonical key is `background-blur` and it
accepts the same integer value (e.g. `background-blur = 20`).
Update the two docs that introduced the alias name:
- skills/cmux/SKILL.md
- web/data/cmux.schema.json (sidebarAppearance.tintOpacity description)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add nucleo FFI command palette benchmark
* Open right sidebar tools as panes (#4065)
* Open right sidebar tools as panes
* Remove unreachable sidebar pane commands
* Fix sidebar pane unit test build
* Address sidebar pane review feedback
* Fix vault pane focus tracking
* Address right sidebar pane review followups
* Use modern sidebar pane flash observer
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Save crash diagnostics under cmux state (#4077)
* Save crash diagnostics under cmux state
* fix: key GhosttyKit artifacts by crash path
* fix: pin cmux crash GhosttyKit archive
* fix: mark crash breadcrumb scan concurrent
* fix: keep crash scan compatible with Xcode 16
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Menubar global search P1 (#3908)
* Ship local global search as a remappable menubar flow
Phase 1 needs browser and markdown value without terminal scrollback, so the index owns durable FTS5 upserts while AppDelegate keeps one navigation path from palette rows to focused panels. The global shortcut is wired through the existing shortcut settings instead of a standalone Carbon shim so Settings and cmux.json remain authoritative.
Constraint: Phase 1 excludes Ghostty terminal scrollback capture
Constraint: User required no local test execution and no reload before CI is green
Rejected: Hardcoded GlobalSearchHotkey shim | violates KeyboardShortcutSettings policy
Confidence: medium
Scope-risk: broad
Directive: Keep future terminal capture feeding SearchIndex documents through GlobalSearchCoordinator rather than adding another palette/navigation path
Tested: git diff --check; python3 -m json.tool Resources/Localizable.xcstrings; python3 -m json.tool web/data/cmux.schema.json; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj
Not-tested: Local unit/UI tests and tagged app launch per task constraints
* Remove duplicate search refresh work
Cursor review pointed out that the menubar toggle and palette onAppear both refreshed the live index. Keeping the refresh in the palette lifecycle avoids resetting browser debounce tasks while still indexing each time the palette opens, and removing the unused workspace delete API keeps the storage surface honest.
Constraint: Review feedback came from PR #3908 after the first CI pass started
Rejected: Keep both refresh calls | causes avoidable debounce cancellation and slower browser result availability
Confidence: high
Scope-risk: narrow
Directive: Add workspace-level deletion only when a real workspace teardown caller is wired
Tested: git diff --check; python3 -m json.tool Resources/Localizable.xcstrings; python3 -m json.tool web/data/cmux.schema.json; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj
Not-tested: Local tests/build per task constraints
* Fix global search stale index entries
* Address global search review feedback
* Fix search query token mapping compile error
* Fix global search CI failures
* Fix stale unavailable markdown search entries
* Cancel stale markdown search captures
* Cancel global search refresh on dismiss
* Fix global search review followups
* Address global search post-CI feedback
* Preserve markdown panel title search on read failure
* Address global search lifecycle feedback
* Address global search review lifecycle feedback
* Refine global search capture ownership
* fix: address global search review blockers
* feat: show open panels in global search
* fix: cover right sidebar tool panel in search
* Fix cmuxTests: rename restorableAgentAutoResumePending to restorableAgentResumeState (#4068)
* fix: handle repeated assistant imessage completions
* Open supported files in cmux on cmd-click (#4041)
* Open supported files in cmux on cmd-click
* Add cmd-click file preview verification script
* Reuse right pane for cmd-click file previews
* Keep cmd-click UI test terminal after preview focus
* Accept numeric cmd-click test payload values
* Capture cmd-click UI test window snapshots
* Add file-type-aware external open actions
* Address supported file routing review feedback
* Fix external open menu sendability warnings
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Suppress native proxy icon on cmux main windows (#3973)
* Add proxy icon regression test
* Suppress native proxy icon on cmux windows
* chore: retrigger preview deployments
* Refine native proxy icon suppression
* fix: keep titlebar folder icon aligned
* fix: restore titlebar folder icon leading offset
* test: cover folder icon frame replacement
* fix: resync folder icon on frame replacement
* test: cover folder icon ancestor movement
* fix: track folder icon ancestor movement
* fix: address folder icon review feedback
* test: stabilize folder icon ancestor sync
* fix: handle sidebar tool panels in global search
---------
Co-authored-by: austinpower1258 <austinwang115@gmail.com>
* Close browser panels when pages request window close (#4070)
* Add browser self-close restore regression test
* Close browser panels from WebKit close callbacks
* fix: index right sidebar tool panels as titles
* fix: keep web close callback synchronous
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add notification policy hooks
Merged https://github.com/manaflow-ai/cmux/pull/4054
* Fix sidebar unread badge after re-marking notifications (#4084)
* Add sidebar unread notification regression test
* Keep sidebar notification badge live during menu freeze
* Cover sidebar presentation fallback cases
* Limit Cloud VMs by active provider state (#4046)
* test: cover active vm limit with paused freestyle vms
* fix: enforce cloud vm limits by active state
* fix: parallelize cloud vm status refresh
* chore: update web security dependencies
* fix: handle right sidebar tools in global search
* fix: guard cloud vm status refresh races
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Fix Settings search synonyms (#4082)
* Add settings search synonym regressions
* Fix settings search synonyms
* Add shortcut bindings anchor regression
* Cover clickable PR settings search alias
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add tagged debug CLI helper (#4092)
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add unread defer shortcut (#4086)
* Add unread defer shortcut
* fix: address unread defer feedback
* fix: keep manual unread jump explicit
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Document notification hooks default off
Document that notification hooks are off by default and style the config key in localized docs.
* Approve installed Codex hooks after dogfood (#4075)
* Reapply Codex hook approval changes for dogfood
* Notify on Codex plan input requests
* Handle Codex plan question transcript items
* Address Codex hook review feedback
* Recover malformed Codex hook trust blocks
* Avoid duplicate consecutive cmux hook reinserts
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Document session restore behavior
Adds session restore docs, blog, README updates, and review cleanup.
* Use nucleo for command palette search
* Skip unrestorable Claude startup sessions
PR: https://github.com/manaflow-ai/cmux/pull/4079
* Revert "Suppress native proxy icon on cmux main windows" (#4099)
* Revert "Suppress native proxy icon on cmux main windows (#3973)"
This reverts commit 5048440ff44b4edbe328b65403724ff79476b2e6.
* Fix titlebar proxy icon without detached panel sync
* Apply proxy icon override to fallback config
* Restore key regain redraw invariant
* Tune nucleo palette initialism ranking
* Fix terminal portal resize lag (#4102)
* Fix Korean 2-Set terminal arrows (#4095)
* Add Korean 2-Set arrow IME regression
* Restore Korean 2-Set arrow forwarding
* Restore Zhuyin IME command routing
* Address IME review feedback
* Fix Korean IME regression test compile
* Address IME review follow-ups
* Address Bopomofo preedit review feedback
* Avoid idle Zhuyin key suppression
* Remove dead text input wrapper
* Address IME suppression review feedback
* Fix palette stitched highlight precedence
* Add Codex Teams subagent panes
* Open markdown files in preview panels from cmux open (#4085)
* fix: open markdown files in preview panels
* fix: preserve markdown viewer transparency
* fix: mute markdown open-with header button
* fix: align markdown header controls
* fix: align file header controls
* fix: address markdown review feedback
* test: cover opaque text editor alpha
* fix: align header open fallback
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add task manager sorting and program aggregates (#4066)
* Add task manager sorting and program aggregates
* Add sorting to cmux top output
* Add flat TSV cmux top output
* Add coding agent task manager totals
* Make task manager program totals payload-backed
* Recognize agent launcher process names in task manager
* Fix task manager test build helpers
* Recognize Claude versioned launcher processes
* Show loading state before task manager sample
* Recognize versioned agent process names
* Use agent totals for task manager hierarchy icons
* fix: address task manager review feedback
* fix: address follow-up task manager review
* fix: address final task manager review
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Keep manual unread sticky until terminal interaction (#4104)
* test: cover sticky manual unread state
* fix: keep manual unread sticky until terminal input
* fix: show workspace manual unread pane ring
* fix: sync manual unread badge on focus changes
* fix: stabilize manual unread representative fallback
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: austinpower1258 <austinwang115@gmail.com>
* Bump version to 0.64.5 (#4107)
* Fix Pi Vault icon and JSONL titles (#4120)
* test: cover Pi JSONL content block titles
* fix: parse Pi JSONL text blocks for Vault titles
* fix: align Pi JSONL title role handling
* fix: require typed text blocks for Pi titles
* Improve Cloud VM error guidance (#4094)
* Improve Cloud VM error guidance
* Address final Cloud VM review feedback
* Narrow Cloud VM sanitizer env var block
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Keep selected workspace visible after sidebar reorders
* Add sidebar scroll regression for workspace move to top
* Reveal selected workspace after sidebar reorders
* Handle right sidebar tool panels in global search
* Test workspace move to top visibility only
* Refine sidebar reorder scroll trigger
* Add move-to-top notification regression
* Skip no-op move-to-top notifications
* Assert no-op move-to-top keeps order
* Require matching manager for reorder scroll
* Scroll selected workspace on index shifts
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Use transparent backgrounds for file preview panels (#4088)
* Handle right sidebar tools in global search browse hits
* Use transparent backgrounds for file preview panels
* Cover panel theme background preservation
* Fix PDF file preview open menu
* Make file open menu button compact
* Fix PDF open menu chrome button
* Fix PDF open-with chrome click target
* Address file preview chrome review feedback
* Fix PDF background cache invalidation
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Inherit stdin in backgrounded ssh inside startup wrapper (#4135)
* Add regression test for SSH startup wrapper dropping stdin
After PR #3786 backgrounded the ssh invocation inside the startup
wrapper for signal/reconnect handling, `cmux ssh <host>` sessions
stopped forwarding keystrokes from the surface PTY to the remote
shell. Output flowed back (the remote prompt rendered in cmux UI),
but anything typed never reached zsh on the other side, which sat
blocked in `do_poll` on the remote pts.
Root cause: POSIX sh redirects stdin of an async command (`&`) to
/dev/null when job control is off — the default for the `/bin/sh -c …`
that runs the startup wrapper. Without an explicit `<&0` on the
`&`'d ssh line, the local PTY stdin is silently dropped.
This commit adds the failing regression test. The fix follows in
the next commit so CI can prove the test catches the bug.
* Inherit stdin in backgrounded ssh inside startup wrapper
PR #3786 wrapped the ssh invocation in `while :; do … & wait` to enable
SIGHUP/INT/TERM trap handling and reconnect-on-exit-255. Backgrounding
ssh, however, drops its stdin: POSIX sh redirects fd 0 of any async
command to /dev/null when job control is off, which is the default for
the `/bin/sh -c …` host that runs this wrapper.
As a result, output from the remote still flowed back to the surface
PTY (ssh's stdout/stderr stayed wired) but the user's keystrokes never
reached the remote — they hit ttysNNN on the Mac side, kernel echoed
them locally, but ssh's stdin was /dev/null so nothing was forwarded.
zsh on the far side sat in `do_poll` forever.
Explicit `<&0` on both the `command` line and the `( … )` shell-snippet
form overrides the POSIX default and re-attaches the wrapper's own
stdin to the backgrounded ssh process.
Verified with the regression test added in the previous commit, plus
manual repro on Darwin:
/bin/sh -c 'cat & wait' # cat's fd 0 → /dev/null (bug)
/bin/sh -c 'cat <&0 & wait' # cat's fd 0 → parent stdin (fix)
* Add docs search
Adds localized Pagefind docs search with heading anchors and section-aware results.
* Fix Swift interpolation escape in SSH stdin regression test (#4154)
`testSSHStartupForwardsStdinToBackgroundedSSH` used `\"<empty>\"` inside a
`\(...)` string interpolation. Swift parses `\"` as the end of the outer
literal, breaking compilation of cmuxTests on main:
Cannot find ')' to match opening '(' in string interpolation
Unterminated string literal
That kept `ci/circleci: macos-unit-tests` red on main after #4135 even
though `macos-debug-build` and `macos-release-build` still passed (the
test target was the only thing affected). The runtime fix (`<&0` on the
backgrounded ssh) is unchanged and was verified end-to-end on a cloud Mac.
Inside an interpolation, the string is already a Swift expression and
literal quotes are unescaped; drop the four backslashes.
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add command palette settings toggles
Adds command palette toggles for boolean Settings rows, including iMessage Mode.
* Ensure Rust toolchain is installed for nucleo FFI builds
* Document nucleo FFI thread and ABI assumptions
* Reuse nucleo index in preview search test helper
* Install Rust for activation perf builds
---------
Co-authored-by: Tobi Lutke <tobi@shopify.com>
Co-authored-by: Austin Wang <austinwang115@gmail.com>
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Kevin Peng <48529172+kays0x@users.noreply.github.com>
* Address command palette review feedback
* Address command palette review follow-ups
* Fix command palette preview responsiveness
* Build command palette search index off main actor
* Keep small cold palette searches synchronous
* fix: clear panel badges when marking notifications read
* fix: preserve nucleo normalized matches
* fix: preserve typo fallback with nucleo search
* fix: keep search fallback semantics
* fix: keep nucleo aliases authoritative
* fix: preserve typo fallback without ffi contention
* fix: avoid stale palette reset snapshot
* fix: narrow nucleo typo fallback
* fix: address command palette review bots
* fix: preserve palette activation during index refresh
* test: cover nucleo multi-token field matching
* fix: tokenize nucleo ffi queries
* fix: preserve palette activations during index refresh
* fix: sync palette pending state before async search
* fix: keep long keyword matches below title matches
* fix: keep nucleo fuzzy matches within fields
* fix: guard palette preview reuse by fingerprint
* test: skip optional nucleo bundle check without cargo
* fix: keep final palette results uncapped
* fix: respect optional nucleo fallback
* fix: initialize rustup toolchain in release workflow
* fix: run pending palette activation after sync refresh
* Fix command palette duplicate ID indexing
* Remove stale command palette label helpers
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Tobi Lutke <tobi@shopify.com>
Co-authored-by: Austin Wang <austinwang115@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Kevin Peng <48529172+kays0x@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Note
Medium Risk
Touches Cloud VM API/CLI/mac-client error surfaces and validation logic; behavior changes could affect user workflows and error parsing, but changes are largely additive and focused on messaging/response shaping.
Overview
Cloud VM failures are now returned as structured, user-actionable errors (message/action/reason + a small allowlisted
details), and the API routes forvm createandvm exectighten request validation and map workflow/provider/database/billing failures into consistent HTTP statuses.The Swift client and
cmuxCLI are updated to format these errors for humans (Reason/What to do/Details), avoid echoing sensitive/provider-specific values, and improve command usage/flag errors (including safer handling of stray args and shell-quoting forvm exec).The macOS “Start Cloud VM” alert gains localized recovery copy and sanitizes captured CLI output before displaying it, and a new review-bot rule/check (
user-facing-errors.md) is added to enforce error-privacy going forward; tests are extended to assert the new payload shape and non-leak guarantees.Reviewed by Cursor Bugbot for commit eeec2dd. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Standardizes Cloud VM errors across the API, CLI, and app with clear messages, “What to do” guidance, and safe, minimal details. Tightens validation, avoids implementation leaks, adds guardrails and tests, and narrows app-side sanitization to reduce false positives.
New Features
vmErrorResponse/vmWorkflowErrorResponse; create/exec return actionable validation (vm_json_parse_failed/vm_expected_object/vm_invalid_json/vm_invalid_request/vm_invalid_command/vm_invalid_provider); maps provider/database/billing failures to 502/503 (vm_cloud_service_unavailable/vm_cloud_state_unavailable/vm_billing_unavailable) with next steps; structuredvm_not_found; never echo unsupported provider values; allowlists safedetails(e.g.limit,amount,vmId,operation).detailswith sensible defaults by status/code; clearer copy for not signed in, unreachable backend, and malformed responses; trims long bodies to a single line.cmux): v2/RPC errors show Reason/What to do/Details with an allowlist of safe fields; clearer guidance for unknown flags/positional args and provider override; improved usage forvm new|shell|ssh|ssh-info|rm|exec; attach/SSH flows show next steps and redact risky payload details.Guardrails/Tests
.github/review-bot-rules/user-facing-errors.md, enforces via the new “cmux user-facing error privacy” pre-merge check; mirrored in.greptile/rules.md.Written for commit eeec2dd. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation / Chores