fleet_profiles reports a frozen "default" profile while placement already moved to a new one #425

Closed
opened 2026-09-10 06:46:21 +02:00 by ltms · 0 comments
Owner

Second finding from the #417 reverse-mismatch sweep — a boot-snapshot read of a key ConfigRef calls hot. Same root key as #424, different consumer, different fix. Verified independently here before filing.

The defect

The default profile is captured from the boot snapshot and handed to three launchers:

Fleetd.java:203   claudeProfiles,   cfg.effectiveDefaultProfile(), System::getenv,
Fleetd.java:210   opencodeProfiles, cfg.effectiveDefaultProfile(), System::getenv,
Fleetd.java:229                     cfg.effectiveDefaultProfile(),

cfg is the boot snapshot (Fleetd.java:121). CompositePeerLauncher stores it in a private final String defaultProfile field (:76) and never re-reads it.

effectiveDefaultProfile() is not an independent setting. FleetConfig.java:1643-1645 makes it defaultProfileFor(MemberRole.DEV) — that is, the first entry of fleet.developers, which is a hot role pool.

So the same quantity is computed twice from the same key, once frozen and once live:

source when
what fleet_profiles reports cfg.effectiveDefaultProfile(), frozen at boot never re-read
what an unqualified spawn actually uses CompositePeerLauncher.defaultProfileFor(role) → poolFor(role).getFirst(), read live every spawn

defaultProfileFor at CompositePeerLauncher.java:511-515 only falls back to the frozen field when the live pool is empty. So with a non-empty fleet.developers, the frozen field is not consulted for placement at all — it is consulted only for reporting.

Consequence 1 — the reported default is simply wrong

FleetMcp.java:1150:

result.put("default", workers.defaultProfile() == null ? "" : workers.defaultProfile());

That is the "default" field of fleet_profiles. Reorder fleet.developers so a different profile is first, reload, and:

  • the reload reports success — role pools are documented hot, and for placement they genuinely are;
  • an unqualified fleet_spawn now places on the new first entry;
  • fleet_profiles keeps reporting the old one, for the life of the daemon.

This is #404's shape in the reverse direction: there, a status field read the live config while the behaviour read the snapshot. Here the behaviour reads live and the status field reads the snapshot. Both make the field a claim about something it is not measuring.

It matters more than a normal cosmetic misreport because a documented procedure depends on it. This project's own CLAUDE.md instructs every lead:

fleet_profiles reports the default; check it once per session.

A lead that follows that instruction, on a daemon whose pool was edited since boot, is told the wrong answer by the one call it was told to trust. The failure is quiet — the spawn succeeds, just on a different backend than the lead was led to expect, with different cost and different liveness.

Consequence 2 — the worktree path uses the stale name for real work

SessionManager.java:587-588:

String preResolvedProfile = (profile == null || profile.isBlank())
        ? launcher.defaultProfile() : profile;

For an unqualified, worktree-provisioned spawn, that stale name is then used to:

  • resolve repoRoot via launcher.effectiveCwd(...) (:595), and
  • choose which parity-overlay files are written into the new worktree via launcher.parityOverlay(preResolvedProfile) (:606) — the .mcp.json / opencode.json / .autoenv neutralization.

The spawn a few lines later is unqualified, so it is placed on the live pool's current default, which may be a different profile than preResolvedProfile named.

If the old and new defaults share cwd, kind and overlay needs, this is latent and harmless. If they differ — in particular across kind: claude-code and kind: opencode, whose overlay needs differ — the worktree can be provisioned against the wrong repo root, or given the wrong neutralization overlay for the backend actually running in it. That last one is the #134 hazard the neutralization exists to close: an un-neutralized .mcp.json mounts the primary's IDE and forge servers into a member.

I have not proved consequence 2 fires on any real configuration. It needs a multi-profile fleet.developers whose entries differ in cwd/kind/overlay, and I cannot read the live fleetd.yaml. Treat consequence 1 as the confirmed defect and consequence 2 as a reachable-but-unmeasured risk. The note in SessionManager should say so either way, because the two-value split is real regardless of whether today's config exposes it.

The plain acquire path does not have this bug — it never calls launcher.defaultProfile() before spawning.

Why nothing catches it

CompositePeerLauncherTest explicitly pins the correct, live half — theRolePoolOutranksTheGlobalDefaultProfile at :843-853, "the pool's first entry wins — not the global defaultProfile". So the live behaviour is tested and the reporting accessor beside it is not. No test exercises composite.defaultProfile() across a reload, fleet_profiles's "default" field across a reload, or acquireWithWorktree with a blank profile after a pool change.

That is the same trap as #416: a well-tested correct neighbour makes the untested one next to it look covered.

What is wanted

Goal, not mechanism:

fleet_profiles' "default" must be the profile an unqualified spawn would actually be placed on right now. The obvious reading is that the reporting accessor should resolve through the same live path placement uses, rather than returning a frozen field — but check the role question below before assuming that is a one-line change.

Then, separately: acquireWithWorktree must not resolve a profile name that the spawn it is about to make may not use. Either resolve the profile once and pass that same name into the spawn, or provision the overlay after placement has chosen. Say which you chose and why.

The question this ticket does not answer for you

fleet_profiles reports one default, but the live default is per role — defaultProfileFor(MemberRole). A DEV pool and a REVIEWER pool can legitimately have different first entries. So "the default" is not well defined once role pools are in play. Decide and state your answer: report the DEV default and document that, or report per-role defaults. Do not quietly pick one and leave the field ambiguous — an ambiguous field is how this one went wrong. If you think the field should change shape, say so in your report rather than changing the tool's output silently; a fleet_* tool's shape is instruction surface and its change has to be propagated.

Acceptance

  • A test that reloads a fleet.developers reorder, then asserts fleet_profiles' "default" matches what an unqualified spawn is actually placed on. Assert applied() on the reload so the test proves the reload happened.
  • The mirror: with an empty role pool, the global default is still reported. Without this, a fix that always returns the live pool's first entry would pass on a configuration that has no pool at all.
  • A test for the worktree path: an unqualified acquireWithWorktree after a pool change provisions the overlay for the profile actually spawned.
  • Mutation proof for each: break it on purpose, quote the failing test name and assertion. Green after a mutation means that half is unpinned.
  • If the tool's output shape changes at all, the intent→tool table and any rule naming fleet_profiles in CLAUDE.md get updated in the same PR, and the canonical block stays byte-identical with the wiki template.

Out of scope

  • HerdrPeerLauncher's own per-adapter defaultProfile field is the same frozen shape but is dead in production: workers is always the CompositePeerLauncher (Fleetd.java:227), which always routes spawn() to its delegates with an explicit, live-resolved profile name. Do not chase it here. If you want it gone, that is a separate cleanup ticket, and it needs the "is it really unreachable" argument written down.
  • The MemberRegistry architect-slot freeze is #424. Same key, different consumer — do not fix both in one PR.

Found by the #417 reverse-mismatch sweep. Verified here: Fleetd.java:121/203/210/229, FleetConfig.java:1643-1645, CompositePeerLauncher.java:76/511-515, FleetMcp.java:1150, SessionManager.java:587-588/595/606, and CompositePeerLauncherTest:843-853.

Second finding from the #417 reverse-mismatch sweep — a boot-snapshot read of a key `ConfigRef` calls **hot**. Same root key as #424, different consumer, different fix. Verified independently here before filing. ## The defect The default profile is captured from the boot snapshot and handed to three launchers: ``` Fleetd.java:203 claudeProfiles, cfg.effectiveDefaultProfile(), System::getenv, Fleetd.java:210 opencodeProfiles, cfg.effectiveDefaultProfile(), System::getenv, Fleetd.java:229 cfg.effectiveDefaultProfile(), ``` `cfg` is the boot snapshot (`Fleetd.java:121`). `CompositePeerLauncher` stores it in a `private final String defaultProfile` field (`:76`) and never re-reads it. `effectiveDefaultProfile()` is not an independent setting. `FleetConfig.java:1643-1645` makes it `defaultProfileFor(MemberRole.DEV)` — that is, **the first entry of `fleet.developers`**, which is a hot role pool. So the same quantity is computed twice from the same key, once frozen and once live: | | source | when | |---|---|---| | what `fleet_profiles` reports | `cfg.effectiveDefaultProfile()`, frozen at boot | never re-read | | what an unqualified spawn actually uses | `CompositePeerLauncher.defaultProfileFor(role)` → `poolFor(role).getFirst()`, read live | every spawn | `defaultProfileFor` at `CompositePeerLauncher.java:511-515` only falls back to the frozen field when the live pool is **empty**. So with a non-empty `fleet.developers`, the frozen field is not consulted for placement at all — it is consulted only for reporting. ## Consequence 1 — the reported default is simply wrong `FleetMcp.java:1150`: ```java result.put("default", workers.defaultProfile() == null ? "" : workers.defaultProfile()); ``` That is the `"default"` field of `fleet_profiles`. Reorder `fleet.developers` so a different profile is first, reload, and: - the reload reports success — role pools are documented hot, and for placement they genuinely are; - an unqualified `fleet_spawn` now places on the **new** first entry; - `fleet_profiles` keeps reporting the **old** one, for the life of the daemon. This is #404's shape in the reverse direction: there, a status field read the live config while the behaviour read the snapshot. Here the behaviour reads live and the status field reads the snapshot. Both make the field a claim about something it is not measuring. It matters more than a normal cosmetic misreport because a documented procedure depends on it. This project's own `CLAUDE.md` instructs every lead: > `fleet_profiles` reports the default; check it once per session. A lead that follows that instruction, on a daemon whose pool was edited since boot, is told the wrong answer by the one call it was told to trust. The failure is quiet — the spawn succeeds, just on a different backend than the lead was led to expect, with different cost and different liveness. ## Consequence 2 — the worktree path uses the stale name for real work `SessionManager.java:587-588`: ```java String preResolvedProfile = (profile == null || profile.isBlank()) ? launcher.defaultProfile() : profile; ``` For an **unqualified, worktree-provisioned** spawn, that stale name is then used to: - resolve `repoRoot` via `launcher.effectiveCwd(...)` (`:595`), and - choose which parity-overlay files are written into the new worktree via `launcher.parityOverlay(preResolvedProfile)` (`:606`) — the `.mcp.json` / `opencode.json` / `.autoenv` neutralization. The spawn a few lines later is unqualified, so it is placed on the **live** pool's current default, which may be a different profile than `preResolvedProfile` named. If the old and new defaults share `cwd`, `kind` and overlay needs, this is latent and harmless. If they differ — in particular across `kind: claude-code` and `kind: opencode`, whose overlay needs differ — the worktree can be provisioned against the wrong repo root, or given the wrong neutralization overlay for the backend actually running in it. That last one is the #134 hazard the neutralization exists to close: an un-neutralized `.mcp.json` mounts the primary's IDE and forge servers into a member. I have **not** proved consequence 2 fires on any real configuration. It needs a multi-profile `fleet.developers` whose entries differ in `cwd`/`kind`/overlay, and I cannot read the live `fleetd.yaml`. Treat consequence 1 as the confirmed defect and consequence 2 as a reachable-but-unmeasured risk. The note in `SessionManager` should say so either way, because the two-value split is real regardless of whether today's config exposes it. The plain `acquire` path does **not** have this bug — it never calls `launcher.defaultProfile()` before spawning. ## Why nothing catches it `CompositePeerLauncherTest` explicitly pins the *correct*, live half — `theRolePoolOutranksTheGlobalDefaultProfile` at `:843-853`, "the pool's first entry wins — not the global defaultProfile". So the live behaviour is tested and the reporting accessor beside it is not. No test exercises `composite.defaultProfile()` across a reload, `fleet_profiles`'s `"default"` field across a reload, or `acquireWithWorktree` with a blank profile after a pool change. That is the same trap as #416: a well-tested correct neighbour makes the untested one next to it look covered. ## What is wanted Goal, not mechanism: **`fleet_profiles`' `"default"` must be the profile an unqualified spawn would actually be placed on right now.** The obvious reading is that the reporting accessor should resolve through the same live path placement uses, rather than returning a frozen field — but check the role question below before assuming that is a one-line change. Then, separately: **`acquireWithWorktree` must not resolve a profile name that the spawn it is about to make may not use.** Either resolve the profile once and pass that same name into the spawn, or provision the overlay after placement has chosen. Say which you chose and why. ### The question this ticket does not answer for you `fleet_profiles` reports **one** default, but the live default is **per role** — `defaultProfileFor(MemberRole)`. A DEV pool and a REVIEWER pool can legitimately have different first entries. So "the default" is not well defined once role pools are in play. Decide and state your answer: report the DEV default and document that, or report per-role defaults. Do not quietly pick one and leave the field ambiguous — an ambiguous field is how this one went wrong. If you think the field should change shape, say so in your report rather than changing the tool's output silently; a `fleet_*` tool's shape is instruction surface and its change has to be propagated. ## Acceptance - A test that reloads a `fleet.developers` reorder, then asserts `fleet_profiles`' `"default"` matches what an unqualified spawn is actually placed on. Assert `applied()` on the reload so the test proves the reload happened. - The mirror: with an **empty** role pool, the global default is still reported. Without this, a fix that always returns the live pool's first entry would pass on a configuration that has no pool at all. - A test for the worktree path: an unqualified `acquireWithWorktree` after a pool change provisions the overlay for the profile actually spawned. - Mutation proof for each: break it on purpose, quote the failing test name and assertion. Green after a mutation means that half is unpinned. - If the tool's output shape changes at all, the intent→tool table and any rule naming `fleet_profiles` in `CLAUDE.md` get updated in the same PR, and the canonical block stays byte-identical with the wiki template. ## Out of scope - `HerdrPeerLauncher`'s own per-adapter `defaultProfile` field is the same frozen shape but is **dead in production**: `workers` is always the `CompositePeerLauncher` (`Fleetd.java:227`), which always routes `spawn()` to its delegates with an explicit, live-resolved profile name. Do not chase it here. If you want it gone, that is a separate cleanup ticket, and it needs the "is it really unreachable" argument written down. - The `MemberRegistry` architect-slot freeze is #424. Same key, different consumer — do not fix both in one PR. Found by the #417 reverse-mismatch sweep. Verified here: `Fleetd.java:121/203/210/229`, `FleetConfig.java:1643-1645`, `CompositePeerLauncher.java:76/511-515`, `FleetMcp.java:1150`, `SessionManager.java:587-588/595/606`, and `CompositePeerLauncherTest:843-853`.
ltms closed this issue 2026-09-10 09:35:44 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#425