Repository navigation
Conversation
|
@ejc3 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
ejc3
force-pushed
the
all-ej-changes
branch
2 times, most recently
from
September 14, 2026 20:45
de51fd9 to
d367f04
Compare
|
Deployment failed for project cmux with the following error: |
ejc3
force-pushed
the
all-ej-changes
branch
5 times, most recently
from
September 20, 2026 05:54
05e4f1e to
3a5ecc7
Compare
…ad of retrying forever A mirrored tmux session froze permanently if its control stream dropped while the shared ControlMaster was gone and the host wanted interactive authentication. The mirror kept showing its last frame with no error, and the only way out was closing every workspace for that host and re-attaching. The reconnect re-runs `tmux -CC attach` over pipes with BatchMode=yes and no controlling tty, so a password, MFA push, or security-key touch can never be satisfied. handleStreamEnd classified every non-"session gone" failure as transient and rescheduled with backoff, so it retried an authentication that was structurally incapable of succeeding. The initial attach already handles this: it returns .authRequired(sshArgv:) and the `cmux ssh-tmux` CLI, which has a tty, runs that argv to open the shared master. A later reconnect has no CLI in the loop, so nothing consumed the signal. Now handleStreamEnd classifies three outcomes via RemoteTmuxReconnectDisposition. On .authRequired the retry loop stops and the connection parks: state stays .reconnecting, so the tmux session and every mirrored workspace stay intact. cmux opens a workspace titled "Sign in to <host>" running the same interactiveAuthInvocation(), polls the local control socket with `ssh -O check`, and resumes the parked connection once the master is live. Details worth knowing while reading this: The classifier uses indicatesInteractiveRetryWillHelp, the same composed predicate every initial-connect site uses, so a ProxyCommand that closes the transport silently under BatchMode also stops retrying rather than staying in the forever-retry. The login command is wrapped in `/bin/sh -c`. A terminal command runs as `bash --noprofile --norc -c "exec -l <command>"`, and that `exec -l` replaces the shell with the command's first program — so with ssh written at the top level, the result message and the interactive shell that holds the pane open are unreachable. The pane then dies the moment ssh exits and cmux closes a workspace whose child exited, so the login tab appeared and vanished in under a second. The login workspace goes through withSocketCommandPolicy(commandKey: "workspace.create"), because `focus` is honored only while a matching allowance is on the calling thread's stack; without it the workspace is created and never surfaced. RemoteTmuxLoginOffers keeps the "one login per host" rule honest, and both halves are load-bearing. The slot is reserved BEFORE the workspace is created, because several sessions on one host report auth-required in the same turn and creating a workspace is not instantaneous — recording afterwards let all of them through and opened a tab each. And a resume ATTEMPT does not release the slot, because a resume can fail authentication again; releasing on the attempt opened another tab per retry. The offer is released when a mirror actually reaches .connected, which also closes the login workspace and resumes the host's other parked connections. Leaving the pane open meant a flapping host collected one tab per flap; not resuming the siblings left them frozen with their waiter already gone. A login workspace is excluded from session restore. A restored terminal is a fresh shell, so a restored login cannot authenticate anything, and it is invisible to the per-host rule — which is how login tabs accumulated across relaunches. Nothing is stranded when the login does not go to plan. If the workspace cannot be created the connection goes back to retrying rather than parking with its retry cancelled. Closing the login without signing in also returns it to retrying, so a later failure offers a fresh one. Both are derived by looking for that workspace rather than by trusting a close path to clear a flag. The wait for the user has no deadline. An MFA push can take seconds or many minutes, and a wait that expired while the login terminal was still open would strand the mirror parked with retrying already stopped. It ends on state instead: the master coming up, the host having no mirror left, or the login being closed unfinished.
The login waiter ran a `Task.sleep` loop, probing `ssh -O check` on a backoff that grew to 15 seconds. Its own doc comment claimed "there is no event to subscribe to", and that claim was wrong twice over. Completing the login is the user opening the shared master, and opening it creates the socket at the host's ControlPath — so the filesystem already reports the moment being waited for. `FileWatcher` watches the parent directory as well as the path, which is what makes creation visible for a socket that does not exist yet. `isMasterLive()` still runs, once per event rather than on a clock, to tell a live socket from a stale file; a negative answer keeps waiting, because other hosts' masters share that directory. An interval here was never a cost/benefit tradeoff: it is dead time a frozen mirror spends *after* the user has already finished, and no interval is short enough to make that right. An event-driven wait has to handle the condition already being true, so the master is checked once before the watcher starts. Otherwise a user who finishes signing in between the failure that parked the connection and the waiter starting would wait forever for an edge that already happened. Dismissal needed its own edge and had been relying on the poll without saying so: a closed tab is neither a filesystem nor a protocol event. `closeWorkspace` already tells the controller when a mirror closes, so a login closing now reports the same way, and the handler declines that offer and resumes quietly. Removing the poll would otherwise have silently dropped the behavior. The waiter task is also held per host now rather than fire-and-forget, so it can be cancelled — on a successful connect, on teardown, and when replaced — instead of probing for an offer that no longer exists. Tests cover the lookup the dismissal edge depends on, that a claimed-but-unopened offer matches no workspace (the lookup runs on every workspace close, so a false match would decline an unrelated host), and that a stale decline cannot silence a newer offer. Also: the harness wrote to a fixed /tmp path, which anyone on the box can pre-create or symlink, so the stderr it reads back to explain a failure was not necessarily its own. It uses a private mktemp dir removed on exit, which also makes two concurrent runs safe.
Three faults, all from an adversarial pass over the edge-driven waiter. A cancelled waiter erased its replacement. Re-arming cancels the old task and stores the new one, and the old task's `defer` then ran and cleared the dictionary entry it no longer owned — leaving a live waiter invisible to both dismissal and teardown, which is worse than the stale waiter the re-arm was fixing. A waiter now only retracts its own registration. Closing a window bypassed the decline. The login cmux opens can be in a different window from the mirror it exists for, and the window-close path forwarded only mirror cleanup, so closing the login's window left that host parked with retrying stopped and no waiter: the mirror in the other window stayed frozen until cmux restarted. Window close now declines every login it takes with it. (Group deletion needed nothing — it delegates to `closeWorkspace`, which already reports.) The socket watch can silently never fire. `FileWatcher` returns nil from its `open(O_EVTONLY)` and continues, so under fd exhaustion the stream has no sources attached and cannot yield — the login would complete with nothing listening. The edge stays primary and a slow re-check sits behind it, logging a warning when it is the one that finds the master: if the backstop is what resumes a mirror then the watch is broken, and a shorter interval would only hide that.
… it be closed Two review findings, both about a login outliving its purpose. A login exists to unfreeze mirrors, so when the last mirror for a host goes away — closed, detached, or taken with its window — the offer, its waiter and the sign-in pane are all orphaned: the waiter keeps watching a socket nobody is waiting on and the pane sits there with nothing left to resume. One helper now releases the offer, and every path that removes a mirror calls it, because which path ran is not something the offer should have to know. And the login could not always be closed. `closeWorkspace` refuses to close the last tab in a window, so a login that ended up alone in its own window stayed on screen after its offer cleared. cmux opened that window, so cmux has to be able to remove it: when the login *is* the window, the window is discarded instead.
- The 30s backstop is gone. It existed because `FileWatcher` returns nil from a failed `open(O_EVTONLY)` and carries on, so a watch that never attached could never yield and the wait would hang. Rather than put a timer behind the edge, the watcher now reports whether it is actually watching, and a failed watch hands the host back to the connection's own bounded backoff. That also drops the last `Task.sleep` from this file, which swift-blocking-runtime.md bans in shipped runtime code. - `host.destination` no longer goes to the log at `.public`. It can carry a username or an internal host, and this file was the only place in Sources logging it unredacted; `connectionHash` identifies the endpoint just as well for diagnostics. The user-facing workspace title and the error payload keep the real destination. - Release the login offer before closing its workspace. `TabManager.closeWorkspace` reports every login workspace to `noteLoginWorkspaceClosed`, which cannot tell cmux's own close from the user's, so closing first took the decline path on a successful reconnect and logged a dismissal that never happened. `FileWatcherTests.watcherReportsAFailedAncestorWatch` pins the new property against a directory it cannot open, with the positive case beside it. Verified the assertion can fail: with the property stubbed to `true` the negative test goes red, and both pass once it reads `directorySource`.
…snapshot The doc comments still described the timer that is gone: one promised an interval that backs off per probe, and a duplicated pair called the waiter a poll loop. Left in place they regenerate the impression that this path polls, which is the thing the review flagged. `Array(authWaitTasks.keys)` is belt-and-braces rather than a fix. `.keys` already yields an independent value that retains the storage, so the mutation inside `cancelAuthWait` triggers copy-on-write and the loop walks a snapshot. Copying explicitly says so at the call site.
…pported locale The new keys shipped with en and ja only, while every sibling remoteTmux.* key carries all twenty, and full-internationalization.md treats a partially localized new key as a blocking gap. Filled in the remaining eighteen, following each locale's existing register in this catalog (formal Sie for de, vous for fr, informal for es and it). Inserted as locale blocks rather than by rewriting the file, so the diff is the 54 new entries instead of a reformat of all 4163 keys.
…onds Both checks slept out the worst case (45s and 40s) before asking whether a login had reappeared. The thing they are waiting for is a reconnect attempt reaching the loopback sshd, and that failed attempt IS the auth-required report — the exact moment the old behavior reopened a tab. So wait on that edge instead, with a short settle for the app's turn between the ssh exit and a workspace appearing. The straggler check gains something it did not have: if no retry ever reaches the sshd it now fails, saying the case was never exercised, rather than passing because nothing happened.
…ormatter Review asked for String.localizedStringWithFormat on the parameterized title. String(format:) applies the fixed C conventions; the localized formatter picks up the locale the rest of the string was translated for. The three new keys already carry all 20 catalog locales.
The reconnect-auth path logged the whole V2CallResult at .public. On success that result wraps the created workspace fields, and on failure it carries an arbitrary data value, so both landed in the system log unredacted. Log the outcome instead: ok, or err with the error code, which is a fixed identifier rather than caller data and is the part worth having when a login-workspace create fails. This codebase already treats an ssh destination as sensitive, and a raw command result deserves the same handling.
main added onPaneSeed as a required parameter of RemoteTmuxConnectionObservers.add after this branch was written, and these two calls list every observer explicitly, so they no longer compile against it: cmuxTests/RemoteTmuxAuthTests.swift: missing argument for parameter 'onPaneSeed' in call nil matches what the surrounding tests want: they exercise the auth-required observer and have nothing to say about pane seeding.
… paths releaseLoginOfferIfHostHasNoMirrors ran only from handleWindowWorkspacesClosed and detachMirrorWorkspaceKeptOpenLocally. Closing the last mirror through tearDownMirrorAndCloseWorkspace or handleWorkspaceClosed left the offer claimed, so the next attempt that needed a sign-in tab saw the host already offered and parked without presenting one.
tmux reports a socket it cannot open as `error connecting to /tmp/tmux-501/default (Permission denied)`. The auth classifier looked for that phrase anywhere in stderr, and listSessions asks about auth before it asks about the server, so a socket cmux has no permission to open offered the user an interactive login instead. Logging in again changes nothing about the socket's mode. The phrases are matched per line now, and a tmux socket-connect line never counts. Multi-line stderr still classifies on any other line, which is what keeps a stale ssh-agent warning from masking a real Permission denied.
…able and proven The choice of how a control stream reaches a host is currently spelled out at the point of use: `Process` is handed `ssh` plus `host.controlModeArguments(…)` directly. A transport that survives a network change cannot be added without first naming that decision. ssh stays the default with unchanged argv, so behavior does not move. `RemoteTmuxTransportProfile` decides what to run — binary, control-stream argv, one-shot argv — and deliberately not how to run it: `RemoteTmuxSSHTransport` keeps process spawning, the shared ControlMaster, and stderr classification, so a second transport inherits all of it. One-shot commands keep riding ssh's master even when the `-CC` stream does not. The transport is a property of the host, not a global switch, because one host can be reachable over a session-preserving transport while another is plain ssh: `cmux ssh-tmux --transport et <host>` validates against a closed set in the CLI, rides the socket as `transport`, is re-validated at the socket boundary so an unknown value is refused rather than producing an unspawnable host, and lands on `RemoteTmuxHost`. A host's `port` means the port of ITS transport: for et that is etserver's 2022, not sshd's 22. Everything below was measured against et 6.2.11 on a loopback etserver (`scripts/remote-tmux-et-host.sh`), because none of it is visible in `et --help`. ET needs a controlling terminal. On pipes it writes nothing at all — not even for a trivial `echo` — and aborts at session end, which reads as "the host produced no output" rather than "this was spawned without a tty". cmux spawns pipes, so this, not argv, is what decides whether a transport can start. Hence `requiresPseudoTerminal` and `RemoteTmuxPseudoTerminal`, which wraps the invocation in the system's pty allocator. ET types the command into a login shell rather than exec'ing it, appending `; exit`. A real `tmux -CC` stream therefore arrives after ~1.2 KB of preamble — the echoed command, then prompt escapes — and the pty delivers CRLF. The parser already survives both (unrecognized lines yield no messages, and it already strips a trailing CR), which is why the protocol works at all; a captured real stream is now a fixture so that cannot silently regress. Two pty consequences a control protocol must respect, both measured: the pty echoes what cmux writes, and those echoes are ignored rather than misread; and anything written before the stream is up is consumed by the transport's login shell, not by tmux. cmux already withholds commands until `%enter`, so that ordering is load-bearing rather than incidental. `reconnectsInternally` is the property that changes behavior rather than argv, and `RemoteTmuxStreamEndDisposition` states the consequence: for ssh, EOF means respawn; for a transport that owns recovery, EOF means the session is over, because it does not end for a network drop — observed directly, where the client attempts a reconnect first and only then reports the session gone. `RemoteTmuxPreConnectHook` covers hosts needing a step before connecting. It runs once per connection rather than once per command, because a single-use credential must not race itself, and a non-zero exit is not fatal so a broken hook cannot make a host unreachable. A seeded model-based fuzz suite covers the transport state the ssh-only world never had: process alive, stream flowing, session exists. Its central invariant is that an internally-reconnecting transport is never respawned for a stall or a resume — the property the whole design rests on and the easiest to regress. It also pins that a frame split across a resume never surfaces as a partial message, that a gone session is never read as an auth failure, and that the hook runs once per open however many callers race. `scripts/remote-tmux-et-e2e.sh` drives the running app against the loopback etserver and checks the two things only a live run can show: that cmux spawns an et-carried control stream, and that it does so under a pty. `-x` / `--kill-other-sessions` is never passed: it kills every session that user has on the host, not just stale ones.
The et transport could never carry `tmux -CC`. Two separate faults, both found by running the end-to-end harness against a real etserver rather than by reading argv. et does not exec its `--command`: it types it into a login shell and appends `; exit`. That shell reads from a pty in canonical mode, which delivers at most MAX_CANON (1024 on macOS) bytes per line, and the PATH resolver ssh needs is about 1113 bytes. The line never completed, so the shell ran nothing and the stream sat silent until the attach timed out with nothing to explain it. Measured: a 23-byte command exits 0, a 1123-byte one hangs. et gets plain `tmux` now, which is correct as well as short, because a login shell already has the user's full PATH. ssh keeps the resolver — a non-login shell with a minimal PATH is the case it exists for. With bytes flowing, the stream still never reached control mode. The parser recognised tmux's `ESC P 1000 p` only at the start of a line, and the login shell leaves its echo and OSC title sequences ahead of it with no newline between, so `.enter` never fired while every later notification parsed normally. Commands are withheld until `.enter`, so the mirror waited forever. The scan now finds the sequence anywhere in the line and drops the shell noise before it, scoped to before control mode is entered and outside a command block: block content is raw `capture-pane -e` bytes that can carry that DCS legitimately, and matching it there would cut the pane apart. Tests. The fixture test for the real et stream asserted only that `%session-changed` parsed, which is true on the broken code; it now asserts `.enter` is produced and precedes it. The argv test asserted the resolver was present, pinning the bug, and now pins its absence. New: a bound on the command against MAX_CANON including a long session name, injection-safe quoting of the session name now that the resolver no longer does it, mid-line enter recognition, and a DCS inside block content staying whole. The harness proved neither fault, so it grew the checks that would: it asserted a process listing, which both faults satisfied, and skipped the session it attaches by name. It now provisions that session on the server both transports reach — one-shot commands ride ssh while the stream rides et, so a private TMUX_TMPDIR hides it from the ssh side — refuses to adopt a session it did not create, and reads cmux's own view of the stream: the handshake parsed and windows arrived.
…lled one Two faults from review, both real and both invisible to the end-to-end run, which exercises one host over one transport that works. `connectionHash` was built from destination, ssh port, and identity file, and it keys the attach single-flight, the transport registry, and matching a mirror to a host. So an ssh host and an et host at one destination were the same endpoint, as were two et hosts on different etserver ports, and an attach could be handed a cached connection whose profile or port was wrong. The transport and its port are in the fingerprint now, appended only for a non-default transport so a plain ssh host keeps the hash it has today — it names the shared master's socket path and persisted mirror state, and moving it would orphan both. `probeLiveness` had no caller outside a test. A transport that reconnects internally never delivers the EOF that drives ssh recovery: during a network change its process stays up and the stream pauses, which is the behavior worth having but leaves a wedged transport looking exactly like an idle one. Nothing else in the lifecycle can tell those apart, so a wedged et connection stayed `.connected` forever and the mirror froze with no error and no retry. A probe now runs while such a transport is connected, starting on `.enter` and cancelled with the rest of the scheduled work, and a stall routes through the existing `beginReconnecting()` rather than a second reconnect path — the remote session is likely still there, it is the client that is wedged, so this recovers rather than ends. ssh is excluded and tested to stay excluded: it gets its EOF, and probing an idle ssh stream would add traffic and a failure mode where there is none. `processGeneration` is readable across the type's extensions so a probe answered after a respawn can tell its answer describes a stream that no longer exists. Without that check a late answer would tear down the healthy connection that replaced it. The harness needed the same lesson the attach did: its `remote.tmux.state` lookup omitted the transport, so once identity included it the lookup matched nothing and reported the stream as never having reached control mode. Both e2e regressions today came from a call that under-specified the endpoint.
…ew polling The liveness monitor could not detect the stall it exists for. ET can accept stdin while producing no control output, and the probe had no response deadline, so probes were written, never answered, accumulated, and the connection sat `.connected` forever. The deadline is now the next probe's due time rather than a second clock: probe N must be answered before N+1 is due, which is a generous bound on a local round-trip. A respawn clears the flag so a stale outstanding probe cannot condemn a fresh stream. `connectionHash` gains the case its tests were missing — `.ssh` with a transport port, which the discriminator admits and nothing else covered. It must not alias the plain ssh host, the ssh host whose *ssh* port matches, or the et host on the same transport port. Also adds a guard against the class of bug this work kept producing. A reviewer found a `Task.sleep` backoff in the login waiter whose own comment claimed "there is no event to subscribe to"; the event was the ControlMaster socket being created and `FileWatcher` had been in the tree the whole time. Nothing failed when that shipped, so nothing would fail next time. `scripts/lint-remote-tmux-no-polling.sh` fails on a new time-based wait in remote-tmux sources and names the edges this codebase actually has. It blesses nothing: six pre-existing waits are recorded in a baseline, matched by file and symbol so unrelated edits do not trip it, and the sizing debounces there are worth revisiting since sizing is supposed to converge on tmux's ordered acknowledgements rather than a clock. Two exceptions carry the reason no edge exists.
Measured against et 6.2.11+7: restarting only `etserver` ends the control stream while `tmux has-session` still succeeds. cmux read that as the session being over and removed the mirror, throwing away a session that was alive and reattachable. The rule it came from sounded right — a transport that reconnects internally does not end for a network drop, so its exit must mean the session ended — but EOF cannot distinguish "the transport died" from "the session died" for any transport. So it no longer tries: end-of-stream reconnects, and the reattach reports whether the session is gone, which is an answer rather than an inference. `forStreamEnd` takes no argument now; a parameter that no longer decides anything would only invite the same inference back. The model-based fuzz test caught this honestly. Its invariants encoded the old premise — an exit ends the session, and a stall never respawns — and both are now false: a stall respawns because a wedged transport will not recover itself. Rewritten to the properties that survive the change: a stall and an exit never *end* a connection, only a session found gone does, and a connection is never left both ended and alive.
… comments The et profile always sent the path et --macserver sends, because nothing called the probe or set a per-host path. Finding the path per host would cost an extra ssh connection before every attach, which a host that asks for a tap on every connection cannot afford, so the probe, its candidate list and the unused RemoteTmuxHost field are removed and the comments say what the code does. Also corrects comments that described the old end-of-stream rule, quoted the et line limit two different ways, and named a remoteTmux.linkedView beta that does not exist.
…ry it A real prompt has no trailing newline, so the check only ran when a full line arrived and the attach waited out its 15 s deadline. The tail is now checked after every chunk. On a first attach a prompt resolves the wait at once, and a transport that reconnects by itself ends the attempt instead of retrying, since each retry is a new connection that prompts again. Reconnects after control mode behave as before.
A link now leaves for the system browser only while a real input event is in flight, and the socket browser.click runs JavaScript, so the matched and unmatched link cases click through accessibility the way a person does. The scripted popup and form cases keep the socket click, since a scripted action is what they test.
RemoteHostColorRegistry maps each remote host destination to a stable color slot in the workspace palette: a stable FNV-1a hash picks the starting slot, collisions linear-probe to the next free slot, and the slot is cached per host so a server keeps one color everywhere for the life of the session. Pure and @mainactor; unit-tested for the hash, probe, exhaustion, and empty-palette behavior. Not wired into UI yet.
New remoteTmux.originColors.beta.enabled flag (off by default) in the Beta Features catalog, with a "Remote host colors" toggle in Settings → Beta Features and en/ja localization. Gates the per-host coloring; while off, remote workspaces render with no origin tint.
When the flag is on, a remote workspace's sidebar row and tab take its host's color (manual workspace color still wins). The color is resolved above the LazyVStack row boundary and passed into TabItemView as a plain value, and it's folded into the row snapshot's presentation key so toggling the flag — or a mirror host resolving after the row first appears — repaints immediately instead of showing a stale cached color. RemoteTmuxController.hostDestination(forWorkspaceId:) resolves a mirror workspace's host (mirror workspaces carry it only through the session mirror, not remoteConfiguration). A refresh-policy test pins that the effective color is part of the presentation key.
Every other Beta Features toggle has a curated search entry; without one the new toggle can't be found or scrolled to from Settings search. Add the entry and list its explicit anchor in the resolution test.
…t singleton) RemoteHostColorRegistry was a `static let shared` ambient global. Move ownership onto RemoteTmuxController (itself owned by AppDelegate at the app seam) and reach it through `AppDelegate.shared?.remoteTmuxController.hostColorRegistry` — the same seam the origin-color resolver already uses for `hostDestination`. No behavior change; assignments stay consistent process-wide without a second global owner. The registry's own tests already construct their own instances, so they're unaffected.
The AppKit workspace-row-cell test (added on main after this feature branch was cut) builds a SidebarWorkspaceSnapshot and a presentationKey. Origin colors added `customColorHex` to `presentationKey(...)` and `hasManualCustomColor` to the Snapshot, both required, so on current main the test no longer compiles. Pass the defaults (nil / false) so the cmuxTests target builds.
- Drop `RemoteHostColorRegistry.reset()`. It was a test-only seam in production source with no callers; the registry is constructable, so tests make their own. - Resolve origin colors from a destinations map walked once per sidebar refresh instead of re-scanning the session mirrors for every row, which made a batch refresh O(N²). The map is derived fresh each pass and carried on the render context (the same shape the context already uses for its other row lookups); it is not cached, because `mirroredWorkspaceId` follows a weak workspace reference and an index kept next to `sessionMirrors` would survive a workspace close without the dictionary changing. - Compare the stable hash through locals in the determinism test so SwiftLint stops flagging identical operands.
Review flagged the else branch in originColorHex as dead, and it is. Every caller resolves mirrorDestinations from mirrorDestinationsForOriginColors(), which returns nil only when remoteTmuxOriginColorsEnabled is false — and originColorHex guards on that same flag before it ever reads the map. The three makeWorkspaceSnapshot call sites and the direct row lookup all pass that value, so by the time the branch could run, the map is never nil. The docstring claimed a single row passes nil and takes a direct hostDestination(forWorkspaceId:) lookup. No caller does that. Removed the branch and rewrote the comment to describe what actually happens.
main added SidebarWorkspaceRowSuspensionTests after this branch last built; its fixture calls presentationKey and Snapshot without the origin-color fields this branch adds. nil/false keeps the fixture's behavior: no manual color and no origin tint.
…shares (beta) Connecting two servers that each have a tmux session named main gives two sidebar rows that both read "main", and nothing on the row says which server is which. With the new "Remote host names on duplicate titles" beta on, a row whose title another workspace shares shows its remote host after the title, for example "main · web1.us-east" beside "main · web2.eu-west". A group of same-titled workspaces gets hosts only when its members come from more than one place: two hosts, or a host and this Mac. The host drops any user@ and the trailing domain labels every host in the group shares, but never its first label, and the row truncates the end, so as much of the host shows as fits. IP addresses keep every label, a remote workspace beside a local one with the same title shows its whole host, and managed Cloud VMs, which share one gateway destination, show their VM id. Whether a title collides depends on every other title, so ContentView resolves the hosts once over the whole workspace list and hands each row its host as a plain value, like the origin color. The host is part of the row's presentation key, the context-menu refresh carries it with the title, and a single-workspace refresh also rebuilds any other row whose saved host no longer matches, since a rename can start or end a collision. Only the displayed title changes: renaming, checklist titles and the tmux session name keep using the real title.
Two servers showed rails in nearly the same red. The registry hashes a host into the workspace tab palette, and once the rail brightens those colors Magenta and Rose differ by 2.4 in OKLab (x100), with Red and Crimson close behind. hostColorsStayDistinguishableOnTheSidebarRail reads the color each slot resolves to through colorHex(for:), brightens it the way the rail draws it, and requires every pair to differ by at least 14, palette neighbors by at least 25 (a host bumped by a collision takes the next slot), and every color to differ from the selected row's blue by at least 12.
…shable Two servers drew nearly the same red rail. The registry hashed a host into the workspace tab palette, whose dark reds and magentas collapse into each other once the rail brightens them: Magenta and Rose differ by 2.4 in OKLab (x100), Red and Crimson by 4.6. Host colors now come from sixteen colors picked for this job. As the rail draws them, every pair differs by at least 14, colors that are neighbors in palette order differ by at least 29, and each differs from the selected row's blue by at least 12. A host still hashes to a start slot and a collision still takes the next free slot, so a bumped host always lands on a clearly different color. Host colors no longer follow edits to the workspace tab palette.
The sixteen host colors were bright enough to compete with the row titles. They are now soft tones, with the violet as the reference for how saturated a color may be. As the rail draws them, every pair differs by at least 11 in OKLab (x100), palette neighbors by at least 26, each color differs from the grey sidebar by at least 15, and from the selected row's blue by at least 12. The test gains the grey-sidebar floor and a lower pairwise floor to match.
Main moved row snapshots into SidebarRowSnapshotCache, built outside rendering, and rows now only read from it. The origin color and the host after a colliding title differ per row, so the cache's reconcile takes a per-row presentation key: a row whose color or host suffix changed is rebuilt, and every other row keeps its snapshot. A targeted refresh also rebuilds rows whose host suffix went stale because another workspace was renamed. The render context no longer carries mirror destinations or host suffixes, since nothing reads them there.
…key changes Main replaced the cmuxAccentNSColor function with CmuxAccentColor, so the palette test measures the rail colors against CmuxAccentColor(mode: .cmux), the colors the old function returned. The presentation key now carries each row's effective color, so the test main added for the compact-status setting passes nil for it.
…t does The setting's description promised the row and the tab, but the origin color only reaches the sidebar row snapshot.
# Conflicts: # Sources/RemoteTmuxController.swift
# Conflicts: # Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swift # Resources/Localizable.xcstrings # Sources/AppDelegate.swift # Sources/RemoteTmuxConnectionState.swift # Sources/RemoteTmuxControlConnection.swift # Sources/RemoteTmuxController+Attach.swift # Sources/RemoteTmuxController+Decisions.swift # Sources/RemoteTmuxController.swift # cmuxTests/RemoteTmuxAuthTests.swift # cmuxTests/RemoteTmuxNewWorkspaceHostRoutingTests.swift
ejc3
force-pushed
the
all-ej-changes
branch
from
September 29, 2026 22:05
357e8c0 to
ca967d7
Compare
Collaborator
|
This is a validation roll-up explicitly marked not for merging; the individual PRs are the reviewable units. Closing the roll-up. |
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
2 participants
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.
Every open change of mine, merged onto
mainat211b8bb35fso the combination can be built and run as one thing. Not for merging. The individual PRs are the reviewable units, and this exists because green in isolation is not the same as green together.mainhas moved 12 commits since, to8c7ae55e03.Merged here (29).
xcodebuild build-for-testingsucceeds on this tree. The full test suite has not been run on it yet.#8428 and #8556 are not merged on their own. #8721 was built on both and already carried most of their commits, and the ones it lacked are now cherry-picked onto its branch, so merging #8721 brings in all of it. Merging either branch again would repeat the same work.
Conflicts
#8629 and #8833 both grade the app-host unit-test step. They merged without a textual conflict but disagreed on one case. When a batch's host died but its last launch printed "(0 unexpected)", #8629's per-batch check continued it and #8833's shard check failed it.
classify-app-host-test-result.shon #8629's branch now fails a batch with unreadable output, a host-restart line, or no executed tests before it reads the status, and the step grades every batch with it. #8833's branch now stubs the crash-report scan in the step's test harness, which never supplied it.#7193 and #7214 add functions at the same place in
RemoteTmuxController.swift. They share no state, so both are kept.Merging #8721 touched 14 files. For the files only #8555 had changed, #8721's copies contain #8555's head and are taken whole.
RemoteTmuxController.swiftkeeps one copy of each New Workspace member. The routing is #8721's, which also handles a multiplexed host, but it reads the host through #7214's helper and revalidates against registered main windows the way #7214 does.AppDelegate.swiftkeeps #7214'sforceLocalescape hatch. The beta catalog keeps both flags, andRemoteTmuxWindowMirror+Configuration.swiftkeeps #11248'snonisolated. The no-polling lint is #11264's script with #8721's allowlist of deadline arms, and its baseline was regenerated from the merged sources.The no-polling lint passes (13 documented, 5 baselined), and so does its self-test. The localization parity check, the strings lint, the pbxproj test wiring lint and
tests/test_ci_change_areas.pypass as well.