iframe-proxy

bilby91 · 2026-05-10T04:01:20Z

Summary

New `HostExecutor` interface (`host_executor.go`) that callers supply via `EngineOptions.HostExecutor` to enable host-side spec hooks.
`initializeCommand` now dispatches through it (Single + Parallel forms; Parallel runs concurrently and reports the first non-zero exit, mirroring `execParallel`).
Sentinel `ErrHostExecutorNotConfigured` returned (wrapped in `*LifecycleError`) when a host hook is configured but no executor is set, so callers can `errors.Is` and surface a useful message.

Closes #11. Unblocks #23 (`secretsCommand`) — same interface; the follow-up just captures stdout and merges into `ContainerEnv`.

API surface

```go
type HostExecutor interface {
ExecHost(ctx context.Context, cmd HostCommand) (HostExecResult, error)
}

type HostCommand struct {
Shell string
Exec []string
Env map[string]string
WorkingDir string
}

type HostExecResult struct {
ExitCode int
Stdout, Stderr string
}
```

Mirrors `runtime.ExecOptions` / `runtime.ExecResult` so consumers building both sides reuse the same mental model. Engine sets `WorkingDir = LocalWorkspaceFolder` for spec hooks.

Why no default impl?

Host execution is the most security-sensitive surface in the spec. `devcontainer.json` is user-supplied; running `exec.Command` from inside a library against it would bake policy (env passthrough, timeouts, sandboxing) into the wrong place. Caller-supplied keeps the library spec-faithful and lets each embedder make the right call:

DAP: opt out entirely (no executor) or supply a sandboxed one.
A CLI front-end: supply a passthrough that inherits the host env.
A CI agent: supply one that strips secrets.

Test plan

`go test ./...` — all green
`go vet ./...`, `gofmt` clean
New tests:
- `TestInitializeCommand_RequiresHostExecutor`: verifies the typed-error path via `errors.Is`.
- `TestInitializeCommand_RoutesToHostExecutorSingle`: verifies `Shell` and `WorkingDir` propagate correctly.
- `TestInitializeCommand_NonZeroExitProducesLifecycleError`: verifies host exit code reaches `*LifecycleError.ExitCode`.
- `TestInitializeCommand_SkippedByDefault`: confirms `RunInitializeCommand=false` (default) bypasses the executor entirely.

No integration test — host execution is hostile to CI (would need a sandbox), and the contract is fully covered by unit tests via the in-memory `fakeHostExecutor`.

Summary by CodeRabbit

Release Notes

New Features
- Introduced host executor configuration to the engine, enabling execution of security-sensitive initialization commands on the host.
- Engine now properly routes and executes initializeCommand hooks with output capture and exit code reporting.
- Support for both single and parallel initialization command execution modes.
- Enhanced error messaging when host executor is not configured.
Tests
- Added comprehensive test coverage for host executor integration and initialization command workflows.

… hooks) Adds a caller-supplied HostExecutor interface that the engine dispatches host-side spec hooks to. initializeCommand now routes through it (Single + Parallel forms). secretsCommand will plug into the same interface in a follow-up. Library does NOT ship a default HostExecutor: host execution is security-sensitive (devcontainer.json can declare arbitrary commands), and policy decisions — sandboxing, env filtering, timeouts, working dir — belong to the embedding application. Unconfigured executor + a configured initializeCommand returns a *LifecycleError wrapping a typed sentinel ErrHostExecutorNotConfigured so callers can errors.Is and surface a useful message. Closes #11. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-05-10T04:01:27Z

📝 Walkthrough

Walkthrough

The PR implements host-side execution of initializeCommand by adding a HostExecutor interface to EngineOptions. The engine now routes initialization commands (Single and Parallel forms) to the host executor and surfaces errors or exit codes via LifecycleError. When no executor is configured, initialization fails explicitly with ErrHostExecutorNotConfigured.

Changes

Host Executor Integration for initializeCommand

Layer / File(s)	Summary
Host Executor Contract `host_executor.go`	Defines `HostExecutor` interface (`ExecHost(ctx, cmd)`), `HostCommand` input (Shell/Exec, Env, WorkingDir), `HostExecResult` output (ExitCode, Stdout, Stderr), and `ErrHostExecutorNotConfigured` sentinel error.
Engine Options Wiring `engine.go`	`EngineOptions` gains optional `HostExecutor` field; nil executor causes host hooks to return `LifecycleError(ErrHostExecutorNotConfigured)`.
Initialization Logic `lifecycle.go`	`runInitialize` routes Single and Parallel commands to host executor; `runInitializeSingle` executes one command via `HostExecutor.ExecHost`; `runInitializeParallel` runs sorted command names concurrently and aggregates results.
Tests and Coverage `lifecycle_test.go`	Adds four tests: missing executor returns `LifecycleError`, single command routed with correct shell and working directory, non-zero exit propagates exit code, and initialization skipped by default unless `RunInitializeCommand=true`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A host executor hops into the garden,
Safely running commands (no need for pardon),
Single and Parallel dance hand-in-hand,
With exit codes and streams oh so grand!
Initialize, test, and deploy with cheer,
The devcontainer engine sparkles here! ✨

🚥 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%.	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 specifically describes the main change: introducing a HostExecutor interface for initializeCommand execution, which is the core of this PR.
Linked Issues check	✅ Passed	All coding objectives from issue `#11` are met: HostExecutor interface is added to EngineOptions, initializeCommand routes through it for Single/Parallel forms, ErrHostExecutorNotConfigured is returned when nil, and comprehensive unit tests validate the behavior.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing the HostExecutor interface and wiring initializeCommand execution. No unrelated refactoring or features are introduced.

_{✏️ 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 host-executor

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

bilby91 · 2026-05-10T13:36:51Z

@CodeRabbit review

coderabbitai · 2026-05-10T13:36:56Z

coderabbitai

🧹 Nitpick comments (1)

lifecycle_test.go (1)

302-369: ⚡ Quick win

Consider adding test coverage for the Parallel command form.

The tests comprehensively cover the Single command path, but runInitializeParallel (which handles initializeCommand as a map of named commands running concurrently) lacks direct test coverage. Given the concurrency logic involved, a test verifying parallel execution would strengthen confidence.

Example test skeleton

func TestInitializeCommand_RoutesToHostExecutorParallel(t *testing.T) {
	rt := newScriptedRuntime()
	hx := &fakeHostExecutor{}
	eng, _ := New(EngineOptions{Runtime: rt, HostExecutor: hx})
	ws := writeImageDevcontainer(t, `{
		"image":"alpine:3.20",
		"initializeCommand": {
			"setup": "echo setup",
			"prepare": "echo prepare"
		}
	}`)
	if _, err := eng.Up(context.Background(), UpOptions{
		LocalWorkspaceFolder: ws,
		RunInitializeCommand: true,
	}); err != nil {
		t.Fatalf("Up: %v", err)
	}
	if len(hx.calls) != 2 {
		t.Fatalf("expected 2 host calls, got %d", len(hx.calls))
	}
}

🤖 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 `@lifecycle_test.go` around lines 302 - 369, Add a unit test that exercises the
parallel form of initializeCommand by invoking eng.Up with RunInitializeCommand:
true on a devcontainer.json where "initializeCommand" is a map (e.g.
{"setup":"echo setup","prepare":"echo prepare"}), using newScriptedRuntime() and
a fakeHostExecutor to capture host calls; assert that runInitializeParallel
(indirectly via Up/EngineOptions and HostExecutor) results in two HostExecutor
calls (fakeHostExecutor.calls length == 2) and that each call’s Shell matches
the expected commands, and ensure the test mirrors existing patterns used in
TestInitializeCommand_RoutesToHostExecutorSingle and
TestInitializeCommand_SkippedByDefault.

🤖 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 `@lifecycle_test.go`:
- Around line 302-369: Add a unit test that exercises the parallel form of
initializeCommand by invoking eng.Up with RunInitializeCommand: true on a
devcontainer.json where "initializeCommand" is a map (e.g. {"setup":"echo
setup","prepare":"echo prepare"}), using newScriptedRuntime() and a
fakeHostExecutor to capture host calls; assert that runInitializeParallel
(indirectly via Up/EngineOptions and HostExecutor) results in two HostExecutor
calls (fakeHostExecutor.calls length == 2) and that each call’s Shell matches
the expected commands, and ensure the test mirrors existing patterns used in
TestInitializeCommand_RoutesToHostExecutorSingle and
TestInitializeCommand_SkippedByDefault.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d54d45e2-4db2-4244-9310-ab89f570351a

📥 Commits

Reviewing files that changed from the base of the PR and between ad340b0 and 698bb2e.

📒 Files selected for processing (4)

engine.go
host_executor.go
lifecycle.go
lifecycle_test.go

coderabbitai Bot reviewed May 10, 2026

View reviewed changes

coderabbitai Bot approved these changes May 10, 2026

View reviewed changes

bilby91 merged commit b245721 into main May 10, 2026
5 checks passed

bilby91 mentioned this pull request May 10, 2026

engine: secretsCommand via HostExecutor (closes #23) #35

Merged

4 tasks

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

engine: HostExecutor interface for initializeCommand#33

engine: HostExecutor interface for initializeCommand#33
bilby91 merged 1 commit into
mainfrom
host-executor

bilby91 commented May 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 10, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

bilby91 commented May 10, 2026

Uh oh!

coderabbitai Bot commented May 10, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

bilby91 commented May 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

API surface

Why no default impl?

Test plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

bilby91 commented May 10, 2026

Uh oh!

coderabbitai Bot commented May 10, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bilby91 commented May 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 10, 2026 •

edited

Loading