runtime: probe user env before lifecycle hooks#55
Conversation
… PATH Engine.Up ran every lifecycle phase before probing the user's shell env, so ws.probedEnv was empty during postCreateCommand. Hooks that relied on PATH entries published via .bashrc / /etc/profile.d snippets (nvm-managed node, asdf, devcontainers-extra/pnpm) failed with "command not found", diverging from @devcontainers/cli, which awaits the probe before each lifecycle exec. Probe before runAllLifecycle so hooks see rc-derived env, then re-probe after so rc edits made by the hooks themselves reach subsequent Exec calls. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR reorders the user-environment probe in ChangesShell-environment probe and lifecycle hooks
Sequence DiagramsequenceDiagram
participant EngineUp
participant probeUserEnv
participant LifecycleHooks
participant Exec
EngineUp->>probeUserEnv: Execute before lifecycle
probeUserEnv->>EngineUp: ws.probedEnv updated with shell PATH
EngineUp->>LifecycleHooks: Run postCreateCommand (sees probed env)
LifecycleHooks->>EngineUp: Lifecycle completes (may modify env)
EngineUp->>probeUserEnv: Re-execute after lifecycle
probeUserEnv->>EngineUp: ws.probedEnv refreshed from rc changes
EngineUp->>Exec: Subsequent Exec calls see updated env
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
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 226-245: The current code only updates ws.probedEnv before
runAllLifecycle and after it finishes, leaving lifecycle steps (e.g.,
onCreateCommand → postCreateCommand) using a stale env; modify the lifecycle
handling so the environment is refreshed between individual lifecycle execs:
either call e.probeUserEnv(ctx, ws, ws.Config.UserEnvProbe) and update
ws.probedEnv before each lifecycle exec inside runAllLifecycle (or have
runAllLifecycle invoke a helper that re-probes between phases), ensuring errors
remain non-fatal (keep the same err==nil check) and preserving existing
semantics of e.runAllLifecycle, e.probeUserEnv, and ws.probedEnv.
🪄 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: fd7272be-0d07-49c4-9735-16374fd07e8d
📒 Files selected for processing (2)
test/integration/userenvprobe_test.goup.go
Summary
userEnvProbeonly after running every lifecycle phase, sows.probedEnvwas empty duringpostCreateCommand(and earlier phases). Hooks that relied on PATH entries published via.bashrc//etc/profile.dsnippets — nvm-managed node, asdf, the officialdevcontainers-extra/pnpmfeature — failed withcommand not found. This diverged from@devcontainers/cli, which awaits the probe promise before every lifecycle exec.runAllLifecycleso hooks see the rc-derived env, then re-probe after so any rc edits made by the hooks themselves are reflected in subsequentExeccalls. Probe failures stay non-fatal.lifecycle.go/exec.go:Execalready mergesws.probedEnv, and lifecycle hooks route throughExec— updatingws.probedEnvat the right moment is sufficient.Test plan
go test ./...(unit) passesgo test -tags=integration -run TestUserEnvProbe ./test/integration/passesTestUserEnvProbe_LifecycleSeesBashrcPathregression test: builds an image that drops a binary in/opt/mytool/binand a/etc/profile.d/mytool.shthat prepends it to PATH;postCreateCommandrunscommand -v mytool && mytool > /tmp/lifecycle-out. Fails on the old code (Up errors because the hook can't find the binary), passes after the fix.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests