The rcfile installs its hook into PROMPT_COMMAND as an array only on
bash 5.1 and up, and appends to a string below that — a version check
inherited from bash-preexec. macOS still ships 3.2.57, so a developer run
takes the older branch and a Linux CI runner takes the newer one.
Neither machine covers both, which is worth saying in the test rather
than leaving for someone to work out from a passing run.
zsh is steered with ZDOTDIR in the environment; bash is steered with
`--rcfile … -i` on the command line, and `replaces_argv` is true for bash
alone. Two mechanisms with two ways to break — a dropped `-i` leaves a
shell that runs the rc and exits, a wrong `--rcfile` path leaves one with
no integration and nothing said — and neither had been run.
It works: all four marks arrive and OSC 7 names the real cwd. `-i` is
asserted on the injection as well as implied by the marks, because that
is the argument whose absence would otherwise show up as a timeout rather
than as an explanation.
Same sentinel discipline as the zsh test, and the same reason for it.
Confirmed the run is real rather than an early return: /bin/bash, args
["--rcfile", "…/tty7-bashrc-<pid>-0/bashrc", "-i"], 417 bytes off the
pty — which also exercises the exclusive scratch-directory creation from
earlier on this branch, since that rcfile is read out of one.
Four shells had a pty test — git-bash and WSL on Windows, pwsh, and
nushell (Windows). Not zsh, the macOS default, and not bash. So the route
almost every user of this project takes — ZDOTDIR aimed at a scratch
directory of redirectors, each sourcing the user's own file and then
adding the hooks — was covered only by unit tests of the strings it
writes, never by running it.
It works: 133;A, 133;B, 133;C and 133;D;1 all arrive, and OSC 7 names the
real cwd.
Through `setup`, never `setup_zsh`, and the injection is asserted to carry
the empty sentinel before the shell is spawned. This suite runs inside a
tty7 pane, so TTY7_SHELL_INTEGRATION=1 is already in the environment and
every redirector guards on it being empty; reaching for the builder
directly leaves the whole injection inert while stock zsh still emits
marks of its own, and the test passes having proved nothing. Checked by
putting the sentinel back and watching the assertion fire.
The test also asserted a title at first, and was wrong to. Only
PowerShell's integration writes an OSC 0 title, because PowerShell has no
convention of its own; on a POSIX shell the title belongs to the user's
prompt and tty7 names the pane from OSC 7. That is now asserted the other
way round, so a title appearing here is a change someone has to mean.
`repo_home_resolves_worktree_layouts` feeds `repo_home` the --git-dir and
--git-common-dir this file believes git prints. That tests the model, and
the model is the part that can be wrong: the only reason `repo_home`
exists is that those two disagree inside a linked worktree, and what they
look like when they do is git's to decide.
So: `git worktree add` for real, then probe from inside it. The root has
to be the worktree the pane sits in and the home the repository it was
added from — swap them and every path the SCM panel offers belongs to the
wrong checkout. tty7 is developed in worktrees, so this is the layout the
panel runs in most days.
They agree, and `assert_ne!(home, root)` keeps the test honest: equal
there would mean the branch under test was never taken. Both paths are
canonicalized because the temp directory is reached through a symlink on
macOS. Checked that the run reaches its assertions rather than taking one
of its two early returns — root ends in /linked, home in /main, branch
"side".
`added` and `removed` are counted while parsing the patch rather than
read from git, so they are a second implementation of a number git
already computes — and every test of them was against a patch this file
wrote. `--numstat` asks git the same question directly.
Over a tree holding each shape that counts differently: a plain edit, a
pure addition, a pure deletion, a rename with an edit, and a file with no
trailing newline. They agree exactly, and the two hardest cases are the
ones worth naming — the file without a trailing newline comes out (0, 1)
rather than a line off, and the rename is (1, 1) like git's own -M rather
than the (6, 6) it would be if the rename went unnoticed.
Floored at five files so a comparison of two nearly-empty maps cannot
pass for agreement, and both sides were printed once to confirm the
comparison had real content in it.
`in_group` decides which of Staged, Changes, Untracked and Merge a file
is drawn under, and every test of it was against records this file wrote
— which checks the mapping against our reading of the porcelain rather
than against git. The two rows a mistake there produces are a Stage
button on a file with nothing staged, and a Discard button on a file
whose only change is already in the index.
So the same tree described twice: once by `probe_status`, once by the
plumbing each group mirrors — `diff --cached --name-only`,
`diff --name-only`, `ls-files --others --exclude-standard`. They agree,
including the two cases worth arranging on purpose: a file staged and
then edited again belongs to Staged *and* Changes, and a deletion that
was never staged belongs to Changes.
Two ways this could have passed without proving anything, both closed:
every group is asserted non-empty, so agreement between two empty sets
does not count; and the run was checked to reach its assertions rather
than take the no-git early return.
Two tests already cover spaces, quotes, newlines and non-UTF8 paths — but
against records this file writes itself, which tests the model of what
git emits rather than what git emits. Porcelain v2 separates records with
NUL precisely so these names need no quoting, and that assumption had
never been put to the real thing.
So: a scratch repository holding a name with a space, a double quote, a
backslash, a newline, a tab and non-ASCII, each made with std::fs rather
than through a shell — a shell eats exactly the characters under test.
Real git, real parse, and every path asserted to open on disk, because
that is what the consequence would be: a quoted or re-spelled name is one
the panel offers to discard and git then cannot find.
It passes, which is the answer worth recording — git hands all six back
byte for byte and the parser keeps them. Checked that the test really
reaches its assertions rather than taking one of its two early returns:
six entries come back from git, one per name.
`LinkShutdown` asks for a way to "force the read half to return … while
the reader is blocked inside read", and calls itself "not optional
politeness; without it a client cannot be closed". Both socket
implementors do exactly that with shutdown(Both), and a test shows one of
them releasing a parked reader.
The stdio one cannot. A pipe has no shutdown, so `StdioDuplex::split`
hands the *writer* back as the closer: shutting the link down closes our
outbound pipe and nothing else, and the reader parked on the inbound one
is released only when the peer notices that EOF and closes its own end.
Measured rather than argued — with a real pipe pair the reader is still
parked 200ms after shutdown_link and returns as soon as the peer closes.
Nothing here is broken: for the SSH-piped server the peer does exit on
EOF, which is the whole design. But the trait claimed something one
implementor gets only by way of the far end, and a pipe carries no read
timeout to fall back on, so a wedged peer leaves a reader parked where a
socket would not. That is worth being written down rather than
rediscovered.
The body also had `if !taken { return Ok(()) } Ok(())` — two arms doing
the same thing, which reads as something lost in an edit. It is a drop
and a comment now saying why the slot is emptied at all.
`quote_for_shell` decides how a path is typed into the user's own shell,
so a name that argues with the quoting is a path that names the wrong
file. Posix and PowerShell had tests; cmd had none for quoting at all —
only `unquote_word` and `quoting_for` — so what it does with an awkward
name was written down nowhere.
The `"` case is genuinely closed and the doc already said why: a Windows
path cannot hold one. `%` is the open one. It is legal in a Windows
filename and cmd expands `%NAME%` inside double quotes, before it parses
them, so a file actually called `%USERPROFILE%.txt` pastes as a line
naming somewhere else. There is no escape for it on an interactive line —
`%%` is a batch-file rule and `^` is not read inside quotes — so this is
pinned and documented rather than fixed, and the doc now says what it
costs: the wrong path, never a command, because `%` substitutes an
environment variable and can do nothing else.
The rest is the coverage cmd was missing, plus a check that `is_bare`
never lets a metacharacter through unquoted — which is the actual
boundary, since anything bare is typed with no quoting whatsoever.
`SCM_KEYS_AWAITING_A_CALLER` says it out loud: "delete a key from this
list as soon as something renders it". `ScmCheckoutBranch` had been the
placeholder of the inline "switch to which branch" input for some time
and was still on the list.
That is not a warning anyone sees, but it spends the list's whole purpose
in reverse. It exists so `dead_code` keeps reporting on the rest of the
enum and a genuinely stale key cannot hide in a crowd of unused ones; a
key that is in use makes the list an inventory of nothing in particular.
The other five are real — commit detail, staged count, reset to commit,
branch search, stash and switch are all unbuilt.
The guard reads the sources rather than trusting the list, because
trusting the list is what failed. It fails with the offending key named,
which the instruction alone could not do.
The other three live SSH tests fall back to localhost, $USER and
~/.ssh/id_ed25519, so `cargo test --lib live_ -- --ignored` is something
to run on any developer machine — it exercises the transport every remote
pane sits on, which CI cannot reach. This one panicked there on a bare
`TTY7_LIVE_SSH_HOST`, for want of an environment that cannot be assumed,
and a failure claims something is broken where a skip would have said
"no Kerberos here".
Now opted into by name with TTY7_LIVE_GSSAPI=1, and everything else comes
from `live_key_spec` like its siblings, so one set of overrides steers all
four. Checked both ways: without the variable it skips and says how to
enable it; with it set it really connects and reports what it found —
"gssapi-with-mic could not be tried; the server offers publickey,
password, keyboard-interactive" — so it is skipped, never disabled.
All four now pass against a live sshd on this machine: key auth, a
direct-tcpip forward carrying bytes both ways, and SFTP listing and
editing a real directory.
`parse_agent_event`, `osc::parse_notification`, `OscTokenizer` and the
kitty graphics control parser all read bytes chosen by whatever runs in
the pane — `cat` of a hostile file is enough. A panic in any of them is a
pane a stranger can end, and osc.rs and cli_agent.rs had no fuzz coverage
at all.
Every truncation of eight seeds plus 4,000 seeded corruptions: no panic
in any of them. That is a negative result, and it is only worth having
because the harness was checked against planted panics in two of the four
first — a fuzz that reaches nothing passes just as quietly as one that
finds nothing.
Read while writing it, and left alone because each is already right:
`OscTokenizer` abandons a payload past MAX_PAYLOAD rather than truncating
it, so an unterminated escape cannot grow the buffer without bound; the
kitty parser returns None on `width * height * bpp` overflow and caps the
inflate; and `parse_notification` indexes only what it has just measured.
`parse_porcelain_v2` had tests for every record type, non-UTF8 paths,
newlines in paths and both caps — and none for a stream that stops in the
middle. Truncating a realistic sample at every byte, plus 2,000 seeded
corruptions, found no panic but did find 27 cuts that produce a
`StatusEntry` naming no file at all, one for each record type: a record
severed between its last field and its path parses down to an entry with
an empty path.
Not reachable today — `status_of` parses only when git exited 0, and a
git that exits 0 has written all of it. But this module's stated bargain
is that a record it cannot read is dropped, "far better for the panel
than no status at all", and a pathless entry is one it passed on
half-built instead. It would draw as a blank row whose own Stage and
Discard buttons hand git an empty pathspec.
So `push` drops it, and does not count it either. The fuzz stays as the
guard: every truncation rather than a sampled few, because the
interesting cuts are exactly the ones between a field and its separator,
and the corruptions are seeded so a failure reproduces from its message.
`run_scm_action` opened with a guard that returned when the panel had no
active repository, and said nothing. Every Git verb goes through it:
commit, push, pull, sync, fetch, stage all, discard all, checkout,
refresh.
The panel's tiles and the Git menu disable themselves in that state, so
the mouse never reaches this. The key bindings and the palette do — and
the palette lists every Git command whatever the pane's directory is,
which `palette.rs` pins on purpose: "every Git action you can bind is
also a palette command". So in a directory that is not a repository,
choosing Git: Commit did nothing and reported nothing.
That is the case `scm_push` already argues about its own guard one level
down, citing the same issue: a swallowed click looks exactly like an
action that finished instantly, and the toast is the only place a key
binding or a palette entry can say why nothing moved (#545). The message
is `DiffNotARepo`, which already existed in all three locales, so the
wording matches what the diff overlay says about the same state.
Sparing them is the right bargain and it is argued for in place: one
dialog per tab is not a question anyone can answer, so "Close Other Tabs"
skips what it will not take — a tab holding unsaved edits, or an SSH
profile that asked to be warned about — and closes the rest.
It did that silently. Choosing Close Other Tabs on a window with three
such tabs left three tabs standing and said nothing, which reads as the
menu item having half worked, and gives the reader nothing to act on.
This is the rule `scm_push` already states about its own swallowed click —
"a swallowed click on Push looks exactly like a push that finished
instantly" — and a close that keeps something back is one of those. Both
bulk closes now count what they spared and name the number, in all three
locales; a close that took everything still says nothing, because there
is nothing to explain.
The same hole as the local one, on the far side of the connection and in
generated shell rather than Rust — and on a shared server, which is where
several accounts actually coexist.
__tty7_d=${TMPDIR:-/tmp}/tty7-zdotdir-$$
command mkdir -p "$__tty7_d" 2>/dev/null
command cat > "$__tty7_d/.zshrc" <<'EOF' ...
... && export ZDOTDIR="$__tty7_d"
`mkdir -p` succeeds on a directory that already exists, and $$ is the
remote shell's pid, which every other account on that box can read. So
the startup files went into whatever held the name, and its owner could
rewrite them in the moment before zsh read them.
Now `mkdir -m 700` without `-p`, which fails when the path exists, with
the writes moved inside the `if`. A taken name means nothing is written,
the `[ -s ... ]` guards that were already there see nothing, and the login
falls through to the plain `exec <shell> -l` those guards exist for.
Run rather than reasoned about. Against a directory planted at the name
with mode 777 and an `evil` .zshrc: the fixed script leaves ZDOTDIR empty
and the plant untouched, while the `mkdir -p` version exported the
planted path for zsh to read. Both scripts pass `sh -n`, and the ordinary
path still creates drwx------ and exports ZDOTDIR.
What lands in this directory is sourced by the shell. `setup_zsh` points
ZDOTDIR at it and zsh reads .zshenv, .zprofile, .zshrc and .zlogin out of
it on every pane; bash and nu are pointed at theirs the same way.
`throwaway_dir` built the name from the pid and a counter — which the
reaper depends on, so it is entirely predictable — and then called
`create_dir_all`, which succeeds on a directory that already exists. On
Linux the system temp directory is /tmp, mode 1777, so any account can
take that name first. tty7 would then write the shell's startup files
into a directory somebody else owned, and they could rewrite them in the
moment before the shell read them. No mode was set either, so on a stock
umask the files were readable by everyone regardless.
Now `DirBuilder::create`, which fails if the path exists, with the mode on
the creating syscall rather than a chmod after it — there is no instant
where the directory is present and open. A taken name is stepped over,
not reported: stepping over it is the whole of what an attacker holding
one achieves, and after 64 of them the shell simply starts without tty7's
integration rather than reading files from a directory it does not own.
Verified on a live daemon, not only in a test: a pane's `cd /usr/local`
still moved the daemon's tracked cwd, so integration works, and the
scratch directory that daemon made is drwx------ next to the drwxr-xr-x
ones earlier builds left in the same temp dir.
`write_clipboard_image` did `create_dir_all($TMPDIR/tty7-clipboard)` and
wrote into whatever that turned out to be. On macOS $TMPDIR is a
per-user /var/folders path and this is moot. On Linux it is /tmp, mode
1777: the directory lands 0755 under the stock umask, so any other
account can read the screenshots pasted through it — or create the name
first and be handed everything pasted afterwards, which is also a place
to plant a symlink and have the write land somewhere else.
The remote half of this feature already defends all of it, and says so:
"a staging directory anyone else can enter is one anyone else can read
the pasted screenshots out of", and `staging_dir_is_safe` proves
ownership by chmodding to 0700 and reading the mode back — POSIX lets
only the owner change a mode. The local half, 200 lines up, did nothing.
Same proof, same predicate shape, and the same refusal of symlinks. The
ordering is the part worth stating: `set_permissions` follows links, so
the symlink test has to come before the chmod, not after it. My first
attempt had it after, and re-permissioned the link's target on the way to
refusing it — the test caught that, and pins it now.
Failing closed was already safe: `None` here returns false and the caller
forwards the paste the ordinary way.
It holds `history` — every command with its cwd and exit status — plus
the SSH profiles in `config.json`, `session.json` and `views.json`.
`machine.json` and `appearance.json` are written 0600 and the sockets
are 0600, but those four are not, and the directory around them was made
with plain `create_dir_all`: 0755 under the stock umask of 022, with the
history file 0644 inside it. Another account on the machine could read
the lot.
The rule already existed twice. `daemon::history` closes its own
subdirectory, saying "closing the directory is what makes the mode of
what is inside it moot", and `transport::bind` closes the socket's parent
when it is the config dir. Neither covered the directory holding
everything else, and the daemon reaches it first through the pidfile, the
singleton lock and the TCP endpoint, none of which closed anything.
`ensure_private_dir` closes every directory the call creates, and the
config directory itself even when it already existed — so an install made
by an earlier build is repaired rather than left open for its lifetime.
Directories it did not create and that are not ours are left alone, which
is the distinction `transport::bind` drew with `owns_parent`: $HOME and
~/.config are on this walk on a first run.
Verified against a running daemon rather than only in a test: with umask
022 a fresh config dir came out drwxr-xr-x before and drwx------ after,
and a directory chmodded back to 755 was closed again on the next start.
The unit test caught a hole in the first attempt — closing only the leaf
left the config directory, which `create_dir_all` had just made, exactly
as open as before.
Surviving a panic is not the same as answering it. The pool now outlives
a panicking job, but `run_job` still skipped `finish`, so the id stayed
in `inflight` and the client waited on a reply that was never coming.
Every other exit answers, including the one in `submit` for a queue too
full to take the request, so "every request gets a reply" is the
invariant the rest of this file already keeps. The catch sits behind
`caught`, a named seam, because a panic reachable on demand is what a
test of this needs and no control request offers one — `caught` can be
driven directly, and an ordinary Err still passes straight through it.
Both thread pools ran `job()` bare in their worker loop. A panic unwinds
out of `worker`, which skips the `threads -= 1` / `workers -= 1` that
every deliberate exit performs, so the count kept believing in a thread
that was gone.
`wants_another_thread` is `jobs > idle && threads < MAX`. After MAX
panics the count sits at the ceiling with nothing alive behind it: the
pool spawns no more workers and no worker is left to wake, so every later
job is queued and never run. On the UI side that stops the file tree, git
status, saving a file, SFTP and the diffs together; on the daemon side
the connection accepts requests and answers none. Both are silent, both
persist until a restart, and a daemon lives for days.
A host op runs somebody else's code — a git parse, an SFTP read, a
filesystem walk — so a panic there is exactly the event the pools should
survive. It is also what poisons the mutexes both of them already take
poison-tolerantly: the poison was handled, the accounting was not.
Each pool has a test that exhausts its ceiling, because counting workers
cannot distinguish one the pool believes in and has from one it believes
in and has lost. The op itself is still dropped — `off_thread`'s sender
drops unsent, so the landing never runs and the caller's in-flight flag
stays set. That costs one operation rather than every one, and landing
would need the value the panic is the reason we do not have.
`assign_keybinding` used `find`, so it unbound the first holder and left
any others. A chord can legitimately be held twice — `secondary-enter`
is Fullscreen in the window and Commit inside the commit box, which
`binding_conflicts` blesses because their scopes differ — and on macOS
that is a shipped default.
Rebinding it therefore left the new action sharing the chord with one
the note had not named. Which holder survived came down to their order
in `default_bindings`: Fullscreen sits at line 475 and Commit at 560, so
the right one was displaced by table position rather than by design, and
reordering the table would have silently swapped it.
Scope is still not consulted here, unlike in the conflict check. That is
the tested intent — a user who assigns a chord means it to be theirs
everywhere, which recording_an_extra_default_chord_displaces_its_owner
pins — so the fix is to displace every holder and name every one of them
in the note, not to start honouring scope.
The tree's rename / new-file / new-folder box subscribed to PressEnter
and Blur and no other key. So the way out of an edit you had changed
your mind about was to click somewhere else, and the other key already
under your hands — Return — commits the rename instead of abandoning it.
Every other box the app opens pairs Escape with Return: sftp_open_edit
says so out loud ("every other box in the app opens focused and answers
Return"), and the switcher, the branch inputs and the graph search all
handle both. The local file tree, the surface people touch most, was the
one that did not.
Both rows an edit can be drawn in are wired, through one free function
rather than a closure each, so they cannot drift apart on which keys
they answer — a test per row, each failing when only its own site loses
the handler.
Same rule, same reason, the surface next door. A branch name, a machine
label, a settings source path, an agent's display name, and the path out
of a terminal hyperlink — which is whatever the program writing to your
terminal chose to emit — all went into a notification raw.
An error message is deliberately left alone. In a toast the error *is*
the content, not a fragment inside a sentence of ours, and git and ssh
write genuinely multi-line errors whose second line is the useful one.
A dialog embeds one mid-question, so there it is still folded; the guard
now carries a different key list per surface and says why.
`terminal::view::one_line` states the rule and gives the reason:
"anything that draws a name it did not compose is exposed to it." Every
row surface followed it. No dialog did.
sftp.rs held both halves fifteen lines apart — the row folded
`entry.name` under a comment about bytes chosen on a machine this window
has no say over, and the delete confirmation for the same entry
interpolated it raw. A dialog is the worse place to lose it: gpui breaks
text on a newline whatever the style says, NSAlert renders one too, and
the dialog is where the destructive action gets authorised. A file named
`notes.txt\n\nThis one is safe to delete.` wrote its own second line into
the question.
Seventeen substitutions across eleven dialogs: delete (file tree, SFTP,
settings), discard changes, replace-on-drop, remove worktree, close
window, and the unsaved-edits prompts on close, quit and relaunch. Also
the machine label and the far end's error text in the remote dialogs,
which are no more ours than a hostname is.
The guard in ui::tests walks the prompt call sites rather than trusting
this sweep to have been complete — it found five of the seventeen after
I thought I was done.
`scm_commit_landed` clears `scm.drafts` — the commit message the user
typed. Two of the three conditions guarding that were held by nothing:
dropping the repo check let a commit in one working tree throw away
another's draft, and dropping the message check discarded text the user
had edited while the commit was in flight.
Both mutations now fail the suite. The third condition, waiting for HEAD
to move, was already covered — it is the one that keeps a message in the
box when a hook rejects the commit.
`remote_strip_action` exists because of a bug its own comment records: the
label and the action used to be decided separately, so a `ServerMismatch` —
the one state a retry cannot fix — wore a Retry Now button and looped on it
forever. Nothing tested it, so the regression could return in silence.
Three mutations left the suite green: making every state return `Retry`,
dropping the `hosts_our_server` check so a peer somebody else runs offers an
Update Server button that cannot work, and giving a state with no label a
button anyway.
Held now on all four answers — the install for a machine whose server is
ours, a retry for the states a retry fixes, and no button at all for
`Attached`, `RouteLost`, or a server that is not ours to install.
`can_spawn_locally` is one line over `WorkspaceStore::host_of` and nothing
called it. Inverting it left the suite green, as did making `spawn_host`
answer `LOCAL` for every workspace — so a window bound to a build box would
have started its pane on the laptop, which is the same class of mistake as
deleting a path on the wrong machine.
`host_for` underneath is well covered; it was the wrapper the decision
actually goes through that nothing exercised. Held now both ways: a window
with no remote may spawn here, the same window bound to a machine may not.
`install_phase_caption` is shared by the switcher's progress bar and the
strip's, precisely so a user watching both is not told two different things —
which also means a wrong caption is wrong in two places at once.
A server that sends no `content-length` gives `total: None`. Rendering that
through the with-total string reads "12 MB / 12 MB" while the transfer is
still running: it claims the download has finished, and the bar beside it
disagrees. Nothing failed when the two arms were swapped.
Held now across all four phases — no fraction when the size is unknown, one
when it is, an upload always knowing its total, and restarting having no
fraction at all.
No gap here — the property is already protected. This adds the assertions the
existing test cannot make, and records why.
`a_metacharacter_inside_quotes_does_not_start_a_new_command` asks
`at_command_position`, which is false whenever any non-whitespace sits between
the boundary and the word. A closing quote is non-whitespace, so that test
holds identically whether or not quoting suppresses a separator. The case that
separates the two readings is a separator inside an *unterminated* quote with
only space after it — `echo "a| ` and Tab — where reading the `|` as a pipe
fills the menu with every binary on PATH instead of the argument's files.
`segment_start` is now asserted on its own: real separators outside quotes,
the same characters inside terminated and unterminated quotes, an escaped one,
a backslash inside single quotes being literal so the quote closes at it, and
a closed quote no longer protecting what follows.
`lane_centres_rise_and_land_on_device_pixels` checks four scales and cannot
see this: with `GRAPH_PAD_L` at 6 and `GRAPH_LANE_W` at 12 every centre is
already `12 + 12·column`, an integer at 1x, 1.25x, 2x and 3x alike. Deleting
the whole body of `snap` left that test green.
So the rounding is insurance against a constant that stops dividing evenly —
change either to a half-pixel value and it starts carrying the lane strip, and
its own comment says a column of lines that changes width as it scrolls is the
most visible artefact this element can produce. Insurance that no test can
see is insurance somebody deletes.
Held on its own terms now: a centre between device pixels moves to one, a
centre already on one does not, the rounding is to the device grid rather
than the logical one, and a nonsense scale returns the value rather than NaN
geometry.
`AgentSessionState` travels further than most: `DaemonMsg::AgentStatus` on the
pane wire, the control reply behind `tty7 agents` and `tty7 wait`, and the
handoff blob a daemon writes so panes survive its own restart. It has no
struct-level `#[serde(default)]`, so each field default is the whole of what
keeps an older writer readable, and a state that fails to decode is an agent
whose status silently stops arriving — the orchestration primitives read it.
Three load-bearing defaults, none held: `status`, `rich`, `activity`. Pinned
as an empty state and as the shape a peer that only ever reported a status
would send. `status` falling back to `Idle` rather than the derived default is
part of what is pinned — that is why it names a function.
Also swept `shells.rs` and `router.rs`, which needed nothing.
`Session` carries `#[serde(default)]` on the struct, so a field missing there
falls back. `SessionPane` and `RemoteTarget` are enums and `RouteSnapshot` is
a plain struct, and none of them inherits that — their field defaults are the
whole of what stands between an older `session.json` and a window that opens
without the layout it saved.
Four such fields, held by nothing: a split's ratio, and the user and port on
both a direct SSH target and the route cached beside a window. Pinned as the
shapes older builds wrote, and re-swept: none of the nine load-bearing
defaults in the file can be removed now without failing.
`ssh_profile.rs` looked like three more and is not: `SshProfile` has a
struct-level `#[serde(default)]`, so removing a field-level one falls through
to the `Default` impl, which supplies the same id, port and integration flag.
Equivalent mutations, checked rather than assumed — the existing test already
decodes a profile with none of those keys.
`machine.json` is a user's whole layout across sessions, and failing to parse
it is not a degraded read: `load_machine` quarantines the file and starts
empty, so every workspace they had is gone from the app.
Swept it the way the protocol was swept — remove a `#[serde(default)]`, run
the suite. Eleven of the twenty-seven are load-bearing, because serde fills a
missing `Option` itself but a missing `Vec`, `String`, number or `bool` fails
the document. Seven of those eleven were held by nothing: the workspace list,
a workspace's tabs, a pane record's title and live flag, an attachment's host
and time, and the cached appearance.
Pinned as the shapes older builds actually wrote — a document with no
workspaces key, a workspace before it had tabs, a pane record that was only
an id — and re-swept afterwards: none of the eleven can lose its default now
without failing.
`Config` and `Session` needed nothing: both carry `#[serde(default)]` at the
struct level, and three tests already fail if `Config` loses it.
`rename_all` derives these strings from the Rust variant names, so a rename
in a refactor changes what goes on the wire without changing a string in the
source. The dialect number guards a variant being *added*; nothing guarded
one being renamed. Probed by giving each variant a `#[serde(rename)]` and
running the suite: 43 of them changed spelling with nothing failing.
Written as the tag an older peer sends, decoded here, because that is the
direction that breaks people: a build that no longer recognises `"dir"` does
not degrade — it fails the frame, and every directory in a remote listing
disappears.
Covers the enums where the tag is the whole message. The data-carrying ones
are deliberately left out: their payloads move with them, so renaming one is
a wider change than a string and will not pass silently. Re-swept afterwards
— none of the 21 unit variants changes spelling now without failing.
The previous commit covered the four structs carrying the most traffic. This
covers the rest, and the re-sweep that proves it: removing any of the 36
load-bearing `#[serde(default)]` attributes in `protocol.rs` now fails a test,
where before this pair of commits 33 of them did not.
The six added here are the ones nothing could reach by accident.
`SshAlgorithms`, `SshForwardRule` and the transfer specs are nested inside
structs whose own defaults leave them out of the JSON entirely, so a test that
decodes the outer struct never constructs them — they had to be named
separately or be held by nothing. `DaemonVersion` is the oldest shape of all:
a daemon that answered with a protocol number and nothing else.
`control.rs` was swept the same way and needed nothing; all five of its
load-bearing fields were already guarded.
Swept every `#[serde(default)]` in the protocol by removing it and running the
suite: 56 fields, and most removals changed nothing, because serde defaults a
missing `Option` on its own — verified against serde directly rather than
assumed. The attribute is only load-bearing on a `Vec`, `String`, number or
`bool`, where a missing field fails the whole frame instead of degrading.
Thirteen structs have such a field. Three were held. This adds the four that
carry the most traffic across a version boundary: `NativeSshSpec`, which goes
to the daemon on every SSH connect and has eight of them; `SftpEntry`, which
every remote listing is a page of; and the `ProcEntry`/`PortEntry` rows behind
`tty7 procs`.
Each is decoded from the JSON an older peer would send — required fields only
— and each of the four attributes was removed to confirm the test fails
without it.
Every wire struct here defaults its later fields so an older peer's message
degrades instead of failing the frame, and the rule is guarded — removing
`#[serde(default)]` from `PaneInfo` or `DaemonVersion` fails a test. Removing
either of `ShellSpec`'s passed the whole suite.
It is not a struct that can afford it. A `ShellSpec` crosses the wire inside
every `Spawn` that names a shell, in both directions and between builds: a
GUI talks to whatever `tty7-server` is installed on the far machine, and that
is regularly older. A required field there does not degrade — the frame fails
and the pane never starts.
`default_spawn_stays_wire_compatible_with_old_daemons` looked like it covered
this and does not: it exercises the `shell: None` legacy encoding, so it never
constructs one. This decodes the JSON an older peer sends and round-trips a
spawn that names a shell. Both defaults are checked by removal.
This codebase folds control characters in three deliberate places, each with
its reason written down: workspace and tab names at the machine store so no
two drawers disagree, an OSC title at the daemon so one line goes in and one
line is stored, and a *filename* at the point of drawing, because the raw
bytes are what `join` and `rename` are handed.
The sidebar draws two things in that third category and folded neither: the
cwd under a tab row, and the section headers built from repo roots and path
components. `mkdir $'a\nb'` is a directory somebody can be sitting in, and
gpui breaks a label on a newline whatever its wrapping says — the row grows
and paints over what is under it, which is the failure the rule's own comment
describes.
The grouping key is deliberately left raw: it is a path, and the sections are
grouped by comparing it. The test pins that too, since folding the key would
silently split one project into two.
The same bug as the delete, one entry point over, and a lexical search for it
missed this one: `file_tree_copy_into` looks the host up itself, and the
drop-replace path calls it *after* asking whether to replace. A workspace
repointed while that prompt is up would have put the files on the machine the
window had by then.
The host is now an argument, so the question of which machine cannot be
answered late: the caller decides before it asks, and the direct drop — which
never awaits — passes what it already had.
Checked by walking every awaiting closure in the file: none look up a host
after an await now. `sftp.rs` had the right shape all along, verifying its
pane is still the open one before acting; it is the model for this.
`active_host` is the host of the *window's workspace*, and a workspace can be
repointed at another machine while a prompt is up — a reconnect landing is
enough. Reading it after the answer meant the path the user was shown could be
deleted on whichever machine the window had by then.
`editor_save_file` already states the rule and the reason: the operation
belongs to the host it was asked about, however the window has moved since.
This was the one place that read the host after an await; every other
`active_host` in the file tree is synchronous.
Delete is the tree's one destructive action and had no test at all — the
prompt's wording was corrected earlier this session with nothing checking the
prompt is even reached. There is one now, both answers, and it watches the
file rather than assuming the removal lands in a single turn: it goes out on
a blocking pool that `run_until_parked` does not wait for, which is what made
the first version of the test fail. Checked by dropping the yes/no guard,
which makes the cancelled half delete.
The rustdoc gate is one this branch put in CI, and this branch had left it
failing — which is the worst state for a gate to be in, since it reads as
enforced and enforces nothing.
`displayed_registry` was the lower-case spelling of `DisplayedRegistry`, so
it resolves now. `PIP_SIZE` names nothing anywhere in the tree; rather than
guess which constant it used to be, the sentence now points at the rem-sized
constants it was contrasting against, which is the part that was actually
true.
All six CI gates now pass as CI runs them: fmt, host boundary, clippy with
`-D warnings`, rustdoc with `-D warnings`, the suite under `--locked`, and
the updater's own feature build.
CI runs `cargo fmt --check` and I had not run it once across this branch,
while making most edits by inserting text rather than writing it. 27 files
were non-conformant; `origin/main` is clean, so all of it is mine and CI
would have failed on the first push.
No behaviour change — the suite is identical either side of it. Also checked
clippy the way CI does, `--locked --workspace --all-targets -D warnings`,
which is stricter than the invocation I had been using.
The last way out of the app that had not learned about unwritten buffers.
⌘Q, the window close and the update relaunch each ask; the tray's Quit and
Stop asked its own question — every shell ends — and never mentioned them.
Worse than silence, its body reassures: "your tabs and layout reopen with
fresh shells next launch". They do. A buffer nobody has written down does
not, so a reader who accepts on the strength of that sentence loses something
the sentence implied was safe. It is the delete prompt's mistake in another
place: precise about what survives, quiet about what does not.
Named in the body rather than raised as a second dialog. They are already
being asked one question about what they are about to lose, and that answer
should account for all of it.
`wait_below_high_water` takes its mutex through `Locked::locked` and then
unwrapped the condvar wait on the next line. `Condvar::wait_timeout` hands
back the same poison the lock does, so the guard and the hole in it sat four
lines apart: a thread that panicked holding that mutex would stop the PTY
reader parking on it, which is the thread that pumps a pane's output.
The drift guard that was supposed to prevent exactly this only looked for
`.lock().unwrap()`, so it never saw the wait. It now covers both ways poison
reaches a caller, and names this site when the fix is reverted.
`host/server.rs` already did it the tolerant way and `control.rs` reads a
poisoned wait as "not done", so this was the one site out of step.
The docs site's reference was corrected when `--orphans` learned to spare
panes a client is attached to. The skill that ships with the product was not,
and it is the worse of the two to have wrong: an agent reads it before it runs
anything, and it still said `--orphans` "closes every pane no workspace holds"
and "closes exactly what `pane ls --all` marks orphaned".
The reference is drift-guarded against the CLI; the skill is guarded against
nothing, which is exactly why it drifted. Both are now held to stating the
attachment half of the test — to the rule, not to a sentence, so a reword that
drops it from either file fails at the moment someone can still check the
other.
Half a change, found by reading the docs rather than the code. `pane ls --all`
learned to count strays by whether anybody is attached; the per-row `orphan`
flag beside that count did not. The same JSON object contradicted itself, and
the reference says `--orphans` "closes what `pane ls --all` lists as orphaned"
— which had stopped being true for exactly the rows that matter, the panes a
window is adopting during a restore.
A script filtering `.panes[] | select(.orphan)` would have gone after panes
the reaper deliberately spares.
The row, the count, the doctor's row and the reaper now make one test, held by
a test that walks all four. The reference states both halves of what an orphan
is, since one of them is the half that keeps the command from taking a session
that is still coming up.
The Known section described a leak that has since been fixed, which makes it
worse than no entry: it tells a reader to expect a bug that is not there and
to reach for a recovery command they do not need. It is replaced by the fix.
Six more user-visible changes had landed without an entry — the reaper that
could take a live session mid-restore, the three ways out of the app that
discarded unwritten editor buffers, a saved file losing the line endings it
came with, a delete prompt implying a trash that does not exist, agent hooks
broken by a `$` in the install path, and one pane's panic costing every pane
on the machine.
Also corrects an issue reference I had wrong in both the changelog and a
comment: #672 is the rebuild that deleted tabs off the machine, not the
tab-close prompt for unsaved edits, which cites nothing.
Two rules of the authority's `pane_close` were unverified, and both are the
server-side twin of a gap the mirror had.
`active_tab` is an id, so one naming a tab that has just been emptied points
at nothing — and since the move never went out as a delta, every client keeps
its own stale answer too. Deleting the healing passed the suite.
`collect_orphan_panes` reads `m.panes`, so a record left behind after its tab
closed answers "orphan" on every later call, for ever, because nothing else
removes it. It persists as well: `machine.json` gains a row per closed pane,
and `pane ls --all` reads them back as panes no workspace holds. Deleting the
prune passed the suite too.
Both are held now, at the last pane of a tab and at a plain split close.
Checked by re-running the two mutations that had survived.
The mirror had no test for `RatioChanged` at all. Writing a fixed 0.5 instead
of the ratio the delta carries passed the suite, and so did answering `true`
for a path that names no split.
The second is the worse half. That return value is how `apply_delta` tells
its caller the mirror is still in step with the machine; a `true` it has not
earned is a divergence nobody notices, and the re-pull that would have
repaired it never happens.
Pinned against a tree with a split inside a split, so "the right node" is a
claim the test can actually make: the inner ratio moves and the outer one
does not, an empty path is the root, and a path ending on a leaf, a path
running off the end, and an unknown tab each write nothing and say so.
l0ng-ai