Smithery: fix build entry by adding module field in package.json#19
Conversation
…gate coverage/artifacts to 22.x
…es input; keep uploads on 22.x only
…ation to 22.x, Codecov v4 with OIDC, artifact warn, publish on 22.x)
…ligns with smithery.yaml entry
WalkthroughThe CI workflow was updated to test against Node.js versions 20.x, 22.x, and 24.x, with conditional coverage and integration steps for 22.x only. Coverage upload and artifact steps are refined, and action versions are updated. The build-and-publish job now uses Node.js 22.x. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Matrix (Node 20.x/22.x/24.x)
participant Test Runner
participant Codecov
GitHub Actions->>Matrix: Start job (Node.js 20.x, 22.x, 24.x)
alt Node.js 22.x
Matrix->>Test Runner: Run tests with coverage + integration
Test Runner->>Codecov: Upload coverage
else Node.js 20.x or 24.x
Matrix->>Test Runner: Run unit tests only
end
alt Node.js 22.x
Matrix->>GitHub Actions: Upload HTML coverage artifact
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
package.json (1)
5-5: Smithery fix looks correct, but verify publish-time implications of pointing "module" at TS source.Adding
"module": "./src/server.ts"satisfies @smithery/cli’s entry-point requirement. However, your"files"whitelist excludessrc/, so the published tarball will containpackage.jsonadvertising a"module"that doesn’t exist in the package. Some bundlers prefer"module"over"main"and may try to resolve it when consumers import this package.Recommended:
- Keep
"module"for Smithery.- Add an
"exports"map that directs both ESM and CJS consumers to the built artifact indist/to avoid accidental TS resolution by bundlers.- Optionally add
"types"if you intend to be importable as a library.Example (non-diff, for clarity):
{ "exports": { ".": { "import": "./dist/server.js", "require": "./dist/server.js", "default": "./dist/server.js" } }, "types": "./dist/server.d.ts" }If this package is strictly a CLI and not meant to be imported, consider adding
"private": false(already implied) and documenting that import is unsupported, or add"exports": { ".": "./dist/server.js" }to keep imports predictable..github/workflows/ci.yml (1)
94-94: Build-and-publish on Node 22.x aligns with test matrix.Consider adding npm cache to speed the job:
- - uses: actions/setup-node@v4 + - uses: actions/setup-node@v4 with: node-version: '22.x' registry-url: 'https://registry.npmjs.org' + cache: 'npm'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(3 hunks)package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test (24.x)
🔇 Additional comments (7)
.github/workflows/ci.yml (7)
12-14: Good permissions hardening for test job.Minimal permissions plus
id-token: writeis appropriate for OIDC (Codecov v4).
16-16: fail-fast disabled is appropriate for matrix reliability.Prevents early termination from masking per-version failures.
18-18: Matrix bump to 20.x/22.x/24.x looks good.Covers current and next Node lines; aligns with build-and-publish on 22.x.
49-52: Split unit-only path for non-22.x is clear and correct.Keeps matrix runtime sensible.
63-63: Artifact upload conditioned to 22.x only is sensible.Reduces noise and storage; good use of
always().
68-68: Nice touch using if-no-files-found: warn.Prevents hard failures when coverage output changes.
72-72: Coverage summary append scoped to 22.x is appropriate.Concise summary improves UX without spamming other matrix jobs.
| run: npm run typecheck | ||
|
|
||
| - name: Run tests with coverage (and integration) | ||
| if: matrix.node-version == '22.x' |
There was a problem hiding this comment.
Gate integration/coverage step to avoid secret-less fork PRs.
On forked PRs, secrets.* are unavailable; running integration may fail. Consider additionally gating by event or fork status.
Suggested change:
- if: matrix.node-version == '22.x'
+ if: matrix.node-version == '22.x' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false)This keeps full coverage/integration on branch pushes and trusted PRs while avoiding failures on forks.
🤖 Prompt for AI Agents
In .github/workflows/ci.yml at line 42, the condition for running the
integration/coverage step only checks for node version '22.x' but does not
prevent execution on forked PRs where secrets are unavailable. Modify the if
condition to also check that the event is not a pull request from a fork or that
secrets are available, for example by gating on the event type or checking if
the repository is the base repo. This will prevent running the step on forked
PRs without secrets, avoiding failures while preserving coverage on branch
pushes and trusted PRs.
| if: matrix.node-version == '22.x' | ||
| uses: codecov/codecov-action@v4 | ||
| with: | ||
| file: ./coverage/coverage-final.json | ||
| files: ./coverage/coverage-final.json | ||
| flags: unittests |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Codecov v4: explicitly enable OIDC and consider token fallback.
With id-token: write, be explicit to avoid ambiguity and future regressions:
- uses: codecov/codecov-action@v4
+ uses: codecov/codecov-action@v4
with:
files: ./coverage/coverage-final.json
+ use_oidc: true
flags: unittests
name: codecov-umbrella
fail_ci_if_error: falseIf the repo becomes private or you encounter OIDC issues, add:
token: ${{ secrets.CODECOV_TOKEN }}🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 54 to 58, explicitly add the parameter
'id-token: write' to the codecov-action step to enable OIDC authentication
clearly. Additionally, include a fallback by adding 'token: ${{
secrets.CODECOV_TOKEN }}' under 'with:' to handle cases where the repository is
private or OIDC fails. This ensures secure and reliable authentication for the
Codecov upload.
1 participant
This should unblock Smithery’s Docker build step that failed with:
"No entry point found in package.json. Please define the "module" field"
Summary by CodeRabbit