fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

The #175 model-mismatch check never runs for opencode spawns without a provisioned worktree — the majority of them #267

New Issue
Closed
opened 2026-09-03 15:10:34 +02:00 by ltms · 1 comment
ltms commented 2026-09-03 15:10:34 +02:00
Owner
Copy Link
Copy Source

Found during the #184 unverified-claim audit, confirmed by me in the code.

What

OpenCodeLauncher.SessionAwareHandle.agentSessionId() is the only place checkModelMatch(...) is called (OpenCodeLauncher.java:835, the sole call site — the method is private).

That call sits after the fleetd #249 gate:

if (!worktreeProvisioned) {
    return null;
}
...
String id = discovery.sessionIdForDirectory(cwd);
...
checkModelMatch(id);   // line 835 — never reached above

So when fleet_spawn is called without worktree:true / worktree:<slug>, the method returns early and the model check never runs. The comment immediately above that gate describes this case as "the ordinary, expected shape of the large majority of spawns (no worktree requested)".

Why it matters

fleetd #175 exists because the xf profile named opencode/x-preview-f-free, that model was withdrawn from the catalogue, and opencode does not fail on an unknown -m — it silently falls back to a default. Every xf member therefore ran gpt-5.6-sol: the paid credential, at the expensive variant, for 97.6% of dev spawns, and outside the credentialId/maxLoad accounting that exists to protect that credential. No log line, no warning.

checkModelMatch is the detector built so that cannot happen again. It does not run for most spawns.

The exposure is the same one: a rotted model name plus a spawn with no worktree. Both are ordinary. A model name rots silently and on its own schedule — nothing in fleetd reports it — so the detector's coverage is the only thing standing between that and another silent paid-credential run.

This is not a false claim, and that is why it survived the audit

The auditing worker correctly did not report this as an unverified-claim finding: the code is honest about it in-line, and says plainly why it returns null. Nothing lies. That is exactly why it needs its own ticket — an accurate comment about a missing check does not make the check present.

The shape worth naming

fleetd #249 added the !worktreeProvisioned gate for a good correctness reason: sessionIdForDirectory is a directory-keyed heuristic, and with a shared cwd it cannot tell this member's row from a sibling's (measured: a three-day-old row from a different profile). Refusing to guess is right.

The collateral is that a safety check rode on the same return value as an identity lookup. Tightening the identity lookup silently narrowed the safety check to a minority of spawns. Neither change was wrong on its own; the coupling is the defect.

Same family as fleetd #258: a gate closes the direction the incident came from, and something else walks through the other way with a green build.

What a fix has to do

The model check does not need a trustworthy session id. It needs to know which model actually ran, which is a different question from "which session row is this member's".

Options, roughly in order of preference:

  1. Decouple them. Give the model check its own path that does not depend on agentSessionId() resolving. If the only available evidence is ambiguous, report the ambiguity — UNKNOWN is already this codebase's answer for absent evidence, and an UNKNOWN that is logged beats a check that silently does not run.
  2. If a shared cwd genuinely makes the model unknowable, then say so once per profile, at WARN, naming the profile — the same treatment discoveryUnavailable already gets a few lines above. Today that case is silent.
  3. Consider whether worktree:true should simply be the default for opencode spawns. That is a bigger behaviour change and belongs in its own discussion, not this ticket.

Whatever the fix, the acceptance criterion is the one that has caught this class of bug here before: a test that drives the real path a spawn takes, not one that calls checkModelMatch directly. A test that calls the seam would have passed every day this gap existed.

Not in scope

Changing the #249 gate itself. It is correct, and this ticket must not weaken it.

Found during the #184 unverified-claim audit, confirmed by me in the code. ## What `OpenCodeLauncher.SessionAwareHandle.agentSessionId()` is the only place `checkModelMatch(...)` is called (`OpenCodeLauncher.java:835`, the sole call site — the method is private). That call sits **after** the fleetd #249 gate: ```java if (!worktreeProvisioned) { return null; } ... String id = discovery.sessionIdForDirectory(cwd); ... checkModelMatch(id); // line 835 — never reached above ``` So when `fleet_spawn` is called without `worktree:true` / `worktree:<slug>`, the method returns early and the model check never runs. The comment immediately above that gate describes this case as *"the ordinary, expected shape of the large majority of spawns (no worktree requested)"*. ## Why it matters fleetd #175 exists because the `xf` profile named `opencode/x-preview-f-free`, that model was withdrawn from the catalogue, and `opencode` does not fail on an unknown `-m` — it silently falls back to a default. Every `xf` member therefore ran `gpt-5.6-sol`: the **paid** credential, at the expensive variant, for 97.6% of dev spawns, and outside the `credentialId`/`maxLoad` accounting that exists to protect that credential. No log line, no warning. `checkModelMatch` is the detector built so that cannot happen again. It does not run for most spawns. The exposure is the same one: a rotted model name plus a spawn with no worktree. Both are ordinary. A model name rots silently and on its own schedule — nothing in fleetd reports it — so the detector's coverage is the only thing standing between that and another silent paid-credential run. ## This is not a false claim, and that is why it survived the audit The auditing worker correctly did **not** report this as an unverified-claim finding: the code is honest about it in-line, and says plainly why it returns null. Nothing lies. That is exactly why it needs its own ticket — an accurate comment about a missing check does not make the check present. ## The shape worth naming fleetd #249 added the `!worktreeProvisioned` gate for a good correctness reason: `sessionIdForDirectory` is a directory-keyed heuristic, and with a shared cwd it cannot tell this member's row from a sibling's (measured: a three-day-old row from a different profile). Refusing to guess is right. The collateral is that a **safety check rode on the same return value as an identity lookup.** Tightening the identity lookup silently narrowed the safety check to a minority of spawns. Neither change was wrong on its own; the coupling is the defect. Same family as fleetd #258: a gate closes the direction the incident came from, and something else walks through the other way with a green build. ## What a fix has to do The model check does not need a *trustworthy* session id. It needs to know **which model actually ran**, which is a different question from "which session row is this member's". Options, roughly in order of preference: 1. Decouple them. Give the model check its own path that does not depend on `agentSessionId()` resolving. If the only available evidence is ambiguous, report the ambiguity — `UNKNOWN` is already this codebase's answer for absent evidence, and an UNKNOWN that is logged beats a check that silently does not run. 2. If a shared cwd genuinely makes the model unknowable, then say so **once per profile**, at WARN, naming the profile — the same treatment `discoveryUnavailable` already gets a few lines above. Today that case is silent. 3. Consider whether `worktree:true` should simply be the default for opencode spawns. That is a bigger behaviour change and belongs in its own discussion, not this ticket. Whatever the fix, the acceptance criterion is the one that has caught this class of bug here before: **a test that drives the real path a spawn takes, not one that calls `checkModelMatch` directly.** A test that calls the seam would have passed every day this gap existed. ## Not in scope Changing the #249 gate itself. It is correct, and this ticket must not weaken it.
ltms commented 2026-09-04 03:49:52 +02:00
Author
Owner
Copy Link
Copy Source

Fixed as 5d75f72 (PR #271). Closing.

The ticket's preferred option was wrong, and the worker was right to refuse it. I asked for the model check to be decoupled from agentSessionId(). It cannot be, safely.

OpenCodeSessionDiscovery exposes exactly two lookups — I checked this myself:

  • sessionIdForDirectory(directory) — the "most recently updated row for this directory" heuristic that #249 exists to distrust
  • actualModelForSessionId(sessionId) — keyed on the resolved id, deliberately, per #234

There is no third route to the model, and nothing in opencode's session table (no terminal or pid column) can disambiguate a shared directory. So decoupling means re-deriving from directory, which reintroduces exactly the false positive #234 removed: a sibling session running a different, correctly-configured model would look like this profile's mismatch, and quarantine an innocent credential.

That is a worse failure than the gap. A false quarantine takes capacity away on bad evidence; the gap only fails to detect. So the fix is option 2 — the silence, which was the real defect, is gone:

opencode model-mismatch check (fleetd #175) cannot run for profile 'X': it was spawned
without a fleetd-provisioned worktree (fleetd #249), so its cwd may be shared with other
sessions and the actual model it is running cannot be safely told apart from a sibling's —
spawn with worktree:true to enable the check for this profile.

Once per profile, and only when a model is actually configured. The #249 gate is untouched — agentSessionId() still returns null and fleetd still never guesses an identity.

Verified myself rather than taken on report. Removing the WARN block turns aSpawnWithoutAProvisionedWorktreeNeverRunsTheModelCheckButWarnsOncePerProfile red: expected: <1> but was: <0>. Restored, git diff --exit-code clean, full build 1271 tests / 0 failures.

The test drives the real path — SessionManager.acquire() plus two get() re-polls — not a direct call to checkModelMatch. That was the acceptance criterion that mattered, because a test calling the seam would have passed on every day this gap existed.

What this does not do. No-worktree opencode spawns still have no model check. They now say so. The remedy an operator has today is worktree:true. Whether that should become the default for opencode spawns is a real question and deliberately out of scope here — it is a behaviour change, and worth its own ticket if the WARN starts showing up often.

Fixed as `5d75f72` (PR #271). Closing. **The ticket's preferred option was wrong, and the worker was right to refuse it.** I asked for the model check to be decoupled from `agentSessionId()`. It cannot be, safely. `OpenCodeSessionDiscovery` exposes exactly two lookups — I checked this myself: - `sessionIdForDirectory(directory)` — the "most recently updated row for this directory" heuristic that #249 exists to distrust - `actualModelForSessionId(sessionId)` — keyed on the **resolved** id, deliberately, per #234 There is no third route to the model, and nothing in opencode's session table (no terminal or pid column) can disambiguate a shared directory. So decoupling means re-deriving from `directory`, which reintroduces exactly the false positive #234 removed: a sibling session running a different, correctly-configured model would look like *this* profile's mismatch, and quarantine an innocent credential. That is a worse failure than the gap. A false quarantine takes capacity away on bad evidence; the gap only fails to detect. So the fix is option 2 — the silence, which was the real defect, is gone: ``` opencode model-mismatch check (fleetd #175) cannot run for profile 'X': it was spawned without a fleetd-provisioned worktree (fleetd #249), so its cwd may be shared with other sessions and the actual model it is running cannot be safely told apart from a sibling's — spawn with worktree:true to enable the check for this profile. ``` Once per profile, and only when a model is actually configured. The #249 gate is untouched — `agentSessionId()` still returns null and fleetd still never guesses an identity. **Verified myself rather than taken on report.** Removing the WARN block turns `aSpawnWithoutAProvisionedWorktreeNeverRunsTheModelCheckButWarnsOncePerProfile` red: `expected: <1> but was: <0>`. Restored, `git diff --exit-code` clean, full build 1271 tests / 0 failures. The test drives the real path — `SessionManager.acquire()` plus two `get()` re-polls — not a direct call to `checkModelMatch`. That was the acceptance criterion that mattered, because a test calling the seam would have passed on every day this gap existed. **What this does not do.** No-worktree opencode spawns still have no model check. They now say so. The remedy an operator has today is `worktree:true`. Whether that should become the default for opencode spawns is a real question and deliberately out of scope here — it is a behaviour change, and worth its own ticket if the WARN starts showing up often.
ltms closed this issue 2026-09-04 03:50:01 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#267
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?