fix(scripts): declare minimatch at the repo root so the marker check runs in a fresh worktree - #1177
Conversation
…runs in a fresh worktree script/upstream/analyze.ts imports minimatch but lives at the repo root, so it resolves against root node_modules where the package was never declared (it is declared in packages/opencode). In a fresh worktree the marker check fails with 'Cannot find package minimatch' before it can run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017KXpxBn4zteNfXTwv8cf93
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe root ChangesDevelopment dependency update
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to Adding the root development dependency lets the marker check run in fresh worktrees without changing shipped behavior; no actionable merge-blocking risk remains after normal checks. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5547071803
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous Review Summary (commit 5547071)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 5547071)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file)
Reviewed by deepseek-v4-pro · Input: 33.9K · Output: 5.2K · Cached: 246.3K Review guidance: REVIEW.md from base branch |
…pendency Adding `minimatch` to the root `package.json` without the matching lockfile entry leaves the manifest and `bun.lock` inconsistent, so `bun install --frozen-lockfile` rejects a fresh install and a plain `bun install` leaves a dirty working tree. The resolved `minimatch@10.0.3` package entry was already present in the lockfile as a transitive dependency, so this only records it as a direct root-workspace devDependency. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VqnuBDGkh1ZT65Ti7e6DHZ
Review dispositionTwo reviewers raised the same single finding.
On
|
Issue for this PR
Closes #1176
Type of change
What does this PR do?
Declares
minimatchin the rootdevDependencies.script/upstream/analyze.tsimportsminimatch(three call sites), but the script lives at the repo root, so it resolves against the rootnode_modules. The package was only declared inpackages/opencode/package.json. In a fresh worktree the marker check therefore fails withCannot find package 'minimatch'before it can evaluate anything — which is confusing, because the failure has nothing to do with the changes being checked.One line, no behavior change.
How did you verify your code works?
Reproduced and fixed in a clean worktree created off
main:bun installthenbun run script/upstream/analyze.ts --markers --base main --strict→Cannot find package 'minimatch'.ok All custom code in upstream-shared files is properly marked.Screenshots / recordings
N/A
Checklist
Note
Low Risk
Dev-only dependency wiring for a root script; no changes to application or release artifacts.
Overview
Adds
minimatch(10.0.3) to the rootdevDependencies(and lockfile) soscript/upstream/analyze.tscan resolve it when run from the repository root.That script dynamically imports
minimatchfor glob-stylekeepOurs/skipFilesmatching during upstream marker checks. The dependency lived only underpackages/opencode, so a fresh worktree afterbun installcould fail withCannot find package 'minimatch'before any marker logic ran. No runtime or shipped-product behavior changes—only dependency placement for root-level tooling.Reviewed by Cursor Bugbot for commit d559efb. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Declares
minimatchat the repo root and commits the matchingbun.lockentry so the upstream analyze marker check runs in a fresh worktree.script/upstream/analyze.tsimportsminimatchbut lives at the repo root, so a fresh checkout previously failed withCannot find package 'minimatch'before the check could run, since the package was only declared inpackages/opencode.bun install --frozen-lockfilefrom rejecting a fresh install and a plainbun installfrom leaving a dirty tree. Closes Marker check fails in a fresh worktree: minimatch not declared at the repo root #1176.Written for commit d559efb. Summary will update on new commits.
Summary by CodeRabbit
minimatchdevelopment dependency.