From fc2e26c0c5502869f61f67a61fad30cc703c0c6e Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Mon, 31 Aug 2026 14:04:41 +0700 Subject: [PATCH] CB-175: quarantine opencode model mismatches --- REPORT-cb175.md | 52 ++++++++++++ .../src/main/java/dev/ltms/fleet/Fleetd.java | 15 +++- .../ltms/fleet/member/OpenCodeLauncher.java | 79 ++++++++++++++++--- .../member/OpenCodeSessionDiscovery.java | 33 ++++++-- .../fleet/placement/BackendQuarantine.java | 17 +++- .../fleet/member/OpenCodeLauncherTest.java | 63 +++++++++++++++ 6 files changed, 239 insertions(+), 20 deletions(-) create mode 100644 REPORT-cb175.md diff --git a/REPORT-cb175.md b/REPORT-cb175.md new file mode 100644 index 0000000..bca3259 --- /dev/null +++ b/REPORT-cb175.md @@ -0,0 +1,52 @@ +# CB-175 report + +## Change + +`OpenCodeLauncher` reads the newest opencode session record for the worker cwd after spawn readiness. +It compares the requested `provider/model` selector with `model.providerID/model.id` from the record. +`variant` is not compared because a profile selector has no variant part. + +An absent record, malformed record, or incomplete model object is unknown evidence. It does not log +an error or quarantine the profile. + +On a real mismatch, fleetd logs an ERROR with the requested and resolved selectors. The mismatch goes +through `ExhaustionSink` into the existing `BackendQuarantine` and uses the profile's +`effectiveCredentialId()`. + +I chose a permanent, process-lifetime quarantine. A withdrawn selector cannot become correct after a +cooldown. A timed retry could silently use the paid fallback again. `fleet_list` will show the usual +quarantine state, with a very large remaining time, until fleetd restarts after an operator fixes the +profile. + +## Tests + +Added tests for an exact match, a mismatch, and missing or unreadable session storage. + +I proved the mismatch test fails without the quarantine call. I commented out the call and ran: + +```text +mvn -Dtest=OpenCodeLauncherTest#differentResolvedModelPermanentlyQuarantinesTheProfile test +``` + +The result was: + +```text +[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 +org.opentest4j.AssertionFailedError: a fallback model must block later spawns ==> expected: but was: +[INFO] BUILD FAILURE +``` + +I restored the call. I then ran `mvn clean install` in `fleetd/` without a pipe. Its result was: + +```text +[INFO] Tests run: 1040, Failures: 0, Errors: 0, Skipped: 0 +[INFO] BUILD SUCCESS +``` + +## Limits and scope + +I could not spawn a real opencode member or restart fleetd. I did not test this end to end against a +live opencode session database. + +I confirmed `ClaudeCodeLauncher` passes `--model` but does not read back the resolved model. I did +not change it because it is outside this ticket's scope. diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index c63aa07..a1cb851 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -168,6 +168,17 @@ public final class Fleetd { claudeProfiles.put(name, w); } }); + // One tracker covers timed backend exhaustion and permanent model-selector mismatches. The + // latter cannot heal on a retry, so OpenCodeLauncher uses quarantinePermanently through the + // sink below rather than letting a cooldown reopen a paid fallback. + BackendQuarantine quarantine = new BackendQuarantine(System::nanoTime, + TimeUnit.SECONDS.toNanos(cfg.quarantineCooldownSeconds())); + ExhaustionSink modelMismatchSink = (profileName, reason) -> { + FleetConfig.Profile profile = config.get().profiles().get(profileName); + if (profile != null) { + quarantine.quarantinePermanently(profile.effectiveCredentialId()); + } + }; List adapters = new ArrayList<>(); // The claude-code adapter is the always-present default; keep it even with no profiles (so a // bridge configured with no workers, or opencode-only, still has a well-defined base adapter) @@ -184,15 +195,13 @@ public final class Fleetd { opencodeProfiles, cfg.effectiveDefaultProfile(), System::getenv, cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), () -> config.get().fleet(), - () -> config.get().memberCredentials())); + () -> config.get().memberCredentials(), modelMismatchSink)); } AtomicReference> liveCountRef = new AtomicReference<>(_ -> 0); // CB-578 stage B: one quarantine tracker for the whole daemon, shared between the launcher // (checked at spawn) and the exhaustion sink wired in below (written on BACKEND_EXHAUSTED). // The cooldown is deferred (see FleetConfig#quarantineCooldownSeconds): it is read once // here, at startup, and a config reload only changes it for a daemon restart. - BackendQuarantine quarantine = new BackendQuarantine(System::nanoTime, - TimeUnit.SECONDS.toNanos(cfg.quarantineCooldownSeconds())); PeerLauncher workers = new CompositePeerLauncher( adapters, cfg.effectiveDefaultProfile(), diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java index 0401d07..dcaab6e 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java @@ -6,6 +6,7 @@ import dev.ltms.fleet.config.FleetConfig; import dev.ltms.fleet.herdr.Agent; import dev.ltms.fleet.herdr.AgentControl; import dev.ltms.fleet.herdr.WorkspaceControl; +import dev.ltms.fleet.inject.ExhaustionSink; import dev.ltms.fleet.peer.Capability; import dev.ltms.fleet.peer.CharterReceipt; import dev.ltms.fleet.peer.PeerHandle; @@ -71,6 +72,9 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { */ private final OpenCodeSessionDiscovery discovery; + /** Receives the profile when a resolved model differs from its requested selector. */ + private final ExhaustionSink modelMismatchSink; + /** * Production constructor — disables the spawn-ready gate ({@code spawnReadyTimeoutMs == 0}) so it * matches the legacy non-blocking spawn semantics. Config dirs are created under the JVM temp dir. @@ -121,7 +125,20 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { Supplier memberCredentials) { this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), - defaultConfigRoot(), defaultDiscoveryRoot(), fleet, memberCredentials); + defaultConfigRoot(), defaultDiscoveryRoot(), fleet, memberCredentials, ExhaustionSink.none()); + } + + /** Production constructor with permanent-quarantine wiring for a model mismatch. */ + public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, long spawnReadyPollMs, + Supplier fleet, + Supplier memberCredentials, + ExhaustionSink modelMismatchSink) { + this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, + System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), + defaultConfigRoot(), defaultDiscoveryRoot(), fleet, memberCredentials, modelMismatchSink); } /** @@ -163,12 +180,10 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { Function env, long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper, - Path configRoot, Path discoveryRoot, - Supplier fleet) { - super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, - spawnReadyTimeoutMs, nowMillis, sleeper, fleet); - this.configRoot = configRoot; - this.discovery = new OpenCodeSessionDiscovery(discoveryRoot); + Path configRoot, Path discoveryRoot, + Supplier fleet) { + this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, sleeper, + configRoot, discoveryRoot, fleet, null, ExhaustionSink.none()); } /** @@ -179,13 +194,32 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { Function env, long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper, - Path configRoot, Path discoveryRoot, - Supplier fleet, - Supplier memberCredentials) { + Path configRoot, Path discoveryRoot, + Supplier fleet, + Supplier memberCredentials) { + this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, sleeper, + configRoot, discoveryRoot, fleet, memberCredentials, ExhaustionSink.none()); + } + + /** + * Full constructor with the model-mismatch quarantine callback. The callback is an + * {@link ExhaustionSink} so model mismatches use the existing quarantine path rather than a + * second state tracker. + */ + public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, + Path configRoot, Path discoveryRoot, + Supplier fleet, + Supplier memberCredentials, + ExhaustionSink modelMismatchSink) { super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials); this.configRoot = configRoot; this.discovery = new OpenCodeSessionDiscovery(discoveryRoot); + this.modelMismatchSink = modelMismatchSink; } private static Path defaultConfigRoot() { @@ -498,9 +532,34 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { @Override public PeerHandle spawn(SpawnRequest req) { PeerHandle inner = super.spawn(req); + verifyResolvedModel(requireProfile(req.profileName()), effectiveCwd(req)); return new SessionAwareHandle(inner, discovery, effectiveCwd(req)); } + /** + * Read the session record once after the spawn-readiness gate. opencode writes the model it + * actually selected there. No record, incomplete model object, or a selector without one slash + * is unknown evidence, so it must not quarantine a working profile. + */ + private void verifyResolvedModel(FleetConfig.Profile cfg, String cwd) { + String[] requested = splitProviderModel(cfg.model()); + if (requested == null) { + return; + } + OpenCodeSessionDiscovery.SessionRecord record = discovery.sessionForDirectory(cwd); + if (record == null || record.providerId() == null || record.modelId() == null) { + return; + } + String actual = record.providerId() + "/" + record.modelId(); + if (cfg.model().equals(actual)) { + return; + } + log.error("opencode model mismatch for profile '{}': requested '{}' but resolved '{}'", + cfg.profile(), cfg.model(), actual); + modelMismatchSink.onExhausted(cfg.profile(), "opencode model mismatch: requested " + + cfg.model() + ", resolved " + actual); + } + /** * A {@link PeerHandle} that delegates everything to the base's worker handle but resolves * {@link #agentSessionId()} lazily through opencode session discovery. Delegate-only, so the diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java index a5ea50f..f2990c2 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java @@ -58,6 +58,15 @@ final class OpenCodeSessionDiscovery { * @return the matching session id, or {@code null} if none is known yet */ String sessionIdForDirectory(String directory) { + SessionRecord record = sessionForDirectory(directory); + return record == null ? null : record.id(); + } + + /** + * The newest session record for {@code directory}, or {@code null} when opencode has not written + * one yet. This is the single storage seam for both session identity and resolved-model checks. + */ + SessionRecord sessionForDirectory(String directory) { if (directory == null || directory.isBlank()) { return null; } @@ -65,20 +74,20 @@ final class OpenCodeSessionDiscovery { if (!Files.isDirectory(sessionRoot)) { return null; } - String best = null; + SessionRecord best = null; long bestMtime = Long.MIN_VALUE; try (Stream projectDirs = Files.list(sessionRoot)) { for (Path projectDir : projectDirs.filter(Files::isDirectory).toList()) { try (Stream records = Files.list(projectDir)) { for (Path record : records.toList()) { - String id = matchId(record, directory); - if (id == null) { + SessionRecord matched = matchRecord(record, directory); + if (matched == null) { continue; } long mtime = lastModifiedEpochMillis(record); if (mtime > bestMtime) { bestMtime = mtime; - best = id; + best = matched; } } } catch (IOException ignored) { @@ -97,7 +106,7 @@ final class OpenCodeSessionDiscovery { * that is not JSON, lacks {@code id}/{@code directory}, or points at a different directory is * simply not our session; a malformed one is skipped, never fatal. */ - private String matchId(Path record, String directory) { + private SessionRecord matchRecord(Path record, String directory) { try { JsonNode node = json.readTree(record.toFile()); JsonNode id = node == null ? null : node.get("id"); @@ -105,12 +114,24 @@ final class OpenCodeSessionDiscovery { if (id == null || dir == null || !directory.equals(dir.asText())) { return null; } - return id.asText(); + JsonNode model = node.path("model"); + String providerId = text(model, "providerID"); + String modelId = text(model, "id"); + return new SessionRecord(id.asText(), providerId, modelId); } catch (IOException e) { return null; } } + private static String text(JsonNode node, String name) { + JsonNode value = node.get(name); + return value == null || value.isNull() || value.asText().isBlank() ? null : value.asText(); + } + + /** The opencode fields fleetd reads from one session record. Null model fields mean unknown. */ + record SessionRecord(String id, String providerId, String modelId) { + } + /** The record's last-modified epoch ms, or {@code Long.MIN_VALUE} if unreadable (never wins). */ private static long lastModifiedEpochMillis(Path record) { try { diff --git a/fleetd/src/main/java/dev/ltms/fleet/placement/BackendQuarantine.java b/fleetd/src/main/java/dev/ltms/fleet/placement/BackendQuarantine.java index 3fb437a..6269925 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/placement/BackendQuarantine.java +++ b/fleetd/src/main/java/dev/ltms/fleet/placement/BackendQuarantine.java @@ -76,6 +76,20 @@ public final class BackendQuarantine { quarantinedUntilNanos.put(credentialId, nowNanos.getAsLong() + cooldownNanos); } + /** + * Quarantine {@code credentialId} until the daemon restarts. This is for a configuration error + * that cannot heal with time, unlike an exhausted backend. A model selector that opencode silently + * resolves to another model stays wrong until an operator changes the profile, so a cooldown would + * start the same unsafe work again. + */ + public void quarantinePermanently(String credentialId) { + Objects.requireNonNull(credentialId, "credentialId"); + if (inert) { + return; + } + quarantinedUntilNanos.put(credentialId, Long.MAX_VALUE); + } + /** Whether {@code credentialId} is quarantined right now. */ public boolean isQuarantined(String credentialId) { return remainingNanos(credentialId) > 0; @@ -110,6 +124,7 @@ public final class BackendQuarantine { } private static long toSecondsRoundedUp(long nanos) { - return (nanos + 999_999_999L) / 1_000_000_000L; + long seconds = nanos / 1_000_000_000L; + return seconds + (nanos % 1_000_000_000L == 0 ? 0 : 1); } } diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java index 6bcc820..71bffc6 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java @@ -12,6 +12,7 @@ import dev.ltms.fleet.peer.MemberRole; import dev.ltms.fleet.peer.PeerHandle; import dev.ltms.fleet.peer.PeerUnreachableException; import dev.ltms.fleet.peer.SpawnRequest; +import dev.ltms.fleet.placement.BackendQuarantine; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -22,6 +23,7 @@ import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import static org.junit.jupiter.api.Assertions.*; @@ -61,6 +63,23 @@ class OpenCodeLauncherTest { 0, System::currentTimeMillis, () -> { }, configRoot, configRoot, null, () -> creds); } + private static OpenCodeLauncher serviceWithModelMismatchSink(FakeHerdr herdr, Path root, + FleetConfig.Profile cfg, + BackendQuarantine quarantine) { + return new OpenCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null, + 0, System::currentTimeMillis, () -> { }, root, root, null, null, + (profile, _) -> quarantine.quarantinePermanently(profile)); + } + + private static void writeResolvedModelRecord(Path root, String directory, String providerId, + String modelId) throws Exception { + Path record = Files.createDirectories(root.resolve("session").resolve("p1")).resolve("ses_a.json"); + Files.writeString(record, "{\"id\":\"ses_a\",\"directory\":\"" + directory + + "\",\"model\":{\"id\":\"" + modelId + "\",\"providerID\":\"" + + providerId + "\",\"variant\":\"high\"}}"); + } + @SuppressWarnings("unchecked") private static Map lastStart(FakeHerdr herdr) { return (Map) herdr.lastCall("agent.start").params(); @@ -211,6 +230,50 @@ class OpenCodeLauncherTest { "--auto is unconditional: a model-less worker still must never block on approval"); } + // --- CB-175: verify opencode's recorded resolved model after spawn readiness ---------------- + + @Test + void matchingResolvedModelDoesNotQuarantineTheProfile(@TempDir Path root) throws Exception { + FleetConfig.Profile cfg = opencodeCfg("opencode/x-preview-f-free", null, null); + BackendQuarantine quarantine = new BackendQuarantine(() -> 0L, TimeUnit.MINUTES.toNanos(1)); + writeResolvedModelRecord(root, root.toString(), "opencode", "x-preview-f-free"); + + serviceWithModelMismatchSink(new FakeHerdr(), root, cfg, quarantine) + .spawn(new SpawnRequest(null, root.toString(), null, null, null, MemberRole.DEV)); + + assertFalse(quarantine.isQuarantined("gemini"), "the exact provider/model match is safe"); + } + + @Test + void differentResolvedModelPermanentlyQuarantinesTheProfile(@TempDir Path root) throws Exception { + FleetConfig.Profile cfg = opencodeCfg("opencode/x-preview-f-free", null, null); + BackendQuarantine quarantine = new BackendQuarantine(() -> 0L, TimeUnit.MINUTES.toNanos(1)); + writeResolvedModelRecord(root, root.toString(), "openai", "gpt-5.6-sol"); + + serviceWithModelMismatchSink(new FakeHerdr(), root, cfg, quarantine) + .spawn(new SpawnRequest(null, root.toString(), null, null, null, MemberRole.DEV)); + + assertTrue(quarantine.isQuarantined("gemini"), "a fallback model must block later spawns"); + assertEquals(Long.MAX_VALUE / 1_000_000_000L + 1, quarantine.remainingSeconds("gemini").orElseThrow(), + "a withdrawn selector cannot become safe after the normal cooldown"); + } + + @Test + void missingOrUnreadableSessionDatabaseDoesNotQuarantine(@TempDir Path root) throws Exception { + FleetConfig.Profile cfg = opencodeCfg("opencode/x-preview-f-free", null, null); + BackendQuarantine missing = new BackendQuarantine(() -> 0L, TimeUnit.MINUTES.toNanos(1)); + serviceWithModelMismatchSink(new FakeHerdr(), root, cfg, missing) + .spawn(new SpawnRequest(null, root.toString(), null, null, null, MemberRole.DEV)); + assertFalse(missing.isQuarantined("gemini"), "a missing database is unknown evidence"); + + Path broken = Files.createDirectories(root.resolve("session").resolve("p1")).resolve("ses_a.json"); + Files.writeString(broken, "not JSON"); + BackendQuarantine unreadable = new BackendQuarantine(() -> 0L, TimeUnit.MINUTES.toNanos(1)); + serviceWithModelMismatchSink(new FakeHerdr(), root, cfg, unreadable) + .spawn(new SpawnRequest(null, root.toString(), null, null, null, MemberRole.DEV)); + assertFalse(unreadable.isQuarantined("gemini"), "an unreadable database is unknown evidence"); + } + // --- CB-617: --agent when the role has an agent-definition file -------------------- @Test -- 2.52.0