Repository navigation
fix(agent-pane): trust folders opened by the user (cx-pagr0p) - #26130
Conversation
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 13531e3 to compare; do not merge until green; see lane-rules |
|
Found 6 test failures on Blacksmith runners: Failures
|
|
Bead |
9cbcca2 to
27e857b
Compare
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Orphaned: no activity for 12 h, and no lane holds a lease on it. Adopted by the orphan sweep: merged |
|
Owned by an agent session · session |
|
Review (codex gpt-6-astra high, next-review): SHIP at 80b076a No blocking findings. Reviewed the supplied diff and surrounding code. Native tests were not run in this read-only environment. |
Conflict in AgentPanePromptJournal.write: base (#26130) added pendingTrust to the document and moved the generation marker only on success. Kept base's write for every change() caller (now through writeFile); the record path writes the whole document, pendingTrust included, through the same writeFile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-ceed7afbcaf7b9e913b7
|
Shipped in:
|
|
Shipped in:
|
…g record Review of the base merge: #26130's test calls AgentPanePromptJournal.record, which this PR made throwing; it did not compile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-ceed7afbcaf7b9e913b7
…ath is refused, not a crash (#28492) (#28495) * test(next agent pane): a status read on a socket path past sun_path must throw, not trap (#28492) AcpmuxStatusClient.call hands the path to NWEndpoint.unix, which traps on a path longer than sun_path. Red at feat-cmux-next df9b52b: the CmuxNextAgentPaneTests process crashes in AcpmuxStatusClient.call (AcpmuxStatusClient.swift:223), and the swift test job reports it as 'no test finished in 300s' (jobs 124160068616, 124162143724). * fix(next agent pane): refuse an acpmux socket path past sun_path instead of trapping (#28492) AcpmuxStatusClient.call built NWEndpoint.unix(path:) from any path. The Network framework traps on a path longer than sun_path, so the page's _acpmux/status read (AgentPaneTransport+Check.swift:45) with a long TMPDIR path crashed the whole process. The test journalReplayWaitsForDeferredTrustAcknowledgement (#26130) sends that read with $TMPDIR/send-picked-<UUID>/acpmux.sock (115 bytes on CI), so the CmuxNextAgentPaneTests process crashed in every full run and the swift test job stopped with 'no test finished in 300s', hiding every later test. AcpmuxStatusClient.unixEndpoint now refuses such a path with Failure.socketPathTooLong, the same limit (maxSocketPathBytes) that LocalRouterClient.endpoint and the raw-socket clients check, and both NWConnection call sites in the module (call, watchHarnesses) use it. * test(next): the mobile daemon lane and the Cloud hub opener refuse a socket path past sun_path, not trap (#28492) UnixSocketLane.connect and HubLoopbackOpener build NWEndpoint.unix(path:) from a path their caller gives (a daemon socket, the hub's reply). The Network framework traps on a path longer than sun_path, so both crash the process at feat-cmux-next, as AcpmuxStatusClient.call did. * fix(next): one guard before NWEndpoint.unix for every Unix socket client (#28492) NWEndpoint.unixSocket(path:) in CmuxNextCompat refuses a path past sun_path (103 bytes) with UnixSocketPathTooLong; NWEndpoint.unix(path:) traps on one. Every NWConnection to a Unix socket now goes through it: AcpmuxStatusClient (status reads, harness watch), LocalRouterClient, the mobile daemon lane (UnixSocketLane.Failure.pathTooLong) and the Cloud hub opener (an unavailable tunnel). No raw .unix(path:) is left in Sources. * test(next): the Cloud hub path test binds its opener before #require (Sendable hub closure) #require(HubLoopbackOpener(hub: { ... })) passed the hub closure through the macro's non-Sendable parameter; the other tests in the suite bind the opener first. Compile error in fleet compile-check f23285df1e3c2d24b0a79b48 (CloudHubLoopbackTests.swift:22). * test(next): the sun_path regression tests run the call in an exit test's child and print its frames (#28492) A trap in the test process ends every test still running, and a fleet step prints no Swift backtrace (SWIFT_BACKTRACE is not on the ci-step allowlist), so a red proved only 'exited with signal 5'. Each of the three site tests now runs its call in an exit test's child. ChildCrashFrames (CmuxNextTestSupport) replaces the child's fatal-signal handlers: a trap writes the crashing thread's frames to stderr and exits 128 + signal, and the parent records them in its own failure. The child accepts any refusal; the guard's exact refusal is UnixSocketEndpointTests. * test(next): each sun_path child asserts the guard's own refusal, not any error (#28492) The exit-test children accepted any error, so a failure other than the guard (a timeout, a failed connect) also passed. Each child now requires the guard's exact refusal (AcpmuxStatusClient.Failure.socketPathTooLong, UnixSocketLane.Failure.pathTooLong, the hub opener's new socketPathTooLong reason) and names anything else on stderr (ChildCrashFrames.fail). ChildCrashFrames only reports a crash: its handler exits 128 + signal, so a caught trap still fails processExitsWith: .success. * test(next): the hub path child compares its error as LoopbackTunnelFailure In the exit test's do/catch the caught error is any Error, so 'error == .unavailable(...)' resolved .unavailable against Locale.LanguageCode. Compile error in fleet compile-check d3a1f9d7a96e641f6ad37b48 (CloudHubLoopbackTests.swift:34). * test(next): exit test children hand their crash frames to the parent through a file (#28492) #expect(processExitsWith:) returns nil when it fails, so the observed standard error of a crashed child never reached ChildCrashFrames.record: red step 3910a55b2487a344830dd60c showed only '.success -> .exitCode(133)' (128 + SIGTRAP from the handler). The child's handler now also writes the frames to a file named by the test and the parent's pid (open/write are async-signal-safe), and record(name) reads it after the expectation in every case. * test(next): ChildCrashFrames makes its frame buffer before it sets the handlers (review of #28495) A Swift global is made at its first use, and frameBuffer was first used inside the signal handler, so the handler called malloc; a signal raised inside malloc could then deadlock the child (review comment 6533311272). printOnFatalSignal now touches the buffer and its size before it installs the handlers. * test(next): ChildCrashFrames' frame count is nonisolated (CmuxNextTestSupport defaults to the main actor) Compile error in fleet compile-check 969f886c20bcb6869ffd737b (ChildCrashFrames.swift:42, :78, :81): the target's default isolation made the new global frameCapacity main-actor-isolated.
…s, not on _acpmux/status (#28492) (#28529) pendingTrustNeverMovesFromDaemonAToDaemonB (#26130) used the page's _acpmux/status as its barrier: it waited until the stand-in daemon saw that frame, then checked that no acp.trust.set had reached daemon B. Since #27102 (9328d55) the pane answers a page's _acpmux/status from its daemon's socket and never relays it (AgentPaneTransport+Check.swift :99, covered by AgentPaneTransportStatusTests), so the barrier never came and both cases failed at :336, at feat-cmux-next df9b52b (fleet job f44e8e745193fb4ba92d2646) and with #28495 (b2ffa8a35039b75baf009b9e). The crash fixed by #28495 hid it in CI. The local answer is the intended behavior, so the test changes: its barrier is _acpmux/harnesses, a read-only request the pane relays unchanged. What the test checks is the same. Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
A user who explicitly opens a local project folder in cmux now records that folder's persistent trust consent. The harness reuses that consent instead of showing a second trust prompt. Deferred consent is acknowledged before a queued chat starts, and a folder chosen after the connection closes remains queued for reconnect.
Native folder selection and existing-root selection preserve the gesture needed by the first message. Page-provided paths, remote folders, and home or ancestor paths retain their existing confirmation boundaries.
Notes addressed
Observed / Not run
At
a845f39141f7f5571632f66591d34e0b861a45f2, the pre-push gate andgit diff --checkpassed. Focused native verification passed in cmux-ci jobc681ca6be79915d19442bca3: full test-bundle compilation, all 19AgentPaneSendPickedFolderPromptTests, and all 7AgentPaneUnsentPromptTests(prompt-journal tests), exit 0. Command:The tests cover trust acknowledgement before a chat starts or a saved prompt replays, refused and failed sends, app restart, queued consent, daemon isolation, picks between connections, empty journals, and existing folder guards. Test-only baseline job
9c2c71175467220c6a3a8bc4at7c8f4f82cdc94bc860ea17e8bf7151b08d4fc9bdreproduced six refusal/replay failures. Earlier baselinea47d8920fc93542de7569d01reproduced replay before trust and consent lost on failed sends.Not run: the complete native test suite. The focused suites above passed; broader check reconciliation is still pending.
No capture: no visual UI changes; this change records trust and orders transport requests.
Change details
The native host sends
acp.trust.setfor canonical local project picks after initialization and person proof. The drain waits for the daemon's response before forwarding a queued chat start or replaying journaled prompts. A failed socket send preserves consent for reconnect and ends that failed connection. Disconnect resolves outstanding callers and preserves their folder consent for a later connection. A refused trust write keeps both consent and journaled prompts for reconnect; it cannot release the trust barrier. All failed host completions use the same path. Pending consent is written beside the prompt journal when the pick is enqueued, before waiting for an existing drain or sending trust, then restored after a full app restart and removed only after the matching daemon acknowledges it. Every in-memory and queued request carries its daemon identity; switching connections never sends daemon A’s consent to daemon B. A journal write failure blocks sends. Journal I/O uses its existing file queue, and folder plus home canonicalization run off the main actor.Check reconciliation
The current release-notary job reproduces the same three failures as base
ca6ea42e4b4b26d70322c567c137a9eb6a65e843, job122324498981: the existingcloud-start-session-chat-link.mddocumentation digest (the same 865-character difference), the nightly appcast-signing guard, andtest_the_appcast_comes_from_the_tested_plist(ValueError: substring not found). The aggregate guard inherits those failures. No changed-code failure is exempted. The exact-head Release compile passed. Broader native checks remain active; their terminal results must be reconciled before landing.Ownership and review
This PR owns folder trust only. Model-picker appearance, fork behavior, and the empty project state are separate work. The overlap check found shared transport foundation files but no duplicate folder-trust fix.
Review (codex gpt-6-astra xhigh, review-26130g): SHIP at a845f39
Agent session: agent session