#770 made a lead's tab: optional, which silently disarms validatePanePlacementAgainstLeadTabs #775

Closed
opened 2026-10-05 14:00:20 +02:00 by ltms · 2 comments
Owner

Found while writing the Features entry for #770, by re-reading the existing entry "Startup refuses placement: pane while a lead names a tab". Latent on this host, not live. Reachable by exactly the config edit #770 tells operators to make.

The guard

FleetConfig.validatePanePlacementAgainstLeadTabs() refuses startup when a profile places members by pane while a lead or collaborator names a tab. Its trigger:

boolean anyLeaderHasTab = fleet.leaders().values().stream()
        .anyMatch(leader -> leader != null && leader.tab() != null && !leader.tab().isBlank());
boolean anyCollaboratorHasTab = ...;
if (!anyLeaderHasTab && !anyCollaboratorHasTab) {
    return;                       // ← disarmed
}

Why #770 breaks it

Before #770, tab: was required on every lead, so anyLeaderHasTab was true whenever any lead existed. The trigger was a reliable proxy for "a lead has a labelled tab".

After #770, tab: is optional and deprecated, and the lead tab is the fixed constant Leader.LEAD_TAB_LABEL = "lead". So a lead still has a labelled tab — but the field the guard reads is gone. The proxy stopped tracking the thing it stood for.

Config that disarms it, which is the config #770 recommends:

fleet:
  leaders:
    opus:
      profile: opus
      workspace: fleet     # no tab: — this is the new recommended form

What it costs

The guard's own refusal message says what it protects:

a pane-placed member can land inside that labelled tab, and while its pane carries no entry in the spawned-member roster, it is read back as the lead or collaborator and granted that identity's authority

A pane-placed member splits the currently focused tab, so where it lands depends on what the operator was looking at. Landing in the lead tab grants that member fleet_spawn, fleet_stop, fleet_drain and fleet_handover over the whole fleet, intermittently and by accident of focus.

Worse than an explicit placement: pane

public boolean tabPlacement() {
    return "tab".equals(placement);
}

placement unset is therefore not tab placement. So after the tab: key is dropped, a profile that merely omits placement: is a pane-placer with no guard watching. It does not take a deliberate placement: pane.

Reachability here — latent, measured 2026-10-05

awk over fleetd.yaml profiles → local, local-direct, gx, opus, sonnet, sol, terra, xf  ALL placement: tab
fleet.leaders.opus.tab → "lead: opus"   (still set, so the guard still fires today)

So nothing is broken in the running fleet. It is two config edits away, and one of them is the tab: deletion #770 asks for. Do not delete tab: from fleetd.yaml until this lands.

Fix

Make the trigger read the thing, not the proxy. Every lead now has a labelled tab by construction, so:

boolean anyLeaderHasTab = !fleet.leaders().isEmpty();

That is stronger and simpler than the current test, and it cannot drift again when another field changes. The refusal message needs a matching reword: it currently says "or remove the tab from every fleet.leaders and fleet.collaborators entry", and removing tab: is no longer a way to satisfy it for a lead — only placement: tab is.

The lesson, not the instance

A guard whose trigger is a proxy for the condition it protects goes quiet when the proxy changes, with a green build and no refusal to notice. This one had no test that configured a lead without tab: and a pane-placed profile, because before #770 that config could not exist.

The same entry predicted this shape from a different direction: "a new tab-attached role must widen this validator in the same change, because a guard written for one registry is silent about the next one." Here it was not a new registry. It was the same registry losing the field the guard read. So the rule is wider than it was written: any change to what makes a tab a lead's tab must re-check this validator.

Related: #770 (cause), #661 / PR #667 (the guard).

Found while writing the Features entry for #770, by re-reading the existing entry *"Startup refuses `placement: pane` while a lead names a tab"*. Latent on this host, not live. Reachable by exactly the config edit #770 tells operators to make. ## The guard `FleetConfig.validatePanePlacementAgainstLeadTabs()` refuses startup when a profile places members by pane while a lead or collaborator names a tab. Its trigger: ```java boolean anyLeaderHasTab = fleet.leaders().values().stream() .anyMatch(leader -> leader != null && leader.tab() != null && !leader.tab().isBlank()); boolean anyCollaboratorHasTab = ...; if (!anyLeaderHasTab && !anyCollaboratorHasTab) { return; // ← disarmed } ``` ## Why #770 breaks it Before #770, `tab:` was **required** on every lead, so `anyLeaderHasTab` was true whenever any lead existed. The trigger was a reliable proxy for "a lead has a labelled tab". After #770, `tab:` is optional and deprecated, and the lead tab is the fixed constant `Leader.LEAD_TAB_LABEL = "lead"`. So a lead still has a labelled tab — but the field the guard reads is gone. The proxy stopped tracking the thing it stood for. Config that disarms it, which is the config #770 recommends: ```yaml fleet: leaders: opus: profile: opus workspace: fleet # no tab: — this is the new recommended form ``` ## What it costs The guard's own refusal message says what it protects: > a pane-placed member can land inside that labelled tab, and while its pane carries no entry in the spawned-member roster, it is read back as the lead or collaborator and granted that identity's authority A pane-placed member splits the **currently focused** tab, so where it lands depends on what the operator was looking at. Landing in the `lead` tab grants that member `fleet_spawn`, `fleet_stop`, `fleet_drain` and `fleet_handover` over the whole fleet, intermittently and by accident of focus. ## Worse than an explicit `placement: pane` ```java public boolean tabPlacement() { return "tab".equals(placement); } ``` `placement` **unset** is therefore not tab placement. So after the `tab:` key is dropped, a profile that merely omits `placement:` is a pane-placer with no guard watching. It does not take a deliberate `placement: pane`. ## Reachability here — latent, measured 2026-10-05 ``` awk over fleetd.yaml profiles → local, local-direct, gx, opus, sonnet, sol, terra, xf ALL placement: tab fleet.leaders.opus.tab → "lead: opus" (still set, so the guard still fires today) ``` So nothing is broken in the running fleet. It is two config edits away, and one of them is the `tab:` deletion #770 asks for. **Do not delete `tab:` from `fleetd.yaml` until this lands.** ## Fix Make the trigger read the thing, not the proxy. Every lead now has a labelled tab by construction, so: ```java boolean anyLeaderHasTab = !fleet.leaders().isEmpty(); ``` That is stronger and simpler than the current test, and it cannot drift again when another field changes. The refusal message needs a matching reword: it currently says "or remove the tab from every fleet.leaders and fleet.collaborators entry", and removing `tab:` is no longer a way to satisfy it for a lead — only `placement: tab` is. ## The lesson, not the instance A guard whose trigger is a *proxy* for the condition it protects goes quiet when the proxy changes, with a green build and no refusal to notice. This one had no test that configured a lead without `tab:` and a pane-placed profile, because before #770 that config could not exist. The same entry predicted this shape from a different direction: *"a new tab-attached role must widen this validator in the same change, because a guard written for one registry is silent about the next one."* Here it was not a new registry. It was the same registry losing the field the guard read. **So the rule is wider than it was written: any change to what makes a tab a lead's tab must re-check this validator.** Related: #770 (cause), #661 / PR #667 (the guard).
Author
Owner

CORRECTION TO THE BRIEF — read this before you commit

The worker on this unit was right and my brief was wrong. It raised the point as a fleet_ask, the ~55s window expired before my answer landed, and a send cannot reach it while it is working. So the correction is here, which is the channel that does reach it. This supersedes the brief on the one point below; everything else in the brief stands.

What my brief got backwards

I wrote that Profile.tabPlacement() is "tab".equals(placement), so "a profile with no placement: key at all is already not tab placement". That is the opposite of what the code does. Measured, not recalled:

FleetConfig.java:525   placement = (placement == null || placement.isBlank()) ? "tab" : placement.toLowerCase();
FleetConfig.java:846   public boolean tabPlacement() { return "tab".equals(placement); }

An unset placement: defaults to "tab", so tabPlacement() returns true and such a profile is tab-placed — safe, and the guard is right not to fire on it. git log -S dates that line at 2e138a1 (2026-08-25, CB-634).

This narrows the hazard, it does not remove it. The defect needs a profile with an explicit placement: pane, not merely an omitted key. The live config has none — all eight profiles set placement: tab — so #775 stays latent today, as the ticket already said.

One route I had left implicit is closed, and nothing here needs to cover it: rejectUnknownPlacement(yaml) is called unconditionally at FleetConfig.java:1956, before the record is parsed, and refuses any placement: outside {tab, pane}. So a typo like placement: tabb is refused at load whatever the leads configure.

Criterion #2 is replaced by two tests

2a — the real defect. Red before the fix, green after.
A lead with no tab: at all plus a profile with an explicit placement: pane must be refused. Today anyLeaderHasTab is false, the method returns early, and that config starts. That is the whole of #775.

2b — a positive control pinning the default I got wrong.
A lead with no tab: plus a profile with no placement: key must be allowed. Assert the allowed outcome, name the test so the reason is visible, and give the assertion a one-line message: an unset placement defaults to tab, so the profile is tab-placed and nothing should fire.

2b is worth more than the test it replaces. It makes the fact the worker found into a build check: if someone later changes that default from tab to pane, 2b goes red and names the reason, instead of this hole widening again in silence.

The refusal message is a hard criterion, not a nicety

The current text ends "or remove the tab from every fleet.leaders and fleet.collaborators entry." After #770 that advice is actively harmful for a lead. Removing tab: is not an escape from the guard — it is what disarms the guard, and the lead's tab is still labelled lead afterwards. The message must offer placement: tab as the only remedy for a lead, and may keep the remove-the-tab remedy for a collaborator, where it is still true. Say which half applies to which.

Unchanged

The fix is still boolean anyLeaderHasTab = !fleet.leaders().isEmpty();. Scope is still that one method, its message and its tests. Do not touch Profile's placement default — the worker was right to refuse that as out of scope.

Record the premise error in your fleet_reply: what the brief claimed, what the code does, and which criterion you replaced. I want it in the record rather than hidden inside a passing test.

The lesson, which outlives this ticket

This is the second time in two tickets that a claim of mine about placement was wrong in the same direction — the first was the IT-suffix regex, this is the default. Both came from reading a method body ("tab".equals(placement)) without reading the constructor that feeds it. An accessor does not tell you the field's domain; the compact constructor does. For a record, the defaulting is the contract, and it lives somewhere else in the file.

## CORRECTION TO THE BRIEF — read this before you commit **The worker on this unit was right and my brief was wrong.** It raised the point as a `fleet_ask`, the ~55s window expired before my answer landed, and a send cannot reach it while it is working. So the correction is here, which is the channel that does reach it. This supersedes the brief on the one point below; everything else in the brief stands. ### What my brief got backwards I wrote that `Profile.tabPlacement()` is `"tab".equals(placement)`, so *"a profile with no `placement:` key at all is already not tab placement"*. That is the opposite of what the code does. Measured, not recalled: ``` FleetConfig.java:525 placement = (placement == null || placement.isBlank()) ? "tab" : placement.toLowerCase(); FleetConfig.java:846 public boolean tabPlacement() { return "tab".equals(placement); } ``` An **unset** `placement:` defaults to `"tab"`, so `tabPlacement()` returns `true` and such a profile is tab-placed — safe, and the guard is right not to fire on it. `git log -S` dates that line at `2e138a1` (2026-08-25, CB-634). **This narrows the hazard, it does not remove it.** The defect needs a profile with an **explicit** `placement: pane`, not merely an omitted key. The live config has none — all eight profiles set `placement: tab` — so #775 stays latent today, as the ticket already said. One route I had left implicit is closed, and nothing here needs to cover it: `rejectUnknownPlacement(yaml)` is called **unconditionally** at `FleetConfig.java:1956`, before the record is parsed, and refuses any `placement:` outside `{tab, pane}`. So a typo like `placement: tabb` is refused at load whatever the leads configure. ### Criterion #2 is replaced by two tests **2a — the real defect. Red before the fix, green after.** A lead with **no `tab:` at all** plus a profile with an **explicit `placement: pane`** must be refused. Today `anyLeaderHasTab` is false, the method returns early, and that config starts. That is the whole of #775. **2b — a positive control pinning the default I got wrong.** A lead with no `tab:` plus a profile with **no `placement:` key** must be **allowed**. Assert the allowed outcome, name the test so the reason is visible, and give the assertion a one-line message: an unset `placement` defaults to `tab`, so the profile is tab-placed and nothing should fire. 2b is worth more than the test it replaces. It makes the fact the worker found into a build check: if someone later changes that default from `tab` to `pane`, 2b goes red and names the reason, instead of this hole widening again in silence. ### The refusal message is a hard criterion, not a nicety The current text ends *"or remove the tab from every fleet.leaders and fleet.collaborators entry."* After #770 that advice is actively harmful for a lead. Removing `tab:` is not an escape from the guard — it is what **disarms** the guard, and the lead's tab is still labelled `lead` afterwards. The message must offer `placement: tab` as the only remedy for a lead, and may keep the remove-the-tab remedy for a **collaborator**, where it is still true. Say which half applies to which. ### Unchanged The fix is still `boolean anyLeaderHasTab = !fleet.leaders().isEmpty();`. Scope is still that one method, its message and its tests. **Do not touch `Profile`'s placement default** — the worker was right to refuse that as out of scope. Record the premise error in your `fleet_reply`: what the brief claimed, what the code does, and which criterion you replaced. I want it in the record rather than hidden inside a passing test. ### The lesson, which outlives this ticket This is the second time in two tickets that a claim of mine about `placement` was wrong in the same direction — the first was the `IT`-suffix regex, this is the default. Both came from reading a method body (`"tab".equals(placement)`) without reading the constructor that feeds it. **An accessor does not tell you the field's domain; the compact constructor does.** For a record, the defaulting is the contract, and it lives somewhere else in the file.
Author
Owner

Fixed, merged and live — b314cb4 + fc786d0 + c043d14

main is c043d14. Daemon redeployed: pid 26683, jar 72801148bc3b, fleetd listening 14:23:25, no ERROR lines. fleet_whoami still primary.

The lead half of the trigger now reads the registry:

boolean anyLead = !fleet.leaders().isEmpty();

A configured lead is a labelled tab, whether or not it says so in YAML. The collaborator half still reads its own tab:, which stays correct — a collaborator with no tab feeds nothing into LeadTabScanner.

The refusal message no longer sends the operator in a circle. It now says placement: tab is the only fix when a lead triggered it, and keeps the remove-the-tab remedy for a collaborator, where that remedy still works.

Verification I did myself

I re-ran the red-before claim rather than trusting it. Reverting only the trigger line in a throwaway worktree:

Tests run: 3, Failures: 1
aPanePlacedProfileWithNoLeadTabRefusesToStart
  AssertionFailedError: Expected java.lang.IllegalStateException to be thrown, but nothing was thrown.

Full build MVN_EXIT=0, 2175 tests / 0 failures, confirmed from Maven's own line and from my own sum over 179 surefire XML files. @Test delta +2 (172 → 174), reconciling 2173 → 2175. Tree parity between the rebased branch and the merge I tested: dfe2c1c7… on both sides.

The guard only fires on an invalid config and I cannot write fleetd.yaml, so rather than claim a live refusal I proved the running jar holds the new code, with controls either side:

CONTROL  'refusing to start: profile'           PRESENT
NEW      'the only fix when a lead triggered'   PRESENT
OLD      'remove the tab from every fleet...'   absent
NEGATIVE CONTROL (must not exist)               absent

My brief was wrong, and the worker caught it

I wrote that a profile with no placement: key is already not tab-placed. The opposite is true — FleetConfig.java:525 defaults an unset placement to "tab". I had read the accessor tabPlacement() and never the compact constructor 321 lines above it.

So this hazard always needed an explicit placement: pane. Real, and narrower than I filed it.

The worker refused to write a test to a premise it could see was false, raised a fleet_ask, got no answer inside the ~55s window, proceeded on its own judgment, and then found my ticket correction on the mandatory re-read and matched it. That is the turn contract doing exactly its job, and it is the reason this ticket has a correct positive-control test instead of a wrong one.

That control is the lasting part: a test now asserts that a profile with no placement: key is allowed, naming the default as the reason. If the default ever changes, it fails loudly instead of reopening this hole in silence.

One commit is mine, not the worker's

c043d14 renames anyLeaderHasTab to anyLead. The flag tested leader.tab() and was named for it; it now tests whether any lead exists at all. My correction comment handed the worker that stale name verbatim, so the fix is mine. Rebuilt green.

Unblocked: tab: can now be deleted

This was the only thing blocking #770 step 2. tab: "lead: opus" can come out of fleetd.yaml whenever the operator wants, and the legacy-label branch in Leader.acceptedLabels() can be removed after that. The rename of tab w2:tY to lead is independent and also safe at any time.

Docs

wiki/11-Features.md (2012fff): the entry's What now describes the registry trigger and the role-split remedy; the "trigger is disarmed" gotcha is replaced by the fix and the two tests; and the "do not delete tab:" blocker is cleared from the #770 entry. CLAUDE.md ↔ wiki sync check: in sync: True.

Lesson for the next ticket

An accessor shows you a comparison, not a field's domain. For a Java record the compact constructor holds every default and normalization, so the constructor is the contract. A one-line getter looks like the whole truth precisely because there is nothing in it to warn you. Read the constructor before asserting what a field can hold — especially when the claim is in the negative ("if it is not set, then…"), because that is a claim about the default.

## Fixed, merged and live — `b314cb4` + `fc786d0` + `c043d14` `main` is `c043d14`. Daemon redeployed: pid 26683, jar `72801148bc3b`, `fleetd listening` 14:23:25, no ERROR lines. `fleet_whoami` still `primary`. The lead half of the trigger now reads the registry: ```java boolean anyLead = !fleet.leaders().isEmpty(); ``` A configured lead **is** a labelled tab, whether or not it says so in YAML. The collaborator half still reads its own `tab:`, which stays correct — a collaborator with no tab feeds nothing into `LeadTabScanner`. The refusal message no longer sends the operator in a circle. It now says `placement: tab` is the only fix when a lead triggered it, and keeps the remove-the-tab remedy for a collaborator, where that remedy still works. ### Verification I did myself I re-ran the red-before claim rather than trusting it. Reverting only the trigger line in a throwaway worktree: ``` Tests run: 3, Failures: 1 aPanePlacedProfileWithNoLeadTabRefusesToStart AssertionFailedError: Expected java.lang.IllegalStateException to be thrown, but nothing was thrown. ``` Full build `MVN_EXIT=0`, 2175 tests / 0 failures, confirmed from Maven's own line and from my own sum over 179 surefire XML files. `@Test` delta `+2` (172 → 174), reconciling 2173 → 2175. Tree parity between the rebased branch and the merge I tested: `dfe2c1c7…` on both sides. The guard only fires on an invalid config and I cannot write `fleetd.yaml`, so rather than claim a live refusal I proved the running jar holds the new code, with controls either side: ``` CONTROL 'refusing to start: profile' PRESENT NEW 'the only fix when a lead triggered' PRESENT OLD 'remove the tab from every fleet...' absent NEGATIVE CONTROL (must not exist) absent ``` ### My brief was wrong, and the worker caught it I wrote that a profile with no `placement:` key is already not tab-placed. The opposite is true — `FleetConfig.java:525` defaults an unset `placement` **to** `"tab"`. I had read the accessor `tabPlacement()` and never the compact constructor 321 lines above it. So this hazard always needed an **explicit** `placement: pane`. Real, and narrower than I filed it. The worker refused to write a test to a premise it could see was false, raised a `fleet_ask`, got no answer inside the ~55s window, proceeded on its own judgment, and then found my ticket correction on the mandatory re-read and matched it. That is the turn contract doing exactly its job, and it is the reason this ticket has a correct positive-control test instead of a wrong one. That control is the lasting part: a test now asserts that a profile with no `placement:` key is *allowed*, naming the default as the reason. If the default ever changes, it fails loudly instead of reopening this hole in silence. ### One commit is mine, not the worker's `c043d14` renames `anyLeaderHasTab` to `anyLead`. The flag tested `leader.tab()` and was named for it; it now tests whether any lead exists at all. **My correction comment handed the worker that stale name verbatim**, so the fix is mine. Rebuilt green. ### Unblocked: `tab:` can now be deleted This was the only thing blocking #770 step 2. `tab: "lead: opus"` can come out of `fleetd.yaml` whenever the operator wants, and the legacy-label branch in `Leader.acceptedLabels()` can be removed after that. The rename of tab `w2:tY` to `lead` is independent and also safe at any time. ### Docs `wiki/11-Features.md` (`2012fff`): the entry's *What* now describes the registry trigger and the role-split remedy; the "trigger is disarmed" gotcha is replaced by the fix and the two tests; and the "do not delete `tab:`" blocker is cleared from the #770 entry. `CLAUDE.md` ↔ wiki sync check: `in sync: True`. ### Lesson for the next ticket An accessor shows you a comparison, not a field's domain. For a Java record the compact constructor holds every default and normalization, so **the constructor is the contract**. A one-line getter looks like the whole truth precisely because there is nothing in it to warn you. Read the constructor before asserting what a field can hold — especially when the claim is in the negative ("if it is *not* set, then…"), because that is a claim about the default.
ltms closed this issue 2026-10-05 14:26:11 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#775