fix: ignore node_modules at all depths across workspaces#174
Conversation
…iles (#173) Remove leading slash from /node_modules in .gitignore so it matches at any directory depth, covering all npm workspace packages. Remove the two accidentally tracked files under packages/feature-flags/node_modules. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughThe .gitignore pattern for node_modules was changed from absolute ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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. Comment |
Deploying portfolio with
|
| Latest commit: |
224713a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://253a7005.portfolio-next.pages.dev |
| Branch Preview URL: | https://fix-gitignore-node-modules-w.portfolio-next.pages.dev |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
portfolio-feature-flags | 224713a | Commit Preview URL Branch Preview URL |
Feb 11 2026, 09:59 PM |
taearls
left a comment
taearls
left a comment
There was a problem hiding this comment.
PR Review: fix: ignore node_modules at all depths across workspaces
Summary
Fixes .gitignore to match node_modules at any directory depth (not just root), removes two accidentally tracked files from packages/feature-flags/node_modules/, and updates ROADMAP.md with completion status.
Changed files: 4 files, +22 additions, -11 deletions
Impact areas: Git configuration, tracked file cleanup, documentation
Review depth: Full validation
Quality Checks Results
- ✅ Linting:
npm run lint:check- Pass (0 errors) - ✅ Tests:
npm run test- Pass (300/300 tests, 15 test files) - ✅ Build:
npm run build- Pass (tsc -b && vite build) - ⏭️ Format:
npm run format:check- Only warning is.serena/project.yml(local file, not in PR) - ⏭️ E2E tests - Skipped (no UI changes, config-only PR)
Code Review Findings
🔴 Critical Issues
Sensitive data was committed to git history
The wrangler-account.json file contained a Cloudflare account ID (cda170e385b7046f78933270660431c2) and account name/email. The cf.json file contained detailed Cloudflare edge metadata including geolocation, TLS session data, and bot management scores.
While this PR correctly removes the files from the index with git rm --cached, the data remains in git history. Since this is a public repository, consider:
- Rotating any sensitive Cloudflare credentials/tokens associated with this account
- If the account ID is considered sensitive, using
git filter-repoor BFG Repo Cleaner to scrub history (though this rewrites history and is disruptive) - At minimum, verifying that the Cloudflare account ID alone doesn't grant any access
Note: The account ID by itself is typically not a security risk (it's not an API key), but it's worth verifying. The
cf.jsondata appears to be Cloudflare request metadata from a local dev session, which is low-risk.
✅ Positive Observations
- Correct
.gitignorefix: Removing the leading/from/node_modulesis the right approach — it now matches recursively at any depth per gitignore spec - Clean removal:
git rm -r --cachedcorrectly untracks the files without deleting them from the working tree - Future-proof: The fix covers all current and future workspace packages automatically
- ROADMAP updates: Thorough — updated the issue status, category counts, status table totals, and changelog with consistent formatting
Security Review
- ✅ No new secrets introduced
⚠️ Previously committed sensitive files removed from index (see critical note above about git history)- ✅
.gitignorefix prevents future accidental commits of workspacenode_modules
Documentation Review
- ✅ ROADMAP.md updated with completion entry
- ✅ Issue status table counts updated correctly (10→11 low closed, 32→33 total closed)
- ✅ CI/CD category updated (3→4 closed)
- ✅ Changelog entry follows existing format
Testing Analysis
- ⏭️ No code changes requiring tests — this is a git configuration fix
- ✅ All 300 existing tests continue to pass
- ✅ Build succeeds confirming no side effects
Recommendations
- [Low] Consider whether the Cloudflare account ID in git history warrants any action
- File:
packages/feature-flags/node_modules/.cache/wrangler/wrangler-account.json(deleted) - Issue: Account ID remains in git history
- Suggestion: Verify this doesn't grant access; if concerned, rotate associated credentials
- File:
Approval Status
✅ APPROVE - The .gitignore fix is correct and the ROADMAP updates are thorough. The only consideration is the sensitive data remaining in git history, which is a pre-existing issue that this PR improves (by preventing future occurrences).
Review completed using: npm run lint:check, npm run format:check, npm run test, npm run build
Reviewed by: Claude Code Agent
1 participant
Summary
.gitignoreto ignorenode_modulesat any directory depth (not just root), covering all npm workspace packagespackages/feature-flags/node_modules/Closes #173
Changes
.gitignore: Changed/node_modules→node_modules(removes leading slash so the pattern matches recursively)packages/feature-flags/node_modules/.cache/wrangler/wrangler-account.jsonpackages/feature-flags/node_modules/.mf/cf.jsonROADMAP.md: Added completion entry, updated issue status table and changelogTest plan
git ls-files --cached -- '*/node_modules/*'returns no results.gitignorenow usesnode_moduleswithout leading slashnode_modulesfiles🤖 Generated with Claude Code
Summary by CodeRabbit