A drag preview exists to make the strip look exactly like the list already
reordered. Every test here samples that with a few pixel values read off one
particular layout, which pins the arithmetic without ever saying what the
arithmetic is for — and the case where reading numbers off by hand stops
being convincing is chips of different widths, which is the case a tab strip
actually has.
So this builds the reordered list, lays it out from nothing, and requires
the displaced positions to be that layout: eight shapes, even and uneven,
two to six chips, three gaps, every (from, target) pair — 1,968 positions.
Caught: dropping the gap from `shift`, and narrowing the range of slots that
step aside by one. Not caught, and correctly so: adding `from` back into the
crossed range changes nothing, because the filter on the next line removes
it again. That `+ 1` is redundant, not load-bearing.
`read_file` refuses a file larger than a frame on its metadata, then reads
through `take` and refuses again on what came back. The second refusal is
what makes the *read* safe, so the first looks like a fast path — and it is,
except for one thing it was also quietly doing: `Vec::with_capacity` was
reserving `meta.len()` verbatim.
`name` arrives in an escape sequence, so that length is an attacker's number
and costs nothing to make enormous — a sparse file is one `set_len`. Between
the two caps, a file that is merely *named* could still ask for its own size
in memory before a byte of it was read. Reserving `min(len, MAX_IMAGE_BYTES)`
makes the bound the allocation's own, so it survives whatever happens to the
check above it.
The new test pins the refusal of an oversized file (sparse, so it costs
nothing) and that a small one is still read. It does not distinguish the two
caps — with the metadata check removed it still passes, because `take` gets
there — and that is said here rather than implied, since the allocation is
the part no test can see.
Two conditions whose second half no fixture reached, both guarding a step
that cannot be taken back.
`hand_over` hangs every native-SSH pane up before it execs, on the promise
that this process is about to stop existing. The checks it makes first are
all that stands between a bad `$0` and panes destroyed for an exec that
never happens. A directory is the bad path that looks most like a good one —
0755 is what `mkdir` gives you, so the mode bits alone say yes — and an
ordinary file with no execute bit is the other everyday way `$0` stops being
a program. Neither had a fixture. With either half dropped the test now
fails, and it fails *at the exec*, with the panes already hung up: exactly
the sequence the guard exists to prevent.
`check_branch` rejects control characters, and every bad-name fixture was
punctuation. A newline is the one that costs: `for_each_ref` and the log
parse are line- and record-oriented, so a ref carrying one arrives as two
and the second half is whatever was written into the name. An ESC is the
other — a branch name is drawn in the SCM panel and echoed into a pane, and
an escape sequence that reaches either is the terminal's to obey. Newline,
carriage return, ESC, NUL and DEL are all fixtures now.
`glob_match` decides which `known_hosts` line speaks for the machine being
connected to. A pattern that matches one host too many trusts a key for a
host it was never written for; one that matches too few sends someone
through a host-key warning they should never see and teach themselves to
click past. It is also the shape of code whose bugs hide in the cases nobody
writes a fixture for — a trailing star, two in a row, a star that has to
give ground more than once.
Checked now against the obvious recursive definition, over every pattern of
up to five characters from `a b * ?` against every text of up to four from
`a b A`: 165,165 pairs, exhaustive rather than sampled. `A` is in the text
alphabet and not the pattern's, so case folding is exercised on its own
rather than only where both sides happen to agree. Runs in 50ms.
A second test anchors it to the shapes a real file carries, including the
one that matters most: `*.example.com` must not match `example.com.evil.net`.
Both mutations tried are caught. Dropping the trailing-star sweep fails on
`*` against the empty string. Dropping the one-character advance the star
makes when it gives ground does not fail the suite — it hangs it, the
matcher never advancing; recorded here because a reader looking at a timeout
should know it is this line.
Both revocation tests ask about the host the `@revoked` line names, so the
host check in that condition was never the half deciding anything. Dropping
it makes every `@revoked` line in the file revoke for every host: a key
retired at one name and still in service at another is refused, and refused
with the one message a user is meant to take seriously rather than click
through.
Fail-closed, so no one is let in who should not be — but a wrong "revoked"
is a connection that cannot be made and an alarm that means nothing, which
is its own way of being unsafe. OpenSSH scopes `@revoked` to the host too.
`scrollback` writes a pane's output through a 0600 open. The history — the
commands someone actually typed, which is where a token pasted onto a
command line ends up — was opened with no mode at all, so it landed at
whatever the umask allowed, usually 0644.
The config directory around it is closed, so this is the second lock rather
than the first. But a directory is not the file's own lock: it does not
travel with the file when it is copied out, and it is one `chmod` away from
not being there. The two call sites of `ensure_private_dir` that handle its
error both refuse to write rather than lean on the directory; this file
leaned on it.
Existing history files are tightened on the way past, since `mode` only
decides what a *new* file gets — the same second pass `ensure_private_dir`
already makes over a directory it did not create. Both halves are kept: the
creation mode closes the window in which a new file is world-readable until
the chmod lands, which a test that looks at the end state cannot tell from
the chmod alone. That is written down where someone would otherwise delete
it as dead weight.
Every fuzzy list in the app — the palette, reverse search, completion
filtering — is ordered by one dynamic program, and a dynamic program is the
kind of code that can be slightly wrong for a long time without looking
broken: it keeps returning *a* match, so the feature works, and the list is
merely ordered a little worse than it should be. The existing tests are
examples of that shape, which cannot tell a suboptimal answer from the best
one.
This lays every possible placement of the term over the line by hand and
takes the best, for eight thousand short lines. The alphabet is five
characters including two that earn a boundary bonus, so ties and boundaries
come up constantly rather than by luck.
The positions are checked as well as the score, and against the score: they
must be in order, must actually match the term, and must add up to exactly
the number reported. They are what the UI underlines, so a backtrack walking
the wrong parents would highlight characters that had nothing to do with the
ranking — which no score-only check would notice.
Confirmed to bite: moving the gap origin by one column fails it, and so does
inverting the choice between starting a gap and extending one. Flipping the
`>=` that breaks ties between staying adjacent and crossing a gap does not,
and should not — both paths score the same, and the test asserts the
positions add up rather than that they are one particular optimum.
The prompt takes a directory name and a branch, and either alone is enough
because each falls back to the other. That rule was written out three times:
once to decide whether Create is live, once to build the path the card
previews, and once in `submit_worktree_prompt` to build the request.
Nothing held two of them. Dropping the branch half of the button's condition
greys out Create for a user who typed only a branch — a case the fallback
exists to support — and the preview and the request were free to drift into
describing different worktrees, which is worse than previewing nothing: the
card would name a directory that the click would not create.
One function now, unit-tested for each of the four field combinations and
for whitespace in either field, which is not an answer and must neither
light up Create nor put a directory named " " in the preview.
Also adds the switcher's missing arrow case: Ctrl+arrow was tested, but the
secondary modifier is a different key on macOS and ⌘↑/⌘↓ are the system's.
`key_intent` parked them correctly and nothing checked it.
Completion depends on `quote_for_shell` and `unquote_word` being inverses —
it inserts a quoted candidate, and the next Tab reads that word back off the
line to resolve against the filesystem. A path that does not survive the
round trip is a directory the second Tab cannot enter.
Each dialect had examples going one way; nothing asserted they compose.
Twenty-six paths through all three now do: spaces, both quote characters,
backslashes, `~/` prefixes that have to stay outside the quoting, the POSIX
`'\''` seam, the PowerShell doubled-quote seam, empty and whitespace-only
words, shell metacharacters, a Windows drive path, and non-ASCII.
One case does not round-trip and is asserted as it stands: `a"b` under
`Quoting::Cmd`, where the wrapper has no seam for a literal double quote.
Cmd is only reached for a Windows shell, and Windows forbids `"` in a file
name, so there is no such path to lose — the fix would be a quoting rule for
input that cannot arrive. Written down here so the limit is met in the test
rather than in a bug report.
Tab types in the prefix every candidate shares. `complete_path` stops
reading the directory at MAX_CANDIDATES, and a directory hands its entries
over in no order at all — so past four hundred matches, what survives is an
arbitrary slice, and the prefix that slice happens to share need not be the
prefix every match shares.
Five hundred `data-NNNN.csv` beside one `data.json` is the shape of it. The
four hundred the scan reached all share `data-0`, so `data` + Tab typed in
`-0`, and `data.json` was gone from the line — a file that was there all
along, now reachable only by deleting what Tab had just written, with
nothing on screen to say a candidate had been dropped. Which four hundred
got read decides whether it happens at all, so it comes and goes between
runs on the same directory.
The prefix is now refused whenever the list stands at the cap. A list of
exactly four hundred that really was complete loses the courtesy too; that
is the side to be wrong on, because the cost is a Tab that types nothing
against a Tab that types something untrue.
The cap itself stays: completion runs on the UI thread, and reading a
hundred-thousand-entry directory to be sure of a prefix would trade this for
a worse problem.
`delete_profile_confirmed` releases the keychain entries a profile was the
last thing pointing at — a password, and any passphrase for a key no
surviving profile still lists. Both calls threw the result away.
The store returns `Ok` when there was nothing there (`NoEntry` is mapped),
so an `Err` is a real refusal: a locked keychain, or a user who dismissed
the authorisation prompt. When that happened the profile was deleted anyway
and the secret stayed behind — permanently, because the profile that reached
it is exactly what just went away, and "Forget password" lives on the menu
that no longer exists. That is the stranding the two comments in this
function were written to fix; they fixed never asking, not being refused.
Every other caller of these two already says when the store refuses —
`ssh_prompt` logs both, and the standalone "Forget password" returns a
message either way. This one now does too: it logs, and hands its caller
what could not be finished, which goes to the window the prompt belonged to
as a notification.
`SettingsCouldntForgetPassphrase` is new and names the key by path; the
existing password message already had a key. Placeholders match across all
three locales, which the i18n guards check.
`quote_range` refuses outright when the clicked apostrophe is a contraction,
and no test ever clicked one that had anything to pair with. The parity
count already skips contractions, so on a line carrying no real quote there
is nothing to pair with and the refusal looks redundant — every fixture was
that line.
`echo 'it isn't so' done` is not. Dropping the refusal makes a click inside
`isn't` select `'it isn'`: a range that opens at a quote and stops in the
middle of a word. The fixture was already there; only the interesting click
was missing.
`<` only pairs when neither end is padded — a generic or a tag, not a
comparison. Every negative fixture put a space on both sides, so either half
of that rule threw them all out and neither half was ever the one deciding.
Dropping the opening check left the whole suite green while `if a < b, x>y`
started selecting `< b, x>` on a click, and dropping the closing check did
the same for `echo <a >b`.
Two lines with a space on one side only, so each half has a case that is
its alone.
The bar draws in cells and edited in `char`s. Everything about the drawing
side is cluster-aware — `input_cells` groups a base with its marks, the
wrapping keeps a cell whole, an emoji presentation sequence is re-scored to
two columns — and none of that reached the editor, where Backspace, Delete
and the arrow keys each stepped exactly one `char`.
So one Backspace on `❤️` left `❤`: the variation selector went and the heart
stayed, as a monochrome glyph nobody typed. Left or Right parked the caret
between a base and its mark, where no glyph begins, and the next character
typed landed inside someone else's. Thai tone marks, Devanagari, decomposed
accents and every multi-scalar emoji behaved the same way.
Latin hides this because macOS composes accents on the way in — but nothing
composes an emoji sequence, and NFD is exactly what the filesystem hands
back for a pasted path.
The boundary is now stated once, as `opens_cell`, and both sides use it:
`input_cells` groups by it and `cmd_editor` steps by it. A character opens a
cell unless it is zero width and has something other than a newline in front
of it to ride in — so a mark stranded at the start, or left alone after a
newline, is still a character in its own right, which is what the bar
already drew. `set_cursor` snaps to a boundary, so a click or a vertical
move cannot leave the caret inside a character either.
All nine existing cell tests pass against the shared rule unchanged.
A fifth site asking the same question the wrong way, missed on the first
pass because it spells the phase check inline rather than through the shape
the others used.
`connected_ssh` decides whether a pane gets the port-forwards panel, and it
asked the phase alone. So a pane whose SSH client had already exited still
offered to add forwards — over a link with nothing at the far end to carry
them, on a panel whose whole subject is a connection that is gone.
Now the same `ssh_session_live()` as the other four.
Checked the remaining phase readers while here: the status strip and the tab
dot both go through `ssh_disconnected()` and already account for it, and
`ssh_prompt` only records the phase for display.
Four places asked "does this pane have a live SSH session", and two of them
had already drifted to asking only half of it.
The phase is a report from the daemon's status stream, not a state the app
maintains. Nothing walks `Connected` back when the pane's process ends — an
`exit` on the far side, a dropped link, a killed client all leave the last
thing anyone said standing. So the phase alone answers "was it ever
connected", and the pane's own liveness is the rest of the question.
`remote_files_pane` and the native-SSH action gate asked the phase alone.
Both therefore went on working over a link that was gone: the remote file
browser offered itself for a dead session, and the SSH actions stayed live
on a pane whose client had already exited. The close warning and the
in-use-profile scan had the full condition and behaved correctly, which is
what made the drift invisible — the feature that was right and the feature
that was wrong looked the same in every test.
Now one method on `TerminalView`, called by all four, with a test that runs
a pane from connected to exited and checks the phase still reads
`Connected` — the trap itself — while the features gated on it turn off.
`ForwardFields` exists, by its own doc, so "do these make a rule?" can be
asked without a `Window` and answered the same way in both places that need
it. "Should the form be saying what is still missing?" is the same kind of
question and was being answered in the render instead, as a pair of halves
neither of which was held.
Dropping the blank half puts an error message under a panel nobody has typed
into yet — the exact greeting the check was added to prevent. Dropping the
other half leaves it up after the form is valid. Nothing failed either way.
Now a method beside `collect` and `is_blank`, with a truth table over it:
untouched, whitespace-only, half typed, finished, and dynamic — where the
same fields that are half typed for a local forward are already a rule.
The elevation rule was spelled out twice — once to decide whether to offer a
"next launch" button, once twenty lines later to decide whether answer index
1 meant that button. With elevation in play there is no such button and
index 1 is "Later", so the two must agree exactly.
Nothing held the second copy. Dropping `!needs_elevation()` from it turned
every "Later" on an elevation-needing update into a staged install: the user
declines, and the app schedules the thing they declined for the next launch
— the one install it already knows it cannot carry out unattended, which is
why the button was withheld in the first place (#504).
Derived once now and read in both places, so the reader cannot believe in a
button the layout did not draw.
Two gaps of the same shape — a compound condition whose second half no
fixture ever reached.
`segment_row` asks whether the cell after a marked one is a wide char's
spacer, and that is the only read in the loop that indexes the row directly
instead of going through `get`. Every mark fixture put something after the
marked cell, so the bound was never the thing under test. A mark in the last
column — an accented letter typed at the right margin — panics without it,
mid-frame, taking the window with it. Now covered for the wide-spacer
lookahead, the sara-am lookahead beside it, and a row of exactly one cell.
`ImageStore::place` treats identity as the (id, placement) pair, because
kitty lets one image be shown in several places at once and a shell drawing
the same icon down a column sends exactly that. Every fixture used placement
0, so matching on the id alone passed everything: an image shown three times
would have rendered once, at the last spot it was sent to. The `placed(id,
placement)` helper already took the parameter — nothing had varied it.
The placement half of that condition turned out to be held already, by the
delete-and-retire tests. Only the id half was open.
`cmd_finished` and `turn_finished` were each a verbatim restatement of a
condition the `match` five lines below already decides. Two spellings of one
rule, free to drift the moment either is edited — and the drift would show
up not here but downstream, where both flags drive the source control edge
refresh.
Neither copy was held by a test. Weakening `cmd_finished` to `!running`
fires the edge on every poll of an idle pane, so every pane re-reads git
status continuously while nothing is happening; weakening it the other way
fires it throughout a command and never at the boundary, so the working tree
a command just changed is the one state never picked up. Nothing failed for
either.
Setting the flag inside the arm makes both unrepresentable rather than
merely detectable: there is no second condition left to get wrong, and the
arm itself is held by the notification tests.
`turn_finished`'s other half was already unreachable — `poll_agent_status`
returns early when the status has not changed, so `status == Done` implies
`prev != Done`. Now stated once, where that is evident.
`usable` asks two things of an existing path — not a directory, and carrying
the executable bit — and only the mode half had a fixture. A directory has
that bit as a matter of course, 0755 being what `mkdir` gives you, so
dropping `!stat.is_dir` left every directory reading as an installed server.
The installer would skip the install and hand `ensure_daemon` a path nothing
can exec: the remote never comes up, says nothing useful about why, and
never tries again, because as far as it is concerned the binary is there.
A directory at that path is not exotic — an interrupted install, an `scp -r`
aimed a level too high, or a hand-made `~/.local/share/tty7` all leave one.
The same predicate guards `published_binary_serves_us`, and there it is
deliberately not asserted: dropping it falls through to a probe that execs
the path, which fails on a directory and reaches the same answer a round
trip later. Equivalent in production, so a test would pin the fake rather
than the behaviour.
Also considered and left alone: `binary = added == "-" && removed == "-"` in
the numstat parse. Git emits the two dashes together or not at all, so one
without the other is input that cannot arrive.
Two compound conditions where one half decided every existing fixture, so
the other was never asked anything.
`remote.rs` exempts a frame larger than the whole backlog bound — a paste is
whatever the clipboard holds — but only onto an *empty* queue: a second one
arriving before the first has moved a byte is a link that has stopped
reading, and letting it through too trades the bound for a heap that grows
five megabytes at a time. The existing paste test drains the peer, so the
bound is never reached and the empty-queue half never matters. Dropping it
left a stalled link accepting oversize frames forever, and nothing failed.
`log.rs::load_page` calls a page complete only when git answered with fewer
commits than asked for *and* the parse read everything git answered with.
The truncation half had a test — of `parse_log`, whose doc comment promises
`load_page` turns the flag into `complete: false` while asserting only the
flag. Nothing checked the promise. A graph cut at MAX_RECORD would have read
as the end of history and frozen paging there, the commits below it
unreachable with nothing on screen to say why.
Both tests fail against the mutation and pass against the code as written.
No production change: the conditions were right, only unheld.
`release` and `release_conn` are two operations on purpose: one lets go of
a single workspace, the other of everything a connection holds when it
disappears. Dropping the workspace half of `release`'s filter collapses
the first into the second, and the suite stayed green.
What that costs: a client detaching from one remote workspace silently
loses its hold on all of them, and the far end considers them free for
somebody else to take. The connection half of the pair *was* held — only
the pairing was not, which is the shape every recent find has had.
Confirmed by dropping it: the test fails on the workspace that should
still have been this connection's to release.
The head candidates for eliding include a hardcoded 6 and 3, and
`longest_tail` computes `cells.len() - head_n`. On a label with fewer than
six clusters that subtraction underflows, so the filter keeping a
candidate within the label is the whole of what stands between a narrow
tab and a crash.
Nothing reached it. The long-branch cases have clusters to spare, and
"main" returns early because it fits — a label has to be *both* short and
too wide, which is an ordinary tab in a narrow column.
Confirmed by dropping the bound: the test panics at the subtraction rather
than failing an assertion, which is what the guard was there for.
`short_rev` cuts a rev to eight characters only when it is forty-plus
characters *and* all hex. Every case in its test is shorter than forty, so
the length half decides them alone and the hex half never gets a say.
A ref name that long is the only input that asks it anything, and they
exist: `origin/feature/a-thoroughly-descriptive-branch-name` is fifty-one.
`DiffSource::Range` puts two revs in one header, so losing the hex test
turns a branch comparison into "origin/…origin/".
Also pinned: an oid-length string with a single non-hex character is still
a name, which is the boundary the `all` is there to draw.
`file_candidate_at` decides what text in the grid is clickable, and every
existing case walks the happy path — a path-shaped token clicked in the
middle of itself. Two of its arms could be removed with the suite green:
The trailing colon a compiler prints before its message. `src/main.rs:` is
not a filename, and a candidate that keeps the colon names nothing, so the
text simply never underlines.
And that the span a candidate reports contains the click. Trimming moves
the edges inward, so a click on punctuation that was trimmed away is a
click on nothing.
Two neighbouring guards are deliberately not asserted. Mutating them
survived, and checking why showed both to be equivalent mutations rather
than gaps: `url_span_at`'s whitespace check is a fast path, since a token
spanning a space fails the scheme test downstream regardless; and
`location.path.is_empty()` has no input that reaches it, because `":42"`
parses as the path `":42"` and a token trimming to nothing is refused a
line earlier. A surviving mutation is a question, not a finding, and the
answer here was no.
`expand_home` has three arms and only the middle one was covered. A bare
`~` is a link to the home directory — nothing requires a file of it, since
`require_file` is false without a line number — and `~\` is the same path
where the separator is a backslash.
Dropping either left the whole suite green while the link resolved to a
literal `~` joined onto each root in turn, which names nothing, so the
click would find no file and the text would never underline.
Also pinned: a `~` that begins something else is not a home reference.
`~tmp` is a filename and `~user` is an expansion this deliberately does
not perform, so both must survive unexpanded — which is what keeps the
bare-`~` arm from being written as a prefix match.
`reclaims_due` says what lands in it: "a Take Back sits in `reclaiming`
until an attach answers for it", *and* a workspace nobody has attached
over the link that is up now. Two reasons joined by `||`, and only the
second was held.
Dropping the `reclaiming` half left the suite green while Take Back did
nothing for any workspace this client had already sent an attach for —
which is every workspace it is currently showing, so the feature would
have been inert exactly where it is used.
The second of the two gaps the fixed mutation harness turned up, and like
its neighbour it cannot be reached by driving the app: a remote link
cannot be opened from here at all, so the test is the whole safety net.
`link_alive_or_connecting` decides whether a machine entry survives its
SSH profile being deleted, and says why: the connection holds an
authenticated spec rather than a profile reference, so forgetting the
entry "would release the link under any window still attached to it".
It is two halves and only one was held. Dropping the `LinkState::Connecting`
arm left the suite green while a profile deleted mid-connect pulled the
link out from under a window that was in the middle of getting it.
That half cannot be reached by driving the app from here: opening a
remote link is GUI-only and `tty7 machine connect` reports itself
unimplemented, so the test is the entire safety net for it.
Found only after fixing the harness. A restore had failed silently
earlier — the replacement's anchor was no longer unique and the error was
suppressed — so a mutation stayed in the file and every later round was
"caught" by the leftover rather than by its own change. Re-run against a
verified-clean baseline, two of nine results reversed. The harness now
refuses to start on a dirty tree and checks the file is clean again
afterwards.
OpenSSH reads `Host *.example.com !secret.example.com` as "everything
under example.com except that one", and people write it to keep a
production host off a wildcard's shared user and key. Nothing held the
negation at resolution — disabling it left the suite green while
`secret.example.com` picked up the wildcard's `User` and `Port`, and by
extension its identity file. The connection still succeeds, as somebody
else.
`import_skips_match_blocks_and_negations` covers what *import* makes of
such a block. This is resolution, which is the path that decides what a
connection actually uses, and the two go through different code.
Found by mutating the free predicates no test names directly. Most of
that list is covered through their callers — `is_bare` by `quote_as`,
and so on — so the list is a place to aim mutations, not a list of gaps.
Of the batch tried, this was the only one that survived: the smart-select
predicates, the drop-conflict logic that stops a copy overwriting
unasked, and `unstage_prefix`'s unborn-HEAD branch were all held.
`only_our_own_staging_directories_are_swept` covered names with nothing of
ours in them — `tty7.app`, `.Trash`. It never covered a name that
*contains* the prefix without starting with it, which is the one way the
predicate can be weakened while still looking right.
That distinction is worth holding here more than most. On macOS
`stage_roots` is the parent of the app bundle — `/Applications` for an
ordinary install — and whatever matches is handed to `remove_dir_all`.
Relaxed to `contains`, the sweep would take `my-tty7-update-backup` or
`Adobe tty7-update-helper` on the strength of a substring.
Found by mutation. Three others in the same batch were already held: the
unborn-HEAD branch of `unstage_prefix`, the `-f` that lets `rm --cached`
drop a staged-then-edited file, and the liveness check that keeps the
shell-scratch reaper off a running daemon's directories.
`tab_keeps_unclosed_work` is two halves — a tab holding an unsaved buffer,
and one holding a connection whose profile asked to be warned about — and
only the first was held by a test. Dropping the SSH half left the whole
suite green while "Close Other Tabs" began taking live sessions without
asking, which is the one thing a bulk close is documented not to do.
Nothing could reach that gate from a test before. `leaf_is_warn_ssh` reads
the pane's SSH phase, and reaching `Connected` for real wants a server, a
handshake and a channel — so a `#[cfg(test)]` seeder sets the one value
the gate actually consults, in the same spirit as
`editor_seed_dirty_file_for_test`.
Found by mutation: of the four conditions tried across `settle_save` and
this, three were already caught — a stale save marking the buffer clean, a
failed save requeueing, and a tab with unsaved edits being bulk-closed.
This was the one that was not.
`settle_census` and `sweep_parked` each built `parked ∪ spawned` from the
state, separately. I introduced that duplication myself when I added the
first of them earlier on this branch, by copying the shape of the second.
They are not interchangeable readings. One decides whether the tree is
worth pulling, the other which panes to end — so drift means either
ending a pane the census never weighed, or pulling for one the sweep will
not judge. `census_of` is now the single answer to "what is this window
still holding".
Which also made the gap visible. Mutating the census showed the `spawned`
half held by nothing: `the_census_holds_the_panes_this_window_made` read
`s.spawned` straight off the state, so it proved the set was filled in
and never that the decision reads it. Dropping `spawned` from the census
left the test green while a pane spawned and not yet in the tree went
back to being invisible to the sweep — which is the whole of the stray
shell leak that set exists to close.
It asserts through `census_of` now, on both halves, and each fails when
dropped. `stranded_of`'s two conditions were already held.
Mutating `discard_all_ops` left two conditions standing.
The `-d` on `git clean`. Porcelain v2 names an untracked directory with a
trailing slash, and that slash is the whole of how this decides whether
to recurse. Measured against real git: `clean -f` removes loose.txt and
leaves newdir/ standing; `clean -fd` takes both. Without the flag,
"Discard all" quietly leaves every untracked directory where it was —
not what the button says, and not what its own confirmation described.
And the filter that drops a path git cannot be given. A name that is not
UTF-8 has no pathspec, and `GitOp::validate` rejects an *operation*
carrying one — so leaving it in the list turns "discard everything" into
an error that discards nothing, on a repository that happens to hold one
awkward filename. Dropping it there keeps the rest discardable, and the
test asserts the ops built are ones validate accepts.
The third condition — that discard-all reaches only unstaged paths — was
already held. So were all three of `orphan_panes_of`, which decides which
panes a card offers to close.
expand_file_command_template splits the template into tokens first and
substitutes {path} inside a token, which is what keeps
`/Users/me/My Notes/a.rs` together. Substituting first and splitting
after reads like the same operation and is not — the editor is handed
`/Users/me/My` and `Notes/a.rs` and opens neither.
Nothing pinned that order, and every existing case used a path without a
space, so the invariant that makes the design correct was never exercised.
Paths with spaces are the common case on the platform this ships on:
Application Support, My Documents.
Three shapes: the plain `{path}`, a compound `{path}:{line}:{column}` where
the glue would part company with it too, and a template carrying spaces of
its own, which are still separators — only the substituted value is
protected.
Checked by writing the refactor it guards against: the path comes back as
three arguments.
`GitOp::CreateBranch` with checkout: false builds `git branch <name>`,
which the commit graph's "branch at this commit" uses — naming a place
without moving to it. That puts a name the user typed where git parses
options.
Measured against real git rather than reasoned about:
git branch --list main lists branches, exits 0, creates nothing
git branch -- --list main fatal: '--list' is not a valid branch name
An exit of 0 is what `run_op` reads as success, so the first line is a
panel reporting a branch that does not exist. `-D` and `-foo` are merely
confusing by comparison; `--help` opens the man page.
So `--` goes ahead of the name on both `branch` forms, create and delete.
Not on `checkout -b`, and that asymmetry is the point: `-b` takes the next
argument as its value whatever it looks like, so that form already
answers "not a valid branch name", and for `checkout` a `--` means
"paths follow" — a different instruction. `CheckoutBranch` already carried
a trailing `--` for its own version of this, so the class was known here;
`branch` was the form that missed it.
My first version of the test searched the argv for the name, which finds
the force flag when the two look alike — `["branch", "-D", "--", "-D"]`
is correct and it called that broken. It asserts what follows the
separator instead.
views.json is which workspaces a window has open and where they point;
session.json is the tabs and panes a cold start brings back. Both are
serde(default) throughout, so a rename does not fail — the field decodes
to its default and the windows, or the panes, are quietly not there.
Same floor as machine.json and the settings file, and the last two files
in the config directory that carry state a user would miss.
The two enums here are tagged differently and both spellings are on disk.
SessionPane is external, so Leaf and Split are keys and a walk finds
them. RemoteTarget is internal — {"kind":"direct",…} — so its variants
are values of kind and a walk over keys never sees them; those are
asserted separately. The two look alike in the source and only the
serialized document tells them apart, which is why this was read out of
serde rather than derived from the types.
Checked both paths: renaming the views field, and respelling the direct
target's kind. Each fails naming what went, and prints what is written
now.
machine.json is the user's workspaces, tabs, panes and layout. Every
field on it carries serde(default), which is what lets an older file load
— and also what makes a rename silent: workspaces under any other name
decodes to an empty Vec, so the whole tree is gone at the next boot with
nothing logged and nothing to repair from. That is a worse loss than a
forgotten setting, and it had no guard at all.
Twenty-two names, as a floor: adding a field does not fail this, dropping
or renaming one does.
The split layout's variant tags are in the list too. PaneNode is stored
externally tagged — {"Leaf":{"pane":3}} — so the variant name sits on
disk exactly as a field name does, and renaming Leaf or Split unmakes
every saved layout. The fixture builds a Split of two Leaves so both are
actually written rather than assumed.
Checked both ways: renaming the workspaces field, and renaming the Leaf
variant. Each fails naming what was lost, and prints the names now being
written so the drift is visible rather than guessed at.
The values were pinned last commit; this pins the names they live under.
A key tty7 renames does not fail to load — de_lenient leaves the field at
its default, the user's choice is gone, and the old name surfaces through
unknown_config_keys only if they run doctor and look at it.
Seventy-nine keys, recorded as a floor rather than an inventory: adding a
setting does not fail this, renaming or removing one does. The fix when
it fires is usually a serde alias, which is how RightPanelTab went on
reading configs written before it was renamed.
Checked by renaming copy_on_select the way a refactor would. It fails
naming the key and the remedy. The floor also asserts it is not empty,
since a Config that serialized to nothing would satisfy the first check
by having no keys to lose.
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.
`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.
l0ng-ai