Skip to content

fix(skills): stop vcn.sh launch at a tool that cannot run, and name it - #578

Open
ThomasK33 wants to merge 15 commits into
mainfrom
skills-vcn-start-check
Open

ThomasK33 wants to merge 15 commits into
mainfrom
skills-vcn-start-check

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Why

The verification skill (.agents/skills/verify-claudecode-nvim) is the end-to-end check that a plugin change is judged against. scripts/vcn.sh launch is how it starts.

A verifier of the wire probe found that the probe stopped without a clear message when nvim on PATH was a mise shim in a checkout that mise does not trust. The probe got a start check. vcn.sh has the same start pattern, and nobody had run it that way. I ran it in a fresh clone of main in five states.

The defect: in a checkout that mise does not trust, nvim and jq on PATH are shims that fail. launch goes on, and its agent-tty step judges agent-tty doctor through the jq that cannot run. Every agent-tty then reads as "no usable output". The user sees a message that blames agent-tty and tells them to install one. The real cause, a checkout that mise does not trust, is not named.

A second defect: connect resolved setsid again, so a setsid that cannot run and sits first on PATH broke connect in a run that was healthy (main has the same line). The page promised the opposite.

What changed

  • launch runs these tools with --version before it makes a run: nvim (or the one VCN_NVIM names), node, setsid and jq. A tool that cannot run stops launch at once. The message names the tool, its path, the first line of its own error and the possible cause ("If it is a mise shim, mise may not trust this checkout."), and stops. It gives no remedy and never tells the reader to trust the checkout. No state dir and no evidence dir exist then.
  • Only launch runs this check. bin() is the same as on main (bin-vs-main.txt), and runs_or_die has no caller outside cmd_launch (callers.txt). So a jq or setsid that cannot run, or a PATH with no setsid, does not stop cleanup of a run that exists.
  • launch writes SETSID_BIN into run.env next to JQ_BIN, and connect uses it. For a run that an older vcn.sh started, run.env has no SETSID_BIN: start_client resolves it through bin() (mise first, then PATH), and only connect gets there. load does not resolve setsid any more, so cleanup of such a run on a PATH with no setsid exits 0, as on main.
  • No code line of runs_or_die holds the word jq or setsid: the message takes the tool's name from its path. The no-bare-tools case of proof-static.sh, part of mise run check, fails on a code line that names either as a bare call or as the default of a parameter expansion, except the two resolutions, and it skips a line that is only a comment.
  • proof-launch-refusals.sh gets eight cases and proof-static.sh gets one, no-trust-hint (below). SKILL.md says which four tools launch runs, what no other command checks, that launch records both paths, and that connect resolves setsid for a run that an older vcn.sh started. What it says about the time of the refusals proof is main's text (fix(skills): say only what the static proof sees and make the getz recipe fail loudly #566), which this branch keeps.
  • The first security finding of the automated reviewer was a P2 on vcn.sh line 66: the message told the reader to trust the checkout, and a trusted mise.toml runs code as the reader, while this skill checks other people's branches. Commit 6b878f8d cuts that hint instead of warning about it: the message keeps the cause ("mise may not trust this checkout") and SKILL.md no longer names a trust command. It changes 3 files, with 6 lines added and 6 deleted (SKILL.md, vcn.sh, and the two lines of proof-launch-refusals.sh that check the end of the message).
  • A verifier then found that no case failed when the hint came back (round 4, finding F-H: the hint put back in the message of vcn.sh, or the trust command put back in SKILL.md, passed every case). Commit d5d38f23 adds the case no-trust-hint to proof-static.sh, which mise run check and CI run. It fails when a file of the skill folder, apart from proof-static.sh, holds the exact text of mise's trust command or the exact name of mise's trust variable. mise run check now prints CASES 6. Commit fdc30da4 leaves proof-static.sh out of the file count in the ok line and in the check that a file exists to scan. Commit a077dda4 corrects the sentence of SKILL.md about the case: it finds those two exact texts and not every line that names the command or the variable, so a variant with other spacing or case, or a split line, passes. A git grep of the skill folder for the two texts prints nothing apart from scripts/proof-static.sh, which holds them in its own patterns and which the case skips. (git grep on ff9d02b2: vcn-push3/grep-trust.txt.)
  • The automated reviewer then raised a second P2, on vcn.sh line 62 (comment 4556655709): the remedy at the end of the message, a working tool earlier on PATH, does not help when mise which returns a mise-managed tool that is itself broken, because bin() takes the mise path before PATH. The next launch checks the same tool and prints the same advice. That remedy sentence had now drawn a finding in three rounds, so commit ff9d02b2 cuts it and does not reword it. runs_or_die takes one argument and gives no advice, and the VCN_NVIM call passes none. The message names the tool, its path, the first line of its own error and the possible cause, and stops. SKILL.md gives no remedy either (lines 17 and 134). The two proof cases that checked the advice text, tool-nvim and tool-vcn-nvim, now check that the whole message is exactly that. The commit changes 3 files, with 13 lines added and 13 deleted. The last commit, 540b7844, corrects the sentence of SKILL.md line 134 about those two cases: the proof checks the whole message for nvim and for the nvim that VCN_NVIM names, and only the first line for node, setsid and jq.

What a user sees before and after

State b, the checkout is not trusted. Before (main), exit 1 after 11 s, stderr:

mise ERROR error parsing config file: <checkout>/mise.toml
mise ERROR Config files in <checkout>/mise.toml are not trusted.
<a line of mise's own text is left out here>
mise ERROR Version: 2026.10.25 linux-x64 (2026-10-31)
mise ERROR Run with --verbose or MISE_VERBOSE=1 for more information
vcn: no agent-tty passes `agent-tty doctor`:
  <dir>/node_modules/.bin/agent-tty: no usable output
  ~/.local/share/mise/shims/agent-tty: mise ERROR error parsing config file: <checkout>/mise.toml
If a failure names pty.node, that agent-tty lacks node-pty's native build, as a mise-installed npm-agent-tty can. Install one outside mise and name it:
  npm install --prefix <dir> agent-tty@0.5.0
  VCN_AGENT_TTY=<dir>/node_modules/.bin/agent-tty .agents/skills/verify-claudecode-nvim/scripts/vcn.sh launch
vcn: launch failed (VCN_STATE=<state dir>); tearing the run down
vcn: cleanup ok, no processes left; evidence kept in <evidence dir>

After, in a live run on ff9d02b2 (a fresh extract that mise does not trust, real shims, vcn-push3/live-new-head.txt), exit 1 after 1 s, stderr:

vcn: nvim at ~/.local/share/mise/shims/nvim does not run: mise ERROR error parsing config file: <checkout>/mise.toml
If it is a mise shim, mise may not trust this checkout.

State c, VCN_NVIM names a real Neovim, so the first failing tool is jq. Before, exit 1 after 10 s, the same message about agent-tty without the five mise ERROR lines. After, in the same run, exit 1 after 2 s:

vcn: jq at ~/.local/share/mise/shims/jq does not run: mise ERROR error parsing config file: <checkout>/mise.toml
If it is a mise shim, mise may not trust this checkout.

connect with a setsid that cannot run first on PATH, after a healthy launch: before (main), exit 1 with vcn: no response for id init within 10s. After, exit 0 and the client starts.

Paths in these outputs are shortened to <checkout>, <dir>, <evidence dir> and <state dir>, and ~ is the home folder.

The five states (launch in a fresh clone that mise does not trust, then doctor and cleanup where it started a run)

State main (05b97858) the fix
a: the clone is trusted exit 0, 21 s exit 0, 8 s
b: the clone is not trusted exit 1, 11 s, blames agent-tty, evidence dir kept exit 1, 1 s, names nvim, no evidence dir
c: b, with VCN_NVIM set to a real Neovim exit 1, 10 s, blames agent-tty, evidence dir kept exit 1, 1 s, names jq, no evidence dir
d: nvim missing from PATH exit 1, 1 s, vcn: nvim not found on PATH exit 1, 0 s, the same message
e: agent-tty not on PATH exit 0, 29 s, launch uses mise's agent-tty exit 0, 24 s, the same

In a and e, doctor exits 0 and cleanup exits 0 with no tagged process and no state dir, on main and on the fix. In b, c and d the fix leaves no /tmp/vcn.* dir and no evidence dir. Main leaves no /tmp/vcn.* dir either. The seconds are not a measure: the host was loaded. The main column comes from a run on main 05b97858 (vcn-r3/states-main/). The fix column comes from a fresh clone of 79e5ec60 (vcn-r4/states-fix/), before commit 6b878f8d cut the trust hint from the message. The exit codes and the dirs left do not depend on the text of that hint. A round 4 verifier ran states b and c again on 6b878f8d, and I ran them on ff9d02b2: the same exit codes, the two messages above (on ff9d02b2 without any remedy), no state dir and no evidence dir. Nobody ran states a, d and e again after 79e5ec60.

How I proved it

The proof uses stand-ins, not a real mise shim. The mise stand-in answers which only for the real agent-tty, and for the real node unless node is the tool under test. The tool under test is a stand-in that prints mise ERROR stand-in <tool> cannot run and exits 1. The five states above use the real shims.

  1. Eight cases in scripts/proof-launch-refusals.sh and one case in scripts/proof-static.sh:
    • tool-nvim, tool-vcn-nvim, tool-jq, tool-node and tool-setsid are refused cases: launch exits 1 with the tool's message and starts no run. tool-nvim and tool-vcn-nvim also check that the whole message is the tool, its path, its own error line and the possible cause, and nothing more, so no remedy sentence can come back.
    • old-run-cleanup starts a real run, drops JQ_BIN from its run.env (an older vcn.sh wrote runs like that), puts a jq that cannot run first on PATH, and expects cleanup to exit 0 and leave no process and no state dir.
    • connect-setsid starts a real run, puts a setsid that cannot run first on PATH, and expects connect to start a client. It then drops SETSID_BIN from run.env and expects connect and cleanup to work.
    • old-run-setsid starts a real run and drops SETSID_BIN from its run.env. It expects get to exit 0 with a setsid that cannot run first on PATH, and cleanup to exit 0 and leave no process and no state dir on a PATH that holds links to every command of /usr/bin but setsid.
    • no-trust-hint, in proof-static.sh, guards the cut of the trust hint (round 4's finding F-H). It reads each text file of the skill folder except proof-static.sh (32 files at the head), line by line, and fails on a line that holds the exact text of mise's trust command or the exact name of mise's trust variable, with the file and the line in its message. It first runs its scanner on a sample folder that holds both texts and requires both hits, so a scanner that reads nothing fails. mise run check and CI run it. The output has six ok lines and CASES 6. A round 5 verifier ran round 4's mutants on fdc30da4: the hint back in the message of vcn.sh (two forms) and the trust command back in SKILL.md line 17. Each one made proof-static.sh exit 1 with FAIL no-trust-hint and the file and the line. The unchanged head printed ok no-trust-hint and CASES 6.
  2. Red proof of the six first cases, on seven trees of vcn.sh (measured on b4c1871a with the case texts of that commit: vcn-r3/red-cases.txt, red-table.txt). Since then tool-nvim and tool-vcn-nvim also check the whole message. The table uses the case texts of b4c1871a. With the cases of the head, tool-nvim and tool-vcn-nvim fail on the trees that a round 6 verifier ran: 2b7fe46f (its tool-nvim cell below says PASS, which holds only for the old case text, and its tool-vcn-nvim cell was already marked FAIL), b4c1871a, 79e5ec60 and 6b878f8d (cases-old-trees-head-cases.txt of that verifier). Every tree in the table holds the old remedy sentence, or fails these cases anyway (main's vcn.sh), so I expect both cases to fail in every row. Nobody ran the mutant rows again with the head's cases.
Tree tool-nvim tool-vcn-nvim tool-jq tool-node tool-setsid old-run-cleanup
the new code PASS PASS PASS PASS PASS PASS
commit 2b7fe46f (the check in bin()) PASS FAIL (with the head's case) PASS PASS PASS FAIL
main's vcn.sh FAIL FAIL FAIL FAIL FAIL PASS
new code without the check for nvim FAIL FAIL PASS PASS PASS PASS
new code without the check for node PASS PASS PASS FAIL PASS PASS
new code without the check for setsid PASS PASS PASS PASS FAIL PASS
new code without the check for jq PASS PASS FAIL PASS PASS PASS

Each mutant that skips the check for one tool fails exactly that tool's case. The nvim mutant fails tool-nvim and tool-vcn-nvim, which share the check. Commit 2b7fe46f is the only tree that fails old-run-cleanup: its cleanup exits 1 and leaves the state dir and 4 processes (round 1 run: task1-proof-r1/f1-experiment.out).
3. Each case of the commit 79e5ec60 ran alone on 79e5ec60 and on mutants (vcn-r4/cases-a.txt, cases-b.txt). 79e5ec60 passes all eight. The red runs:

  • old-run-setsid fails on b4c1871a: cleanup on a PATH with no setsid exits 1 (setsid not found on PATH) and leaves the state dir and the processes of the run. It fails on a tree that resolves setsid in load again and checks it in bin(): get with a setsid that cannot run exits 1.
  • tool-nvim and tool-vcn-nvim fail on the remedy sentence: on a077dda4 with only the case changes of ff9d02b2 applied (vcn.sh still ends the message with the remedy), both fail with the message in their output. On ff9d02b2 both pass (vcn-push3/cases-old-vcn-new-cases.txt and cases-new-head.txt).
  • connect-setsid fails on the new code without the SETSID_BIN fallback in start_client: connect on a run.env without SETSID_BIN exits 1. The merge commit f299fcc3 and the code without the SETSID_BIN line in run.env fail it too (vcn-r3/connect-case.txt, connect-case-main.txt, measured on b4c1871a). Those two files have no row for main's vcn.sh. A verifier ran the head's case on main's vcn.sh: it fails, connect exits 1 with vcn: no response for id init within 10s (cases-main-c2b7.txt).
  • old-run-cleanup fails on the new code with a bin() that checks every tool.
  • A bin() that checks only setsid does not turn old-run-setsid red on the new head: get and cleanup no longer call bin for setsid, so the check cannot reach them. connect on a run that an older vcn.sh started does reach that check, through start_client, and no case covers it.
  1. Both full proof scripts on 79e5ec60, with TMPDIR unset, run at the same time (vcn-r4/proofs/): proof-launch-refusals.sh exits 0 in 816 s with 50 PASS lines and 1 SKIP line (setfacl cannot set a default ACL here: three cases skip), and proof-agent-tty-resolution.sh exits 0 in 1414 s with CASES 49. The load average was 20 at the start and 53 (refusals proof) and 35 (agent-tty proof) at the end. A verifier ran both again on 79e5ec60: exit 0 in 517 s with 50 PASS lines and 1 SKIP line, and exit 0 in 647 s with CASES 49. A round 4 verifier ran proof-launch-refusals.sh on 6b878f8d with TMPDIR unset: exit 0 in 222 s, 50 PASS lines, 1 SKIP line. Nobody ran proof-agent-tty-resolution.sh after 79e5ec60. Nobody has run the full proof-launch-refusals.sh on ff9d02b2. Commit ff9d02b2 changed vcn.sh and two cases of it, and I ran each of the eight cases alone on ff9d02b2 (item 5). The last commit, 540b7844, changes SKILL.md only. Since 79e5ec60, vcn.sh differs in comment lines (main's usage comment and the comment above runs_or_die), in the code lines of runs_or_die that held the advice (the local line, the line that set it and the end of the message) and in the VCN_NVIM call. proof-launch-refusals.sh differs in the two lines that check the message, the line that holds the expected message and the header lines of the two cases. proof-agent-tty-resolution.sh does not differ (git diff 79e5ec60 HEAD).
  2. Gates on the head 540b7844ebd3e3e61419eaea7bddc10b1b86d0f4 (vcn-push4/gate-format-check.txt, gate-check.txt and gate-hash-only.txt, each file ends with its exit code, all 0): mise run format-check (435 files formatted, 0 changed), mise run check (0 warnings / 0 errors in 193 files, then the six ok lines of proof-static.sh, ok no-trust-hint among them, and CASES 6: main's five cases and the new one) and check.sh --hash-only (covered.lock matches the code). The eight cases of proof-launch-refusals.sh each ran alone on ff9d02b2 and pass (vcn-push3/cases-new-head.txt): tool-nvim, tool-vcn-nvim, tool-jq, tool-node, tool-setsid, old-run-cleanup, connect-setsid, old-run-setsid. The last commit changes SKILL.md only, so those runs hold for the head. mise run test is not run: the diff has no Lua line.
  3. Independent checks: the first commit (2b7fe46f) passed a verify round (VERDICT vcn-start 2b7fe46f2a81e4ed7ed98529308c6396e6351efa PASS). The head b4c1871a passed a second one (VERDICT vcn-start b4c1871ad5a6d540cd74d865f715e4e0dffafb2a PASS, report reports/verify-vcn-start-r2-opus.md). Its five minor points are the commit 79e5ec60. A third verifier passed 79e5ec60 (VERDICT vcn-start 79e5ec60060def1a89750394d90035a7eb82f279 PASS, report reports/verify-vcn-start-r3-opus.md) with two minor findings. F-B: a bin() that checks only setsid, written without the bare word, is told apart from the head only by connect on a run that an older vcn.sh started, and no case covers that (the third item above says so, and I add no case: connect fails on the head and on that mutant alike, and cleanup, the command that the old-run-setsid case protects, is covered). F-G: SKILL.md and this text said connect takes setsid from PATH, but bin() asks mise first. The commit 07a223c0 fixes the wording. It also rewords the comment above runs_or_die to say what main's proof-static.sh checks now (fix(skills): say only what the static proof sees and make the getz recipe fail loudly #566 rewrote it). A fourth verifier checked 6b878f8d, which holds the merge of main, the wording commit (07a223c0) and the trust-hint commit (VERDICT vcn-start 6b878f8dc3f32459b8ae0dbb9d7628cbe2178f2c ISSUES, report reports/verify-vcn-start-r4-opus.md). Its one impact finding was F-H, the missing guard (items 1 and 5 above), which d5d38f23 and fdc30da4 answer. A fifth verifier passed fdc30da4 (VERDICT vcn-start fdc30da4fa963db65b45750a92965e70395358f0 PASS, report reports/verify-vcn-start-r5-opus.md). It found four minor points. The wording of SKILL.md line 133 is fixed in a077dda4. A sixth verifier passed ff9d02b2, which holds a077dda4 (VERDICT vcn-start ff9d02b253abf0316c3c1d2d79d66a220fff2561 PASS, report reports/verify-vcn-start-r6-opus.md). It found two minor points. F-1: the sentence of SKILL.md line 134 said that the proof checks the whole message for all five tools, and it does that only for nvim and the nvim that VCN_NVIM names (for node, setsid and jq it checks the first line). The last commit, 540b7844, fixes that wording, and no verifier has read it. F-2: no case uses a tool path that mise which returns, so a remedy added only for mise paths would pass all eight cases. I declined it. Such a remedy needs new code and not a revert, and the verifier ran that case on ff9d02b2 and got the exact two lines.

What this PR does not cover

  • A setsid that does not know --version. SKILL.md requires the setsid from util-linux, which knows it. I did not test a setsid from another source.
  • A tool whose --version hangs. I did not test it.
  • A mise shim for a tool that is not installed: a run of it can start an install. I did not test it.
  • The proof does not use a real mise shim. Only the five states do.
  • connect on a run that an older vcn.sh started, on a PATH with no setsid: start_client calls bin, which stops with setsid not found on PATH (exit 1, no client recorded). A verifier ran it, and cleanup of that run then exits 0 and leaves no state dir and no process (p107.txt).
  • Three minor points of the round 5 verifier are open, all in the scanner of no-trust-hint. A hint line that also holds a byte that is not valid UTF-8 passes under a UTF-8 locale, because grep -I prints nothing for that line. The file count in the ok line comes from a separate find, so a binary file counts. An unreadable folder inside the skill folder stops the script with the error of find and not with a FAIL no-trust-hint line. All 33 files of the skill folder are valid UTF-8 text.
  • The message gives no remedy. A reader works out the fix from the tool's path and its own error line. A round 6 verifier tested a broken tool that a stand-in mise returns, for nvim, jq, node and setsid, and got the same two lines each time. No case of the proof uses a path that mise which returns.
  • The agent-tty proof takes 1414 s in this run. SKILL.md does not give a time for it, and this PR does not add one.
  • This branch merges main c43b6a308ea97f9dcfb0844ce6770e7ce7699ec0 (merge commit 17edfd427ef79ffaf57a4207d48866b64770c1c1). The merge had one conflict, in SKILL.md near line 134, where main's fix(skills): say only what the static proof sees and make the getz recipe fail loudly #566 rewrote the sentence about proof-static.sh and the time of the refusals proof, and this branch added two clauses to the sentence about proof-launch-refusals.sh. I kept main's text and added only this branch's two clauses. scripts/vcn.sh merged without a conflict (main changed one usage comment line in it). Against main, this PR changes SKILL.md on lines 17, 133 and 134 only. Main has moved on since (c7e2eb4e). None of the four files of this PR changed there, and git merge-tree of the head with that main reports no conflict.
  • The open PR fix(skills): make doc-get-exact an exact allow-list of the getz recipe #576 (head 55e68244) changes SKILL.md on line 133 and proof-static.sh. This PR changes SKILL.md on lines 17, 133 and 134, and proof-static.sh. Both change line 133, the sentence about proof-static.sh, so git merge-tree of the two heads reports one text conflict, in SKILL.md on that line. It merges proof-static.sh without a conflict, and I did not run the merged result. The PR that lands second takes both changes to line 133.

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

`vcn.sh` resolved nvim, node, jq and setsid with `mise which` or `command -v`
and never ran what it found. In a checkout that mise does not trust, the nvim
and jq on PATH are mise shims that fail. Launch went on, and its agent-tty step
judged `agent-tty doctor` through the jq that could not run: every agent-tty
read as "no usable output", and the closing message said that no agent-tty
passes doctor and told the user to install one. The cause was not named.

`bin` and the `VCN_NVIM` branch now run the tool once with `--version`. A tool
that fails stops launch before it makes a run, with the tool, its path, the
first line of its error and one line of fix (MISE_TRUSTED_CONFIG_PATHS or
`mise trust`, otherwise a working tool on PATH).

`proof-launch-refusals.sh` gets three cases that are red on main's `vcn.sh`:
a nvim on PATH that cannot run, a `VCN_NVIM` that cannot run, and a jq that
cannot run with `VCN_NVIM` set. SKILL.md says what launch checks.
… and setsid in the proof

The check of 2b7fe46 sat in `bin()`, which `load()` and `connect` also call. A run
whose run.env has no JQ_BIN (an older vcn.sh wrote it) resolves jq again in `load()`.
With a jq that cannot run, `cleanup`, `expr` and `disconnect` then refused, and the
run kept its state dir and its processes. Main cleans up there.

`bin()` is main's again. `launch` calls `runs_or_die` right after it resolves
nvim (or takes VCN_NVIM), node, setsid and jq, so no other command checks a tool.
`runs_or_die` names the tool by its file name, so the source holds no bare word jq
or setsid, which no-bare-tools in proof-agent-tty-resolution.sh looks for.

proof-launch-refusals.sh gets three cases. `tool-node` and `tool-setsid` refuse a
node and a setsid that cannot run, and each turns red when launch skips the check
for that tool. `old-run-cleanup` starts a real run, drops JQ_BIN from its run.env,
puts a jq that cannot run first on PATH, and expects cleanup to exit 0 and leave no
process and no state dir. It is red on 2b7fe46 and green on main.

SKILL.md says that launch runs each tool with `--version` before it makes a run
(not "once", and in the order of the code), that no other command checks, and that
`connect` resolves setsid again, so a failing setsid on PATH breaks it when mise
names none. The line about the refusals proof says that it takes minutes, not 60 s.
PR 545 changed cmd_launch, load, proof-launch-refusals.sh and the same SKILL.md
sentences. vcn.sh: main's new code stays, and runs_or_die and its four calls stay
in cmd_launch, each directly after the tool is resolved. SKILL.md: main's text
wins where both changed a sentence, and the clauses about a tool that cannot run
go on top of it. proof-launch-refusals.sh merged without a conflict.
…wn use the one that launch checked

The page said that a failing `setsid` earlier on PATH does not break the run. It did
not hold for `connect`: run.env recorded JQ_BIN but not SETSID_BIN, and `connect`
resolved setsid again with `bin`. With a setsid that cannot run first on PATH, it
failed with `no response for id init within 10s`.

`cmd_launch` writes SETSID_BIN into run.env next to JQ_BIN. `load` resolves it for a
run that an older vcn.sh started, the same way as JQ_BIN, and `cmd_connect` no longer
resolves setsid itself. SKILL.md says that launch records both paths and that every
later command uses them.

proof-launch-refusals.sh gets `connect-setsid`: after a real launch, a setsid that
cannot run is first on PATH and `connect` still starts a client. The same case drops
SETSID_BIN from run.env, and `connect` and `cleanup` still work. It is red on the
merge commit before this change, and red on a vcn.sh that does not record SETSID_BIN
and on one that has no fallback in `load`.
…als.sh takes

The merged script has more cases than the one that main measured at 149 s to 626 s.
A full run on a host with a load average of about 60 took 852 s. The sentence now
says 2 to 15 minutes (149 s to 852 s measured).
…d the start check's advice and docs as the script does

cleanup of a run that an older vcn.sh started (its run.env has no SETSID_BIN) stopped with "setsid not found on PATH" when
no setsid was on PATH, and left the state dir and the processes of the run. Main's cleanup exits 0 there. load no longer
resolves setsid: only start_client needs one, so connect resolves it for such a run, and no other command does.

The proof gets a case for it: get exits 0 with a setsid that cannot run first on PATH, and cleanup exits 0 and leaves no
state dir and no process on a PATH with no setsid.

When the nvim that cannot run is the one VCN_NVIM names, the message now ends with the advice to set VCN_NVIM, not to put a
working nvim on PATH. SKILL.md lists the four tools that launch runs, and says what cleanup is spared. The comment on
runs_or_die says what proof-static.sh looks for: the word jq or setsid in a code line.
@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 12, 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-12T22:23:22.855845Z 540b784 Manual request
🔒 Security Review ✅ Completed 2026-11-12T22:22:59.420755Z 540b784 Manual request

Security findings

Advisory findings (1)

ℹ️ 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. What shall we delve into next?

Reviewed commit: 07a223c0ce

ℹ️ 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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Codex Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: 07a223c0ce

ℹ️ 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.

… texts

The SKILL.md sentence said the case finds any line that names the mise command that trusts a checkout or the variable that lists trusted checkouts. The case matches only the exact texts, so a variant with other spacing, other case or a split line passes. The sentence now says that and does not write the two texts, because the case scans SKILL.md too.
@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 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a077dda41a

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

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.

The message that launch prints for a tool that cannot run ended with a remedy: put a working tool earlier on PATH, or set VCN_NVIM. That is wrong when `mise which` returns a broken mise-managed tool, because bin() takes the mise path before PATH, so the next launch checks the same tool and prints the same advice. The remedy sentence has now drawn a review finding in three rounds, so it is cut and not reworded.

runs_or_die takes one argument and gives no advice. The message names the tool, its path, the first line of the tool's own error and the possible cause, and stops. The two proof cases that checked the advice text now check that the message is exactly that. SKILL.md gives no remedy either.
SKILL.md said that the proof checks the whole tool message, with no remedy sentence, for all five tools. It does that only for nvim (tool-nvim and tool-vcn-nvim). For node, setsid and jq it checks the first line. The sentence now says exactly that.
@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 Security Review

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

Reviewed commit: 540b7844eb

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 540b7844eb

ℹ️ 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".

# The message gives no remedy: the fix that works depends on whether mise or PATH supplied the tool (bin() asks mise first).
runs_or_die() {
local why first tool=${1##*/}
why=$("$1" --version 2>&1 >/dev/null </dev/null) || {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Capture stdout from failed version checks

When VCN_NVIM or another resolved tool is a wrapper that prints its failure diagnostic to stdout and exits nonzero, the left-to-right redirections in 2>&1 >/dev/null discard that diagnostic, so launch incorrectly reports it printed no error. This defeats the new failure-reporting behavior for a supported custom executable; capture combined stdout and stderr, then ignore the captured version text only when the command succeeds.

Useful? React with 👍 / 👎.

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