mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 06:16:06 -10:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Every pull request runs the `ci / Build` check. This check was the slowest check in recent green runs: 432s in run 39254079691, 480s in run 39251304886 and 472s in run 39245233967. > - The slowest step in Build is "Verify built-in Grok from a clean public npm install" (162s and 207s). Its consumer `npm install` took 69s and 75s, even though a background step fills the npm cache first. > - The loopback registry in `scripts/public-install-registry.mjs` sends packages that it does not serve to npmjs with a 302 redirect. It escaped scoped names with `encodeURIComponent` (`%40scope%2Fname`). npm requests and caches the same metadata as `@scope%2fname`, and npm keys cached metadata by URL. > - Thus the consumer install could not use the scoped metadata that the warm-up had just put in the cache. It downloaded and parsed all 291 scoped packuments again (about 285 MB of full packument JSON) inside the 4-vCPU container. > - This pull request makes the redirect use the same escaping as npm. > - The benefit is a shorter Build check on every pull request. The installed bytes do not change. ## Linked Issues or Issue Description **Describe the bug** The consumer install in the public-install verifier does not use cached scoped package metadata. The warm-up step (`scripts/warm-public-install-npm-cache.mjs`) caches this metadata through direct npm requests. **Steps to reproduce** 1. Run `PAPERCLIP_PUBLIC_INSTALL_NPM_CACHE=/tmp/c node scripts/warm-public-install-npm-cache.mjs`. 2. Install the same dependency set through the loopback registry with `npm_config_cache=/tmp/c` and `--loglevel http`. 3. Count the `(cache miss)` lines. **Expected behavior** No cache misses for metadata that the warm-up already fetched. **Actual behavior** 291 cache misses. All of them are scoped packages that are requested as `https://registry.npmjs.org/%40scope%2Fname`. The cache holds them as `https://registry.npmjs.org/@scope%2fname`. Related: #16513 changes when the verifier waits for the warm-up. This pull request does not overlap with it, and the two changes add together. ## What Changed - `scripts/public-install-registry.mjs`: the 302 redirect for packages that the registry does not serve now uses npm's own escaping (`name.replace('/', '%2f')`, the same as npm-package-arg `escapedName`). The `packageName` check already allows only characters that need no other escaping. - `scripts/__tests__/public-install-registry.test.mjs`: the test now expects `@other%2fpkg` for a scoped redirect. A new assertion covers an unscoped redirect. ## Verification - `node --test scripts/__tests__/public-install-registry.test.mjs`: 3 of 3 pass. This includes the real npm install test that compares installed bytes from file installs and registry installs. - Local replay (Linux, npm 11.12.1). I filled a fresh cache with the warm-up script, then installed the same 62-dependency set (627 packages) through the registry: - Before: 291 cache misses, 28.3s. - After: 0 cache misses, 19.8s. - I also ran the real `scripts/public-install-registry.mjs` with a packed root package that depends on the full public dependency set. Result: 2 misses (only the loopback package and its tarball, as expected) and 1438 cache hits. - To compare on CI, look at `consumer_install_ms` in the `paperclip.public-npm-install.timing.v1` line of the Build log. Baseline: 68822 ms (run 39254079691) and 75175 ms (run 39251304886). ## Risks - Low risk. The redirect target is the same npmjs document. Only the URL escaping changes. npm still checks every tarball against the integrity in the packument. - If the npm cache holds stale scoped metadata, npm revalidates it the same way as it does for direct requests (`cache-control: max-age=300`). - The expected time saving comes from a local measurement. The real Build saving depends on the network and CPU of the runner. I expect about 10–20s per Build run. The CI timing line will confirm the number. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code as a Paperclip agent with tool use (shell, file edit). I did the root-cause analysis, the local replays and the code change with this model. ## 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 - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: devinfoley <[email protected]> Co-authored-by: Paperclip <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
89 lines
4.2 KiB
JavaScript
89 lines
4.2 KiB
JavaScript
import assert from 'node:assert/strict';
|
|
import { execFileSync, spawn } from 'node:child_process';
|
|
import { createHash } from 'node:crypto';
|
|
import { createReadStream, readFileSync, readdirSync, writeFileSync } from 'node:fs';
|
|
import { createServer } from 'node:http';
|
|
import { join } from 'node:path';
|
|
import { pipeline } from 'node:stream/promises';
|
|
import { fileURLToPath } from 'node:url';
|
|
|
|
const catalogFile = 'registry-packages.json';
|
|
const packageName = /^(?:@[a-z0-9_.-]+\/)?[a-z0-9_.-]+$/;
|
|
|
|
export async function writePackedRegistryCatalog(directory) {
|
|
const entries = [];
|
|
for (const file of readdirSync(directory).filter(file => file.endsWith('.tgz')).sort()) {
|
|
const archive = join(directory, file);
|
|
const manifest = JSON.parse(execFileSync('tar', ['-xOf', archive, 'package/package.json'], {
|
|
encoding: 'utf8', maxBuffer: 1024 * 1024,
|
|
}));
|
|
const hash = createHash('sha512');
|
|
for await (const chunk of createReadStream(archive)) hash.update(chunk);
|
|
entries.push({ file, manifest, integrity: `sha512-${hash.digest('base64')}` });
|
|
}
|
|
assert.ok(entries.length, 'The registry requires actual packed artifacts');
|
|
writeFileSync(join(directory, catalogFile), JSON.stringify(entries));
|
|
return entries;
|
|
}
|
|
|
|
export async function startPackedRegistry(directory) {
|
|
const entries = JSON.parse(readFileSync(join(directory, catalogFile), 'utf8'));
|
|
const packages = new Map();
|
|
for (const entry of entries) {
|
|
assert.match(entry.file, /^[a-zA-Z0-9._-]+\.tgz$/);
|
|
assert.match(entry.manifest.name, packageName);
|
|
assert.equal(typeof entry.manifest.version, 'string');
|
|
assert.match(entry.integrity, /^sha512-[A-Za-z0-9+/]+={0,2}$/);
|
|
assert.ok(!packages.has(entry.manifest.name), 'Duplicate packed package');
|
|
packages.set(entry.manifest.name, entry);
|
|
}
|
|
let url;
|
|
const server = createServer((request, response) => {
|
|
if (request.method !== 'GET') { response.writeHead(405).end(); return; }
|
|
let path;
|
|
try { path = decodeURIComponent(new URL(request.url, 'http://localhost').pathname.slice(1)); }
|
|
catch { response.writeHead(400).end(); return; }
|
|
const tarball = /^-\/(\d+)\.tgz$/.exec(path);
|
|
if (tarball) {
|
|
const entry = entries[Number(tarball[1])];
|
|
if (!entry) { response.writeHead(404).end(); return; }
|
|
response.writeHead(200, { 'content-type': 'application/octet-stream' });
|
|
pipeline(createReadStream(join(directory, entry.file)), response).catch(() => response.destroy());
|
|
return;
|
|
}
|
|
if (!packageName.test(path)) { response.writeHead(404).end(); return; }
|
|
const entry = packages.get(path);
|
|
if (!entry) {
|
|
response.writeHead(302, { location: `https://registry.npmjs.org/${path.replace('/', '%2f')}` }).end();
|
|
return;
|
|
}
|
|
const { manifest, integrity } = entry;
|
|
const version = { ...manifest, dist: { tarball: `${url}/-/${entries.indexOf(entry)}.tgz`, integrity } };
|
|
response.writeHead(200, { 'content-type': 'application/json' });
|
|
response.end(JSON.stringify({ name: manifest.name, 'dist-tags': { latest: manifest.version }, versions: { [manifest.version]: version } }));
|
|
});
|
|
await new Promise((resolve, reject) => { server.once('error', reject); server.listen(0, '127.0.0.1', resolve); });
|
|
url = `http://127.0.0.1:${server.address().port}`;
|
|
return {
|
|
url,
|
|
specs: entries.map(({ manifest }) => `${manifest.name}@${manifest.version}`),
|
|
close: () => new Promise((resolve, reject) => {
|
|
server.close(error => error ? reject(error) : resolve());
|
|
server.closeAllConnections();
|
|
}),
|
|
};
|
|
}
|
|
|
|
if (process.argv[1] === fileURLToPath(import.meta.url)) {
|
|
assert.equal(process.argv.length, 3, 'Usage: public-install-registry.mjs PACKED_ASSETS');
|
|
const registry = await startPackedRegistry(process.argv[2]);
|
|
try {
|
|
const status = await new Promise((resolve, reject) => {
|
|
const child = spawn('npm', ['install', '--registry', registry.url, '--replace-registry-host=never', '--ignore-scripts', '--omit=dev', ...registry.specs], { stdio: 'inherit' });
|
|
child.once('error', reject);
|
|
child.once('exit', (code, signal) => signal ? reject(new Error(`npm install received ${signal}`)) : resolve(code));
|
|
});
|
|
process.exitCode = status;
|
|
} finally { await registry.close(); }
|
|
}
|