fleetd #175: check opencode's actual model against the profile, quarantine on a real mismatch #231

Merged

ltms merged 2 commits from worker/cb175-model-readback-0f085f-1 into main 2026-09-02 13:01:34 +02:00

Conversation 0 Commits 2 Files Changed 5

+594 -10

agent commented 2026-09-02 12:54:45 +02:00

Member

Copy Link

Copy Source

fleetd #175 — opencode model read-back

opencode does not fail on an unknown -m <model> flag — it silently falls back to a default model. The xf profile named a withdrawn model and every spawn actually ran on a paid credential (gpt-5.6-sol) undetected, because BackendQuarantine never saw evidence of what model actually ran. This extends the existing #209 late-resolve path to catch that.

Production diff summary

• OpenCodeSessionDiscovery.java: new ActualModel(provider, id) record and actualModelForDirectory(String directory), reading the session.model JSON column ({"id":...,"providerID":...}) via its OWN independent query/connection — deliberately NOT combined with sessionIdForDirectory's query, so a schema that lacks the model column can never break id resolution (proven by a dedicated test). Absent database, absent row, null/blank column, or unparseable/incomplete JSON all resolve to null (UNKNOWN), never thrown.
• OpenCodeLauncher.java: spawn() now re-resolves the profile config (cheap map lookup, same deterministic name already used by buildLaunch) and wraps the inner handle in SessionAwareHandle with that config plus a new ExhaustionSink field. SessionAwareHandle.agentSessionId() calls a new checkModelMatch() on every invocation (naturally once-only in practice, since the caller stops calling it once the id resolves); it splits the profile's model on the FIRST / into provider+id, compares id always, compares provider only when the profile specified one, and returns silently on UNKNOWN evidence (actualModelForDirectory returned null) or no configured model (claude-code profiles never reach this code at all — SessionAwareHandle only wraps OpenCodeLauncher spawns). On a real mismatch it logs one ERROR naming the profile, the requested model, and the actual model, then calls the existing ExhaustionSink.onExhausted(...) — no new quarantine mechanism. An AtomicBoolean guards against a duplicate report on a later call.
• Fleetd.java: wires a real ExhaustionSink into OpenCodeLauncher, using the same AtomicReference forwarding pattern already used for liveCountRef in this file — needed because the real sink depends on sessions, which depends on workers, which depends on the adapter being constructed. A forwarding sink is handed to the constructor now and pointed at the real one once it exists. Also reworded the sink's log line since it now serves two triggers, not just backend exhaustion.

Build result (verbatim)

$ mvn -o clean install
...
[INFO] Tests run: 1115, Failures: 0, Errors: 0, Skipped: 0
...
[INFO] BUILD SUCCESS

Full unpiped output read start to finish (1827 lines); every module's test class reported Failures: 0, Errors: 0, including OpenCodeLauncherTest (43 tests) and OpenCodeSessionDiscoveryTest (16 tests). FleetHealthMonitorTest's visible stack traces are from a test double literally named AlwaysThrowingFailTarget — intentional retry-logic fixture noise, not a failure.

Tests added, and whether each was watched to fail first

All new tests were run against a temporarily neutered version of the fix (three separate red/green cycles) and confirmed RED before the fix was restored and confirmed GREEN:

1. if (true) return null; at the top of actualModelForDirectory → OpenCodeSessionDiscoveryTest: 16 run, 1 failure + 1 error (the two tests expecting real model data). Reverted → 16/16 green.
2. if (true) return; at the top of checkModelMatch() → OpenCodeLauncherTest: 43 run, 2 failures (the mismatch-detection tests). Reverted → 43/43 green.
3. idMatches = false; forced right before the match check → OpenCodeLauncherTest: 43 run, 3 failures (all three THE TRAP "must be a match" tests correctly caught the false positive). Reverted → 43/43 green.

OpenCodeSessionDiscoveryTest (8 new): parsesTheModelJsonIntoProviderAndId, prefersTheModelOfTheMostRecentlyUpdatedRow, aNonMatchingDirectoryYieldsUnknownModelRatherThanAMismatch, aNullModelColumnYieldsUnknownWithoutThrowing, unparseableModelJsonYieldsUnknownWithoutThrowing, modelJsonMissingIdYieldsUnknown, aMissingDatabaseYieldsUnknownModelWithoutThrowing, aMissingModelColumnYieldsUnknownButIdResolutionStillWorks (proves a schema with NO model column at all still resolves the session id — this is what forced the two-query design).

OpenCodeLauncherTest (8 new): theRealSessionManagerLateResolvePathCatchesAModelMismatch, aProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch, aGxProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch, aBareModelWithNoProviderPrefixMatchesOnIdAloneAndIsNotAMismatch, aRealIdMismatchLogsAnErrorNamingBothModelsAndQuarantinesThroughTheSink, unknownOrUnparseableModelEvidenceNeverQuarantines, aProfileWithNoConfiguredModelIsNeverCheckedForAMismatch.

Acceptance criterion 1 — which test proves the check runs on the REAL late-resolve path

theRealSessionManagerLateResolvePathCatchesAModelMismatch in OpenCodeLauncherTest. It builds a REAL SessionManager wrapping a REAL OpenCodeLauncher (not a mock of either), calls sessions.acquire(...), then sessions.get(paneId) BEFORE any opencode DB row exists (asserts agentSessionId() is still null and nothing is exhausted yet), then writes the session row with a mismatched model, then calls sessions.get(paneId) AGAIN — this second call is what drives SessionManager's existing resolveAgentSessionId re-poll (from #209), which is what calls handle.agentSessionId(), which is where checkModelMatch() lives. Only after that second, genuine re-poll does the test assert the id resolved AND exactly one exhaustion entry was recorded naming both models. This is the exact timing #203 got wrong (its check ran inside spawn(), before the row existed, so it never fired against real data) — this test forces the check through the real gap SessionManager fills by re-polling.

THE TRAP — how each case was handled

profile asks	DB stores	verdict	covered by
openai/gpt-5.6-terra	{"id":"gpt-5.6-terra","providerID":"openai"}	MATCH	aProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch
gx/deepseek-v4-flash	{"id":"deepseek-v4-flash","providerID":"gx"}	MATCH	aGxProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch
deepseek-v4-flash (no /)	{"id":"deepseek-v4-flash","providerID":"gx"}	MATCH (id only, no provider requested)	aBareModelWithNoProviderPrefixMatchesOnIdAloneAndIsNotAMismatch
claude-code profile	no opencode row exists	NOT APPLICABLE	structural — SessionAwareHandle is only ever constructed by OpenCodeLauncher.spawn(); ClaudeCodeLauncher never builds one, so this code path cannot run for a claude-code profile at all

A bare model name (no /) is compared on id alone regardless of which provider opencode actually resolved it to — the profile never asked for a specific provider, so that dimension is not checked. Unknown/unparseable evidence (actualModelForDirectory returning null) always short-circuits before any comparison, covered by unknownOrUnparseableModelEvidenceNeverQuarantines.

Scope note (not fixed, per instruction — one line)

OpenCodeLauncher.buildLaunch's autoCompactWindow is set the same fire-and-forget way (no read-back that it took effect) — same shape as this bug, out of scope, not touched.

Not touched

SessionManager.java required zero changes — the existing unchanged #209 resolveAgentSessionId → handle.agentSessionId() re-poll IS the real late-resolve path this hooks into. The periodic tick (comment at SessionManager.java:591) was left untouched, per instruction.

---

Review round 2 (addressed)

Lead found: parseModel returns a populated ActualModel when the JSON has an id but no providerID (a real shape opencode can write), and checkModelMatch's providerMatches check treated that missing provider as a mismatch for any provider-prefixed profile whose id genuinely matched. That is incomplete evidence being treated as a mismatch, which acceptance rule 4 forbids.

Fix chosen (as recommended): compare the provider only when both sides have one — requestedProvider == null || actual.provider() == null || requestedProvider.equals(actual.provider()). Comment added at the call site explaining why. parseModel was NOT changed to reject a missing providerID — the id is still real evidence and is what actually caught the xf bug.

New tests, both watched to fail first:

1. aMissingProviderIdInTheEvidenceIsUnknownNotAMismatchWhenTheIdMatches — evidence {"id":"gpt-5.6-terra"} (no providerID) against profile openai/gpt-5.6-terra — must NOT quarantine. Run against the pre-fix providerMatches logic: FAILED (Tests run: 2, Failures: 1 — this exact test), confirming the reported bug reproduces. Then the fix was applied and this test went green.
2. aMissingProviderIdInTheEvidenceStillCatchesARealIdMismatch — evidence {"id":"gpt-5.6-sol"} (no providerID) against profile openai/gpt-5.6-terra — must STILL quarantine, since the id genuinely differs. This one already passed before the fix (id comparison was never broken) and still passes after — proves the fix narrows the provider check rather than disabling the whole comparison.

After the fix: both tests green, mvn -o test -Dtest=OpenCodeLauncherTest → Tests run: 45, Failures: 0, Errors: 0 (43 existing + 2 new).

Full build after the fix (verbatim, unpiped, full output read):

$ mvn -o clean install
...
[INFO] Tests run: 1117, Failures: 0, Errors: 0, Skipped: 0
...
[INFO] BUILD SUCCESS

## fleetd #175 — opencode model read-back opencode does not fail on an unknown `-m <model>` flag — it silently falls back to a default model. The `xf` profile named a withdrawn model and every spawn actually ran on a paid credential (`gpt-5.6-sol`) undetected, because `BackendQuarantine` never saw evidence of what model actually ran. This extends the existing #209 late-resolve path to catch that. ### Production diff summary - **`OpenCodeSessionDiscovery.java`**: new `ActualModel(provider, id)` record and `actualModelForDirectory(String directory)`, reading the `session.model` JSON column (`{"id":...,"providerID":...}`) via its OWN independent query/connection — deliberately NOT combined with `sessionIdForDirectory`'s query, so a schema that lacks the `model` column can never break id resolution (proven by a dedicated test). Absent database, absent row, null/blank column, or unparseable/incomplete JSON all resolve to `null` (UNKNOWN), never thrown. - **`OpenCodeLauncher.java`**: `spawn()` now re-resolves the profile config (cheap map lookup, same deterministic name already used by `buildLaunch`) and wraps the inner handle in `SessionAwareHandle` with that config plus a new `ExhaustionSink` field. `SessionAwareHandle.agentSessionId()` calls a new `checkModelMatch()` on every invocation (naturally once-only in practice, since the caller stops calling it once the id resolves); it splits the profile's model on the FIRST `/` into provider+id, compares id always, compares provider only when the profile specified one, and returns silently on UNKNOWN evidence (`actualModelForDirectory` returned `null`) or no configured model (claude-code profiles never reach this code at all — `SessionAwareHandle` only wraps `OpenCodeLauncher` spawns). On a real mismatch it logs one ERROR naming the profile, the requested model, and the actual model, then calls the existing `ExhaustionSink.onExhausted(...)` — no new quarantine mechanism. An `AtomicBoolean` guards against a duplicate report on a later call. - **`Fleetd.java`**: wires a real `ExhaustionSink` into `OpenCodeLauncher`, using the same `AtomicReference` forwarding pattern already used for `liveCountRef` in this file — needed because the real sink depends on `sessions`, which depends on `workers`, which depends on the adapter being constructed. A forwarding sink is handed to the constructor now and pointed at the real one once it exists. Also reworded the sink's log line since it now serves two triggers, not just backend exhaustion. ### Build result (verbatim) ``` $ mvn -o clean install ... [INFO] Tests run: 1115, Failures: 0, Errors: 0, Skipped: 0 ... [INFO] BUILD SUCCESS ``` Full unpiped output read start to finish (1827 lines); every module's test class reported `Failures: 0, Errors: 0`, including `OpenCodeLauncherTest` (43 tests) and `OpenCodeSessionDiscoveryTest` (16 tests). `FleetHealthMonitorTest`'s visible stack traces are from a test double literally named `AlwaysThrowingFailTarget` — intentional retry-logic fixture noise, not a failure. ### Tests added, and whether each was watched to fail first All new tests were run against a temporarily neutered version of the fix (three separate red/green cycles) and confirmed RED before the fix was restored and confirmed GREEN: 1. `if (true) return null;` at the top of `actualModelForDirectory` → `OpenCodeSessionDiscoveryTest`: 16 run, 1 failure + 1 error (the two tests expecting real model data). Reverted → 16/16 green. 2. `if (true) return;` at the top of `checkModelMatch()` → `OpenCodeLauncherTest`: 43 run, 2 failures (the mismatch-detection tests). Reverted → 43/43 green. 3. `idMatches = false;` forced right before the match check → `OpenCodeLauncherTest`: 43 run, 3 failures (all three THE TRAP "must be a match" tests correctly caught the false positive). Reverted → 43/43 green. **`OpenCodeSessionDiscoveryTest`** (8 new): `parsesTheModelJsonIntoProviderAndId`, `prefersTheModelOfTheMostRecentlyUpdatedRow`, `aNonMatchingDirectoryYieldsUnknownModelRatherThanAMismatch`, `aNullModelColumnYieldsUnknownWithoutThrowing`, `unparseableModelJsonYieldsUnknownWithoutThrowing`, `modelJsonMissingIdYieldsUnknown`, `aMissingDatabaseYieldsUnknownModelWithoutThrowing`, `aMissingModelColumnYieldsUnknownButIdResolutionStillWorks` (proves a schema with NO `model` column at all still resolves the session id — this is what forced the two-query design). **`OpenCodeLauncherTest`** (8 new): `theRealSessionManagerLateResolvePathCatchesAModelMismatch`, `aProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch`, `aGxProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch`, `aBareModelWithNoProviderPrefixMatchesOnIdAloneAndIsNotAMismatch`, `aRealIdMismatchLogsAnErrorNamingBothModelsAndQuarantinesThroughTheSink`, `unknownOrUnparseableModelEvidenceNeverQuarantines`, `aProfileWithNoConfiguredModelIsNeverCheckedForAMismatch`. ### Acceptance criterion 1 — which test proves the check runs on the REAL late-resolve path `theRealSessionManagerLateResolvePathCatchesAModelMismatch` in `OpenCodeLauncherTest`. It builds a REAL `SessionManager` wrapping a REAL `OpenCodeLauncher` (not a mock of either), calls `sessions.acquire(...)`, then `sessions.get(paneId)` BEFORE any opencode DB row exists (asserts `agentSessionId()` is still null and nothing is exhausted yet), then writes the session row with a mismatched model, then calls `sessions.get(paneId)` AGAIN — this second call is what drives `SessionManager`'s existing `resolveAgentSessionId` re-poll (from #209), which is what calls `handle.agentSessionId()`, which is where `checkModelMatch()` lives. Only after that second, genuine re-poll does the test assert the id resolved AND exactly one exhaustion entry was recorded naming both models. This is the exact timing #203 got wrong (its check ran inside `spawn()`, before the row existed, so it never fired against real data) — this test forces the check through the real gap `SessionManager` fills by re-polling. ### THE TRAP — how each case was handled | profile asks | DB stores | verdict | covered by | |---|---|---|---| | `openai/gpt-5.6-terra` | `{"id":"gpt-5.6-terra","providerID":"openai"}` | MATCH | `aProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch` | | `gx/deepseek-v4-flash` | `{"id":"deepseek-v4-flash","providerID":"gx"}` | MATCH | `aGxProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch` | | `deepseek-v4-flash` (no `/`) | `{"id":"deepseek-v4-flash","providerID":"gx"}` | MATCH (id only, no provider requested) | `aBareModelWithNoProviderPrefixMatchesOnIdAloneAndIsNotAMismatch` | | claude-code profile | no opencode row exists | NOT APPLICABLE | structural — `SessionAwareHandle` is only ever constructed by `OpenCodeLauncher.spawn()`; `ClaudeCodeLauncher` never builds one, so this code path cannot run for a claude-code profile at all | A bare model name (no `/`) is compared on `id` alone regardless of which provider opencode actually resolved it to — the profile never asked for a specific provider, so that dimension is not checked. Unknown/unparseable evidence (`actualModelForDirectory` returning `null`) always short-circuits before any comparison, covered by `unknownOrUnparseableModelEvidenceNeverQuarantines`. ### Scope note (not fixed, per instruction — one line) `OpenCodeLauncher.buildLaunch`'s `autoCompactWindow` is set the same fire-and-forget way (no read-back that it took effect) — same shape as this bug, out of scope, not touched. ### Not touched `SessionManager.java` required zero changes — the existing unchanged #209 `resolveAgentSessionId` → `handle.agentSessionId()` re-poll IS the real late-resolve path this hooks into. The periodic tick (comment at `SessionManager.java:591`) was left untouched, per instruction. --- ## Review round 2 (addressed) Lead found: `parseModel` returns a populated `ActualModel` when the JSON has an `id` but no `providerID` (a real shape opencode can write), and `checkModelMatch`'s `providerMatches` check treated that missing provider as a mismatch for any provider-prefixed profile whose id genuinely matched. That is incomplete evidence being treated as a mismatch, which acceptance rule 4 forbids. **Fix chosen (as recommended):** compare the provider only when **both** sides have one — `requestedProvider == null || actual.provider() == null || requestedProvider.equals(actual.provider())`. Comment added at the call site explaining why. `parseModel` was NOT changed to reject a missing `providerID` — the `id` is still real evidence and is what actually caught the `xf` bug. **New tests, both watched to fail first:** 1. `aMissingProviderIdInTheEvidenceIsUnknownNotAMismatchWhenTheIdMatches` — evidence `{"id":"gpt-5.6-terra"}` (no `providerID`) against profile `openai/gpt-5.6-terra` — must NOT quarantine. Run against the pre-fix `providerMatches` logic: **FAILED** (`Tests run: 2, Failures: 1` — this exact test), confirming the reported bug reproduces. Then the fix was applied and this test went green. 2. `aMissingProviderIdInTheEvidenceStillCatchesARealIdMismatch` — evidence `{"id":"gpt-5.6-sol"}` (no `providerID`) against profile `openai/gpt-5.6-terra` — must STILL quarantine, since the id genuinely differs. This one already passed before the fix (id comparison was never broken) and still passes after — proves the fix narrows the provider check rather than disabling the whole comparison. After the fix: both tests green, `mvn -o test -Dtest=OpenCodeLauncherTest` → `Tests run: 45, Failures: 0, Errors: 0` (43 existing + 2 new). **Full build after the fix (verbatim, unpiped, full output read):** ``` $ mvn -o clean install ... [INFO] Tests run: 1117, Failures: 0, Errors: 0, Skipped: 0 ... [INFO] BUILD SUCCESS ```

agent added 1 commit 2026-09-02 12:54:46 +02:00

fleetd #175: check opencode's actual model against the profile, quarantine on a real mismatch ... 1178b3f684

opencode does not fail on an unknown -m <model> flag — it silently falls back to a
default model, which can be a paid credential. Extends the existing late-resolve
path (#209's SessionManager -> handle.agentSessionId() re-poll) so that once the
opencode session row exists, OpenCodeSessionDiscovery also reads its `model` JSON
column and OpenCodeLauncher's SessionAwareHandle compares it against the profile's
configured model.

Comparison rule: split the profile's model on the first '/' into provider+id. Compare
id always; compare provider only when the profile specified one. A bare model name
with no '/' matches on id alone. Absent/unparseable evidence is UNKNOWN, never a
mismatch, so a working profile is never quarantined on missing data. A real mismatch
logs an ERROR naming both models and the profile, then quarantines through the
existing ExhaustionSink path (wired via an AtomicReference forwarding sink in
Fleetd.java to break the sessions/workers/adapters construction cycle).

agent added 1 commit 2026-09-02 12:59:12 +02:00

fleetd #175 review: a missing providerID in opencode's model JSON is UNKNOWN, not a mismatch ... 32ebf065ac

parseModel already tolerates a model JSON with an id but no providerID (a real shape
opencode can write). checkModelMatch's providerMatches check did not: a provider-
prefixed profile whose id matched but whose evidence had no providerID was reported
as a mismatch and quarantined on incomplete data, which acceptance rule 4 forbids.

Compare the provider only when BOTH the profile requested one AND the evidence has
one. A genuine id mismatch is still caught either way — narrows the check, does not
disable it.

ltms merged commit 3fbd43fe3f into main 2026-09-02 13:01:34 +02:00

ltms referenced this pull request 2026-09-02 13:06:01 +02:00

fleetd asks opencode for a model and never checks it got that model, so a withdrawn name silently bills a paid credential #175

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

blocked

Cannot start until something else lands. The body says what.

needs-live-proof

Merged and green, but never shown working on the running daemon. Not the same as done.

ready-to-delegate

Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.

silent-default

A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent kevin ltms

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#231