mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-16 05:17:41 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Each pull request runs the Trusted PR CI. The `ci / Build` lane is the longest check, or close to the longest, in each green run. > - On 2026-10-13, `ci / Build` took 415-446s in runs 38426120144, 38432086560 and 38423713537. Of that, the public-install verifier took 226-241s. > - In the verifier, the consumer `npm install` took 119-129s. It downloads the full public registry graph into an empty npm cache. > - Source manifests set that graph. Thus CI can download it before `pnpm build` makes any tarball. > - This pull request fills the npm cache in the background while the Rust restore, Evalbook and `pnpm build` run. Then the verifier reads that cache. > - The benefit is a shorter `ci / Build` lane. The verifier keeps all its checks. ## Linked Issues or Issue Description Refs #15213, #15849, #15868, #15888 (earlier changes that made this verifier faster). **Enhancement description** The `ci / Build` lane runs the public-install verifier after `pnpm build`. The verifier starts with an empty npm cache. Its consumer install then downloads the full public registry graph: 119-129s of a 226-241s step. **Proposed change** Download the same registry graph into an npm cache in the background, during the earlier part of the lane. Give that cache to the verifier. **Why it helps** The download moves off the critical path of the lane. A local test of the same 628-package install took 51s with a cold cache and 21s with a warm cache. ## What Changed - Add `scripts/public-install-closure.mjs`. It holds the one definition of the public package closure: `@paperclipai/server`, `paperclipai`, and each public workspace package that they reach through `workspace:` specs. It also gives the registry dependencies of that closure. - Add `scripts/warm-public-install-npm-cache.mjs`. It installs those registry dependencies into a temporary consumer, with scripts disabled, to fill `PAPERCLIP_PUBLIC_INSTALL_NPM_CACHE`. Then it removes the temporary consumer. - `scripts/verify-grok-npm-install.mjs` now gets its closure from the shared module. When `PAPERCLIP_PUBLIC_INSTALL_NPM_CACHE` is set, it uses that directory as the consumer npm cache. When the variable is not set, it uses a new empty cache, as before. `docker-runner-check.yml` and `release-smoke.yml` do not change. - `pr-trusted.yml` (Build job): a new step starts the warm-up in the background after `Install dependencies`. The verify step waits for the warm-up for a maximum of 180s and then gives the cache to the verifier. If the warm-up fails, the step writes a notice and the verifier continues as before. - Add `scripts/__tests__/public-install-closure.test.mjs` and add it to `test:release-registry`. Update the closure assertions in `grok-public-install-sandbox.test.mjs`. ## Verification - `node --test scripts/__tests__/public-install-closure.test.mjs`: 4/4 pass. - `node --test scripts/__tests__/grok-public-install-sandbox.test.mjs`: 24/24 pass. - `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs scripts/__tests__/e2e-shard.test.mjs scripts/__tests__/runner-release-crate-cache.test.mjs`: all pass. - `release-verify-workflow.test.mjs`: the two "publisher lockfile" tests fail in my sandbox on clean master too, because they need pnpm. Two canary git-fixture tests timed out once under load. They pass when they run alone. - I ran the warm-up against current master: 62 direct dependencies, 628 packages, 51s cold. The same install with the warm cache took 21s. - Important: `pr.yml` calls `pr-trusted.yml@master`. Thus the CI of this pull request runs the refactored verifier on its default path (empty cache), but not the new warm-up steps. The warm-up steps start after merge. To measure them, compare the `consumer_install_ms` value in the `paperclip.public-npm-install.timing.v1` line of a Build job before and after merge. - Final review: all latest-head CI checks pass (54 successes, one neutral result, two intentional skips). Fresh Apex is 5/5 on `64d091b8b0c5359fa3dc0edb1fd7ef9d24294477` with no unresolved findings. The two relevant local script suites pass all 28 tests. Current master combines cleanly. ## Risks - Low risk. The verifier result does not change. npm checks each cached tarball against the integrity of its own resolution, and the consumer directory and lock start empty. - npm can use a cached registry packument that is a maximum of about 5 minutes old (registry `max-age`). This has the same effect as starting the verifier a few minutes earlier. - The warm-up runs at the same time as `pnpm build`. It uses network, disk and some CPU. In the measured runs, `pnpm build` took 76-96s. - If the warm-up is slow or fails, the verify step waits a maximum of 180s and then continues with what is in the cache. - The verifier receipt still reports `cleanNpmInstall: true`. The consumer install is clean. Only the download cache is shared. ## Expected time savings - `ci / Build`: an estimated 65-85s less, from 415-446s to about 340-370s. The consumer install goes from 119-129s to about 40-55s, from the local cold/warm ratio. - Full PR time: after this change, `ci / Build` is no longer the longest lane. The longest lanes become `General tests (server N/12)` and `Verify Paperclip Runner (vitest 1/2)` at about 405-496s. Thus the full PR time decreases only when Build is the critical path. That is approximately 0-30s in the runs above. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in a Paperclip agent run. OpenAI GPT-6 (`gpt-6`) through Codex assisted the final repository review and targeted script validation. This session does not expose an exact context-window size. ## 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 - [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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Paperclip <noreply@paperclip.ing>
50 lines
2.5 KiB
JavaScript
50 lines
2.5 KiB
JavaScript
// The public packages the public-install verifier stages: `@paperclipai/server`
|
|
// and `paperclipai` plus every public workspace package they reach through
|
|
// `workspace:` specs, in the depth-first order the verifier packs them. The
|
|
// npm cache warm-up reads the same closure from source manifests before
|
|
// `pnpm build` has produced anything, so both share this one definition.
|
|
import assert from 'node:assert/strict';
|
|
import { execFileSync } from 'node:child_process';
|
|
import { readFileSync } from 'node:fs';
|
|
import { join } from 'node:path';
|
|
|
|
export const PUBLIC_INSTALL_ROOTS = ['@paperclipai/server', 'paperclipai'];
|
|
|
|
// Returns `{ packages, needed }`: `packages` maps every public workspace
|
|
// package name to `{ dir, manifest }`, and `needed` is the closure in visit
|
|
// order.
|
|
export function publicInstallClosure(repo, roots = PUBLIC_INSTALL_ROOTS) {
|
|
const listing = execFileSync(process.execPath, [join(repo, 'scripts/release-package-map.mjs'), 'list'], { cwd: repo, stdio: 'pipe' })
|
|
.toString().trim().split('\n').map(line => line.split('\t'));
|
|
const packages = new Map(listing.map(([dir, name]) => [name, { dir, manifest: JSON.parse(readFileSync(join(repo, dir, 'package.json'), 'utf8')) }]));
|
|
const needed = new Set();
|
|
function visit(name) {
|
|
if (needed.has(name)) return;
|
|
const entry = packages.get(name); assert.ok(entry, `Missing public workspace dependency ${name}`); needed.add(name);
|
|
for (const [dep, spec] of Object.entries({ ...entry.manifest.dependencies, ...entry.manifest.optionalDependencies })) {
|
|
if (spec.startsWith('workspace:')) visit(dep);
|
|
}
|
|
}
|
|
for (const root of roots) visit(root);
|
|
return { packages, needed };
|
|
}
|
|
|
|
// The registry dependencies the closure's packages declare, as one consumer
|
|
// manifest. A name two packages declare with different specs keeps the first
|
|
// spec: the warm-up only fills a cache, and the real install fetches whatever
|
|
// it still lacks.
|
|
export function publicRegistryDependencies({ packages, needed }) {
|
|
const dependencies = {};
|
|
const optionalDependencies = {};
|
|
for (const name of needed) {
|
|
const { manifest } = packages.get(name);
|
|
for (const [section, target] of [['dependencies', dependencies], ['optionalDependencies', optionalDependencies]]) {
|
|
for (const [dep, spec] of Object.entries(manifest[section] ?? {})) {
|
|
if (needed.has(dep) || /^(workspace|file|link):/.test(spec) || dep in dependencies || dep in optionalDependencies) continue;
|
|
target[dep] = spec;
|
|
}
|
|
}
|
|
}
|
|
return { dependencies, optionalDependencies };
|
|
}
|