Repository navigation
fix(collect-remote): a bare --out, --workflow or --ref is refused, not used as "true" - #281
Merged
StepenkoAnatoli merged 2 commits intoOct 11, 2026
Merged
Conversation
…t used as "true"
What changed: bin/collect-remote.mjs adds out, workflow and ref to the checkFlagValues call that already guards --max-pages, --depth, --runner, --search-transport and --timeout, with empty specs: the only rule a free-string flag needs is the one checkFlagValues applies before any spec is read, "--<name> needs a value", exit 3 (EXIT.CANNOT_START). One test in cli.test.mjs runs the command with each of the three flags bare in a scaffolded project and asserts exit 3, the refusal as the first stderr line, and that no folder named "true" exists afterwards. README inventory 1786 to 1787.
Why: parseFlags reads a flag with no value as `true`, and these three were not among the checked values. A bare --out became path.resolve("true"), and usableOutDir CREATED that folder in the working directory before the token was asked for - a no-token run exited 3 and left an empty ./true behind; with a token, a bare --workflow or --ref would have dispatched a run against the literal file or branch "true" (#270, found by the owner 2026-10-11). The remedy is the one #270 named and the one PR #276 by bonilokesh carried; that PR was closed before it merged, and its CI had failed only on the README count it did not bump. The remedy is theirs and is credited below. Alternative set aside: a `=== true` check on each of the three where it is read - three home-made checks beside the validator that exists for this, which is how the gap arose. No ADR: a bug fix with one obvious remedy.
What it touched: research-kit/bin/collect-remote.mjs (three entries in the checkFlagValues specs, and the comment above them); research-kit/test/cli.test.mjs (one test); research-kit/README.md (the inventory line, 1787); docs/ARCHITECTURE.md (the bin/collect-remote.mjs row gains the clause); CHANGELOG.md (Unreleased). No new flag, format, key or check; the documented flags and their defaults are unchanged.
What you verified:
- verified: baseline before any edit - `node research-kit/bin/selftest.mjs cli` from the repository root: 115 passed, 0 failed, 22.5 s, Node 22.22.0 linux.
- verified: red first - with only the test in place, the same group: 115 passed, 1 failed, the new test, "--out was not refused by name first" with the no-token refusal as the first line (red-270.log in the session scratchpad).
- verified: after the fix, the same group: 116 passed, 0 failed, 24.6 s.
- verified: by hand from an empty folder - `--repository o/r --topic t --out` -> "--out needs a value", exit 3; the same for --workflow and --ref; the folder held nothing afterwards. `--out sub` still reaches the no-token refusal and creates sub, as before.
- verified: the full offline suite from the repository root on Node 24.21.0 linux: 1787 passed, 0 failed, 0 unsupported, 207.4 s, exit 0.
- verified: the same suite on this container's Node 22.22.0: 1785 passed, 2 failed, both `runtime > G7` windows-1252 decoding tests, cwd /home/user/research-kit. The same two fail on an untouched checkout of main 1c1759f in the same container, and `new TextDecoder('windows-1252')` on 22.22.0 returns U+0080 for byte 0x80 where 22.23.3 (CI's line) and 24.21.0 return U+20AC: a defect of that Node build, not of this change, and recorded as an issue rather than fixed here.
- verified: git diff --check clean; the five files LF-only, no BOM.
- untested when written: this commit's all-seven CI, and the installed commit gate (not installed in this container; the suite was run by hand instead).
What you got wrong and fixed: nothing to report.
Co-authored-by: bonilokesh <bonilokesh@users.noreply.github.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gg4isLfN2Ng16zr5DwTnH9
This was referenced Oct 11, 2026
StepenkoAnatoli
marked this pull request as ready for review
October 11, 2026 09:47
StepenkoAnatoli
deleted the
claude/collect-remote-bare-values-20261011
branch
October 11, 2026 21:43
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #270.
What changed
bin/collect-remote.mjsaddsout,workflowandrefto thecheckFlagValuescall that already guards--max-pages,--depth,--runner,--search-transportand--timeout. A bare flag is now refused as--<name> needs a value, exit 3, before anything is created or dispatched. One regression test incli.test.mjsruns each of the three bare in a scaffolded project and asserts exit 3, the refusal as the first stderr line, and no folder namedtrueafterwards. README inventory 1786 → 1787;docs/ARCHITECTURE.mdrow and CHANGELOG bullet in the same commit.Why
parseFlagsreads a bare flag astrue. A bare--outbecamepath.resolve("true"), andusableOutDircreated that folder before the token was asked for; with a token, a bare--workflowor--refwould have dispatched against the literal name "true". The remedy is the one #270 named and the one PR #276 by @bonilokesh carried; that PR was closed unmerged (its CI had failed only on the README count it did not bump), and the commit credits them as co-author.Verified
selftest.mjs clifailed on the new test (--out was not refused by name first).selftest.mjs cli116 passed, 0 failed.--out,--workflow,--refeach refused by name, exit 3, nothing created;--out substill reaches the no-token refusal as before.git diff --checkclean; the five files LF-only.runtime > G7windows-1252 tests, which fail identically on untouchedmainthere. That is a defect of that Node build'sTextDecoder(22.23.3 and 24.21.0 decode correctly) and is filed as a separate issue, not fixed here.Untested
This PR's all-seven CI.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Gg4isLfN2Ng16zr5DwTnH9
Generated by Claude Code