Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions internal/app/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,26 @@ func (p runtimeImageBuildPlan) NeedsRebuild() bool {
return p.StructuralRebuild || p.AgentUpdates.NeedsRebuild
}

func coordinateRuntimeImageBuild(home string, imageName string, forceRebuild bool, resolveBuildPlan func() (runtimeImageBuildPlan, error), executeBuildPlan func(runtimeImageBuildPlan) error) error {
lockName := "image-build-" + util.HashString(imageName) + ".lock"
lockPath := config.HostLockPath(home, lockName)
release, _, err := util.AcquireFileLock(lockPath, func() {
logx.Infof("Waiting for another enclave process to finish building %s.", imageName)
})
if err != nil {
return err
}
defer release()
buildPlan, err := resolveBuildPlan()
if err != nil {
return err
}
if !forceRebuild && !buildPlan.NeedsRebuild() {
return nil
}
return executeBuildPlan(buildPlan)
}

var (
dockerBuildImage = docker.Build
dockerImageExists = docker.ImageExists
Expand Down
42 changes: 24 additions & 18 deletions internal/app/command_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,33 +274,39 @@ func ensureRuntimeImage(input *CommandInput, opts model.Options, buildCfg *build
return buildConfig{}, 1
}
if opts.ForceRebuild || buildPlan.NeedsRebuild() {
if code := buildOrReuseRuntimeImage(input, opts, host, resolved, buildPlan); code != 0 {
if code := buildOrReuseRuntimeImage(input, opts, host, resolved); code != 0 {
return buildConfig{}, code
}
}
return resolved, 0
}

func buildOrReuseRuntimeImage(input *CommandInput, opts model.Options, host model.Host, resolved buildConfig, buildPlan runtimeImageBuildPlan) int {
reused := false
if !opts.ForceRebuild && buildPlan.StructuralRebuild && !buildPlan.AgentUpdates.NeedsRebuild {
ok, reuseErr := reuseRuntimeImageByContentHash(context.Background(), resolved.ImageName, buildPlan.CombinedHash)
if reuseErr != nil {
logx.Debugf("content-cache lookup failed: %v", reuseErr)
func buildOrReuseRuntimeImage(input *CommandInput, opts model.Options, host model.Host, resolved buildConfig) int {
resolveBuildPlan := func() (runtimeImageBuildPlan, error) {
return resolveRuntimeImageBuildPlan(input.Ctx.Paths, resolved, opts.BuildOptions, opts.Tool, host.Home, opts.ForceRebuild, time.Now().UTC())
}
executeBuildPlan := func(buildPlan runtimeImageBuildPlan) error {
reused := false
if !opts.ForceRebuild && buildPlan.StructuralRebuild && !buildPlan.AgentUpdates.NeedsRebuild {
ok, reuseErr := reuseRuntimeImageByContentHash(context.Background(), resolved.ImageName, buildPlan.CombinedHash)
if reuseErr != nil {
logx.Debugf("content-cache lookup failed: %v", reuseErr)
}
reused = ok
}
reused = ok
}
if reused {
return 0
}
if !opts.ForceRebuild {
if buildPlan.StructuralRebuild {
logx.Infof("Build inputs changed, rebuilding automatically.")
} else {
logx.Infof("Agent update interval elapsed, rebuilding automatically.")
if reused {
return nil
}
if !opts.ForceRebuild {
if buildPlan.StructuralRebuild {
logx.Infof("Build inputs changed, rebuilding automatically.")
} else {
logx.Infof("Agent update interval elapsed, rebuilding automatically.")
}
}
return buildImage(context.Background(), input.Ctx.Paths, host, buildPlan.CombinedHash, resolved, opts.BuildOptions, opts.Tool, buildPlan.AgentUpdates)
}
if err := buildImage(context.Background(), input.Ctx.Paths, host, buildPlan.CombinedHash, resolved, opts.BuildOptions, opts.Tool, buildPlan.AgentUpdates); err != nil {
if err := coordinateRuntimeImageBuild(host.Home, resolved.ImageName, opts.ForceRebuild, resolveBuildPlan, executeBuildPlan); err != nil {
logx.Errorf("%v", err)
return 1
}
Expand Down
90 changes: 90 additions & 0 deletions internal/app/command_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import (
"context"
"errors"
"strings"
"sync"
"sync/atomic"
"testing"
"time"

"enclave/internal/backend"
"enclave/internal/model"
Expand Down Expand Up @@ -110,3 +113,90 @@ func TestEnsureExistingRuntimeImageWith(t *testing.T) {
}
})
}

func TestCoordinateRuntimeImageBuildRechecksAfterWaiting(t *testing.T) {
home := t.TempDir()
var imageReady atomic.Bool
var builds atomic.Int32
var resolves atomic.Int32
firstBuildStarted := make(chan struct{})
releaseFirstBuild := make(chan struct{})
secondResolveStarted := make(chan struct{})

resolve := func() (runtimeImageBuildPlan, error) {
if resolves.Add(1) == 2 {
close(secondResolveStarted)
Comment thread
xai marked this conversation as resolved.
}
return runtimeImageBuildPlan{StructuralRebuild: !imageReady.Load()}, nil
}
execute := func(runtimeImageBuildPlan) error {
if builds.Add(1) == 1 {
close(firstBuildStarted)
<-releaseFirstBuild
imageReady.Store(true)
}
return nil
}

var wg sync.WaitGroup
errs := make(chan error, 2)
wg.Add(1)
go func() {
defer wg.Done()
errs <- coordinateRuntimeImageBuild(home, "enclave-codex:latest", false, resolve, execute)
}()
<-firstBuildStarted

secondStarted := make(chan struct{})
wg.Add(1)
go func() {
defer wg.Done()
close(secondStarted)
errs <- coordinateRuntimeImageBuild(home, "enclave-codex:latest", false, resolve, execute)
}()
<-secondStarted
serialized := true
select {
case <-secondResolveStarted:
serialized = false
case <-time.After(20 * time.Millisecond):
}
close(releaseFirstBuild)
wg.Wait()
if !serialized {
t.Fatal("second caller resolved its build plan while the first build held the lock")
}
close(errs)
for err := range errs {
if err != nil {
t.Fatalf("coordinateRuntimeImageBuild returned error: %v", err)
}
}
if got := builds.Load(); got != 1 {
t.Fatalf("build count = %d, want 1", got)
}
if got := resolves.Load(); got != 2 {
t.Fatalf("in-lock build-plan resolution count = %d, want 2", got)
}
}

func TestCoordinateRuntimeImageBuildPreservesForceRebuild(t *testing.T) {
home := t.TempDir()
var builds atomic.Int32
resolve := func() (runtimeImageBuildPlan, error) {
return runtimeImageBuildPlan{}, nil
}
execute := func(runtimeImageBuildPlan) error {
builds.Add(1)
return nil
}

for range 2 {
if err := coordinateRuntimeImageBuild(home, "enclave-codex:latest", true, resolve, execute); err != nil {
t.Fatalf("coordinateRuntimeImageBuild returned error: %v", err)
}
}
if got := builds.Load(); got != 2 {
t.Fatalf("build count = %d, want 2", got)
}
}
10 changes: 6 additions & 4 deletions internal/app/command_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,11 @@ func updateToolImage(ctx *AppContext, opts model.Options, host model.Host, proje
return err
}
buildCfg.HashSuffix = appendEffectiveBuildIdentityHashSuffix(buildCfg.HashSuffix, host, opts.BuildOptions)
buildPlan, err := resolveRuntimeImageBuildPlan(ctx.Paths, buildCfg, opts.BuildOptions, tool, host.Home, true, time.Now().UTC())
if err != nil {
return err
resolveBuildPlan := func() (runtimeImageBuildPlan, error) {
return resolveRuntimeImageBuildPlan(ctx.Paths, buildCfg, opts.BuildOptions, tool, host.Home, true, time.Now().UTC())
}
executeBuildPlan := func(buildPlan runtimeImageBuildPlan) error {
return buildImage(context.Background(), ctx.Paths, host, buildPlan.CombinedHash, buildCfg, opts.BuildOptions, tool, buildPlan.AgentUpdates)
}
return buildImage(context.Background(), ctx.Paths, host, buildPlan.CombinedHash, buildCfg, opts.BuildOptions, tool, buildPlan.AgentUpdates)
return coordinateRuntimeImageBuild(host.Home, buildCfg.ImageName, true, resolveBuildPlan, executeBuildPlan)
}
7 changes: 2 additions & 5 deletions internal/backend/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ func runForeground(ctx context.Context, config *dockercmd.ContainerConfig, hostC
if attach.TTY {
return dockercmd.RunInteractiveWithStartHook(ctx, config, hostConfig, name, attach.OnStarted)
}
return dockercmd.Run(ctx, config, hostConfig, name)
return dockercmd.RunWithStartHook(ctx, config, hostConfig, name, attach.OnStarted)
}
in := attach.In
out := attach.Out
Expand All @@ -569,10 +569,7 @@ func runForeground(ctx context.Context, config *dockercmd.ContainerConfig, hostC
if errOut == nil {
errOut = io.Discard
}
if attach.TTY {
return dockercmd.RunWithIOAndTTY(ctx, config, hostConfig, name, in, out, errOut)
}
return dockercmd.RunWithIO(ctx, config, hostConfig, name, in, out, errOut)
return dockercmd.RunWithIOAndStartHook(ctx, config, hostConfig, name, in, out, errOut, attach.TTY, attach.OnStarted)
}

func applyContainerHardening(hostConfig *dockercmd.HostConfig) {
Expand Down
8 changes: 5 additions & 3 deletions internal/backend/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,11 @@ type AttachIO struct {
Err io.Writer
TTY bool
DetachKeys string
// OnStarted, if set, is invoked once the backend confirms the session is
// running (e.g. to announce published ports). Backends that cannot observe
// startup may skip it.
// OnStarted, if set, must be invoked exactly once as soon as the backend
// confirms the session is running. The runtime releases the session-start
// lock here (and announces published ports); a foreground Run that never
// calls it holds that lock until the session exits, blocking every other
// session start for the same tool and project.
OnStarted func()
Comment thread
xai marked this conversation as resolved.
}

Expand Down
30 changes: 21 additions & 9 deletions internal/docker/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ func classifyRunError(args []string, err error, stderr string) error {
// Run runs a container to completion, discarding its output, and returns an
// *ExitError when the container exits non-zero.
func Run(ctx context.Context, config *ContainerConfig, hostConfig *HostConfig, name string) error {
return RunWithStartHook(ctx, config, hostConfig, name, nil)
}

// RunWithStartHook runs a container to completion, discarding its output, and
// invokes onStarted after Docker reports the named container is running.
func RunWithStartHook(ctx context.Context, config *ContainerConfig, hostConfig *HostConfig, name string, onStarted func()) error {
if ctx == nil {
ctx = context.Background()
}
Expand All @@ -44,22 +50,24 @@ func Run(ctx context.Context, config *ContainerConfig, hostConfig *HostConfig, n
cmd := exec.CommandContext(ctx, dockerBinary, args...) // #nosec G204 -- args built from caller config, passed without a shell.
cmd.Stdout = io.Discard
cmd.Stderr = &stderr
return classifyRunError(args, cmd.Run(), stderr.String())
return classifyRunError(args, runCommandWithStartHook(ctx, name, cmd, onStarted), stderr.String())
}

// RunWithIO runs a container wired to the supplied streams (no TTY) and returns
// an *ExitError when the container exits non-zero.
func RunWithIO(ctx context.Context, config *ContainerConfig, hostConfig *HostConfig, name string, in io.Reader, out io.Writer, errOut io.Writer) error {
return runWithIO(ctx, config, hostConfig, name, in, out, errOut, false)
return RunWithIOAndStartHook(ctx, config, hostConfig, name, in, out, errOut, false, nil)
}

// RunWithIOAndTTY runs a container wired to the supplied streams with a TTY
// allocated and returns an *ExitError when the container exits non-zero.
func RunWithIOAndTTY(ctx context.Context, config *ContainerConfig, hostConfig *HostConfig, name string, in io.Reader, out io.Writer, errOut io.Writer) error {
return runWithIO(ctx, config, hostConfig, name, in, out, errOut, true)
return RunWithIOAndStartHook(ctx, config, hostConfig, name, in, out, errOut, true, nil)
}

func runWithIO(ctx context.Context, config *ContainerConfig, hostConfig *HostConfig, name string, in io.Reader, out io.Writer, errOut io.Writer, tty bool) error {
// RunWithIOAndStartHook runs a container wired to the supplied streams and
// invokes onStarted after Docker reports the named container is running.
func RunWithIOAndStartHook(ctx context.Context, config *ContainerConfig, hostConfig *HostConfig, name string, in io.Reader, out io.Writer, errOut io.Writer, tty bool, onStarted func()) error {
if ctx == nil {
ctx = context.Background()
}
Expand All @@ -69,7 +77,7 @@ func runWithIO(ctx context.Context, config *ContainerConfig, hostConfig *HostCon
cmd.Stdin = in
cmd.Stdout = out
cmd.Stderr = errOut
return classifyRunError(args, cmd.Run(), "")
return classifyRunError(args, runCommandWithStartHook(ctx, name, cmd, onStarted), "")
}

// RunCapture runs a container and returns its trimmed stdout, surfacing stderr
Expand Down Expand Up @@ -106,25 +114,29 @@ func RunInteractiveWithStartHook(ctx context.Context, config *ContainerConfig, h
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return classifyRunError(args, runCommandWithStartHook(ctx, name, cmd, onStarted), "")
}

func runCommandWithStartHook(ctx context.Context, name string, cmd *exec.Cmd, onStarted func()) error {
if err := cmd.Start(); err != nil {
return classifyRunError(args, err, "")
return err
}
waitCh := make(chan error, 1)
go func() {
waitCh <- cmd.Wait()
}()
if onStarted != nil && strings.TrimSpace(name) != "" {
if err, done := waitForContainerRunning(ctx, name, waitCh); done {
return classifyRunError(args, err, "")
return err
}
select {
case err := <-waitCh:
return classifyRunError(args, err, "")
return err
default:
}
onStarted()
}
return classifyRunError(args, <-waitCh, "")
return <-waitCh
}

func waitForContainerRunning(ctx context.Context, name string, waitCh <-chan error) (error, bool) {
Expand Down
41 changes: 41 additions & 0 deletions internal/docker/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package docker

import (
"context"
"io"
"os"
"path/filepath"
"slices"
Expand Down Expand Up @@ -63,6 +64,46 @@ func TestRunInteractiveWithStartHookSkipsHookWhenRunFailsBeforeStart(t *testing.
}
}

func TestRunWithStartHookWaitsForRunningContainer(t *testing.T) {
withRunStartHookStub(t)
t.Setenv("STUB_RUN_TOUCH", "1")
t.Setenv("STUB_RUN_SLEEP", "0.3")
t.Setenv("STUB_RUN_EXIT", "0")

called := false
err := RunWithStartHook(context.Background(), &ContainerConfig{Image: "example:test"}, nil, "session", func() {
called = true
})
if err != nil {
t.Fatalf("RunWithStartHook returned error: %v", err)
}
if !called {
t.Fatal("expected start hook to be called")
}
}

func TestRunWithIOAndStartHookWaitsForRunningContainer(t *testing.T) {
for _, tty := range []bool{false, true} {
t.Run(map[bool]string{false: "without TTY", true: "with TTY"}[tty], func(t *testing.T) {
withRunStartHookStub(t)
t.Setenv("STUB_RUN_TOUCH", "1")
t.Setenv("STUB_RUN_SLEEP", "0.3")
t.Setenv("STUB_RUN_EXIT", "0")

called := false
err := RunWithIOAndStartHook(context.Background(), &ContainerConfig{Image: "example:test"}, nil, "session", nil, io.Discard, io.Discard, tty, func() {
called = true
})
if err != nil {
t.Fatalf("RunWithIOAndStartHook returned error: %v", err)
}
if !called {
t.Fatal("expected start hook to be called")
}
})
}
}

func withRunStartHookStub(t *testing.T) {
t.Helper()
dir := t.TempDir()
Expand Down
Loading
Loading