Commit Graph
4 Commits
Author SHA1 Message Date
2223b7de9e perf(ci): stop building the standalone packages inside the public-install verifier (#15908)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Every pull request runs the `Build` check. Its last step is the
public npm install verifier, `scripts/verify-grok-npm-install.mjs`,
which stages the eighteen public packages reachable from the server and
the CLI, packs them, and installs them in a clean consumer.
> - Before it stages anything, the verifier runs
`scripts/build-standalone-public-packages.mjs`. That script installs and
compiles the seven sandbox-provider plugins that live outside the pnpm
workspace: one network `pnpm install --ignore-workspace` and one `tsc`
per plugin. The line was copied from the build block of `release.sh`
when the verifier was written in #14024.
> - release.sh needs that build because it publishes those plugins. The
verifier does not for anything it stages. Its package closure follows
`workspace:` specs from the server and the CLI, and a `workspace:` spec
can only name a workspace package, so none of the eighteen packages it
packs is a standalone package and no tarball reads that build's output.
> - One optional path at the end of the verifier does read it:
qualification-package retention, which `docker-runner-check.yml` enables
with `PAPERCLIP_RUNNER_QUALIFICATION_PACKAGES_DIR` after running only
`pnpm build`, packs the Daytona plugin from its `dist`. The PR `Build`
job never sets that variable.
> - The timing line from #15868 and #15888 puts the verifier's staging
phase at 75.8s on the latest run, of which the staged sources, the
bundled installs and the packs account for 46.5s. The rest, about 29s,
is the work before staging, and the standalone build is almost all of
it. `Build` at 433-530s is the slowest check in most PR runs once the
server shards drop, so that time sits on the run's critical path.
> - This pull request runs the standalone build only when retention is
requested, keeps the `ui-dist` copy the server package needs, and times
both on the timing line so what is left before staging is visible.
> - The benefit is a `Build` job about 21-28s shorter, with no change to
what the verifier installs or checks, no loss of coverage for the
standalone build because Canary Dry Run still runs it through
`release.sh`, and no change to the retention path that reads it.

## Linked Issues or Issue Description

Replaces #15892, which GitHub closed automatically when the branch of
#15888 (its base at the time) was deleted on merge. Same two commits,
rebased onto `master` on top of #15888 (1ac48aff), #15868 (026bdcd0) and
#15861 (85f4205d), which are all merged. #15892 reached Greptile 5/5
with every check green on the same content; the review history is there.

**What existing behavior does this improve?**

The `Build` PR check. Its public npm install verifier builds the seven
standalone sandbox-provider plugins before it stages the eighteen
packages it installs, and none of those packages uses that build.

**Subsystem affected**

CI and release scripts (`scripts/verify-grok-npm-install.mjs`,
`scripts/build-standalone-public-packages.mjs`).

**Current behavior**

The verifier runs `node scripts/build-standalone-public-packages.mjs`
and then `bash scripts/prepare-server-ui-dist.sh` before it stages any
package, on every run. The first script removes each sandbox-provider
plugin's `node_modules`, runs `pnpm install --ignore-workspace` against
the registry, links the in-repo plugin SDK, and runs the plugin's `tsc`
build, four plugins at a time. On the last `Build` run before this
change
([38146108750](https://github.com/paperclipai/paperclip/actions/runs/38146108750))
`stage_packages_ms` was 75.8s and the staged sources, bundled installs
and packs inside it took 0.9s, 15.7s and 29.9s, which leaves about 29s
before staging. The verifier's package closure, computed in the same way
as the script computes it, contains eighteen workspace packages and no
sandbox-provider plugin.

**Proposed behavior**

Run the standalone build only when
`PAPERCLIP_RUNNER_QUALIFICATION_PACKAGES_DIR` is set, because that is
the one path that reads it, and report it as `stage_standalone_build_ms`
when it runs. Always run the `ui-dist` copy before staging. With
`PAPERCLIP_RELEASE_REUSE_UI_DIST=1`, which the verifier already sets,
that copies the `ui/dist` that `pnpm build` produced into
`server/ui-dist`, which the server package publishes. Report that copy
as `stage_ui_dist_ms` on the timing line. Leave `release.sh` unchanged,
so the release flow and Canary Dry Run still build the standalone
packages where they are published.

**Reason and benefit**

The verifier proves that the eighteen public packages install cleanly
from the public registry. The standalone build adds seven network
installs and compiles whose output is never packed. Removing it from the
PR path shortens the staging phase by about 21-28s. Measured on this
PR's first commit
([38150787810](https://github.com/paperclipai/paperclip/actions/runs/38150787810)):
`stage_packages_ms` 54.2s against 75.8s, with `stage_ui_dist_ms` 21ms,
and the time before staging, `stage_packages_ms` minus the four staging
keys, down from about 29s to under 0.1s.

**Breaking changes**

None. The same eighteen packages are staged, packed, checked and
installed from the same sources. The retention path builds exactly what
it built before. `scripts/build-standalone-public-packages.mjs` exports
two existing helpers for the new guard test and otherwise does not
change.

## What Changed

- `scripts/verify-grok-npm-install.mjs`: the
`build-standalone-public-packages.mjs` call before staging runs only
when `PAPERCLIP_RUNNER_QUALIFICATION_PACKAGES_DIR` is set, in the same
position as before, and reports `stage_standalone_build_ms`. The flag is
read once into `retainQualificationPackages`, and the retention call at
the end is gated by the same variable, so the build and the retention
that reads it cannot diverge. The `prepare-server-ui-dist.sh` call
stays, is timed, and reports `stage_ui_dist_ms`. The comment explains
why release.sh and the retention path need the standalone build and the
staged packages do not. The timing-line comment lists both new keys.
- `scripts/build-standalone-public-packages.mjs`:
`parseWorkspaceEntries` and `isWorkspacePackage` are exported. No
behavior change.
- `scripts/__tests__/grok-public-install-sandbox.test.mjs`: one new
guard test. It asserts the verifier references the standalone build
exactly once, inside the retention gate, timed under its own key, and
that no second reading of the retention variable exists; that
`retain-runner-qualification-packages.mjs` still packs the Daytona
plugin from its `dist`, and that the Daytona plugin is a standalone
package, so the gate cannot pass vacuously; recomputes the verifier's
package closure from `release-package-map.mjs list` and the manifests
through `workspace:` specs; checks that the public package map still
carries packages outside the pnpm workspace; checks that every package
in the closure is inside the workspace globs of `pnpm-workspace.yaml`;
checks that the server package still publishes `ui-dist`; and checks the
order: closure, gated standalone build, `ui-dist` copy, then staging,
with the reuse flag set and the timing keys written.

### Review fix, second commit

Greptile's P1 on the first commit was valid: `docker-runner-check.yml`
runs `pnpm build` and then the verifier with
`PAPERCLIP_RUNNER_QUALIFICATION_PACKAGES_DIR` set, and
`retainRunnerQualificationPackages` copies
`packages/plugins/sandbox-providers/daytona/dist`. The Daytona plugin is
excluded from the pnpm workspace, so `pnpm build` never produces that
directory and the first commit would have failed that job with `ENOENT`
after every install check had passed. The second commit restores the
build on exactly that path: the verifier builds the standalone packages
when, and only when, retention is requested, and the guard test pins the
gate to the reason for it.

## Verification

- `node --test scripts/__tests__/grok-public-install-sandbox.test.mjs
scripts/__tests__/pack-staged-packages.test.mjs
scripts/__tests__/stage-bundled-packages.test.mjs
scripts/__tests__/prefetch-child.test.mjs
scripts/__tests__/retain-runner-qualification-packages.test.mjs`: 37
tests pass.
- The updated guard test fails against the first commit's verifier (no
retention gate around the build) and passes with the second commit. The
first commit's test failed against #15888's verifier the same way.
- `node --check` passes on the verifier.
- The verifier itself needs Docker and a network, which this workspace
does not have. The first commit's `Build` job
([38150787810](https://github.com/paperclipai/paperclip/actions/runs/38150787810))
ran the branch's verifier and measured the saving: `stage_packages_ms`
54.2s (was 75.8s), `stage_ui_dist_ms` 21ms, `stage_sources_ms` 0.8s,
`stage_bundled_ms` 21.0s, `stage_pack_ms` 32.4s, so under 0.1s remains
before staging where about 29s was. The second commit does not change
the PR path: the gate is false on every PR run, so that job's timing
line should show the same shape with no `stage_standalone_build_ms` key.
- The retention path was not run here: it needs Docker and the
`docker-runner-check` workflow's inputs. The code on that path is the
same `run(process.execPath, [...build-standalone-public-packages.mjs])`
call the verifier made before this PR, in the same position.
- `scripts/__tests__/release-dry-run-notes.test.mjs` exercises the
standalone build inside `release.sh` and is unchanged; 11 of its 12
tests spawn `pnpm`, which is not installed in this workspace, and fail
the same way on #15888's head. They pass in CI, where `pnpm` is
installed.
- The first commit's run failed only on `General tests (server
(11/12))`: all 74 files and 1257 tests passed, then Vitest caught a
teardown `EACCES` removing the per-worker `PAPERCLIP_HOME` under
`native-session-resumption.test.ts`, the same error as the first attempt
of #15888's run 38146108750. Neither PR touches server code; the failure
is noted on the tracking issue.

## Risks

- If a future change makes a package the verifier installs depend on a
standalone package, the verifier would stage an unbuilt `dist`. That
cannot happen through a `workspace:` spec, which is the only kind the
closure follows, and the guard test fails if the closure ever contains a
package outside the workspace globs.
- If a future path in the verifier reads a standalone build without
setting the retention variable, it would see no `dist`. The guard test
pins the single reference to the gate; a new reader has to extend the
gate and the test together.
- The sandbox-provider plugins' builds are no longer exercised on every
PR by the `Build` job. They are still built by `release.sh` in Canary
Dry Run on every PR, by the verifier whenever retention is requested,
and the `build-standalone-concurrency` and `release-dry-run-notes` tests
still cover the script.
- Low risk otherwise. The bundled installs, the packs, the consumer
install, and every probe are unchanged.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

- Claude Fable 5.1 (Anthropic), model ID `claude-fable-5-1`, run as a
Paperclip agent through the Claude Code local adapter with tool use
(shell, file edits, GitHub CLI). Reasoning mode: extended thinking.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green (CI is running on the second
commit)
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(the P1 on the first commit is fixed in the second; re-review pending)
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Bender (Fable) <bender-fable@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>
2026-10-11 12:14:32 -07:00
Devin FoleyandPaperclip ef37203a48 perf(ci): build standalone public packages concurrently (#8567)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - CI runs a Canary Dry Run job that exercises `release.sh`, which
builds the standalone sandbox-provider packages for publish
> - That step (`scripts/build-standalone-public-packages.mjs`) built the
7 provider plugins serially — each doing `rm -rf dist && tsc` — making
it the dominant cost (~49s) inside the slowest PR check (~4.9m wall)
after the general-server lane was already sharded
> - The packages are independent (their own `node_modules` via
`--ignore-workspace`, their own `dist`), so the serial build is pure
latency with no correctness benefit
> - This pull request builds them with a bounded-concurrency pool sized
to the runner CPU count (overridable via
`STANDALONE_BUILD_CONCURRENCY`), buffering each package's output and
flushing it as one block so parallel logs stay readable, and aggregating
failures by original index
> - The benefit is a faster Canary Dry Run / PR feedback loop without
changing what gets built or published

## Linked Issues or Issue Description

No public GitHub issue exists. Inline feature/perf description:

### Problem or motivation

`build-standalone-public-packages.mjs` builds standalone provider
packages serially, making it the largest single cost inside the slowest
PR check.

### Proposed solution

Run independent per-package builds through a bounded-concurrency worker
pool sized to runner CPU count, with an env override and readable
buffered logs.

### Alternatives considered

Keep the serial build for simpler logs, but that preserves the avoidable
CI latency.

### Roadmap alignment

This is CI maintenance and does not overlap planned core roadmap work.

## What Changed

- `scripts/build-standalone-public-packages.mjs`: replaced the serial
per-package build loop with a bounded-concurrency pool (default = runner
CPU count, override via `STANDALONE_BUILD_CONCURRENCY`); per-package
stdout/stderr is buffered and flushed as a single block; failures are
aggregated by original package index so one failure neither aborts the
others mid-flight nor obscures which package broke.
- `scripts/__tests__/build-standalone-concurrency.test.mjs`: new
`node:test` unit suite covering the pool (limit respected, all items
run, ordered failure aggregation, env-override resolution).
- `.github/workflows/pr.yml`: wired the new unit test into the policy
job.

## Verification

- `node --test
./scripts/__tests__/build-standalone-concurrency.test.mjs` → 6/6 pass
- `node ./scripts/release-package-map.mjs check` → OK (29 enabled for CI
publish)
- `git diff --check origin/master..HEAD` → clean

## Risks

- Low risk. Build inputs/outputs are unchanged; only scheduling differs.
The concurrency is bounded by CPU count and overridable; output is
buffered per package so logs remain attributable. If a package fails,
all failures are still reported with their package index.

## Model Used

- Claude (Anthropic), `claude-opus-4-8`, extended thinking with tool
use.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-06-23 17:52:12 -07:00
Devin FoleyandPaperclip 93291df5c8 fix(plugins): move dev SDK linking out of plugin postinstall scripts (#8255)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Sandbox-provider plugins (cloudflare, daytona, e2b, exe-dev,
kubernetes, modal, novita) and `plugin-workspace-diff` are published as
standalone npm packages, but during local dev they need the in-repo
`@paperclipai/plugin-sdk` symlinked in
> - Each of these plugins shipped a `postinstall` lifecycle script that
traversed *out* of its own package directory (`node
../../../../scripts/link-plugin-dev-sdk.mjs`) to do that linking
> - The publishable manifest is built by a `prepack` whitelist that
drops the `scripts` field, so npm consumers don't see the postinstall
today — but that safety property depends entirely on `prepack` running
on every publish. A publish that skips lifecycle scripts would ship a
tarball whose postinstall escapes its package directory at consumer
install time
> - This pull request removes the escape-the-package-dir lifecycle
script from every plugin source manifest and moves the dev linking to a
single root-level postinstall that iterates the excluded plugin
directories itself
> - The benefit is that plugin tarballs can no longer carry an
install-time script that reaches outside their own directory, regardless
of whether `prepack` runs

## Linked Issues or Issue Description

This is a follow-up hardening change flagged during review of the Novita
sandbox provider PR (#7595).

**Problem (security):** Excluded plugin packages each carried
`"postinstall": "node ../../../../scripts/link-plugin-dev-sdk.mjs"`. The
relative path traverses outside the package root. Today the published
manifest is sanitized by a `prepack` whitelist that drops `scripts`, so
consumers are unaffected in the normal publish path. The risk is that
this is a defense-in-depth gap: if a publish ever skips lifecycle
scripts (e.g. `npm publish --ignore-scripts` is *not* used, or a tool
publishes the raw manifest), the tarball would ship a postinstall that
runs out-of-tree code at the consumer's install time.

## What Changed

- Added a single root `package.json` `postinstall`: `node
scripts/link-plugin-dev-sdk.mjs`.
- Rewrote `scripts/link-plugin-dev-sdk.mjs` to iterate the excluded
plugin directories itself (`packages/plugins/sandbox-providers/*` + the
orchestration smoke example) instead of relying on each plugin to invoke
it from its own cwd. Preserves both prior behaviors: leave a real
installed SDK dir alone, and skip when already correctly symlinked
(idempotent).
- Removed `scripts.postinstall` from all 7 sandbox-provider plugins
(cloudflare, daytona, e2b, exe-dev, kubernetes, modal, novita).
- Removed `scripts.postinstall` from `plugin-workspace-diff` (a pnpm
workspace member — pnpm already links the SDK, so the script was a no-op
there).

## Verification

- `node scripts/link-plugin-dev-sdk.mjs` from repo root: links the SDK
into the excluded plugins and reports skipped (already-linked) dirs;
re-running is idempotent.
- `grep -r "link-plugin-dev-sdk" packages/plugins/*/package.json
packages/plugins/sandbox-providers/*/package.json` returns no matches —
no plugin source manifest references the linker any longer.
- All affected `package.json` files re-validated as parseable JSON.

## Risks

Low risk. Dev-only tooling: the linker only runs at the repo root during
local install and only touches `node_modules/@paperclipai/plugin-sdk`
symlinks inside excluded plugin dirs. No change to published plugin
behavior or runtime code. Worst case if the root postinstall failed to
run, local dev of an excluded plugin would not find the SDK symlink —
easily re-run manually.

## Model Used

Claude Opus (claude-opus-4-8), extended reasoning, with tool use / code
execution in an agentic coding harness.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues OR (b) described the
issue in-PR following the relevant issue template
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable (added
`scripts/link-plugin-dev-sdk.test.js`, wired into
`test:release-registry`)
- [ ] If this change affects the UI, I have included before/after
screenshots (N/A — no UI change)
- [ ] I have updated relevant documentation to reflect my changes (N/A)
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-06-18 07:45:53 -07:00
Devin Foley 40782f703d Fix release packaging for standalone public packages (#4494)
## Thinking Path

> - Paperclip orchestrates AI agents for zero-human companies, and the
sandbox-provider work just moved E2B into a standalone publishable
plugin package.
> - That plugin is intentionally excluded from the root pnpm workspace
so it can model third-party install behavior without forcing lockfile
churn in the main repo.
> - The merged architecture change exposed a follow-up release problem:
the canary publish workflow tried to publish `@paperclipai/plugin-e2b`,
but the tarball had no `dist/` payload because standalone public
packages were not being built in the release path.
> - That means the release pipeline needed a packaging fix in core
release tooling, not another architectural change in the sandbox
provider itself.
> - This pull request adds a generic release step for public packages
that live outside the pnpm workspace, instead of hardcoding E2B-specific
behavior into the release script.
> - The benefit is that standalone publishable packages can be built and
packed correctly during release, including future sandbox-provider
plugins that follow the same pattern.

## What Changed

- Added `scripts/build-standalone-public-packages.mjs` to discover
public packages outside the pnpm workspace, run a clean package-local
install, and build them before publish.
- Updated `scripts/release.sh` to invoke that helper immediately after
the normal workspace build step.
- Kept the behavior generic by driving off the existing public package
map and pnpm workspace patterns rather than special-casing
`@paperclipai/plugin-e2b`.

## Verification

- `rm -rf packages/plugins/sandbox-providers/e2b/dist`
- `node ./scripts/build-standalone-public-packages.mjs`
- `cd packages/plugins/sandbox-providers/e2b && npm pack --dry-run`
- Confirm the tarball now includes the rebuilt `dist/` files instead of
only `README.md` / `package.json`

## Risks

- Low risk: this only changes the release build path for public packages
outside the pnpm workspace.
- The helper performs a clean package-local install for each standalone
public package, so release time may increase slightly as more such
packages are added.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

- OpenAI Codex via `codex_local`
- Model ID: `gpt-5.4`
- Reasoning effort: `high`
- Context window observed in runtime session metadata: `258400` tokens
- Capabilities used: terminal tool execution, git, GitHub CLI, and local
build/test inspection

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] If this change affects the UI, I have included before/after
screenshots
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] I will address all Greptile and reviewer comments before
requesting merge
2026-04-25 12:16:23 -07:00