Skip to content

fix(skills): read positionals declared with .addArgument() - #13009

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.51from
ntdatt812:fix/cli-parser-addargument
Sep 10, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.51from
ntdatt812:fix/cli-parser-addargument

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

check:agent-skills-sync fails on every branch, and regenerating with --apply does not fix it — it makes it worse:

-### `tunnel create [type]`
+### `tunnel create`

Why

Commander takes a positional two ways: inline in .command("stop <type>"), or through .addArgument(new Argument("[type]")). cliRegistryParser matched only the first:

const COMMAND_RE = /\.command\(\s*["']([^"']+)["']/g;
const OPTION_RE  = /\.option\(\s*["']([^"']+)["']/g;
// nothing for .addArgument

So tunnel create came out bare, disagreed with the committed page, and the gate reported drift that it could only "resolve" by deleting a real argument.

Two commands use that form, and the second is worse than cosmetic:

declared published before actual
tunnel create + Argument("[type]") tunnel create optional, choices, default cloudflare
resilience profile set + Argument("<name>") resilience set required, four choices

An agent reading the second page was told to run omniroute resilience set with no argument.

Result

Agent Skills Generator [dry-run]
Generated: 0 · Unchanged: 46 · Pruned: 0 · Orphans: 0 · Errors: 0

Exit 0, with tunnel create [type] reproduced rather than removed. The only regenerated content is the two resilience lines that gain <name>.

What this deliberately does not do

The parser flattens subcommands onto the family name, so resilience profile set still publishes as resilience set. Resolving the receiver chain looks like a small addition. It is not — and I tried it before deciding:

cloud.mjs builds its task commands under cloud.command(agent) inside for (const agent of AGENTS), so the real path is cloud codex task create and no regex can reach a name held in a loop variable. A parent-chain pass rewrote 12 files and turned cloud create into task create — differently wrong rather than less wrong — so it is not in this change. The file's own header already records that limitation ("uses regex, not a full AST").

Verification

tests/unit/agentSkills-cliRegistryParser.test.ts — 13 passed, including one fixture test and one that asserts against the real bin/cli/commands/tunnel.mjs.

Reverting the change (fullName = base) fails exactly those two new tests and puts the gate back to exit 2 — the same check both ways.

`check:agent-skills-sync` fails on every branch, and regenerating with
--apply does not fix it -- it makes it worse:

  -### `tunnel create [type]`
  +### `tunnel create`

Commander takes a positional two ways: inline in `.command("stop <type>")`
or through `.addArgument(new Argument("[type]"))`. cliRegistryParser only
matched the first, so `tunnel create` came out bare, disagreed with the
committed page, and the gate reported drift it could only "fix" by
deleting a real argument.

Two commands are affected, and both are the .addArgument form:
`tunnel create [type]` and the resilience profile `set <name>`, whose
argument is required and carries four choices. The generated page for the
latter said `omniroute resilience set` with no argument at all.

After this, the generator reports `Generated: 0 · Unchanged: 46` and exits
0, with `tunnel create [type]` reproduced rather than removed.

Nesting is deliberately left alone. The parser flattens subcommands onto
the family name, so `resilience profile set` publishes as
`resilience set`. Resolving the receiver chain looked like a small
addition and is not: `cloud.mjs` builds its tasks under
`cloud.command(agent)` inside `for (const agent of AGENTS)`, so the real
path is `cloud codex task create` and no regex can reach it. A parent-
chain pass turned `cloud create` into `task create` across 12 files --
differently wrong, not less wrong -- so it is not in this change.
@diegosouzapw
diegosouzapw merged commit 9a56147 into cheaper-inference:release/v3.8.51 Sep 10, 2026
9 of 16 checks passed
diegosouzapw added a commit that referenced this pull request Sep 11, 2026
check:agent-skills-sync exits 2 on the clean tip (reproduced in a throwaway
probe worktree cut from origin/release/v3.8.51, with none of this branch's
commits): the generator wants to rewrite skills/cli-tunnel/SKILL.md.

Cause: #13009 ('read positionals declared with .addArgument()') taught the
generator to surface positional arguments, so the committed skill's
'tunnel create' heading and example went stale against 'tunnel create [type]'.

Regenerated with the official generator (scripts/skills/generate-agent-skills.mjs
--apply), never hand-edited. The gate now exits 0.

Refs #12732
diegosouzapw added a commit that referenced this pull request Sep 11, 2026
…l-arg fix

#13009 taught the CLI registry parser to read positionals declared with
.addArgument() but did not re-run the generator, so check:agent-skills-sync
has been red on the base tip ever since. Regenerated with the official
generator (--apply --only=cli-tunnel); the only delta is 'tunnel create'
gaining its [type] positional.

Co-authored-by: diegosouzapw <[email protected]>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…nference#13009)

Approved by the maintainer for the agent-instruction surface it touches: the SKILL.md change is regenerated output from the corrected parser (`resilience set` -> `resilience set <name>`), restoring the required argument the published page had been hiding. No hand-written directive was added.

Boarded with 13 sibling PRs and validated as a set: 132 focused tests pass, typecheck:core clean, changelog integrity and file-size gates green.

Thank you — the table contrasting the declared argument against the published page is what made the second case (an agent told to run `resilience set` with no argument) visible as more than cosmetic.
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.

2 participants