Skip to content

fix(generate): preserve exact tool names in generated calls - #353

Merged
steipete merged 2 commits into
openclaw:mainfrom
MohammedAlkindi:fix/generate-digit-leading-tool-names
Sep 11, 2026
Merged

fix(generate): preserve exact tool names in generated calls#353
steipete merged 2 commits into
openclaw:mainfrom
MohammedAlkindi:fix/generate-digit-leading-tool-names

Conversation

@MohammedAlkindi

@MohammedAlkindi MohammedAlkindi commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Generated CLIs and typed clients fail to parse when a tool name starts with a digit. Changing the shared metadata name to add a leading underscore also makes previously distinct tools collide and disappear from discovery or types-only output.

This repair preserves metadata names and quotes the exact advertised tool name in both generators. The default runtime proxy prefers exact advertised names before its loose aliases, including after cold schema discovery, so tools such as 1password_get_item and __1password_get_item reach distinct MCP methods. Inherited JavaScript properties no longer hide advertised tools. Schemas are stored under exact names, so aliases cannot overwrite another tool’s defaults. Custom property mappers retain their existing behavior. The schema-default test uses the advertised dashed names and disables disk caching so its alias assertion is independent of earlier cache writes.

Validation:

  • New regressions fail against current main for generation syntax and exact-name dispatch; the repaired focused suite passes all 146 tests.
  • pnpm check and pnpm test --maxWorkers=4 pass: 220 files, 1,941 tests, 26 existing opt-in/skipped cases.
  • Built CLI live proof used a real local stdio MCP server advertising 1password_get_item, __1password_get_item, tools.search, __proto__, and __defineGetter__. Discovery and emitted declarations retain all five. generate-cli --bundle and emit-ts --mode client produce runnable artifacts; all ten generated calls reach the exact requested MCP method.
  • Codex autoreview is clean through P2 for both the candidate and committed branch. Linux, macOS, and Windows CI passed at 900fc62fe08b47bab447531219d46fdf31ba035a: https://github.com/openclaw/mcporter/actions/runs/34649451208.

Thanks @MohammedAlkindi for identifying the generation defect and supplying the original regression. Changelog credit is carried in the final consolidated notes PR so this branch remains independent.

…ames

toProxyMethodName camelCases a tool name without checking that the result
can start an identifier, and lowercasing a digit is a no-op. Both codegen
sites splice that name into bare dot access, so a server advertising a
tool such as 1password_get_item makes generate-cli and emit-ts --mode
client emit `proxy.1passwordGetItem`, and the generated module does not
parse.

Guard the identifier start the way emit-ts-templates.ts already guards
the interface member name with SAFE_IDENTIFIER, prefixing an underscore
when the camelCased name cannot begin one. buildToolMetadataList already
throws on a methodName collision, so the prefix cannot silently merge two
tools.
@clawsweeper

clawsweeper Bot commented Sep 8, 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 P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 8, 2026
@clawsweeper

clawsweeper Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 11, 2026, 5:36 PM ET / 21:36 UTC (Revision 4).

ClawSweeper review

What this changes

Preserves advertised MCP tool names in generated CLIs and TypeScript clients, with exact-name proxy dispatch and regression coverage for unusual names and alias collisions.

Merge readiness

Ready for maintainer review

This remains a useful, mergeable repair: current main and v0.13.11 still contain the generation defect. The earlier metadata-collision finding is resolved, and the maintainer’s real stdio verification supports the repaired behavior. No blocking introduced defect was found.

Priority: P2
Reviewed head: 900fc62fe08b47bab447531219d46fdf31ba035a

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused repair with resolved prior feedback, relevant regression coverage, and sufficient reported real-runtime verification.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The captured maintainer terminal-verification report exercises both changed generators and the production proxy through a real local stdio MCP server, observing exact results for all ten generated calls and distinct collision-case defaults.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The captured maintainer terminal-verification report exercises both changed generators and the production proxy through a real local stdio MCP server, observing exact results for all ten generated calls and distinct collision-case defaults.
Evidence reviewed 7 items Current main still needs the repair: Current main emits proxy access using an unquoted metadata method name; digit-leading names therefore produce invalid generated syntax. GitHub confirmed main remains at the supplied base revision and this PR remains open and unmerged.
Latest release retains the defective generation: The v0.13.11 template also emits unquoted proxy method access; GitHub's latest-release endpoint identifies v0.13.11 as the latest release.
Earlier metadata finding resolved: The metadata implementation has identical blob identities on main and the reviewed head. The final introduced delta instead quotes access at both generation sites, preserving existing discovery and declaration names.
Findings None None.
Security None None.

How this fits together

MCPorter generates command-line tools and TypeScript clients from MCP server metadata. Their calls pass through a runtime proxy that resolves tool names, applies schema defaults, and invokes the server.

flowchart LR
  A[MCP tool names and schemas] --> B[CLI and client generators]
  B --> C[Quoted tool access]
  C --> D[Runtime proxy]
  D --> E[Exact name or loose alias]
  E --> F[Schema defaults and validation]
  F --> G[MCP server call]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test delta Production +13/-9; tests +83/-6; docs +1 The small production increase supports exact-name dispatch, with most added lines devoted to regression coverage.
Real generated-call coverage 5 unusual names across 2 artifact formats The maintainer reports all ten generated calls reached their exact MCP methods.

Technical review

Best possible solution:

Keep metadata identities stable while generating valid access syntax and resolving each advertised name to its own tool and schema.

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

Yes: current main interpolates a digit-leading metadata name into dot access, producing invalid generated syntax. Source inspection establishes the path; the supplied maintainer report independently describes failing main regressions.

Is this the best way to solve the issue?

Yes: quoting advertised names at generation sites avoids changing shared metadata, and exact-name resolution prevents loose aliases from redirecting those calls. Existing direct-call APIs do not by themselves repair generated artifacts.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The captured maintainer terminal-verification report exercises both changed generators and the production proxy through a real local stdio MCP server, observing exact results for all ten generated calls and distinct collision-case defaults.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured maintainer terminal-verification report exercises both changed generators and the production proxy through a real local stdio MCP server, observing exact results for all ten generated calls and distinct collision-case defaults.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove merge-risk: 🚨 compatibility: Current PR review selected no merge-risk labels.

Label justifications:

  • P2: This repairs generated artifacts and dispatch for unusual MCP tool names with a bounded affected surface.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured maintainer terminal-verification report exercises both changed generators and the production proxy through a real local stdio MCP server, observing exact results for all ten generated calls and distinct collision-case defaults.
  • proof: sufficient: Contributor real behavior proof is sufficient. The captured maintainer terminal-verification report exercises both changed generators and the production proxy through a real local stdio MCP server, observing exact results for all ten generated calls and distinct collision-case defaults.

Evidence

What I checked:

  • Current main still needs the repair: Current main emits proxy access using an unquoted metadata method name; digit-leading names therefore produce invalid generated syntax. GitHub confirmed main remains at the supplied base revision and this PR remains open and unmerged. (src/cli/emit-ts-templates.ts:83, c66549857341)
  • Latest release retains the defective generation: The v0.13.11 template also emits unquoted proxy method access; GitHub's latest-release endpoint identifies v0.13.11 as the latest release. (src/cli/emit-ts-templates.ts:83, 8944ad951362)
  • Earlier metadata finding resolved: The metadata implementation has identical blob identities on main and the reviewed head. The final introduced delta instead quotes access at both generation sites, preserving existing discovery and declaration names. (src/cli/generate/tools.ts:46, 900fc62fe08b)
  • Exact dispatch and compatibility boundary: Exact advertised names take precedence both before and after metadata discovery. Existing own methods and custom-mapper selection remain intact. The changed schema map is in memory; persisted snapshot structure and writing remain unchanged. (src/server-proxy.ts:373, 900fc62fe08b)
  • Maintainer real-behavior verification: The captured maintainer comment reports built CLI discovery, bundled CLI execution, declaration generation, and runnable TypeScript-client calls against a real local stdio MCP server. All five unusual names reached their exact tools in both artifact formats; a separate runtime/proxy integration verified distinct foo-bar and fooBar targets and defaults. See fix(generate): preserve exact tool names in generated calls #353 (comment). The supplied body also reports 1,941 passing tests and successful platform CI. These are supplied observations, not reviewer-executed tests. (900fc62fe08b)
  • Related notes PR is complementary: The open chore: refresh dependencies and prepare 0.13.12 notes #359 explicitly carries consolidated release-note credit and is intended to land after the fixes; it does not replace this implementation. (35c1cbcc1516)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)
  • reviewed 2026-09-08T07:31:13.983Z sha 054af74 :: needs real behavior proof before merge. :: [P1] Preserve distinct metadata names when fixing generated access
  • reviewed 2026-09-09T04:50:07.780Z sha 054af74 :: needs real behavior proof before merge. :: [P1] Preserve distinct metadata names when fixing generated access
  • reviewed 2026-09-11T21:31:07.997Z sha 900fc62 :: needs maintainer review before merge. :: none

Keep shared metadata stable, quote generated access, and preserve exact proxy names and schemas across alias collisions.

Co-authored-by: Mohammed Alkindi <alkndymhmd692@gmail.com>
@steipete steipete changed the title fix(generate): emit a spellable proxy method for digit-leading tool names fix(generate): preserve exact tool names in generated calls Sep 11, 2026
@steipete

Copy link
Copy Markdown
Collaborator

Maintainer live proof for the repaired branch:

Used the built node dist/cli.js against a synthetic local stdio MCP server advertising five tools: 1password_get_item, __1password_get_item, tools.search, __proto__, and __defineGetter__.

  1. list digits --json retained all five tools.
  2. generate-cli digits --bundle <output>.mjs --runtime node produced a standalone CLI. Invoking each original tool name returned its own exact name from the server.
  3. emit-ts digits --out <types>.d.ts retained all five declarations.
  4. emit-ts digits --mode client --out <client>.ts produced a runnable client. Calling all five methods returned the correct tool name through result.json().

The initial bracket-only repair exposed a second issue during this live run: loose proxy aliases sent the underscore-prefixed tool to the unprefixed one. The final repair uses exact advertised names and gives those precedence over loose aliases. Regression coverage exercises both cold discovery and preloaded schemas, while retaining ordinary camelCase aliases and custom mapper behavior.

pnpm check and the complete pnpm test --maxWorkers=4 pass (1,941 tests). The new syntax and exact-dispatch regressions fail against current main. No hosted credentials or private data were used.

A separate real runtime/proxy integration advertised both foo-bar and fooBar; each retained its exact call target and distinct schema defaults. The inherited-property cases __proto__ and __defineGetter__ also pass in both generated artifact formats.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Sep 11, 2026
@steipete
steipete merged commit 0e79753 into openclaw:main Sep 11, 2026
6 of 7 checks passed
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: 🐚 platinum hermit Good normal PR readiness with ordinary 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.

2 participants