runtime: return Workspace alongside lifecycle error from Engine.Up

[bilby91]

Stacked on #55. Diff against #55 is just up.go (the lifecycle-error path) and a new unit test. Merge #55 first; this PR's base will retarget to main automatically.

Summary

• Engine.Up dropped the just-created *Workspace whenever runAllLifecycle returned an error, returning (nil, err) and forcing every caller to treat a postCreateCommand bug as a fatal "workspace failed to start" condition — even though the container is created and still running. This diverges from @devcontainers/cli, which exits 1 on lifecycle failure but leaves the container intact and reattachable, the foundation of VS Code / Codespaces' "show a warning, keep the user in the container" UX.
• On lifecycle failure, return (ws, err) instead of (nil, err). Callers pick the policy: tear down, downgrade to warning, or reattach for debugging. *LifecycleError is already the natural discriminator — no new sentinel error needed.
• Non-lifecycle failures (image build, feature install, container create) still return (nil, err) unchanged. Strict callers keep current behavior with the usual if err != nil { return nil, err }.

Test plan

• New TestUp_LifecycleFailureReturnsWorkspace (unit): scripted runtime injects exit=17 for the postCreate hook; asserts (a) *LifecycleError surfaces, (b) returned *Workspace is non-nil with a real Container.ID, (c) an Exec against the returned workspace succeeds — proving the container wasn't torn down.
• Existing TestUp_LifecycleErrorsSurfaceAsLifecycleError still passes (error type/shape unchanged).
• Full go test ./... passes.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • When a post-create lifecycle step fails, the workspace is preserved and returned with the error—keeping the container reachable and executable. The system now always re-checks the environment after lifecycle runs; non-lifecycle failures retain prior behavior.

• Tests

  • Added a lifecycle-focused test verifying the workspace is returned and the container remains executable when a lifecycle step fails.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4962dedb-d288-4c8c-8862-460c38113e4f

📥 Commits

Reviewing files that changed from the base of the PR and between 03c07a7 and 61e4c60.

📒 Files selected for processing (1)

• lifecycle_test.go

---

📝 Walkthrough

Walkthrough

Engine.Up now captures lifecycle errors and returns the created workspace alongside them when the error is a lifecycle error, and it always re-probes the user environment after lifecycle execution. A new test verifies the returned workspace and container remain reachable after a lifecycle failure.

Changes

Preserve workspace on lifecycle failure

Layer / File(s)	Summary
Engine.Up lifecycle error handling up.go	Engine.Up stores runAllLifecycle errors in lifecycleErr, always re-probes the workspace user environment after lifecycle, and returns the workspace with lifecycle errors (while non-lifecycle errors still return nil workspace).
Lifecycle failure test lifecycle_test.go	TestUp_LifecycleFailureReturnsWorkspace simulates a failing postCreateCommand, asserts the error unwraps to *LifecycleError, and verifies the returned workspace and its container are non-nil and executable via Exec.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• crunchloop/devcontainer#55: Both PRs modify Engine.Up's lifecycle and probe control flow—runtime: probe user env before lifecycle hooks #55 changes when probeUserEnv runs relative to postCreateCommand, while this PR preserves the workspace and probed environment when lifecycle fails.

Poem

🐰 I patched the hooks that caused a fright,
The workspace lingered through the night,
The probe still ran, the container played,
Though post-create had sadly swayed,
A rabbit nods — the shell's alright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% 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 accurately and specifically describes the main change: Engine.Up now returns a Workspace alongside lifecycle errors instead of nil, enabling callers to handle partial failures gracefully.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch runtime/return-workspace-on-lifecycle-failure

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@up.go`:
- Around line 236-258: The current code returns (ws, lifecycleErr) for any error
from runAllLifecycle; change this to only return the workspace when the error is
a lifecycle-classified error: after calling runAllLifecycle and probeUserEnv,
test lifecycleErr with a type assertion or errors.As to see if it is a
*LifecycleError (or whatever concrete type you use for lifecycle failures) and
only then return (ws, lifecycleErr); for any other non-nil lifecycleErr return
(nil, lifecycleErr) (i.e., keep the usual if err != nil { return nil, err }
semantics) so non-lifecycle failures do not leak a running workspace.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ce0701a9-37f5-470e-9cad-feb206c81212

📥 Commits

Reviewing files that changed from the base of the PR and between 75a8c84 and 4b34fd6.

📒 Files selected for processing (2)

• lifecycle_test.go
• up.go