Skip to content

ci: drop the apt build-dependency step that the runner image does not need - #514

Merged
ThomasK33 merged 2 commits into
mainfrom
ci/apt-install-retry
Nov 11, 2026
Merged

ThomasK33 merged 2 commits into
mainfrom
ci/apt-install-retry

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Why

CI's "Install build dependencies" step failed twice on 2026-11-11 when the Ubuntu mirrors could not be resolved: run 43034439048 (merge group for #513) and run 43033301588 (#485, both attempts), between 13:53 and 14:22 UTC. In run 43034439048, apt-get update printed Temporary failure resolving for every mirror and still exited 0. apt-get install then failed with exit 100. One pass of the step took 9 minutes.

The step is not needed, and it is the only reason a mirror outage breaks CI:

  • It runs before the caches restore. On a warm run nothing compiles: the mise cache restores (mise already installed, 15 tools already installed), and .luarocks already populated (run 43030815711).
  • On the runner image (Ubuntu 26.04.1), it installs only build-essential and libreadline-dev (185 kB). unzip is already there.
  • A cold run builds without both packages (below). Lua 5.5.0 builds with make guess, which does not use readline. mise prints a warning that readline line editing is missing and carries on.

What changes

The step is deleted from .github/workflows/test.yml (6 lines). Nothing else in .github uses apt. (.devcontainer/post-create.sh has the same two apt lines. It is not CI, and this PR leaves it.)

The first commit on this branch retried the install through an outage. It is superseded: its skip check never fires on this image (build-essential and libreadline-dev are not installed), a full outage would hold the job about 30 minutes, and every run would still depend on the mirrors. The net diff of the PR is the deletion.

I also checked gating the step on cache hits. jdx/mise-action (pinned 1648a78) restores its cache and runs mise install in one step. Its install: false input skips the install and also the cache save (saveCache runs inside if (install), and the action has no post step), so gating would need separate cache steps. Deleting the step needs none.

Evidence

Cold run without the step passes. Run 43038727992 (workflow_dispatch on branch scratch/apt-cold-proof-1, head ba9455f602bf7e16d577c243d685a03ab6d05849) used new cache keys (mise-cold-1, luarocks-cold-1) and a diagnostic step in place of the apt step. All jobs passed: unit-tests, covered-lock, integration-tests (stable), integration-tests (v0.8.0), Required.

What the log shows Value
image Ubuntu 26.04.1 LTS
present gcc 15.2.0, make, unzip, curl, tar
not installed build-essential, libreadline-dev
mise cache not found, so cold
mise install 15 tools installed in 31.7 s (Lua 5.5.0 built from source)
luarocks cache not found, so cold; busted, luacheck, luacov built with gcc
apt calls 0

Warm run without the step. This PR's own run 43040639103 (head 345373422bd06ae3a973e91518300bbcac8cdc3a, cache keys unchanged) passed every job: unit-tests, covered-lock, integration-tests (stable), integration-tests (v0.8.0), Required. It has no apt call and no install step. The log shows:

What the log shows Value
mise cache hit for mise-v1-linux-x64-2026.11.5-551ea187...
mise install installed 0 tools · 15 already installed
luarocks cache hit; .luarocks already populated
unit tests 1490 passed, 0 failed, 19 pending

Local gate: mise run all exit 0 (seed 1794407676, 1490 passed, 0 failed, 19 pending). actionlint and mise run format-check pass.

Risk. A future runner image without gcc, make, unzip or curl would fail a cold run at mise install or mise run setup, with a compiler or unzip error. Then the step comes back.


Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh

The step passed -o Acquire::Retries=3, which retries within seconds, so a DNS
outage at the Ubuntu mirrors failed the unit-tests job (runs 43034439048 and
43033301588). apt-get update exits 0 when a mirror is unreachable, so only the
install failed.

The step now skips apt when build-essential, libreadline-dev and unzip are
installed, and otherwise runs update and install as one unit up to 3 times,
waiting 30 s and then 90 s, before it fails with a message.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
@ThomasK33
ThomasK33 marked this pull request as ready for review November 11, 2026 14:19
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-11T14:41:59.524410Z 3453734 Manual request
🔒 Security Review ✅ Completed 2026-11-11T14:42:34.229400Z 3453734 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 745029de74

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 745029de74

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

A cold-cache run without the step passes on the runner image (Ubuntu 26.04.1,
run 43038727992): the image has gcc, make, unzip, curl and tar, mise builds
Lua 5.5.0 without libreadline-dev, and luarocks builds the test rocks with gcc.
A warm run compiles nothing. The step ran before the caches restored, so every
run depended on the Ubuntu mirrors for two packages nothing uses, and a DNS
outage at the mirrors failed the unit-tests job.

This replaces the retry added in the previous commit.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@ThomasK33 ThomasK33 changed the title ci: retry the build dependency install through an outage of minutes ci: drop the apt build-dependency step that the runner image does not need Nov 11, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 345373422b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 345373422b

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33
ThomasK33 added this pull request to the merge queue Nov 11, 2026
Merged via the queue into main with commit 687d99d Nov 11, 2026
9 checks passed
@ThomasK33
ThomasK33 deleted the ci/apt-install-retry branch November 11, 2026 15:24
hareki pushed a commit to hareki/claudecode.nvim that referenced this pull request Nov 12, 2026
…its (coder#482)

## What users see

With the snacks provider and `auto_close`, a `:ClaudeCode`,
`:ClaudeCodeOpen` or `:ClaudeCodeFocus` that runs right after Claude
exits can show the dead terminal. The scheduled close then wipes it, so
you end with no Claude and no window and have to run the command again.
Now it starts a new Claude, and a later reload (`:Lazy reload`) still
finds that Claude, so the next open does not start a second one beside
it.

The window is small. The entry must run after Claude's `TermClose`
starts and before the provider's scheduled close. These can land there:

- A `TermClose` autocmd that you defined before Claude started. Autocmds
run in definition order (I checked this on Neovim 0.12.3), so it runs
before Snacks' handler. This needs no reload.
- A `TermClose` autocmd that you defined after Claude started. It runs
after the handler.
- After a reload, any entry in the window. The adopted terminal (coder#464)
has only the earlier module copy's handlers, which clear only that
copy's `terminal`.

## Cause

Two things in `snacks.lua` went wrong together:

1. The provider forgot an exited Claude only in the `TermClose` handler
of `setup_terminal_events`. A handler cannot help an autocmd that runs
before it, and after a reload it belongs to the earlier module copy, so
it does not clear the new copy's `terminal`. Until the scheduled close
runs, `terminal:buf_valid()` is true and an entry shows the dead
instance.
2. After a reload, the provider found its Claude through
`Snacks.terminal.list()`. Snacks keys an instance by its command,
directory, env and count (`Snacks.terminal.tid`) and clears that key
when a buffer is wiped, whichever instance holds it. A Claude B that
starts while the exited Claude A waits for its close gets A's key, and
A's late wipe clears it. B leaves the list, so after the next reload an
open starts a third Claude while B keeps running. On main this already
happened for an autocmd that runs after the handler.

## Fix

- **Find the Claude by its mark, not by the Snacks list.** While nothing
is tracked, every entry takes the buffer that carries
`b:claudecode_snacks_terminal` and whose job has not ended
(`owned.find`). `Snacks.terminal.list()` is no longer asked, and there
is no registry.
- **Wrap the buffer in a Snacks window.** The adopted buffer gets a new
Snacks window (`Snacks.win` with `buf`, nothing shown yet), patched like
a window that `snacks` started. It is built from
`b:claudecode_snacks_window`, the resolved window options of the first
start, which the provider now writes as plain data. It writes two more
buffer variables next to it, `b:claudecode_snacks_auto_close` and
`b:claudecode_snacks_backdrop`, so there are three besides the mark. So
a split or a float comes back as it started, also when its window is
gone, and re-created splits get `snacks_win` and the winbar again. For a
float, the wrapper takes the Snacks backdrop window again, the one whose
id the first start stored in `b:claudecode_snacks_backdrop` (it must
still sit one z-index below the float and show a `snacks_win_backdrop`
buffer), so a hide and a show reach it and not the backdrop of another
Snacks float. The wrapper is built in the `terminal` style, so `q` and
`gf` hide the terminal as before (the generic `q` would close the
instance, and with the scratch buffer it would stop Claude). `close`
clears the mark of an adopted terminal before it stops the job: the
`TermClose` handler of the instance that started the Claude cannot see
the wrapper's `closing` flag, and would report the stop as a failure.
The entry's config applies before the ended-job check, so a configless
adoption of an older terminal (for example `get_active_bufnr`) does not
hide the dead one.
- **Bind the keys of the buffer to the wrapper.** Snacks maps the keys
of an instance to the buffer when it shows a window, and each map closes
over that instance. The maps of the first start close over the instance
that started the Claude. After a reload and a hide and a show, that
instance has a stale window, so `q` did nothing, `gf` left the file in
Claude's window, and a user's `self:hide()` key did nothing after a
float was closed and opened. The wrapper now calls `Snacks.win:map()`,
with the keys of the `terminal` style and the entry's config (functions
included, because the config is not stored). An entry without a config
(`get_active_bufnr`, which selection tracking calls on a cursor move,
and `close`) maps no key, because a map without the user's keys would
turn on a key that the user turned off, and Snacks never removes a map.
The next entry that has a config upgrades the wrapper in place
(`upgrade_wrapper`): it maps the keys, and it gives a terminal of an
older version its window options (so a float comes back at its
configured size, not at Snacks' default).
- **Track the terminal's window as main does.** The wrapper took
`win_findbuf(buf)[1]` at every entry, which takes a copy that a diff tab
shows (in another tab) for the terminal's window: after a hide in the
first tab, an open jumped to the diff tab and left the first tab without
Claude. Now the adoption takes a window that Snacks made for the buffer,
one that carries the `snacks_win` marker (the copy of a diff tab, shown
by `nvim_win_set_buf`, does not): one of the current tab first, then one
of another tab (as an instance that `snacks` started tracks the window
that it showed, in whichever tab), then an unmarked window of the
current tab (a split that an older version re-created). After that the
wrapper tracks only the window that it hides and shows itself, like an
instance that `snacks` started.
- **Write the backdrop variable at every re-creation.** Snacks makes a
new backdrop each time it re-creates a float. The variable was written
only at the first start, so after "float, reload, `:close`, open,
reload" a hide left the new backdrop on screen. `remember_backdrop` now
writes it after the first start and after each re-creation in `cc_show`,
and clears it when the new float has no backdrop.
- **Keep the anchor of a float that is relative to a window.** The
stored window options dropped `win`, so after a reload Snacks anchored a
re-created `relative = "win"` float to whichever window was current at
the adoption. The stored options keep `win` for such a float, and the
wrapper keeps it while that window exists.
- **Register the Snacks window events for a wrapped split.** A wrapper
starts without a `Win:show`, and `cc_show` re-creates a split without
one (a float goes through `Win:show`), so Snacks' resize events and its
`BufWinEnter` handler (`fixbuf`, which moves a file that you open in the
terminal's window to another window) were missing: an `:edit` in the
re-created window replaced Claude's buffer, and a toggle opened a second
split. `register_window_events` registers them once, at the wrapper's
first native re-creation of a split. It registers no `WinClosed` event:
that event is bound to the id of one window, an instance that `snacks`
started never has it for a window that `cc_show` re-created, and Snacks'
handler would move the cursor to the previous window (`wincmd p`) when
the window closes, where main leaves it to Neovim.
- **Forget an exited terminal at every entry.**
`adopt_existing_terminal()` is the first call in `open`,
`simple_toggle`, `focus_toggle`, `close` and `get_active_bufnr`. It
starts with `forget_ended_terminal()`: with `auto_close` and an ended
job it sets `terminal = nil`. This is the snacks counterpart of native's
`forget_ended_terminal()`. The `TermClose` handler is unchanged: it
forgets the instance and schedules the close.
- **No wipe inside `TermClose`.** A wipe while Neovim dispatches
`TermClose` deletes the buffer-local autocmds that remain for that
event. I first wiped in the entry (and, in a variant, in the handler).
Codex round 3 and a probe showed that a user's later autocmd, and the
provider's own handler with its exit-status log, then no longer ran.
Nothing is wiped there now: the scheduled close does it.

With `auto_close = false` nothing changes: Snacks keeps an exited Claude
in that mode by design (terminal/README.md).

### Differences from main that I kept

I measured an adopted terminal against main with probes: 52 scenarios (a
split and a float with a backdrop, `auto_close` true and false, 0 to 3
reloads, a diff in a new tab or not, a window closed and opened, an
older terminal), plus the advisory reviewer's two-tab scenarios (a hide
in one tab and an open in another, a reload in a second tab). After I
drop the thread address in an error line, the outputs are identical to
main's (window kind, size, `snacks_win`, winbar, backdrop, keys, tab of
each window, cursor after a hide, failed-exit reports) except for
differences 2 and 3 below. Three differences stay, and
`terminal/README.md` states the same three (its wording of difference 3
gets "or to show" in the skill PR, because this head is final):

1. A user's `on_win` and `on_buf` callbacks, and any function inside
`snacks_win_opts` other than a key, do not run for the wrapped window,
because a function cannot be stored in a buffer variable. Keys come from
the entry's config. Before the first entry that has a config, the user's
function keys are those of the first start.
2. The first start's `FileType` is not set again (the float that comes
back fires one `FileType` less than main).
3. A terminal that an older version started has none of the variables.
The entry's config then decides its window options, a split gets the
default winbar, a float has no backdrop of its own to hide or to show (a
float that was hidden before the upgrade comes back without its backdrop
until Snacks re-creates it), and its `auto_close` is the entry's
`auto_close`. The window options and `auto_close` come from the entry's
config until that terminal ends. The missing backdrop lasts only until
Snacks re-creates the float, which records the new backdrop.

The wrapper is a new Snacks instance. The instance that the first start
created stays alive for its `TermClose` and `BufWipeout` handlers on
that buffer.

Not changed, on purpose: a job that `close` stopped with `jobstop` reads
as ended at once, so an entry in the next moments starts a new Claude
while the stopped one dies within a few seconds. The stopped job cannot
resume, so you get one live Claude. I also did not find a way to make
the key clash happen from `close`'s own wait, and without the Snacks
list it cannot matter.

I checked the native provider for the same shape (a handler of the
earlier copy that clears only its own state). It has none: coder#464 adds the
`ClaudeCodeNativeAdopted` handler, and native's entries run
`forget_ended_terminal()` first. A real-Neovim probe with a `TermClose`
autocmd that calls `open`, `ensure_visible`, `simple_toggle`,
`focus_toggle` or `close` gave the same result with and without a
reload, for `auto_close` true and false. So no native change.

## How I found it

A TLA+ model of the terminal lifecycle (`verify-terminal-lifecycle`, a
separate PR that waits for this one) broke `OpenShowsLiveClaude` on
main: open, reload, open (adopts), Claude exits, open. The verifier of
this PR found the key clash, and Codex round 3 found the two ways that
closing earlier goes wrong. I replayed all of them on real Neovim
0.12.3. Claude is a stub script that exits after 0.5 s.

For each of `open`, `ensure_visible`, `simple_toggle` and
`focus_toggle`, and for a `TermClose` autocmd defined before and one
defined after the handler (8 scenarios, 16 checks), the replay runs:
open, reload, adopt, Claude exits, the entry runs, reload, open. The
promise: exactly one Claude runs, and it is the one that the entry
started. A second replay runs the two Codex round-3 scenarios
(`snacks-exit-handlers.lua`, 4 checks), and a third compares an adopted
split and float with the first start (`snacks-reload-adopted.lua`, 8
checks, with termguicolors on so that Snacks makes a backdrop, and a
second Snacks float with its own backdrop open on top of Claude's): (1)
the entry runs inside `TermClose`, and a later buffer-local autocmd and
the provider's exit-status log must still run. (2) The plugin reloads,
no entry adopts the Claude, it exits, an entry in the gap starts a new
one, the plugin reloads, open: one Claude.

| Code        | 16-check replay | 4-check replay | 8-check replay |
| ----------- | --------------- | -------------- | -------------- |
| main        | 16              | 1              | 0              |
| 1994544     | 8               | not run        | not run        |
| 2b2b399     | not run         | 3              | 0              |
| 6d8b514     | 0               | 0              | 3              |
| ecb636d     | 0               | 0              | 3              |
| 45c5c04     | 0               | 0              | 0              |
| this branch | 0               | 0              | 0              |

The table counts the checks that show a finding. The seven replays that
the skill PR adds for the wrapper (`snacks-reload-keys` 12 checks,
`-tabs` 6, `-backdrop` 9, `-older` 3, `-edit` 4, `-anchor` 1, `-focus`
6) show 6, 4, 4, 1, 0, 0 and 0 findings on 45c5c04, the head that the
first two verifiers rejected. `-edit` and `-anchor` show 1 finding each
on cff28b7, the head of round 7, and `-focus` shows 2 on 5c092d2, the
head of round 8. All seven show 0 on this branch and on main. The
8-check replay finds no bug on main: it guards the adopted window. It
found 3 problems in 6d8b514 (a backdrop left after a hide, and a
failed-exit report after a reload and a close, for a split and a float),
which I fixed after Codex round 4. With the other float open, it found 3
problems in ecb636d (the other float's backdrop hidden instead of
Claude's, a `q` key that closes, and the other float's backdrop deleted
by `close`), which I fixed after round 5.

The other snacks replays (`snacks-reload`, `snacks-reentrant-open`,
`auto-snacks-later`) and the six native replays show no finding either.
The replay scripts go into the skill PR. The skill is not in this PR, so
no gate here runs its model. I ran it by hand: on the exact tree of
`f18407a` with the skill branch at `f601c2d`, `check.sh` reads 32 of 32
facts TRUE and the model passes (258,982 distinct states), but only
after a scratch `--record` (the skill's own `covered.lock` is stale
against this tree, and the skill branch needs a sync with main before it
passes its own gate, which the skill PR does after this PR merges). The
`--mutants` run, which caught every guard, was on skill commit `c2edd8d`
merged with fix commit `afba2c3`. I did not rerun `--mutants` on later
heads. `check.sh` on the exact tree of `f18407a` reads the same 32 facts
TRUE.

## Review record

- **Codex rounds 1 to 6** (heads up to 45c5c04): every finding was fixed
or answered, and round 6 was clean on both lanes (code and security) at
45c5c04.
- **Two independent verifiers rejected 45c5c04.** Each ran the head on
real Neovim and found broken core behavior that this PR introduced:
after a reload, a hide and a show, `q` did nothing and `gf` opened the
file in Claude's window, because the keys of the buffer closed over the
first instance (also a user's `self:hide()` key after a float was closed
and opened). With a diff tab that shows the terminal, a hide in the
first tab and an open jumped to the diff tab. A re-created float left
its new backdrop on screen after the next reload. Smaller findings: an
older float came back at Snacks' default size, a toggle in a diff tab
hid the copy, and a missing test for the backdrop variable and for the
key strip. This head fixes all of them.
- **Review-cap waiver:** the maintainer allowed rounds 7 and 8 for these
fixes, for blockers only
([comment](coder#482 (comment))).
A new security flaw, data loss or broken core behavior stops the PR and
goes back to the maintainer.
- **Codex round 7** (cff28b7): CI green, security clean, and two P2
findings that I reproduced: the lost anchor of a `relative = "win"`
float, and the missing window events of a wrapped split. This head fixes
both (replies on the threads). Round 8 is the last the waiver allows.
- **Codex round 8** (5c092d2): both lanes clean. CI was red on that
head: coder#447, merged into main after cff28b7, added
`tests/integration/diff_terminal_float_reload_spec.lua`, whose stub of
Snacks has no `Snacks.win`, and this PR's wrapper calls it after a
reload (6 of 9 cases). Real Snacks has it, and a probe on real Snacks
passes 9 of 9. This head gives the stub a `Snacks.win`.
- **Two verifiers on 5c092d2:** the gpt verifier passed it. The opus
verifier found no product failure, and four things that this head fixes:
a mutant that kept the terminal style's filetype in the upgrade of a
wrapper passed every test (the upgrade now has an assertion), the first
hide after a hide and a show of an adopted split moved the cursor to the
previous window and not where main leaves it (the wrapper registers no
`WinClosed` event now), one README sentence said that a wrapper never
runs `Win:show` (a float does), and the upgrade happening once had no
test.
- **Round 9 waiver:**
[comment](coder#482 (comment)):
one more push and one Codex round (round 9), for the test-only CI fix
and the verifiers' findings
- **An advisory review of my first fix** (a clean-context agent, before
this push) found two regressions in it, and `afaf05c` fixes both: a
wrapper that an entry without a config made (selection tracking runs
`get_active_bufnr` on a cursor move) turned on a key that the user had
turned off, and a Claude window in another tab was no longer tracked.

## Tests

`tests/unit/terminal/snacks_reload_spec.lua` goes from 27 to 92 tests.
The mocks changed: they keep Snacks' keyed list and clear the key on a
wipe, `Snacks.win` exists (an instance for an existing buffer, with no
buffer or window of its own, a close that wipes only its scratch buffer,
`keys` and a `map` that records which instance mapped the buffer, like
Snacks), windows belong to tabs (`nvim_get_current_tabpage`,
`nvim_win_get_tabpage`) and the first start's window carries Snacks'
`snacks_win` marker, a map only adds or replaces a key (Snacks never
removes one), `vim.tbl_deep_extend` merges maps deeply, `vim.bo` raises
for an invalid buffer like Neovim (before, it returned channel 0 and hid
a `nil` buffer), and `snacks_close_spec.lua` mocks `jobpid` and raises
the same way.

The 65 new tests cover:

- `open`, `simple_toggle` and `focus_toggle`, each with and without a
reload, with Snacks' handler run or not yet run (12).
- A terminal that was hidden, and one that was visible, when it was
adopted (2).
- A Claude that an entry started in the gap stays findable after a later
reload, for both orders of handler and entry, with and without a reload
before the exit (4).
- The provider finds its Claude when `Snacks.terminal.list()` does not
hold it (1).
- Every entry and `close` raise no error once the exited instance is
closed (2).
- `get_active_bufnr` is nil and an open without focus starts a Claude,
which is the path of `ensure_visible` (2), and `close` stops nothing
twice (2).
- The handler leaves the wipe to the scheduled close (1), and
`auto_close = false` keeps the exited Claude (1).
- The adopted window is wrapped with the options that it started with
(1), and wiped when it closes, with and without `auto_close` (2).
- A Claude from an older version follows the entry's `auto_close` (2),
gets the default winbar, and is forgotten by the next entry's config
after a configless adoption (1).
- `close` after a reload reports no failed exit (1), the backdrop of an
adopted float is hidden and shown again, also when the float was hidden
before the reload (2), and the Claude that an entry started in the gap
is found after a later reload when no entry had adopted the exited one
(1). The adopted float does not take another Snacks float's backdrop
(1), and the wrapper is in the terminal style (1).
- **Keys (8):** the wrapper maps its own keys over those of the first
start (1), `q` hides after a hide and a show, after one reload and after
two (2), the entry's config keys are mapped with their functions (1), a
terminal of an older version gets the real keys, not a copy without
functions (1), a configless adoption maps no key (1) and takes the
user's keys at the next entry (1), and an older terminal takes the
window options of the next entry's config (1).
- **Window per tab (7):** with a copy in a diff tab, a hide and an open
put Claude back in the first tab (1), and a toggle in the diff tab hides
the tracked window and the next one shows Claude there (1). The adoption
takes the window of Snacks over a copy in the current tab (1), takes the
window of Snacks in another tab when the current tab shows none, and an
open focuses it (1), takes the window of Snacks in the current tab first
(1), ignores a copy that only another tab shows, so an open shows Claude
in the current tab (1), and takes an unmarked window of the current tab
(1).
- **Backdrop (3):** the first start writes the backdrop variable (1), a
float that Snacks re-creates writes the new backdrop, which the next
reload hides (1), and a re-creation without a backdrop clears the
variable (1).
- The stored window options win over another entry's config (1).
- **Window events (4):** the wrapper registers the resize events and the
`BufWinEnter` handler at its first re-creation of a split (1), registers
no event that is bound to a window (1), registers them once however
often it hides and shows (1), and registers nothing for a terminal that
the provider started (1).
- **Upgrade (assertions in the keys test, not new tests):** the upgrade
drops the filetype of the terminal style, and it happens once.
- **Anchor (3):** a float keeps the anchor of a `relative = "win"`
window while it exists (1), drops it when that window is gone (1), and a
float that is not relative to a window stores none (1).

Against main's `snacks.lua`, 41 of the 92 are red (32 fail, 9 raise).
Against 45c5c04, the head that the first two verifiers rejected, 17 are
red, and they are the new tests for keys, tabs, backdrop re-creation,
older-terminal options, window events and the anchor. Against cff28b7,
the head of round 7, 4 are red, and against 5c092d2, the head of round
8, 3 are red (the filetype of the upgrade, the upgrade happening once,
and the events that are bound to a window). Against my first local fix
(a777129), an advisory review found two regressions (a wrapper made
without a config turned on a key that the user had turned off, and a
window of Snacks in another tab was no longer tracked), and 3 of them
are red there. Against 6d8b514, which round 4 reviewed, 6 of the earlier
66 are red.

Mutations of the fix, each run against both terminal specs:

| Mutation | Red tests |
| ------------------------------------------------------- |
----------------------------------------------------------------------------------------------------------------------------------------------------------------------
|
| no `forget_ended_terminal()` call | 18, among them "is forgotten when
it was hidden" and "from an older version follows the configured
auto_close (true)" |
| adoption never wraps (`owned.find` result ignored) | 40 |
| wrapper without `scratch_buf` | 1: "is wiped when the wrapped instance
closes, also without auto_close" |
| `auto_close` variable ignored | 2 |
| stored window options ignored | 1: "is wrapped in a Snacks window with
the options that it started with" |
| forget without the `auto_close` check | 28 across the terminal specs,
among them "from an older version follows the configured auto_close
(false)". The other specs' fake jobs read as ended without `jobpid` |
| forget without the `owned.job_ended` check | 3, in
`snacks_close_spec.lua` (a live Claude would be dropped) |
| `if not terminal then` instead of the `buf_valid` guard | 1: "raises
no error in any entry once the exited instance is closed (reload=true)"
|
| `get_active_bufnr` without `adopt_existing_terminal` | 25, among them
"reports the terminal as active before any command runs" and "is not the
active buffer, so ensure_visible starts a new Claude" |
| the backdrop is never found | 2: "hides and shows the backdrop of an
adopted float" |
| `close` keeps the mark of an adopted terminal | 1: "is not reported as
a failure when close stops it" |
| the entry's config applies after the ended-job check | 1: "from an
older version is forgotten by the config of the entry after a configless
adoption" |
| the backdrop variable is ignored (first backdrop wins) | 1: "does not
take the backdrop of another Snacks float" |
| no `terminal` style, or its `bo` kept | 1 each: "is wrapped in the
terminal style, so that q hides it and does not close it" |
| the wrapper does not map its keys | 4: "maps its own keys over those
of the instance that started the Claude", "hides with the q key after a
hide and a show" (2), "maps the keys of the entry's config" |
| an entry without a config maps the keys | 1: "maps no key when the
entry has no config" |
| no upgrade of a configless wrapper | 2: the keys and the window
options of the next entry's config |
| no `map()` in the upgrade | 1: "takes the keys of the next entry's
config after a configless adoption" |
| the upgrade keeps the old window options | 1: "takes the window
options of the next entry's config for a terminal of an older version" |
| keys copied through `plain` (functions dropped) | 3: the real keys of
an older terminal, of the entry's config, and after an upgrade |
| no keys at all | 3 |
| keys of an older terminal stripped | 1: "maps the real keys of a
terminal that an older version started" |
| `win_findbuf` scan at every entry (the old behavior) | 3: the two
diff-tab tests and "ignores a copy that only another tab shows" |
| the adoption ignores the `snacks_win` marker | 2 |
| an unmarked window of another tab counts | 1: "ignores a copy that
only another tab shows, so an open shows Claude in the current tab" |
| no unmarked window of the current tab | 2, among them "takes an
unmarked window of the current tab" |
| the current tab ranks before the marker | 1: "takes the window of
Snacks over a copy in the current tab" |
| another tab ranks before the current one | 1: "takes the window of
Snacks in the current tab first" |
| no `remember_backdrop` in `cc_show` | 2: "remembers the backdrop that
Snacks makes when it re-creates a float" and "clears the backdrop ..." |
| `remember_backdrop` never clears | 1: "clears the backdrop that it
remembers when Snacks re-creates the float without one" |
| no `remember_backdrop` at the first start | 1: "writes the backdrop of
a float that Snacks made at the first start" |

| no event registration for a wrapped split | 2: "registers the Snacks
events and the BufWinEnter handler of the wrapper" and "registers them
once" |
| the upgrade keeps the filetype of the terminal style | 1: "takes the
keys of the next entry's config after a configless adoption" |
| the configless flag stays set after the upgrade | 1: the same test,
which counts the instances that `Snacks.win` built |
| the wrapper registers the events that are bound to a window | 3:
"registers no event that is bound to a window" and the two tests that
count the registered events |
| events registered for every instance | 1: "registers no window events
for a terminal that the provider started" |
| events registered at every re-creation | 1: "registers them once,
however often it hides and shows" |
| no `BufWinEnter` handler, or no `_on` of the events     | 2 each |
| the anchor is never stored, or always dropped | 1 each: "keeps the
anchor while that window exists" |
| the anchor is stored for every float | 1: "stores no anchor for a
float that is not relative to a window" |
| the anchor is never checked against the window list | 1: "drops the
anchor when that window is gone" |

`close` forgets an exited terminal whether or not the forget runs first,
so skipping the forget there changes nothing a test can see. The close
tests check the end state.

## Checks

- `busted tests` on the final head: 1390 successes, 0 failures, 0
errors, 19 pending. The file set of `mise run test` (every `*_spec.lua`
and `*_test.lua`, seed 4247): 1555 successes, 0 failures, 0 errors, 19
pending. The integration runner
(`scripts/run_integration_tests_individually.sh`) passes on Neovim
0.12.3 (311 successes) and on a 0.8-era build (224 successes), with no
failure. The stub gap that failed CI on 5c092d2 is fixed in
`diff_terminal_float_reload_spec.lua` (the stub of Snacks gets
`Snacks.win`, built by the same function as `terminal.open`).
- Real Neovim 0.12.3 and snacks.nvim 882c996c, with the probes of both
verifiers and of an advisory reviewer: `q` after a reload and a hide and
a show 0 of 12 broken (45c5c04: 4 of 12), a user's `self:hide()` key 0
of 12 broken (45c5c04: 4 of 12), a float re-created before a reload 0 of
9 broken, the two-tab hide and open stays in the first tab, an older
float keeps its configured size, a key that the user turned off stays
off after a cursor move that adopts without a config, Claude in another
tab is found, `:edit` in Claude's window after a reload and a hide and a
show moves the file to the other window (cff28b7 replaced Claude's
buffer), and a `relative = "win"` float keeps its anchor (cff28b7
anchored it to the other window). The 52 matrix scenarios, and the
verifier's lists (46 window and diff, 28 operations, 32 exit scenarios),
equal main's except differences 2 and 3, and the cursor after a hide in
the layout `a.txt | b.txt | Claude` is where main leaves it (`b.txt`)
after 0 to 2 reloads.
- coder#447 and coder#504 (a diff hides a floating Claude terminal, only when the
float is in the diff's tab) merged into main after cff28b7, and
`snacks.lua` and `terminal/README.md` merged without a conflict
(`8527a65` merged main at d10fecf, and `f18407a`, this head, merges main
at b1a5e55: coder#513 and coder#514, which change only the workflow and a luacov
spec). A probe on the merged tree (float, 0 to 2 reloads, the diff's
`get_active_terminal_bufnr` and `hide_float`, then a toggle through the
plugin or through the original Snacks instance, then `q`) gives the same
result as main: the float and its backdrop hide, show and hide again.
Only `covered.lock` conflicted. I recorded it on the merged tree, and
`verify-diff-lifecycle/check.sh` passes (89,205 states).
- `luacheck lua/ tests/ --no-unused-args --no-max-line-length`: 0
warnings, 0 errors. `stylua --check lua tests` and prettier on
`terminal/README.md`: clean.
- `terminal/README.md` documents the mark-based adoption, the three
buffer variables and the mark, the keys of the wrapper, the window
choice, the window events, the anchor, the entry check, why nothing is
wiped there, and the three differences above.

---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant