fix(showcase/harness): scope --isolate rebuild to target slug only (A21)

Root cause
----------
Both the shell `cmd_up` (showcase/bin/showcase line 98) and the harness
auto-start path (showcase/harness/src/cli/lifecycle.ts up() line 268) emit
`docker compose up -d --build` with NO positional service filter after `up`.
Under `docker compose` semantics, an unfiltered `--build` rebuilds EVERY
service in every active profile — for `--isolate` runs that means the
target slug PLUS aimock + pocketbase + dashboard + harness-pool-worker
all rebuild on every isolated test invocation.

Concurrent `--isolate` runs (one per `SHOWCASE_ISO_SLOT=<N>`) contend on
the shared BuildKit instance, serializing what should be parallel work.
This is the BuildKit-contention stall that has been blocking csdkts (A20)
when it runs alongside other concurrent isolated cells: a small slug
rebuild waits on every other slot's full-stack rebuild.

Fix
---
Scope `--build` to the targeted slug(s) by passing them as positional
service args AFTER `up`. Compose then rebuilds ONLY those services and
falls through to cached images for infra services. First-time bootstrap
still works because `--build` only FORCES a rebuild of services with an
existing image — missing images are built automatically by compose.

Two call sites:
 - showcase/bin/showcase  (user-facing `showcase up <slug>...`)
 - showcase/harness/src/cli/lifecycle.ts (npx tsx test path, the one
   actually triggered under `--isolate`)

When no slugs are provided (infra-only bring-up), the blanket `--build`
is preserved so first-time infra bootstrap rebuilds whatever is missing.

`--rebuild` regression: the `--rebuild` flag goes through `rebuild()`,
not `up()`, and that path is unchanged — it still force-rebuilds and
force-recreates every targeted slug as before.

Red→Green proof
---------------
3 new vitest cases in showcase/harness/src/cli/lifecycle.test.ts assert
the compose argv shape:
 - up(["langgraph-python"]): slug positional after `up` (was: missing)
 - up([]):                  no slug positional (infra-only blanket)
 - up(["a","b"]):            both slugs as positionals after `up`

Pre-fix:  2 failed | 4 passed (6)
Post-fix: 0 failed | 6 passed (6)

The compose-argv assertion is the deterministic proof of behavior
change. Wall-clock timing improvement on concurrent --isolate runs
follows mechanically from the compose semantics: with the fix, slot
N's BuildKit work no longer blocks slot M's because they only rebuild
their respective slug images, not the shared infra stack.

(cherry picked from commit 8a47dbb0d2a2fe72e21c09a824485e1bb5afc463)
This commit is contained in:
Jordan Ritter
2026-06-16 20:07:18 -07:00
parent 97e9ce324a
commit a9114a8317
3 changed files with 136 additions and 10 deletions
+17 -6
View File
@@ -67,12 +67,14 @@ cmd_up() {
local pool_count="${HARNESS_POOL_COUNT:-1}"
export HARNESS_POOL_COUNT="$pool_count"
# Split args: bare slugs → `--profile <slug>`; flags (-*) → pass through.
# `--dev` is intercepted (not passed to compose): it layers the dev overlay
# (docker-compose.dev.yml) which bind-mounts integration source + overrides
# the run command to a hot-reload entrypoint — fast iteration without an
# image rebuild. The built-image mode (no --dev) stays the faithful default.
# Split args: bare slugs → `--profile <slug>` + build target; flags (-*) →
# pass through. `--dev` is intercepted (not passed to compose): it layers
# the dev overlay (docker-compose.dev.yml) which bind-mounts integration
# source + overrides the run command to a hot-reload entrypoint — fast
# iteration without an image rebuild. The built-image mode (no --dev) stays
# the faithful default.
local profile_args=()
local build_targets=()
local passthrough_args=()
local dev_mode=false
local arg
@@ -83,6 +85,7 @@ cmd_up() {
passthrough_args+=("$arg")
else
profile_args+=(--profile "$arg")
build_targets+=("$arg")
fi
done
@@ -93,10 +96,18 @@ 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.
#
# 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[@]}"}
--scale harness-pool-worker="$pool_count" ${passthrough_args[@]+"${passthrough_args[@]}"} \
${build_targets[@]+"${build_targets[@]}"}
}
cmd_down() {
+98 -3
View File
@@ -1,4 +1,4 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
// ---------------------------------------------------------------------------
// Hoisted mocks — vi.hoisted() runs before vi.mock factories so the spies
@@ -19,15 +19,23 @@ vi.mock("node:child_process", () => ({
vi.mock("node:fs", () => {
// stageSharedModules() short-circuits when INTEGRATIONS_DIR is absent.
// readFileSync is consulted by loadPortsFile() to map slugs → host ports
// for health checks; provide stub ports for any slug the up() tests use.
const api = {
existsSync: (...args: unknown[]) => existsSyncMock(...args),
readdirSync: vi.fn(() => []),
readFileSync: vi.fn(() => "{}"),
readFileSync: vi.fn(() =>
JSON.stringify({
"langgraph-python": 10001,
a: 10002,
b: 10003,
}),
),
};
return { default: api, ...api };
});
import { rebuild } from "./lifecycle.js";
import { rebuild, up } from "./lifecycle.js";
/** Pull out the compose argv (after the `-f <file>` prefix) for each call. */
function composeCalls(): string[][] {
@@ -92,3 +100,90 @@ describe("rebuild() — targeted slugs", () => {
expect(recreateCall).toContain("b");
});
});
describe("up() — scoped --build (A21)", () => {
// 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
// captured by composeCalls() BEFORE health checks run.
// To avoid up() throwing before we can assert, mock fetch globally to resolve OK.
const _origFetch = globalThis.fetch;
beforeEach(() => {
globalThis.fetch = vi.fn(() =>
Promise.resolve({ ok: true, status: 200 } as Response),
) as unknown as typeof fetch;
});
afterEach(() => {
globalThis.fetch = _origFetch;
});
/**
* The compose argv shape we want:
* [--profile infra --profile <slug>... up -d --build <slug>...]
*
* 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.
*
* `--profile <name>` arguments do NOT scope the build target on their own;
* docker compose only treats positional args after `up` as the service
* filter for `--build`. So the test asserts the slug appears AFTER the `up`
* keyword (as a positional), not merely anywhere in the argv.
*/
function positionalsAfterUp(argv: string[]): string[] {
const upIdx = argv.indexOf("up");
if (upIdx < 0) return [];
const out: string[] = [];
for (let i = upIdx + 1; i < argv.length; i++) {
const tok = argv[i];
// Skip flags and their values. `-d`/`--build` take no value; `--progress`
// takes a value. Keep it simple — the only flags we emit here are
// `-d`, `--build`, `--progress plain`.
if (tok === "-d" || tok === "--build") continue;
if (tok === "--progress") {
i++; // skip the value
continue;
}
if (tok.startsWith("-")) continue;
out.push(tok);
}
return out;
}
it("scopes --build to the targeted slug (does NOT rebuild infra services)", 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 <slug>` argument
// (profiles don't scope --build).
expect(positionalsAfterUp(upCall!)).toEqual(["langgraph-python"]);
});
it("with no slugs (infra-only), --build remains blanket (first-time bootstrap)", async () => {
await up([]);
const upCall = composeCalls().find(
(argv) => argv.includes("up") && argv.includes("--build"),
);
expect(upCall).toBeDefined();
// 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([]);
});
it("multi-slug: scopes --build to ALL targeted slugs", 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"]);
});
});
+21 -1
View File
@@ -265,7 +265,27 @@ export async function up(
}
const verboseFlag = opts?.verbose ? ["--progress", "plain"] : [];
const args = [...profileArgs, "up", "-d", "--build", ...verboseFlag];
// 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).
//
// 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,
];
log.info("starting services", { slugs: slugs.length ? slugs : ["infra"] });
compose(...args);