Skip to content

collect-remote: --topic= and --repository= (empty value) pass the required-flag check and would dispatch an empty topic or repository #287

Closed

Description

@StepenkoAnatoli

What's wrong

parseFlags supports --name=value, and --topic= yields the empty string. collect-remote's required-flag check only asks for undefined or true:

https://github.com/StepenkoAnatoli/Research-Kit/blob/9a474396c1ff7fd51a2c0b1c7b1dc4be36a60a9f/research-kit/bin/collect-remote.mjs#L131-L136

So --topic= and --repository= pass it, and the command goes on to the token step. With a token it dispatches a run with topic: "" or a request to repository "". Measured on main at 9a47439 with no token:

node research-kit/bin/collect-remote.mjs --repository o/r --topic=
no GitHub token in the environment. ...      (exit 3 - the token refusal, not a topic refusal)

node research-kit/bin/collect-remote.mjs --repository= --topic t
no GitHub token in the environment. ...      (same)

The same reads apply to --client-ref=, --prior= and --prefer=, which are guarded by !== true only, so an empty string travels as the input's value.

checkFlagValues already treats '' the same as a bare flag (--<name> needs a value); the five value flags it checks (--max-pages, --depth, --runner, --search-transport, --timeout) and, since #281, --out, --workflow and --ref are covered. The required ones are not.

The fix

Add topic: {}, repository: {}, 'client-ref': {}, prior: {}, prefer: {} to the checkFlagValues spec in bin/collect-remote.mjs. The existing "collect-remote needs --repository and --topic" message stays for flags that are absent; checkFlagValues runs after it today, so either move the call above the required check or accept that a bare --topic is reported by the "needs" message and --topic= by "needs a value" (both exit 3). If #286 lands first, the same spec entries give those flags its repeat rule too.

Test

In cli.test.mjs, next to "collect-remote refuses a bare --out, --workflow or --ref": run with --topic= and with --repository= and assert exit 3 and stderr /--(topic|repository) needs a value/ as the first line, before any token wording.

Checking your change

node research-kit/bin/selftest.mjs cli

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomershelp wantedExtra attention is needed

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions