fix(spawn): harden abort/reset race handling, bump Pi to 0.84.1, clear security allowlist - #23
fix(spawn): harden abort/reset race handling, bump Pi to 0.84.1, clear security allowlist#23ofriw wants to merge 11 commits into
Conversation
grzegorznowak
left a comment
There was a problem hiding this comment.
-
A rendered child can be aborted twice or remain registered after failure.
spawn/renderer.ts, the module that renders child-agent activity, callssession.abort()directly instead of using the new shared abort helper instate.ts, the module that tracks active child sessions.Why it matters: A reset and a screen cleanup can cancel the same child twice. If cancellation throws immediately, the stale child remains in the live-session registry.
Concrete example: The review traces this sequence: a signal starts the first abort, then renderer disposal starts a second abort. It also shows that a synchronous abort error prevents registry deletion.
Action: Route renderer disposal through
abortChildSession()and delete the registry entry in afinallyblock. -
Reset errors change depending on race timing.
spawn/index.ts, the module that creates and runs child agents, reports an abort failure if reset happens before the child is registered. If reset happens later, it replaces that failure with “Spawn invalidated by reset.”Why it matters: The same failed cleanup can produce different operator-visible results. In one path, the cleanup failure can disappear completely.
Concrete example: A reset during child creation can return “abort failed.” A reset during an active prompt can return only “Spawn invalidated by reset,” with no cleanup notification.
Action: Define one reset error policy and preserve abort failures consistently across every timing path.
-
Cleanup reporting can cause an unhandled promise rejection. A promise rejection is an asynchronous failure.
spawn/index.ts, the child execution module, starts a detached error handler that calls the user-interface notification function. It does not handle a failure from that notification call.Why it matters: If notification itself throws, the extension can produce a process-level unhandled rejection. The extension runs inside the same process as the terminal interface, so this can destabilize the interface.
Concrete example: The review traces: abort rejects, the error handler runs,
ctx.ui.notify()throws, and the resulting promise has no observer.Action: Catch notification failures in the abort-reporting path and preserve them deterministically, as the existing dispose path already does.
-
One Windows compatibility failure hides the next Windows result.
.github/workflows/test.yml, the GitHub Actions workflow that runs automated checks, runs the current-Pi Windows check after the packed-host check. The later step uses the default “run only after success” behavior.Why it matters: When packaging fails, maintainers lose the independent Windows runtime result they need to determine whether the problem affects packaging, the Pi API, or both.
Concrete example: If “Packed latest Pi host contract” fails, its diagnostics upload runs, but “Synchronized current Pi compatibility on Windows” is skipped.
Action: Give the current-Pi step an explicit non-cancelled condition or move the two checks into separate jobs.
-
The package claims wider compatibility than CI verifies. A peer dependency is a library that the host application must provide.
package.json, the package manifest, accepts every Pi and TypeBox version through*, but the old minimum-version test was removed.Why it matters: Developers can install this extension with older versions without an
npmwarning, even though CI now tests only the pinned development versions and the latest available host.Concrete example: An older Pi host satisfies the wildcard peer declarations, but no remaining compatibility lane tests that combination.
Action: Set peer ranges to the versions the project supports, or restore a test for the oldest supported host.
Note: This PR was generated by an AI agent. If you'd like to talk with other humans, drop by our Discord!
What Changed
Spawn lifecycle hardening — Race conditions between signal aborts, reset invalidation, and prompt rejections are now handled deterministically via a shared abort-promise pattern (
abortChildSessioninstate.ts). Previously, concurrent abort + reset could double-firesession.abort()or swallow real prompt errors; now the sharedWeakMap<AgentSession, Promise<void>>ensures one abort per session, and non-AbortErrorprompt failures propagate even during abort races.CI / compat testing overhaul —
runNpmWithRetry()with exponential backoff for transient registry failures.compat-fixture.mjsextracts shared compat utilities (createCompatCopy,prepareCompatCopy,resolveLatestPi, diagnostics).workflow_dispatchtrigger.Security audit cleanup — Emptied the
audit-ciallowlist entirely. All previously suppressed advisories forbrace-expansionandundici(pinned by Pi 0.82.0) are resolved by bumping to Pi 0.84.1 / TypeBox 1.3.7.Renovate config added —
.github/renovate.jsonpins Pi packages and TypeBox to exact versions.Breaking Changes
None user-facing. The
test-compat-floor.mjsscript andtest:compat:floornpm script are removed.Why
/resetor signal cancellation — important for correctness of child session lifecycle management.Testing
abort-child-session.test.ts+ 4 new race-condition tests inspawn.test.tscover abort dedup, sync/async abort throws, and signal-vs-reset priority.runNpmWithRetrytested with fixture npm CLI simulating transient failures.Attached is an agent optimized description of the changes in this PR - AGENT_REVIEW.md