Skip to content

arch: inject the peer approver, Cloud machines and remote sidebar instead of app-wide statics - #28268

Merged
lawrencecchen merged 6 commits into
feat-cmux-nextfrom
fix/arch-rise-pane-approvals
Oct 31, 2026
Merged

lawrencecchen merged 6 commits into
feat-cmux-nextfrom
fix/arch-rise-pane-approvals

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Five arch-ratchet sites that rose on feat-cmux-next become instances made once at the composition root and injected. No behavior change.

  • The app makes one AgentPanePeerApprover at launch (PeerApprovalsApp.approver() in AppDelegate). It holds this Mac's approver key, the Enroll this Mac command and the local daemons that got the key. AppServices gets it through its init and gives it to AgentTabStore through its init. The store gives it to every AcpmuxHost it makes (its own host, the Chief home hosts in ChiefHomeAcpmux, and the default hosts of ChiefHomeDirectory), and to every pane transport in makeView. The host registers the key with each daemon it connects. The transport signs with the key. The view runs Enroll through the same object.
  • AgentTabStore owns one AgentPaneCloudMachines (as it owns readAloud). It wires it in wired(to:) and gives it to every page model in makeView. The palette's destinations read it from the store.
  • RemoteMachinesSidebar.shared is removed. Nothing binds the sidebar (65c9441, Lawrence 2026-10-26), so it draws no rows and follows no lanes. It had two callers, and both did nothing:

Sites fixed (paths under Packages/macOS/CmuxNext/Sources/):

Site Added in
CmuxNextAgentPane/AcpmuxMacApprover.swift:25 static let shared (and its author lint:allow) 03076d6 (#26201)
CmuxNextAgentPane/AgentPanePeerApprovals.swift:16 static var enroll 221e6b4 (#26202)
CmuxNextAgentPane/AgentPanePeerApprovals.swift:20 static var daemons 221e6b4 (#26202)
CmuxNextAgentPane/AgentPaneCloudMachineRequest.swift:71 static let shared = AgentPaneCloudMachines() 2c480ab (#26011)
CmuxNextAgentLanes/RemoteMachinesSidebar.swift:25 static let shared = RemoteMachinesSidebar() 9980f32 (#24950)

Ratchet (arch_counts.py --delta origin/feat-cmux-next HEAD): stateful_singletons -3 (CmuxNextAgentPane -2, CmuxNextAgentLanes -1), global_mutable_statics -2 (CmuxNextAgentPane -2), app_lines +0, all other rows +0. arch-ratchet.py --base origin/feat-cmux-next --head HEAD: no count rises. arch-ratchet.json is not changed in this PR.

Owners: Performance Lead. PL-acpmux for the approver key path (AcpmuxMacApprover, AgentPanePeerApprover.register).

Takes back #26201: its AcpmuxMacApprover.shared singleton (and the PeerApprovalsApp.install() call in AppDelegate.swift, AgentPaneHost.swift reading it) becomes one approver made at launch and injected; the signed-Allow behavior stays.
Takes back #26202: its AgentPanePeerApprovals statics enroll and daemons move onto that injected approver; Enroll this Mac runs through the same object.
Takes back #26786: the turns.peerStateChanged hook it added in ChatsFeed+Start.swift called RemoteMachinesSidebar.shared, which is unbound and does nothing (laneWatch is nil); the hook is removed with the singleton. The machine link state #26786 added is untouched.

No capture: no UI change; the same objects are injected instead of read from statics (no view, layout or string changes).

Performance

No perf numbers: an architecture change (singletons and mutable statics become injected instances); no speed claim. perf claim: none.

Testing

The tests that used the statics now make their own instances: AgentPanePeerApprovalsNoticeTests (the Enroll closure and the daemons map go on a test approver), AgentPaneRelayedAllowTests (each peer chat gets its own memory key), RemoteMachinesSidebarTests (its own sidebar).

Observed / Not run

  • Observed: prepush-gate.py reports no new violations. lint-ios-package-conventions.sh --namespace-fix reports no violations. arch-ratchet.py and arch_counts.py --delta give the numbers above.
  • Fleet compile-check a36a982b6a2 of 75d6838 failed: MacName was not in scope in AgentPanePeerApprovals.swift. Commit ed0ebc9 adds the CmuxNextCompat import.
  • Submitted: the fleet compile-check 600bd68b6d084034d4258dc1 of head 0937e42 (the merge of feat-cmux-next). No result yet.
  • Not run: the Swift tests (agents do not build on this VM). The peer-route Allow and Enroll paths have not been tried in a running app.

Changelog

none

Fleet red / green

No fleet red/green: a refactor (statics become instances injected from the composition root); it fixes no bug, production behavior is unchanged, and the removed RemoteMachinesSidebar callers did nothing (nothing binds the sidebar). Test assertions are unchanged (independent review).

…tead of app-wide statics

AcpmuxMacApprover.shared, AgentPanePeerApprovals.enroll/daemons and
AgentPaneCloudMachines.shared become instances made once at the
composition root: AppDelegate makes the app's AgentPanePeerApprover
(key, Enroll this Mac, the daemons its key went to) and hands it to
AppServices, which gives it to AgentTabStore; the store hands it to every
AcpmuxHost it makes (its own, the Chief home's, ChiefHomeDirectory's) and
to every pane's transport in makeView, beside its own AgentPaneCloudMachines.
RemoteMachinesSidebar.shared goes: nothing binds it (65c9441), so its
one caller, activateClientItem, never matched a row.

No behavior change. Tests make their own instances.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fleet compile-check of head 75d6838: job a36a982b6a263d490f09cf9d (cmux-ci compile-check --app --package CmuxNext --recipe-version 2 --timeout 5400). No result yet.

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 291e0a2: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr28268 -f ref=fix/arch-rise-pane-approvals -f platform=macos -f backend_mode=local

Not verified: macOS jobs did not run: compile and app tests were skipped on 291e0a2799; ci-status is green without them.

CI passes on 291e0a2799 (run 41158048957 attempt 1).
macOS jobs did not run: compile and app tests were skipped: Fast static checks skipped (the macos job needs both to succeed). Re-run CI once they pass, before merging.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

web and size checks: green

Shipped in:

…cName

Fleet compile a36a982b6a263d490f09cf9d at 75d6838 failed:
AgentPanePeerApprovals.swift:56:26: error: cannot find 'MacName' in scope. The registration moved
here from AcpmuxMacApprover.swift, which imports CmuxNextCompat; this file did not.
@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Found 1 test failure on Blacksmith runners:

Failure

Test View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs

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

AgentTabs+Wiring.swift: kept feat's machine-link wiring; the Cloud machines
stay the store's own instance (tabs.cloudMachines.wire).
ChatsFeed+Start.swift (6ba8e71, #26786): removed the new caller of the
deleted RemoteMachinesSidebar.shared. Nothing binds that sidebar
(65c9441), and peerStateChanged starts with
`guard laneWatch != nil else { return }`, so the call did nothing; with
turns.peerStateChanged nil the feed's optional call does nothing either.
No behavior change.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

New head 0937e42 (feat-cmux-next merged; the new RemoteMachinesSidebar.shared caller in ChatsFeed+Start.swift removed). Fleet compile-check: job 600bd68b6d084034d4258dc1, no result yet. The previous job a36a982b6a2 at 75d6838 failed on the missing CmuxNextCompat import (fixed in ed0ebc9).

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent review subagent, arch-rise): SHIP at 0937e42
perf claim: none
compile: 600bd68b6d084034d4258dc1 passed

Re-review after the merge 31728ff (origin/feat-cmux-next) and 0937e42 (one declaration per line, no behavior change):

  • AgentTabs+Wiring.swift: the merge keeps feat's machineLink.state/use/reconnect wiring from cmux NEXT: one machine link state for the sidebar and the agent pane; a down machine reconnects on use #26786 unchanged. It also keeps the injected tabs.cloudMachines.wire(services) and the peerApprover given to AgentTabStore, ChiefHomeAcpmux and ChiefHomeDirectory. The diff against origin/feat-cmux-next for this file has only those injection lines.
  • Removed caller ChatsFeed+Start.swift:21 (turns.peerStateChanged = { RemoteMachinesSidebar.shared.peerStateChanged(...) }): peerStateChanged starts with guard laneWatch != nil else { return }, and only bind(host:) sets laneWatch. In Sources nothing calls RemoteMachinesSidebar.bind (git grep finds only PeerStatePushTests). AgentTurnStateFeed calls the optional hook with ?., so a nil hook does nothing. Behavior is the same.
  • git merge-tree --write-tree HEAD origin/feat-cmux-next (6dc0ccf) is clean. The 6 newer feat commits change no Swift. In HEAD and in the merged tree, there are no references to RemoteMachinesSidebar.shared, AgentPanePeerApprovals.enroll/daemons, AcpmuxMacApprover.shared or AgentPaneCloudMachines.shared.
  • Each production construction gives the approver: AppServices (AppDelegate:123, PeerApprovalsApp.approver()), AgentTabStore (Wiring:113), ChiefHomeAcpmux (Wiring:143), ChiefHomeDirectory (Wiring:150), and each AcpmuxHost (ChiefHost:94, :111; PaneSource:67; ChiefHomeDirectory:61). The merge adds no new construction.
  • No static under another name: approver() is a factory with one call at the composition root. cloudMachines and peerApprover are instance lets on AgentTabStore.
  • arch-ratchet against the new base: stateful_singletons -3, global_mutable_statics -2, no count rises.
  • The PR body names both former callers (WhatsNewPage and ChatsFeed+Start).

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 30, 2026
@cmux-bot cmux-bot Bot removed the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
…se-pane-approvals

AgentPaneCloudMachineRequest.swift: feat's recoveryList/attach/complete/account reply, read through the PR's optional injected machines (machines?.).
AgentPaneHost.swift: feat's removal of the unresponsive watch (#28083) and its shared-lookup daemonReady task, plus the PR's injected peerApprover passed to findOrStart.
AgentTabs+Wiring.swift: feat's machineTargets/moveAgentToMachine wiring, plus the PR's peerApprover argument to AgentTabStore.
AppDelegate.swift: feat's keyWindowSource/appActiveSource arguments, plus the PR's peerApprover: PeerApprovalsApp.approver().
AppServices.swift: feat's keyWindowSource/appActiveSource assignment, plus the PR's peerApprover assignment.
@cursor

cursor Bot commented Oct 31, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot 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.

@lawrencecchen lawrencecchen removed the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 31, 2026
…se-pane-approvals

No conflicts. Takes in a582ddf (the Glass.swift keepsActiveLook base compile fix).
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (Claude Opus 5.5 reviewer): SHIP at 291e0a2
reviewer-session: 613245d1-cb2c-44c1-b1c4-9c741720dffa
author-session: arch-rise-pane-approvals-author
R9: pass
perf claim: none
compile: fleet compile-check 0c88eda6f4824615527d3384 at this head passed

Scope: git diff HEAD^2 HEAD (the net change against feat-cmux-next 4d1e169), the merges 31728ff and 8fa43c3, the description, and the checks at this head.

Findings:

  • One instance for each object. git grep at this head finds no AcpmuxMacApprover.shared, AgentPanePeerApprovals.enroll/daemons/daemon(, AgentPaneCloudMachines.shared, RemoteMachinesSidebar.shared, PeerApprovalsApp.install or AcpmuxMacApprover.register in any file, feat code included. PeerApprovalsApp.approver() runs one time (AppDelegate.swift:126). AppServices is the only holder (AppServices.swift:601). AgentTabStore is made only in wired(to:) (AgentTabs+Wiring.swift:113), and gives the same approver to all 4 production AcpmuxHost sites: AgentTabs+PaneSource.swift:67, AgentTabs+ChiefHost.swift:94 and :111, ChiefHomeDirectory.swift:61 (the default ownHost). There is no other AcpmuxHost( outside Tests. So there is one key, one daemons map, and the host registers the same key that the transport signs with.
  • One Cloud machines object: AgentTabStore.cloudMachines (AgentTabs.swift:203), wired at AgentTabs+Wiring.swift:152, and read by the palette (PaletteSourcesBridge.swift:206). All production AgentPaneModel( calls are in AgentTabs.swift (:362, :397, :433, :581), and each one goes through makeView, which sets cloudMachines and transport.peerApprover (AgentTabs.swift:552). No source replaces model.transport after this.
  • machines?. (AgentPaneCloudMachineRequest.swift:176-191): nil occurs only for a model that makeView did not make, and no such model exists in production. A nil gives unsupported, as an unwired .shared gave before. It is not a silent success and not a lost request.
  • peerApprover?.key.decision (AgentPaneTransport+PeerAllow.swift:32, :63): a nil returns the same early exit as an unconfigured .shared before. All production transports get the approver.
  • Merge: feat's recoveryList/attach/complete/account replies are kept with only ?. added. In AgentPaneHost.swift, feat's cmux NEXT: never restart acpmux because it answered late; a waiting pane offers Restart agent service #28083 removal of the unresponsive watch stays; the PR adds only approver:. In AgentTabs+Wiring.swift, feat's machineTargets/moveAgentToMachine/machineLink stay. In AppDelegate.swift, keyWindowSource/appActiveSource stay. In AppServices.swift, the tuple has the fields of both sides. I compared the PR diff at 0937e42 with the diff at this head: the only differences are the lines of these resolutions.
  • The removed turns.peerStateChanged hook (ChatsFeed+Start.swift) did nothing. RemoteMachinesSidebar.peerStateChanged starts with guard laneWatch != nil, and no source calls bind(host:). AgentTurnStateFeed calls the hook with ?.. The machineLink state from cmux NEXT: one machine link state for the sidebar and the agent pane; a down machine reconnects on use #26786 does not change.
  • RemoteMachinesSidebarTests.remoteChatsNeverDrawInTheTopBand now uses its own instance. SidebarModel.topItems(chief:) never read .shared, so the test still tests the same thing.
  • prepush-gate.py --base HEAD^2 --head HEAD: no new violations; stateful_singletons -3, global_mutable_statics -2.
  • Checks: risk evidence, pr-capture-check and pr-perf-section-check started at 01:37Z, after the description edit at 01:33Z, and all passed. Reds: tooling-tests ("Python test execution registry validation failed") and nightly-guards / workflow-guard-tests / release-notary are base findings. Both are red at feat tip 1fd2b84 (run 41155236578) and on next-stable-pipeline (run 41155282453). This PR changes only Swift. nightly-guards / Guard status is a wrapper of release-notary. The failed pr-red-checks / comment was a cancelled run, and a later run of it passed.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (Claude Opus 5.5 independent reviewer): SHIP at 291e0a2
reviewer-session: a074b5bd-225b-49b7-a4eb-68982200ba2f
author-session: merge-28268
R9: pass | no new cap, limit, retry or timeout; the same objects move from statics to instances made once at the composition root
perf claim: none
compile: 0c88eda6f4824615527d3384 passed

Scope: git diff 4d1e169b8e7 291e0a2799e (23 files, +140/-118; 4d1e169 is the merge base with origin/feat-cmux-next). I read the code myself and did not use the earlier review of 0937e42. Paths are under Packages/macOS/CmuxNext/ and line numbers are at the head.

  1. One instance at every site (no nil where production had the shared object)
  • Approver: AppDelegate.swift:125 makes it once (PeerApprovalsApp.approver(), Remote/PeerApprovals+App.swift:13, which always returns an approver). The call is at the same launch point as the old install(), before AppServices. The path is AppServices.swift:527 -> AgentTabs+Wiring.swift:113 -> AgentTabStore.peerApprover (AgentTabs.swift:248).
  • Every production AcpmuxHost( (git grep, whole repo) gets the store's approver:
    • AgentTabs+PaneSource.swift:67: the store's own host, through resolvePane(... peerApprover:) at AgentTabs.swift:256.
    • AgentTabs+ChiefHost.swift:94 and :111: the Chief home hosts, from AgentTabs+Wiring.swift:174.
    • ChiefHomeDirectory.swift:61: the default ownHost of the other Chief homes, from AgentTabs+Wiring.swift:178.
  • The only internal init is reached through the public resolve: init, which forwards peerApprover (AgentPaneHost.swift:200). All four old AcpmuxMacApprover.register calls in findOrStart are now approver?.register (AgentPaneHost.swift:524, :551, :592, :610).
  • Panes: every AgentPaneModel( in Sources is in AgentTabs.swift (:362, :397, :433, :581). Each goes through makeView, which sets readAloud, cloudMachines and transport.peerApprover before the view exists (AgentTabs.swift:552). Spares (makeSpare) and standalone views use the same path, and nothing replaces a model's transport later.
  • Cloud machines: one AgentTabStore.cloudMachines (AgentTabs.swift:203), wired in AgentTabs+Wiring.swift:152. Page models read it at request time (AgentPaneModel.swift:458) and the palette reads the same object (PaletteSourcesBridge.swift:206). There is one AppServices and one AgentTabStore in production (AppServices.swift:473, lazy).
  • The daemons map and Enroll were statics. They are now on the one approver (AgentPanePeerApprovals.swift:25-41). The view reads them through model.transport.peerApprover (AgentPaneView+RemoteApprovals.swift:34, :102-104). This is the same instance for every pane, so registration on one host and Enroll from another pane see the same map, as before.
  • RemoteMachinesSidebar.shared removal: the sidebar is never bound (SidebarSectionHandlers.swift:42), and peerStateChanged returns while laneWatch == nil (RemoteMachinesSidebar.swift:149). The removed ChatsFeed+Start hook did nothing. activate(id) matches only ids from its own rows, and nothing in Sources builds those rows, so the WhatsNewPage.swift:28 removal changes nothing a click can reach.
  1. Leftover static uses: git grep for AcpmuxMacApprover.shared, AgentPanePeerApprovals.enroll/daemons/daemon(, AgentPaneCloudMachines.shared, RemoteMachinesSidebar.shared and PeerApprovalsApp.install finds nothing at the head. On origin/feat-cmux-next b828e44, every hit is a site that this PR changes. Since the merge base, the only feat commit that touches these files is b8e9b0a (cmux NEXT: a subagent's pane is named after the subagent, not its parent chat #28497), which adds no model, host or transport. git merge-tree of the head with b828e44 is clean. The tests that used the statics now make their own instances (AgentPanePeerApprovalsNoticeTests.swift:34, :59, :113; AgentPaneRelayedAllowTests.swift:346, :387, :603; RemoteMachinesSidebarTests.swift:125).

  2. Merge resolutions (8fa43c3, 5 conflicts): for each conflicted file, git diff 99e28e4b57b 8fa43c32504 (feat parent -> merge) is the same set of lines as the PR's net diff. So the merge kept all of feat and added only the PR's lines.

  • AgentPaneCloudMachineRequest.swift:176-192: feat's recoveryList, attach, complete and account reply all read machines?.. With nil, every method is refused, which is the same as the old unset shared object.
  • AgentPaneHost.swift:346-352: feat's shared lookup task (daemonReady inside the task) is unchanged, and approver is captured and passed to findOrStart. The removed unresponsive watch (cmux NEXT: never restart acpmux because it answered late; a waiting pane offers Restart agent service #28083) does not come back.
  • AppDelegate.swift:125 and AppServices.swift:527: feat's keyWindowSource and appActiveSource are present, plus peerApprover.
  • AgentTabs+Wiring.swift: feat's machineTargets and moveAgentToMachine are present, plus peerApprover.
  • The PR's own diff before the merges (0937e42 against its base) and after them differs only in feat's lines (unresponsive watch, recoveryList and attach, window sources). The intent of both sides survives. 291e0a2 had no conflicts.
  1. Repo rules: no new main-thread blocking. AcpmuxMacApprover(keychainService:) does the same lock-and-detach as the old configure call. registerNow stays MainActor with the identity read @concurrent off main, as before. No new Task without an owner (the existing task-owner comments moved with the code). No new CI gate. The takes-back lines for cmux-next: Allow on a peer-route chat's prompt goes out signed by this Mac's approver key (peer-route approvals 4/5) #26201, cmux-next: a peer-route chat's card says "Enroll this Mac on HOST" and enrolls it (peer-route approvals 5/5) #26202 and cmux NEXT: one machine link state for the sidebar and the agent pane; a down machine reconnects on use #26786 are in the description. No user-visible string changed.

Not verified (read only): no Swift test was run (on this VM, agents do not run tests locally). The peer-route Allow and Enroll paths have not been tried in a running app.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Red checks at head 291e0a2, classified (merge agent):

Compile of this exact head: fleet compile-check 0c88eda6f4824615527d3384 passed (CmuxNext and app).

@lawrencecchen
lawrencecchen merged commit 8b3fa55 into feat-cmux-next Oct 31, 2026
164 of 175 checks passed
@lawrencecchen
lawrencecchen deleted the fix/arch-rise-pane-approvals branch October 31, 2026 02:03
lawrencecchen added a commit that referenced this pull request Oct 31, 2026
Conflicts: feat removed AcpmuxUnresponsiveWatch (#28083), so AcpmuxLocalDaemons
drops its watch and AcpmuxHost its restartsUnresponsive toggle; the host keeps
feat's AcpmuxDaemonStop (Restart agent service) beside the injected daemons.
feat's peer approver (#28268) and this PR's daemons/updates both flow through
AgentTabStore, ChiefHomeAcpmux, resolvePane and the ChiefHomeDirectory ownHost.
ChatsFeed drops both removed statics (peerStateChanged's sidebar, the
peerSessionChanged static). New feat tests (AcpmuxDaemonStopTests,
AcpmuxSlowDaemonTests, AgentChatPeersPendingTests, CloudLinkEndedHeaderTests)
pass their own AcpmuxLocalDaemons / DaemonUpdateBoard.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 291e0a27 Deployed Oct 31, 2026 by lawrencecchen via cmux-tui tree preflight #86322
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant