fix(components): scope the handleApplication lock per application

[ldt1996]

One application's hung plugin load could time out every other application's load of the same plugin type, because the component loader serialized handleApplication on a lock keyed by plugin name alone. This PR scopes that lock per (application, plugin) and adds a loader-level regression test that fails on the old key and passes on the new one. It is the lock half of #3184; the status-honesty half is #2883.

Problem

sequentiallyHandleApplication serializes plugin handleApplication calls with a Status.primaryStore lock keyed by scope.pluginName, which is the plugin type (jsResource, graphqlSchema, ...). Applications load concurrently (each serialized per app by serializeComponentLoad), so that lock is shared across applications: one application hanging inside its handleApplication holds the jsResource lock, and every other application's jsResource load rejects with Timeout waiting for lock on jsResource after its own timeout plus the 5s acquisition grace. In production this cascaded into 500s while /status stayed green.

Fix

The lock key is now ${scope.appName}\0${scope.pluginName} in tryLock and the matching finally unlock. The NUL separator cannot appear in a path segment or config key, so two distinct (application, plugin) pairs can never alias to one key; the key is never displayed, so nothing user-visible changes. Cross-thread serialization of the same application's plugin load is preserved, which was the lock's stated purpose; only the cross-application coupling is removed. The timeout message keeps the Timeout waiting for lock on prefix that ops runbooks grep for and now also names the application.

For the human reviewer

• decision: lock-key separator. Chosen NUL over a dot (which would match the app.plugin status-name convention) because appName can contain dots and slashes, so any printable separator could alias two distinct pairs. Raised by the Codex review leg; both storage engines accept the key (regression test exercises the real store).
• decision: review scope of the test. The regression test pins cross-application isolation only; it does not exercise same-application cross-worker serialization (the preserved pre-existing behavior). That property holds by inspection (identical app and plugin names produce identical keys on every worker) but has no executed evidence in this PR.
• known adjacent defect, deliberately not fixed here: a lock waiter that times out stays queued, so the eventual unlock re-runs sequentiallyHandleApplication late for the already-failed scope, and a rejection from that re-run is unhandled. Pre-existing; the per-application key narrows the window but does not remove it. Fixing it needs the store's lock-callback wake semantics (one waiter or all).

Verification

All runs against the built dist on macOS, Node 24.

• Regression test unitTests/components/componentLoaderLock.test.js: two real applications loaded through loadComponent, both declaring the same plugin; the first hangs inside handleApplication holding its lock, the second must load within its own timeout budget.
  • With the old key (this change reverted in the compiled dist): 1 failing in ~6s, the second app's status carries Timeout waiting for lock on lockIsolationProbe.
  • With this change: 1 passing in ~30-40ms, while the first app is still hanging.
• Components tests, CI's glob (unitTests/components/**/*test.*js) minus applicationSpawn.test.js: 1885 passing, 3 pending, 5 failing. The identical 5 failures reproduce with this change reverted, so all are pre-existing environment issues on this machine (4x macOS /private/var vs /var tmpdir realpath mismatches in RuntimeModuleTracker.test.js, 1x OptionsWatcher.test.js deletion race). applicationSpawn.test.js was excluded because it hangs the whole suite with no output here, the same behavior windowsGate.mjs documents for it.
• oxlint and prettier clean on both changed files; npm run build emits only the pre-existing utility/interactivePrompts.ts errors from the renovate @inquirer bump (stale local node_modules; tsc still emits).

Review coverage

author: claude; cross-model coverage: codex ok (3 rounds, resumed session) / gemini failed (agy not on PATH, no GEMINI_API_KEY) / domain adjudication failed (claude CLI not on PATH), adjudicated inline by the authoring session. Codex findings: separator aliasing (fixed in 9ad2e64, round 2 confirms closed), cross-worker test scope (acknowledged above), test-header verbosity (declined, matches repo regression-test precedent). Round 3 (the Windows cleanup fix in 112671c) surfaced nothing new.

Complexity: easy

Lavinia, via Claude

🤖 Generated with Claude Code

_{Review-Coverage: authored=claude; ran=codex; blocked=gemini; declined=cursor-grok,cursor-composer,domain; rounds=3 @ 112671c
Human-Review-Need: 3 @ 112671c}

[gemini-code-assist]

Code Review

This pull request scopes the plugin lock in sequentiallyHandleApplication per application and plugin (${scope.appName}.${scope.pluginName}) instead of using the plugin name alone, preventing concurrent application loads from starving each other. A regression test is added to verify this behavior. Feedback was provided to improve the robustness of the test cleanup in the after hook by guarding against undefined variables if the before hook fails.

[claude]

Suggestion (non-blocking): when the waiter's own setTimeout (line ~705) fires first and rejects, this promise has already settled. If the lock's wake callback still fires later, whenResolved(sequentiallyHandleApplication(scope, plugin)) calls resolve on an already-settled promise (a no-op per spec), but the argument is still evaluated — spawning a fresh, completely unreferenced retry promise for a scope whose caller already gave up. If that retry later rejects, it's an unhandled promise rejection (Node crashes by default). The PR body already flags this as a known, pre-existing gap ("a rejection from that re-run is unhandled"), not introduced by this diff. A minimal fix: const retry = sequentiallyHandleApplication(scope, plugin); retry.catch((e) => harperLogger.warn?.(...)); whenResolved(retry); — or adopt the wait-loop pattern already used in resources/recordLock.ts (acquireRecordKey), which retries by resolving an internal wait promise instead of recursively spawning a fresh unawaited chain.

[ldt1996]

Good catch, and the analysis is right: the timed-out waiter's wake callback still evaluates a fresh sequentiallyHandleApplication retry after the outer promise has rejected, so a late unlock can leave an unhandled rejection. As you note it predates this change, and keying the lock per application only narrows the window (same app on two threads still contends) rather than closing it.

I'm keeping this PR scoped to the lock-key fix and tracking the waiter rejection separately in #2889, which captures both the minimal .catch() mitigation and the acquireRecordKey wait-loop pattern you pointed to as the cleaner option.

Lavinia, via Claude

[claude]

Reviewed; no blockers found.

[cb1kenobi]

The handleApplication lock is now keyed per application and plugin, so one hung load no longer times out every other app of that plugin type. Lock and unlock use the same NUL-separated id, and the timeout text still matches the ops grep prefix. Existing discussion already covers the pre-existing stale-waiter rejection and the test after-hook guards. No confirmed blocking issues on the changed lines.

—
Reviewed 112671c

[cb1kenobi]

The lock consistently scopes acquisition and release to one application and plugin. The regression test covers cross-application isolation, and no confirmed blocking issue remains on the changed lines.

—
Reviewed 112671c

[cb1kenobi]

The handleApplication lock is now keyed per application and plugin, so one hung load no longer times out every other app of that plugin type. Lock and unlock use the same NUL-separated id, and the timeout text still matches the ops grep prefix. Existing discussion already covers the pre-existing stale-waiter rejection and the test after-hook guards. No confirmed blocking issues remain on the changed lines.

—
Reviewed 112671c

[kriszyp]

_{🤖 Reviewed with Codex}

[kriszyp]

Could this key encode the two names unambiguously? For example, application billing.eu with plugin rest and application billing with plugin eu.rest both produce billing.eu.rest. Application names come from component identities, and plugin names can be supplied through the built-in component mapping (components/Application.ts:5524-5533), so these distinct loads can still block each other and the second can hit the lock timeout. A tuple encoding such as JSON.stringify([scope.appName, scope.pluginName]) would preserve the intended per-application isolation.

[kriszyp]

Please close the two scopes before removing tempRoot. Each Scope installs an OptionsWatcher and deploy lifecycle listeners, which Scope.close() removes (components/Scope.ts:162-177, components/Scope.ts:292-302). Awaiting hogLoad settles the loader promise but does not close those scopes, so this suite leaves watchers and listeners in the shared Mocha process and removes files they still watch. The existing lifecycle test in unitTests/components/componentLoader.test.js:430-471 demonstrates this leak. Capture both scopes in the synthetic plugin and close them in after, including when the test fails partway through.