Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-07-02
27 changes: 27 additions & 0 deletions openspec/changes/fix-bun-install-trust-rollback/design.md
Original file line number Diff line number Diff line change
@@ -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.
24 changes: 24 additions & 0 deletions openspec/changes/fix-bun-install-trust-rollback/proposal.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (archive follow-up): This delta re-lists existing Bun trust scenarios plus the new rollback scenario. The archive PR must merge additively into openspec/specs/agent-update/spec.md — extend the existing requirement with the rollback sentence/scenario only; do not replace or drop the current spec text.


#### Scenario: Trusting a requested scoped package from Windows Bun output

- GIVEN a Bun-managed agent package was requested by `quantex install`, `quantex update <agent>`, 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
10 changes: 10 additions & 0 deletions openspec/changes/fix-bun-install-trust-rollback/tasks.md
Original file line number Diff line number Diff line change
@@ -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`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: bun run openspec:validate is recorded here but omitted from the PR Validation checklist. For future behavior PRs, mirror OpenSpec validation in the template checklist when openspec/ changes.

17 changes: 16 additions & 1 deletion src/package-manager/bun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
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<boolean> {
Expand Down
20 changes: 18 additions & 2 deletions test/package-manager/bun.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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)
})
})

Expand Down
21 changes: 21 additions & 0 deletions test/package-manager/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading