From 4701dee38aca4a8846db1df017260cd3d30d5659 Mon Sep 17 00:00:00 2001 From: Drswith <49299002+Drswith@users.noreply.github.com> Date: Fri, 3 Jul 2026 10:32:46 +0800 Subject: [PATCH] fix(lifecycle): stop install fallback when cancellation races managed install When a managed install subprocess exits successfully but the CLI context is already cancelled, waitForSpawnedCommand reports failure. installAgent now rolls back the current method and stops instead of trying later install methods, preventing duplicate global installs. OpenSpec change: fix-install-cancel-method-fallback --- .../.openspec.yaml | 2 + .../design.md | 28 +++++++++++++ .../proposal.md | 24 +++++++++++ .../specs/agent-update/spec.md | 40 +++++++++++++++++++ .../tasks.md | 9 +++++ src/package-manager/index.ts | 9 +++++ test/package-manager/index.test.ts | 26 +++++++++++- 7 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 openspec/changes/fix-install-cancel-method-fallback/.openspec.yaml create mode 100644 openspec/changes/fix-install-cancel-method-fallback/design.md create mode 100644 openspec/changes/fix-install-cancel-method-fallback/proposal.md create mode 100644 openspec/changes/fix-install-cancel-method-fallback/specs/agent-update/spec.md create mode 100644 openspec/changes/fix-install-cancel-method-fallback/tasks.md diff --git a/openspec/changes/fix-install-cancel-method-fallback/.openspec.yaml b/openspec/changes/fix-install-cancel-method-fallback/.openspec.yaml new file mode 100644 index 0000000..43e65ca --- /dev/null +++ b/openspec/changes/fix-install-cancel-method-fallback/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-03 diff --git a/openspec/changes/fix-install-cancel-method-fallback/design.md b/openspec/changes/fix-install-cancel-method-fallback/design.md new file mode 100644 index 0000000..872bd90 --- /dev/null +++ b/openspec/changes/fix-install-cancel-method-fallback/design.md @@ -0,0 +1,28 @@ +## Context + +`waitForSpawnedCommand()` intentionally returns exit code `1` when the CLI context is cancelled, even if the child already exited `0`. Managed installers such as Bun use this helper, so a timed-out or cancelled install can leave a package on disk while `executeMethod()` returns `false`. + +`installAgent()` already rolls back when `executeMethod()` returns `true` and cancellation is observed before persistence. It does not handle the case where cancellation makes a successful install look like a failed method attempt. + +## Goals / Non-Goals + +- Goals: prevent duplicate global installs and orphaned packages when cancellation races a successful managed install subprocess exit. +- Non-Goals: redesign cancellation semantics, change batch command orchestration, or alter update/uninstall flows in this change. + +## Decisions + +- Check `getCliContext().cancelled` at the start of each install-method loop iteration and return failure immediately. +- When `executeMethod()` returns `false` and the context is cancelled, call `rollbackManagedInstall()` for the current method and return failure without trying later methods. +- Mirror the existing rollback helper used for post-success cancellation paths. + +## Risks / Trade-offs + +- Best-effort rollback on cancellation may call uninstall for a method whose install never completed; existing uninstall commands are already tolerant of absent packages. + +## Migration Plan + +- Patch release only; no migration required. + +## Open Questions + +- None. diff --git a/openspec/changes/fix-install-cancel-method-fallback/proposal.md b/openspec/changes/fix-install-cancel-method-fallback/proposal.md new file mode 100644 index 0000000..79bb641 --- /dev/null +++ b/openspec/changes/fix-install-cancel-method-fallback/proposal.md @@ -0,0 +1,24 @@ +## Why + +When a managed install subprocess exits successfully but the CLI context is already cancelled, `waitForSpawnedCommand()` reports failure. `installAgent()` treats that as a failed method attempt and continues to the next install method without rolling back the package that was just installed on disk, leaving duplicate globals and inconsistent state. + +## What Changes + +- Stop install method fallback when the CLI context is cancelled. +- Roll back the current managed install method when cancellation turns a successful subprocess exit into a reported failure. +- Add regression tests covering cancellation after a successful managed install subprocess exit. + +## Capabilities + +### New Capabilities + +- None. + +### Modified Capabilities + +- `agent-update`: cancelled managed installs must not fall through to later install methods and must roll back packages installed by the cancelled attempt. + +## Impact + +- Affected code: `src/package-manager/index.ts`, `test/package-manager/index.test.ts`. +- No CLI flags, schema version, or command catalog changes. diff --git a/openspec/changes/fix-install-cancel-method-fallback/specs/agent-update/spec.md b/openspec/changes/fix-install-cancel-method-fallback/specs/agent-update/spec.md new file mode 100644 index 0000000..005cce2 --- /dev/null +++ b/openspec/changes/fix-install-cancel-method-fallback/specs/agent-update/spec.md @@ -0,0 +1,40 @@ +## MODIFIED Requirements + +### Requirement: Managed lifecycle cancellation MUST terminate Windows wrapper process trees before wrapper fallback kill + +When Quantex cancels a managed lifecycle installer on Windows and a child process identifier is available, it SHALL attempt process-tree termination before directly killing the wrapper process. Quantex MUST preserve sticky cancellation semantics if process-tree termination is unavailable, denied, or races with child exit. + +#### Scenario: Windows wrapper child owns a long-running installer descendant + +- **GIVEN** Quantex is running a managed installer through a Windows wrapper process +- **AND** the wrapper has started a long-running installer descendant +- **WHEN** the managed installer operation is cancelled by signal or timeout +- **THEN** Quantex attempts process-tree termination for the wrapper process identifier before direct wrapper termination +- **AND** the installer descendant does not continue producing installer progress after Quantex returns the cancelled result +- **AND** Quantex does not persist normal installed-agent state for the cancelled operation + +#### Scenario: Batch install does not continue after timeout cancellation + +- **GIVEN** the user runs `quantex install --timeout ` +- **AND** the first agent's install work exceeds the configured timeout and late-completion grace window +- **WHEN** Quantex emits a timeout cancellation result for the command +- **THEN** it does not install or persist state for `` +- **AND** it does not persist normal installed-agent state for the cancelled `` operation + +#### Scenario: Batch update does not continue after timeout cancellation + +- **GIVEN** the user runs `quantex update --all --timeout ` +- **AND** an early update item exceeds the configured timeout and late-completion grace window +- **WHEN** Quantex emits a timeout cancellation result for the command +- **THEN** it does not perform later update work for remaining agents in the same command +- **AND** it does not persist normal installed-agent state for the cancelled update operation + +#### Scenario: Cancelled managed install does not fall through to later install methods + +- **GIVEN** an agent definition lists multiple managed install methods in priority order +- **AND** the first managed install subprocess exits successfully on disk +- **AND** the CLI context is cancelled before Quantex records the install as successful +- **WHEN** Quantex evaluates the first managed install attempt +- **THEN** it rolls back packages installed by that attempt when rollback is supported +- **AND** it does not run later install methods for the same agent in the same command +- **AND** it does not persist normal installed-agent state for the cancelled operation diff --git a/openspec/changes/fix-install-cancel-method-fallback/tasks.md b/openspec/changes/fix-install-cancel-method-fallback/tasks.md new file mode 100644 index 0000000..c2c300f --- /dev/null +++ b/openspec/changes/fix-install-cancel-method-fallback/tasks.md @@ -0,0 +1,9 @@ +## 1. Implementation + +- [x] 1.1 Guard `installAgent()` method fallback when the CLI context is cancelled +- [x] 1.2 Roll back the current managed install method when cancellation turns a successful subprocess exit into failure + +## 2. Validation + +- [x] 2.1 Add regression tests for cancelled install method fallback and rollback +- [x] 2.2 Run `bun run lint`, `bun run format:check`, `bun run typecheck`, and `bun run test` diff --git a/src/package-manager/index.ts b/src/package-manager/index.ts index d0d942d..c7accc9 100644 --- a/src/package-manager/index.ts +++ b/src/package-manager/index.ts @@ -219,6 +219,10 @@ export async function installAgent(agent: AgentDefinition): Promise { bunUninstallSpy.mockResolvedValue(true) expect(await installAgent(testAgent)).toEqual({ success: false }) + expect(bunInstallSpy).not.toHaveBeenCalled() expect(setInstalledAgentStateSpy).not.toHaveBeenCalled() - expect(bunUninstallSpy).toHaveBeenCalledWith('test-pkg') + expect(bunUninstallSpy).not.toHaveBeenCalled() resetCliContext() }) @@ -286,6 +287,29 @@ describe('installAgent', () => { expect(npmInstallSpy).toHaveBeenCalledWith('test-pkg') }) + it('rolls back and stops fallback when cancellation turns a successful managed install into failure', async () => { + setCliContext({ + interactive: false, + outputMode: 'human', + runId: 'cancel-after-exit-install-id', + }) + isBunSpy.mockResolvedValue(true) + isNpmSpy.mockResolvedValue(true) + bunInstallSpy.mockImplementation(async () => { + markCliContextCancelled() + return false + }) + bunUninstallSpy.mockResolvedValue(true) + npmInstallSpy.mockResolvedValue(true) + + expect(await installAgent(testAgent)).toEqual({ success: false }) + expect(bunInstallSpy).toHaveBeenCalledWith('test-pkg') + expect(bunUninstallSpy).toHaveBeenCalledWith('test-pkg') + expect(npmInstallSpy).not.toHaveBeenCalled() + expect(setInstalledAgentStateSpy).not.toHaveBeenCalled() + resetCliContext() + }) + it('falls back to the next install method after bun trust failure rolls back the install', async () => { isBunSpy.mockResolvedValue(true) isNpmSpy.mockResolvedValue(true)