Repository navigation
fix(skills): make every new vcn evidence dir 0700, and refuse pairs()-ordered tests - #400
Conversation
… work out-made now puts VERIFY_OUT two levels below a dir with a default ACL. On main, launch refuses it: mkdir -p gives the middle dir the ACL's mode, 770, and the check after mkdir reads "…/acl/mid, above the evidence dir, is writable by others and not sticky". Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
…0700 launch made the evidence path with mkdir -p -m 700, which sets only the last dir's mode. Under a parent with a default ACL, each new dir above it got the ACL's mode (770), and the check after mkdir refused the run. That hit a VERIFY_OUT more than one level below such a dir, and a new default parent dir (verify-claudecode-nvim) under XUM_SCRATCH_DIR. launch now runs mkdir -m 700 on each missing dir, shallowest first, and removes the ones it made if a later one fails. The proof case parent-made-unsafe expected the old refusal, so it is gone. out-made covers the same code. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
LuaJIT seeds its string hash in each process, so pairs() visits string keys in a different order in each run. A spec that registers its tests from such a loop runs them in a different order each time, and the seed that mise run test prints does not repeat it. #386 fixed three such loops. scripts/check_test_order.sh prints file:line for each for ... in pairs() loop whose body calls it, describe or pending, and mise run check runs it. On this commit it flags visual_commands_select_mode_spec.lua:23, added after #386, so mise run check fails. On the tree before #386 it flags the three loops that #386 fixed. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
The three cases came from pairs() over string keys, so they ran in a different order in each run: 12 runs gave three different orders. They come from an array now, and check_test_order.sh passes. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa37d35c13
ℹ️ 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".
A pairs() loop at the top level of a spec has no indentation, and the check used an empty loop indent to mean "not in a loop", so it passed that loop. A loop closed by "end -- comment" never ended for the check, so it flagged the next ordinary it() after it. Both cases fail here. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
… end check_test_order.sh keeps an in_loop flag instead of reading an empty indent as "not in a loop", and a loop ends at an end at its indentation that only a comment follows. The new spec passes, and the check still flags the three loops that #386 fixed in the tree before #386. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dfc2509be
ℹ️ 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".
…scanning check_test_order.sh read the source text, and each review round found another shape it misread: a top-level loop, end with a comment, a one-line loop, test-like text in a long string. The order problem is a run-time fact, so check it at run time instead. tests/helpers/test_order.lua is a busted helper that .busted loads for every busted run, mise run test included. When a spec registers a test, it walks the stack, and in each frame of a file under tests/ it looks for a generic for loop whose iterator is next, which pairs() returns. If it finds one, the registration fails with the loop's file and line. On the tree before #386 it flags the three loops that #386 fixed, and on main it flags visual_commands_select_mode_spec.lua. It adds about 1 s to the suite under coverage. The script, its spec and its mise run check step are gone. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d49b31e88b
ℹ️ 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 helper counts every frame whose file path has a tests/ part. Under a checkout below a dir named tests, that includes busted's own mediator, whose event loop iterates with pairs(), so every registration fails. The new case runs a pairs() loop in a chunk named like that file and fails. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
The helper takes the tests dir from its own chunk name, @tests/ for the relative paths that mise run test passes, and counts a frame only when its chunk name starts with that. With the checkout and its rocks under <scratch>/tests/claudecode.nvim, busted on config_spec.lua gave 1 error at d49b31e and passes now. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
Two first launches can both find the default parent dir missing, and the one whose mkdir comes second fails with EEXIST. mkdir -p accepted the existing dir. The new race case puts a mkdir shim on PATH that makes the dir first, group-writable. On this commit launch exits with "cannot create …/race/mid/out" instead of refusing that dir in the check after mkdir. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
When mkdir fails on a dir above the evidence dir that now exists as a real dir, launch uses it and lets the check after mkdir decide whether it is safe. made now lists only the dirs this launch made, so a refusal never removes a dir that another process made. The race case and all 20 proof cases pass. Part of #393 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 882da0fa0f
ℹ️ 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".
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
… and #400 Against main's record, only the put_accepted_text_in_buffer block and the diff.lua file hash change. All 21 facts read TRUE and the model is unchanged. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
Why
The operator goal is to clear the open follow-up issues in this repository (#393 now) with small reviewed PRs. This PR takes #393 item 7. Item 6 is in #395.
VERIFY_OUTwere refused under a default ACL. Since ci(test): shuffle the unit suite with a printed seed, and create the vcn evidence dir 0700 #386,vcn.sh launchcreated the evidence path withmkdir -p -m 700, which sets only the last dir's mode. Under a parent with a default ACL, each new dir above it got the ACL's mode (770), and the check aftermkdirrefused the run. This hit aVERIFY_OUTmore than one level below such a dir. It also hit a new default parent dir (verify-claudecode-nvim) under such anXUM_SCRATCH_DIR. I fixed it instead of documenting it, because the fix is one loop.pairs()order. LuaJIT seeds its string hash in each process, sopairs()visits string keys in a different order in each run. A spec that registers tests from such a loop runs them in a different order each time, and the seed thatmise run testprints does not repeat that order. ci(test): shuffle the unit suite with a printed seed, and create the vcn evidence dir 0700 #386 fixed three such loops. A fourth one came in with fix(selection): count characters as Neovim does, and take the visual path in Select mode #388 (visual_commands_select_mode_spec.lua): in 12 runs, its three tests came in three different orders.What changed
vcn.sh: launch runsmkdir -m 700on each missing dir of the evidence path, shallowest first. This replaces themkdir -p -m 700and itsshellcheck disable. Two other changes come with it:mkdirdecides whether it is safe. Before,mkdir -paccepted such a dir.proof-launch-refusals.sh:out-madeputsVERIFY_OUTtwo levels below the default-ACL dir and also checks the middle dir's mode.parent-made-unsafeexpected the old refusal of a new default parent dir, so I removed it. That dir now goes through the same code asout-made's middle dir.race(new) puts amkdirshim onPATH. The shim plays another launch that makes the dir aboveVERIFY_OUTfirst, group-writable. Launch must refuse that dir in the check aftermkdirand leave it in place.SKILL.md: the creation mode and the ACL cases.tests/helpers/test_order.lua(new) is a busted helper, and the new.bustedfile loads it for every busted run,mise run testincluded. When a spec registers a test (describe,itorpending), the helper walks the stack. In each frame of a file in this repository'stests/dir, it looks for a genericforloop whose iterator isnext, which is whatpairs()returns. If it finds one, the registration fails with<file>:<line> registers "<name>" inside a pairs() loop; loop over an array with ipairs. Because it checks at run time, it does not depend on formatting, comments, one-line loops or strings.tests/unit/test_order_helper_spec.lua(new) tests the helper. It covers a loop in the spec, a loop in a calling frame, a loop in code outside thetests/dir, anipairsloop, and a registration after a loop.tests/unit/visual_commands_select_mode_spec.lua: the three cases come from an array now.DEVELOPMENT.md,AGENTS.mdandREADME.md(System Verification): the rule and the guard.Codex round 1 found two holes in my first version, a text scan (
scripts/check_test_order.sh). Round 2 found two more. So I replaced the scan with the run-time check, and the script is gone. Round 3 found the remaining two issues: the helper counted any path with atests/part, and launch aborted when it lost the race for a parent dir. Both are fixed.Scope
#393 item 7. No plugin code changes.
Blast Radius
vcn.sh launchunder a dir with a default ACL. A deeperVERIFY_OUT, or a new default parent dir, now works and starts private, where launch refused it before.pairs()loop now errors inmise run test, and in anybustedrun from the repo root. The error names the file and line.Verification
VERIFY_OUT=<0700 dir with default ACL g::rwx>/mid/deeper/out vcn.sh launch…/mid/deeper, above the evidence dir, is writable by others and not sticky. The dirs it made are removedmid,mid/deeperandoutare700, and every evidence file is0600XUM_SCRATCH_DIR=<0700 dir with that ACL> vcn.sh launch(new default parent dir)…/verify-claudecode-nvim is group- or world-writableverify-claudecode-nvimis700VERIFY_OUT=<dir>/a/<300-character name>cannot create …. The proof dir is empty afterwards, so theathat launch made is removed againproof-launch-refusals.sh(red)out-madeon main'svcn.shFAIL: out-made: launch failed: vcn: refusing evidence dir: …/acl/mid, above the evidence dir, is writable by others and not stickyracecase alone (red, Codex round 3 finding)vcn.shcannot create …/race/mid/outracecase alone…/race/mid, above the evidence dir, is writable by others and not sticky.midstays, andmid/outis gone. With a shim that makesmid0700, launch starts the runproof-launch-refusals.sh(green)PASSin 78 s on a busy host (56 s before theracecase, which starts no run), with an emptyTMPDIRafterwardsnative_terminal_toggle_spec.lua:523and:969anddiff_bulk_close_reentrancy_spec.lua:71, the three loops #386 fixedbusted tests/unit/visual_commands_select_mode_spec.luawith the helpervisual_commands_select_mode_spec.lua:24 registers "runs the visual handler for a charwise Select-mode selection" inside a pairs() loop; loop over an array with ipairstests/unit/test_order_helper_spec.luawith each of two helper mutants (never matchnext, never match atests/frame)<scratch>/tests/claudecode.nvim,busted tests/unit/config_spec.lua(Codex round 3 finding)mise run check,mise run testGates on 5df456a:
stylua --check,prettier --check,shellcheck,shfmt -dandtreefmt --cipass. This PR changes no file underlua/orplugin/. On 5df456a,verify-diff-lifecycle/check.shreads STALE only for main's own unrecordedselection.lua,tools/open_file.luaandvisual_commands.luachanges (#396, #397). #395 re-records them.Part of #393
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh