Skip to content

Persist lifecycle state before shell and skill post-processing#29

Merged
ametel01 merged 1 commit into
mainfrom
fix/23-persist-state-before-post-processing
Mar 14, 2026
Merged

Persist lifecycle state before shell and skill post-processing#29
ametel01 merged 1 commit into
mainfrom
fix/23-persist-state-before-post-processing

Conversation

@ametel01
Copy link
Copy Markdown
Owner

@ametel01 ametel01 commented Mar 14, 2026

Summary

  • Saves state.json immediately after install/update/uninstall plan execution, before shell-hook prompting and skill generation, preventing receipt loss when post-processing steps fail
  • Extracts postInstallWorkflow to keep runInstall within cyclomatic complexity limits
  • Adds regression tests covering shell-hook read failures, skill-generation output-write failures, and uninstall-removal persistence

Closes #23

Test plan

  • TestEarlySavePreservesReceiptsOnShellHookFailure — broken stdin during shell-hook prompting does not lose install receipts
  • TestEarlySavePreservesReceiptsOnSkillOutputWriteFailure — failing stdout during skill generation does not lose update receipts
  • TestEarlySavePreservesRemovalsOnSkillOutputWriteFailure — failing stdout during skill generation does not lose uninstall removals
  • All existing tests continue to pass
  • golangci-lint run reports 0 issues

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fixed potential receipt loss during install, update, and uninstall operations by saving state immediately after execution, before post-processing steps. This ensures data integrity even if subsequent steps fail.
  • Tests

    • Added regression test coverage to verify receipt preservation during post-processing failures.

Save state.json immediately after install, update, and uninstall plan
execution completes, before shell-hook prompting and skill generation.
This prevents receipt loss when post-processing steps fail after the
machine state has already changed.

Extracts postInstallWorkflow to keep runInstall within cyclomatic
complexity limits.

Closes #23

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6cd2cd84-188b-4456-b4e4-bd2bd77a16dd

📥 Commits

Reviewing files that changed from the base of the PR and between aee0983 and 154af63.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • cmd/atb/runtime.go
  • cmd/atb/runtime_test.go

📝 Walkthrough

Walkthrough

The changes move state persistence earlier in the install/update/uninstall workflows by saving state immediately after plan execution, before shell-hook prompting and skill generation. This prevents receipt loss when post-processing steps fail.

Changes

Cohort / File(s) Summary
Lifecycle State Persistence
cmd/atb/runtime.go, CHANGELOG.md
Refactored install workflow to save state immediately after plan execution via postInstallWorkflow, before shell integration and skill generation. Update and uninstall flows similarly save state immediately after plan execution. Finalization moved from inline in runInstall to postInstallWorkflow.
Regression Tests
cmd/atb/runtime_test.go
Added three new test cases verifying state persistence on post-processing failures: shell-hook stdin failure, skill output write failure during install, and skill output write failure during uninstall. Introduced failingWriter helper and assertStateOnDiskContains utility function.

Sequence Diagram

sequenceDiagram
    participant Client as Install/Update/Uninstall Handler
    participant Plan as Execute Plan
    participant StateSave as Save State
    participant PostProc as Post-Processing<br/>(Shell Hooks/Skills)
    participant Summary as Render Summary

    Client->>Plan: Execute lifecycle plan
    Plan-->>Client: Plan completed
    
    Client->>StateSave: Save state immediately
    activate StateSave
    Note over StateSave: Receipts persisted durably<br/>before post-processing
    StateSave-->>Client: State saved
    deactivate StateSave
    
    Client->>PostProc: Apply shell integration<br/>& select targets
    alt Post-processing succeeds
        PostProc-->>Client: Complete
        Client->>Summary: Render install summary
    else Post-processing fails
        PostProc-->>Client: Error
        Note over StateSave: Receipts already saved<br/>Loss prevented ✓
        Client->>Summary: Handle error gracefully
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related issues

  • Persist lifecycle state before shell and skill post-processing #23: Persist lifecycle state before shell and skill post-processing — The changes directly address all acceptance criteria by saving state immediately after plan execution and adding regression tests covering shell-hook failures, skill output write failures, and removal persistence.

Possibly related PRs

Poem

🐰 State saved before the hooks take flight,
Receipts preserved through post-processing night,
No more lost tools from a failure's plight,
Installation flows now get it right! 💾✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately summarizes the main change: persisting lifecycle state before post-processing steps, which is the core objective addressed in the PR.
Linked Issues check ✅ Passed All acceptance criteria from issue #23 are met: state saved immediately after lifecycle execution before shell-hook and skill generation, skill generation failures preserve receipts via new tests, and regression tests cover shell-hook and output-write failures.
Out of Scope Changes check ✅ Passed All changes are directly aligned with issue #23: runtime.go refactors install/update/uninstall flows for early state persistence, runtime_test.go adds regression tests for failure scenarios, and CHANGELOG.md documents the fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/23-persist-state-before-post-processing
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ametel01 ametel01 merged commit 11ad82b into main Mar 14, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Persist lifecycle state before shell and skill post-processing

1 participant