CB-175: quarantine opencode model mismatches #203
@@ -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: <true> but was: <false>
|
||||
[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.
|
||||
@@ -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<HerdrPeerLauncher> 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<Function<String, Integer>> 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(),
|
||||
|
||||
@@ -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<FleetConfig.MemberCredentials> 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<String, FleetConfig.Profile> profiles, String defaultProfile,
|
||||
Function<String, String> env,
|
||||
long spawnReadyTimeoutMs, long spawnReadyPollMs,
|
||||
Supplier<FleetConfig.Fleet> fleet,
|
||||
Supplier<FleetConfig.MemberCredentials> 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<String, String> env,
|
||||
long spawnReadyTimeoutMs,
|
||||
LongSupplier nowMillis, Runnable sleeper,
|
||||
Path configRoot, Path discoveryRoot,
|
||||
Supplier<FleetConfig.Fleet> 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<FleetConfig.Fleet> 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<String, String> env,
|
||||
long spawnReadyTimeoutMs,
|
||||
LongSupplier nowMillis, Runnable sleeper,
|
||||
Path configRoot, Path discoveryRoot,
|
||||
Supplier<FleetConfig.Fleet> fleet,
|
||||
Supplier<FleetConfig.MemberCredentials> memberCredentials) {
|
||||
Path configRoot, Path discoveryRoot,
|
||||
Supplier<FleetConfig.Fleet> fleet,
|
||||
Supplier<FleetConfig.MemberCredentials> 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<String, FleetConfig.Profile> profiles, String defaultProfile,
|
||||
Function<String, String> env,
|
||||
long spawnReadyTimeoutMs,
|
||||
LongSupplier nowMillis, Runnable sleeper,
|
||||
Path configRoot, Path discoveryRoot,
|
||||
Supplier<FleetConfig.Fleet> fleet,
|
||||
Supplier<FleetConfig.MemberCredentials> 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
|
||||
|
||||
@@ -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<Path> projectDirs = Files.list(sessionRoot)) {
|
||||
for (Path projectDir : projectDirs.filter(Files::isDirectory).toList()) {
|
||||
try (Stream<Path> 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 {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String, Object> lastStart(FakeHerdr herdr) {
|
||||
return (Map<String, Object>) 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 <role> when the role has an agent-definition file --------------------
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user