From d895b45354abf99f1f988d10c38472a7d5b6857f Mon Sep 17 00:00:00 2001 From: Drswith <49299002+Drswith@users.noreply.github.com> Date: Thu, 2 Jul 2026 01:05:56 +0000 Subject: [PATCH] fix(lifecycle): roll back bun install when trust verification fails When bun add -g succeeds but post-install trust verification fails, installAgent() falls through to the next install method and leaves a duplicate Bun global package on disk. Roll back packages added by a successful bun add -g before reporting install failure. Update paths are unchanged. OpenSpec change: fix-bun-install-trust-rollback --- .../.openspec.yaml | 2 + .../fix-bun-install-trust-rollback/design.md | 27 +++++++++++++ .../proposal.md | 24 +++++++++++ .../specs/agent-update/spec.md | 40 +++++++++++++++++++ .../fix-bun-install-trust-rollback/tasks.md | 10 +++++ src/package-manager/bun.ts | 17 +++++++- test/package-manager/bun.test.ts | 20 +++++++++- test/package-manager/index.test.ts | 21 ++++++++++ 8 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 openspec/changes/fix-bun-install-trust-rollback/.openspec.yaml create mode 100644 openspec/changes/fix-bun-install-trust-rollback/design.md create mode 100644 openspec/changes/fix-bun-install-trust-rollback/proposal.md create mode 100644 openspec/changes/fix-bun-install-trust-rollback/specs/agent-update/spec.md create mode 100644 openspec/changes/fix-bun-install-trust-rollback/tasks.md diff --git a/openspec/changes/fix-bun-install-trust-rollback/.openspec.yaml b/openspec/changes/fix-bun-install-trust-rollback/.openspec.yaml new file mode 100644 index 00000000..8e26fbec --- /dev/null +++ b/openspec/changes/fix-bun-install-trust-rollback/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-02 diff --git a/openspec/changes/fix-bun-install-trust-rollback/design.md b/openspec/changes/fix-bun-install-trust-rollback/design.md new file mode 100644 index 00000000..c54c20a4 --- /dev/null +++ b/openspec/changes/fix-bun-install-trust-rollback/design.md @@ -0,0 +1,27 @@ +## Context + +`runGlobalBunCommandWithTrust()` runs `bun add -g` or `bun update -g`, then probes `bun pm -g untrusted` and may run `bun pm -g trust`. When trust verification fails, the function returns `false` even though the primary command already mutated global packages. `installAgent()` treats that as a failed attempt and continues to the next install method. + +## Goals / Non-Goals + +**Goals** + +- Remove packages added by a successful `bun add -g` when trust verification fails afterward. +- Preserve existing fail-closed semantics for trust probe failures. +- Allow safe fallback to the next install method without leaving a duplicate Bun global install. + +**Non-Goals** + +- Roll back successful `bun update -g` mutations on trust failure. +- Change npm/bun fallback ordering or installer selection. + +## Decisions + +- Detect install vs update from the Bun subcommand in the command array (`add` vs `update`). +- On trust failure after `add`, best-effort `bun remove -g` for each requested package name before returning `false`. +- Keep rollback inside `bun.ts` so all Bun install entry points share the behavior. + +## Risks / Trade-offs + +- [Risk] Rollback remove fails and leaves the package installed. → Mitigation: best-effort remove; fallback install methods still work, but duplicate risk is reduced when remove succeeds. +- [Risk] Partial batch add in `updateMany` is not in scope; Bun install paths use single-package commands today. diff --git a/openspec/changes/fix-bun-install-trust-rollback/proposal.md b/openspec/changes/fix-bun-install-trust-rollback/proposal.md new file mode 100644 index 00000000..31f3c1a0 --- /dev/null +++ b/openspec/changes/fix-bun-install-trust-rollback/proposal.md @@ -0,0 +1,24 @@ +## Why + +When a Bun global install succeeds but post-install trust verification fails, `bun.ts` returns `false` without removing the package that was just added. `installAgent()` then tries the next install method (often npm), leaving duplicate global installs and an untracked Bun copy on disk. + +## What Changes + +- Roll back Bun global packages when trust verification fails after a successful `bun add -g`. +- Leave update paths unchanged: failed trust after `bun update -g` must not remove an already-installed package. +- Add regression tests for rollback-on-trust-failure and for install fallback after rollback. + +## Capabilities + +### New Capabilities + +- None. + +### Modified Capabilities + +- `agent-update`: Bun-managed install trust failure after a successful add must roll back the newly installed package before reporting failure. + +## Impact + +- Affected code: `src/package-manager/bun.ts`, `test/package-manager/bun.test.ts`, `test/package-manager/index.test.ts`. +- No CLI flags, schema version, or command catalog changes. diff --git a/openspec/changes/fix-bun-install-trust-rollback/specs/agent-update/spec.md b/openspec/changes/fix-bun-install-trust-rollback/specs/agent-update/spec.md new file mode 100644 index 00000000..49f1b45e --- /dev/null +++ b/openspec/changes/fix-bun-install-trust-rollback/specs/agent-update/spec.md @@ -0,0 +1,40 @@ +## MODIFIED Requirements + +### Requirement: Bun-managed updates MUST trust requested blocked lifecycle scripts across platform path styles + +The agent update system SHALL recognize Bun global untrusted package output for requested managed packages regardless of whether Bun prints `node_modules` paths with POSIX or Windows separators. When the untrusted probe cannot be read after a successful Bun global install or update command, Quantex SHALL NOT report that managed operation as successful. When trust verification fails after a successful Bun global **install** command, Quantex SHALL roll back the newly added package before reporting install failure. + +#### Scenario: Trusting a requested scoped package from Windows Bun output + +- GIVEN a Bun-managed agent package was requested by `quantex install`, `quantex update `, or `quantex update --all` +- AND `bun pm -g untrusted` reports that package using a Windows-style path such as `.\node_modules\@scope\name @1.2.3` +- WHEN the Bun install or update command exits successfully +- THEN Quantex trusts the requested package lifecycle script +- AND the agent update does not leave the requested package's required postinstall blocked because of path separator parsing +- AND the managed operation is reported as successful only after trust completes successfully + +#### Scenario: Ignoring unrelated blocked packages + +- GIVEN a Bun-managed install or update requested one or more package names +- AND `bun pm -g untrusted` reports additional packages that were not requested +- WHEN Quantex evaluates blocked lifecycle packages +- THEN Quantex only trusts blocked packages whose names match the requested package list + +#### Scenario: Failing closed when the untrusted probe is unavailable + +- GIVEN a Bun-managed install or update requested one or more package names +- AND the Bun global install or update command exits successfully +- AND `bun pm -g untrusted` exits non-zero or cannot be executed +- WHEN Quantex evaluates blocked lifecycle scripts +- THEN Quantex reports the managed operation as failed +- AND it does not claim the install or update succeeded without completing trust verification + +#### Scenario: Rolling back a Bun install when trust verification fails + +- GIVEN a Bun-managed install requested one or more package names +- AND `bun add -g` exits successfully for those packages +- AND Bun trust verification fails afterward +- WHEN Quantex evaluates the managed install outcome +- THEN Quantex removes the packages that were just added with `bun remove -g` +- AND it reports the Bun install attempt as failed +- AND a subsequent fallback install method may run without leaving a duplicate Bun global install behind diff --git a/openspec/changes/fix-bun-install-trust-rollback/tasks.md b/openspec/changes/fix-bun-install-trust-rollback/tasks.md new file mode 100644 index 00000000..65f0b942 --- /dev/null +++ b/openspec/changes/fix-bun-install-trust-rollback/tasks.md @@ -0,0 +1,10 @@ +## 1. Implementation + +- [x] 1.1 Roll back Bun global packages on trust failure after successful `bun add -g` +- [x] 1.2 Add regression tests in `test/package-manager/bun.test.ts` +- [x] 1.3 Add installAgent fallback regression test 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/bun.ts b/src/package-manager/bun.ts index 4713a33c..ecea2dbf 100644 --- a/src/package-manager/bun.ts +++ b/src/package-manager/bun.ts @@ -99,7 +99,22 @@ async function runGlobalBunCommandWithTrust(command: string[], packageNames: str if (exitCode !== 0) return false - return trustBlockedGlobalPackages(packageNames) + const trusted = await trustBlockedGlobalPackages(packageNames) + if (!trusted && command[1] === 'add') { + await rollbackGlobalBunPackages(packageNames) + } + + return trusted +} + +async function rollbackGlobalBunPackages(packageNames: string[]): Promise { + for (const packageName of new Set(packageNames)) { + try { + await waitForSpawnedCommand(spawnWithQuantexStdio(['bun', 'remove', '-g', packageName])) + } catch { + // Best-effort rollback so fallback install methods do not inherit duplicate Bun globals. + } + } } async function trustBlockedGlobalPackages(packageNames: string[]): Promise { diff --git a/test/package-manager/bun.test.ts b/test/package-manager/bun.test.ts index 7b905b71..13693521 100644 --- a/test/package-manager/bun.test.ts +++ b/test/package-manager/bun.test.ts @@ -48,11 +48,12 @@ describe('bun install', () => { it('returns false when the untrusted probe fails after install', async () => { const { install } = await import('../../src/package-manager/bun') - mockSpawn.mockReturnValueOnce(createProc(0)).mockReturnValueOnce(createProc(1)) + mockSpawn.mockReturnValueOnce(createProc(0)).mockReturnValueOnce(createProc(1)).mockReturnValueOnce(createProc(0)) expect(await install('some-package')).toBe(false) expect(mockSpawn).toHaveBeenNthCalledWith(2, ['bun', 'pm', '-g', 'untrusted'], expect.any(Object)) - expect(mockSpawn).toHaveBeenCalledTimes(2) + expect(mockSpawn).toHaveBeenNthCalledWith(3, ['bun', 'remove', '-g', 'some-package'], expect.any(Object)) + expect(mockSpawn).toHaveBeenCalledTimes(3) }) it('trusts blocked postinstall packages after install', async () => { @@ -106,6 +107,21 @@ describe('bun update', () => { expect(await update('some-package')).toBe(false) expect(mockSpawn).toHaveBeenNthCalledWith(3, ['bun', 'pm', '-g', 'trust', 'some-package'], expect.any(Object)) + expect(mockSpawn).toHaveBeenCalledTimes(3) + }) + + it('rolls back install when trust fails for a blocked package', async () => { + const { install } = await import('../../src/package-manager/bun') + mockSpawn + .mockReturnValueOnce(createProc(0)) + .mockReturnValueOnce(createProc(0, './node_modules/some-package @1.0.0\n » [postinstall]: node install.cjs\n')) + .mockReturnValueOnce(createProc(1)) + .mockReturnValueOnce(createProc(0)) + + expect(await install('some-package')).toBe(false) + expect(mockSpawn).toHaveBeenNthCalledWith(3, ['bun', 'pm', '-g', 'trust', 'some-package'], expect.any(Object)) + expect(mockSpawn).toHaveBeenNthCalledWith(4, ['bun', 'remove', '-g', 'some-package'], expect.any(Object)) + expect(mockSpawn).toHaveBeenCalledTimes(4) }) }) diff --git a/test/package-manager/index.test.ts b/test/package-manager/index.test.ts index e44e7e94..41ea9a0a 100644 --- a/test/package-manager/index.test.ts +++ b/test/package-manager/index.test.ts @@ -286,6 +286,27 @@ describe('installAgent', () => { expect(npmInstallSpy).toHaveBeenCalledWith('test-pkg') }) + it('falls back to the next install method after bun trust failure rolls back the install', async () => { + isBunSpy.mockResolvedValue(true) + isNpmSpy.mockResolvedValue(true) + bunInstallSpy.mockResolvedValue(false) + bunUninstallSpy.mockResolvedValue(true) + npmInstallSpy.mockResolvedValue(true) + setInstalledAgentStateSpy.mockResolvedValue() + + expect(await installAgent(testAgent)).toMatchObject({ + success: true, + installedState: { + agentName: 'test-agent', + installType: 'npm', + packageName: 'test-pkg', + }, + }) + expect(bunInstallSpy).toHaveBeenCalledWith('test-pkg') + expect(bunUninstallSpy).not.toHaveBeenCalled() + expect(npmInstallSpy).toHaveBeenCalledWith('test-pkg') + }) + it('returns false if all methods fail', async () => { isBunSpy.mockResolvedValue(true) isNpmSpy.mockResolvedValue(true)