diff --git a/openspec/changes/fix-uninstall-ghost-state/.openspec.yaml b/openspec/changes/fix-uninstall-ghost-state/.openspec.yaml new file mode 100644 index 0000000..d6b53de --- /dev/null +++ b/openspec/changes/fix-uninstall-ghost-state/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-30 diff --git a/openspec/changes/fix-uninstall-ghost-state/design.md b/openspec/changes/fix-uninstall-ghost-state/design.md new file mode 100644 index 0000000..4723bb1 --- /dev/null +++ b/openspec/changes/fix-uninstall-ghost-state/design.md @@ -0,0 +1,25 @@ +## Context + +`uninstallAgent()` runs managed uninstall, then calls `removeInstalledAgentState()` only when uninstall returns `true`. A prior attempt can succeed at package removal but fail while writing `state.json`, leaving the agent untracked on disk but still recorded in state. Retries call the package manager again, get `false`, and never clear state. + +## Goals / Non-Goals + +**Goals** + +- Clear ghost state when uninstall can verify the managed package is absent. +- Avoid false recovery when the package manager is unavailable. + +**Non-Goals** + +- Change Windows deferred binary self-upgrade success semantics. +- Add doctor-only repair flows. + +## Decisions + +- After managed uninstall returns `false`, call a narrow helper that checks package absence through the existing `getInstalledVersion` installer hook. +- Skip recovery when the installer is unavailable or lacks version probing. +- Reuse `removeInstalledAgentState()` once absence is confirmed. + +## Risks / Trade-offs + +- [Risk] Mis-detecting absence when version probing is flaky. → Mitigation: only recover when installer is available and `getInstalledVersion` returns `undefined`. diff --git a/openspec/changes/fix-uninstall-ghost-state/proposal.md b/openspec/changes/fix-uninstall-ghost-state/proposal.md new file mode 100644 index 0000000..6d5af30 --- /dev/null +++ b/openspec/changes/fix-uninstall-ghost-state/proposal.md @@ -0,0 +1,24 @@ +## Why + +`uninstallAgent()` only clears installed-agent state when the managed package-manager uninstall returns success. If the package is already removed but state persistence failed on a prior attempt—or the package manager reports failure because the package is gone—Quantex leaves permanent ghost state. That breaks `update --all`, `uninstall`, and doctor flows until users manually edit `state.json`. + +## What Changes + +- Recover ghost uninstall state when a managed package is no longer present on disk but Quantex still tracks it. +- Only perform recovery when the relevant package manager is available and can confirm absence. +- Add regression tests for the ghost-state recovery path. + +## Capabilities + +### New Capabilities + +- None. + +### Modified Capabilities + +- `agent-update`: uninstall must clear tracked state when the managed package is already absent, even if the package-manager uninstall command reports failure. + +## 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-uninstall-ghost-state/specs/agent-update/spec.md b/openspec/changes/fix-uninstall-ghost-state/specs/agent-update/spec.md new file mode 100644 index 0000000..fda3818 --- /dev/null +++ b/openspec/changes/fix-uninstall-ghost-state/specs/agent-update/spec.md @@ -0,0 +1,43 @@ +## MODIFIED Requirements + +### Requirement: Uninstall MUST clear tracked unmanaged install state + +When an agent is recorded with install type `script` or `binary`, Quantex SHALL remove the installed-agent state entry on uninstall even though no managed package uninstall command exists. + +#### Scenario: Uninstalling a tracked script install + +- **GIVEN** an agent has recorded install state with install type `script` +- **WHEN** the user runs `quantex uninstall ` +- **THEN** Quantex removes the installed-agent state entry +- **AND** the uninstall command reports success +- **AND** Quantex does not require a managed package-manager uninstall for that install type + +#### Scenario: Uninstalling a tracked binary install + +- **GIVEN** an agent has recorded install state with install type `binary` +- **WHEN** the user runs `quantex uninstall ` +- **THEN** Quantex removes the installed-agent state entry +- **AND** the uninstall command reports success + +### Requirement: Uninstall MUST recover ghost managed install state + +When a managed package is no longer installed but Quantex still records install state, uninstall SHALL clear the stale state entry after confirming package absence through the recorded installer. + +#### Scenario: Retrying uninstall after package removal succeeded but state persistence failed + +- **GIVEN** an agent has recorded managed install state +- **AND** the managed package is no longer installed on the system +- **AND** the recorded package manager is available and can confirm absence +- **WHEN** the user runs `quantex uninstall ` +- **AND** the managed package-manager uninstall command reports failure +- **THEN** Quantex removes the installed-agent state entry +- **AND** the uninstall command reports success + +#### Scenario: Ghost recovery does not run when the package manager is unavailable + +- **GIVEN** an agent has recorded managed install state +- **AND** the recorded package manager is unavailable +- **WHEN** the user runs `quantex uninstall ` +- **AND** the managed package-manager uninstall command reports failure +- **THEN** Quantex does not remove the installed-agent state entry +- **AND** the uninstall command reports failure diff --git a/openspec/changes/fix-uninstall-ghost-state/tasks.md b/openspec/changes/fix-uninstall-ghost-state/tasks.md new file mode 100644 index 0000000..cc87f16 --- /dev/null +++ b/openspec/changes/fix-uninstall-ghost-state/tasks.md @@ -0,0 +1,10 @@ +## 1. Implementation + +- [x] 1.1 Add managed package absence probe helper in `src/package-manager/index.ts` +- [x] 1.2 Recover ghost state in `uninstallAgent()` when absence is confirmed +- [x] 1.3 Add regression tests in `test/package-manager/index.test.ts` + +## 2. Validation + +- [x] 2.1 Run `bun run lint`, `bun run format:check`, `bun run typecheck`, and `bun run test` +- [x] 2.2 Run `bun run openspec:validate` diff --git a/src/package-manager/index.ts b/src/package-manager/index.ts index 9d70e1f..760d1b4 100644 --- a/src/package-manager/index.ts +++ b/src/package-manager/index.ts @@ -337,6 +337,23 @@ export async function getManagedInstalledPackageVersion( return installer.getInstalledVersion(packageName, packageTargetKind) } +async function isManagedPackageAbsent( + state: InstalledAgentState, + agent?: Pick, +): Promise { + if (!isManagedInstallType(state.installType)) return false + + const installer = getManagedInstaller(state.installType) + if (!installer.getInstalledVersion) return false + if (!(await installer.isAvailable())) return false + + const packageName = resolveManagedPackageName(state, agent) + if (!packageName) return false + + const installedVersion = await installer.getInstalledVersion(packageName, state.packageTargetKind) + return installedVersion === undefined +} + export async function uninstallAgent(agent: AgentDefinition): Promise { return withResourceLock(lifecycleLock, async () => { const installedState = await getInstalledAgentState(agent.name) @@ -348,7 +365,16 @@ export async function uninstallAgent(agent: AgentDefinition): Promise { } const success = await executeInstalledState(installedState, 'uninstall', { agent }) - if (success) await removeInstalledAgentState(agent.name) - return success + if (success) { + await removeInstalledAgentState(agent.name) + return true + } + + if (await isManagedPackageAbsent(installedState, agent)) { + await removeInstalledAgentState(agent.name) + return true + } + + return false }) } diff --git a/test/package-manager/index.test.ts b/test/package-manager/index.test.ts index 90d0a7a..cbdf01f 100644 --- a/test/package-manager/index.test.ts +++ b/test/package-manager/index.test.ts @@ -26,6 +26,7 @@ const bunInstallSpy = vi.spyOn(bunPm, 'install') const bunUpdateSpy = vi.spyOn(bunPm, 'update') const bunUpdateManySpy = vi.spyOn(bunPm, 'updateMany') const bunUninstallSpy = vi.spyOn(bunPm, 'uninstall') +const bunGetInstalledVersionSpy = vi.spyOn(bunPm, 'getInstalledVersion') const cargoInstallSpy = vi.spyOn(cargoPm, 'install') const cargoUpdateSpy = vi.spyOn(cargoPm, 'update') const cargoUpdateManySpy = vi.spyOn(cargoPm, 'updateMany') @@ -78,6 +79,7 @@ beforeEach(() => { bunUpdateSpy.mockClear() bunUpdateManySpy.mockClear() bunUninstallSpy.mockClear() + bunGetInstalledVersionSpy.mockClear() cargoInstallSpy.mockClear() cargoUpdateSpy.mockClear() cargoUpdateManySpy.mockClear() @@ -133,6 +135,7 @@ afterAll(() => { bunUpdateSpy.mockRestore() bunUpdateManySpy.mockRestore() bunUninstallSpy.mockRestore() + bunGetInstalledVersionSpy.mockRestore() cargoInstallSpy.mockRestore() cargoUpdateSpy.mockRestore() cargoUpdateManySpy.mockRestore() @@ -1002,4 +1005,48 @@ describe('uninstallAgent', () => { expect(bunUninstallSpy).not.toHaveBeenCalled() expect(removeInstalledAgentStateSpy).toHaveBeenCalledWith('test-agent') }) + + it('recovers ghost state when managed uninstall fails but the package is already absent', async () => { + getInstalledAgentStateSpy.mockResolvedValue({ + agentName: 'test-agent', + installType: 'bun', + packageName: 'test-pkg', + }) + isBunSpy.mockResolvedValue(true) + bunUninstallSpy.mockResolvedValue(false) + bunGetInstalledVersionSpy.mockResolvedValue(undefined) + + expect(await uninstallAgent(testAgent)).toBe(true) + expect(bunUninstallSpy).toHaveBeenCalledWith('test-pkg') + expect(bunGetInstalledVersionSpy).toHaveBeenCalledWith('test-pkg') + expect(removeInstalledAgentStateSpy).toHaveBeenCalledWith('test-agent') + }) + + it('does not recover ghost state when the managed package is still installed', async () => { + getInstalledAgentStateSpy.mockResolvedValue({ + agentName: 'test-agent', + installType: 'bun', + packageName: 'test-pkg', + }) + isBunSpy.mockResolvedValue(true) + bunUninstallSpy.mockResolvedValue(false) + bunGetInstalledVersionSpy.mockResolvedValue('1.2.3') + + expect(await uninstallAgent(testAgent)).toBe(false) + expect(removeInstalledAgentStateSpy).not.toHaveBeenCalled() + }) + + it('does not recover ghost state when the package manager is unavailable', async () => { + getInstalledAgentStateSpy.mockResolvedValue({ + agentName: 'test-agent', + installType: 'bun', + packageName: 'test-pkg', + }) + isBunSpy.mockResolvedValue(false) + bunUninstallSpy.mockResolvedValue(false) + + expect(await uninstallAgent(testAgent)).toBe(false) + expect(bunGetInstalledVersionSpy).not.toHaveBeenCalled() + expect(removeInstalledAgentStateSpy).not.toHaveBeenCalled() + }) })