engine: add UpOptions.ExtraMounts and ExtraContainerEnv

[bilby91]

Summary

Adds two opt-in knobs on UpOptions:

• ExtraMounts []runtime.MountSpec — appended after cfg.WorkspaceMount and cfg.Mounts on image source. On compose source, bind-type entries are converted into compose overrides; non-bind entries are silently dropped (matching how devcontainer.json mounts are filtered there today).
• ExtraContainerEnv map[string]string — merged into cfg.ContainerEnv with extras winning on key collision. Baked into RunSpec.Env so lifecycle scripts and feature installs inherit it.

Both fields are opt-in. Default behavior is unchanged.

Motivation

DAP's workspace runtime layers a /dap bind-mount plus PATH / GIT_EXEC_PATH / proxy env on every up, currently through the npm CLI's --mount and --remote-env flags. The runtime is moving off the CLI to this library (crunchloop/dap#2455). Without these fields, the only options are mutating the resolved config from outside (no clean injection point — Engine.Up re-resolves internally) or writing a transient devcontainer.json to disk before each call. Both are workarounds that fight the package design; the right place for the knob is on UpOptions.

Test plan

• New unit test TestUp_ExtraMountsAndContainerEnv in engine_test.go exercises both fields plus the override-precedence rule.
• go test ./... — full suite passes.
• go test -tags=integration ./test/integration/... — pre-existing integration suite, no behavior change expected here (extras default to nil).

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Extended configuration options to allow specifying additional container mounts and environment variables, enabling more flexible and customizable container setup in both compose-based and direct container creation scenarios.

• Tests

  • Added comprehensive test coverage to validate correct handling of additional mounts and environment variables across various configuration scenarios, including proper conflict resolution when values overlap.

[bilby91]

@CodeRabbit review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR extends Engine.Up with caller-provided mount and environment variable extensions. UpOptions gains ExtraMounts and ExtraContainerEnv fields; buildRunSpec and a new mergeEnv helper merge these into the final runtime spec; both compose and non-compose container creation paths apply the extras; and a comprehensive test validates the behavior.

Changes

Extra Mounts and Container Environment Support

Layer / File(s)	Summary
UpOptions API Contract up.go	UpOptions extended with ExtraMounts []runtime.MountSpec and ExtraContainerEnv map[string]string fields.
Core Merging and Build Logic up.go	buildRunSpec refactored to accept extraMounts and extraEnv, merges extras into final RunSpec; new mergeEnv helper produces combined env map where extras override base, or returns nil when both empty.
Non-Compose Path Integration up.go	createFresh passes opts.ExtraMounts and opts.ExtraContainerEnv to buildRunSpec.
Compose Path Integration up.go	createFreshCompose filters opts.ExtraMounts into compose ExtraBindMounts (bind-type only, non-empty source/target) and merges opts.ExtraContainerEnv into compose ExtraEnvironment via mergeEnv.
Test Coverage engine_test.go	New TestUp_ExtraMountsAndContainerEnv validates that config mounts and extra mounts coexist in RunSpec, and that ExtraContainerEnv overrides conflicting containerEnv keys.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hops of joy for mounts so new,
Extra envs to help us through!
Mounts and merges, blend with care,
Compose paths and non-compose share! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% 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 clearly and concisely summarizes the main change: adding two new fields (ExtraMounts and ExtraContainerEnv) to the UpOptions type.
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 extra-mounts-and-container-env

---

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

[coderabbitai]

🧹 Nitpick comments (1)

engine_test.go (1)

337-354: ⚡ Quick win

Assert mount ordering explicitly, not just presence.

The test comment says extras are appended after config mounts, but the assertions currently only check existence. Please also assert cfg mount index < extra mount index so this contract is protected.

Proposed test tightening

 	var sawCfg, sawExtra bool
+	cfgIdx, extraIdx := -1, -1
-	for _, m := range rt.createdSpec.Mounts {
+	for i, m := range rt.createdSpec.Mounts {
 		if m.Source == "/cfg-src" && m.Target == "/cfg-tgt" {
 			sawCfg = true
+			if cfgIdx == -1 {
+				cfgIdx = i
+			}
 		}
 		if m.Source == "/host/dap" && m.Target == "/dap" {
 			sawExtra = true
+			if extraIdx == -1 {
+				extraIdx = i
+			}
 		}
 	}
@@
 	if !sawExtra {
 		t.Errorf("ExtraMounts entry missing from RunSpec: %+v", rt.createdSpec.Mounts)
 	}
+	if sawCfg && sawExtra && cfgIdx > extraIdx {
+		t.Errorf("expected cfg mounts before extra mounts, got cfgIdx=%d extraIdx=%d", cfgIdx, extraIdx)
+	}

🤖 Prompt for 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.

In `@engine_test.go` around lines 337 - 354, The test currently only checks
presence of the cfg and extra mounts in rt.createdSpec.Mounts; modify it to also
assert the cfg mount appears before the extra mount by recording their indices
while iterating rt.createdSpec.Mounts (use the existing identification by
Source/Target: cfg is "/cfg-src"->"/cfg-tgt" and extra is "/host/dap"->"/dap"),
then add an assertion that cfgIndex < extraIndex (use t.Errorf/t.Fatalf with a
clear message and include rt.createdSpec.Mounts for diagnostic output) to
enforce the ordering contract.

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

Nitpick comments:
In `@engine_test.go`:
- Around line 337-354: The test currently only checks presence of the cfg and
extra mounts in rt.createdSpec.Mounts; modify it to also assert the cfg mount
appears before the extra mount by recording their indices while iterating
rt.createdSpec.Mounts (use the existing identification by Source/Target: cfg is
"/cfg-src"->"/cfg-tgt" and extra is "/host/dap"->"/dap"), then add an assertion
that cfgIndex < extraIndex (use t.Errorf/t.Fatalf with a clear message and
include rt.createdSpec.Mounts for diagnostic output) to enforce the ordering
contract.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 445409be-3133-4734-8cab-b81fad7598e8

📥 Commits

Reviewing files that changed from the base of the PR and between 9db634f and 356b002.

📒 Files selected for processing (2)

• engine_test.go
• up.go