Skip to content

fix(generate-cli): reserve --raw so a schema property named raw cannot collide - #489

Closed
giuli007 wants to merge 1 commit into
openclaw:mainfrom
giuli007:fix/generate-cli-reserve-raw-flag
Closed

giuli007 wants to merge 1 commit into
openclaw:mainfrom
giuli007:fix/generate-cli-reserve-raw-flag

Conversation

@giuli007

@giuli007 giuli007 commented Nov 3, 2026 •

Copy link
Copy Markdown

Problem

Every generated tool command registers a built-in --raw <json> option.

assignCliNames only de-duplicated tool properties against each other, so a tool with a property named raw registered a second --raw. Commander throws while the CLI builds its command tree, so the generated CLI crashes on startup, even for --help:

error: Cannot add option '--raw <raw:true|false>' to command 'get-page' due to conflicting flag '--raw'
 - already used by option '--raw <json>'

I hit this when regenerating a CLI for an HTTP MCP server whose get-page and get-presentation tools have since gained a boolean raw parameter. A binary generated from the earlier schema had worked, so the bug stayed hidden until such a parameter was added on the server side.

Fix

buildToolMetadata, buildToolMetadataList and extractOptions accept an optional list of flag names the caller registers itself, and assignCliNames treats them as taken. generateCli passes ['raw'], so a schema property named raw becomes --raw-2 and the built-in keeps --raw. This follows the existing -N suffix scheme, so the name is open to discussion if you'd prefer something more descriptive. Users can also still pass "raw": true through --raw '<json>'.

The reservation is opt-in on purpose: mcporter list, call and emit-ts share the same metadata builder through tool-cache.ts but register no --raw flag, so they keep the natural name.

Changelog

Added an entry under 0.16.2.

Tests

Two unit tests in tests/generate-cli-helpers.test.ts:

  • with raw reserved, a raw property gets raw-3 and a sibling raw_2 gets raw-2 (and the same through buildToolMetadataList);
  • with nothing reserved, as in list and call, raw keeps its natural name.

Behavior proof

Generated from a local stdio fixture with one tool, get-page, taking id (required string) and raw (boolean). Built with pnpm build and run through dist/cli.js generate-cli --runtime node --bundle.

On main (e8c7be5), get-page --help crashes:

Error: Cannot add option '--raw <raw:true|false>' to command 'get-page' due to conflicting flag '--raw'
-  already used by option '--raw <json>'

On this branch:

$ cli.mjs get-page --help
Usage: get-page --id <id> [--raw-2 <raw-2:true|false>] [--raw <json>]
...
  --raw <json>                Provide raw JSON arguments to the tool, bypassing flag parsing.
  --id <id>                   Set id. (example: example-id)
  --raw-2 <raw-2:true|false>  Return the full resource (example: true)

$ cli.mjs get-page --id abc --raw-2 true        # tool receives raw
{"id":"abc","raw":true}
$ cli.mjs get-page --id abc                     # optional flag omitted
{"id":"abc"}
$ cli.mjs get-page --raw '{"id":"abc","raw":true}'   # JSON passthrough
{"id":"abc","raw":true}
Fixture server
import readline from 'node:readline';
readline.createInterface({ input: process.stdin }).on('line', line => {
  const request = JSON.parse(line);
  if (request.id === undefined) return;
  let result;
  if (request.method === 'initialize') {
    result = { protocolVersion: request.params.protocolVersion, capabilities: { tools: {} }, serverInfo: { name: 'raw-demo', version: '1' } };
  } else if (request.method === 'tools/list') {
    result = { tools: [{ name: 'get-page', description: 'Demo tool with a raw parameter', inputSchema: { type: 'object', properties: { id: { type: 'string' }, raw: { type: 'boolean', description: 'Return the full resource' } }, required: ['id'] } }] };
  } else {
    result = { content: [{ type: 'text', text: JSON.stringify(request.params.arguments) }] };
  }
  process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: request.id, result }) + '\n');
});

Generate with node dist/cli.js generate-cli '{"name":"raw-demo","command":"node","args":["raw-fixture.mjs"]}' --runtime node --bundle cli.mjs.

Verification

  • pnpm check (format, type-aware lint, typecheck) passes.
  • The new unit tests pass; the reservation test fails without the fix.

Not covered

  • No spawn-based test of a generated CLI with a raw property was added to tests/generate-cli-option-collisions.test.ts.
  • Other template-added flags, such as -h/--help, could collide the same way. I haven't checked them, and they could be a follow-up.
  • fix(generate-cli): reserve the raw JSON flag #487 fixes the same collision with a broader end-to-end test. I'm happy to defer to it or fold this into it, whichever you prefer.

@clawsweeper

clawsweeper Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Nov 3, 2026
@clawsweeper

clawsweeper Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

Reserve the generated CLI’s built-in --raw option so a schema property named raw receives a collision-free flag.

Example: Generate a CLI for get-page with required string id and optional boolean raw.

  • Before: get-page --help crashes because both the built-in option and schema property register --raw.
  • After: get-page --id abc --raw-2 true sends {"id":"abc","raw":true}, while --raw still accepts JSON.

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) A focused root-cause repair with meaningful allocator coverage and supplied runtime evidence; existing stored formats and working non-colliding flag assignments remain unchanged.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The updated body exercises the compiled generate-cli entry point and its generated Node bundle against a local stdio MCP fixture, showing successful help, renamed-flag delivery, optional omission, and unchanged JSON passthrough.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A generated CLI crashes before displaying help or calling tools when any tool declares a raw property.
Reason: Supporting legal schema properties follows the documented collision-handling contract and repository compatibility goals.

Merge readiness

✅ Ready for maintainer review

This PR fixes a current-main startup failure with a focused patch and sufficient runtime proof; the overlapping fix is still open, so repository policy does not permit closing this PR.

Priority: P2
Reviewed head: 97297ff725518176fd3864616cbfa73fd1bfd305

Before merge

None.

Findings

None.

Agent review details

How this fits together

CLI generation converts MCP tool schemas into Commander commands, mapping assigned flags back to original JSON property names before calling the server.

flowchart LR
  A[MCP tool schema] --> B[Tool metadata builder]
  B --> C[Flag allocator with reserved raw]
  C --> D[Generated Commander command]
  D --> E[Original JSON arguments]
  E --> F[MCP server call]
Loading

Technical review

Best possible solution:

Land one focused reservation fix, preferably retaining the generated-executable coverage in #487.

Do we have a high-confidence way to reproduce the issue?

Current-main allocation and template registration establish the duplicate --raw path, supported by the contributor’s recorded base failure; target code was not executed during this read-only review.

Is this the best way to solve the issue?

The patch fixes the allocation root cause using the existing suffix allocator and preserves other metadata consumers through an optional internal argument.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against e8c7be5dfecd.

Provenance checked

  • Commander flag and storage-key allocation keeps the original intent (19630dc: The merged collision repair preserved legal schema names and prevented distinct flags from sharing Commander attribute keys.)
  • Shared metadata construction keeps the original intent (7491ed5: Generated artifacts were consolidated onto buildToolMetadataList; existing callers retain their defaults and collision policies.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

  • Current main still contains the collision: Main initializes the allocator without reserved names, while the unchanged template registers --raw before schema options. (src/cli/generate/tools.ts:140, e8c7be5dfecd)
  • Reservation stays with the generated CLI owner: Only generateCli passes reservedOptionNames: ['raw']; tool-cache retains its existing metadata call, preserving list, call, and emit-ts names. (src/generate-cli.ts:67, 97297ff72551)
  • Updated runtime proof resolves the earlier request: The full PR body contains compiled-generator and generated-Node-CLI output against a separate stdio fixture: base help fails, branch help succeeds, raw-2 reaches the server, omission stays absent, and JSON passthrough works; the author's follow-up connects this evidence to the updated branch. (97297ff72551)
  • Overlapping open fix: fix(generate-cli): reserve the raw JSON flag #487 makes equivalent production changes and extends the existing generated-executable regression; its recorded runtime transcript also exercises required raw fields and sibling raw_2. (4b80349550c9)
  • Existing collision contract is preserved: fix(generate-cli): preserve unusual schema names without option collisions #339 integrated Yigtwxx’s normalization work and reserved Commander storage keys; this PR extends that allocator without replacing its ordering or key rules. (src/cli/generate/tools.ts:144, 19630dc192a1)
  • Feature history and local history limit: Path history identifies Peter Steinberger’s generator modularization and collision work, and Vincent Koc’s metadata-list consolidation; some full-tree historical inspection encountered unavailable promisor objects, while targeted current source, blame, and relevant diffs remained readable. (src/cli/generate/tools.ts, e8c7be5dfecd)

Likely related people:

  • Peter Steinberger: Raw commit 19630dc adds src/cli/generate/tools.ts:138 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 19630dc192a1; files: src/cli/generate/tools.ts)
  • Vincent Koc: Raw commit 7491ed5 adds src/generate-cli.ts:14 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: 7491ed5a857e; files: src/generate-cli.ts)

Root-cause cluster

Relationship: duplicate
Canonical: #487
Summary: Both PRs reserve the same built-in raw option through equivalent metadata-allocation changes.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: A legal raw schema property prevents generated executables from starting; the affected schema case and repair are narrow.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-11-03T12:52:27.466Z sha 112a838 :: needs real behavior proof before merge. :: none

Reviewed November 3, 2026, 8:35 AM ET / 13:35 UTC (Revision 2).

…t collide

Every generated tool command registers a built-in --raw <json> option, but
assignCliNames only de-duplicated tool properties against each other. A tool
with a property named `raw` therefore registered a second --raw and Commander
threw at startup, so the generated CLI crashed on launch.

Let callers pass the flag names they register themselves, and have generateCli
reserve `raw`: the schema property becomes --raw-2 and the built-in keeps --raw.
Other consumers of the tool metadata (list, call, emit-ts) register no --raw, so
they keep the natural name.

Co-Authored-By: Claude
@giuli007
giuli007 force-pushed the fix/generate-cli-reserve-raw-flag branch from 112a838 to 97297ff Compare November 3, 2026 13:31
@giuli007

giuli007 commented Nov 3, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

Rebased onto main (the CHANGELOG.md conflict is resolved), and added a generated-CLI transcript from a local fixture to the description. While preparing it I found my first version also renamed raw in mcporter list/call/emit-ts via tool-cache.ts, so the reservation is now opt-in and only generateCli passes it.

@clawsweeper

clawsweeper Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. clawsweeper:intake:bug Preliminary submission classification; full source review pending. labels Nov 3, 2026
@giuli007

giuli007 commented Nov 3, 2026

Copy link
Copy Markdown
Author

Closing as a duplicate of #487, which fixes the same --raw collision with the same approach (reserving the flag at the generateCli call site) and has broader end-to-end coverage. Thanks!

One data point that may be useful there: the trigger for me was a server-side schema change. An HTTP MCP server added a boolean raw parameter to existing tools, and a CLI generated from the earlier schema (mcporter 0.13.7) kept working until it was regenerated.

@giuli007 giuli007 closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant