-
Notifications
You must be signed in to change notification settings - Fork 0
fix(lifecycle): recover uninstall ghost state when package is absent #396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-06-30 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
43 changes: 43 additions & 0 deletions
43
openspec/changes/fix-uninstall-ghost-state/specs/agent-update/spec.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <agent>` | ||
| - **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 <agent>` | ||
| - **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 <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 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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` | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 非阻塞(PR body hygiene): 此处记录了 |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
非阻塞(archive):
MODIFIED块重新列出了 main 上已有的Uninstall MUST clear tracked unmanaged install state(内容未变)。归档到openspec/specs/agent-update/spec.md时请仅新增下方的 ghost recovery requirement,避免替换整块导致已有场景丢失(参见 #386 / #387 教训)。