Skip to content

[codex] Support pnpm workspace native build deps#1999

Merged
riderx merged 3 commits into
mainfrom
codex/native-build-pnpm-node-modules
May 1, 2026
Merged

[codex] Support pnpm workspace native build deps#1999
riderx merged 3 commits into
mainfrom
codex/native-build-pnpm-node-modules

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented May 1, 2026

Summary (AI generated)

  • Added --node-modules support to build request so native build packaging can use explicit monorepo dependency roots.
  • Materialized native packages from pnpm .pnpm virtual stores into flat node_modules/<package> paths inside the build archive.
  • Added regression coverage for a pnpm workspace iOS plugin dependency and updated CLI docs/skill metadata.

Motivation (AI generated)

Customers using pnpm workspace monorepos can have app-local native dependency symlinks that point outside the Capacitor app directory. bundle upload already accepts --node-modules, but build request did not, so native cloud builds could upload an archive missing the real plugin package content.

Business Impact (AI generated)

This unblocks native cloud builds for pnpm workspace customers and reduces failed iOS build requests caused by unresolved monorepo symlinks on Capgo runners.

Test Plan (AI generated)

  • Confirmed the new pnpm workspace regression test fails before the implementation.
  • Ran bun run test:build-zip-filter.
  • Ran bun run lint.
  • Ran bun run build.
  • Ran bun run test:mcp.
  • Ran bun run test:bundle.

Generated with AI

Summary by CodeRabbit

  • New Features

    • Added --node-modules option to the build request command, letting users supply comma-separated alternate node_modules paths (including pnpm-style layouts) so native dependencies from monorepos are bundled into build archives.
  • Documentation

    • CLI docs and build guide updated to describe the new --node-modules flag and usage examples for monorepo setups.
  • Tests

    • New integration tests validating archive creation and pnpm workspace resolution when alternate node_modules roots are provided.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 23f19770-c394-46f9-98c2-e61478c1d202

📥 Commits

Reviewing files that changed from the base of the PR and between 1e086ed and 4298cf4.

📒 Files selected for processing (2)
  • cli/src/build/request.ts
  • cli/test/test-build-zip-filter.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
  • cli/src/build/request.ts

📝 Walkthrough

Walkthrough

A new --node-modules CLI option is added to the build request command. The value (comma-separated node_modules paths) is validated by schemas, passed through the SDK and MCP handler, and consumed by zipDirectory to locate and include native dependencies from alternate node_modules roots.

Changes

Cohort / File(s) Summary
Documentation
cli/README.md, cli/skills/native-builds/SKILL.md, cli/webdocs/build.mdx
Document new --node-modules flag (comma-separated node_modules directories) and minor whitespace tweak.
CLI & SDK Integration
cli/src/index.ts, cli/src/sdk.ts, cli/src/mcp/server.ts
Expose/accept --node-modules, propagate nodeModules through SDK requestBuild and MCP handler payloads.
Schema Validation
cli/src/schemas/build.ts, cli/src/schemas/sdk.ts
Add optional nodeModules: string to build request schemas and inferred types.
Core Implementation
cli/src/build/request.ts
Extend zipDirectory signature with ZipDirectoryOptions (adds nodeModules), add addFileToZip() helper (path normalization, duplicate skipping), and resolve/include native packages from provided node_modules roots (handles pnpm .pnpm store resolution and ambiguity detection).
Tests
cli/test/test-build-zip-filter.mjs
Add integration tests for pnpm workspace resolution with explicit nodeModules (success and multi-entry failure cases).

Sequence Diagram(s)

sequenceDiagram
    participant User as CLI User
    participant Parser as CLI Parser (index.ts)
    participant SDK as SDK Layer (sdk.ts)
    participant MCP as MCP Handler (server.ts)
    participant BuildReq as Build Request (request.ts)
    participant ZipDir as Zip Handler (zipDirectory)
    participant Archive as Output Archive

    User->>Parser: build request --node-modules=path1,path2
    Parser->>SDK: requestBuild({ nodeModules: "path1,path2", ... })
    SDK->>MCP: send build request (internalOptions incl. nodeModules)
    MCP->>BuildReq: requestBuildInternal({ nodeModules: "path1,path2", ... })
    BuildReq->>ZipDir: zipDirectory(projectDir, out, platform, config, { nodeModules })
    ZipDir->>ZipDir: Resolve node_modules roots (incl. .pnpm)
    ZipDir->>ZipDir: addFileToZip() for project files and native packages
    ZipDir->>Archive: Write complete archive
    Archive-->>User: build.zip with monorepo dependencies
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I hopped through nodes and pnpm lanes,

Collected pods and swift-made chains,
Monorepo paths now bundled tight,
Native bits tucked in zip at night,
Hooray — the build hops into flight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title '[codex] Support pnpm workspace native build deps' directly and specifically describes the main change: adding pnpm workspace support for native build dependencies.
Description check ✅ Passed The PR description includes a comprehensive summary, motivation, business impact, and test plan with checkmarks confirming execution of multiple test steps.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/native-build-pnpm-node-modules

Review rate limit: 2/5 reviews remaining, refill in 28 minutes and 53 seconds.

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

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 1, 2026

Merging this PR will not alter performance

✅ 28 untouched benchmarks


Comparing codex/native-build-pnpm-node-modules (4298cf4) with main (37063ae)

Open in CodSpeed

@riderx riderx marked this pull request as ready for review May 1, 2026 09:29
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cli/src/mcp/server.ts`:
- Around line 523-527: The inline Zod object that defines appId, platform, path
and nodeModules duplicates the SDK request-build validation; instead import and
reuse the canonical request-build Zod schema (e.g., requestBuildSchema or
RequestBuildSchema) from the shared schemas module and extend it only if
necessary for MCP-specific fields (merge/augment rather than re-declare),
ensuring the nodeModules key is provided via the canonical schema or added
through schema.extend/merge; update any usages in server.ts (the object
containing nodeModules) to reference the imported schema and remove the
duplicated inline definition.
🪄 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

Run ID: b6682a3a-6636-45c8-a368-5d1e28f1f3b5

📥 Commits

Reviewing files that changed from the base of the PR and between 37063ae and 40bf9e1.

📒 Files selected for processing (10)
  • cli/README.md
  • cli/skills/native-builds/SKILL.md
  • cli/src/build/request.ts
  • cli/src/index.ts
  • cli/src/mcp/server.ts
  • cli/src/schemas/build.ts
  • cli/src/schemas/sdk.ts
  • cli/src/sdk.ts
  • cli/test/test-build-zip-filter.mjs
  • cli/webdocs/build.mdx

Comment thread cli/src/mcp/server.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 40bf9e1fdd

ℹ️ 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".

Comment thread cli/src/build/request.ts
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 1, 2026

@riderx riderx merged commit b0d82e1 into main May 1, 2026
50 checks passed
@riderx riderx deleted the codex/native-build-pnpm-node-modules branch May 1, 2026 09:50
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.

1 participant