Skip to content

base red: a Forget's acpmux peer removal moves to CmuxNextServer (#28799 app_lines +75) - #29478

Merged
cmux-bot[bot] merged 2 commits into
feat-cmux-nextfrom
base-red/ssh-peer-forget
Oct 31, 2026
Merged

cmux-bot[bot] merged 2 commits into
feat-cmux-nextfrom
base-red/ssh-peer-forget

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Base red, priority zero (TIP RED 1881). The tip watcher's arch-ratchet.py --base 30806df8b37d --head 644491fdf9ef refuses [app_lines] CmuxNextApp Swift lines: +75. By the same rule on each first-parent commit, the only culprit is 4a46cfa (#28799, "Forget and Devices Remove each take out both the SSH machine and its acpmux peer"): app_lines +75, with no exception entry. Every other commit in the range shows no rise.

Takes back #28799: it put the Forget's acpmux peer removal (about 75 lines) in CmuxNextApp's SSHService, and the app_lines ratchet refuses that. This PR moves that logic, with the same behavior, to CmuxNextServer, next to DeviceDirectory, and the App keeps only the calls.

Fix forward with a move (no exception row):

  • SSHPeerForgets (new, CmuxNextServer/AddDevice) owns the removal of a written Forget's acpmux peer:

    • the default acp host ls / acp host rm through the app's cmux-tui
    • the marks of Forgets whose Undo still shows
    • the pending Forget record (Pending, was SSHService.PendingForget)
    • the exact-route match (reaches, sameRoute)

    CmuxNextServer now depends on CmuxNextRemote (ui -> service, allowed).

  • SSHService keeps the calls: hide, unhide, commitForget (with its check that the Forget still holds and no other session reaches the route), forgetMachines(reachingPeer:), and the connect that clears a machine's marks.

  • Reconnect's decision table moves to CmuxNextRemote as SSHConnectionMachine.Status.reconnectStep(daemonConnected:daemonFailed:). It read only daemon == .connected. This second move is what takes out more App lines than Forget and Devices Remove each take out both the SSH machine and its acpmux peer #28799 added.

No behavior change.

Testing

The watcher's rule (arch-ratchet.py, row app_lines):

  • before, the culprit alone (--base 4a46cfa244c^ --head 4a46cfa244c): [app_lines] ... +75 ... push refused.
  • this commit (--base 644491fdf9e --head HEAD): app_lines -79, no architecture count rises.
  • the watcher's range (--base 30806df8b37d --head HEAD): app_lines -4, no architecture count rises.

The app-size rule (scripts/cmux-next/check-app-size.sh --base):

  • at 4a46cfa against its parent: CmuxNextApp source grew (118015 source lines now, 117971 at 4a46cfa244c^), exit 1.
  • at this head against 644491f: ok ... no source-line growth (117973 now, <= 118015 at 644491fdf9e).

After the merge of feat (head 61c459c, base 19807ed), measured the watcher's way (arch-ratchet.py --base 30806df8b37d --head <rev>, row app_lines):

prepush-gate.py (pre-push): no new violations, at both pushes. The namespace lint on the changed sources: OK.

Tests:

  • ForgetRemovesBothListsTests read the same cases through the moved code. It also has two new checks:
    • a written Forget takes the SSH machine out of the list as well as its peer;
    • SSHPeerForgets on its own: a shown Undo keeps the peer, only the route's peer is picked, and a connect clears the marks.
  • RemoteConnectRecoveryTests read the moved reconnect step.

Observed / Not run

Observed:

  • The rule runs and the prepush gate above.
  • At the first head 39e589b: fleet app compile job 4daae3f9591e0ba9093f620c passed.
  • At this head 61c459c (after the merge of feat, including base red: CmuxNextAppTests compile again (missing CmuxNextWakeups import; deleted SidebarRowKey.tab) #29486's test compile fix): compile-check --app --package CmuxNext --recipe-version 2, job d551212ef05470352fc592f6: CmuxNext passed (308 s), app passed, cmux-tui passed.
  • Fleet tests at 61c459c, step 046dbdf157d63b98a617b6d5 on aws-m4pro-7: ForgetRemovesBothListsTests 8 of 8 passed, RemoteConnectRecoveryTests 10 of 10 passed.
    Not run: no local Swift build or test (fleet only). No tagged build: no behavior change.

Changelog

none

Proof

No capture: no visible change; code moves between targets with the same behavior.

…_lines)

#28799 (4a46cfa) added 75 CmuxNextApp Swift lines (SSHService.swift,
AddDeviceServices+App.swift) with no exception entry; arch-ratchet's
app_lines row refuses it alone (--base 4a46cfa^ --head 4a46cfa: +75),
and the tip watcher's range 30806df..644491f is +75.

- SSHPeerForgets (new, CmuxNextServer/AddDevice, next to DeviceDirectory)
  owns the removal of a written Forget's acpmux peer: the default `acp host
  ls`/`acp host rm` through the app's cmux-tui, the marks of Forgets whose
  Undo still shows, the pending Forget record (Pending, was
  SSHService.PendingForget) and the exact-route match (reaches, sameRoute).
  CmuxNextServer now depends on CmuxNextRemote (ui -> service).
- SSHService keeps the calls: hide, unhide, commitForget (with its "Forget
  still holds and no other session reaches the route" check),
  forgetMachines(reachingPeer:) and the connect's mark clear.
- Reconnect's decision table moves to CmuxNextRemote as
  SSHConnectionMachine.Status.reconnectStep(daemonConnected:daemonFailed:)
  (it read only `daemon == .connected`), so the move takes out more App
  lines than #28799 added.

CmuxNextApp -79; the watcher's range is -4. No behavior change.
ForgetRemovesBothListsTests read the same cases through the moved code, now
also check that a written Forget takes the SSH machine out of the list as
well as its peer, and test SSHPeerForgets on its own.
RemoteConnectRecoveryTests read the moved reconnect step.
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
@github-actions github-actions Bot added risk:normal Risk tier (docs/ci/risk-tiers.md) and removed risk:low Risk tier (docs/ci/risk-tiers.md) labels Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): SHIP at 39e589b

Independent read-only review of the diff against 644491f and of the moved code from 4a46cfa (#28799). I did not compile Swift locally (fleet only).

Correctness: Forget and peer removal (no behavior change found)

  • Predicate: before, it was stillForgotten && !(machines.ssh reaches route || undoable reaches route) && reaches(route, peer). Now holds() = stillForgotten && !machines.ssh reaches route, then !undoable reaches route, then reaches. All three are pure reads, so the result is the same in every case.
  • The default remover asks the predicate again in the for ... where matches(...) clause on each iteration, after the previous host rm await. Same as before.
  • ended(pending) runs before the guard in both unhide and commitForget. connected(machineID) replaces the undoable.filter at the same place in connect. hide adds the mark before session.close(), as before.
  • [weak self]: SSHService's holds returns false when the service is gone, as before. The new weak self in SSHPeerForgets has no effect, because the async method keeps self alive for the call. A cli of nil gives remove == nil, so await remove?(...) does nothing, as before.
  • token = UUID() is a let with a default value, so the synthesized init leaves it out and creates a new UUID each time. Every Forget still gets its own token.

Reconnect

  • DaemonStartupState is Sendable, Equatable, and .connected has no payload. So startup == .connected is true only for .connected, and false for .connecting and .unavailable(_). That is the same as the old where daemon == .connected. It is now evaluated eagerly, but the comparison has no side effects. The case table did not change, and the switch in reconnect is still exhaustive.

Compile (read against the Package.swift settings)

  • Pending: the synthesized memberwise init is internal and leaves out token. The only call is Pending(...) inside CmuxNextServer, with the argument order matching the declarations. All stored properties are Sendable (SSHDestination is Hashable, Sendable).
  • public import CmuxNextRemote and public import Foundation cover the public API (SSHDestination, URL, UUID). HostCLI.init(binary:environment:) and json(_:label:) are public and match. DeviceDirectory is in the same module.
  • while is a valid argument label. The calls use trailing closures.
  • RemoteForgetUndo.swift never names the type. MemberImportVisibility is not on, so pending.label resolves without import CmuxNextServer.
  • peerForgets = SSHPeerForgets(cli: binary, environment: Self.environment) reads binary before all stored properties are set. The same init already does this with installer = RemoteInstaller(paths: paths), and SSHService is final. I expect it to compile, but only the fleet compile can confirm it.
  • Test targets import CmuxNextRemote and CmuxNextServer and depend on both.
  • Package edge CmuxNextServer -> CmuxNextRemote: no cycle (CmuxNextRemote -> Compat, Cloud). Layers: ui -> service. layer_gate.py --base 644491fdf9e --head HEAD says layer-gate: ok.

Arch ratchet

  • --base 644491fdf9e --head HEAD: app_lines -79, "no architecture count rises".
  • --base 30806df8b37d --head HEAD: app_lines -4, "no architecture count rises".

Repo rules

Non-blocking notes

  • The Package.swift layer comment (about line 103, "CmuxNextServer -> Design, Icons") does not list Remote (and it did not list Cloud before this PR).
  • SSHPeerForgets' default remover (acp host ls, filter, acp host rm) duplicates DeviceDirectory.removePeers(where:). This came with Forget and Devices Remove each take out both the SSH machine and its acpmux peer #28799 and was not added by this PR.
  • Pending is a pure value type. It takes MainActor isolation by default, the same as the old PendingForget did.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): SHIP at 61c459c

Re-review of the merge only (parents 39e589b and origin/feat-cmux-next 19807ed). Earlier SHIP at 39e589b stays valid for the PR's own change.

  • Merge is clean and adds nothing. git diff 19807ed6820 61c459c7d95 equals git diff 644491fdf9e 39e589b02f4 line for line (only index lines and hunk headers differ): the same 6 files, +168/-116.
  • ForgetRemovesBothListsTests.swift keeps import CmuxNextWakeups from base red: CmuxNextAppTests compile again (missing CmuxNextWakeups import; deleted SidebarRowKey.tab) #29486 (0ba83d5). The PR's own edits to that file are unchanged on top of it.
  • Of the 20 commits in 644491f..19807ed, only 0ba83d5 touches the paths this PR moves or calls (SSHService, SSHPeerForgets, DeviceDirectory, SSHConnectionMachine, RemoteForgetUndo, RemoteConnectRecoveryTests, AddDeviceServices+App, Package.swift), and only that one import line.
  • git grep on the merge tree: no caller of SSHService.reaches, SSHService.sameRoute, SSHService.removePeers, SSHService.PendingForget, SSHService.reconnectStep or SSHService.ReconnectStep remains. Every use goes to SSHPeerForgets.reaches/sameRoute/removePeers(at:), SSHPeerForgets.Pending, or SSHConnectionMachine.Status.reconnectStep(...). RemoteForgetUndo still calls ssh.hide/unhide/commitForget, which stay on SSHService.
  • arch-ratchet.py --base 19807ed6820 --head 61c459c7d95: exit 0, "no architecture count rises", app_lines -79 (CmuxNextApp).
  • check-app-size.sh --base 19807ed6820: exit 0, "no source-line growth (118068 now, <= 118110 at 19807ed)".
  • Not run: no Swift compile or tests (static review only, by repo rule). The compile proof must come from CI or the fleet on this head.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fleet evidence at 61c459c: compile with tests, job d551212ef05470352fc592f6: CmuxNext passed (308 s), app passed, cmux-tui passed. Tests, step 046dbdf157d63b98a617b6d5 (aws-m4pro-7): ForgetRemovesBothListsTests 8/8, RemoteConnectRecoveryTests 10/10 passed. Receipt at this head: #29478 (comment)

@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
@cmux-bot
cmux-bot Bot merged commit d1f3afc into feat-cmux-next Oct 31, 2026
225 of 237 checks passed
@cmux-bot
cmux-bot Bot deleted the base-red/ssh-peer-forget branch October 31, 2026 16:31
lawrencecchen added a commit that referenced this pull request Oct 31, 2026
…xtLayout (app_lines) (#29514)

The tip's arch-ratchet app_lines row is red over 30806df: three
commits add CmuxNextApp lines with no exception entry, each refused alone
by `arch-ratchet.py --base <c>^ --head <c>`:
- 0703de4 (#28049, Move Space to This Window / Merge All Windows): +99
- 088b1bc (#27919, a link from another app never reshapes Chief's Home): +16
- 7a12ed3 (#27825, terminals stay quiet while a handoff adopts their shell): +12
(the last two came in through the direct-push merge 8dd8bdc; #28799's
+75 is #29478's).

Their own code is wired into WindowManager, ExternalOpenController and
AppControl, so it cannot leave the App by itself. Instead, the App's pure
window membership model moves out whole: WindowRegistry (struct, Window,
Changes, every transition) and its repairedSelection extension go
unchanged to CmuxNextLayout/Windows with `public` access; its persistence
extension (WindowRegistry+Records) and the observable store stay in the
App. WindowRegistry used only CoreGraphics and Foundation (and Design's
FocusAfterClose, which CmuxNextLayout already depends on). App and test
files that name the type import CmuxNextLayout.

CmuxNextApp -426 against the tip; the watcher's range from 30806df is
app_lines -226 with this alone. No behavior change.
teamleaderleo added a commit that referenced this pull request Oct 31, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

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

Shipped in:

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 61c459c7 Deployed Oct 31, 2026 by lawrencecchen via wait for same-key tree publisher #90448
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