chore: update Dockerfile and CI configuration for Node 20, enhance pa…#112
Merged
miguelangaranocurrents merged 7 commits intomainfrom May 7, 2026
Merged
chore: update Dockerfile and CI configuration for Node 20, enhance pa…#112miguelangaranocurrents merged 7 commits intomainfrom
miguelangaranocurrents merged 7 commits intomainfrom
Conversation
…ckage structure - Updated Dockerfile to use Node 20 for both build and runtime stages. - Modified GitHub Actions workflow to support testing across Node versions 20.x, 22.x, and 24.x. - Enhanced package.json to include new module exports and updated dependencies. - Introduced tsconfig.cjs.json for CommonJS builds and adjusted TypeScript configurations. - Added new scripts for CJS package JSON generation and updated test scripts for improved coverage. - Refactored source code structure, including the introduction of an API entry point and integration tests for ESM and CJS compatibility.
…ion tool in server Made-with: Cursor
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mcp-server/src/server.ts">
<violation number="1" location="mcp-server/src/server.ts:44">
P2: Fail fast when the API key is missing; logging and continuing starts a server that can’t successfully call the Currents API.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Updated integration test to always run for package consumers, removing the dual build check. - Reformatted action descriptions in the server registration to improve readability and maintain consistency across tools.
…ion test timeout - Changed action tools to directly use the zodSchema instead of its shape for schema definition. - Enhanced the integration test timeout for the packaged CLI to ensure reliability during execution.
…SM test - Updated the GitHub Actions workflow to clarify test coverage for workspace ESM/CJS and added a new test for npm pack and import behavior. - Introduced new integration tests for packaged CLI functionality, ensuring proper execution of the `bin` field and `npx` behavior. - Added a test for resolving the package name `@currents/mcp` using `package.json` exports after installation from a tarball. - Included a fixture to validate the ESM entry point and its functionality in a consumer project context.
…ntegration tests - Enhanced error logging in the main entry point to provide clearer guidance when the CURRENTS_API_KEY is not set. - Updated the server startup logic to throw an error if the CURRENTS_API_KEY is missing, improving robustness. - Added a default value for CURRENTS_API_KEY in the environment module to prevent empty strings. - Modified integration tests to include a mock environment variable for CURRENTS_API_KEY, ensuring proper test execution.
📝 WalkthroughWalkthroughThe PR refactors the MCP server package to support dual ESM/CommonJS builds with a separate entry point, extracts server initialization into a reusable ChangesDual ESM/CJS Package Refactor with Server Extraction
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
miguelangaranocurrents
approved these changes
May 7, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
mcp-server/src/tools/actions/create-action.ts (1)
93-99: 💤 Low value
actionandmatchercan use Zod-inferred types instead ofany.The Zod schemas already precisely define these shapes; the loose types in
CreateActionRequestonly weaken compile-time safety.♻️ Proposed refactor
-interface CreateActionRequest { - name: string; - description?: string | null; - action: any[]; - matcher: any; - expiresAfter?: string | null; -} +type CreateActionRequest = Omit<z.infer<typeof zodSchema>, 'projectId'>;Or, if keeping the explicit interface:
interface CreateActionRequest { name: string; description?: string | null; - action: any[]; - matcher: any; + action: z.infer<typeof RuleAction>[]; + matcher: z.infer<typeof RuleMatcher>; expiresAfter?: string | null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp-server/src/tools/actions/create-action.ts` around lines 93 - 99, The CreateActionRequest interface uses any for action and matcher which weakens type safety; replace those any types with Zod-inferred types derived from the existing Zod schemas (e.g., use z.infer<typeof ActionSchema> and z.infer<typeof MatcherSchema> or the actual schema variable names present in this file) so that CreateActionRequest.action and CreateActionRequest.matcher reflect the precise shapes validated by the schemas; update imports/usage accordingly and run TypeScript checks to ensure no further type errors.mcp-server/src/package-published-esm.integration.test.ts (1)
59-86: 💤 Low valueTemp directories are not cleaned up after test.
The test creates
packDirandinstallDirviamkdtempSyncbut never removes them. While the OS eventually cleans tmpdir, repeated test runs accumulate stale directories. Consider adding cleanup in anafterAllorafterEachhook.Additionally,
packTarballis duplicated between this file andcli-bin.integration.test.ts. A shared test utility would reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp-server/src/package-published-esm.integration.test.ts` around lines 59 - 86, This test leaks temporary directories created by mkdtempSync into packDir and installDir; add cleanup by tracking those paths and removing them in an afterEach or afterAll hook (use fs.rmSync or fs.rmdirSync with recursive/force options) so each test run deletes packDir and installDir, and refactor the duplicated packTarball implementation into a shared test utility module and update both this test and cli-bin.integration.test.ts to import and use that single packTarball helper.mcp-server/src/package-environments.integration.test.ts (1)
33-60: 💤 Low valueConsider adding
skipIfguard for consistency with other integration tests.The CLI and published-tarball integration tests use
describe.skipIf(!existsSync(buildIndex))to skip gracefully whenbuild/is missing. This suite relies solely on the assumption thattest:runbuilds first, but runningvitestdirectly (e.g., during development) will produce confusing errors instead of a skip.Proposed fix
+const buildIndex = path.join(root, "build", "index.js"); + -describe("package consumers (Node ESM, CJS require, CJS dynamic import)", () => { +describe.skipIf(!existsSync(buildIndex))("package consumers (Node ESM, CJS require, CJS dynamic import)", () => {Also add
existsSyncto the import fromnode:fs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp-server/src/package-environments.integration.test.ts` around lines 33 - 60, This test suite should skip when the build output is missing to match other integration tests: add an existsSync import from "node:fs" and wrap the existing describe("package consumers ...") with describe.skipIf(!existsSync(buildIndex)) (using the existing buildIndex variable), so the tests gracefully skip when build/ is not present instead of erroring when running vitest directly.mcp-server/package.json (1)
29-29: 💤 Low value
chmodin build script won't work on Windows.The
chmod 755 build/index.jscommand will fail on Windows systems without a Unix compatibility layer. This is acceptable if Windows developers use WSL or if CI handles the primary build, but it could cause issues for Windows-native development.Consider wrapping in a cross-platform solution or documenting the requirement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp-server/package.json` at line 29, The build script currently runs "chmod 755 build/index.js" which fails on Windows; modify the "build" npm script (and/or update scripts/write-cjs-package-json.mjs) to set executable bits in a cross-platform way—either remove the chmod and set permissions inside the Node script using fs.chmod when supported, or use a cross-platform tool (e.g., chmod-cli or shelljs) and call that from the "build" script so Windows builds won't error; ensure the change still runs tsc && tsc -p tsconfig.cjs.json && node ./scripts/write-cjs-package-json.mjs as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Line 5: Update the Dockerfile base image to a supported Node LTS version:
replace the FROM node:20-alpine AS build reference with FROM node:22-alpine AS
build (or FROM node:24-alpine AS build for a longer-lived choice); ensure any
other Dockerfiles referencing node:20-alpine or node:21-alpine (the "21-21"
mention) are updated consistently, and run the build/CI to confirm no runtime or
dependency issues require minor tweaks after the Node upgrade.
---
Nitpick comments:
In `@mcp-server/package.json`:
- Line 29: The build script currently runs "chmod 755 build/index.js" which
fails on Windows; modify the "build" npm script (and/or update
scripts/write-cjs-package-json.mjs) to set executable bits in a cross-platform
way—either remove the chmod and set permissions inside the Node script using
fs.chmod when supported, or use a cross-platform tool (e.g., chmod-cli or
shelljs) and call that from the "build" script so Windows builds won't error;
ensure the change still runs tsc && tsc -p tsconfig.cjs.json && node
./scripts/write-cjs-package-json.mjs as before.
In `@mcp-server/src/package-environments.integration.test.ts`:
- Around line 33-60: This test suite should skip when the build output is
missing to match other integration tests: add an existsSync import from
"node:fs" and wrap the existing describe("package consumers ...") with
describe.skipIf(!existsSync(buildIndex)) (using the existing buildIndex
variable), so the tests gracefully skip when build/ is not present instead of
erroring when running vitest directly.
In `@mcp-server/src/package-published-esm.integration.test.ts`:
- Around line 59-86: This test leaks temporary directories created by
mkdtempSync into packDir and installDir; add cleanup by tracking those paths and
removing them in an afterEach or afterAll hook (use fs.rmSync or fs.rmdirSync
with recursive/force options) so each test run deletes packDir and installDir,
and refactor the duplicated packTarball implementation into a shared test
utility module and update both this test and cli-bin.integration.test.ts to
import and use that single packTarball helper.
In `@mcp-server/src/tools/actions/create-action.ts`:
- Around line 93-99: The CreateActionRequest interface uses any for action and
matcher which weakens type safety; replace those any types with Zod-inferred
types derived from the existing Zod schemas (e.g., use z.infer<typeof
ActionSchema> and z.infer<typeof MatcherSchema> or the actual schema variable
names present in this file) so that CreateActionRequest.action and
CreateActionRequest.matcher reflect the precise shapes validated by the schemas;
update imports/usage accordingly and run TypeScript checks to ensure no further
type errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3fb21cc5-8922-4dca-b1b2-45a553c37c57
⛔ Files ignored due to path filters (1)
mcp-server/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (49)
.github/workflows/test.ymlDockerfilemcp-server/package.jsonmcp-server/scripts/write-cjs-package-json.mjsmcp-server/src/api.tsmcp-server/src/cli-bin.integration.test.tsmcp-server/src/index.tsmcp-server/src/lib/env.tsmcp-server/src/package-environments.integration.test.tsmcp-server/src/package-published-esm.integration.test.tsmcp-server/src/server.tsmcp-server/src/tools/actions/create-action.tsmcp-server/src/tools/actions/delete-action.tsmcp-server/src/tools/actions/disable-action.tsmcp-server/src/tools/actions/enable-action.tsmcp-server/src/tools/actions/get-action.tsmcp-server/src/tools/actions/get-affected-test-executions-by-action.tsmcp-server/src/tools/actions/get-affected-test-executions.tsmcp-server/src/tools/actions/list-actions.tsmcp-server/src/tools/actions/list-affected-tests.tsmcp-server/src/tools/actions/update-action.tsmcp-server/src/tools/errors/get-errors-explorer.tsmcp-server/src/tools/projects/get-project-insights.tsmcp-server/src/tools/projects/get-project.tsmcp-server/src/tools/projects/get-projects.tsmcp-server/src/tools/runs/cancel-run-github-ci.tsmcp-server/src/tools/runs/cancel-run.tsmcp-server/src/tools/runs/delete-run.tsmcp-server/src/tools/runs/find-run.tsmcp-server/src/tools/runs/get-run.tsmcp-server/src/tools/runs/get-runs.tsmcp-server/src/tools/runs/reset-run.tsmcp-server/src/tools/specs/get-spec-files-performance.tsmcp-server/src/tools/specs/get-spec-instances.tsmcp-server/src/tools/tests/get-test-results.tsmcp-server/src/tools/tests/get-tests-performance.tsmcp-server/src/tools/tests/get-tests-signature.tsmcp-server/src/tools/webhooks/create-webhook.tsmcp-server/src/tools/webhooks/delete-webhook.tsmcp-server/src/tools/webhooks/get-webhook.tsmcp-server/src/tools/webhooks/list-webhooks.tsmcp-server/src/tools/webhooks/update-webhook.tsmcp-server/src/tools/webhooks/webhooks.test.tsmcp-server/test/fixtures/consumer-cjs-dynamic-import.cjsmcp-server/test/fixtures/consumer-cjs.cjsmcp-server/test/fixtures/consumer-esm.mjsmcp-server/test/fixtures/consumer-published-esm.mjsmcp-server/tsconfig.cjs.jsonmcp-server/tsconfig.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ckage structure
Summary by cubic
Move to a dual ESM/CJS build with a clean programmatic API, keep the CLI stdio entrypoint, and adopt Node 20 across Docker and CI. Adds clear failure handling when
CURRENTS_API_KEYis missing.New Features
startMcpServerentry point; CLI stdio stays insrc/index.ts, server/tool registration moves tosrc/server.ts;src/api.tsexposes programmatic exports; tools now use directzodschemas.npxvianpm pack, and published tarball resolution viaimport "@currents/mcp"; tests build first, increase CLI test timeout, and mockCURRENTS_API_KEYfor startup.CURRENTS_API_KEY(MISSING_CURRENTS_API_KEY_MESSAGE); CLI logs a short guidance message without a stack trace.@modelcontextprotocol/sdkto^1.28.0.Migration
enginesupdated).@currents/mcp@currents/mcp) or import(@currents/mcp)Written for commit da86eed. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Chores