diff --git a/showcase/bin/showcase b/showcase/bin/showcase index fd44000d6f..a2b00342fb 100755 --- a/showcase/bin/showcase +++ b/showcase/bin/showcase @@ -96,18 +96,43 @@ cmd_up() { fi info "Launching fleet topology: control-plane + ${pool_count} worker(s) + PocketBase + demos + aimock (HARNESS_POOL_COUNT=${pool_count})" - # Scope `--build` to the targeted slug(s) when provided. Without a positional - # service filter, `up -d --build` rebuilds EVERY service in every active - # profile (infra + slug profiles), which serializes BuildKit and stalls - # concurrent `--isolate` runs (issue #5495, A21). When no slugs are given - # (infra-only bring-up), the blanket `--build` is preserved so first-time - # bootstrap still builds whatever infra services are missing. + # Two-call strategy to preserve A21's BuildKit-contention fix (target-only + # rebuild) WITHOUT regressing infra startup that A21 inadvertently dropped + # (A21b, issue #5495). + # + # docker compose semantics: positional service names after `up` restrict + # WHICH services start to the named ones + their `depends_on` chain — not + # just which ones get `--build`-rebuilt. So a single + # `up -d --build ` only brings up the slug + its depends_on (aimock), + # leaving the rest of the infra profile (pocketbase/dashboard/harness/ + # harness-pool-worker) down. Under `--isolate` with a sibling stack on the + # same host ports, health checks then cross onto foreign containers and + # cells silently misroute → 0.0s red. + # + # Fix: when slugs are present, emit TWO compose calls: + # 1. up -d (no --build, no positional services) — start all + # services in the active profiles using cached images. + # 2. up -d --build — force-rebuild ONLY the named + # services and ensure they're up. Other services from call (1) are + # no-ops. + # When no slugs are present (infra-only bring-up), keep the single blanket + # `--build` call so first-time bootstrap still builds missing images. # # bash 3.2 (macOS default) errors on `${arr[@]}` for an EMPTY array under # `set -u`; the `${arr[@]+...}` guard expands to nothing when unset/empty. - $compose_cmd --profile infra ${profile_args[@]+"${profile_args[@]}"} up -d --build \ - --scale harness-pool-worker="$pool_count" ${passthrough_args[@]+"${passthrough_args[@]}"} \ - ${build_targets[@]+"${build_targets[@]}"} + if [[ ${#build_targets[@]} -gt 0 ]]; then + # Call 1: bring up ALL services in active profiles, no build. + $compose_cmd --profile infra ${profile_args[@]+"${profile_args[@]}"} up -d \ + --scale harness-pool-worker="$pool_count" ${passthrough_args[@]+"${passthrough_args[@]}"} + # Call 2: rebuild ONLY the targeted slug(s) and ensure they're up. + $compose_cmd --profile infra ${profile_args[@]+"${profile_args[@]}"} up -d --build \ + --scale harness-pool-worker="$pool_count" ${passthrough_args[@]+"${passthrough_args[@]}"} \ + "${build_targets[@]}" + else + # Infra-only: single call with blanket --build for first-time bootstrap. + $compose_cmd --profile infra up -d --build \ + --scale harness-pool-worker="$pool_count" ${passthrough_args[@]+"${passthrough_args[@]}"} + fi } cmd_down() { diff --git a/showcase/harness/src/cli/lifecycle.test.ts b/showcase/harness/src/cli/lifecycle.test.ts index a599eaaf1c..718d0ccae8 100644 --- a/showcase/harness/src/cli/lifecycle.test.ts +++ b/showcase/harness/src/cli/lifecycle.test.ts @@ -101,7 +101,7 @@ describe("rebuild() — targeted slugs", () => { }); }); -describe("up() — scoped --build (A21)", () => { +describe("up() — two-call infra-start + scoped --build (A21b)", () => { // Stub healthCheck transitively by making fetch unreachable but compose succeed. // up() throws on unhealthy services, so we mock the health-check path via // network fetch failing — but the IMPORTANT assertion is the compose argv @@ -118,13 +118,20 @@ describe("up() — scoped --build (A21)", () => { }); /** - * The compose argv shape we want: - * [--profile infra --profile ... up -d --build ...] + * The two-call shape we want when slugs are provided: + * Call 1: [--profile infra --profile ... up -d] + * (no --build, no positional services — brings up ALL services in + * active profiles using cached images) + * Call 2: [--profile infra --profile ... up -d --build ...] + * (rebuilds ONLY the named services; others are no-ops) * - * Without slug positionals AFTER `--build`, compose rebuilds EVERY service - * in every active profile (infra + slug profiles). With slug positionals, - * compose rebuilds ONLY the named services and uses cached images for - * everything else — that is the A21 fix. + * Background: docker compose positional service names after `up` restrict + * WHICH services start (not just which get rebuilt). A21's single-call + * `up -d --build ` therefore prevented infra services without an + * explicit `depends_on` from the slug from starting at all — health checks + * crossed onto sibling stack containers and cells silently misrouted (0.0s + * red). A21b splits into two calls to keep target-only rebuild while + * restoring the full infra startup. * * `--profile ` arguments do NOT scope the build target on their own; * docker compose only treats positional args after `up` as the service @@ -151,39 +158,66 @@ describe("up() — scoped --build (A21)", () => { return out; } - it("scopes --build to the targeted slug (does NOT rebuild infra services)", async () => { + /** Pull the `--profile ` set from a compose argv. */ + function profilesIn(argv: string[]): string[] { + const out: string[] = []; + for (let i = 0; i < argv.length - 1; i++) { + if (argv[i] === "--profile") out.push(argv[i + 1]); + } + return out.sort(); + } + + it("emits TWO compose calls when slugs provided: infra-up then target-build", async () => { await up(["langgraph-python"]); - const upCall = composeCalls().find( - (argv) => argv.includes("up") && argv.includes("--build"), - ); - expect(upCall).toBeDefined(); - // The slug must appear as a positional AFTER `up` so compose treats it as - // the build target filter — not merely as a `--profile ` argument - // (profiles don't scope --build). - expect(positionalsAfterUp(upCall!)).toEqual(["langgraph-python"]); + const upCalls = composeCalls().filter((argv) => argv.includes("up")); + expect(upCalls.length).toBe(2); + + // Call 1: no --build, no positional services. + const [call1, call2] = upCalls; + expect(call1).not.toContain("--build"); + expect(positionalsAfterUp(call1)).toEqual([]); + + // Call 2: --build present, slug as positional after `up`. + expect(call2).toContain("--build"); + expect(positionalsAfterUp(call2)).toEqual(["langgraph-python"]); }); - it("with no slugs (infra-only), --build remains blanket (first-time bootstrap)", async () => { + it("with no slugs (infra-only), emits a SINGLE blanket --build call", async () => { await up([]); - const upCall = composeCalls().find( - (argv) => argv.includes("up") && argv.includes("--build"), - ); - expect(upCall).toBeDefined(); + const upCalls = composeCalls().filter((argv) => argv.includes("up")); + expect(upCalls.length).toBe(1); + const [call] = upCalls; + expect(call).toContain("--build"); // No slug positional after `up`: compose builds whatever is missing in the // infra profile (cached images skipped, fresh ones built). This preserves // first-time bootstrap behaviour when no infra images exist yet. - expect(positionalsAfterUp(upCall!)).toEqual([]); + expect(positionalsAfterUp(call)).toEqual([]); }); - it("multi-slug: scopes --build to ALL targeted slugs", async () => { + it("multi-slug: scopes --build to ALL targeted slugs (call 2)", async () => { await up(["a", "b"]); - const upCall = composeCalls().find( - (argv) => argv.includes("up") && argv.includes("--build"), - ); - expect(upCall).toBeDefined(); - expect(positionalsAfterUp(upCall!).sort()).toEqual(["a", "b"]); + const upCalls = composeCalls().filter((argv) => argv.includes("up")); + expect(upCalls.length).toBe(2); + const [, call2] = upCalls; + expect(call2).toContain("--build"); + expect(positionalsAfterUp(call2).sort()).toEqual(["a", "b"]); + }); + + it("both calls use IDENTICAL profile flags (no profile drift)", async () => { + await up(["langgraph-python"]); + + const upCalls = composeCalls().filter((argv) => argv.includes("up")); + expect(upCalls.length).toBe(2); + const [call1, call2] = upCalls; + // Same --profile set in both calls so services across calls agree on + // which profiles are "active" — drift here would mean call 1 brings up a + // service that call 2 doesn't know about (or vice versa). + expect(profilesIn(call1)).toEqual(profilesIn(call2)); + // And the slug's profile is actually present. + expect(profilesIn(call1)).toContain("infra"); + expect(profilesIn(call1)).toContain("langgraph-python"); }); }); diff --git a/showcase/harness/src/cli/lifecycle.ts b/showcase/harness/src/cli/lifecycle.ts index 7dcf82733e..68e645e2f7 100644 --- a/showcase/harness/src/cli/lifecycle.ts +++ b/showcase/harness/src/cli/lifecycle.ts @@ -265,30 +265,47 @@ export async function up( } const verboseFlag = opts?.verbose ? ["--progress", "plain"] : []; - // Scope `--build` to the targeted slug(s) when provided. Without a - // positional service filter, `up -d --build` rebuilds EVERY service in - // every active profile (infra + slug profiles), which serializes BuildKit - // and stalls concurrent `--isolate` runs — slot N's full-stack rebuild - // contends with slot M's rebuild on the same daemon (issue #5495, A21). + // Two-call strategy to preserve A21's BuildKit-contention fix (target-only + // rebuild) WITHOUT regressing the infra startup that A21 inadvertently + // dropped (A21b, issue #5495). // - // With positional slugs after `up`, compose only rebuilds the named - // services; everything else uses cached images and falls through to plain - // start. First-time bootstrap (no infra images) still works because - // compose builds missing images automatically — `--build` only FORCES a - // rebuild of services that already have an image. For a forced full - // rebuild, callers should use `rebuild()` (the `--rebuild` flag path). - const buildTargets = slugs.length > 0 ? slugs : []; - const args = [ - ...profileArgs, - "up", - "-d", - "--build", - ...verboseFlag, - ...buildTargets, - ]; - + // docker compose semantics: positional service names after `up` restrict + // WHICH services start to the named ones + their `depends_on` chain — not + // just which ones get `--build`-rebuilt. A21 passed the target slug as a + // positional after `up -d --build`, which (correctly) scoped the rebuild + // to the slug but (incorrectly) prevented infra services without an + // explicit `depends_on` from the target (pocketbase, dashboard, harness, + // harness-pool-worker) from coming up. Under `--isolate` with a sibling + // stack holding the same host ports, health checks then crossed onto the + // foreign containers and the cell silently misrouted → 0.0s red. + // + // Fix: split into two calls when slugs is non-empty. + // 1. compose up -d — start ALL services in the + // active profiles using cached images. No `--build`, no positional + // services. Brings up the full infra profile + the slug's profile. + // 2. compose up -d --build + // Force-rebuild ONLY the named services and ensure they're up. Other + // services already running from call (1) are no-ops. + // + // When slugs is empty (infra-only bring-up), keep the single blanket call + // so first-time bootstrap still builds whatever infra images are missing. log.info("starting services", { slugs: slugs.length ? slugs : ["infra"] }); - compose(...args); + if (slugs.length > 0) { + // Call 1: bring up all services (no build, cached images). + compose(...profileArgs, "up", "-d", ...verboseFlag); + // Call 2: rebuild target slug(s) and ensure they're up. + compose( + ...profileArgs, + "up", + "-d", + "--build", + ...verboseFlag, + ...slugs, + ); + } else { + // Infra-only: single call with blanket --build for first-time bootstrap. + compose(...profileArgs, "up", "-d", "--build", ...verboseFlag); + } // Determine which services to health-check const servicesToCheck =