Skip to content

feat(storage): add cleanup review and resilient recreation recovery - #84

Merged
tdeverx merged 4 commits into
nightlyfrom
codex/recreate-recovery-and-cleanup
Oct 7, 2026
Merged

tdeverx merged 4 commits into
nightlyfrom
codex/recreate-recovery-and-cleanup

Conversation

@tdeverx

@tdeverx tdeverx commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Add Free Up Space… to System storage analysis and low-space alerts. Only compaction is preselected; cache/image/volume/network deletion requires explicit opt-in. The review also exposes existing automatic-compaction controls, never automatic deletion.
  • Preserve the resolved Apple Container executable when editing/recreating so image entrypoint arguments do not accumulate. Keep intentional repeated arguments.
  • Preserve the inspected original image through a verified local recovery alias before a replacement pull can move its tag; verify rollback image identity and startup.
  • Require a successfully saved original recipe before teardown. Retain that recipe through partial-runtime refreshes and disk-backed reopen.
  • Expose Review Recovery…, Restore Original, and confirmation-gated Keep Existing Container for interrupted/failed recreations. Recovery never overwrites an existing runtime object.
  • Block destructive cleanup during lifecycle operations or while recreation/database recovery is pending, protecting the saved recovery image, volumes and networks.
  • Address all eleven findings from the follow-up performance and consistency audit, preserving the existing UI design and without adding dependencies.

Performance and consistency audit fixes

  1. Prevent repeated recreation from overwriting an unresolved original recovery.
  2. Pause persistence on corrupt personalization or health-check records instead of crashing; stage decoding before changing loaded state.
  3. Require fresh, successful inventories before orphan cleanup, preserving recovery/migration identities, original-image metadata and unqueried-runtime state.
  4. Track successful runtime inventory scopes so outages retain last-known resources, metrics and saved image-update status instead of appearing as deletions. Confirmed absence still removes stale records.
  5. Consolidate duplicate container records without mismatching the saved original recipe and recovery state.
  6. Restart and verify a previously running original when teardown cannot delete it; retain recovery if restart fails.
  7. Resolve expanded container details against current scoped inventory, with a disappearance fallback.
  8. Preserve image-inventory observation on warm grouping-cache hits.
  9. Share inventory and necessary host-allocation measurement across combined cleanup previews; retain fresh execution-time validation.
  10. Cache narrowly filtered recreation-recovery projections with dedicated invalidation instead of reloading them for unrelated database mutations.
  11. Measure image-group geometry only for pending navigation, not every scrolling row.

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

  • Fixed stopped-original recovery in 3ae5ad9d: Core receives the saved running/stopped intent, uses native create instead of run for 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.
  • Reused UI.Panel.Scaffold for the recovery sheet instead of app-local zero spacing. Existing sizing, scrolling, materials and actions are preserved.
  • Checked the create-only contract against Apple Container 1.4.1 source and Docker's official command reference. No live commands were used.

Change type

  • App/UI behavior
  • Core/runtime logic
  • User-facing documentation and release notes
  • Conventional-commit PR title

Validation

  • SwiftPM debug build, including ./Scripts/package.sh app debug
  • swift test: 190 app tests passed
  • swift test --package-path Packages/ContainedCore: 148 tests passed; opt-in live-storage tests skipped
  • ./Scripts/package.sh smoke Contained.app
  • ./Scripts/check.sh repo
  • git diff --check
  • Native Xcode build/tests: blocked because Xcode requires approval for the existing SwiftTermBuildInfoPlugin. No plugin-validation bypass used.
  • Live UI/runtime smoke test and profiling: intentionally not performed; no live containers, mounts, volumes or caches were changed. Performance improvements are source/test-backed, not measured FPS or latency claims.

The 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.

Refs #83. Preserve inspected entrypoints, retain and verify original image recovery, and expose recovery actions for missing containers.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T07:58:28.147067Z 5213004 Manual request
🔒 Security Review ✅ Completed 2026-10-07T08:20:35.535395Z 5213004 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread Sources/ContainedApp/App/AppModel+ContainerRecovery.swift Outdated
Comment thread Sources/ContainedApp/Features/Containers/Details/ContainerRecoveryView.swift Outdated
@tdeverx

tdeverx commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

@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.

@tdeverx

tdeverx commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

@codex security review

Please review exact head 3ae5ad9, including runtime create-only recovery, saved-state handling and cleanup boundaries. This is a separate fresh security review request; the previous completed review covered an earlier commit.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

if updateState.needsPull,
!(await pullImageUpdate(snapshot.image, runtimeKind: snapshot.runtimeKind)) {

P1 Badge Preserve the rollback image before pulling the update

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".

@tdeverx

tdeverx commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

@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.

@tdeverx

tdeverx commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

@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.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 52130045f6

ℹ️ 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".

@tdeverx

tdeverx commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 52130045f6

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@tdeverx
tdeverx merged commit 00b298b into nightly Oct 7, 2026
4 checks passed
@tdeverx
tdeverx deleted the codex/recreate-recovery-and-cleanup branch October 7, 2026 08:26
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.

1 participant