Skip to content

fix(compose): regenerate overrides on recreation - #1407

Merged
skevetter merged 2 commits into
mainfrom
codex/issue-1404-compose-recreate
Oct 7, 2026
Merged

skevetter merged 2 commits into
mainfrom
codex/issue-1404-compose-recreate

Conversation

@skevetter

@skevetter skevetter commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Explicit Compose recreation restored the previous container's generated overrides, allowing removed or changed lifecycle hooks to survive in the replacement container's metadata. Bypass that cache when Recreate is set so overrides are regenerated from the current configuration before the old container is stopped or deleted.

Ordinary starts retain persisted-override reuse, and the existing secret-migration refresh path remains intact. Compose project identity and teardown ordering are preserved. Recreation intentionally performs fresh build/metadata generation while retaining Docker layer caching. The change introduces no new cache state or migration; existing generated-file cleanup remains outside this fix.

Regression coverage:

  • Unit tests for --recreate and --reset --recreate prove generation errors occur before stop/delete. Both fail on the original implementation.
  • Docker Compose integration tests verify changed and removed hooks, retained user/image hooks, actual lifecycle execution, replacement container identity, and stable Compose project identity. Both run in the existing up-docker-compose CI matrix.

Validation on final head d2e421af78d976a6b3ba668b369f373036d382e2:

  • Local devcontainer race tests, focused Docker Compose regression, CLI lint, changed-file pre-commit, Linux E2E compilation, and local CodeRabbit review passed.
  • Full Ubuntu Compose CI: 53 passed, 0 failed. Both race-test jobs, Lint, Pre-commit, all CLI builds, remaining integration jobs, and CI Success passed. There are no incomplete or failed checks; expected desktop skips and informational neutral checks are not applicable.
  • A Windows SSH job initially failed before its suite in the newly merged fix(ci): enforce Windows Podman bootstrap deadline #1402 Podman watchdog self-test. The failed job and aggregate passed on retry without code changes.
  • Greptile: 5/5, no new actionable findings. Its earlier duplicate-cleanup finding was fixed and resolved.
  • CodeRabbit full remote review: all six files, no actionable findings.
  • GitHub verifies both signed commits as valid. PR remains draft.

Full local Compose sweep: 47 passed, 6 failed, including passes for both new regression cases. Existing failures were a Debian repository connection timeout during a multistage build, three first-start mount-count assertions observing an additional secrets tmpfs, and two UID expectations on macOS while UID mapping is Linux-gated. These do not exercise the changed persisted-override recreation branch. The origin of the extra local secret mount has not been established; the full Linux CI suite passed.

Fixes #1404. Based on the reported diagnosis and cache-invalidation approach in the contributor's proposed fix.

@netlify

netlify Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit d2e421a
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6ac5ce3bdba5280008dcc50c

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8f241193-822c-4a3b-840e-e068de0120ed
📥 Commits

Reviewing files that changed from the base of the PR and between fed51e6 and d2e421a.

📒 Files selected for processing (6)
  • e2e/tests/up-docker-compose/recreate_lifecycle.go
  • e2e/tests/up-docker-compose/testdata/docker-compose-recreate-lifecycle/.devcontainer.json
  • e2e/tests/up-docker-compose/testdata/docker-compose-recreate-lifecycle/Dockerfile
  • e2e/tests/up-docker-compose/testdata/docker-compose-recreate-lifecycle/docker-compose.yaml
  • pkg/devcontainer/compose.go
  • pkg/devcontainer/compose_recreate_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

Compose recreation now skips persisted override restoration. Unit and end-to-end tests cover override handling and lifecycle hooks after recreation.

Changes

Compose recreation

Layer / File(s) Summary
Override refresh during recreation
pkg/devcontainer/compose.go, pkg/devcontainer/compose_recreate_test.go
Recreation bypasses persisted Compose override restoration. The regression test covers recreate and reset-recreate when image inspection fails, and verifies that teardown does not stop or delete the container.
Recreated lifecycle metadata
e2e/tests/up-docker-compose/testdata/docker-compose-recreate-lifecycle/*, e2e/tests/up-docker-compose/recreate_lifecycle.go
The fixture defines user and image lifecycle hooks. End-to-end tests check updated metadata, container identity, Compose project label, and hook log counts after recreation.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to d2e42

The recreation change has no identified issue requiring a fix before merge; normal checks remain appropriate.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d2e42

Recreation now regenerates settings from current configuration before stopping the existing container. No introduced security issue was established. Generated-file cleanup and overlapping recreation attempts remain partially verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The direct changed authority is the selected workspace service's generated runtime configuration. Configuration or image-metadata control can affect container privileges, mounts, environment, and lifecycle behavior through existing paths. The comparison shows no new caller authority or isolation-boundary expansion, but supplied topology does not establish an environment-wide exposure ceiling.

Trust Boundaries and Controls

  • observed — Secret migration still detects missing secure mounts, rejects unsupported providers, and forces recreation with override refresh. Fresh override generation continues to require supported tmpfs mounts rather than silently falling back to persistent secret storage.
  • observed — Generated overrides use owner-only temporary-file creation. Persisted restoration selects paths recorded in container labels and checks filename prefixes and regular-file existence; it does not automatically select every accumulated override in the directory. These controls predate the PR.

Resilience and Maintainability Implications

  • inferred — Pre-teardown generation preserves the existing container on early failure, but does not make recreation atomic or prove concurrent idempotency. The inspected runner and internal command path do not establish same-workspace serialization; this remains a coverage gap, not a demonstrated PR-introduced race.

Hardening Proposals

  • proposed — Consider explicit ownership and reconciliation for unreferenced generated overrides, removing abandoned files only after confirming that no surviving container or in-flight recreation still references them. This would limit retained configuration without weakening recovery.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (3 skipped: 3 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed #1404 requires explicit Compose recreation to regenerate overrides from the current configuration before replacing the container, remove stale user hooks, and retain inherited hooks and project identi…
Out of Scope Changes check ✅ Passed The reported source change updates Compose override refresh behavior for recreation. The added unit tests, E2E cases, and fixtures verify stale-hook removal, hook retention, and recreation identity re…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: regenerating Compose overrides during recreation.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@netlify

netlify Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit d2e421a
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6ac5ce3b8eb66300083e115e

@skevetter
skevetter force-pushed the codex/issue-1404-compose-recreate branch from 96c98f3 to 3d48770 Compare October 7, 2026 03:58
@github-actions github-actions Bot added the size/l label Oct 7, 2026
@skevetter

Copy link
Copy Markdown
Contributor Author

@greptileai review

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds lifecycle test and fixes override regeneration on container recreation.

The PR appears safe to merge; no new actionable issues were found.

What we checked:

  • Workspace cleanup still runs: setupWorkspace registers both workspace deletion and temporary-directory cleanup, so the removed call was redundant.

Summary

Explicit Compose recreation regenerates overrides from the current configuration. Ordinary starts still reuse saved overrides.

  • Compose recreation now uses hooks from the current configuration.

Reviews (2) · Last reviewed commit: "test(compose): avoid duplicate workspace..." · Reviewed by Greptile

Comment thread e2e/tests/up-docker-compose/recreate_lifecycle.go Outdated
@skevetter

Copy link
Copy Markdown
Contributor Author

@greptileai review

@skevetter

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@skevetter
skevetter marked this pull request as ready for review October 7, 2026 05:59
@mergify

mergify Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@skevetter
skevetter merged commit fb9a0e5 into main Oct 7, 2026
164 of 166 checks passed
@skevetter
skevetter deleted the codex/issue-1404-compose-recreate branch October 7, 2026 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Compose --recreate reuses stale overrides and runs removed lifecycle hooks

1 participant