a_config_round_trips_every_field checks bools and numbers and skips
strings on purpose, because de_lenient falls back to the default for a
spelling it does not know. That is right for a file a human may have
mistyped, and it is exactly what makes a rename invisible: the value does
not fail to load, the setting is forgotten and the user finds their
choice reverted with nothing said. The wire has the same guard already;
this is the disk's copy of it.
Fifteen enums, forty-nine variants, two rename rules between them. The
strings were read out of serde rather than derived from the rename_all,
which is how RightPanelTab::Scm turned out to be "changes" — a rename
whose own doc explains it was done in place so an older build could still
read the file back. Someone tidying that enum would undo it and kick
every user off the panel they were on.
Both directions are asserted, since an alias can make a variant readable
under a name it is no longer written as.
Checked by tidying the rename away: fails with left "scm", right
"changes". My first attempt at that mutation edited the word in the doc
comment instead of the attribute and the test passed — a mutation that
does not mutate proves nothing, which is why the second one names what
it changed.
Sweeping the enums for variants no test mentions turned up ConflictKind:
seven XY pairs, three tested. The rest matter because an unrecognised
pair falls back to BothModified rather than failing, so a wrong entry
does not break — it relabels a conflict as one whose resolution is
something else. "Both modified" invites merging two versions of a file;
"deleted by them" means there is no other version to merge.
Four of the pairs are directional, which is the half that is easy to get
backwards: DU is deleted by us, UD by them, AU added by us, UA by them,
and nothing short of a real merge says which way round git means it.
Two merges reach all seven. An ordinary content merge gives AA, UU, UD
and DU. A rename/rename gives the other three at once — DD at the path
both sides renamed away from, AU at the name we chose, UA at theirs —
which is how the three that had never been produced anywhere become
producible in four commands.
Checked by swapping each directional pair in turn: both swaps fail, and
the message prints the whole set so the direction is visible rather than
inferred.
Thirteen of the fourteen GitOpErrorKind variants had a test; Timeout had
none. It is the one that says a git operation ran out of patience rather
than failed to start, and the difference is what the user is told: Spawn
reports that git could not be run and to check the install, which is the
wrong thing to say about a push that is still going on the far side and
may yet land.
The distinction rests on a single ErrorKind travelling three files —
ControlClient::call_with_deadline turns a RecvTimeoutError into
ErrorKind::TimedOut, RemoteHost::git_with_deadline propagates it, and
run_op maps it. Nothing held any link of that. Only a remote host can
produce it at all: the local git_with_deadline has no timeout of its own
and says so.
Both ends are pinned now. The client's contract is tested against a peer
that accepts a request and never answers — the deadline is honoured, the
error is TimedOut, and the request is cancelled rather than left with the
server working on a reply nobody will read. The mapping moved into
unstarted_kind, which can be asked directly what it makes of a timeout
and of five other ways a command fails to start.
Checked by removing each: the TimedOut arm, and the cancel.
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.
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.
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.
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.
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.
`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.
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.
`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.
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 row reads "N running that no workspace holds — `tty7 pane close
--orphans` ends them", and it was counting a different set from the one that
command takes: every pane a window is adopting during a restore counted here,
and the reaper now correctly ends none of them. A row that says seven beside
a command that ends nothing is a worse answer than either alone.
All three surfaces that tell the user about this set — the reaper, the
listing's count, the doctor's row — now spend one predicate, so the next
change to what counts as stray cannot land on two of them. Deleting the
attachment half of it fails a test on each.
The reaper's test was "the registry is running it and no workspace holds
it". A window restoring a layout spawns each pane, attaches to it, and only
then files it into the tree — so for a moment every pane it is adopting
answers to that description.
Not theoretical. Polling `pane ls --all` through a cold start of a seven-tab
window reported one, then two, then three, then six, then seven live panes as
held by no workspace, and the restore afterwards was correct: all seven were
wanted. `--orphans` at any point in that window takes the whole session, and
a script that reaps on a timer will eventually sit in it.
Attachment is the missing half, and it is the daemon's own fact rather than a
guess about timing: `attach` takes the pane's seat and the connection closing
calls `detach`, which clears it. A window adopting a pane is attached to it.
A pane whose layout was thrown away had its view dropped, which closed the
connection, which emptied the seat. Nothing else in the registry tells the
two apart, which is why the GUI had to ask its windows and the CLI could not.
`pane ls --all` counts the same set, because the line it prints tells the
reader to run the reaper.
Same race after the change: zero false positives across 300 polls, restore
intact. And a stray with nobody attached is still reaped — checked by
deleting the new filter, which fails both tests.
There are two, in two crates, and they are not redundant: one puts a path in
front of the user's own shell and has three dialects to pick between, the
other writes a line that some `sh` on a machine will re-read. A reader who
finds one and not the other has every reason to write a third.
Every agent takes the hook as a command *line*, so a shell parses it again
before anything runs. The path went in double-quoted, and inside double
quotes `sh` still expands `$`, a backtick and a backslash — so an install
under `/opt/build$stage` reached the shell as `/opt/build/tty7`. The hook
then never fired: no error, no log line, just an agent that quietly stopped
reporting its status for good.
Checked against a real `sh` first, because the failure is invisible from the
code: the double-quoted form resolves the program to `.../q/tool`, the
single-quoted one to `.../q$stage/tool`. Then checked again through the
generated OpenCode plugin under node with a stubbed `$`, which is the path
that actually ships — the assembled command re-parses to the full path,
dollar segment intact.
Which shell is the target's, not ours: a hook installed on a remote machine
runs there. Only a local Windows target keeps the double-quoted form, which
is what `cmd.exe` wants and what the PATH-resolvable case avoids needing.
`shell_quote` had been written out twice already, in `daemon::install` and
`daemon::shell_integration`. Rather than add a third copy this moves the one
implementation to `core::shells`, where a core module may reach it without
depending on the daemon, and both old homes now re-export it.
`destructive()` decides what the confirmation dialog tells the user they are
about to lose, and the choice is load-bearing: a hard reset onto an older
commit clobbers the worktree *and* drops commits, and the code deliberately
reports the worse of the two, because telling someone only about their edits
invites a yes to a different question than the one being asked.
The test asserted only that such an op warns about something. Downgrading a
hard reset from LosesCommits to LosesWorktreeEdits passed it unchanged —
checked, not assumed. Each operation is now paired with the warning it owes
the user, and the same downgrade fails.
No behaviour change; the classifier was already right.
The daemon holds every shell on the box behind mutexes — the pty master, the
child handle, the writer, the pane state. Seventy-four of those locks were
taken with `.lock().unwrap()`, so a panic in any one critical section
poisoned the mutex and every later taker died on it too. One bug in one
pane's thread, and the daemon can no longer serve any of them.
Poisoning buys nothing here. Whatever inconsistency the panic left is there
either way; the flag only decides whether the next thread also dies. A
garbled write to one pane is recoverable, and losing every session on the
machine is not.
Most of the code already agreed — about seventy places carried on, spelled
out by hand — so this is mostly a drift fix. `Locked::locked` gives the
policy one name, the daemon uses it throughout, and a test walks
`daemon/` and fails with the file and line if a panicking lock comes back.
Somewhere that genuinely cannot tolerate the inconsistency can still take
the poison with `lock()`; the point is that it be a decision rather than
the default.
A save is an overwrite of a file that already exists, and the mode belongs
to the file rather than to the write. A host that wrote by creating a fresh
file, or by sending the far side an explicit set of attributes, would turn a
script into something the shell no longer runs — silently, on a save the
user made for an unrelated reason, with nothing in the UI to report it.
Both hosts already get this right; nothing here is a fix. It is the case
that keeps them right, and it runs against the remote link too, where the
write is a control request rather than a syscall. Checked by making the
local write a remove-then-create: the case fails, as it should.
The two sandbox hooks follow the existing symlink one — a host with no
notion of an executable bit returns None and the case skips.
The grid's own tests cover wide cells and combining marks, but they sit at
the end of the chain: a pty, a ring and a snapshot on a socket come first,
and nothing exercised those with anything but ASCII.
Both spellings of an accented letter go in on purpose. A terminal must not
normalise, so the test asserts not only that each form comes back, but that
neither turned into the other.
`SftpManager` had no live coverage of any kind. Its tests cover the path
arithmetic — `remote_join`, `remote_parent`, `safe_local_name` — and the
transport-failure classifier, none of which needs a server, and all of which
sits downstream of a subsystem no test had ever opened. The whole panel is
built on two calls, `list` and `op`, and neither had been made against sshd.
Now both are, and I ran it: mkdir a scratch directory, create a file in it,
list it and check the entry comes back as a `File`, rename it and check both
that the new name is there and the old one is not, stat a path that was never
created and require the answer to be something other than `Done`, then remove
the file, remove the directory, and confirm listing it fails.
Everything lives under a directory named for the process on the far side and
is removed at the end; the far side being this same machine is what makes it
safe to assert about. Checked afterwards that `$HOME` holds no leftovers.
Takes its turn on the same mutex as the other two, for the reason that one
documents: they share `SshManager::global()`. All three pass together — the
fourth, GSSAPI, still wants a Kerberos realm this machine does not have.
`open_direct_tcpip` is what every local forward and the SOCKS proxy are built
on, and what a remote workspace's control link rides. Its existing tests are
about bookkeeping — which rule is registered, what a teardown reports — and
none of them puts a byte through a socket.
This one does, and I ran it: bind a loopback listener, open a direct-tcpip
channel to it *through a real sshd*, write a line in, have the listener answer
it uppercased, and assert both ends saw what the other sent. A listener of our
own rather than a well-known port, so the test needs nothing to be running and
cannot be fooled by something that is.
Adding it broke the key-auth test, which is the interesting part. Both share
`SshManager::global()`, and sharing is the whole point of it — one of them
asserts that a second `open_connection` is *reused*, and the other evicting the
same key underneath it makes that false. Under `--ignored` they are the only
tests running and they still collide with each other, so they now take turns on
a mutex. Verified: individually and together.
The spec both use is factored into `live_key_spec`, so the defaults that make
this run with no environment at all — localhost, `$USER`, port 22,
`~/.ssh/id_ed25519` — are written once.
tty7 speaks SSH itself rather than shelling out, and remote workspaces, the
routed pane socket and SFTP all stand on the connection it makes. The only
live test beside this one needs Kerberos — the case almost nobody runs — so
the case almost everybody runs had no end-to-end check at all.
It has one now, and I ran it. Against this machine's own sshd:
cargo test -p tty7-core --lib live_key_auth -- --ignored
test daemon::ssh::tests::live_key_auth_connects_and_opens_a_channel ... ok
Key auth against a real server, a channel opened on it, `echo tty7-live-check`
executed, the reply read back and compared, the exit status collected, and
then a second `open_connection` for the same spec asserted to be *reused* —
sharing one transport across panes is the reason `SshManager` exists, and
nothing was checking it either.
Ignored like its neighbour, because it needs a server, but a much smaller one:
Remote Login on the machine running the test and a key it accepts, which is
`ssh localhost true` away. The defaults are `localhost`, `$USER`, port 22 and
`~/.ssh/id_ed25519`, so on such a machine the command above is the whole
setup. `verify_host_keys` is off for the same reason it is next door: the
point is the transport, and a first connection to an unapproved host would sit
on a prompt no test can answer.
One thing the first run taught, kept as a comment: openssh sends the exit
status *after* `Eof`, so breaking on `Eof` reads the output and then reports
no exit code. Waiting for `wait()` to return `None` is the only end worth
waiting for. The output assertion passed on that first run — it was the exit
code that was missing, and it was my loop that was wrong, not tty7.
`civil_from_days` dates the panic log and lives in `core::crash`;
`days_from_civil` lives here. They were written separately, they are each
other's inverse, and each was checked alone — this one by the walk beside it,
that one against four known dates. Four dates leave the century years and
everything before 1970 untouched, which is exactly where era arithmetic earns
its keep.
So they now check each other: 73,049 consecutive days from 1900-01-01, each
converted both ways. Two separately written implementations can only agree
across two centuries if both are right. The walk starts before the epoch on
purpose — asserted, so a later edit cannot quietly move it after — because
negative day counts are the half neither test was exercising.
Verified against the epoch shift, which it catches and which moves every date
in the log by a day.
One mutation turned out not to be one: changing `doe / 146_096` to
`doe / 146_095` is *equivalent* — brute-forced over ±547 years, not a single
day differs — so no test failing on it is the right answer rather than a gap.
Worth writing down, because it looks exactly like a constant somebody typed
wrong.
My own expected day count was off by one on the first run (73,048 for
73,049); the implementations were right and the arithmetic in my head was not.
`days_from_civil` and `days_in_month` are pure arithmetic that no test named,
and every date this crate prints goes through them. Hinnant's algorithm is
correct as written here; what is easy is transcribing one constant wrong, and
a wrong `146097` or `719468` moves every commit date by a fixed amount that
still reads as a plausible date.
Walked rather than compared against a second implementation, so there is
nothing to get wrong twice: step a day at a time from 1900 to 2100 using
`days_in_month`, count as you go, and require `days_from_civil` to agree at
every step. Two centuries of agreement is only possible if both are right — a
leap rule wrong in either drifts them apart within four years and never
recovers — and the count of leap days is asserted at the end so a walk that
skipped February would not pass either.
Anchored first on the epoch, the day before it, and the two century years that
separate the naive leap rule from the real one, so a walk that is merely
self-consistent cannot pass.
Verified against three mistranscriptions: the era length, the epoch shift, and
dropping the divisible-by-400 clause. Each fails it.
Found by looking for pure helpers that no test in their own file names — 607
of them, most trivial or I/O-bound. This pair was the one where being wrong is
both easy and invisible.
An interrupted `tty7 run` strands a pane. So does a window reconciling its
layout while something else edits the tree — measured at 17 from 160
operations, every one a live `zsh` holding a pty and its descriptors, and
`tty7 pane close --orphans` ended all 17.
None of that was news to tty7: `pane ls --all` names them, the switcher lists
them, and the reaper works. What was missing is that nothing volunteered they
existed. `doctor` printed `status pid …, up 6s, 2 panes` and counted the
stray among them, so the one verb somebody runs when something feels wrong had
the number and did not say what it meant.
A row and a `server.orphans` count. Deliberately not an exit code: one stray
after an interrupted `run` is ordinary and the reference documents it as such,
so `tty7 doctor || alert` firing on it would cry wolf. What was missing was
the sentence, not an alarm.
Counted inside the arm that already has the machine tree in hand for the
dangling-context row, so it costs no extra round trip, and under `server`
because it takes a server to have panes at all.
The first draft put the count at the top level of the JSON, and
`the_doctor_json_sections_are_the_ones_the_reference_names` — added earlier
today for exactly this — caught it as an undocumented section before it went
anywhere.
`a_quarantined_tree_can_be_found_by_something_that_wants_to_report_it` carried
two summary lines:
/// What a ninth corruption does to the eight kept copies.
///
/// What `doctor` can find out about a tree that was set aside.
Only the second is about that test. The first belongs to
`a_ninth_corruption_overwrites_the_oldest_kept_copy` forty lines below, whose
own comment opens mid-thought — "`quarantine_path` ends in `unwrap_or(base)`,
which reads like a defensive branch and is the wrap" — with no summary at all.
Same displacement this tree has fixed once before in `daemon::pane`.
Moved back. Both now open with one sentence saying what they are for, which is
what every other test in this file does and what a reader scanning them needs.
Swept the rest of the tree for the same shape — a doc block whose first two
paragraphs are each a lone sentence — and found one other, which is a genuine
summary followed by a genuine second paragraph. This was the only one.
`the_reference_documents_every_flag_of_the_verbs_it_covers` read one level:
`cli.get_subcommands()`, then that verb's own arguments. Two things followed.
It never descended, so every flag a level down was unchecked — `tab new --cwd`,
`pane ls --all`, `pane split --ratio`, `pane close --orphans`,
`server restart --hard`. And its gate for "is this verb documented" looked only
for ``` `tty7 <name> ```, which the page uses for leaf verbs; a group is headed
``` ## `pane` — panes ```. So `ws`, `tab`, `pane`, `machine` and `server` were
skipped whole, taking their children with them. Fifteen flags were checked
where there are twenty-two.
Both fixed: the walk recurses, and the gate accepts either heading shape. The
reference turns out to be complete — nothing was actually undocumented, which
is worth knowing rather than assuming.
The matching rule was too strict as well, and the extension is what showed it:
requiring a backtick immediately before the dashes reported
`server restart --hard` missing when it has its own table row, written as
``` `server restart --hard` ```. It now looks for the flag anywhere on the
page.
The floor is five named nested flags rather than a count, because a count
drifts with the surface and what has to hold is that the walk went down.
Verified both ways: dropping `--orphans` from the page is reported, and
stopping the recursion is reported by name.
A pane costs about three descriptors — the pty master and the pipes behind it
— and the daemon took whatever soft limit its launcher happened to have. On
the historic macOS default of 256 that is a ceiling around eighty panes,
against a `MAX_PANES` of 16,384, and nothing in the failure said so:
tty7: spawning a shell: daemon refused Spawn: dup of fd 93 failed
That sentence does not contain the word "file". Measured with an inherited
soft limit of 96, the daemon stopped at 26 extra tabs.
Two halves. The soft limit is now taken up to the hard limit at startup, which
is the ordinary thing a long-lived server does, needs no privilege, and leaves
an administrator who lowered the *hard* limit in charge of the ceiling. Capped
at `OPEN_MAX` because macOS refuses `RLIM_INFINITY` outright and a refused
request would leave the low limit in place, which is the one outcome worth
avoiding. Same measurement afterwards: past 45 tabs and still going.
And when the hard limit genuinely is the ceiling, the refusal says what it is
and what to do, keeping the original in parentheses for whoever needs it. The
re-wording sits where the spawn handler turns the error into a reply, not at
`openpty`: several calls along that path can hit the limit, and libc's "Too
many open files" and portable-pty's "dup of fd N failed" both arrive there.
Matched on the text because portable-pty hands back a message, not a code —
so the test covers both spellings and, more importantly, that a missing
program and a permission error are left exactly as they were.
Verified against a live daemon in both configurations: a low soft limit is
raised past, and a low hard limit produces the new sentence with the daemon
still serving afterwards.
$ cat config.json
{ "shell": { "program": "/nonexistent/shell" } }
$ tty7 new
tty7: spawning a shell: daemon refused Spawn:
no such program on this machine: /nonexistent/shell
$ tty7 doctor | grep config
config ok
Nothing can open a tab, and the verb that checks the install says the config
is fine — which it is, in the only sense that row means: the file parses. The
same table already reports an unusable `custom_shells` entry, and that costs a
menu row. This costs every tab, every `tty7 new`, and the GUI's new-tab button.
The check is the daemon's own `shell_program_problem`, moved from
`daemon::pane` to `core::shells` beside `unusable_custom_shells` so both
callers share one definition and the row cannot drift from the refusal it is
predicting — it prints the same sentence the spawn will. Missing, a directory,
and not executable are told apart, because they are three different fixes.
A program given as a bare name is still not reported: the OS resolves it
through PATH and guessing at that would be worse than silence — the moved
comment says so and the moved tests pin it. Skipped under `-m` for the same
reason the hooks row is: a path checked here would answer about the wrong
machine.
Verified against a live daemon across all five cases — missing, directory,
non-executable, bare name, and no `shell` set — with exit 1 for the three that
break and 0 for the two that do not.
Three different mistakes, one answer:
{ "shell": "/nonexistent/shell" } -> NOT VALID JSON
{ "font_size": 12,, } -> NOT VALID JSON
{ "font_size": "big" } -> NOT VALID JSON
Two of those *are* valid JSON. They are valid JSON in the wrong shape — a
string where a struct goes, a string where a number goes — which is a
different mistake with a different fix, and telling that reader their file is
not valid JSON sends them hunting for a missing comma that is not missing.
serde has already worked out the answer. It names the field, the type it
wanted, and the line and column. That went to `log::warn!` and nowhere else,
and there is no log unless `TTY7_LOG` is set, so on a default install it went
nowhere at all — which is the same shape as the keybinding faults and the
clamped settings this tree has already fixed.
Now:
DOES NOT FIT — invalid type: string "big", expected f32 at line 1 column 20
NOT VALID JSON — key must be a string at line 1 column 19
NOT VALID JSON — EOF while parsing an object at line 1 column 17
`parse_fault` is a helper rather than a field on `LoadOutcome`, which is
`Copy` and crosses several call sites that only want the verdict; this is
asked once, by a diagnostic, about a file already on disk. The window's
notice appends the same detail on its own line — deliberately after the
translated sentence rather than inside it, because serde's message is English
that is not ours to translate and a placeholder would leave a raw parser
string in the middle of a localized one. The notice's own wording needed no
change: it says "could not be parsed", which was true of all three.
$ tty7 tab rename @1 $'one\ntwo'
$ tty7 tab ls --json
"name": "one\ntwo"
A workspace or tab name is only ever drawn as a label, and gpui breaks a label
on `\n` whatever its wrapping says — the tab strip, the tab sidebar and the
switcher would each have grown a row and painted its tail over whatever sits
below. This tree has fixed that four times already: the OSC title, a filename
in the file tree, a remote name in the SFTP panel, a completion description.
Names were the surface it did not reach.
The CLI's own table folds on the way out, which is the part that makes this
plainly wrong rather than merely unhandled: the daemon was storing, and
handing to every other reader, something one of its readers had already
decided could not be drawn.
Folded in `normalize_name`, which `workspace_create`, `workspace_rename` and
`tab_rename` all pass through, so one change covers the three ways in and the
tab strip, the sidebar, the switcher and a remote client cannot disagree about
it. Same rule and same reasoning as `parse_osc_title` — control character to
space, trim afterwards because folding can leave the edges blank.
Deliberately the opposite of the choice made for a *filename*, which is folded
only where it is drawn because it is also handed to `join`, `rename` and
`remove`. A name has no second life: what is stored is what is shown.
Verified against a live daemon before and after — `alpha\nbeta` comes back
`alpha beta`, a tab named with a tab character comes back spaced.
l0ng-ai