pnpm migration#93
Conversation
|
👀 QA.tech will run a exploratory tests to review this PR as soon as a deployment is available for this PR. Alternatively, you can comment @qa.tech to manually trigger a review. Learn more about configuring preview deployments. What happens next
🤖 AI end-to-end testing powered by |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates repo tooling and docs from npm/yarn to pnpm: adds pnpm workspace and PACKAGE_MANAGER_POLICY, sets root packageManager to pnpm@10.19.0, updates scripts, CI, Docker builder stages, Playwright/test commands, Husky hooks, READMEs/docs, and converts Changespnpm Monorepo Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/copilot-instructions.md:
- Around line 16-20: Replace the explicit three-command gate with the canonical
local gate by requiring `pnpm run verify:local` before calling `task_complete`;
keep the individual commands (`pnpm run format:check`, `pnpm run format`, `pnpm
test`, `pnpm run test:browser`) as explanatory details if desired, but update
the final line to say “Do not call `task_complete` until `pnpm run verify:local`
exits successfully” and ensure any existing wording referencing the three
separate commands is removed or annotated as informational only.
In `@apps/api/src/tests/README.md`:
- Around line 13-24: Update the README test commands so they either state they
must be run from the repository root or change the paths to be local-relative;
specifically update the four Playwright commands that reference
"apps/api/src/tests/health/", "apps/api/src/tests/generate/",
"apps/api/src/tests/fromschema/", and "apps/api/src/tests/options/" to either
include a prefatory note "Run from repo root" or replace each path with
"./health/", "./generate/", "./fromschema/", "./options/" (and keep the
--config=playwright-api.config.js flag) so running the commands from
apps/api/src/tests/ will not produce path errors.
In `@pnpm-workspace.yaml`:
- Around line 14-17: The pnpm config is unsafe and invalid: replace the global
flag dangerouslyAllowAllBuilds with false and remove the placeholder strings
under allowBuilds; for each package key (e.g. '@scarf/scarf' and
'unrs-resolver') set a valid pnpm value (true, false or the string 'warn')
instead of the human-readable placeholders so the file uses only permitted types
and does not globally allow all install scripts.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e27876a-0cb1-47ba-81c0-25e1d63685c8
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.github/copilot-instructions.md.github/workflows/node.js.ymlAGENTS.mdREADME.mdapps/anywaydata/Dockerfileapps/api/Dockerfileapps/api/package.jsonapps/api/src/tests/README.mdapps/cli/README.mdapps/cli/package.jsonapps/mcp/package.jsonapps/web/Dockerfiledocs-src/README.mddocs-src/docs/070-interfaces-and-deployment/020-web-ui.mddocs-src/docs/070-interfaces-and-deployment/030-rest-api.mddocs-src/docs/070-interfaces-and-deployment/040-mcp.mddocs-src/docs/070-interfaces-and-deployment/050-cli-node-and-bun.mdpackage.jsonpackages/core-ui/package.jsonpnpm-workspace.yaml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9627b0a34
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Migrates the monorepo from npm workspaces to pnpm to enable workspace protocol dependency pinning (workspace:^) and introduces pnpm workspace configuration for minimum release age.
Changes:
- Switch root scripts, docs, and contributor instructions from
npm/yarntopnpm, including settingpackageManager. - Update internal monorepo dependencies to use
workspace:^to avoid manual version bump churn. - Update CI and Docker build pipelines to install via pnpm and add
pnpm-workspace.yamlsettings (minimum release age / build controls).
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates repo usage/build/workspace commands to pnpm. |
| pnpm-workspace.yaml | Adds pnpm workspace package globs and minimum release age/build settings. |
| packages/core-ui/package.json | Switches @anywaydata/core dependency to workspace:^. |
| package.json | Sets packageManager, updates scripts to pnpm, adjusts Playwright invocation, updates workspace orchestration. |
| docs-src/README.md | Updates Docusaurus dev/build/deploy commands to pnpm. |
| docs-src/docs/070-interfaces-and-deployment/050-cli-node-and-bun.md | Minor docs formatting tweak (trailing newline). |
| docs-src/docs/070-interfaces-and-deployment/040-mcp.md | Updates MCP start command to pnpm filter usage. |
| docs-src/docs/070-interfaces-and-deployment/030-rest-api.md | Updates API start commands to pnpm filter usage. |
| docs-src/docs/070-interfaces-and-deployment/020-web-ui.md | Updates web UI run/build notes to pnpm. |
| apps/web/Dockerfile | Migrates web Docker build to pnpm install/build with Corepack. |
| apps/mcp/package.json | Switches @anywaydata/core dependency to workspace:^. |
| apps/cli/README.md | Clarifies monorepo install/test vs published package usage. |
| apps/cli/package.json | Switches @anywaydata/core dependency to workspace:^. |
| apps/api/src/tests/README.md | Updates API test commands to pnpm equivalents and corrects paths. |
| apps/api/package.json | Switches @anywaydata/core dependency to workspace:^. |
| apps/api/Dockerfile | Migrates API Docker deps stage to pnpm install with filter. |
| apps/anywaydata/Dockerfile | Migrates site build Dockerfile to pnpm for workspace install/build. |
| AGENTS.md | Updates contributor verification instructions to pnpm. |
| .github/workflows/node.js.yml | Updates CI caching/install/CLI pack smoke test and verification steps for pnpm. |
| .github/copilot-instructions.md | Updates Copilot project instructions to pnpm-based build/test commands. |
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThis PR migrates the repository from npm/yarn to pnpm: adds pnpm workspace manifest, declares pnpm@11.1.1 in root package.json, updates scripts to pnpm, converts Changespnpm Monorepo Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/node.js.yml:
- Around line 93-95: The workflow currently mutates the lockfile and hardcodes a
binding version using the command "pnpm add -Dw
`@rolldown/binding-linux-x64-gnu`@1.0.0", which makes CI stateful and brittle;
replace that mutation with a non-mutating install (use "pnpm install --no-save"
or equivalent) and resolve the binding version dynamically instead of hardcoding
1.0.0 (e.g., read the binding version from rolldown's package metadata or from
the lockfile/installed rolldown package at runtime) so the fallback installs the
exact optionalDependency without writing to the lockfile.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76ddeb69-0eae-44cc-ac99-0dee2a960159
📒 Files selected for processing (7)
.github/workflows/node.js.ymlapps/anywaydata/Dockerfileapps/api/Dockerfileapps/mcp/Dockerfileapps/web/Dockerfilepackage.jsonpnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (1)
- pnpm-workspace.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/mcp/Dockerfile
- apps/api/Dockerfile
- apps/anywaydata/Dockerfile
- package.json
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
684-684:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix malformed Docker command in docs.
The backtick splits
docker, which breaks copy/paste execution.Suggested fix
-- Build first: `d`ocker build -f apps/mcp/Dockerfile -t anywaydata-mcp .` +- Build first: `docker build -f apps/mcp/Dockerfile -t anywaydata-mcp .`🤖 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 `@README.md` at line 684, The README contains a malformed Docker command where "d`ocker" splits the word; locate the line containing the string "d`ocker build -f apps/mcp/Dockerfile -t anywaydata-mcp ." and remove the stray backtick so the command reads "docker build -f apps/mcp/Dockerfile -t anywaydata-mcp .", ensuring the command can be copy/pasted and executed.
🤖 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 `@apps/api/src/tests/README.md`:
- Line 14: Replace the machine-specific absolute path
"D:\github\grid-table-editor" in the README instruction with a generic reference
to the repository root (e.g., "Run these from the repository root") so the docs
are portable across OSes and setups; update the sentence that currently reads
`Run these from the repository root (`D:\github\grid-table-editor`):` to instead
reference "repository root" without any OS-specific path.
---
Outside diff comments:
In `@README.md`:
- Line 684: The README contains a malformed Docker command where "d`ocker"
splits the word; locate the line containing the string "d`ocker build -f
apps/mcp/Dockerfile -t anywaydata-mcp ." and remove the stray backtick so the
command reads "docker build -f apps/mcp/Dockerfile -t anywaydata-mcp .",
ensuring the command can be copy/pasted and executed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9889095d-c43b-46ef-9a22-c30d0673ce40
📒 Files selected for processing (7)
.github/copilot-instructions.md.github/workflows/node.js.ymlPACKAGE_MANAGER_POLICY.mdREADME.mdapps/anywaydata/Dockerfileapps/api/src/tests/README.mdpnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (2)
- PACKAGE_MANAGER_POLICY.md
- .github/copilot-instructions.md
🚧 Files skipped from review as they are similar to previous changes (3)
- pnpm-workspace.yaml
- .github/workflows/node.js.yml
- apps/anywaydata/Dockerfile
closes #89
closes #92
Summary by CodeRabbit
Chores
Documentation
Policy