Repository navigation
feat(storage): add cleanup review and resilient recreation recovery - #84
Conversation
Refs #83. Preserve inspected entrypoints, retain and verify original image recovery, and expose recovery actions for missing containers.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65fbed5912
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please review exact head 3ae5ad9, including the performance/consistency audit fixes and stopped-state recovery follow-up. Both earlier findings are fixed with regression evidence in their resolved threads. Focus on concrete correctness regressions and recovery/cleanup boundaries; do not reuse the earlier-head review. |
There was a problem hiding this comment.
💡 Codex Review
contained-app/Sources/ContainedApp/App/AppModel+ImageUpdates.swift
Lines 271 to 272 in 3ae5ad9
When an Update Container operation needs a pull, this moves the mutable tag before recreateContainer reaches prepareContainerRecovery. If the runtime drops the old tag mapping or no longer exposes the untagged digest through image inventory, the subsequent preparation cannot create the recovery alias, so a failed replacement cannot restore the original image despite the new recovery guarantee. Prepare and persist recovery from snapshot before calling pullImageUpdate, preferably as one Core orchestration operation.
AGENTS.md reference: AGENTS.md:L67-L67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please review exact head 5213004. The latest recovery-ordering feedback is fixed: Update now uses the shared recreation flow, preserves the original image and successfully saves the original recipe before pulling, rejects pending/paused recovery before pulling, and rechecks persistence before teardown. Failed pre-teardown work clears only the marker created by that invocation; failed cleanup saves retain the recipe. The redundant failure-side stop was removed so old recovery status cannot stop a container after failed preflight. The independent final check also fixed child-manifest pinning: Apple digest aliasing now requires top-level identity equality and never substitutes a multi-platform parent index for a child digest. Focused regressions failed before the fixes and pass afterward. All 194 app tests, 149 Core tests, swift build, repository checks and diff checks pass. No UI redesign, dependencies, live container operations or deployment. Please evaluate concrete correctness failures on this exact head; do not reuse older-head results. |
|
@codex security review Please perform a separate fresh security review of exact head 5213004. The two image-recovery fixes now preserve/save originals before update pulls, reject failed/pending persistence before pull and teardown, retain recovery on failed cleanup saves, and preserve exact child-manifest pins instead of substituting parent indices. No live operations, dependencies, schema migration, deployment or UI redesign. Previous security completion covers an older commit and is not clearance for this head. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Summary
Performance and consistency audit fixes
Linked issue
Refs #83: addresses entrypoint duplication and failed-rollback/missing-container recovery. The audit fixes are direct follow-ups to this PR and do not introduce a separate feature or design proposal.
The corrected incident evidence identifies a corrupt local ext4 cache snapshot, not a broken preview image or its VOLUME declaration. Regenerating the unchanged image cache fixed Coast. This PR does not automatically delete or repair runtime cache files; the corruption origin remains unknown, so #83 is not automatically closed.
Codex review follow-up
3ae5ad9d: Core receives the saved running/stopped intent, uses nativecreateinstead ofrunfor stopped originals, and verifies image identity and exact state before completing recovery. Automatic rollback uses the same non-starting path. The recovery command preview also uses the correct lifecycle command.UI.Panel.Scaffoldfor the recovery sheet instead of app-local zero spacing. Existing sizing, scrolling, materials and actions are preserved.Change type
Validation
./Scripts/package.sh app debugswift test: 190 app tests passedswift test --package-path Packages/ContainedCore: 148 tests passed; opt-in live-storage tests skipped./Scripts/package.sh smoke Contained.app./Scripts/check.sh repogit diff --checkThe local checks above cover review-fix commit
3ae5ad9d. GitHub CI and both Codex reviews must run again on that head; earlier green results are not evidence for these new changes.Release notes and docs
Updated Changes/Current.md, Documentation/App/System-Settings.md, Documentation/Features/Containers.md and the ContainedCore README. No dependency, schema-model, signing, appcast, build-number or release-policy changes.
Review focus
Durable recovery state and destructive-cleanup boundaries need human review. Regression coverage includes repeated process translation, rollback startup verification, image alias identity/collision handling, failed backup saves preventing teardown, retained original snapshots, disk-backed recovery reopen, corrupt saved payloads, partial/cold-start runtime failures, authoritative orphan cleanup, original-image metadata behind a partial replacement, and observation/detail-state behavior.
Multiple exact cleanup previews remain fail-closed: an earlier selected action that changes inventory can invalidate later previews, requiring a fresh review.