fix(plugins): retry transient activation failures and bound boot concurrency (BLO-20410)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Plugins are the extension surface — secrets, chat, hindsight and the sandbox providers all ship as plugins loaded by the worker tier at pod start
• Four of eleven installed plugins were found sitting in status: error for 9+ hours, all with the same lastError: RPC call "initialize" timed out after 60000ms
• All four recovered from a single manual POST /api/plugins/<id>/enable — no code change, no config change, no restart — so the plugins were fine; they lost a 60-second race at boot and nothing ever tried again
• The failure is transient in cause but permanent in effect, and invisible: the pod is 1/1 Running with restarts=0, nothing alerts on plugin status, and the one explanatory log line had long since scrolled out of retention
• lucitra.plugin-secrets — the secrets subsystem — was one of the four
• This pull request removes both halves of the trap: the unbounded boot fan-out that causes the contention, and the first-failure latch that makes it permanent
• The benefit is that a plugin that loses a startup race now recovers on its own instead of waiting for someone to happen to list plugins

Linked Issues or Issue Description

Tracked internally as BLO-20410 (Blockcast's Paperclip instance, not a public GitHub issue), so
per CONTRIBUTING.md → "Link Issues or Describe Them In-PR" the bug is described inline below
following .github/ISSUE_TEMPLATE/bug_report.yml.

Partially addresses it — see Not covered below.

What happened?

Four of eleven installed plugins were found sitting in status: error for 9+ hours:

paperclip-chat                0.6.1   error
lucitra.plugin-secrets        0.2.0   error
paperclip-plugin-hindsight    0.2.3   error
penstock.paperclip-plugin     0.1.4   error

All four carried the same lastError:

Activation failed: Worker initialize failed for "<id>": RPC call "initialize" timed out after 60000ms

All four returned to ready with lastError: null from a single POST /api/plugins/<id>/enable —
no code change, no config change, no restart. The plugins were healthy; they lost a 60-second race
at pod startup when the whole plugin set initializes at once, and nothing ever tried again.

The failure is transient in cause but permanent in effect, and invisible everywhere you would
normally look: the pod is 1/1 Running with restarts=0 and no OOM, node CPU at 44%; nothing
alerts on plugin status; and the single explanatory log line is written at activation time and had
long since scrolled out of retention, leaving lastError as the only surviving evidence.

lucitra.plugin-secrets — the secrets subsystem — was one of the four.

Expected behavior

A plugin whose activation times out because of startup contention should be retried automatically
and reach ready without human action. A first-attempt timeout should not be terminal, and
startup should not put the entire plugin set into one shared 60s initialize window.

Steps to reproduce

1. Install enough plugins that their workers contend at startup (11 in the observed case).
2. Restart the worker-tier pod so loadAll() activates all ready plugins at once.
3. Observe that one or more plugins fail with RPC call "initialize" timed out after 60000ms and
   latch status: error.
4. Wait indefinitely — nothing retries. loadAll() only lists status="ready", so an errored
   plugin is never reconsidered on any subsequent boot either.
5. POST /api/plugins/<id>/enable on each errored plugin — every one recovers. Note this endpoint
   is slow: a 15s-timeout probe reports a client timeout and looks like a hang, while the same call
   with a 100s budget returns 200.

Paperclip version or commit

Observed on the deployed worker tier; fix is based on master at 34eb2ab3.

Deployment mode

Self-hosted Kubernetes, split api/worker tiers. Plugin lifecycle runs on the worker tier only —
app.ts gates loadAll() to paperclipNodeRole !== "api".

What Changed

• Bounded boot activation. loadAll() activated every ready plugin through an unbounded Promise.allSettled, putting the whole set into the same 60s initialize window. Replaced with a small worker pool (mapWithConcurrency, default 4, overridable via PAPERCLIP_PLUGIN_ACTIVATION_CONCURRENCY). Order-preserving and settle-semantic, so one bad plugin still cannot abort the rest. loadAll() is fire-and-forget at boot (app.ts), so serializing does not delay readiness.
• Transient activation failures are retried instead of latching status='error' on the first attempt. The existing worker-spawn retry loop already handled the SDK install race; it now carries a second, independent budget for transient startup failures (2 extra attempts, ~10s added delay — short because each attempt can burn the full 60s budget).
• The classifier is deliberately narrow. Only an initialize timeout or a worker that died during startup qualifies. initialize returned ok=false is explicitly excluded — that is the plugin answering "I am broken" inside the budget, i.e. a real fault. Manifest errors and missing entrypoints still fail closed on the first attempt.
• The two retry budgets cannot chain. A crashed-at-import worker matches both classifiers, so an SDK install race that exhausts its own 5 attempts is prevented from borrowing the transient budget and silently getting 7.
• New unit tests in server/src/__tests__/plugin-activation-retry.test.ts.

Verification

npx vitest run server/src/__tests__/plugin-activation-retry.test.ts
  → 14 passed

npx vitest run server/src/__tests__/plugin-activation-retry.test.ts \
              server/src/__tests__/plugin-worker-manager.test.ts
  → 33 passed

npx vitest run server/src/__tests__/plugin-worker-manager.test.ts \
              server/src/__tests__/plugin-lifecycle-restart.test.ts
  → 21 passed   (adjacent regression check)

pnpm --filter @paperclipai/server typecheck
  → clean

The classifier test asserts against the verbatim production lastError string from the four errored plugins, so it fails if the worker-manager error text drifts.

The concurrency test was checked against the old behaviour rather than assumed — replaying the previous Promise.allSettled path over 11 items peaks at 11 in flight, against the new bound of 4, so the assertion is not vacuous.

Risks

Low-to-moderate, and mostly bounded by the narrowness of the classifier.

• A genuinely broken plugin now takes ~10s longer to latch error (two extra attempts). Bounded, and only on the transient-looking error shapes.
• Worst-case boot is slower. With concurrency 4 and a pathological set where every plugin exhausts its retries, activation serializes into minutes. It cannot delay readiness because loadAll() is not awaited, and the tuning knob is an env var.
• The classifier is substring-based on error text. If the worker-manager message changes, a timeout could be misclassified as terminal — i.e. it degrades to today's behaviour rather than to something worse. The test pins the current string.
• Retry reuses the loop's existing stopWorker()-before-startWorker() cleanup, so no new worker-handle leak path is introduced.

Not covered

The third Done when on the issue — "a plugin left in error beyond a short grace period raises an alert" — is not in this PR. There is currently no Prometheus metric for plugin status at all (metrics.ts has zero plugin series), so that needs a gauge plus a rule in the monitoring-rules GitOps lane. Tracked separately and routed to the Platform/SRE lane; this PR is the runtime half.

Model Used

Claude Opus 4.5 (claude-opus-4-5), extended thinking, via Claude Code with tool use.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — n/a, no UI surface
• I have updated relevant documentation to reflect my changes — no doc surface; the new env var is documented in-code at its definition
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-20410

[allyblockcast]

🔗 Paperclip issue: BLO-20410

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

@ally please review at head 158dbc1e.

Focus areas, in order:

1. isTransientActivationError classification boundary (server/src/services/plugin-loader.ts) — this decides whether a failed plugin retries or latches status='error' forever. It is substring matching on worker-manager error text. Is the initialize returned ok=false exclusion sufficient to keep real plugin faults failing closed, or is there an error shape that should be terminal but matches timed out after / Worker exited during startup?

2. Retry-budget independence — an SDK-install-race error and a crashed-at-import worker can match both classifiers. I guard the transient branch with !isSdkInstallRaceError(err) so the budgets cannot chain into 7 attempts. Please check I have not inverted the precedence or left a path where a genuinely broken plugin loops longer than intended.

3. mapWithConcurrency — hand-rolled bounded pool replacing Promise.allSettled. Check the cursor increment is race-free under the shared-slot model, that results stay index-aligned with the input, and that it cannot hang on an empty list or when limit > items.length.

4. stopWorker() reuse on retry — retry re-enters the existing getWorker() → stopWorker() cleanup before re-spawning. startWorker() throws if an existing handle is not stopped. Is that cleanup sufficient after an initialize timeout leaves the handle in crashed?

Context: four plugins including lucitra.plugin-secrets sat dead 9+ hours because a 60s initialize timeout latched on first failure. Does not cover alerting — that half is tracked separately.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: f19f035

Critical Issues (0)

Important Issues (3)

• [pr-review-toolkit / native-codex] server/src/services/plugin-loader.ts:143 — The generic Worker initialize failed marker makes nearly every initialize RPC failure transient, not just timeouts. plugin-worker-manager wraps explicit plugin RPC errors (for example invalid configuration or credentials) with this prefix, so the ok=false exclusion does not keep those real faults failing closed and initialization side effects can be repeated.
  • Remove the generic wrapper marker and classify the exact initialize-timeout/worker-exit causes, preferably through typed error codes. Add a test using the production-wrapped shape of an explicit initialize RPC rejection.
• [gstack/review / native-codex] server/src/services/plugin-loader.ts:2780 — A worker that exits during startup can restart itself before this retry loop cleans it up. Workers use autoRestart: true; their crash backoff begins around one second, while this branch sleeps for two seconds before the next iteration reaches stopWorker(). That creates an unbudgeted initialize attempt and can repeat plugin startup side effects.
  • Stop and unregister the failed worker immediately before sleeping, which also cancels its pending restart timer. Cover this with the real worker manager and assert no autonomous restart occurs during the retry delay.
• [pr-review-toolkit / native-codex] server/src/services/plugin-loader.ts:2763 — The retry budgets can still chain when the error class changes between attempts. The guard prevents one SDK-shaped error from borrowing the transient budget after SDK exhaustion, but an initialize timeout followed by SDK import failures (or the reverse) can consume both counters and exceed the stated attempt bound.
  • Lock an activation to its first retry class or enforce a shared total-attempt ceiling. Add a mixed-error sequence test that asserts the exact start count.

Suggestions (0)

Strengths

• mapWithConcurrency bounds in-flight work, keeps result indexes aligned, settles failures independently, and handles empty or oversized-limit inputs without hanging.
• The normal retry path reuses stopWorker() correctly to remove a crashed timeout handle before spawning its replacement.

Recommended Action

1. Address the Important issues before merge.
2. Add end-to-end retry-loop tests for classification, attempt counts, and worker cleanup.

This PR is authored by app/allyblockcast, so the Ally App cannot review its own PR. Reopen this exact head under an independent author before an App approval can satisfy review/ally-complete; the required singleton Ally team approval remains separate.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 7467fa0

Prior Findings Dispositioned (3)

• prior:f19f035 important 1 — still-present — server/src/services/plugin-loader.ts:143 — The generic Worker initialize failed marker remains in the transient classifier, so production-wrapped initialize RPC errors still retry even when they are not timeouts or worker exits.
• prior:f19f035 important 2 — still-present — server/src/services/plugin-loader.ts:2780 — The transient branch still sleeps before stopping the failed worker; a worker that exits naturally during startup can retain its already-scheduled auto-restart through this delay.
• prior:f19f035 important 3 — still-present — server/src/services/plugin-loader.ts:2720 — SDK-race and transient attempts are still tracked by independent counters without a shared ceiling or first-class lock, so changing error classes can consume both budgets.

Critical Issues (0)

Important Issues (3)

• [prior:f19f035 important 1] server/src/services/plugin-loader.ts:143 — The classifier still treats every error wrapped as Worker initialize failed as transient. plugin-worker-manager uses that wrapper for all initialize failures, including explicit RPC rejection/configuration faults, so startup side effects can be repeated for real plugin failures.
  • Remove the generic wrapper marker and match only the exact timeout and worker-exit causes, preferably via typed error codes. Test a production-wrapped explicit initialize RPC rejection.
• [prior:f19f035 important 2] server/src/services/plugin-loader.ts:2780 — Cleanup still occurs only at the top of the next loop iteration, after the 2-second sleep. A worker that exits during startup schedules its own roughly 1-second auto-restart, allowing an unbudgeted start before stopWorker() cancels it.
  • Stop and unregister the failed handle before sleeping. Add a worker-manager integration test proving no autonomous restart occurs during the retry delay.
• [prior:f19f035 important 3] server/src/services/plugin-loader.ts:2720 — The two retry counters remain independently consumable when successive attempts change error class. The same activation can therefore exceed the intended retry bound despite the same-error guard at line 2763.
  • Lock the activation to its first retry class or add a shared total-attempt ceiling. Test mixed SDK-race and transient error sequences and assert the exact spawn count.

Suggestions (0)

Strengths

• Bounded boot activation preserves input-aligned settled results and isolates plugin failures.
• Persisting retry counters in lastError materially improves post-retention diagnosis.

Recommended Action

1. Address the three still-present Important findings before merge.
2. Add end-to-end retry-loop coverage for classification, cleanup timing, and mixed-error attempt bounds.

This PR is authored by app/allyblockcast, so the Ally App cannot review its own PR. The exact head must be reopened under an independent author before an App approval can satisfy review/ally-complete; the required singleton Ally team approval remains separate.

[kkroo]

Approved: bounded activation concurrency plus narrow transient retry classification is covered by focused unit tests and preserves terminal plugin faults.