From e4c703a51af990c26333f74acfc313d3f46d795f Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 10 Sep 2026 16:50:39 +0700 Subject: [PATCH] fleetd #444: separate the adapter's fallback default from the decided profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the fixture's StubLauncher fell back to 'sol' too — the same profile place() decides — so an UNSTAMPED request could land on spawnCount('sol') by coincidence, and the assertion's claim that the request 'actually carried sol' was unproven. Dropping the stamping (SpawnRequest routedReq = req) while keeping the routing survived the test unchanged. Fix: give the adapter 'b' as its own fallback default instead, so an unstamped request counts against 'b', not 'sol'. Verified both mutations against the single test in isolation: - drop-stamping (routedReq = req): RED, expected but was - re-entering (return spawn(req.withProfile(decision.profile()))) i.e. M1 from the first round: still RED, PlacementException naming the now-quarantined 'sol' Restored both; full suite green at 1575 tests. No changes to src/main — PeerLauncher's javadoc from the first round is unchanged. --- .../member/CompositePeerLauncherTest.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java index afbe51f..f06d74e 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java @@ -1147,12 +1147,17 @@ class CompositePeerLauncherTest { Map profiles = ordered( "sol", stubWorker("sol", "shared-openai"), "b", stubWorker("b")); - StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "sol", Set.of()); + // The adapter's OWN fallback default is "b", deliberately different from the profile place() + // decides ("sol") — see the note below on why this must not be "sol" too. + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "b", Set.of()); BackendQuarantine quarantine = new BackendQuarantine(() -> 0L, TimeUnit.MINUTES.toNanos(30)); CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "sol", profiles, PlacementPolicies.fixed(), _ -> 0, null, quarantine); // 1. Resolve BEFORE anything is quarantined — sol (definition order first, fixed policy) wins. + // composite's own defaultProfile ("sol", the constructor arg above) never enters this: the + // pool poolFor(DEV) resolves to is never empty here, so place() only ever reads that field as + // a fallback for an empty pool, which this test does not exercise. PlacementDecision decision = composite.place(MemberRole.DEV); assertEquals("sol", decision.profile(), "sanity: nothing is quarantined yet, so sol is placed"); @@ -1167,9 +1172,18 @@ class CompositePeerLauncherTest { assertEquals("sol", handle.profile(), "the decision from place() is honored even though sol is now quarantined"); + // A fixture whose adapter falls back to "sol" too would let an UNSTAMPED request (one + // routed but never given req.withProfile("sol")) land on spawnCount("sol") == 1 by + // COINCIDENCE, since StubLauncher.spawn falls back to its own defaultProfile whenever + // req.profileName() is blank. Giving the adapter "b" as its fallback instead means only an + // actually-stamped request can produce this count — an unstamped one would count against + // "b" and this assertion would fail. assertEquals(1, adapter.spawnCount("sol"), - "the request that reached the delegate actually carried sol as its profile"); - assertEquals(0, adapter.spawnCount("b"), "b must never be touched — the decision named sol"); + "the request that reached the delegate actually carried sol as its profile " + + "(the adapter's own fallback default is 'b', so this can't happen by accident)"); + assertEquals(0, adapter.spawnCount("b"), + "b must never be touched — neither as the decision's profile nor as an unstamped " + + "request's accidental fallback"); } @Test