Skip to content

Fix CI node-unit Cloudflare module resolution - #45

Merged
kentcdodds merged 2 commits into
mainfrom
cursor/ci-main-branch-fix-ddef
Mar 25, 2026
Merged

kentcdodds merged 2 commits into
mainfrom
cursor/ci-main-branch-fix-ddef

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Mar 25, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • move MCP registration-facing types into a runtime-free module so node-unit code does not import the Durable Object entry just for types
  • force @cloudflare/codemode through Vitest SSR bundling in node-unit so cloudflare:workers resolves through the test alias
  • extend the Cloudflare Workers test stub with RpcTarget for codemode's node-test load path

Root cause

@cloudflare/codemode imports RpcTarget from cloudflare:workers in its package entry. In node-unit, Vitest was externalizing that dependency, so raw Node tried to resolve the cloudflare: scheme and aborted before tests executed.

Testing

  • bun run vitest run --project node-unit packages/worker/src/mcp/tools/search.test.ts
  • bun run vitest run --project node-unit packages/worker/src/mcp/skills/skill-embed-and-flags.test.ts
  • bun run vitest run --project node-unit packages/worker/src/mcp/capabilities/coding/coding-capabilities.test.ts
  • bun run test
  • bun run typecheck
Open in Web Open in Cursor 

Summary by CodeRabbit

  • Refactor

    • Restructured internal type definitions for improved agent handling architecture.
  • Chores

    • Updated test infrastructure configuration for better dependency management.

cursoragent and others added 2 commits March 25, 2026 13:06
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
@coderabbitai

coderabbitai Bot commented Mar 25, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

A new McpRegistrationAgent type is introduced to replace the broader MCP type across multiple MCP-related functions. This type contracts access to specific server operations and methods, enabling more granular type safety. Additionally, test infrastructure updates include a new RpcTarget class and Vitest SSR configuration.

Changes

Cohort / File(s) Summary
MCP Registration Agent Type & Adoption
packages/worker/src/mcp/mcp-registration-agent.ts, packages/worker/src/mcp/register-resources.ts, packages/worker/src/mcp/register-tools.ts, packages/worker/src/mcp/resources/generated-ui-app-resource.ts, packages/worker/src/mcp/tools/execute.ts, packages/worker/src/mcp/tools/open-generated-ui.ts, packages/worker/src/mcp/tools/search.ts
Introduces new McpRegistrationAgent type with server, getEnv(), getCallerContext(), and requireDomain() members; updates function signatures across registration and tool modules to accept this narrower type instead of MCP.
Test Infrastructure
packages/worker/src/test-support/cloudflare-workers-stub.ts, vitest.node.config.ts
Adds empty RpcTarget class to test stubs and configures Vitest SSR to prevent externalization of @cloudflare/codemode during bundling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A new agent type hops into view,
Replacing broad types with something more true,
McpRegistrationAgent, focused and tight,
With server, methods, and domain in sight,
Registration flows now typed just right! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main objective: fixing CI node-unit Cloudflare module resolution issues.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/ci-main-branch-fix-ddef

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cursor cursor Bot changed the title Fix node-unit MCP type imports in CI Fix CI node-unit Cloudflare module resolution Mar 25, 2026
@kentcdodds
kentcdodds marked this pull request as ready for review March 25, 2026 13:20

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/worker/src/test-support/cloudflare-workers-stub.ts (1)

11-11: Empty RpcTarget stub is sufficient for current usage.

The empty class correctly resolves module imports. RpcTarget is not currently imported or used anywhere in the codebase, so no methods or properties are needed. This follows the established pattern for type stubs that satisfy module resolution without requiring implementation.

Optionally, consider adding a JSDoc comment for maintainability:

Suggested documentation
+/**
+ * Test stub for Cloudflare Workers RpcTarget.
+ * Empty implementation is sufficient as RpcTarget is not instantiated in tests.
+ */
 export class RpcTarget {}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/test-support/cloudflare-workers-stub.ts` at line 11, The
RpcTarget export is intentionally an empty stub to satisfy module resolution;
leave the class definition export class RpcTarget {} as-is but add a short JSDoc
above the RpcTarget declaration explaining it is a type/module resolution stub
for test-support Cloudflare worker emulation and not intended for runtime use;
ensure the JSDoc references RpcTarget so future readers understand why there are
no methods or properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/worker/src/test-support/cloudflare-workers-stub.ts`:
- Line 11: The RpcTarget export is intentionally an empty stub to satisfy module
resolution; leave the class definition export class RpcTarget {} as-is but add a
short JSDoc above the RpcTarget declaration explaining it is a type/module
resolution stub for test-support Cloudflare worker emulation and not intended
for runtime use; ensure the JSDoc references RpcTarget so future readers
understand why there are no methods or properties.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8ae03d63-1c58-4662-adb7-144914a57934

📥 Commits

Reviewing files that changed from the base of the PR and between ad11f09 and 42e2f1e.

📒 Files selected for processing (9)
  • packages/worker/src/mcp/mcp-registration-agent.ts
  • packages/worker/src/mcp/register-resources.ts
  • packages/worker/src/mcp/register-tools.ts
  • packages/worker/src/mcp/resources/generated-ui-app-resource.ts
  • packages/worker/src/mcp/tools/execute.ts
  • packages/worker/src/mcp/tools/open-generated-ui.ts
  • packages/worker/src/mcp/tools/search.ts
  • packages/worker/src/test-support/cloudflare-workers-stub.ts
  • vitest.node.config.ts

@kentcdodds
kentcdodds merged commit 4cc214a into main Mar 25, 2026
15 of 16 checks passed
@kentcdodds
kentcdodds deleted the cursor/ci-main-branch-fix-ddef branch March 25, 2026 13:59
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