Add show inspection and rollback-safe update UX
This commit is contained in:
parent
27a1b806cd
commit
1ad2f8a532
SSH key fingerprint:
SHA256:AZlPfu9hTu042EGtZElmDQoy+KvMOeShLDan/fYLoNI
16 changed files with 2187 additions and 7 deletions
|
|
@ -0,0 +1,171 @@
|
|||
# Show Command And Update Rollback Design
|
||||
|
||||
## Summary
|
||||
|
||||
This change adds a read-only `aim show <value>` command and hardens `aim update` so a failed reinstall restores the previously installed payload and generated integration files when possible. The public UX stays small: one `show` command for both installed-app inspection and install-query inspection, plus safer update execution without introducing a separate rollback command.
|
||||
|
||||
## Goals
|
||||
|
||||
- Add a single `aim show <value>` command for inspecting either an installed app or a resolvable install query.
|
||||
- Resolve `show` inputs by checking installed apps first, then falling back to source/query resolution.
|
||||
- Keep `show` read-only and reuse existing core resolution logic instead of creating a parallel inspection pipeline.
|
||||
- Make update execution restore the previous installation files if the replacement install fails after touching the filesystem.
|
||||
- Preserve the current registry-write ordering so only successful end-state records are written back.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- No standalone `aim rollback` command in this slice.
|
||||
- No pinning or update-policy UX in this slice.
|
||||
- No machine-readable `show --json` output yet.
|
||||
- No registry backup or recovery feature beyond per-update install rollback.
|
||||
- No redesign of existing `add`, `list`, `search`, or `remove` flows outside the minimum shared logic needed for `show` and rollback.
|
||||
|
||||
## Approaches
|
||||
|
||||
### Option 1: Separate `info` and `show` commands
|
||||
|
||||
This keeps installed-app inspection and remote-query inspection conceptually separate, but it forces the user to learn two entry points for what is effectively the same question: "what is this and what would aim do with it?" It also duplicates argument parsing, dispatch, help text, and render logic.
|
||||
|
||||
### Option 2: Make `show` a thin CLI-only wrapper around existing add and list code
|
||||
|
||||
This is faster to wire initially, but it would push installed lookup rules and query fallback rules into `aim-cli`, where they become harder to test and easier to drift from add/remove behavior. It also makes rollback hardening more likely to stay bolted onto `update.rs` without a clear boundary.
|
||||
|
||||
### Option 3: Add a unified core `show` service plus internal update rollback handling
|
||||
|
||||
This is the recommended approach. `aim-core` owns inspection and rollback behavior, while `aim-cli` remains responsible for command parsing and text rendering. The result is a small public surface with testable domain behavior and no extra persistent state.
|
||||
|
||||
## Approved Design
|
||||
|
||||
### Public Command Behavior
|
||||
|
||||
Add `aim show <value>` as a new subcommand. The command accepts the same broad input shapes already supported by install resolution: installed stable ids, display-name-like inputs, provider locators such as `owner/repo`, and supported URLs.
|
||||
|
||||
Resolution order is:
|
||||
|
||||
1. attempt installed-app lookup
|
||||
2. if there is one clear installed match, show installed details
|
||||
3. if there are no installed matches, fall back to query/source resolution
|
||||
4. if installed lookup is ambiguous, fail without remote fallback
|
||||
|
||||
The ambiguity rule matters because an ambiguous installed match should not silently switch meaning and inspect a remote source instead. This should behave like the safe side of `remove`: when the input is not specific enough, the command tells the user to disambiguate.
|
||||
|
||||
### Installed-App Show Output
|
||||
|
||||
Installed inspection should render a concise but complete summary of the current record. The output should include:
|
||||
|
||||
- a compact summary line combining display name, stable id, installed version, and install scope
|
||||
- a split title row with app name and stable id on the left, and installed version, inline update-status tag, and scope on the right when terminal width allows it
|
||||
- a stacked fallback layout for narrower terminals rather than truncating the title row
|
||||
- a secondary source line under the title row that shows provider and normalized source locator
|
||||
- original source input only when it materially differs from the displayed source
|
||||
- a compact `N past versions` history indicator below the source line, including the latest known version when the install is behind
|
||||
- a small metadata block above files, with separate themed sibling lines for metadata kind plus architecture and for checksum
|
||||
- installed payload, desktop entry, and icon paths rendered as the same style of secondary subinfo rather than a heavier bullet list
|
||||
|
||||
This is not intended to dump raw registry TOML. It should be a stable human-oriented summary that answers how the app was installed, whether it is behind, what file paths are currently tracked, and what update lineage exists without repeating a long metadata block.
|
||||
|
||||
### Remote Query Show Output
|
||||
|
||||
If installed lookup finds no match, `show` should resolve the input the same way `add` does, but stop before performing installation. The result should summarize:
|
||||
|
||||
- resolved source kind and locator
|
||||
- selected artifact URL
|
||||
- resolved version when available
|
||||
- trusted checksum when available
|
||||
- artifact selection reason
|
||||
- interaction requirements if the add flow would require user choice or confirmation
|
||||
- warnings produced during resolution
|
||||
|
||||
The remote path should reuse existing add-plan building logic rather than creating a second source-resolution implementation. This keeps install behavior and inspection behavior aligned.
|
||||
|
||||
### Core Architecture
|
||||
|
||||
Add a new inspection module in `aim-core`, with a small domain type such as `ShowResult` that covers the two successful result shapes:
|
||||
|
||||
- installed app details
|
||||
- resolved remote add-plan details
|
||||
|
||||
The core service should accept the user input plus the current installed app list and return either a `ShowResult` or a typed error describing:
|
||||
|
||||
- ambiguous installed match
|
||||
- unsupported query
|
||||
- no installable artifact
|
||||
- provider resolution failure
|
||||
|
||||
This keeps the installed-first policy and error classification in one place. `aim-cli` then only needs to parse the new command, dispatch to the core service, and render the returned structure.
|
||||
|
||||
### CLI Architecture
|
||||
|
||||
`aim-cli` should add a `Show { value: String }` subcommand and a corresponding `DispatchResult::Show(...)` branch. Rendering belongs in the existing text renderer alongside add/list/search/update summaries.
|
||||
|
||||
There should not be separate public `show-installed` and `show-remote` result types in the CLI. The renderer can branch on the shared `ShowResult` model and produce headings such as `Installed App` or `Resolved Source`.
|
||||
|
||||
### Update Rollback Design
|
||||
|
||||
Rollback belongs inside update execution, not in CLI dispatch. `execute_update(...)` already has the install boundary where the old app record, install home, and reinstall attempt are all visible. That is the right point to stage a backup, perform the install, and restore on failure.
|
||||
|
||||
Before reinstalling an app with tracked installation paths, update execution should:
|
||||
|
||||
1. collect the currently tracked payload, desktop entry, and icon paths that still exist
|
||||
2. move those files into a rollback staging directory under the install home
|
||||
3. attempt the replacement install
|
||||
4. on success, delete the rollback staging directory
|
||||
5. on failure, restore the old files to their original locations and return the original app record as the retained registry state
|
||||
|
||||
The rollback staging directory should be private to update execution, deterministic enough to debug, and cleaned up best-effort after either success or restore.
|
||||
|
||||
### Rollback Result Semantics
|
||||
|
||||
The registry should continue to be mutated only after update execution finishes, using the returned app list. That means the current high-level safety property remains unchanged:
|
||||
|
||||
- successful update returns the new record
|
||||
- failed update returns the old record
|
||||
|
||||
The new behavior is filesystem safety. If reinstall fails after replacing or partially generating files, update execution should attempt to restore the old payload and integration files before reporting failure.
|
||||
|
||||
Restore failure should remain visible. The failure reason should include whether the install failed, whether rollback restoration also failed, and which files were involved. This can be represented as a richer failure string in this slice; a new structured rollback-status enum is not required unless the implementation clearly benefits from it.
|
||||
|
||||
### Edge Cases
|
||||
|
||||
- If an app has no tracked install paths, rollback is a no-op and the update can fail exactly as it does today.
|
||||
- If backup creation fails before the replacement install starts, the update should abort rather than risk destructive partial replacement.
|
||||
- If some tracked files are already missing, backup should proceed with the files that still exist and record the rest as warnings.
|
||||
- If installed lookup for `show` is ambiguous, return an ambiguity error and do not attempt remote resolution.
|
||||
- Unsupported source input and "no installable artifact" should remain distinct outcomes in the remote `show` path.
|
||||
- `show` remains read-only even if the resolved add plan contains interactions; it should describe them rather than prompt.
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
### Show Coverage
|
||||
|
||||
Add core tests for:
|
||||
|
||||
- exact installed match returning installed details
|
||||
- no installed match falling back to remote resolution
|
||||
- ambiguous installed matches returning a safe error
|
||||
- unsupported input remaining distinct from no-installable-artifact
|
||||
- remote result carrying artifact URL, checksum, and warnings through the summary model
|
||||
|
||||
Add CLI tests for:
|
||||
|
||||
- `aim show <installed-id>` rendering installed details
|
||||
- `aim show <query>` rendering resolved source details
|
||||
- ambiguity and provider errors surfacing readable messages
|
||||
|
||||
### Rollback Coverage
|
||||
|
||||
Add update execution tests for:
|
||||
|
||||
- successful update removes rollback staging artifacts
|
||||
- failed update restores the original payload path
|
||||
- failed update restores desktop entry and icon files when present
|
||||
- backup creation failure aborts before destructive replacement
|
||||
- restore failure reports a compound reason and still keeps the original record in registry output
|
||||
|
||||
The tests should prefer temporary directories and fixture transports over shelling out or relying on real network calls.
|
||||
|
||||
## Delivery Notes
|
||||
|
||||
- Do not add persistent rollback metadata to the registry for this slice.
|
||||
- Prefer new focused modules for `show` rather than making `lib.rs` or `update.rs` absorb more branching.
|
||||
- Keep the text output stable and human-readable so later `--json` work can be added as a separate renderer decision instead of reworking the domain model.
|
||||
|
|
@ -0,0 +1,168 @@
|
|||
# Show Command And Update Rollback Implementation Plan
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
||||
|
||||
**Goal:** Add a read-only `aim show <value>` command that inspects installed apps first and falls back to remote source resolution, while making `aim update` restore the previous installation files when reinstall fails.
|
||||
|
||||
**Architecture:** Add a small `aim-core` show service that returns either installed details or resolved add-plan details, then wire a single CLI subcommand and text renderer around that result. Harden update execution in `aim-core` by staging tracked install files before reinstall and restoring them on failure without introducing new registry state.
|
||||
|
||||
**Tech Stack:** Rust, clap, serde, toml, tempfile, assert_cmd
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Add the core `show` domain model and resolution service
|
||||
|
||||
**Files:**
|
||||
- Create: `crates/aim-core/src/app/show.rs`
|
||||
- Create: `crates/aim-core/src/domain/show.rs`
|
||||
- Modify: `crates/aim-core/src/app/mod.rs`
|
||||
- Modify: `crates/aim-core/src/domain/mod.rs`
|
||||
- Test: `crates/aim-core/tests/show_resolution.rs`
|
||||
|
||||
**Step 1: Write the failing test**
|
||||
|
||||
Add core tests covering:
|
||||
|
||||
- one installed match returns installed details
|
||||
- no installed match falls back to remote resolution
|
||||
- ambiguous installed matches return a dedicated error
|
||||
- unsupported query stays distinct from `NoInstallableArtifact`
|
||||
|
||||
Include at least one remote-resolution fixture that proves the `show` result carries artifact URL, selection reason, and trusted checksum.
|
||||
|
||||
**Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cargo test --package aim-core --test show_resolution`
|
||||
Expected: FAIL because the show domain and service do not exist.
|
||||
|
||||
**Step 3: Write minimal implementation**
|
||||
|
||||
Implement a new `show` service in `aim-core` that accepts the user input and installed app list, applies installed-first resolution, and returns a typed `ShowResult`.
|
||||
|
||||
Keep the installed branch focused on existing `AppRecord` data. Keep the remote branch focused on summarizing the already-built add plan instead of exposing all of `AddPlan` directly.
|
||||
|
||||
**Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cargo test --package aim-core --test show_resolution`
|
||||
Expected: PASS
|
||||
|
||||
### Task 2: Wire the `show` command through `aim-cli`
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/aim-cli/src/cli/args.rs`
|
||||
- Modify: `crates/aim-cli/src/lib.rs`
|
||||
- Modify: `crates/aim-cli/src/ui/render.rs`
|
||||
- Test: `crates/aim-cli/tests/cli_commands.rs`
|
||||
- Test: `crates/aim-cli/tests/ui_summary.rs`
|
||||
|
||||
**Step 1: Write the failing test**
|
||||
|
||||
Add CLI coverage for:
|
||||
|
||||
- `aim show legacy-bat` dispatching successfully and rendering installed details
|
||||
- `aim show owner/repo` rendering resolved source and artifact details
|
||||
- ambiguous installed lookup rendering a readable failure
|
||||
|
||||
Keep the rendering assertions focused on stable summary lines rather than exact spacing across the entire output block.
|
||||
|
||||
**Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cargo test --package aim-cli --test cli_commands --test ui_summary`
|
||||
Expected: FAIL because the CLI has no `show` command or renderer.
|
||||
|
||||
**Step 3: Write minimal implementation**
|
||||
|
||||
Add `Show { value: String }` to the clap subcommands, route it through dispatch, convert core `ShowResult` into a new `DispatchResult::Show(...)` variant, and render installed and remote summaries in `ui::render`.
|
||||
|
||||
Do not add prompting or mutation to this command.
|
||||
|
||||
**Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cargo test --package aim-cli --test cli_commands --test ui_summary`
|
||||
Expected: PASS
|
||||
|
||||
### Task 3: Add rollback staging for update execution
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/aim-core/src/app/update.rs`
|
||||
- Modify: `crates/aim-core/src/domain/update.rs`
|
||||
- Test: `crates/aim-core/tests/update_planning.rs`
|
||||
|
||||
**Step 1: Write the failing test**
|
||||
|
||||
Add update execution tests covering:
|
||||
|
||||
- a failed reinstall restores the original payload file contents
|
||||
- a failed reinstall keeps returning the previous `AppRecord`
|
||||
- a successful reinstall removes any rollback staging directory
|
||||
|
||||
Use a temporary install home and deterministic fixture inputs so the test does not depend on external services.
|
||||
|
||||
**Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cargo test --package aim-core --test update_planning`
|
||||
Expected: FAIL because update execution does not back up or restore tracked files.
|
||||
|
||||
**Step 3: Write minimal implementation**
|
||||
|
||||
Add a small rollback helper inside `update.rs` that gathers the tracked install paths, moves existing files into a staging directory under the install home, restores them on failure, and deletes the staging directory on success.
|
||||
|
||||
Only enrich `domain::update` if you need a better warning or failure surface for tests and summaries.
|
||||
|
||||
**Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cargo test --package aim-core --test update_planning`
|
||||
Expected: PASS
|
||||
|
||||
### Task 4: Cover desktop integration rollback and human-facing failure output
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/aim-core/src/app/update.rs`
|
||||
- Modify: `crates/aim-cli/src/ui/render.rs`
|
||||
- Test: `crates/aim-core/tests/install_failures.rs`
|
||||
- Test: `crates/aim-cli/tests/end_to_end_cli.rs`
|
||||
|
||||
**Step 1: Write the failing test**
|
||||
|
||||
Add coverage for:
|
||||
|
||||
- update rollback restoring desktop entry and icon files when replacement install fails after file moves
|
||||
- CLI update summary surfacing a rollback-aware failure reason instead of a generic install error
|
||||
|
||||
Use temporary directories and existing fixture-style test seams.
|
||||
|
||||
**Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cargo test --package aim-core --test install_failures && cargo test --package aim-cli --test end_to_end_cli`
|
||||
Expected: FAIL because desktop integration rollback is not restored and the failure output is not rollback-aware.
|
||||
|
||||
**Step 3: Write minimal implementation**
|
||||
|
||||
Extend the rollback helper to include tracked desktop integration paths and surface a clear failure reason when either backup creation or restore fails.
|
||||
|
||||
Keep the CLI output change small: reuse the existing update summary renderer and only improve the failure string content.
|
||||
|
||||
**Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cargo test --package aim-core --test install_failures && cargo test --package aim-cli --test end_to_end_cli`
|
||||
Expected: PASS
|
||||
|
||||
### Task 5: Final verification
|
||||
|
||||
**Files:**
|
||||
- Modify as required by prior tasks only
|
||||
|
||||
**Step 1: Run focused feature tests**
|
||||
|
||||
Run: `cargo test --package aim-core --test show_resolution --test update_planning --test install_failures && cargo test --package aim-cli --test cli_commands --test ui_summary --test end_to_end_cli`
|
||||
Expected: PASS
|
||||
|
||||
**Step 2: Run workspace formatting**
|
||||
|
||||
Run: `cargo fmt --all`
|
||||
Expected: PASS
|
||||
|
||||
**Step 3: Run workspace regression and lint checks**
|
||||
|
||||
Run: `cargo test --workspace && cargo clippy --workspace --all-targets --all-features -- -D warnings`
|
||||
Expected: PASS
|
||||
Loading…
Add table
Add a link
Reference in a new issue