mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-02 02:07:25 +08:00
perf(release): wait for npm registry visibility of all packages concurrently (#13495)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The release system publishes ~33 public npm packages per release across the canary, nightly, beta, and stable channels > - `scripts/release.sh` publishes them strictly sequentially: publish one package, poll npm until that version is registry-visible, then start the next > - npm accepts a publish in seconds, but registry packument propagation can lag minutes per package (the CI budget was raised to 30 minutes per package after two aborted releases), so the total publish time is the sum of every package's lag — about two hours on a bad npm day, paid by every channel run including every canary on every master push > - This pull request keeps the publishes sequential but runs all the registry visibility polls concurrently once every publish is accepted > - The benefit is that the wall-clock cost of npm propagation drops from the sum of all packages' lag to the single slowest package's lag, with every existing safety property preserved ## Linked Issues or Issue Description No existing issue. Description follows the enhancement template: **What existing behavior does this improve?** The npm publish step of `scripts/release.sh` (Step 5), used by every release channel. **Subsystem affected** Release tooling (`scripts/release.sh`, `scripts/release-lib.sh`). **Current behavior** Packages publish one at a time, and after each publish the script polls npm until that package's version is visible in the registry packument before publishing the next. With per-package propagation lag of minutes (observed up to ~15 minutes; per-package CI budget is 30 minutes), the full 33-package set takes up to ~2 hours of mostly idle waiting. **Proposed behavior** Phase 1 publishes every package sequentially exactly as today (a rejected publish still aborts the batch immediately with exact attribution). Phase 2 then polls registry visibility for all packages concurrently. Each package keeps its own `NPM_PUBLISH_VERIFY_ATTEMPTS` × `NPM_PUBLISH_VERIFY_DELAY_SECONDS` budget, and any version that never becomes visible still hard-fails the release, now naming every straggler. **Reason and benefit** Total publish wait becomes the slowest single package's lag instead of the sum of all lags — typically minutes instead of hours. This shortens every canary, nightly, beta, and stable run and reduces exposure to job timeouts during npm slowdowns. **Breaking changes** None. Dry-run output is byte-identical in structure, dist-tags are still applied at publish time (`--tag`), the Sigstore TLOG duplicate-recovery path is untouched, and the later dist-tag integrity check (`wait_for_release_registry_state`) is unchanged. ## What Changed - `scripts/release-lib.sh`: replaced `publish_package_to_npm_and_wait` with `wait_for_npm_package_versions`, which takes the package tuple list and polls every package's visibility in background subshells, each reusing the existing `wait_for_npm_package_version` poll (same per-package budget), then reports per-package success or fails naming all stragglers - `scripts/release.sh` Step 5: the publish loop calls `publish_package_to_npm` only (sequential, fail-fast on a rejected publish), followed by one call to `wait_for_npm_package_versions` for the whole set; Step 6's recap line updated to match - `scripts/release-lib.test.mjs`: the registry-visibility and workflow-budget tests now drive the new function (same assertions on `npm view` counts, virtual sleeps, and the fail-closed message, which now names the straggler); a new cross-visibility test proves concurrency — two fake packages that each become visible only after the other has been polled can only converge when polled in parallel, so the test fails if the waits ever serialize again Safety analysis for the ordering change: nothing in the publish loop resolves sibling packages from the registry. `prepare-bundled-package.mjs` bundles and patches from the local workspace tree, and the TLOG duplicate-recovery path only queries the package it just published. The only consumer of the "visible before next publish" invariant was the release script's own final verification, which still runs against the full set. ## Verification - `node --test scripts/release-lib.test.mjs` — 15 tests pass, including the new concurrency proof and the existing 15-minute-20-second budget tolerance test against the new function - `npm run test:release-registry` — full lane, 140 tests pass - `bash -n` on both scripts; `shellcheck` reports no findings beyond the three pre-existing ones on master (verified by comparing counts against `origin/master` copies) - A real-release exercise happens on the next master push: every canary run executes this exact path ## Risks - Low. The failure mode most worth watching is a release where some packages become visible and others never do: previously the run stopped at the first invisible package with later packages unpublished; now all packages are accepted before visibility is enforced, and the run fails naming every straggler. Recovery is identical in both worlds (the next attempt derives a new version number), and the accepted-but-lagging packages carry the correct dist-tag either way. - Publish jobs run the source commit's copy of `release.sh`, so this change takes effect for a given channel only once its source commit includes this merge — promoted nightlies/betas cut from older commits keep the old sequential behavior until their trains catch up. ## Model Used Claude Fable 5 (`claude-fable-5`), extended thinking with tool use (Claude Code). ## 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
This commit is contained in:
Devin Foley
+71
-13
@@ -410,22 +410,80 @@ publish_package_to_npm() {
|
||||
return 1
|
||||
}
|
||||
|
||||
publish_package_to_npm_and_wait() {
|
||||
local dist_tag="$1"
|
||||
local package_name="$2"
|
||||
local package_version="$3"
|
||||
local publish_tool="${4:-pnpm}"
|
||||
local attempts="${5:-12}"
|
||||
local delay_seconds="${6:-5}"
|
||||
# Wait for every already-published package to become registry-visible,
|
||||
# polling all of them concurrently. npm accepts a publish in seconds, but
|
||||
# packument propagation through the registry CDN can lag minutes per package;
|
||||
# waiting on each package before publishing the next made the total wait the
|
||||
# SUM of every package's lag (~2 hours on a bad day for the full set). Every
|
||||
# publish has already been accepted by the time this runs, so the polls can
|
||||
# race: the wall-clock cost becomes the single slowest package's lag. Each
|
||||
# package keeps its own attempts x delay budget, and a package that never
|
||||
# becomes visible still fails the release, naming every straggler.
|
||||
#
|
||||
# $3 is the list_public_package_info tuple list (pkg_dir<TAB>name<TAB>version
|
||||
# lines); the directory field is ignored.
|
||||
wait_for_npm_package_versions() {
|
||||
local attempts="${1:-12}"
|
||||
local delay_seconds="${2:-5}"
|
||||
local package_info="$3"
|
||||
|
||||
publish_package_to_npm "$dist_tag" "$package_name" "$package_version" "$publish_tool" || return 1
|
||||
# The polling phase runs in a subshell that owns its own EXIT trap: a
|
||||
# cancelled or signalled release reaps every in-flight poller and the
|
||||
# scratch directory instead of leaking one npm poll per package for the
|
||||
# rest of its budget. The subshell also keeps this trap from clobbering
|
||||
# the caller's cleanup trap.
|
||||
(
|
||||
local status_dir
|
||||
local pids=()
|
||||
local specs=()
|
||||
local failures=()
|
||||
local index=0
|
||||
local pkg_name
|
||||
local pkg_version
|
||||
local i
|
||||
|
||||
if wait_for_npm_package_version "$package_name" "$package_version" "$attempts" "$delay_seconds"; then
|
||||
return 0
|
||||
fi
|
||||
status_dir="$(mktemp -d "${TMPDIR:-/tmp}/paperclip-release-visibility.XXXXXX")"
|
||||
|
||||
release_warn "npm accepted ${package_name}@${package_version}, but the version did not become registry-visible."
|
||||
return 1
|
||||
# shellcheck disable=SC2329 # invoked via the trap below
|
||||
reap_visibility_pollers() {
|
||||
local pid
|
||||
for pid in ${pids[@]+"${pids[@]}"}; do
|
||||
kill "$pid" 2>/dev/null || true
|
||||
done
|
||||
rm -rf "$status_dir"
|
||||
}
|
||||
trap reap_visibility_pollers EXIT INT TERM
|
||||
|
||||
while IFS=$'\t' read -r _pkg_dir pkg_name pkg_version; do
|
||||
[ -z "$pkg_name" ] && continue
|
||||
(
|
||||
if wait_for_npm_package_version "$pkg_name" "$pkg_version" "$attempts" "$delay_seconds"; then
|
||||
: > "$status_dir/$index.ok"
|
||||
fi
|
||||
) &
|
||||
pids+=("$!")
|
||||
specs+=("${pkg_name}@${pkg_version}")
|
||||
index=$((index + 1))
|
||||
done <<< "$package_info"
|
||||
|
||||
if [ "${#pids[@]}" -gt 0 ]; then
|
||||
for i in "${!pids[@]}"; do
|
||||
wait "${pids[$i]}" || true
|
||||
if [ -e "$status_dir/$i.ok" ]; then
|
||||
release_info " ✓ ${specs[$i]} is registry-visible"
|
||||
else
|
||||
failures+=("${specs[$i]}")
|
||||
fi
|
||||
done
|
||||
fi
|
||||
|
||||
if [ "${#failures[@]}" -gt 0 ]; then
|
||||
release_warn "npm accepted every publish, but these versions did not become registry-visible: ${failures[*]}"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
exit 0
|
||||
)
|
||||
}
|
||||
|
||||
verify_npm_installable() {
|
||||
|
||||
@@ -29,6 +29,7 @@ function runPublishHelper({
|
||||
npmVersionExistsAfterChecks = 0,
|
||||
verifyAttempts = 1,
|
||||
verifyDelaySeconds = 0,
|
||||
visibilityPackages = null,
|
||||
}) {
|
||||
const fixtureDir = mkdtempSync(join(tmpdir(), "paperclip-release-lib-"));
|
||||
const binDir = join(fixtureDir, "bin");
|
||||
@@ -86,6 +87,26 @@ exit 1
|
||||
set -euo pipefail
|
||||
printf 'npm %s\\n' "$*" >> "$FAKE_CALL_LOG"
|
||||
if [ "$1" = "view" ]; then
|
||||
if [ "\${NPM_VIEW_CROSS:-}" = "true" ]; then
|
||||
# Cross-visibility mode: a package resolves only after every OTHER
|
||||
# package has been polled at least once. Mutually dependent visibility
|
||||
# can only converge when the polls run concurrently.
|
||||
spec="$2"
|
||||
name="\${spec%@*}"
|
||||
safe="$(printf '%s' "$name" | tr '/@' '__')"
|
||||
touch "$FAKE_STATE_DIR/seen-$safe"
|
||||
all_seen=true
|
||||
for other in $CROSS_PACKAGES; do
|
||||
osafe="$(printf '%s' "$other" | tr '/@' '__')"
|
||||
[ "$osafe" = "$safe" ] && continue
|
||||
[ -f "$FAKE_STATE_DIR/seen-$osafe" ] || all_seen=false
|
||||
done
|
||||
if [ "$all_seen" = "true" ]; then
|
||||
echo "1.2.3"
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
checks=0
|
||||
if [ -f "$FAKE_STATE_DIR/view-checks" ]; then
|
||||
read -r checks < "$FAKE_STATE_DIR/view-checks"
|
||||
@@ -147,15 +168,30 @@ exec npm "$@"
|
||||
);
|
||||
|
||||
const shellOptions = callerPipefail ? "set -euo pipefail" : "set -eu";
|
||||
// The cross-visibility concurrency test needs real (fractional-second)
|
||||
// sleeps so slower-starting sibling polls get a chance to run; every other
|
||||
// test records virtual waiting so registry-delay tests stay fast/offline.
|
||||
const sleepStub = visibilityPackages
|
||||
? ""
|
||||
: `sleep() { printf 'sleep %s\\n' "$*" >> "$FAKE_CALL_LOG"; }`;
|
||||
const packageInfo = (
|
||||
visibilityPackages ?? ["@paperclipai/example"]
|
||||
)
|
||||
.map((name) => `packages/example\\t${name}\\t1.2.3`)
|
||||
.join("\\n");
|
||||
const script = `
|
||||
${shellOptions}
|
||||
source "${repoRoot}/scripts/release-lib.sh"
|
||||
# Record virtual waiting so registry-delay tests remain fast and offline.
|
||||
sleep() { printf 'sleep %s\\n' "$*" >> "$FAKE_CALL_LOG"; }
|
||||
${sleepStub}
|
||||
${
|
||||
waitForRegistry
|
||||
? `publish_package_to_npm_and_wait ${distTag} @paperclipai/example 1.2.3 ${publishTool} "$VERIFY_ATTEMPTS" "$VERIFY_DELAY_SECONDS"`
|
||||
: `publish_package_to_npm ${distTag} @paperclipai/example 1.2.3 ${publishTool}`
|
||||
visibilityPackages
|
||||
? `PACKAGE_INFO="$(printf '${packageInfo}')"
|
||||
wait_for_npm_package_versions "$VERIFY_ATTEMPTS" "$VERIFY_DELAY_SECONDS" "$PACKAGE_INFO"`
|
||||
: waitForRegistry
|
||||
? `publish_package_to_npm ${distTag} @paperclipai/example 1.2.3 ${publishTool}
|
||||
PACKAGE_INFO="$(printf '${packageInfo}')"
|
||||
wait_for_npm_package_versions "$VERIFY_ATTEMPTS" "$VERIFY_DELAY_SECONDS" "$PACKAGE_INFO"`
|
||||
: `publish_package_to_npm ${distTag} @paperclipai/example 1.2.3 ${publishTool}`
|
||||
}
|
||||
`;
|
||||
|
||||
@@ -174,6 +210,8 @@ ${
|
||||
NPM_VERSION_EXISTS_AFTER_CHECKS: String(npmVersionExistsAfterChecks),
|
||||
VERIFY_ATTEMPTS: String(verifyAttempts),
|
||||
VERIFY_DELAY_SECONDS: String(verifyDelaySeconds),
|
||||
NPM_VIEW_CROSS: visibilityPackages ? "true" : "false",
|
||||
CROSS_PACKAGES: (visibilityPackages ?? []).join(" "),
|
||||
PNPM_MODE: pnpmMode,
|
||||
REPO_ROOT: fixtureDir,
|
||||
},
|
||||
@@ -270,7 +308,7 @@ test("publish_package_to_npm does not retry stable publishes without provenance"
|
||||
assert.doesNotMatch(result.calls, /--provenance=false/);
|
||||
});
|
||||
|
||||
test("publish_package_to_npm_and_wait confirms registry visibility before returning", () => {
|
||||
test("wait_for_npm_package_versions confirms registry visibility after a publish", () => {
|
||||
const result = runPublishHelper({
|
||||
pnpmMode: "success",
|
||||
npmVersionExists: true,
|
||||
@@ -282,12 +320,29 @@ test("publish_package_to_npm_and_wait confirms registry visibility before return
|
||||
assert.match(result.calls, /^npm view @paperclipai\/example@1\.2\.3 version$/m);
|
||||
});
|
||||
|
||||
test("publish_package_to_npm_and_wait blocks the release when registry visibility lags", () => {
|
||||
test("wait_for_npm_package_versions blocks the release and names the straggler", () => {
|
||||
const result = runPublishHelper({ pnpmMode: "success", waitForRegistry: true });
|
||||
|
||||
assert.notEqual(result.status, 0);
|
||||
assert.match(result.calls, /^npm view @paperclipai\/example@1\.2\.3 version$/m);
|
||||
assert.match(result.output, /did not become registry-visible/);
|
||||
assert.match(result.output, /did not become registry-visible: @paperclipai\/example@1\.2\.3/);
|
||||
});
|
||||
|
||||
test("wait_for_npm_package_versions polls every package concurrently", () => {
|
||||
// In cross-visibility mode each fake package resolves only after the OTHER
|
||||
// package has been polled at least once. Waiting out one package's full
|
||||
// budget before polling the next can never satisfy the first package;
|
||||
// only concurrent polling converges.
|
||||
const result = runPublishHelper({
|
||||
pnpmMode: "success",
|
||||
visibilityPackages: ["@paperclipai/alpha", "@paperclipai/beta"],
|
||||
verifyAttempts: 50,
|
||||
verifyDelaySeconds: 0.2,
|
||||
});
|
||||
|
||||
assert.equal(result.status, 0, result.output);
|
||||
assert.match(result.output, /@paperclipai\/alpha@1\.2\.3 is registry-visible/);
|
||||
assert.match(result.output, /@paperclipai\/beta@1\.2\.3 is registry-visible/);
|
||||
});
|
||||
|
||||
test("the workflow budget tolerates the observed 15-minute 20-second registry delay", () => {
|
||||
|
||||
+13
-7
@@ -342,23 +342,29 @@ else
|
||||
node "$REPO_ROOT/scripts/prepare-bundled-package.mjs" "$REPO_ROOT/$pkg_dir" "$publish_dir"
|
||||
cd "$publish_dir"
|
||||
fi
|
||||
if ! publish_package_to_npm_and_wait \
|
||||
if ! publish_package_to_npm \
|
||||
"$DIST_TAG" \
|
||||
"$pkg_name" \
|
||||
"$pkg_version" \
|
||||
"$publish_tool" \
|
||||
"$VERIFY_ATTEMPTS" \
|
||||
"$VERIFY_DELAY_SECONDS"; then
|
||||
"$publish_tool"; then
|
||||
if [ "$publish_tool" = "npm" ]; then
|
||||
rm -rf "$publish_dir"
|
||||
fi
|
||||
release_fail "stopping release: npm did not publish and expose ${pkg_name}@${pkg_version}"
|
||||
release_fail "stopping release: npm did not accept the publish of ${pkg_name}@${pkg_version}"
|
||||
fi
|
||||
if [ "$publish_tool" = "npm" ]; then
|
||||
rm -rf "$publish_dir"
|
||||
fi
|
||||
release_info " ✓ Published version is registry-visible"
|
||||
release_info " ✓ npm accepted the publish"
|
||||
done <<< "$VERSIONED_PACKAGE_INFO"
|
||||
release_info " ✓ npm accepted every package publish under dist-tag $DIST_TAG"
|
||||
release_info " Waiting for registry visibility of all packages concurrently..."
|
||||
if ! wait_for_npm_package_versions \
|
||||
"$VERIFY_ATTEMPTS" \
|
||||
"$VERIFY_DELAY_SECONDS" \
|
||||
"$VERSIONED_PACKAGE_INFO"; then
|
||||
release_fail "stopping release: npm did not expose every published version under dist-tag $DIST_TAG"
|
||||
fi
|
||||
release_info " ✓ Published the full package set under dist-tag $DIST_TAG"
|
||||
fi
|
||||
|
||||
@@ -369,7 +375,7 @@ else
|
||||
release_info "==> Step 6/7: Confirming npm package availability and dist-tag integrity..."
|
||||
REGISTRY_STATE_VERIFY_ATTEMPTS="${NPM_REGISTRY_STATE_VERIFY_ATTEMPTS:-12}"
|
||||
REGISTRY_STATE_VERIFY_DELAY_SECONDS="${NPM_REGISTRY_STATE_VERIFY_DELAY_SECONDS:-5}"
|
||||
release_info " ✓ Every version was registry-visible before the next package publish"
|
||||
release_info " ✓ Every version was registry-visible before tagging began"
|
||||
|
||||
verify_args=(
|
||||
--channel "$channel"
|
||||
|
||||
Reference in New Issue
Block a user