Repository navigation
Conversation
materializePublishManifest mapped every workspace:* specifier to the dependent's own version. That breaks git-source installs for independently-versioned workspace packages: server@0.3.1 depends on @paperclipai/plugin-sdk via workspace:* while plugin-sdk is at 1.0.0, so staging required the phantom 1.0.0... 0.3.1 that npm cannot resolve (ETARGET on install). Add readWorkspaceVersions(sourceRoot) (pnpm-pack semantics, cached, node_modules KeyError... skipped) with an explicit workspaceVersions override, thread both through prepareBundledPackage, and keep the old fallback for unknown targets (uniform-calver releases unaffected). Covers server->plugin-sdk and any future independently-versioned dep.
 [Medium impact] The change affects bundled package staging and its generated install manifests. Do not merge until bundled staging respects the verifier's staged versions and the contribution requirements are met. Findings
Fix with agent prompt### Issue 1
scripts/prepare-bundled-package.mjs:465-466
Both public-install verifiers give every staged package the same `releaseVersion`, but `prepareBundledPackage` now reads dependency versions from the unchanged checkout. The staged server therefore requires `@paperclipai/plugin-sdk@1.0.0`, while the verifier packs that SDK at `releaseVersion`. The Grok registry exposes only the packed version, so npm cannot satisfy the server's dependency.
Pass the staged version map through these callers, including the child-process path, and add a regression test for this flow.
### Issue 2
scripts/prepare-bundled-package.mjs:18-26
The new documentation block violates the `CONTRIBUTING.md` directive: “Do not submit a PR that includes code comments.” The new manifest-resolution block and tests also add comments. Remove the added comments and keep the explanation in the PR description. This repository requirement must be satisfied before merging.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
### Issue 3
scripts/prepare-bundled-package.mjs:440
The description explains the fix and verification, but lacks the required Thinking Path, Risks, Model Used, and Checklist sections. `AGENTS.md` requires every template section to be filled in before merging. Please add the missing sections, including the reasoning from project context to this fix and the risks of changing version lookup.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryResolves workspace dependencies using each target package's version.
Reviews (1) · Last reviewed commit: "Resolve workspace:* to target package ve..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| workspaceVersions, | ||
| sourceRoot, |
There was a problem hiding this comment.
Both public-install verifiers give every staged package the same releaseVersion, but prepareBundledPackage now reads dependency versions from the unchanged checkout. The staged server therefore requires @paperclipai/plugin-sdk@1.0.0, while the verifier packs that SDK at releaseVersion. The Grok registry exposes only the packed version, so npm cannot satisfy the server's dependency.
Pass the staged version map through these callers, including the child-process path, and add a regression test for this flow.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/prepare-bundled-package.mjs
Line: 465-466
Comment:
**Staged packages cannot install**
Both public-install verifiers give every staged package the same `releaseVersion`, but `prepareBundledPackage` now reads dependency versions from the unchanged checkout. The staged server therefore requires `@paperclipai/plugin-sdk@1.0.0`, while the verifier packs that SDK at `releaseVersion`. The Grok registry exposes only the packed version, so npm cannot satisfy the server's dependency.
Pass the staged version map through these callers, including the child-process path, and add a regression test for this flow.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| /** | ||
| * Build a workspace package name -> version map by scanning `sourceRoot` for | ||
| * package.json files (skipping node_modules, .git, and build output dirs). | ||
| * | ||
| * Used to resolve `workspace:*` specifiers to the *target* workspace | ||
| * package's version (matching `pnpm pack` semantics) instead of the | ||
| * dependent's own version. Results are cached per sourceRoot; call | ||
| * `clearWorkspaceVersionCache()` in tests that mutate the fixture tree. | ||
| */ |
There was a problem hiding this comment.
The new documentation block violates the CONTRIBUTING.md directive: “Do not submit a PR that includes code comments.” The new manifest-resolution block and tests also add comments. Remove the added comments and keep the explanation in the PR description. This repository requirement must be satisfied before merging.
File Used: rules/pull-request-review.mdc (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/prepare-bundled-package.mjs
Line: 18-26
Comment:
**Added comments violate contribution rules**
The new documentation block violates the `CONTRIBUTING.md` directive: “Do not submit a PR that includes code comments.” The new manifest-resolution block and tests also add comments. Remove the added comments and keep the explanation in the PR description. This repository requirement must be satisfied before merging.
**File Used:** `rules/pull-request-review.mdc` ([source](https://github.com/paperclipai/paperclip/blob/765e8feb1fca9059a382bd9afc02048baeac4e7f/rules/pull-request-review.mdc))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
The description explains the fix and verification, but lacks the required Thinking Path, Risks, Model Used, and Checklist sections. AGENTS.md requires every template section to be filled in before merging. Please add the missing sections, including the reasoning from project context to this fix and the risks of changing version lookup.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/prepare-bundled-package.mjs
Line: 440
Comment:
**Required PR sections are missing**
The description explains the fix and verification, but lacks the required Thinking Path, Risks, Model Used, and Checklist sections. `AGENTS.md` requires every template section to be filled in before merging. Please add the missing sections, including the reasoning from project context to this fix and the risks of changing version lookup.
**Context Used:** AGENTS.md ([source](https://github.com/paperclipai/paperclip/blob/master/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This branch has not been deployed
1 participant
materializePublishManifest mapped every workspace:* specifier to the dependent's own version. That breaks git-source installs for independently-versioned workspace packages: server@0.3.1 depends on @paperclipai/plugin-sdk via workspace:* while plugin-sdk is at 1.0.0, so staging required the phantom 1.0.0...0.3.1 that npm cannot resolve (ETARGET on install).
This change adds readWorkspaceVersions(sourceRoot) (pnpm-pack semantics, cached, node_modules skipped) with an explicit workspaceVersions override, threads both through prepareBundledPackage, and keeps the old fallback for unknown targets (uniform-calver releases unaffected). Covers server->plugin-sdk and any future independently-versioned dep.
Verified: scripts/acpx-patch-packaging.test.mjs 31/31 green on this exact tree (3 new tests: target-version resolution, unknown-target fallback, workspace scan with node_modules shadowing excluded).