iframe-proxy

eljese · 2026-10-21T05:57:29Z

Git installs (installGitPayload) skipped the release.sh packaging prep: checkouts built from a git ref never ran the ref's own scripts/prepare-server-ui-dist.sh and never staged root skills/ into packages that declare files:[..., skills]. Result: git-installed servers missing ui-dist and skills payloads.

This change adds prepareGitInstallPackagingInputs and calls it in installGitPayload after the workspace build: it runs the ref's ui-dist prep script with PAPERCLIP_RELEASE_REUSE_UI_DIST=1 and copies root skills/ into packages that declare the skills dir but lack it. Every step is existence-gated, so refs predating ui-dist/skills install exactly as before.

Verified on exact tree: 4 new prepareGitInstallPackagingInputs tests pass 4/4. Full install-command file shows 4 pre-existing environment failures (suite requires Node >=24.11, environment runs Node 22 via hermes path; they abort in assertSupportedNodeVersion before reaching the new call).

installGitPayload packed workspace tarballs without the packaging prep that release.sh performs: server/ui-dist was missing (fresh git checkouts) and root skills/ never reached packages declaring a skills/ payload, so git-source installs shipped incomplete bundles. Add prepareGitInstallPackagingInputs, mirroring the release prep: build server/ui-dist via the ref's own script with PAPERCLIP_RELEASE_REUSE_UI_DIST, then copy root skills/ into packages that declare files: [.., skills] and lack the dir. Every step is existence-gated so older refs install as before. Covered by four hermetic cases (prep runs/skips, skills copy scope, no-clobber).

commitperclip · 2026-10-21T05:57:48Z

greptile-apps · 2026-10-21T06:02:01Z

Confidence Score: 4/5

[Medium impact] The change affects the managed Git-install packaging path.

The repository’s PR-description and code-comment requirements must be satisfied before merging.

Findings

Required PR sections are missing ▶
Added comment breaks contribution rules ▶
Tests miss the install connection ▶

Fix with agent prompt

### Issue 1
cli/src/commands/install.ts:87-91
The description explains the fix and test results, but omits Thinking Path, Risks, Model Used, and Checklist. `CONTRIBUTING.md` requires the PR template and all its sections. Add the missing sections before merging, including why you chose this approach and what could go wrong.

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 2
cli/src/commands/install.ts:92-95
The new four-line comment violates the directive in `CONTRIBUTING.md`: “Do not submit a PR that includes code comments.” Remove it before merging and keep the explanation in the PR description.

```suggestion

```

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
cli/src/__tests__/install-command.test.ts:413-419
All four new tests call `prepareGitInstallPackagingInputs` directly. The existing `installGitPayload` fixture creates neither the prep script nor root skills. These tests would still pass if the new call were removed or moved after packing.

Add an install-path test with both inputs. Check that they are staged before `prepare-bundled-package.mjs` or packing runs, so the tests catch the original bug returning.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Prepares package files before Git installs create their archives.

Git installs stage UI and skills before packing packages.

The required PR sections and the added code comment need attention. An install-path regression test would also protect the fix.

_{Reviews (1) · Last reviewed commit: "Stage packaging inputs for git installs ..." · Reviewed by Greptile}

greptile-apps · 2026-10-21T06:02:04Z

+export async function prepareGitInstallPackagingInputs(
+  checkoutPath: string,
+  runCommand: CommandRunner,
+  buildEnv: (extra?: NodeJS.ProcessEnv) => NodeJS.ProcessEnv,
+): Promise<void> {


Required PR sections are missing

The description explains the fix and test results, but omits Thinking Path, Risks, Model Used, and Checklist. CONTRIBUTING.md requires the PR template and all its sections. Add the missing sections before merging, including why you chose this approach and what could go wrong.

File Used: rules/pull-request-review.mdc (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: cli/src/commands/install.ts Line: 87-91 Comment: **Required PR sections are missing** The description explains the fix and test results, but omits Thinking Path, Risks, Model Used, and Checklist. `CONTRIBUTING.md` requires the PR template and all its sections. Add the missing sections before merging, including why you chose this approach and what could go wrong. **File Used:** `rules/pull-request-review.mdc` ([source](https://github.com/paperclipai/paperclip/blob/d7f207cbfea1701ca9f05cd5fe85aa4c5010f5bc/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!

greptile-apps · 2026-10-21T06:02:04Z

+  // Mirror scripts/release.sh packaging prep for git checkouts: build
+  // server/ui-dist via the ref's own script and stage root skills into
+  // packages that declare them. Every step is existence-gated so pre-ui-dist
+  // refs install exactly as before.


Added comment breaks contribution rules

The new four-line comment violates the directive in CONTRIBUTING.md: “Do not submit a PR that includes code comments.” Remove it before merging and keep the explanation in the PR description.

Suggested change

// Mirror scripts/release.sh packaging prep for git checkouts: build

// server/ui-dist via the ref's own script and stage root skills into

// packages that declare them. Every step is existence-gated so pre-ui-dist

// refs install exactly as before.

Context Used: CONTRIBUTING.md has a guide for a good PR message including a "thinking path". If the PR doesn't include a "thinking path" and "details about what you did, why you did it, why it matters & the benefits, how we can verify it works, and any risks", the... (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: cli/src/commands/install.ts Line: 92-95 Comment: **Added comment breaks contribution rules** The new four-line comment violates the directive in `CONTRIBUTING.md`: “Do not submit a PR that includes code comments.” Remove it before merging and keep the explanation in the PR description. ```suggestion ``` **Context Used:** CONTRIBUTING.md has a guide for a good PR message including a "thinking path". If the PR doesn't include a "thinking path" and "details about what you did, why you did it, why it matters & the benefits, how we can verify it works, and any risks", the... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0)) --- 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!

greptile-apps · 2026-10-21T06:02:04Z

+    it("runs the ref ui-dist prep script with the release reuse flag", async () => {
+      const checkout = createPackagingCheckout(path.join(root, "prep-checkout"), { withPrepScript: true, withRootSkills: false });
+      const runCommand = vi.fn<CommandRunner>(async () => ({ stdout: "", stderr: "" }));
+      await prepareGitInstallPackagingInputs(checkout, runCommand, testBuildEnv);
+      expect(runCommand).toHaveBeenCalledOnce();
+      expect(runCommand).toHaveBeenCalledWith("bash", ["scripts/prepare-server-ui-dist.sh"], expect.objectContaining({ cwd: checkout }));
+      expect(runCommand.mock.calls[0]?.[2]?.env).toMatchObject({ PAPERCLIP_RELEASE_REUSE_UI_DIST: "1" });


Tests miss the install connection

All four new tests call prepareGitInstallPackagingInputs directly. The existing installGitPayload fixture creates neither the prep script nor root skills. These tests would still pass if the new call were removed or moved after packing.

Add an install-path test with both inputs. Check that they are staged before prepare-bundled-package.mjs or packing runs, so the tests catch the original bug returning.

Prompt To Fix With AI

This is a comment left during a code review. Path: cli/src/__tests__/install-command.test.ts Line: 413-419 Comment: **Tests miss the install connection** All four new tests call `prepareGitInstallPackagingInputs` directly. The existing `installGitPayload` fixture creates neither the prep script nor root skills. These tests would still pass if the new call were removed or moved after packing. Add an install-path test with both inputs. Check that they are staged before `prepare-bundled-package.mjs` or packing runs, so the tests catch the original bug returning. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps Bot reviewed Oct 21, 2026

View reviewed changes

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

fix(install): stage packaging inputs for git installs (ui-dist + skills) - #17063

fix(install): stage packaging inputs for git installs (ui-dist + skills)#17063
eljese wants to merge 1 commit into
paperclipai:masterfrom
eljese:pr-upstream/install-git-packaging-compat

eljese commented Oct 21, 2026

Uh oh!

commitperclip Bot commented Oct 21, 2026

Uh oh!

greptile-apps Bot commented Oct 21, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Oct 21, 2026

Uh oh!

greptile-apps Bot Oct 21, 2026

Uh oh!

greptile-apps Bot Oct 21, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

eljese commented Oct 21, 2026

Uh oh!

commitperclip Bot commented Oct 21, 2026

Uh oh!

greptile-apps Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

greptile-apps Bot Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Oct 21, 2026 •

edited

Loading