feat: add update execution and provider contract design and implementation plans
This commit is contained in:
parent
842c390260
commit
8c4d6859ae
2 changed files with 293 additions and 0 deletions
|
|
@ -0,0 +1,115 @@
|
||||||
|
# Update Execution And Provider Contract Design
|
||||||
|
|
||||||
|
## Goal
|
||||||
|
|
||||||
|
Add real update execution to `aim` and tighten the provider abstraction so additional providers can implement a consistent normalize-and-resolve contract instead of each adapter inventing its own shape.
|
||||||
|
|
||||||
|
## Agreed Product Shape
|
||||||
|
|
||||||
|
### Update behavior
|
||||||
|
|
||||||
|
- `aim` with no args remains a review surface for installed updates
|
||||||
|
- `aim update` executes updates for all registered apps
|
||||||
|
- Update execution reuses the existing install engine so payload staging, desktop integration, icon extraction, and rollback semantics stay in one place
|
||||||
|
- Successful updates replace the stored app record with the newly installed record
|
||||||
|
- Failed updates are reported per app and do not prevent other apps from updating
|
||||||
|
|
||||||
|
### Provider contract
|
||||||
|
|
||||||
|
- `SourceAdapter` should define the core behaviors providers are expected to implement:
|
||||||
|
- `normalize(query)`
|
||||||
|
- `resolve(source)`
|
||||||
|
- Adapters should return a shared adapter error type rather than bespoke per-adapter error enums for basic contract failures
|
||||||
|
- Existing GitHub and GitLab adapters should be brought under that shared contract first
|
||||||
|
|
||||||
|
## Recommended Approach
|
||||||
|
|
||||||
|
Use a reuse-first update executor.
|
||||||
|
|
||||||
|
Instead of building a second install path for updates, the update flow should turn each installed app back into an install request and run it through `build_add_plan(...)` and `install_app(...)` using the stored source and install scope metadata. This keeps update behavior aligned with add/install and avoids duplicating staging, desktop integration, and rollback logic.
|
||||||
|
|
||||||
|
For adapters, tighten the trait with the minimum common operations now, without prematurely forcing full discovery logic for every provider. That gives the next provider a clear implementation target while keeping the current GitHub path intact.
|
||||||
|
|
||||||
|
## Update Execution Model
|
||||||
|
|
||||||
|
### 1. Select update targets
|
||||||
|
|
||||||
|
- Build the existing update review plan from installed app records
|
||||||
|
- Treat every planned item as an executable candidate when `aim update` runs
|
||||||
|
|
||||||
|
### 2. Reconstruct install intent
|
||||||
|
|
||||||
|
For each app:
|
||||||
|
|
||||||
|
- use `source_input` when available
|
||||||
|
- otherwise fall back to stored source locator
|
||||||
|
- use persisted install scope from install metadata when available
|
||||||
|
- default legacy records to user scope if no metadata exists
|
||||||
|
|
||||||
|
### 3. Apply update
|
||||||
|
|
||||||
|
- build a fresh add plan for the app query
|
||||||
|
- install using the existing install engine
|
||||||
|
- persist the newly returned `AppRecord`
|
||||||
|
|
||||||
|
### 4. Report results
|
||||||
|
|
||||||
|
For each app capture:
|
||||||
|
|
||||||
|
- updated app id and display name
|
||||||
|
- success or failure
|
||||||
|
- version before and after when available
|
||||||
|
- warnings from install/update execution
|
||||||
|
|
||||||
|
### 5. Persist registry last
|
||||||
|
|
||||||
|
- save the registry after processing all apps
|
||||||
|
- successful app records replace old ones
|
||||||
|
- failed apps retain their previous records
|
||||||
|
|
||||||
|
## Failure Model
|
||||||
|
|
||||||
|
- per-app failures do not abort the entire update command
|
||||||
|
- install failures reuse existing install cleanup behavior
|
||||||
|
- the command returns a summary including updated count and failed count
|
||||||
|
- failed updates leave the previous registry entry intact
|
||||||
|
|
||||||
|
## Provider Contract Model
|
||||||
|
|
||||||
|
### Trait requirements
|
||||||
|
|
||||||
|
`SourceAdapter` should require:
|
||||||
|
|
||||||
|
- `id()`
|
||||||
|
- `capabilities()`
|
||||||
|
- `normalize(query)`
|
||||||
|
- `resolve(source)`
|
||||||
|
|
||||||
|
### Shared error type
|
||||||
|
|
||||||
|
Use a small shared adapter error enum for contract-level failures such as:
|
||||||
|
|
||||||
|
- unsupported query
|
||||||
|
- unsupported source
|
||||||
|
- resolution failure
|
||||||
|
|
||||||
|
Provider-specific rich errors can still exist behind the provider implementation if needed later, but the contract should expose a stable top-level error shape now.
|
||||||
|
|
||||||
|
## Verification Strategy
|
||||||
|
|
||||||
|
### Update execution tests
|
||||||
|
|
||||||
|
- CLI update applies updates for installed apps instead of only rendering a plan
|
||||||
|
- update failure keeps previous registry entry intact and reports failure
|
||||||
|
- update uses persisted install scope for reinstall path
|
||||||
|
|
||||||
|
### Provider contract tests
|
||||||
|
|
||||||
|
- contract tests verify GitHub and GitLab implement normalize/resolve through the trait
|
||||||
|
- adapter smoke tests continue to prove all expected adapters are registered
|
||||||
|
|
||||||
|
## Non-Goals
|
||||||
|
|
||||||
|
- interactive per-app update selection in this slice
|
||||||
|
- provider-specific live discovery for every non-GitHub adapter
|
||||||
|
- refactoring the entire source pipeline around adapters in this change
|
||||||
|
|
@ -0,0 +1,178 @@
|
||||||
|
# Update Execution And Provider Contract Implementation Plan
|
||||||
|
|
||||||
|
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
||||||
|
|
||||||
|
**Goal:** Make `aim update` actually apply updates through the existing install engine and tighten the adapter trait so providers share required normalize and resolve operations.
|
||||||
|
|
||||||
|
**Architecture:** Add an update executor in `aim-core` that rebuilds install intent from each registered app and reuses `build_add_plan(...)` plus `install_app(...)` for the actual replacement. In parallel, extend the `SourceAdapter` trait with `normalize` and `resolve`, add a shared adapter error type, and migrate GitHub and GitLab adapters to that contract.
|
||||||
|
|
||||||
|
**Tech Stack:** Rust, Cargo workspace, existing install engine in `aim-core`, serde registry state, clap CLI, fixture-backed tests in `crates/aim-core/tests` and `crates/aim-cli/tests`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 1: Add red tests for update execution
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `crates/aim-cli/tests/end_to_end_cli.rs`
|
||||||
|
- Modify: `crates/aim-core/tests/update_planning.rs`
|
||||||
|
|
||||||
|
**Step 1: Write the failing tests**
|
||||||
|
|
||||||
|
Add focused tests that assert:
|
||||||
|
- `aim update` updates installed apps rather than only rendering a summary
|
||||||
|
- update failures retain the previous app record
|
||||||
|
|
||||||
|
**Step 2: Run tests to verify they fail**
|
||||||
|
|
||||||
|
Run: `cargo test update_command_applies_updates --package aim-cli --test end_to_end_cli`
|
||||||
|
Expected: FAIL because `aim update` only reviews.
|
||||||
|
|
||||||
|
**Step 3: Keep the tests minimal**
|
||||||
|
|
||||||
|
Use fixture mode and registry tempdirs. Assert on command output and registry state.
|
||||||
|
|
||||||
|
**Step 4: Re-run to confirm clean red state**
|
||||||
|
|
||||||
|
Expected: FAIL for missing update execution, not setup issues.
|
||||||
|
|
||||||
|
**Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add crates/aim-cli/tests/end_to_end_cli.rs crates/aim-core/tests/update_planning.rs
|
||||||
|
git commit -m "test: cover executable update flow"
|
||||||
|
```
|
||||||
|
|
||||||
|
### Task 2: Implement update execution in aim-core
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `crates/aim-core/src/app/update.rs`
|
||||||
|
- Modify: `crates/aim-core/src/app/add.rs`
|
||||||
|
- Modify: `crates/aim-core/src/domain/update.rs`
|
||||||
|
|
||||||
|
**Step 1: Write minimal implementation**
|
||||||
|
|
||||||
|
Implement per-app update execution that:
|
||||||
|
- reconstructs query from stored app data
|
||||||
|
- determines install scope from persisted install metadata
|
||||||
|
- reuses `build_add_plan(...)` and `install_app(...)`
|
||||||
|
- keeps previous app records on failure
|
||||||
|
- returns structured execution results
|
||||||
|
|
||||||
|
**Step 2: Run focused tests**
|
||||||
|
|
||||||
|
Run: `cargo test update_command_applies_updates --package aim-cli --test end_to_end_cli`
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
**Step 3: Add failure reporting**
|
||||||
|
|
||||||
|
Capture updated count, failed count, and per-app messages.
|
||||||
|
|
||||||
|
**Step 4: Re-run the update-related tests**
|
||||||
|
|
||||||
|
Run: `cargo test --package aim-core --test update_planning`
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
**Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add crates/aim-core/src/app/update.rs crates/aim-core/src/app/add.rs crates/aim-core/src/domain/update.rs crates/aim-core/tests/update_planning.rs crates/aim-cli/tests/end_to_end_cli.rs
|
||||||
|
git commit -m "feat: execute updates through install engine"
|
||||||
|
```
|
||||||
|
|
||||||
|
### Task 3: Wire update execution through the 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/end_to_end_cli.rs`
|
||||||
|
|
||||||
|
**Step 1: Implement CLI execution behavior**
|
||||||
|
|
||||||
|
Keep no-arg review behavior, but make `aim update` execute the updates and render a result summary.
|
||||||
|
|
||||||
|
**Step 2: Run focused CLI tests**
|
||||||
|
|
||||||
|
Run: `cargo test update_command_applies_updates --package aim-cli --test end_to_end_cli`
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
**Step 3: Preserve review-only path**
|
||||||
|
|
||||||
|
Ensure plain `aim` with no args still shows the update review summary.
|
||||||
|
|
||||||
|
**Step 4: Re-run the CLI end-to-end test file**
|
||||||
|
|
||||||
|
Run: `cargo test --package aim-cli --test end_to_end_cli`
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
**Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add crates/aim-cli/src/cli/args.rs crates/aim-cli/src/lib.rs crates/aim-cli/src/ui/render.rs crates/aim-cli/tests/end_to_end_cli.rs
|
||||||
|
git commit -m "feat: run updates from cli update command"
|
||||||
|
```
|
||||||
|
|
||||||
|
### Task 4: Tighten the provider contract
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `crates/aim-core/src/adapters/traits.rs`
|
||||||
|
- Modify: `crates/aim-core/src/adapters/github.rs`
|
||||||
|
- Modify: `crates/aim-core/src/adapters/gitlab.rs`
|
||||||
|
- Modify: `crates/aim-core/tests/adapter_contract.rs`
|
||||||
|
|
||||||
|
**Step 1: Write the failing tests**
|
||||||
|
|
||||||
|
Add contract tests that verify GitHub and GitLab implement normalize and resolve through the shared `SourceAdapter` trait.
|
||||||
|
|
||||||
|
**Step 2: Run tests to verify they fail**
|
||||||
|
|
||||||
|
Run: `cargo test --package aim-core --test adapter_contract`
|
||||||
|
Expected: FAIL because the trait does not yet require those methods.
|
||||||
|
|
||||||
|
**Step 3: Write minimal implementation**
|
||||||
|
|
||||||
|
Add:
|
||||||
|
- shared adapter error enum
|
||||||
|
- `normalize` and `resolve` trait methods
|
||||||
|
- GitHub and GitLab implementations under that shared contract
|
||||||
|
|
||||||
|
**Step 4: Run adapter contract tests**
|
||||||
|
|
||||||
|
Run: `cargo test --package aim-core --test adapter_contract`
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
**Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add crates/aim-core/src/adapters/traits.rs crates/aim-core/src/adapters/github.rs crates/aim-core/src/adapters/gitlab.rs crates/aim-core/tests/adapter_contract.rs
|
||||||
|
git commit -m "refactor: tighten provider adapter contract"
|
||||||
|
```
|
||||||
|
|
||||||
|
### Task 5: Full verification
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: none expected
|
||||||
|
|
||||||
|
**Step 1: Run format check**
|
||||||
|
|
||||||
|
Run: `cargo fmt --check`
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
**Step 2: Run clippy**
|
||||||
|
|
||||||
|
Run: `cargo clippy --workspace --all-targets --all-features -- -D warnings`
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
**Step 3: Run tests**
|
||||||
|
|
||||||
|
Run: `cargo test --workspace`
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
**Step 4: Fix only update-execution and provider-contract regressions if needed and re-run verification**
|
||||||
|
|
||||||
|
**Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add -A
|
||||||
|
git commit -m "test: verify update execution and provider contract"
|
||||||
|
```
|
||||||
Loading…
Add table
Add a link
Reference in a new issue