Skip to content

chore(npm): npmignore for mcps - #59

Merged
karelhala merged 4 commits into
RedHatInsights:masterfrom
justinorringer:npmignore
Jun 11, 2026
Merged

chore(npm): npmignore for mcps#59
karelhala merged 4 commits into
RedHatInsights:masterfrom
justinorringer:npmignore

Conversation

@justinorringer

@justinorringer justinorringer commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Description

Adds 3 npm ignore files. Looks like the jira-mcp was killed a while ago (so not included)

RHCLOUD-48283


Blast radius


Rollback plan


Checklist

  • Tested against at least one consuming repo/service
  • No breaking changes to existing consumers (or migration path documented)
  • No hardcoded secrets, tokens, or passwords
  • Container images pinned to specific tags, not latest

AI disclosure

@justinorringer
justinorringer requested review from a team as code owners June 8, 2026 21:33
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 3ac51149-8031-4072-bdf7-f2b480184fbf

📥 Commits

Reviewing files that changed from the base of the PR and between 1e8398a and b36e522.

📒 Files selected for processing (3)
  • packages/hcc-feo-mcp/.npmignore
  • packages/hcc-kessel-mcp/.npmignore
  • packages/hcc-pf-mcp/.npmignore
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/hcc-pf-mcp/.npmignore
  • packages/hcc-kessel-mcp/.npmignore

Summary by CodeRabbit

  • Chores
    • Added package-level publish controls for multiple packages to restrict published contents.
    • Added an ownership rule requiring admin/framework-lead review for changes to those publish-control files.
  • Documentation
    • Updated contributor guidance with a “Package publishing” section and a note about the approval requirement for modifying publish-control files.

Walkthrough

This PR establishes consistent npm package publishing controls by adding documentation and governance for .npmignore files, protecting those files via CODEOWNERS, and implementing explicit ignore patterns across three MCP packages to exclude source, tests, and build artifacts from npm publication.

Changes

npm package publishing controls

Layer / File(s) Summary
Publishing policy and governance
AGENTS.md, CODEOWNERS
AGENTS.md documents that MCP packages publish only dist/, README.md, and LICENSE, and notes the pitfall of .npmignore modifications. CODEOWNERS assigns admin-only access to all .npmignore files to prevent accidental inclusion of development artifacts.
Package-specific .npmignore rules
packages/hcc-feo-mcp/.npmignore, packages/hcc-kessel-mcp/.npmignore, packages/hcc-pf-mcp/.npmignore
Three MCP packages receive .npmignore files with consistent glob patterns excluding TypeScript source files (src/ and *.ts), tests, build configuration (tsconfig*.json, Jest/Nx config), development dotfiles, and selected docs, while preserving *.d.ts declaration files.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description provides the primary change summary and Jira reference, but leaves critical sections (Blast radius, Rollback plan) empty and all checklist items unchecked, indicating incomplete testing and verification documentation. Complete the Blast radius and Rollback plan sections, document testing against consuming repositories, and provide clear verification that no breaking changes were introduced.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore(npm): npmignore for mcps' directly and clearly describes the main change: adding .npmignore files for the MCP packages to control npm publish contents.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@karelhala karelhala 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.

Do we want to have multiple .npmignore files or just one with all configs?

Comment thread packages/hcc-feo-mcp/.npmignore
Comment thread packages/hcc-feo-mcp/.npmignore
@justinorringer

Copy link
Copy Markdown
Contributor Author

Do we want to have multiple .npmignore files or just one with all configs?

Multiple seems like the standard way to go [1]. Claude really preferred removing them entirely and only relying on the files fields though, but that would require adding the package.json to the CODEOWNERS rules (and more labor for dependency changes).

@karelhala karelhala 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.

Looking good, just minor adjustment and we are golden.

Comment thread packages/hcc-feo-mcp/.npmignore Outdated
Comment thread packages/hcc-kessel-mcp/.npmignore Outdated
Comment thread packages/hcc-pf-mcp/.npmignore Outdated
Simplified JavaScript file ignore patterns in all three MCP packages:
- Changed from `/*.js` + `packages/*/**/*.js` + `!packages/*/dist/**/*.js`
  to simpler `**/*.js` + `!dist/**/*.js`
- Removed duplicate `/*.js` line from Development files section
- Kept `.vscode/` ignore (already present)

This addresses PR feedback to use more standard glob patterns.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@justinorringer

Copy link
Copy Markdown
Contributor Author

Great! updated and ran builds locally, looks like the dist dir content is correct.

@karelhala
karelhala merged commit d94be01 into RedHatInsights:master Jun 11, 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.

3 participants