From e586eb7c696fb178e1c9ce66282d6fb125929ecf Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Sat, 22 Aug 2026 12:17:21 -0400 Subject: [PATCH] fix(builder): align SingleImageBuildPlan paths with the output_directory contract The CLI (the recipe consumer) passes output_directory = /builder and resolves recipe paths relative to it, with the build context "." mapping to the service directory. SingleImageBuildPlan emitted "builder/Dockerfile" and "builder/dockerignore" instead, so every recipe build failed validateRecipes against the real caller. Emit the Dockerfile and dockerignore relative to output_directory to match; the unit test that pinned the old layout was a false green (it constructed a tree the CLI never produces). Also from the review: - Detect the dockerignore with Lstat, consistent with the inventory's blanket symlink rejection, so a symlinked dockerignore never enters the recipe. - Warn instead of silently skipping when recipe emission was requested but a custom Docker context root forces the legacy in-process build. - Document why RecipeBuildPlatforms is a fixed multi-arch set and deliberately does not read the single-platform CODEFLY_BUILD_PLATFORM override. Co-Authored-By: Claude Opus 4.8 --- agents/services/base_builder.go | 4 +- agents/services/docker_recipe.go | 41 ++++++++++++------- .../docker_recipe_singleimage_test.go | 13 +++--- runners/golang/agent_builder.go | 15 +++++-- 4 files changed, 47 insertions(+), 26 deletions(-) diff --git a/agents/services/base_builder.go b/agents/services/base_builder.go index bc6406b7..854a79a0 100644 --- a/agents/services/base_builder.go +++ b/agents/services/base_builder.go @@ -262,8 +262,8 @@ func (s *BuilderWrapper) WithBuildPlan(plan *builderv0.DockerBuildPlan) { // caller's output_directory, records it as the build result, and returns the build // response. A language runner calls this — instead of building the image in-process // — when BuildPlanRequested(req) is true. The conventional layout it emits -// (builder/Dockerfile, context = the service directory, an optional -// builder/dockerignore) is language-agnostic, so Go, Rust, Python, Node, and every +// (a Dockerfile in output_directory, context = the service directory, an optional +// dockerignore) is language-agnostic, so Go, Rust, Python, Node, and every // other agent built on the shared builder move to CLI-owned, multi-arch builds // through this one path — just by re-pinning core. func (s *BuilderWrapper) SingleImageBuildResponse(req *builderv0.BuildRequest, image string) (*builderv0.BuildResponse, error) { diff --git a/agents/services/docker_recipe.go b/agents/services/docker_recipe.go index e1aa36d2..50c0fe64 100644 --- a/agents/services/docker_recipe.go +++ b/agents/services/docker_recipe.go @@ -68,11 +68,16 @@ func BuildDockerBuildPlan(destination string, recipes []*builderv0.DockerBuildRe }, nil } -// RecipeBuildPlatforms is the deployment platform set an emitted recipe targets: -// a linux/amd64 + linux/arm64 manifest list, so a consumer never needs a local -// rebuild regardless of node architecture. The legacy in-process build honors -// the single-platform CODEFLY_BUILD_PLATFORM override; the recipe path supersedes -// it with a multi-arch manifest list. +// RecipeBuildPlatforms is the platform set an emitted recipe targets: a +// linux/amd64 + linux/arm64 manifest list, so a pushed image runs on any +// deployment node regardless of the builder's host architecture. It is +// deliberately fixed and does NOT read the single-platform CODEFLY_BUILD_PLATFORM +// override the legacy in-process build honored: the recipe is a durable, +// reproducible artifact whose digest must not vary with the emitting machine's +// environment, and a pushed deploy image must always carry the deployment +// architecture. Narrowing to a single arch for a faster LOCAL (unpushed) build is +// the caller's concern — the CLI selects a host-matching platform from this list +// for a --load build — not something the recipe encodes. func RecipeBuildPlatforms() []string { return []string{"linux/amd64", "linux/arm64"} } @@ -87,20 +92,28 @@ func BuildPlanRequested(req *builderv0.BuildRequest) bool { return req.GetOutputDirectory() != "" } -// SingleImageBuildPlan assembles the plan for a service that emits one image -// from builder/Dockerfile with the service directory (outputDirectory) as its -// build context — the conventional layout every shared-runner agent renders. A -// runner calls it when the caller requested recipe emission (a non-empty -// BuildRequest.output_directory) instead of building the image in-process, so -// the build recipe becomes a durable artifact the caller (the CLI) builds. +// SingleImageBuildPlan assembles the plan for a service that emits one image from +// a Dockerfile the agent rendered into outputDirectory — the service's committed +// builder/ recipe directory, and the value the caller passes as +// BuildRequest.output_directory. The Dockerfile and optional dockerignore live +// directly in outputDirectory (paths are relative to it, not to a nested builder/ +// subdirectory); the build context "." is the service directory, which the caller +// (the CLI) resolves and passes to docker buildx. A runner calls it when the +// caller requested recipe emission (a non-empty BuildRequest.output_directory) +// instead of building the image in-process, so the build recipe becomes a durable +// artifact the caller builds. func SingleImageBuildPlan(outputDirectory, image string, platforms []string) (*builderv0.DockerBuildPlan, error) { dockerignore := "" - if info, err := os.Stat(filepath.Join(outputDirectory, "builder", "dockerignore")); err == nil && info.Mode().IsRegular() { - dockerignore = "builder/dockerignore" + // Lstat, not Stat: inventoryRecipeFiles rejects symlinks outright, so a + // symlinked dockerignore that Stat would follow-and-accept must not enter the + // recipe — it would hard-fail the inventory with an error unrelated to the + // dockerignore reference. + if info, err := os.Lstat(filepath.Join(outputDirectory, "dockerignore")); err == nil && info.Mode().IsRegular() { + dockerignore = "dockerignore" } recipe := &builderv0.DockerBuildRecipe{ Name: "app", - Dockerfile: "builder/Dockerfile", + Dockerfile: "Dockerfile", Context: ".", Dockerignore: dockerignore, Image: image, diff --git a/agents/services/docker_recipe_singleimage_test.go b/agents/services/docker_recipe_singleimage_test.go index 75c1884d..0b34da5f 100644 --- a/agents/services/docker_recipe_singleimage_test.go +++ b/agents/services/docker_recipe_singleimage_test.go @@ -11,12 +11,13 @@ import ( func writeRecipeTree(t *testing.T, withIgnore bool) string { t.Helper() + // dir is the output_directory the caller (the CLI) passes: the service's + // committed builder/ recipe directory, with the Dockerfile — and the optional + // dockerignore — directly inside it, exactly as the runner renders them there. dir := t.TempDir() - builder := filepath.Join(dir, "builder") - require.NoError(t, os.MkdirAll(builder, 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(builder, "Dockerfile"), []byte("FROM alpine\nCOPY . .\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM alpine\nCOPY . .\n"), 0o644)) if withIgnore { - require.NoError(t, os.WriteFile(filepath.Join(builder, "dockerignore"), []byte("code/node_modules\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "dockerignore"), []byte("code/node_modules\n"), 0o644)) } return dir } @@ -30,9 +31,9 @@ func TestSingleImageBuildPlanConventionalLayout(t *testing.T) { recipe := plan.GetRecipes()[0] require.Equal(t, "app", recipe.GetName()) - require.Equal(t, "builder/Dockerfile", recipe.GetDockerfile()) + require.Equal(t, "Dockerfile", recipe.GetDockerfile()) require.Equal(t, ".", recipe.GetContext()) - require.Equal(t, "builder/dockerignore", recipe.GetDockerignore()) + require.Equal(t, "dockerignore", recipe.GetDockerignore()) require.Equal(t, "repo/app:v1", recipe.GetImage()) require.Equal(t, []string{"linux/amd64", "linux/arm64"}, recipe.GetPlatforms()) diff --git a/runners/golang/agent_builder.go b/runners/golang/agent_builder.go index 02cea923..60533b39 100644 --- a/runners/golang/agent_builder.go +++ b/runners/golang/agent_builder.go @@ -80,10 +80,17 @@ func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, // When the caller owns the build (output_directory set), emit the recipe and // let the caller run docker buildx instead of building the image in-process. // A custom ContextRoot builds from a directory other than the service dir, so - // the "context is output_directory" recipe model does not hold — fall through - // to the in-process build, and the caller uses its legacy push path. - if services.BuildPlanRequested(req) && docker.ContextRoot == "" { - return builder.SingleImageBuildResponse(req, image.FullName()) + // the "context is the service directory" recipe model does not hold — fall + // through to the in-process build, and the caller uses its legacy push path. + if services.BuildPlanRequested(req) { + if docker.ContextRoot == "" { + return builder.SingleImageBuildResponse(req, image.FullName()) + } + // The caller asked for a recipe but a custom ContextRoot forces the legacy + // in-process build; surface it so a caller expecting a plan isn't left + // wondering why it got a DockerBuildResult instead. + w.Warn("recipe emission requested but skipped: service uses a custom Docker context root; building in-process", + wool.Field("context_root", docker.ContextRoot)) } configuration, err := goDockerBuilderConfiguration(location, image, w, docker)