fix(lifecycle): fail closed on inconclusive npm ghost probes

openspec/changes/fix-npm-ghost-probe-false-positive/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-07-01

openspec/changes/fix-npm-ghost-probe-false-positive/design.md

@@ -0,0 +1,30 @@
+## Context
+
+`isManagedPackageAbsent()` delegates to installer `getInstalledVersion` hooks. npm's implementation runs `npm list -g --depth=0 --json` and returns `undefined` whenever the command exits non-zero, even when stdout contains valid JSON showing the target package is still installed.
+
+## Goals / Non-Goals
+
+**Goals**
+
+- Confirm npm package absence only when a scoped probe parses valid JSON and the package is missing.
+- Fail closed on inconclusive npm probes during ghost recovery.
+- Keep version probing behavior stable for callers that only need a version string.
+
+**Non-Goals**
+
+- Rework Windows deferred binary self-upgrade semantics.
+- Add doctor-only repair flows.
+- Change bun or other installer ghost recovery beyond existing behavior.
+
+## Decisions
+
+- Add an npm `probePackagePresence` helper returning `present`, `absent`, or `unknown`.
+- Use scoped `npm list -g <package> --depth=0 --json` and parse stdout regardless of exit code when JSON is valid.
+- Treat a matching dependency key as present even when its version is unreadable, and treat structured npm error payloads as unknown.
+- Extend the managed installer interface with optional `probePackagePresence` and use it in `isManagedPackageAbsent()` when available.
+- Fall back to the existing `getInstalledVersion === undefined` check for installers without the new hook.
+
+## Risks / Trade-offs
+
+- [Risk] Scoped npm list still returns inconclusive output on some broken npm installs. → Mitigation: return `unknown` and skip ghost recovery.
+- [Risk] Extra probe hook adds interface surface. → Mitigation: optional hook used only for absence confirmation.

openspec/changes/fix-npm-ghost-probe-false-positive/proposal.md

@@ -0,0 +1,24 @@
+## Why
+
+The recent ghost-state uninstall recovery path treats npm `getInstalledVersion` returning `undefined` as confirmed package absence. npm's global `list` probe returns `undefined` on any non-zero exit code without parsing stdout, so a broken global dependency tree or probe failure can clear tracked state while the agent package remains installed.
+
+## What Changes
+
+- Harden npm managed package presence probing for ghost uninstall recovery.
+- Distinguish confirmed absence from inconclusive probe results before clearing state.
+- Add regression tests for npm probe failure and confirmed-absence paths.
+
+## Capabilities
+
+### New Capabilities
+
+- None.
+
+### Modified Capabilities
+
+- `agent-update`: ghost uninstall recovery must not clear state when npm presence probing is inconclusive.
+
+## Impact
+
+- Affected code: `src/package-manager/npm.ts`, `src/package-manager/installers.ts`, `src/package-manager/index.ts`, related tests.
+- No CLI flags, schema version, or command catalog changes.

openspec/changes/fix-npm-ghost-probe-false-positive/specs/agent-update/spec.md

@@ -0,0 +1,34 @@
+## MODIFIED Requirements
+
+### 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 <agent>`
+- **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 <agent>`
+- **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
+
+#### Scenario: Ghost recovery does not run when npm presence probing is inconclusive
+
+- **GIVEN** an agent has recorded managed install state with install type `npm`
+- **AND** npm is available
+- **AND** npm global presence probing cannot confirm whether the package is installed or absent
+- **WHEN** the user runs `quantex uninstall <agent>`
+- **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

openspec/changes/fix-npm-ghost-probe-false-positive/tasks.md

@@ -0,0 +1,10 @@
+## 1. Implementation
+
+- [x] 1.1 Add npm scoped presence probing with `present` / `absent` / `unknown` outcomes
+- [x] 1.2 Wire optional installer `probePackagePresence` into ghost recovery
+- [x] 1.3 Add regression tests for npm inconclusive probe and confirmed absence paths
+- [x] 1.4 Fail closed for structured npm errors and package entries with unreadable versions
+
+## 2. Validation
+
+- [x] 2.1 Run `bun run lint`, `bun run format:check`, `bun run typecheck`, and `bun run test`

src/package-manager/index.ts

@@ -350,6 +350,11 @@ async function isManagedPackageAbsent(
   const packageName = resolveManagedPackageName(state, agent)
   if (!packageName) return false
 
+  if (installer.probePackagePresence) {
+    const presence = await installer.probePackagePresence(packageName, state.packageTargetKind)
+    return presence === 'absent'
+  }
+
   const installedVersion = await installer.getInstalledVersion(packageName, state.packageTargetKind)
   return installedVersion === undefined
 }

src/package-manager/installers.ts

@@ -34,9 +34,15 @@ export interface ManagedInstallerUpdateOptions {
   packageInstallArgs?: string[]
 }
 
+export type ManagedPackagePresenceProbe = 'present' | 'absent' | 'unknown'
+
 export interface ManagedInstaller {
   type: ManagedInstallType
   getInstalledVersion?: (packageName: string, packageTargetKind?: PackageTargetKind) => Promise<string | undefined>
+  probePackagePresence?: (
+    packageName: string,
+    packageTargetKind?: PackageTargetKind,
+  ) => Promise<ManagedPackagePresenceProbe>
   isAvailable: () => Promise<boolean>
   install: (
     packageName: string,
@@ -123,6 +129,7 @@ const managedInstallers: Record<ManagedInstallType, ManagedInstaller> = {
   npm: {
     type: 'npm',
     getInstalledVersion: async packageName => npmPm.getInstalledVersion(packageName),
+    probePackagePresence: async packageName => npmPm.probePackagePresence(packageName),
     isAvailable: async () => isNpmAvailable(),
     install: async packageName => npmPm.install(packageName),
     uninstall: async packageName => npmPm.uninstall(packageName),

src/package-manager/npm.ts

@@ -79,21 +79,52 @@ export async function uninstall(packageName: string): Promise<boolean> {
   }
 }
 
-export async function getInstalledVersion(packageName: string): Promise<string | undefined> {
+export type PackagePresenceProbe = 'present' | 'absent' | 'unknown'
+
+interface PackagePresenceResult {
+  presence: PackagePresenceProbe
+  version?: string
+}
+
+async function readPackagePresence(packageName: string): Promise<PackagePresenceResult> {
   try {
-    const proc = spawnCommand(['npm', 'list', '-g', '--depth=0', '--json'], {
+    const proc = spawnCommand(['npm', 'list', '-g', packageName, '--depth=0', '--json'], {
       stdio: ['ignore', 'pipe', 'ignore'],
     })
-    const { exitCode, stdout } = await readProcessOutput(proc)
+    const { stdout } = await readProcessOutput(proc)
+    if (!stdout.trim()) return { presence: 'unknown' }
+
+    try {
+      const data = JSON.parse(stdout) as {
+        dependencies?: Record<string, { version?: unknown }>
+        error?: unknown
+      }
+      if (!data || typeof data !== 'object' || Array.isArray(data) || Object.hasOwn(data, 'error')) {
+        return { presence: 'unknown' }
+      }
 
-    if (exitCode !== 0) return undefined
+      if (data.dependencies && Object.hasOwn(data.dependencies, packageName)) {
+        const version = parseGlobalPackageVersion(stdout, packageName)
+        return version ? { presence: 'present', version } : { presence: 'present' }
+      }
 
-    return parseGlobalPackageVersion(stdout, packageName)
+      return { presence: 'absent' }
+    } catch {
+      return { presence: 'unknown' }
+    }
   } catch {
-    return undefined
+    return { presence: 'unknown' }
   }
 }
 
+export async function probePackagePresence(packageName: string): Promise<PackagePresenceProbe> {
+  return (await readPackagePresence(packageName)).presence
+}
+
+export async function getInstalledVersion(packageName: string): Promise<string | undefined> {
+  return (await readPackagePresence(packageName)).version
+}
+
 export function parseGlobalPackageVersion(output: string, packageName: string): string | undefined {
   const data = JSON.parse(output) as {
     dependencies?: Record<string, { version?: unknown }>

test/package-manager/index.test.ts

@@ -43,6 +43,7 @@ const npmInstallSpy = vi.spyOn(npmPm, 'install')
 const npmUpdateSpy = vi.spyOn(npmPm, 'update')
 const npmUpdateManySpy = vi.spyOn(npmPm, 'updateMany')
 const npmUninstallSpy = vi.spyOn(npmPm, 'uninstall')
+const npmProbePackagePresenceSpy = vi.spyOn(npmPm, 'probePackagePresence')
 const uvInstallSpy = vi.spyOn(uvPm, 'install')
 const uvUpdateSpy = vi.spyOn(uvPm, 'update')
 const uvUpdateManySpy = vi.spyOn(uvPm, 'updateMany')
@@ -96,6 +97,7 @@ beforeEach(() => {
   npmUpdateSpy.mockClear()
   npmUpdateManySpy.mockClear()
   npmUninstallSpy.mockClear()
+  npmProbePackagePresenceSpy.mockClear()
   uvInstallSpy.mockClear()
   uvUpdateSpy.mockClear()
   uvUpdateManySpy.mockClear()
@@ -152,6 +154,7 @@ afterAll(() => {
   npmUpdateSpy.mockRestore()
   npmUpdateManySpy.mockRestore()
   npmUninstallSpy.mockRestore()
+  npmProbePackagePresenceSpy.mockRestore()
   uvInstallSpy.mockRestore()
   uvUpdateSpy.mockRestore()
   uvUpdateManySpy.mockRestore()
@@ -1049,4 +1052,48 @@ describe('uninstallAgent', () => {
     expect(bunGetInstalledVersionSpy).not.toHaveBeenCalled()
     expect(removeInstalledAgentStateSpy).not.toHaveBeenCalled()
   })
+
+  it('recovers npm ghost state when uninstall fails but absence is confirmed', async () => {
+    getInstalledAgentStateSpy.mockResolvedValue({
+      agentName: 'test-agent',
+      installType: 'npm',
+      packageName: 'test-pkg',
+    })
+    isNpmSpy.mockResolvedValue(true)
+    npmUninstallSpy.mockResolvedValue(false)
+    npmProbePackagePresenceSpy.mockResolvedValue('absent')
+
+    expect(await uninstallAgent(testAgent)).toBe(true)
+    expect(npmUninstallSpy).toHaveBeenCalledWith('test-pkg')
+    expect(npmProbePackagePresenceSpy).toHaveBeenCalledWith('test-pkg')
+    expect(removeInstalledAgentStateSpy).toHaveBeenCalledWith('test-agent')
+  })
+
+  it('does not recover npm ghost state when presence probing is inconclusive', async () => {
+    getInstalledAgentStateSpy.mockResolvedValue({
+      agentName: 'test-agent',
+      installType: 'npm',
+      packageName: 'test-pkg',
+    })
+    isNpmSpy.mockResolvedValue(true)
+    npmUninstallSpy.mockResolvedValue(false)
+    npmProbePackagePresenceSpy.mockResolvedValue('unknown')
+
+    expect(await uninstallAgent(testAgent)).toBe(false)
+    expect(removeInstalledAgentStateSpy).not.toHaveBeenCalled()
+  })
+
+  it('does not recover npm ghost state when the package is still installed', async () => {
+    getInstalledAgentStateSpy.mockResolvedValue({
+      agentName: 'test-agent',
+      installType: 'npm',
+      packageName: 'test-pkg',
+    })
+    isNpmSpy.mockResolvedValue(true)
+    npmUninstallSpy.mockResolvedValue(false)
+    npmProbePackagePresenceSpy.mockResolvedValue('present')
+
+    expect(await uninstallAgent(testAgent)).toBe(false)
+    expect(removeInstalledAgentStateSpy).not.toHaveBeenCalled()
+  })
 })

test/package-manager/npm.test.ts

@@ -93,3 +93,125 @@ describe('npm uninstall', () => {
     expect(await uninstall('some-package')).toBe(false)
   })
 })
+
+function createListProc(exitCode: number, stdout: string) {
+  return {
+    exited: Promise.resolve(),
+    exitCode,
+    stdout,
+  }
+}
+
+describe('parseGlobalPackageVersion', () => {
+  it('parses scoped and unscoped packages from npm global list JSON', async () => {
+    const { parseGlobalPackageVersion } = await import('../../src/package-manager/npm')
+    const output = JSON.stringify({
+      dependencies: {
+        'test-pkg': { version: '1.2.3' },
+        '@scope/name': { version: '4.5.6' },
+      },
+    })
+
+    expect(parseGlobalPackageVersion(output, 'test-pkg')).toBe('1.2.3')
+    expect(parseGlobalPackageVersion(output, '@scope/name')).toBe('4.5.6')
+    expect(parseGlobalPackageVersion(output, 'missing-package')).toBeUndefined()
+  })
+})
+
+describe('probePackagePresence', () => {
+  it('returns present when scoped npm list JSON includes the package', async () => {
+    const { probePackagePresence } = await import('../../src/package-manager/npm')
+    mockSpawn.mockReturnValue(
+      createListProc(
+        1,
+        JSON.stringify({
+          dependencies: {
+            'test-pkg': { version: '1.2.3' },
+          },
+        }),
+      ),
+    )
+
+    expect(await probePackagePresence('test-pkg')).toBe('present')
+    expect(mockSpawn).toHaveBeenCalledWith(['npm', 'list', '-g', 'test-pkg', '--depth=0', '--json'], expect.any(Object))
+  })
+
+  it('returns absent when scoped npm list JSON omits the package', async () => {
+    const { probePackagePresence } = await import('../../src/package-manager/npm')
+    mockSpawn.mockReturnValue(
+      createListProc(
+        1,
+        JSON.stringify({
+          dependencies: {},
+        }),
+      ),
+    )
+
+    expect(await probePackagePresence('test-pkg')).toBe('absent')
+  })
+
+  it('returns present when the package entry exists without a readable version', async () => {
+    const { probePackagePresence } = await import('../../src/package-manager/npm')
+    mockSpawn.mockReturnValue(
+      createListProc(
+        1,
+        JSON.stringify({
+          dependencies: {
+            'test-pkg': { invalid: true },
+          },
+        }),
+      ),
+    )
+
+    expect(await probePackagePresence('test-pkg')).toBe('present')
+  })
+
+  it('returns unknown when npm reports a structured error', async () => {
+    const { probePackagePresence } = await import('../../src/package-manager/npm')
+    mockSpawn.mockReturnValue(
+      createListProc(
+        1,
+        JSON.stringify({
+          error: {
+            code: 'EJSONPARSE',
+            summary: 'Invalid package metadata',
+          },
+        }),
+      ),
+    )
+
+    expect(await probePackagePresence('test-pkg')).toBe('unknown')
+  })
+
+  it('returns unknown when npm list output is empty', async () => {
+    const { probePackagePresence } = await import('../../src/package-manager/npm')
+    mockSpawn.mockReturnValue(createListProc(1, ''))
+
+    expect(await probePackagePresence('test-pkg')).toBe('unknown')
+  })
+
+  it('returns unknown when npm list output is not valid JSON', async () => {
+    const { probePackagePresence } = await import('../../src/package-manager/npm')
+    mockSpawn.mockReturnValue(createListProc(1, 'npm ERR! broken'))
+
+    expect(await probePackagePresence('test-pkg')).toBe('unknown')
+  })
+})
+
+describe('getInstalledVersion', () => {
+  it('returns the version when scoped npm list JSON includes the package', async () => {
+    const { getInstalledVersion } = await import('../../src/package-manager/npm')
+    mockSpawn.mockReturnValue(
+      createListProc(
+        1,
+        JSON.stringify({
+          dependencies: {
+            'test-pkg': { version: '2.0.0' },
+          },
+        }),
+      ),
+    )
+
+    expect(await getInstalledVersion('test-pkg')).toBe('2.0.0')
+  })
+})