Skip to content

fix(build): bundle patched fastmcp for npm installs; send top-level tool titles - #463

Merged
rakshith48 merged 9 commits into
mainfrom
fix/codex-tool-search-titles
Oct 1, 2026
Merged

rakshith48 merged 9 commits into
mainfrom
fix/codex-tool-search-titles

Conversation

@rakshith48

@rakshith48 rakshith48 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Why

  • npm and npx installs run unpatched fastmcp. Our fastmcp changes live in patches/fastmcp@4.3.2.patch, and only pnpm applies it. The published package imported fastmcp from the registry, so npx firecrawl-mcp users got no canList or beforeValidate hooks. In an AX run, the published 3.26.0 build listed 29 tools, including the deprecated firecrawl_extract, where the patched build lists 27. Only the pnpm-built hosted image had the patch.
  • Top-level title was never sent. fastmcp only emits annotations.title. MCP 2025-06-18 added a top-level Tool.title display name, and some clients read only that field. Codex's tool search, for example, indexes the top-level title and ignores annotations.title.

Summary

  • fastmcp patch: tools/list copies annotations.title into the top-level title. The title wording is unchanged.
  • Bundling: tsup inlines fastmcp (noExternal) and keeps fastmcp's own dependencies external.
  • Direct dependencies: the seven packages the bundled code imports (@modelcontextprotocol/sdk, fuse.js, hono, mcp-proxy, undici, uri-templates, xsschema) are now direct dependencies.
    • They were already installed through fastmcp, at the same locked versions.
    • They need to be direct so they resolve under pnpm's non-hoisted layout in the Docker image.
  • Tests:
    • tests/build-bundle.test.mjs fails if fastmcp goes back to being external, or if the bundle imports a package without a dependency entry. It checks static, side-effect, dynamic and require imports.
    • The stdio smoke test asserts that every tool's top-level title matches its annotations.title.
  • CHANGELOG entries added.

An earlier revision of this PR also rewrote four titles as search keywords. That is reverted: titles are display names, and in the same AX run Codex never called tool_search, so the wording made no difference.

Test Plan

  • npm test: 157/157 pass. Run with FIRECRAWL_MCP_SEARCH_PORT set, because port 3001 was taken locally.
  • npm pack, then npm install of the tarball into an empty directory: stdio tools/list serves every tool with a top-level title.
  • Docker-style pnpm@10.17.1 install --prod --frozen-lockfile --ignore-scripts with dist: same result.

🤖 Generated with Claude Code

Codex hides MCP tools behind tool_search and ranks them with BM25 over the
top-level Tool.title, description and server instructions. fastmcp only
sent annotations.title, so our titles never reached that index. The fastmcp
patch now copies annotations.title to the top-level title, and the search,
scrape, map and crawl titles use the words agents search with.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread src/index.ts Outdated
Comment thread src/index.ts Outdated
Comment thread src/index.ts Outdated
… coverage

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cubic-dev-ai[bot]
cubic-dev-ai Bot previously approved these changes Sep 29, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Auto-approved: Sends top-level MCP Tool.title in tools/list via a focused fastmcp patch, retitles four tools to match agent search terms, and adds smoke-test assertions. The change is bounded, additive, and improves Codex tool discoverability without expanding tool exposure.

Re-trigger cubic

npm does not apply pnpm patches, so the published package loaded an
unpatched fastmcp from the registry: no top-level tool titles, and no
canList/beforeValidate hooks. tsup now inlines fastmcp and keeps its
dependencies external; the packages it imports are direct dependencies so
they resolve under pnpm's non-hoisted layout (hosted Docker image) as well
as npm.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread tests/build-bundle.test.mjs Outdated
Comment thread package.json
@cubic-dev-ai
cubic-dev-ai Bot dismissed their stale review September 29, 2026 13:20

Dismissed because Cubic found issues in a newer review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Requires human review: Patches fastmcp to send top-level tool titles for Codex search, rewrites tool titles, and bundles fastmcp so npm/npx installs get the patch. Needs human sign-off on the tools/list response shape change, new display titles, and the bundling/dependency expansion.

Re-trigger cubic

Titles are display names (MCP spec), and the AX run showed Codex never
called tool_search, so the keyword rewording changed nothing. The top-level
title still mirrors annotations.title, and fastmcp stays bundled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rakshith48 rakshith48 changed the title fix(tools): send top-level tool titles for Codex tool search fix(build): bundle patched fastmcp for npm installs; send top-level tool titles Sep 29, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Auto-approved: Bundles the pnpm-patched fastmcp for npm installs, adds top-level Tool.title from annotations.title, and adds direct dependencies for bundled imports, all covered by new tests.

Re-trigger cubic

@Max17190 Max17190 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!!

rakshith48 and others added 2 commits October 1, 2026 12:04

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Auto-approved: Bundles the pnpm-patched fastmcp into the npm build so npx installs get the canList/title behavior, adds top-level Tool.title from annotations.title, promotes external imports to direct dependencies, and pins it with build and stdio tests.

Re-trigger cubic

@rakshith48
rakshith48 merged commit 5357a84 into main Oct 1, 2026
2 checks passed
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