Sixteen `#[allow(clippy::…)]` in the tree; thirteen carry a comment saying
why. These were the three that did not, and the reason turned out to differ
for each.
`spawn_once` takes eight arguments against a threshold of nine, so its
`too_many_arguments` allowed nothing at all -- removed. `render_ssh_row` does
trip it, at ten with `self`, and `LocalHost::new` really does hand back a
`SharedHost` rather than a `Self`; both now say so.
Removing all three first and reading what came back is what separated them,
and it needed two passes: clippy stops at the first crate that fails, so
tty7-core's error hid settings.rs's until it was fixed. A single clean run is
not evidence about anything downstream of the first failure.
Last of the three item kinds. 48 structs, enums and traits were `pub` with
no name outside tty7-core, and a `pub` type hides more than a function does:
its fields and variants are invisible to the lint too.
Nine more items surfaced, and the same split as before -- most are used only
by tests and take the house idiom; three are unreached here and carry the
reason instead. `absorb` and its neighbours `len`/`is_empty` on
GitignoreChain are simply spare: the matcher sets are built whole rather than
merged.
Most of the work was the compiler correcting me, and it corrected two things
a search could not:
- A type can be *used* without its name ever appearing -- through inference,
a method's return, a chain. `AgentEvent`, `ManagedWorktree`,
`ProfileUsage`, `RouteAction`, `WorkingDirectory` and `KiPrompt` are all
named nowhere outside this crate and all needed by it.
- Reachability is not just signatures. `AgentEventKind` sits behind a public
*field*, `Duplex` and `PaneDirectory` behind public bounds, and `Halves`
behind an associated type. Those are the `private_interfaces` lints, and
only `cargo clippy` reports them -- `cargo build` was clean while five of
them stood.
So the rule for anyone repeating this: narrow, then let the compiler put back
what it must, and read clippy rather than build.
The same blindness as the last commit, one item kind over: 139 constants and
type aliases were `pub` with no name outside tty7-core, and a `pub` constant
is as invisible to the dead_code lint as a `pub` function. Most are the frame
kinds and asset names the protocol and installer keep to themselves.
It turned up one: DEFAULT_REMOTE_SERVER_CMD, which only the router's tests
still name -- the production path builds the command from the install it
found. Kept, with the house idiom and a note saying what it now is: the
statement of that default, and tests that hold it to it.
`SharedConnection` stays `pub`; ssh::mod re-exports it, and the compiler says
so plainly.
tty7-core is `publish = false` and has exactly three consumers, all in this
workspace. 199 of its functions were `pub` without a caller outside it, and
that is not merely untidy: **rustc never reports a `pub` item as dead**, since
it cannot know what an external crate uses. Every one of them was a place the
lint could not look.
Narrowing them turned the lint on and it found things immediately -- 25 items
that had been invisible:
- Most are used only by tests, and take the house
#[cfg_attr(not(test), allow(dead_code))].
- Seven are unreached on this platform and are now annotated with the
reason. Two of those are WSL helpers a #[cfg(windows)] test does use, so
they read as dead here and are very much alive there -- exactly why none
of them were deleted on a macOS build.
Left `pub`: `host::conformance`, whose functions the
`host_conformance_suite!` macro expands into other crates' test binaries. The
compiler caught that one; a search for the names could not, because the only
mention is inside the macro in this crate.
The seven unreached ones are now visible and worth a decision. Some are
plainly spare (`connect_string` wraps `to_connect_string`, which callers use
directly); others are one half of a pair whose other half is used, where
deleting one is worse than keeping both.
The field was listed in `run`'s JSON and nowhere explained, on the surface
agents parse. It says whether the server managed to read a status before it
stopped waiting; when it did not, `run` exits 1 as a stand-in and says so on
stderr, and the flag is the only way to tell that 1 from a real one.
A command killed by a signal also comes back as `exit: 1` -- not the 128+N a
shell reports -- and with `exit_code_known: true`, because a status really was
read. The two are byte-identical in the JSON, so an agent cannot tell a
command the OOM killer took from one that exited 1 by itself, and it should
not be left to find that out the hard way.
That last part is a limitation rather than a choice: the pty crate keeps the
signal in a private field with no accessor, and hands back a placeholder code
of 1 in its place. `success()` cannot separate the two either, since it is
false for both. Reporting the real 128+N needs the child reaped directly --
which the daemon already does for adopted panes, where it gets this right.
Found by running the commands, not by reading them.
Three verbs, one condition, three sentences:
procs %99 no pane %99 on this machine — `tty7 pane ls --all` lists them
capture %99 observing pane %99: daemon refused Observe: no such pane 99
send %99 hi sending input to pane %99: no such pane 99
The daemon's own refusal makes a fine diagnostic and a poor sentence: it
names the wire request rather than the verb that was typed, so `capture`
told the user about "Observe", a word that appears nowhere else they can
see. This is the CLI agents read stderr from, and it was reporting one state
of the world three ways.
They now all answer with the line `procs` already used. The registry is
consulted only once something has failed, so the ordinary path still costs a
single round trip, and a failure with the pane present keeps its own words --
this must not swallow a connection error and call it a missing pane.
`resolve::no_such_pane` is now that sentence's one definition; the copy in
`workspace_of_pane` had drifted to suggesting plain `pane ls`, which does not
list the orphaned pane most likely to be asked about.
Found by running the verbs against a live daemon rather than by reading:
every one of these paths looks right in isolation.
The window drops a view whenever its layout is rewritten -- a delta arriving
for a tab it had built differently, a rebuild from the tree -- and dropping a
view does not end the shell behind it, because that is also what detaching
is. So every reshape left panes running that nothing could reach again. This
is where most of them came from: a 150-op fuzz against an open window shows
31 layout rewrites to 2 rebuilds and 12 refused ops.
Panes put down this way are *parked*, not ended, and judged later. At the
moment of the drop neither mirror can say whether a pane is still somebody's:
the machine copy is missing whatever this window has just done, since a
client is left out of the deltas its own ops raise (#612), and this
workspace's copy runs ahead of or behind ops still in flight. Judging there
does end live panes -- both a machine-mirror check and a two-mirror check
were tried, and the fuzz caught each of them killing panes the tree still
named. The parked set is swept only against a tree the window has just
pulled, which is the one account of the machine here not assembled from
deltas, and only for panes the window is also not showing. Anything that
came back into the tree or onto the screen meanwhile is forgotten rather
than ended, because that is exactly what the mirrors were wrong about.
Over six fuzz seeds: no run ends a pane the tree still names, and where a
pull lands during the run the stranded shells go from 15-26 to 0-2. A run
with no pull sweeps nothing and leaks as before -- the parking is correct,
the coverage is only as good as how often the window pulls -- and
`tty7 pane close --orphans` still reclaims whatever is left.
TabClose and PaneClose answer with the panes the tree let go of, and the
shells behind them keep running until somebody hangs them up. The daemon
will not: the same removal is how a pane crosses to another tab, so it
cannot tell an ending from a move, and whoever sent the op has to say.
`tty7 pane close` always has -- hang_up_removed_panes exists for exactly
this. The window did it only where a *person* closed something, and never
for the identical ops its own reconciliation raises, because `pump` sent the
batch with `if let Err(e) = client.call(op)` and dropped the Ok on the
floor. Every pane the window put down while squaring its layout with the
tree left a live shell nothing could reach.
Measured with a 150-op tree fuzz driving the CLI against an open window:
5-6 shells per run were being stranded this way, and are not any more.
It is not the whole leak -- around 14 orphans per run survive, from the
paths ITER 13 named (a superseded hydration, a delta that would not apply),
which strand panes without sending an op at all.
A pane the window is showing is never hung up, whatever the tree says. The
daemon computes its answer with collect_orphan_panes across the whole
machine *after* the change, so a pane that merely moved is never named --
but a pane this window has spawned and registered and not yet placed in the
tree is, because at that instant nothing in the tree holds it, and its
TabCreate may still be queued behind the op that produced the answer.
Ending that one takes down a pane the user is looking at (#628). If the
window cannot be read at all, nothing is hung up: a leaked shell is
recoverable with `tty7 pane close --orphans`, a shell ended under a live
window is not.
The check runs on the main thread after the batch returns rather than
beside the calls, because `pump` may be running inside an App that already
holds the window entity, and gpui answers that by silently dropping the
update -- which stops the window syncing at all.
gpui picks a prompt button's role by matching the English word -- "cancel"
becomes a cancel answer, anything else becomes a plain Other that answers
neither Escape nor Return. `confirm_answers` exists to say which is which
instead, and sixteen of the app's dialogs use it.
The two SCM confirmations built their answers by hand from localized
strings, so `t(Cancel)` came out as a real cancel answer in English and as
an Other in Chinese and Japanese: discard, reset and amend -- the dialogs
guarding the operations that lose work -- had nothing on Escape for those
users. They also passed Cancel as answer 0, which gpui draws rightmost and
gives Return, so the position that means "do it" everywhere else in the app
meant "don't" here.
Both now go through the helper, with the index check flipped to Ok(0) to
match. The single-button restart-failed dialog had the same problem from the
other side: t(Ok) is "确定" in Chinese, so its only button was not an Ok
button either.
Add a test for the underlying rule rather than the three call sites: it
pins that a localized cancel label does not become a cancel answer on its
own, and says so in the failure message if gpui ever starts reading roles
another way.
app.rs's server-mismatch prompt still builds its answers by hand, and stays
that way -- it is documented as having no safe answer to give Escape to.
confirm_question returned the hard-reset question for GitOp::Reset in any
mode, and that text ends "and uncommitted changes are discarded" -- true of
--hard, untrue of --soft and --mixed, which both keep the worktree.
It is right today only because GitOp::destructive classifies soft and mixed
as harmless, so they never reach a confirmation at all. That invariant lives
in another crate and nothing here said so, leaving a reader to go and check
before they could tell whether a mixed reset shows a warning about losing
work it does not lose.
No behaviour change -- the arm is unreachable for the other two modes either
way. It now states its own precondition instead of borrowing one.
confirm_verb keeps matching every mode on purpose, with a note saying why:
"Reset" is the honest button label whichever mode it is.
The codebase already decided a secret must not reach a formatter --
NativeSshSpec spells its Debug out by hand, AuthResponse prints
Secret(<redacted>), and secrets_are_redacted_in_debug_output pins both.
Three types carrying secrets were missed and still derived Debug.
- KeychainWrite::SetPassword/SetKeyPassphrase hold the plaintext the user
just typed. password_submit returns one of these *beside* an
AuthResponse built from the same String, so the identical secret was
redacted in one return value and printable in the other.
- Attachment::token is the proof a connection holds a workspace. Its own
doc says it "goes over no wire and onto no disk" and #[serde(skip)]
enforces that -- but the log file is disk too, and the token is reached
by the derived Debug of Workspace, Machine, and the whole tree.
- ControlHello::client_token is the same kind of proof, in the one module
that already logs protocol values with {event:?}.
None of the three is formatted anywhere today, so nothing was leaking; this
closes the gap while that is still true, since a single {:?} added later
would have written a password or a capability token to the log.
Each gets a test in the shape of the existing one. Verified non-vacuous:
putting the derive back on Attachment fails with
token leaked: Attachment { token: "s3cr3t-capability", ... }
Five modules carried #[allow(dead_code)] on the `mod` line itself, which
silences the lint for everything inside them -- for good. host_ops and
host_registry are live infrastructure (73 and 22 references between them);
one unused item each had switched the lint off for the whole file, so
anything that died there later would never be reported.
Two of the five were covering nothing at all: tty7-core's core::keychain
and core::ssh_profile have no dead items.
What the allows were actually hiding, and what happened to it:
- HostOps::run_or_notify, HostRegistry::local and HostRegistry::len are
unused everywhere, tests included -- deleted. run_or_notify was only
run_in composed with notify_err, both of which stay.
- InFlight::len and ByHost::len are used, but only by tests, so they take
the house #[cfg_attr(not(test), allow(dead_code))].
- status_rank is used only by the test that guards it against DecoStatus's
Ord, which its doc comment already explains. Same treatment.
The two `len`s are why this is worth doing carefully: --all-targets still
reports them as never used, because the warning comes from the non-test
build of the binary. Deleting on that evidence broke the test build. The
compiler, not the lint, settled which ones were really dead.
Each of these sits on an item the code does use: ssh_config's
import_profiles, merge_imported and jump_alias, SshConnection's key field
and key(), NativeSshSpec::without_secrets, CmdEditor::cursor, and
Pane::ssh_connection. Verified by removing every dead_code allow in the
workspace and reading back what the compiler then reported -- none of
these appeared, in either a production-only or an all-targets build.
A stale allow is worse than no allow. It reads as "this is dead, on
purpose", so the next person leaves it alone, and it goes on covering the
item after the code around it changes -- at which point the item really can
die without anyone hearing about it.
cursor_byte is the one that is genuinely unused outside tests, so it moves
to the house idiom #[cfg_attr(not(test), allow(dead_code))], which still
reports it if the tests stop using it too.
"the server answered a machine tree with {reply}" reads as though a machine
tree was the thing answered. It is the request that was for the machine
tree, and the reply is whatever came back instead.
The Chinese translator had already worked this out and supplied the missing
word -- "回复了机器树请求" says "replied to the machine-tree request" -- so
zh needed no change. English and Japanese carried the ambiguity.
"daemon" is the code's word for it -- the --daemon flag, daemon/server.rs,
the RemoteDaemon* keys -- and the UI deliberately uses "server": the
dialogs say "Restart Server?" and "Quit and Stop Server?", the settings
section is headed "Server", and the errors either side of the one fixed
here say "tty7's local server".
RemoteDaemonTooOld broke that in all three languages. It called the local
process a daemon while naming the far-end one a server in the same
sentence, so the two read as different components and the instruction
"Quit tty7 (which stops the daemon)" sent the user looking for something
the rest of the app never names. It now says "tty7's local server",
matching the two keys directly above it.
The ja settings heading said "デーモンサーバー" where en and zh both say
just "Server" -- mod.rs already documents that this heading is meant to be
that word on its own.
Add a test: no user-facing string may call it a daemon in any of the three
languages. The search-keyword lists are skipped, since they carry the word
on purpose to match what a user might type.
tty7 calls the split regions inside a tab panes and the docked regions
around them panels, and Chinese has a separate word for each -- 窗格 and
面板. Eleven strings across four keys used 面板 for a pane, so they named
a different part of the window rather than reading as a loose synonym.
The settings page showed both senses a few rows apart: "text size ...
tabs, panels and settings" is genuinely 面板, while "give each pane its
own shell history" right below it claimed panels have their own shell
history -- they have no shell at all. The proxy description, the stale
daemon notice and the restart-server body had the same swap.
Japanese was already consistent (ペイン / パネル), and the English is the
source, so only zh.rs changes.
Add a test over every key: when the English names one sense and not the
other, the translation may not carry the wrong word. It matches whole
words, since "panel" contains "pane".
The previous commit documented the `events` schema and listed `agent_status` as
one of the kinds a listener sees. It is not: `ControlEvent::AgentStatus` has the
same shape of problem as `PaneExited`, which is how it slipped through — the
only two references anywhere are the round-trip test in its own file and the
formatter in `tty7-cli`, with no emitter.
Auditing all six protocol enums is what turned it up. `ControlRequest` (42
variants), `ClientMsg` (30) and `DaemonMsg` (28) are clean; every `LayoutDelta`
and every other `ControlEvent` has a real emitter — `Preempted` and
`LayoutResync` in `host::server`, `GuiOpen` beside them — which is exactly what
made these two look supported.
Agent status is reported the same way a pane exit is: a `layout` delta whose
`pane_facts` carries the pane, with `agent.status` on it (`AgentFacts.status`
rides on `PaneRecord`). The note now covers both, and points at `tty7 wait` and
`tty7 agents` as the supported way to ask about an agent, since neither is
built on this stream.
Both variants now carry the comment; the recommendation is unchanged — emit
them or remove them together with their `event_line` arms, but not in passing,
because it is a dialect change.
2951 tests pass.
`events` is the streaming interface an agent builds on, and it was the one verb
whose JSON shape the reference did not give — every other one lists its fields
exactly. Now it does: the externally tagged envelope, the four event kinds a
listener sees, and the thirteen `layout` delta kinds.
The prose also listed "pane exits" as an event type. There is no such event.
`ControlEvent::PaneExited` is defined, encodes, and has a line
`tty7 events` would print for it — and nothing anywhere constructs one outside
the round-trip tests in its own file. `AgentStatus`, `Preempted` and
`LayoutResync` have 2, 10 and 6 emitters respectively; this one has none. An
agent waiting for a pane-exit line waits for good.
What actually happens is checked against a running server: exit the shell in a
pane and exactly one event arrives — a `layout` delta carrying `pane_facts` for
that pane with `"live": false`, and the pane is gone from `pane ls --all`. The
docs now say to watch for that, in a note that says outright there is no
pane-exit line whatever the wire protocol's shape suggests.
The variant keeps a comment saying the same thing, and that resolving it means
either emitting it where the pane is reaped or removing it along with the
`event_line` arm — not something to do in passing, since taking a variant out
of a wire enum is a dialect change.
2951 tests pass.
The warning on `pane close --orphans` said an orphan "can still be doing real
work — an interrupted `run` leaves the command running". True, and it stops one
step short of the case that costs someone work.
A `run` that is executing *right now* is an orphan too. `run --ws W` stamps its
pane with `W` as the owner and files it into no tab until `--keep` does, so for
the whole length of the command the pane reads exactly like a leftover.
Confirmed against a live server: `tty7 run --ws W -- sleep 45` shows up as
`orphan=true, owner=<W>`, the same shape a leaked pane has, and
`pane close --orphans` duly reported `closed 2 panes` — one of which was the
running command.
So the old advice — "look at `pane ls --all` first" — cannot be followed:
nothing in that listing tells the two apart. The docs now say that, and say
what to do instead (close by id when anything might be running).
This also rules out the reaper people will reach for when they meet the pane
leak in `ui::tree_sync`: the daemon's own `spawn_orphan_sweep` already computes
this exact set and deliberately only reports it, and an in-flight `run` is why
acting on it would be wrong. `orphan_panes` now carries that reasoning.
The distinction that would work is whether a client is still attached — an
interrupted `run` has none, a running one does. The daemon knows and
`PaneInfo` does not say; adding the field is backward compatible, since every
other field on it is already `#[serde(default)]`, but it is a wire change and
wants more than a doc pass.
2951 tests pass.
The daemon answers a pane it is not running exactly as it answers an idle one:
`registry.get(pane_id)` misses and the reply is an empty `PaneProcs`. So
tty7 procs %999
printed `nothing running in this pane` and exited 0 for a pane that has never
existed. Every other verb taking a `%PANE` says when the pane is not there, and
an agent reading this one could not tell the two apart — which is the whole
point of the machine-readable half.
Checked against the registry rather than the workspace tree, because a pane no
workspace holds is still a pane the server runs and still worth reporting on;
that is exactly what `pane ls --all` surfaces it for. Verified live: a real
pane and an orphaned pane both still answer with exit 0, and %999 now exits 1
with "no pane %999 on this machine — `tty7 pane ls --all` lists them".
Asked only when the answer came back empty, so a pane with anything running in
it still costs one request.
One existing test needed the mock's registry seeded alongside its machine tree.
That is the fixture becoming faithful rather than the check being loosened: a
server running the pane its tree names is what the real pair look like, and the
mock had the tree without the registry.
2951 tests pass.
With no daemon running, most verbs give the message that helps:
tty7 ls could not reach the tty7 server on this machine —
`tty7 server start` brings one up
The three that go to a pane instead of to the control socket did not:
tty7 send %1 hi sending input to pane %1: No such file or directory (os error 2)
tty7 capture %1 observing pane %1: No such file or directory (os error 2)
tty7 procs %1 No such file or directory (os error 2)
All three read as if the *pane* were missing, or some file — and `procs` gave
no context at all. Panes are reached over the daemon's own socket, and with no
daemon there is no socket, so the connect fails with a bare `NotFound` that
each caller then wrapped in its own words.
`local_control` has contexted its connect all along; this is the same courtesy
one socket over. The local pane client is probed once when it is built, so all
three now say what `ls` says.
Local only, deliberately: the message names *this* machine, and a routed client
would pay a round trip across the link to be told something untrue of the far
end. Cached with the client, so it costs one round trip per process.
Nothing real is swallowed — a daemon that is running answers a bad pane id with
its own message, and still does:
send %999 sending input to pane %999: no such pane 999
capture %999 observing pane %999: daemon refused Observe: no such pane 999
Checked against a live instance both ways, plus a full send/capture round trip
on a real pane.
2949 tests pass.
`serve_sigterm` blocks SIGTERM and waits for it on a dedicated thread, so that
`store_scrollback_now` can write every pane's screen one last time on the way
out — "the periodic writer covers the deaths nobody gets to prepare for; this
covers the ones we do".
It ran too late to do that. `pthread_sigmask` blocks a signal on the *calling*
thread only, and a signal sent to a process is delivered to any one thread that
has not blocked it. The call sat after the control listener was already
serving, so several threads had SIGTERM unblocked and the kernel could hand it
to one of them, where the default disposition ends the process on the spot:
`sigwait` never returns, no screen is written, and nothing is logged.
Measured against a running daemon, sending SIGTERM directly:
before the marker printed after the last periodic snapshot was absent from
the store, and "daemon shutting down on SIGTERM" never appeared
after the marker is in the store and the line is logged
So a machine shutting down, a logout, a supervisor stopping the daemon or a
plain `kill` cost every pane up to SNAPSHOT_INTERVAL — 30 seconds — of screen
that the code was written to keep.
The fix is where the call sits, not what it does: immediately after the
registry exists and before the control listener, which is the first thing in
`run_daemon` to start a thread. A thread inherits the mask of the one that
created it, so blocking there makes the whole process deaf to SIGTERM except on
the waiter. Blocking and waiting stay in the same call, so there is never a
window where the signal is blocked with nobody to answer it.
`tty7 server stop` was never affected: it asks over the protocol
(`ClientMsg::Shutdown`) and only falls back to signals, so it took the other
`store_scrollback_now` path.
2949 tests pass.
`Config::load` runs on every pane spawn and every palette command, and the
parse-failure arm logged a warning each time. One stray comma in config.json
therefore produced a log made of the same line: measured at 23 copies after
opening three tabs and sending one command, growing for as long as the session
lasts and burying every other warning in the file.
The answer was already there. `quarantine` refuses to keep a second copy of
contents it has kept before — that check is what stops a broken config filling
the directory with identical `.corrupt` files — so it already knows whether a
breakage is newly seen. It now says so, and the parse message is a warning the
first time and a debug line afterwards.
Nothing else moves: the file is still kept aside, `save` still refuses to run
against a quarantined config, and the user's file is untouched.
Verified against a running instance with a deliberately malformed config.json:
the same four operations that produced 23 warnings now produce 1, there is
still exactly one `.corrupt` file, and config.json is byte-identical to what
was written before the app started.
2949 tests pass.
The note said refused tree operations are how this window strands a shell.
They are one way, and the minority one — which matters, because it is the way
that would attract a fix.
Measured with a randomized tree fuzz over the CLI (70 operations: tab new,
split, pane close, tab close, tab move, invariants checked after each):
daemon only, no GUI 0 orphaned shells
same run, window open 16 orphaned shells, 12 panes actually held either way
A second run of 70 left 10 orphans, and the warning this note sits on fired for
3 of them. The other 7 were spawned for a layout the window then threw away
without sending an operation at all — the log shows "dropping a superseded
hydration" and a delta that "did not apply cleanly" forcing a re-pull. Nothing
is refused on those paths, so nothing reaches this arm.
They are live `zsh` processes, not bookkeeping: each orphan answers
`tty7 procs` with a pid.
So the note now says which share this path accounts for, and that the sweep has
to be phrased against the end state — a pane this window spawned that neither
the machine tree nor any live view holds is stranded however it got there —
rather than against any single failure, which would fix a third of it.
Still diagnosis only, for the reason already recorded: the sweep belongs after
a hydration settles, and verifying it needs a GUI this environment cannot
watch.
2949 tests pass.
Last commit hedged on whether the missing russh fixes matter to a client,
because the release notes describe them as a client crashing its own session.
Matching the whole lockfile against the GitHub advisory database settles it —
the advisory titles say plainly what the release notes did not:
GHSA-5xvq-cp9x-6p6r "Pre-auth remote panic via all-zero Curve25519 peer
public value (encode_mpint OOB)"
GHSA-g9hv-x236-4qp3 "client wrong-length X25519 clone_from_slice panic
(pre-auth DoS)"
tty7 is the client, the peer is whatever server a profile dials, and this
lands before authentication — so anything sitting in front of that server
reaches it too. Both are patched in 0.62.4; this rev is from 2026-07-14 and
predates it.
The other three are named too, with the two server-side ones marked as such
since tty7 runs no SSH server.
Still documentation only, for the reason the previous commit gives: moving the
SSH layer wants an SSH connection to test against, and connecting needs the GUI
connection manager, which this environment cannot drive. The fix is unchanged —
rebase the fork onto v0.62.6+, and drop the patch once #738 ships in a release.
2937 tests pass.
The patch note said "temporary until upstream releases gssapi-with-mic
support (PR #737)" and left it there. Checking that exit condition turned up
three things worth writing down.
The PR number is wrong: #737 does not exist. The real one is Eugeny/russh#738,
and it was merged upstream on 2026-08-11T09:27:51Z — so the condition this
patch was waiting for has half arrived. Only half: v0.62.6 was published at
09:26:56Z the same day, one minute before the merge, so no release carries it
yet and the patch still has to stay.
The part that matters more: this rev is dated 2026-07-14 and branched before
v0.62.3, so it is missing every security fix upstream has published since —
v0.62.4's three (malformed PTY request, malformed Curve25519 KEX packet, zero
Curve25519 key), v0.62.5's channel-ID validation and v0.62.6's
max_auth_attempts. The last two are described as server-side and tty7 runs no
SSH server, but "fix mpint encoding and validate curve25519 keys" is in key
exchange, which a client runs too, against whatever server it dials. Flagged
for assessment rather than asserted: I have not reproduced it.
No dependency change here. Moving the SSH layer wants an SSH connection to
test it against, and connecting needs the GUI's connection manager, which
cannot be driven from this environment. The interim fix is to rebase the fork
onto v0.62.6 or later; the real one is to drop the patch when #738 ships.
Verified with the GitHub API rather than assumed: commit dates on both sides,
and the fork does not contain any of the four fix commits.
2937 tests pass.
tty7 has a `ClipboardLoad` arm that reads the system clipboard and writes it
back down the PTY, and nothing in this crate says that must not happen. It does
not happen — `alacritty_terminal` refuses a paste request before it becomes an
event, because `Config::osc52` defaults to `OnlyCopy` — but that is the VT
crate's decision, taken by a field tty7 never sets, protecting a handler tty7
has already written.
So the exposure is one upstream default away: a release that moved `osc52` to
`CopyPaste` would hand any program able to write to a pane whatever is on the
clipboard, with no prompt and nothing in the sequence identifying who asked.
The program can be on the far end of an SSH connection, and a clipboard
routinely holds a password, a token or a private key. Terminals that offer the
read at all put it behind a prompt or an explicit opt-in.
`osc52: Osc52::OnlyCopy` is now stated where the rest of the terminal config
is, so writes still work and reads stay refused by tty7's own decision. No
behaviour change today. Same shape as `conpty_resize` two lines up, and it gets
the same kind of test, for the reason that one documents: a field whose default
already matches is a field whose line can vanish unnoticed.
The `ClipboardLoad` arm keeps a note saying what makes it unreachable and that
unlocking it needs a consent step first.
2937 tests pass.
A kitty graphics escape is untrusted input — any program running in a pane can
print one — and every bound on that path is deliberate and spelled out, except
this one. `to_rgba8` clamps an inflate to `min(declared, MAX_IMAGE_BYTES)` and
says why the declared size cannot be trusted on its own; the PNG branch called
`image::load_from_memory`, which leaves the ceiling to `Limits::default()`.
That default is 512 MiB — a real bound, eight times `MAX_IMAGE_BYTES`, and
somebody else's number. A patch release that relaxed it would silently widen
what a hostile escape sequence can allocate, and this runs in the GUI process,
so an OOM here takes every pane down rather than the pane that asked for the
image. The limit is now stated locally at the value the default has today, so
behaviour is unchanged and the invariant stops depending on a dependency's
opinion.
Why the number is not simply `MAX_IMAGE_BYTES`: that bound comes from what a
wire frame carries, and a PNG crosses the frame still compressed, so its
decoded size is the one thing on this path not already settled. Lowering it
would bound this properly but decides how large an image tty7 means to display,
which is a product call and not a cleanup — noted in the comment.
The PNG branch had no test at all, so the change is covered both ways: a real
2x1 PNG decodes at its own dimensions rather than the escape's `s=`/`v=`, and
a 1x1 PNG whose IHDR is rewritten to 0xffff x 0xffff (~17 GB, CRC repaired so
the decoder parses it) is refused. Checked out of band that the refusal really
is "Memory limit exceeded" and not a malformed-file rejection, so the test
cannot pass for the wrong reason.
2936 tests pass.
`StdioDuplex::take` dup'd stdin and stdout, redirected both to /dev/null, and
only then wrapped the raw descriptors in `File`. Any of the four `?`s between
the first `dup` and the wrap returned without closing what it already held —
a descriptor leak on the error path.
`dup_fd` now hands back an `OwnedFd`. `dup` returns a fresh descriptor nothing
else holds, which is exactly that type's contract, and ownership from the
moment it exists is what makes the early returns safe. `take` drops two
`unsafe` blocks in the process: `File::from(OwnedFd)` is the safe conversion.
Immaterial in production — `take` is called once in tty7-server's main and the
`?` there exits the process — but it is unsafe-adjacent code where the correct
version is also the shorter one.
Exercised by the 55 `stdio_conformance` tests, which drive
`tty7-server --stdio` through this constructor. 2934 pass.
The table synthesized the `local` row itself while `--json` serialized the
server's routes alone, and a route is a link to some *other* machine. So on a
machine with no remotes — which is every machine until someone connects one —
`tty7 machine ls` printed a row for itself and `tty7 machine ls --json`
answered `{"machines":[]}`. An agent enumerating machines read that as "there
are none", including the one it was running on.
The docs already said what it should be: "the local machine plus every link",
with `{"machines":[{"key","kind","connected"}]}`.
The list is now assembled once in `machine_ls` and handed to both renderings,
so the two cannot disagree again; `routes_table` renders what it is given
instead of adding a machine to it. Pinned by a test that asserts the table and
the JSON agree, with and without a link.
Found by running the command against a dev instance rather than by reading it —
the human output looked right, and it was the half nothing prints by default
that was wrong.
2934 tests pass.
`TabCreate`, `PaneSplit` and `PaneReplace` each carry a pane the window has
already spawned — the shell is running on the machine before the tree is told
about it. When the tree refuses one of those, nothing ever takes the pane and
the re-pull leaves the window without the tab it was for, so the shell goes on
running with nothing referencing it. The log said only that an operation was
refused, which is the one thing that does not point at the leak.
Reproducible against a running instance: with a window open on a workspace,
`tty7 tab new` and `tty7 tab close` back to back. The GUI restores the new tab,
finds its pane already hung up, spawns a replacement, and `PaneReplace` is
refused because the tab is gone. Four cycles in five leak a shell; leave half a
second between the two and none do. Killing the GUI and repeating the loop
leaks nothing — the CLI half is correct throughout.
Not swept here, and the comment says why: at the point of the refusal the
window still holds a view for the pane and only drops it once the re-pull
lands, so hanging it up here would kill a pane that is still on screen. The
sweep belongs after the pull settles and has to test the whole machine tree
rather than this workspace's mirror, since a pane belonging to another
workspace on the same host is not this window's to end. That change wants a
GUI it can be watched in; this one only makes the leak say its own name, and
`tty7 pane ls --all` already points at the recovery.
`seeded_pane` is pinned by a test, because the set of requests that name a pane
into existence is exactly the set a refusal can strand one from — a new one
added without it would leak silently.
2933 tests pass.
Found by driving a dev instance with the CLI rather than by reading, which is
the only way any of these would have surfaced.
`ws attach` is documented as "become its controlling client", and the CLI
cannot be one: the claim belongs to the connection that made it and the
connection ends when the command does, so `tty7 ls` reads unattached again the
moment it returns. Its real and only lasting effect is displacing whoever held
it — which is exactly what the human output says (`took over from HOST`) and
what the docs never did.
`ATTACHED` said it names "a GUI window, or another client". A GUI only claims a
workspace it is showing as a *remote* one; a window on a workspace of its own
machine claims nothing, because there is no second client to arbitrate against.
So with a window sitting on it `tty7 ls` prints `-`, and the column read as
"nothing has this open" when it means "no remote client holds this".
`tab move @TAB INDEX` never said which end `INDEX` counts from. It is 0-based —
beside an `@N` address that is 1-based, on the same command line — so
`tab move @1 2` moves the first tab to the third slot. Verified against a
running server: `@1 0` leaves it, `@1 1` puts it second, past the end clamps to
last the way `split --ratio` already documents. `to` in the JSON echoes the
number you asked for rather than where the tab landed, which is worth saying
since the clamp makes those differ.
Also records why `WorkspaceDetach` throws away the one thing it computes: the
reply has always been `Unit`, and the dialect is spoken to whatever build was
pushed to a remote machine, so widening it to `Bool` would break every server
already out there.
2932 tests pass.
Nothing had ever run `cargo doc`, so 16 warnings had collected. Four were
links to items that do not exist, and two of those were worse than a dead
link: `git_badge` and `info_chip` documented their sizes in terms of
`PANEL_TEXT` and `PANEL_TEXT_META`, px constants deleted when the interface
font scale landed. The module comment twenty lines up already says they went;
the prose downstream still derived pixel arithmetic from them, so a reader was
being told the pill is 20px tall against a 19px neighbour when both are now
rems that move with `ui_font_size`.
Rewritten against the ladder that exists (`META_MONO` beside `TEXT_MONO`), and
`info_chip`'s comment now records what its own numbers imply: its padding and
radius are pixels wrapped around rem-sized text, so the two stop agreeing once
the interface scale leaves 100% — the same trap `PIP_SIZE` right below it is
written in rems to avoid. Left as a note rather than changed, because that is a
visual decision and this cannot see the result.
The other ten are `private_intra_doc_links`, and that lint does not apply here:
it exists so a *published* crate does not ship docs whose links dead-end, and
all four crates are `publish = false`. Allowed at the crate root with that
reason, because a public item explaining how it relates to a private one is the
useful half of these comments.
The `clippy` job becomes `lint` and runs rustdoc too, on the same warm cache.
Still non-required.
2932 tests pass.
`theme_preset` replaced `theme` on 2026-07-17, but the field stayed on the
struct, so every save since has written a `"theme": "light"` into the user's
config.json that nothing reads. It has no doc comment — the only field in the
file without one — no migration reads it, and grepping for it finds only the
`ui::theme` module.
Removing it is backward compatible in the direction that matters: `Config`
takes no `deny_unknown_fields`, so a file still carrying the key loads and the
key simply stops being written back. There is nothing to migrate; by the time
it was removed no build in the wild consulted it, which is also why a config
upgraded from before July never had its theme carried across.
Pinned by a test, because "the old file still loads" is the whole claim.
Found by diffing the documented configuration keys against the struct: it was
one of three fields with no entry in docs/reference/configuration.mdx, and the
other two (`command_frecency`, `ssh_profile_frecency`) are usage counters
rather than settings and are right to be undocumented.
2932 tests pass.
`closing_the_pool_drops_queued_work` never tested a queued job. `submit`
grows the pool whenever `jobs.len() > idle`, so blocking one worker and
submitting a second job spawns a second worker and runs it immediately —
the assertion only held when `close()` beat a brand-new thread to the state
lock. On a loaded machine it lost that race about one run in six.
A job only stays queued when the pool is saturated, so the test now fills it
to MAX_WORKERS first, asserts the job is on the queue before closing, and
waits for the workers to drain afterwards. 14 consecutive `-p tty7-core --lib`
runs green, against 2 failures in 16 before.
Placeholders in a translation were checked by nobody: `apply_template`
substitutes by name and leaves anything it was not given alone, so a `{name}`
dropped from the zh string reaches the user as a sentence that has simply
stopped naming the host, and an invented one draws braces on screen. Neither
is a missing or empty translation, which is all the parity test could see.
Both are now checked, for the plural and select branches too. Nothing was
wrong today.
The `KEPT_IN_ENGLISH` staleness check printed rather than asserted, and cargo
swallows a passing test's stdout, so the list could only ever grow.
The Linux build dependencies existed in four copies across three workflows;
release and nightly had the AppImage packaging tools (libfuse2, file,
imagemagick) mixed into the same list, which reads as if they were needed to
build. One script now, with the packaging extras installed separately, and the
ci.yml comment points at the docs page that actually documents the list rather
than at the README, which never did.
2931 tests pass.
CI checked `cargo fmt --check` and the build, so nothing ever read the
content of the code — ~200 clippy findings had accumulated, a third of them
in `#[cfg(test)]` modules.
Two of them were real:
- `resolved_background_appearance` took `backdrop`, and only the
`#[cfg(windows)]` arm used it. Renaming it to `_backdrop` is what clippy
asks for and compiles cleanly on macOS; on Windows it is an undefined
name. Kept the parameter and discharged it in the non-Windows arm the way
`package_for_current_install` already does.
- `SettingsSearchBackdropKeywords` and seven other `L10nKey` variants were
carrying translations in three languages for strings nothing reads.
The rest is mechanical: let-chains for collapsible `if let`s, struct-update
syntax for `Default::default()` reassignment, `sort_by_key` where a manual
reversed comparator was doing the same job the same file already did with
`Reverse` two functions later.
Where clippy was wrong, the reason is now in the tree rather than rediscovered:
per-platform `#[cfg]` blocks each keep their `return` (dropping it only
compiles on whichever target's block lands last), the loopback parser keeps
three parallel arms instead of folding one into a `?`, and the four wide enums
are all built on the stack and consumed immediately, so boxing them would add
an allocation rather than save one.
`L10nKey` cannot be clean under `dead_code` on any single platform, so the
allow there records how to audit it instead — which is how the eight dead keys
were found.
`english()` gained the cross-platform test its doc comment already claimed;
only the Windows hint had been pinned.
The clippy job is non-required until it has green history, for the same
branch-protection reason `host-boundary` documents.
2930 tests pass.
* feat(tabs): drag a tab in as a pane, and a pane out as a tab
A tab dragged by its chip or its sidebar row can be dropped over the
panes to become one of them, and a pane dragged by its grip can be
dropped on the strip or the sidebar to become a tab of its own. Both
carry the panes across as they are: nothing is spawned and nothing is
killed, so a shell mid-command, an SSH session or an agent mid-turn
keeps running.
The landing is read the way a pane drag's already is, minus the middle:
an arriving tab has nothing here to trade places with, so a pane's core
means "split it the way it is longest". A tab that was itself split
arrives with its own shape intact and takes one share of the row or
column it joined. A pane on its way out is offered a caret between two
tabs, and the last pane in a tab is offered nothing, being a tab of its
own already.
Picking a tab up no longer switches to it: the strip and the sidebar
now activate on the click rather than on the press. Without that the
merge cannot be expressed at all — pressing the tab to drag it would
put it on screen, leaving no other tab to drop it into.
Two things in the machine tree had to follow:
* Panes that change tabs are told as PaneMove, one at a time, rather
than as a tab closing and another being rebuilt around them.
* The tabs the machine already has are reconciled before new ones are
created, so a pane leaving for a tab of its own is given up by the
old tab before the new one asks to register it. The machine refuses a
pane that is in two tabs at once, and the refusal desynced the window.
Closes#621
* test(tree-sync): a tab grafted above a whole layout still converges
* fix(tabs): keep a click on the close button from switching tabs
Switching on the release rather than the press means every click inside
a chip or a sidebar row now reaches the row itself, and gpui-component's
`Button` does not stop propagation on a click it handled. So one click on
a tab's close button ran `close_tab(i)` and then `activate(i)` — with `i`
by then naming whichever tab had slid into that slot, which moved the
active tab somewhere nobody asked for. A click into the rename field did
the same: it switched away from the tab whose name was being typed, and
took the focus out of the field with it.
Both now hold the click where they handled it, the way they already held
the press.
---------
Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
* fix(scm): keep the sync tile on the branch row at any panel width
The branch row's flex constraints were set on the Button, but
`dropdown_menu_with_anchor` hands that Button to a `Popover`, which wraps
it in a plain div and never applies the trigger style it was given
(`trigger_style` is stored and never read). The constraints landed inside
a box that still measured its own content, so at the panel's 216px floor
a 24-character branch name overflowed the row and pushed the sync tile
out of the panel entirely, with no way to reach it.
Carry `flex_1` on a wrapper instead, and truncate the name against the
width it is actually given rather than against a character budget that
was guessing at that width. The `elide_middle` ceiling is gone: stacked
on top of a real truncation it produced two ellipses in a row
(`fix/new-tab-……`) and threw away the tail it existed to keep.
Notes and chips move into one `overflow_hidden` group that is allowed to
shrink, so the order of who gives way is explicit: branch name first,
badges second, the tile never.
Also stop offering "Publish Branch" from a HEAD that cannot publish. A
detached or unborn HEAD has no upstream by definition, so the token fired
there unconditionally — the widest thing on the row, naming the one
operation the tile beside it already refuses (#545), and on its own
enough to push that tile off a 216px panel.
* fix(scm): drop the branch row's note box when it holds nothing
The row lays the notes and chips out in one shrinkable box so the sync
tile keeps its place. An empty box is still a flex item, so the row's
6px gap was spent on either side of nothing: on the quiet branch that
is most of what anyone looks at, the caret sat 12px off the tile
instead of 6px. Build the notes first and only add the box when there
is something in it.
Also drop two comment citations of #549, which is about palette
commands that no-op silently and has nothing to do with this row.
---------
Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
* fix(new-tab): keep the SSH menu inside a menu's shape
The saved-host rows carried names and endpoints long enough to drag the panel
out to the 500px ceiling PopupMenu falls back to, and the row that meant to
elide was clipped mid glyph instead. The menu now stops at 360px, and a row
that runs out of room cuts the endpoint first — the name is what the reader is
picking by, so it keeps whatever is left rather than being squeezed to "..".
The height ceiling moves up to fit the shape everyone actually sees — nine
shells, both headings, six hosts and the two closing rows — so the default menu
arrives whole instead of scrolled with "Local" cut off above, and is capped
again against the window so a short one never gets a menu taller than itself.
The rule above the split hint goes: a separator divides two lists of things to
pick, and the hint is a footnote about the list it follows.
Bumps gpui-component, where a scrollable PopupMenu painted a scrollbar whether
or not it overflowed, custom rows could not elide, and labels had no padding of
their own.
* fix(new-tab): measure the menu ceiling off the viewport, and elide nameless hosts
`window_bounds()` answers how a window should be reopened after it is
closed, so a fullscreen macOS window reports the bounds it would restore
to rather than the screen it currently fills. A terminal spends much of
its life fullscreen, where that reading capped the menu at 80% of a
window nobody is looking at — putting back the scrollbar and the
cut-off `Local` this branch is here to remove. `viewport_size()` is what
every other window-relative size in the app already measures against.
A host saved on its address alone is *named* `user@host:port` and carries
no note, so it took the plain-item path — bare text with nothing to elide
against, on the longest string in the menu and the row least able to cut
it. Every host row is a custom element now, and `menu_row` drops its
right half when the note is empty rather than holding the gap open with
a zero-width child.
Also drops 17 unrelated dependency downgrades that rode along with the
`gpui-component` bump. The lockfile moves only the three `source` lines
it meant to; `cargo check --locked` accepts it.
---------
Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
Adds a saved-SSH-hosts section to the New Tab menu on both the tab strip and the sidebar, ordered by frecency, with a row that opens the full host palette. Holding the modifier opens the host in a split instead of a tab.
The menu scrolls once the rows outgrow the popup, long host names and their endpoints truncate rather than overflow, a host with no name of its own draws its endpoint once instead of twice, and the rows are built when the menu opens rather than on every painted frame.
Hoists the SSH password prompt's overlay from the body area to the window root so its scrim covers the title bar, tab strip and side panels, and aligns its top offset with the switcher card.
Replaces the raw target string with the resolved machine label on the reconnect banner and on the connecting pane, so a profile-backed machine no longer shows a bare config UUID in "Connecting to …" or "Could not reach …".
Bumps the gpui-component pin to pick up the notification restyle: flow-positioned status icon and close button that centre on the first line at any wrap count, a hairline-shadow surface in light theme, and a type ranking expressed in rems so it survives the ui_font_size setting.
* feat(ui,daemon): retire to the tray on window close and make cold start immune to stale daemon files
Two problems shared a root: the daemon outlived every window, and
nothing could stop it gracefully.
Window lifecycle:
- Closing the last window retires the app to the tray instead of
quitting (QuitMode::Explicit), so the daemon stays reachable. The
tray restores the most recent workspace, and Quit — after the
confirmation that protects running shells — stops the daemon. Every
explicit exit path (tray, palette, keybinding) now stops the server;
no exit leaves an orphaned daemon behind a dead icon.
- A pathless launch (double-click) hands off to the registered GUI via
GuiOpen(None) and exits, instead of starting a second process with a
second tray icon.
- The tray subsystem initializes once per process; reopening a window
no longer creates a duplicate icon.
Cold-start robustness:
- Liveness connects are bounded to 500 ms, the version handshake times
out in 1 s, and an unresponsive daemon is reaped by its recorded pid
instead of polled for a 6 s graceful stop.
- A dead recorded pid skips the TCP probes entirely — the GUI's
ensure_running, the new daemon's endpoint check, and the
control-listener occupancy check (which could also misread a reused
port as a live control server and refuse to boot). Stale cleanup now
also removes the leftover control.port.
* fix(daemon,gui): skip the GuiOpen handoff probe when the recorded daemon is dead
* fix(lifecycle): keep the stale-endpoint cleanup, and do not retire into a tray that is not there
Three gaps in the tray-persist and cold-start work.
`ensure_running` moved the refused-connect branch under the new liveness
check, so a connect that fails while `recorded_daemon_is_dead` says "not
dead" now skips the reap and the stale-endpoint removal entirely. The
pidfile answers "not dead" to two cases it has no evidence about: it is
missing (the daemon died between `transport::bind`, which writes
daemon.port, and `pidfile::write_current`), or it records a pid the OS has
since reused. Both then leave daemon.port on disk and the spawn poll pays
the OS's refusal delay on it — the cost this path was rewritten to avoid.
Restore the branch, and split the rule into `recorded_daemon_is_dead_with`
so a test can state that a missing pidfile is not evidence of death,
without an env var every parallel test would inherit.
The tray's windowless Quit stopped the server without a prompt, reasoning
that the confirmation is about the panes behind a window. It is not: it
says "anything still running in your shells is terminated", and retiring to
the tray is precisely what leaves those shells running with no window. Bring
the window back and deliver the action to it, so the confirmation appears;
only when no window can be opened does the bare stop remain, with a warning.
`show_tray_icon` is a request, not an outcome. `Backend::create` can fail
for a whole run — a Linux session with no StatusNotifier host is the
ordinary case — and after MAX_ATTEMPTS the loop gives up and logs. Retiring
on the config alone then leaves a process with no window and no icon: not
reachable, and still holding the daemon. Gate the retirement on an icon
actually being up.
---------
Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>