Skip to content

test(ci): make the workflow spec read only a bare or a fully quoted key - #596

Merged
ThomasK33 merged 5 commits into
mainfrom
ci/workflow-spec-bare-key
Nov 13, 2026
Merged

ThomasK33 merged 5 commits into
mainfrom
ci/workflow-spec-bare-key

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

Why

#591 made field() in the workflow spec accept a quote around a key. Its pattern ^ *["']?([%w_%-]+)["']?: also accepts a quote on one side of the key only. YAML reads cache_save": ... as the key cache_save", so mise-action ignores it and keeps its default true, which saves the tool cache on a pull request. The spec read the same line as cache_save and passed.

The same gap hides a second jdx/mise-action step with cache': false, a cache_key": ... and a MISE_HTTP_RETRIES": "6". actionlint and zizmor pass all four.

A second gap is older and main has it too. A second jdx/mise-action step with with: and then env : or "env" : holding cache: false passes the spec. No pattern read the env line, so the spec still thought it was inside with: and counted that cache as an input. YAML puts it under env, so the action keeps its defaults and saves its cache on a pull request. actionlint and zizmor pass it.

What changed

Only tests/unit/workflow_cache_spec.lua (+99, -16). The workflows are unchanged. The spec still has no YAML parser.

  1. field() accepts a key written bare, as "key": or as 'key':. A quote on one side only is not a key.
  2. Any non-blank line at the step-key indent now sets the current block, even when no pattern reads its key. A line such as env : ends the with: block.
  3. Four new tests check how the spec reads keys and blocks, so that a full revert of these changes makes mise run test fail. A partial change to the quoted-key patterns can still pass (a follow-up PR adds a test for it). parse_workflow now takes an iterator over lines, and the tests feed it small workflow texts as inline strings. They assert that an env : or "env" : line after with: ends the block, that a line of spaces does not end it, that cache': false and cache_save": are not read as keys, and that "key":, 'key': and a bare key are read.
  4. The comments say what the spec does. The header says that full-line comments are skipped, so a comment can name an action (the one above the rock cache steps does). The comment of the line-count test says that @ or / must follow the action name in uses:. The comment above the block line says that a non-blank line opens a new block and a line of spaces only does not.

The reader reads no new YAML form. Aliases, YAML escapes and expression spellings stay under "Not covered" in #591, with their reason.

Evidence

Before is the spec of #591 (12 tests, merged as b44dab46), after is this branch (16 tests). Each mutant is the workflow of #591 with one edit that applies exactly once.

Mutant Edit Before After
SQ1 the setup step has cache_save": ${{ github.event_name == 'push' }} green red (2 tests)
SQ2 a second jdx/mise-action step has cache': false green red (1 test)
SQ3 the setup step has cache_key": ... green red (1 test)
SQ4 the setup step has MISE_HTTP_RETRIES": "6" green red (1 test)
SQ7 "cache_save: ... (a quote on the left only) green red (2 tests)
SQ5, SQ6 controls: "cache_save": and 'cache_save': green green
K30 a second jdx/mise-action step with with:, then env : and cache: false under it green red (1 test)
K31 the same with "env" : green red (1 test)
K32 the same with env": (a quote on one side only) red red (1 test)
K33 control: the same step with "env": right after with: red red

The failing tests are the ones that read the key: the cache_save, cache_key and MISE_HTTP_RETRIES tests of the setup step, and the test that every jdx/mise-action step keeps its cache off a pull request.

Counts over all 112 mutants (the 102 of #591, SQ1 to SQ7 and K30 to K32). Before, 74 are red and 38 are green. After, 81 are red and 31 are green. 74 stay red, 7 turn red and 31 stay green. K32 is red with #591's spec, which read env": as the key env. The first commit of this PR made it pass, and 36feb3b7 makes it red again. The 102 mutants of #591 give the same result as before, so no test result changed for them. The 31 green mutants are the 29 controls and known gaps of #591 and the two controls SQ5 and SQ6. SQ7 is not valid YAML, and actionlint and zizmor reject it. They pass SQ1 to SQ4, K30 and K31. actionlint rejects K32, and zizmor passes it. I also ran all 169 workflows of the verifier's table (the 112 mutants, the other probes and the base trees) with the spec of b558d22d and of 36feb3b7. 111 stay red, 3 turn red (K30, K31, K32) and 55 stay green. No workflow turns green, and no failing test changed for the red ones. I ran the 169 workflows again with the four new tests. 114 are red and 55 are green, and the result and the failing test names are the same for all 169.

The four new tests are checked against changes of the reader itself. Each change is one edit of the spec, and the spec runs on the workflow of main. Before is the spec without the new tests, after is the spec with them.

Change of the reader Before After (failing test)
S1: the block line back to if name and indent == 8 (the code of #591) green red (the env : test)
S2: the block line without its %S check green red (the line of spaces test)
S3: the key pattern of #591, which takes a quote on one side green red (the one-sided quote test)
S4, S5: no pattern for "key":, or none for 'key': green red (the quoted keys test)
S7: the loop over the key patterns without its break green red (the quoted keys test)
S8: the bare pattern takes a trailing quote green red (the one-sided quote test)
S9: the "key": pattern takes the left quote as optional green red (the one-sided quote test)
S10: the block of an entry is not compared green red (the env : test)
S6: no bare pattern red red

Each of the 10 changes fails exactly one new test, except S6, which fails 15. Without the new tests, only S6 fails.

Claim Evidence
The spec passes on main with this change 16 successes, 0 failures
The workflows are valid The workflows are identical to main. actionlint 1.7.12 on all 5: exit 0. zizmor 1.25.2 --offline on all 5: no findings (25 suppressed). mise run format-check: exit 0 (444 files, 0 changed)
Gate mise run clean && mise run all exits 0 in 495 s: 1908 successes, 0 failures, 0 errors, 36 pending (seed 1794545933). It ran on the merge of main (bcf1f084) into this branch. luacheck: 0 warnings, 0 errors in 200 files. The stub claude and GNU timeout first on PATH

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

field() accepted a quote on one side of a key. YAML reads cache_save": as the key
cache_save", so mise-action ignores it and keeps its default of true, but the
spec read it as cache_save and passed. A key must now be bare or inside a
matching pair of quotes ("key": or 'key':).

The spec comments now say that full-line comments are skipped, and that the
action name in uses: must be followed by @ or /.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
@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 13, 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-13T05:10:04.678019Z dc82fd5 Manual request
🔒 Security Review ✅ Completed 2026-11-13T05:10:26.264152Z dc82fd5 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. 🎉

Reviewed commit: b558d22d56

ℹ️ 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: b558d22d56

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.

…n the workflow spec

field() set the current block only on a line that one of its key patterns read. A
second jdx/mise-action step with "with:" and then "env :" (or "\"env\" :") put
"cache: false" under env in YAML, so mise-action kept its defaults and saved its
cache on a pull request. No pattern read the env line, so the spec still thought
it was inside with: and counted that cache as the input. Any non-blank line at the
step-key indent now sets the current block, read or not.

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

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 06e2dbf4c5

ℹ️ 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: 06e2dbf4c5

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 13, 2026
No test checked the reader of the workflow spec, so reverting the block-end line
or dropping its non-blank check kept mise run test green. parse_workflow now takes
an iterator, so the new cases feed it small workflow texts as inline strings:
- an env : or "env" : line after with: ends the block
- a line of spaces does not end with:
- cache': false and cache_save": are not read as keys
- "key": and 'key': are read, and so is a bare key

The comment above the block-end line now says a non-blank line opens a new block.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
@ThomasK33
ThomasK33 removed this pull request from the merge queue due to a manual request Nov 13, 2026
_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: dc82fd54f7

ℹ️ 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: dc82fd54f7

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

Copy link
Copy Markdown
Member Author

Maintainer decision (ThomasK33, 2026-11-13, in the agent chat): take the recommended option for this PR.

Waiver recorded before the merge: merge this PR at dc82fd54 with the round 3 verifier finding open. The finding is a missing test: three one-line changes to the key patterns keep every test green. The code on this head is correct. A follow-up PR adds the tested patch that makes those changes fail a test. This PR is at its six-request Codex cap, so it gets no further push.


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

@ThomasK33
ThomasK33 added this pull request to the merge queue Nov 13, 2026
Merged via the queue into main with commit d5bf131 Nov 13, 2026
9 checks passed
@ThomasK33
ThomasK33 deleted the ci/workflow-spec-bare-key branch November 13, 2026 10:43
hareki pushed a commit to hareki/claudecode.nvim that referenced this pull request Nov 14, 2026
…rules (coder#604)

## Why

The verifier of coder#596 found three one-line changes of the key patterns in
`field()` that keep all 16 tests of the workflow spec green:

1. `X1`: the closing quote of the single-quoted pattern is optional.
2. `X2`: the bare pattern takes a leading `'`.
3. `X11`: the two quoted patterns merge into one that takes any quote on
each side (`["']key["']:`).

Under each of them a workflow that CI accepts saves the mise cache on a
pull request and still passes the spec, for example a key `'cache:
"false" #': x` or `"cache': false #": x`. `actionlint`, `zizmor` and
`prettier --check` pass both.

The verifier of coder#596 found a fourth change, `X6`, that also kept every
test green. It compares the block with `current ~= nil` instead of
`current == block`, so an entry of the `env` block counts as an input of
`with:`.

The verifier of coder#596 had run a fifth change, `X9`, and found no input
that tells it apart. The verifier of this PR found one. Before the new
test below, `X9` kept every test green. It replaces the `%S` check of
the block line with `%w`, so a step-key line with no letter or digit
does not end the `with:` block. An explicit YAML key is such a line: `?`
and `:` alone on their lines at the step-key indent. With `X9`, `cache:
false` after them stays an input of `with:`, and YAML puts it under
`env`. GitHub's workflow parser accepts that text.

coder#596 merged under the maintainer's waiver for this open test gap. This
PR closes it.

## What changed

Only `tests/unit/workflow_cache_spec.lua` (+18, -17). The workflows are
unchanged. The spec still has no YAML parser, and no new form is read.

1. The test of one-sided quotes also feeds a key with a left quote only
(`'cache: c`) and two mismatched pairs (`"cache_save': d` and
`'cache_key": e`). It is renamed to say so.
2. A new test checks that an entry of the `env` block is not read as an
input of `with:`. It reads `cache: false` under `env:` and expects no
`cache` under `with` and `false` under `env`.
3. A new test feeds the explicit key text (`?` and `:` at the step-key
indent, `env` and `cache: false` at the entry indent under them) and
expects no `cache` under `with`. It kills `X9`.
4. `split_lines` is gone. The call `(text .. "\n"):gmatch("(.-)\n")` is
already an iterator, and every result stays the same.

## Evidence

Each change of the reader is one edit of the spec, and the spec runs on
the workflow of main. Before is the spec of main (16 tests), after is
this branch (18 tests).

| Change of the reader | Before | After (failing test) |
| --- | --- | --- |
| `X1`: closing `'` of the single-quoted pattern optional | green | red
(the one-sided and mismatched pair test) |
| `X2`: the bare pattern takes a leading `'` | green | red (the
one-sided and mismatched pair test) |
| `X11`: the two quoted patterns merged | green | red (the one-sided and
mismatched pair test) |
| `X6`: `current ~= nil` instead of `current == block` | green | red
(the new env block test) |
| `S1` to `S10`, `X3`, `X4`, `X5`, `X7`, `X10` | red | red |
| `X9`: `%w` for `%S` in the block line | green | red (the new step-key
test) |
| `X8`: block indent `<= 8` | green | green |

Counts over the 21 changes. Before, 15 are red and 6 are green. After,
20 are red and 1 is green. 15 stay red, 5 turn red (`X1`, `X2`, `X6`,
`X9`, `X11`) and 1 stays green (`X8`). The failing tests of the 15 that
stay red are the same, apart from the renamed test and the two new
tests: `S6` and `S10` also fail the env block test, and `S1`, `S10` and
`X7` also fail the step-key test. `X9` is red because an explicit YAML
key (`?` and `:` alone on their lines) tells it apart, GitHub's workflow
parser accepts that text, and the new test catches it. `X8` stays green
on every spec. No input that GitHub's parser accepts was found to tell
it apart.

I ran 170 workflows (the 169 of the verifier of coder#596 and the workflow of
main) with the spec of main, with the spec after the first commit of
this PR and with the final spec. 114 are red and 56 are green, and the
result and the failing test names are the same for all of them. The 169
give the same rows as before this change.

| Claim | Evidence |
| --- | --- |
| The spec passes on main with this change | 18 successes, 0 failures |
| The workflows are valid | The workflows are identical to main.
`actionlint` 1.7.12 on all 5: exit 0. `zizmor` 1.25.2 `--offline` on all
5: no findings (25 suppressed). `mise run format-check`: exit 0 (458
files, 0 changed) |
| Gate | `mise run clean && mise run all` exits 0 in 654 s: 1910
successes, 0 failures, 0 errors, 36 pending (seed 1794569343).
`luacheck`: 0 warnings, 0 errors in 200 files. The stub `claude` and GNU
`timeout` first on `PATH` |

---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
hareki pushed a commit to hareki/claudecode.nvim that referenced this pull request Nov 14, 2026
…the owned stop (coder#602)

## Why

This is a follow-up to coder#593. It fixes the Snacks close and the stop in
`owned.lua`, the two close paths that coder#593 left open.

A job that was just started can still be a fork of Neovim until its
child has run `exec`. A `jobstop` on it then ends Neovim with `Caught
deadly signal 'SIGHUP'`, or the buffer delete after it hangs. coder#593
documents the race, and coder#544 made the native close wait for the exec of
a young job. The Snacks close and the stop in `owned.lua` still called
`jobstop` at once, so a close within milliseconds of the start could
kill Neovim.

## Cause

`pty_process_spawn` forks the child, and `init_child()` runs `setsid()`
and resets the signal handlers only after that. Until `exec` the child
has Neovim's libuv handlers. When the pty master closes in that gap (a
`jobstop`), the kernel sends SIGHUP to the child, libuv's handler in the
child writes into the signal pipe that it shares with Neovim, and Neovim
reads a SIGHUP of its own and exits. With the child held in the gap (see
Evidence), a close right after the start killed Neovim in every run for
the Snacks provider and for the stop in `owned.lua`, on 0.8.0 and
0.12.3. The native provider survived in every run.

## Fix

The wait moves out of `native.lua` into `owned.wait_for_exec(jobid)`,
with `owned.is_young(started_at)` for the 10 s rule. The code is the
same: it reads the names of Neovim and of the child from
`/proc/<pid>/comm` (or `nvim_get_proc` where `/proc` is missing) and
waits up to 1 s until the child has another name.

- `native.close` calls it for a job younger than 10 s, as before.
- `snacks.M.open` records `term._cc.job_started_at` where it already
patches the instance. `snacks.close` reads it and calls the wait for a
job younger than 10 s. A terminal that a reload adopted has no start
time and is not waited for, as in native. Without the age rule, every
Snacks close on macOS would pay about 200 ms for the two `ps` calls of
the `nvim_get_proc` fallback.
- `owned.lua` `stop_buffer` has no start time, so it calls it for every
job. That path runs only after a reload or a provider switch, and the
wait ends at the first look for a job that has run `exec`.

The wait runs scheduled callbacks while the job still runs. Two Codex
findings on `ea0fbfd6` were about that, and `12f1d45f` fixes both:

- `snacks.close` now clears the owned mark before the wait for every
terminal, not only for an adopted one. An `open()` among the callbacks
can no longer adopt the terminal that is closing by its mark.
- `stop_buffer` reads the channel before the wait and checks
`nvim_buf_is_valid(buf)` before `jobstop`. A callback that deleted the
buffer during the wait no longer makes `jobstop` raise `Invalid buffer
id`.

`lua/claudecode/terminal/README.md` and the `verify-terminal-lifecycle`
skill describe the three places. The skill changed as follows:

- The fact `OwnedStopOtherStops` reads `M.wait_for_exec(channel)` and
the buffer check before `jobstop`.
- The new fact `SnacksCloseClearsMark` reads the mark clear before the
wait. The model has a switch of the same name in `CloseBegin`. The model
counts a closing terminal as ended, so the fact breaks a rule only
together with `OwnedLiveSkipsEndedJob`. `check.sh --mutants` has a row
for the pair.
- `OwnedLiveSkipsEndedJob` alone now breaks `OpenShowsLiveClaude`, not
`CloseNeverReuses`, because the model clears the mark in every Snacks
close.
- The model still passes with 279492 distinct states.

## CI on `ea0fbfd6` and the fixes

Two checks failed on `ea0fbfd6`.

1. The integration spec compiled its shim into a TMPDIR folder. CI's
second pass runs with `TMPDIR="$RUNNER_TEMP/odd tmp 50%"`. `LD_PRELOAD`
splits its value at a space or a colon, so the shim was ignored, the
child was never held, and the probe timed out. `f910015b` puts the shim
into `mktemp -d /tmp/claudecode.XXXXXX` when the TMPDIR path holds a
space or a colon.
2. `verify-diff-lifecycle` also hashes
`terminal/{native,owned,snacks}.lua`, so its `covered.lock` was stale.
`3a1961a0` re-records the locks of `verify-terminal-lifecycle` and
`verify-diff-lifecycle`.

## Tests

- `tests/integration/terminal_close_young_job_spec.lua` runs one Neovim
per case with `-u NONE` (so `minimal_init.lua` does not wait for the
exec and hide the race) and `tests/helpers/hold_pty_child.c` preloaded.
The shim holds the child inside `init_child()` and creates a file when
it does. The probe (`tests/helpers/close_young_job_probe.lua`) starts a
Claude terminal job, waits for that file, and closes the job. Cases: a
plain `jobstop` as the control (it must end Neovim with SIGHUP, so the
other cases can fail), the native close, the Snacks close (with a
stand-in for Snacks) and the `owned.lua` stop. The spec is pending
without Linux, a C compiler and `timeout`. It puts the shim into a
`/tmp` folder when TMPDIR holds a space or a colon, and
`tests/helpers/ld_preload_path.lua` makes that choice.
- `tests/unit/ld_preload_path_spec.lua` has three cases: a plain path, a
path with a space and a path with a colon. Before it, a spec pattern
without the colon half passed in every CI configuration.
- `tests/unit/terminal/owned_wait_for_exec_spec.lua` covers the wait
(names from `/proc`, from `nvim_get_proc`, the 1 s limit, a job with no
pid, unreadable names), `is_young`, the order of the stop in `owned.lua`
(wait, then `jobstop`, then delete) and "leaves a buffer alone that a
callback deleted during the wait".
- `tests/unit/terminal/snacks_close_spec.lua` covers the Snacks close:
it waits before `jobstop` for a job that just started, not for a job
older than 10 s, and not for a job that exited. The case "does not let
an open during the wait adopt the terminal that is closing" covers the
mark clear.
- Five Snacks unit specs gain `vim.loop.hrtime` in their vim mock, and
one gains `vim.fn.getpid`, because the provider now reads a clock and
the wait reads the pid.
- `proof-pty-child-hangup.sh` compiles the same shim instead of a copy
of it. Under a TMPDIR with a percent sign it failed all three cases, on
`main` too. With only a space or only a colon it failed one of three,
and its wrapper case passed only because the shim was not loaded. Now
`d6019d13` puts its work folder into `/tmp` for such a path.
- The header of `hold_pty_child.c` now says that the shim holds every
fork of Neovim that calls `signal(SIGCHLD, SIG_DFL)`, including the `ps`
children that `nvim_get_proc` starts, not only the pty child.

## Evidence

Builds: the official `v0.8.0` release binary (`NVIM
v0.8.0-1210-gd367ed9b2`, as in CI) and Neovim 0.12.3, scratch HOME and
XDG dirs, on a shared host.

| Run | main plus the new tests | this branch |
|---|---|---|
| integration spec, 10 runs per build | 10 of 10 fail on both builds
(the Snacks and `owned.lua` cases fail, the control and native pass) | 0
of 10 fail on both builds |
| integration spec, 20 runs per build, plain TMPDIR | not run | 0 of 20
fail on 0.8.0 and on 0.12.3 |
| integration spec, 20 runs per build, `TMPDIR="odd tmp 50%"` | the spec
of `ea0fbfd6` times out in this TMPDIR, as in CI | 0 of 20 fail on 0.8.0
and on 0.12.3 |
| the unit specs of `tests/unit/terminal` plus
`native_terminal_toggle_spec`, 20 runs | not run | 0 of 20 fail (374
successes each) |

Mutants of the Lua files, one run each, on `89e74e31`. The merge of main
changed no terminal file and no terminal spec, so I did not rerun them
on the merge commit. "Unit" counts the failing tests among 374 in
`tests/unit/terminal` plus `native_terminal_toggle_spec`. "Integration"
is the new spec on 0.8.0 and 0.12.3, with the same result on both.
"Fact" is the `verify-terminal-lifecycle` fact that turns FALSE in a
facts-only run of `check.sh`.

| Mutant | Unit | Integration | Fact |
|---|---|---|---|
| Snacks close does not wait | 2 failures | red |
`SnacksCloseClearsMark` |
| Snacks close always waits (no age rule) | 1 failure | green (the wait
is harmless but slow) | none |
| Snacks close never waits (age rule always false) | 2 failures | red |
none |
| Snacks does not record the start time | 2 failures | red | none |
| Snacks clears the mark only for an adopted terminal (the form on
`ea0fbfd6`) | 1 error (the open adopts the closing terminal) | green |
`SnacksCloseClearsMark` |
| Snacks clears the mark after the wait | 1 error (same case) | green |
`SnacksCloseClearsMark` |
| `stop_buffer` does not wait | 2 failures | red | `OwnedStopOtherStops`
|
| `stop_buffer` has no `nvim_buf_is_valid(buf)` check before `jobstop` |
1 failure | green | `OwnedStopOtherStops` |
| `stop_buffer` re-reads the channel with no check (the form on
`ea0fbfd6`) | 1 failure | green | `OwnedStopOtherStops` |
| native close does not wait | 3 failures | red | none |
| native close never counts a job as young | 3 failures | red | none |
| `wait_for_exec` does not call `vim.wait` | 6 failures | red | none |
| `wait_for_exec` stops while the names are equal | 4 failures | red |
none |
| `wait_for_exec` waits 0 ms | 4 failures | red | none |
| `wait_for_exec` has no guard for an unreadable name of Neovim | 4
failures | green | none |
| `wait_for_exec` calls `jobpid` without `pcall` (no guard for a job
without a pid) | 1 failure and 2 errors | green | none |
| `is_young` uses `<=` | 2 failures | green | none |
| `is_young` reads a `nil` start time as young (no `nil` guard) | 1
failure | green | none |

The control mutant (no change) is green in every column, with 374
successes. I ran the facts-only `check.sh` on every row, and "none"
means that every fact stayed TRUE. A mutant of
`tests/helpers/ld_preload_path.lua` that drops the colon, and one that
drops the space, each fail one case of `ld_preload_path_spec.lua`.

The model has its own mutants. `check.sh --mutants` removes each guard
of the model in turn, and it ends with "every guard is caught" on the
merge commit `191a3e24`. The pair `SnacksCloseClearsMark` with
`OwnedLiveSkipsEndedJob` is caught by `CloseNeverReuses` and
`OpenShowsLiveClaude`.

- Gate: `MISE_LOCKFILE=false mise run clean && mise run all` with GNU
timeout first on PATH exits 0 with 1925 successes, 0 failures, 0 errors
and 38 pending, and luacheck reports 0 warnings in 206 files. The extra
pending case is the new integration spec of coder#536, which needs a real
Neovim.
- `mise run format-check`: 464 files, 0 changed.
- Integration runner, with a plain TMPDIR and with `TMPDIR="odd tmp
50%"`: 865 successes, 0 failures, 0 errors on 0.8.0, and 869, 0, 0 on
0.12.3.
- `proof-pty-child-hangup.sh` passes 3 of 3 cases on 0.8.0 and on
0.12.3, with a plain TMPDIR, with `TMPDIR="odd tmp 50%"` and (on 0.12.3)
with a TMPDIR that holds a colon. On `d6019d13`'s parent, with the odd
TMPDIR, it failed 3 of 3 on both builds.
- The five `check.sh` scripts (`verify-diff-lifecycle`,
`verify-mention-queue`, `verify-selection-tracking`,
`verify-terminal-lifecycle` and `verify-ws-connection`) all print PASS.
The branch merges `origin/main` (coder#597, coder#594, coder#474, coder#596 and coder#536). coder#536
changed `diff.lua`, its README, `verify-diff-lifecycle` (SKILL.md,
`check.sh` and `covered.lock`) and added
`diff_answer_every_call_spec.lua`. Git merged the two edits of
`verify-diff-lifecycle/covered.lock` with no conflict, and `check.sh` of
all five skills prints PASS on the merge result, so no lock needed a new
record after `3a1961a0`.
- On the merge commit `191a3e24`, the specs this PR adds or changes pass
20 of 20 runs in a plain TMPDIR and in `TMPDIR="odd tmp 50%"` (24
successes for `owned_wait_for_exec_spec`, `snacks_close_spec` and
`ld_preload_path_spec`, and 374 for the terminal unit specs plus
`native_terminal_toggle_spec`). The integration spec of this PR passes
20 of 20 runs per build in both TMPDIRs. `diff_answer_every_call_spec`
from coder#536 passes 10 of 10 runs per build in both TMPDIRs (9 successes
each run).

## Not covered

- The Snacks case uses a stand-in for Snacks, because CI installs no
`snacks.nvim`. The provider code under test is the real one, and the
stand-in starts a real pty job with `termopen`.
- The wait reads `/proc` or `ps`. Where neither gives a name, the close
goes on to `jobstop` and the race stays open, as in native before this
PR. A `ps` that hangs would hang the close for as long as it hangs. I
did not reproduce that.
- I tested the `ps` fallback only on Linux, with `/proc` hidden from
Lua. I did not run it on macOS or BSD.
- A `jobstop` that other code issues, for example `:bwipeout` of the
terminal buffer by the user right after the start, is not covered.
- Another PR that edits `native.lua`, `snacks.lua`, `owned.lua` or a
`covered.lock` must be merged one at a time with this one, and re-record
the lock after its merge.

---
_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