Compare commits
4 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Dai Ha
|
780cb342ad | ||
|
Dai Ha
|
5c563f02c8 | ||
|
Dai Ha
|
ad593c9bb9 | ||
|
Dai Ha
|
05244a82b3 |
@@ -669,6 +669,15 @@ fleet:
|
||||
# kind: opencode
|
||||
# model: openai/gpt-5.6-terra
|
||||
|
||||
# A collaborator tab, keyed by name (fleetd #669). This block is parsed and validated today;
|
||||
# nothing yet recognises or addresses the tab it names. Recognise-only, like a profile-less
|
||||
# `leaders:` entry above: there is no `profile:`, no `instances:` and no `kind:`. `tab:` is
|
||||
# REQUIRED and is the only field identity depends on, matched case-insensitively — the same
|
||||
# GET-THE-VALUE-RIGHT warning above the `leaders:` block applies here too.
|
||||
# collaborators:
|
||||
# reviewer-alex:
|
||||
# tab: "collab: alex"
|
||||
|
||||
# architects:
|
||||
# architect-1:
|
||||
# profile: opus # a strong model, on the operator's subscription
|
||||
|
||||
@@ -1176,6 +1176,25 @@ public record FleetConfig(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A tab fleetd recognises as a collaborator, keyed by name (fleetd #669).
|
||||
*
|
||||
* <p>Recognise-only: there is no {@code profile}, no {@code instances} and no {@code kind}.
|
||||
* Nothing here ever launches a pane.
|
||||
*
|
||||
* <p>{@code tabPrefix} is absent. Identity is matched on the exact {@code tab} alone.
|
||||
*
|
||||
* @param tab the exact tab label hosting this collaborator, matched case-insensitively; the
|
||||
* only field identity depends on. Required — an entry with no {@code tab} can
|
||||
* never be discovered.
|
||||
*/
|
||||
@JsonIgnoreProperties(ignoreUnknown = true)
|
||||
public record Collaborator(String tab) {
|
||||
public Collaborator {
|
||||
tab = (tab == null || tab.isBlank()) ? null : tab.strip();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* One entry of a {@code fleet:} role pool — a role paired with the backend it runs on.
|
||||
*
|
||||
@@ -1213,15 +1232,18 @@ public record FleetConfig(
|
||||
* is exactly compatible with that. The pool is also what replaced {@code defaultProfile:} — an
|
||||
* unqualified spawn names a role, and the role's pool supplies the candidates.
|
||||
*
|
||||
* @param leaders panes that orchestrate rather than are orchestrated, keyed by lead name
|
||||
* @param architects profiles the {@code architect} role may run on
|
||||
* @param developers profiles the {@code dev} role may run on
|
||||
* @param hunters profiles the {@code hunter} role may run on
|
||||
* @param reviewers profiles the {@code reviewer} role may run on
|
||||
* @param charters optional launch-charter text keyed by singular role wire name
|
||||
* @param tabLabel template for a member tab's label; {@code {role}}, {@code {profile}},
|
||||
* {@code {model}} and {@code {n}} (a per role+profile counter) are
|
||||
* substituted. Default {@link #DEFAULT_TAB_LABEL}
|
||||
* @param leaders panes that orchestrate rather than are orchestrated, keyed by lead name
|
||||
* @param architects profiles the {@code architect} role may run on
|
||||
* @param developers profiles the {@code dev} role may run on
|
||||
* @param hunters profiles the {@code hunter} role may run on
|
||||
* @param reviewers profiles the {@code reviewer} role may run on
|
||||
* @param charters optional launch-charter text keyed by singular role wire name
|
||||
* @param tabLabel template for a member tab's label; {@code {role}}, {@code {profile}},
|
||||
* {@code {model}} and {@code {n}} (a per role+profile counter) are
|
||||
* substituted. Default {@link #DEFAULT_TAB_LABEL}
|
||||
* @param collaborators tabs fleetd recognises as collaborators (fleetd #669), keyed by name.
|
||||
* Recognise-only, exactly like a {@code profile}-less {@link Leader}:
|
||||
* nothing here is ever auto-launched.
|
||||
*/
|
||||
@JsonIgnoreProperties(ignoreUnknown = true)
|
||||
public record Fleet(Map<String, Leader> leaders,
|
||||
@@ -1230,7 +1252,8 @@ public record FleetConfig(
|
||||
Map<String, Slot> hunters,
|
||||
Map<String, Slot> reviewers,
|
||||
Map<String, String> charters,
|
||||
String tabLabel) {
|
||||
String tabLabel,
|
||||
Map<String, Collaborator> collaborators) {
|
||||
|
||||
/**
|
||||
* Role first, so the tab bar identifies the member's fleet role.
|
||||
@@ -1245,26 +1268,30 @@ public record FleetConfig(
|
||||
reviewers = unmodifiableOrEmpty(reviewers);
|
||||
charters = unmodifiableOrEmpty(charters);
|
||||
tabLabel = (tabLabel == null || tabLabel.isBlank()) ? DEFAULT_TAB_LABEL : tabLabel;
|
||||
collaborators = unmodifiableOrEmpty(collaborators);
|
||||
}
|
||||
|
||||
/**
|
||||
* A fleet with no configured launch charters — the shape every deployment had before
|
||||
* CB-566, and what most tests want.
|
||||
* A fleet with no configured launch charters and no collaborators — the shape every
|
||||
* deployment had before CB-566, and what most tests want.
|
||||
*
|
||||
* <p>Kept deliberately, even though an overload that drops a new field is normally the
|
||||
* shape to avoid. It is safe here because nothing <em>reads</em> a charter through a
|
||||
* constructor: the launcher reads {@code fleet.charters()} from the live config. Jackson
|
||||
* binds the canonical constructor, so this one cannot swallow an operator's YAML.
|
||||
* {@code collaborators} is dropped the same way and for the same reason: no caller of
|
||||
* this overload has ever needed to set it, so it defaults to empty here exactly as the
|
||||
* canonical constructor would default an absent YAML key.
|
||||
*/
|
||||
public Fleet(Map<String, Leader> leaders, Map<String, Slot> architects,
|
||||
Map<String, Slot> developers, Map<String, Slot> reviewers,
|
||||
Map<String, String> charters, String tabLabel) {
|
||||
this(leaders, architects, developers, null, reviewers, charters, tabLabel);
|
||||
this(leaders, architects, developers, null, reviewers, charters, tabLabel, null);
|
||||
}
|
||||
|
||||
public Fleet(Map<String, Leader> leaders, Map<String, Slot> architects,
|
||||
Map<String, Slot> developers, Map<String, Slot> reviewers, String tabLabel) {
|
||||
this(leaders, architects, developers, null, reviewers, null, tabLabel);
|
||||
this(leaders, architects, developers, null, reviewers, null, tabLabel, null);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1910,7 +1937,7 @@ public record FleetConfig(
|
||||
|
||||
/** The {@code fleet:} child blocks whose direct children are slot names. */
|
||||
private static final Set<String> FLEET_POOL_KEYS =
|
||||
Set.of("leaders", "architects", "developers", "hunters", "reviewers");
|
||||
Set.of("leaders", "architects", "developers", "hunters", "reviewers", "collaborators");
|
||||
|
||||
/**
|
||||
* Reject a {@code fleet:} role pool whose slot names repeat (CB-548, re-homed by CB-557).
|
||||
@@ -1920,7 +1947,7 @@ public record FleetConfig(
|
||||
* daemon would never know. Jackson's YAML parser does not fail on duplicate mapping keys by
|
||||
* default, so duplicates are caught here, at parse time, before the map is built.
|
||||
*
|
||||
* <p>Only the five pools <em>directly under the top-level {@code fleet:}</em> are considered,
|
||||
* <p>Only the six pools <em>directly under the top-level {@code fleet:}</em> are considered,
|
||||
* and only their direct child keys (the slot names). A nested field elsewhere, even one also
|
||||
* named {@code developers:}, is ignored, so parsing of the rest of the config is unaffected.
|
||||
*
|
||||
@@ -2679,17 +2706,21 @@ public record FleetConfig(
|
||||
}
|
||||
|
||||
/**
|
||||
* Reject a member tab-label template that could render as a configured lead tab or match a
|
||||
* lead-tab naming convention, and reject two {@code fleet.leaders} entries that share one exact
|
||||
* tab.
|
||||
* Reject a member tab-label template that could render as a configured lead or collaborator
|
||||
* tab or match a lead-tab naming convention, and reject two {@code fleet.leaders} or
|
||||
* {@code fleet.collaborators} entries — across either registry — that share one exact tab.
|
||||
*
|
||||
* <p>{@code fleet.collaborators} has no {@code tabPrefix}: identity is matched on the exact
|
||||
* {@code tab} alone, so only the exact-render check applies there, not the prefix check.
|
||||
*
|
||||
* @throws IllegalStateException when the fleet template or a profile {@code tabLabel} override
|
||||
* can render as a configured lead tab or match a lead-tab prefix,
|
||||
* or when two {@code fleet.leaders} entries carry the same exact
|
||||
* {@code tab} (case-insensitively)
|
||||
* can render as a configured lead or collaborator tab or match a
|
||||
* lead-tab prefix, or when two entries — of either registry, or
|
||||
* one of each — carry the same exact {@code tab}
|
||||
* (case-insensitively)
|
||||
*/
|
||||
public void validateLeadTabPrefixes() {
|
||||
if (fleet == null || fleet.leaders().isEmpty()) {
|
||||
if (fleet == null) {
|
||||
return;
|
||||
}
|
||||
List<String> bad = new ArrayList<>();
|
||||
@@ -2722,11 +2753,32 @@ public record FleetConfig(
|
||||
}
|
||||
});
|
||||
});
|
||||
fleet.collaborators().forEach((collabName, collaborator) -> {
|
||||
if (collaborator == null) {
|
||||
return;
|
||||
}
|
||||
String tab = collaborator.tab();
|
||||
if (templateCanRenderAs(fleet.tabLabel(), tab)) {
|
||||
bad.add("fleet.tabLabel=\"" + fleet.tabLabel() + "\" can render as the tab of "
|
||||
+ "collaborator '" + collabName + "' (\"" + tab + "\")");
|
||||
}
|
||||
profiles().entrySet().stream()
|
||||
.map(Map.Entry::getKey)
|
||||
.sorted()
|
||||
.forEach(p -> {
|
||||
String label = profiles().get(p).tabLabel();
|
||||
if (templateCanRenderAs(label, tab)) {
|
||||
bad.add("profile '" + p + "' overrides tabLabel with \"" + label
|
||||
+ "\", which can render as the tab of collaborator '"
|
||||
+ collabName + "' (\"" + tab + "\")");
|
||||
}
|
||||
});
|
||||
});
|
||||
if (!bad.isEmpty()) {
|
||||
throw new IllegalStateException("refusing to start: " + String.join("; ", bad)
|
||||
+ ". Every member labelled that way would be read back as a lead and granted "
|
||||
+ "spawn/stop/send on the whole fleet. Change one of the two so member tabs "
|
||||
+ "and lead tabs cannot be confused.");
|
||||
+ ". Every member labelled that way would be read back as a lead or "
|
||||
+ "collaborator and granted that identity's authority. Change one of the two "
|
||||
+ "so member tabs cannot be confused with a lead's or collaborator's tab.");
|
||||
}
|
||||
|
||||
List<String> collisions = new ArrayList<>();
|
||||
@@ -2749,13 +2801,49 @@ public record FleetConfig(
|
||||
}
|
||||
}
|
||||
}
|
||||
List<String> collabNames = fleet.collaborators().keySet().stream().sorted().toList();
|
||||
for (int i = 0; i < collabNames.size(); i++) {
|
||||
String nameA = collabNames.get(i);
|
||||
Collaborator a = fleet.collaborators().get(nameA);
|
||||
if (a == null || a.tab() == null || a.tab().isBlank()) {
|
||||
continue;
|
||||
}
|
||||
for (int j = i + 1; j < collabNames.size(); j++) {
|
||||
String nameB = collabNames.get(j);
|
||||
Collaborator b = fleet.collaborators().get(nameB);
|
||||
if (b == null || b.tab() == null || b.tab().isBlank()) {
|
||||
continue;
|
||||
}
|
||||
if (a.tab().equalsIgnoreCase(b.tab())) {
|
||||
collisions.add("collaborator '" + nameA + "' and collaborator '" + nameB
|
||||
+ "' both use tab \"" + a.tab() + "\"");
|
||||
}
|
||||
}
|
||||
}
|
||||
for (String leadName : leadNames) {
|
||||
Leader lead = fleet.leaders().get(leadName);
|
||||
if (lead == null || lead.tab() == null || lead.tab().isBlank()) {
|
||||
continue;
|
||||
}
|
||||
for (String collabName : collabNames) {
|
||||
Collaborator collaborator = fleet.collaborators().get(collabName);
|
||||
if (collaborator == null || collaborator.tab() == null
|
||||
|| collaborator.tab().isBlank()) {
|
||||
continue;
|
||||
}
|
||||
if (lead.tab().equalsIgnoreCase(collaborator.tab())) {
|
||||
collisions.add("lead '" + leadName + "' and collaborator '" + collabName
|
||||
+ "' both use tab \"" + lead.tab() + "\"");
|
||||
}
|
||||
}
|
||||
}
|
||||
if (collisions.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
throw new IllegalStateException("refusing to start: " + String.join("; ", collisions)
|
||||
+ ". Tab identity is matched exactly, so only one of two leads sharing a tab can "
|
||||
+ "ever be found — the other is silently unreachable. Give each lead its own "
|
||||
+ "exact tab.");
|
||||
+ ". Tab identity is matched exactly, so only one of two entries sharing a tab can "
|
||||
+ "ever be found — the other is silently unreachable. Give each lead and "
|
||||
+ "collaborator its own exact tab.");
|
||||
}
|
||||
|
||||
private static boolean templateCanRenderAs(String template, String tab) {
|
||||
@@ -2785,25 +2873,28 @@ public record FleetConfig(
|
||||
|
||||
/**
|
||||
* Reject a profile that places its members by {@code "pane"} while any {@code fleet.leaders}
|
||||
* entry names a {@code tab}. A pane-placed member lands inside the focused tab rather than its
|
||||
* own, so it can land inside a lead's own labelled tab. {@link
|
||||
* dev.ltms.fleet.herdr.LeadTabScanner} identifies a lead purely by that tab's label — it does
|
||||
* not exclude the member space — so a member that ends up there would be read back as the lead
|
||||
* and granted spawn/stop/send on the whole fleet.
|
||||
* or {@code fleet.collaborators} entry names a {@code tab}. A pane-placed member lands inside
|
||||
* the focused tab rather than its own, so it can land inside a lead's or collaborator's own
|
||||
* labelled tab. {@link dev.ltms.fleet.herdr.LeadTabScanner} identifies a lead or collaborator
|
||||
* purely by that tab's label — it does not exclude the member space — so a member that ends up
|
||||
* there would be read back as that lead or collaborator and granted that identity's authority.
|
||||
*
|
||||
* <p>Only a leader with a non-blank {@code tab} is in scope: one with no {@code tab} feeds
|
||||
* <p>Only an entry with a non-blank {@code tab} is in scope: one with no {@code tab} feeds
|
||||
* nothing into {@link dev.ltms.fleet.herdr.LeadTabScanner}, so it creates no hazard here.
|
||||
*
|
||||
* @throws IllegalStateException when any {@code profiles:} entry is pane-placed while any
|
||||
* {@code fleet.leaders} entry names a non-blank {@code tab}
|
||||
* {@code fleet.leaders} or {@code fleet.collaborators} entry
|
||||
* names a non-blank {@code tab}
|
||||
*/
|
||||
public void validatePanePlacementAgainstLeadTabs() {
|
||||
if (fleet == null || fleet.leaders().isEmpty()) {
|
||||
if (fleet == null) {
|
||||
return;
|
||||
}
|
||||
boolean anyLeaderHasTab = fleet.leaders().values().stream()
|
||||
.anyMatch(leader -> leader != null && leader.tab() != null && !leader.tab().isBlank());
|
||||
if (!anyLeaderHasTab) {
|
||||
boolean anyCollaboratorHasTab = fleet.collaborators().values().stream()
|
||||
.anyMatch(c -> c != null && c.tab() != null && !c.tab().isBlank());
|
||||
if (!anyLeaderHasTab && !anyCollaboratorHasTab) {
|
||||
return;
|
||||
}
|
||||
List<String> bad = new ArrayList<>();
|
||||
@@ -2816,10 +2907,11 @@ public record FleetConfig(
|
||||
return;
|
||||
}
|
||||
throw new IllegalStateException("refusing to start: profile(s) " + bad
|
||||
+ " use placement: pane while fleet.leaders names a tab. A pane-placed member can "
|
||||
+ "land inside a lead's labelled tab and be read back as the lead, granted "
|
||||
+ "spawn/stop/send on the whole fleet. Set placement: tab for each named profile, "
|
||||
+ "or remove the tab from every fleet.leaders entry.");
|
||||
+ " use placement: pane while fleet.leaders or fleet.collaborators names a tab. A "
|
||||
+ "pane-placed member can land inside that labelled tab and be read back as the "
|
||||
+ "lead or collaborator, granted that identity's authority. Set placement: tab for "
|
||||
+ "each named profile, or remove the tab from every fleet.leaders and "
|
||||
+ "fleet.collaborators entry.");
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -2926,8 +3018,14 @@ public record FleetConfig(
|
||||
* so duplicates are unrepresentable by construction once loaded — and {@link #load(Path)}
|
||||
* already rejects a duplicated slot name at parse time, before the map collapses.
|
||||
*
|
||||
* @throws IllegalStateException when a slot names no profile or an unknown one, or when a lead
|
||||
* can be neither found nor created, naming the offending entry
|
||||
* <p>Also rejects a {@code fleet.collaborators} entry with no (or a blank) {@code tab}. A
|
||||
* {@code profile}-less lead is still useful recognise-only — {@code tab} is the only field
|
||||
* that matters to it either way. A collaborator carries no other field at all, so a blank
|
||||
* {@code tab} leaves nothing for the entry to mean.
|
||||
*
|
||||
* @throws IllegalStateException when a slot names no profile or an unknown one, when a lead
|
||||
* can be neither found nor created, or when a collaborator names
|
||||
* no tab, naming the offending entry
|
||||
*/
|
||||
public void validateMembers() {
|
||||
if (fleet == null) {
|
||||
@@ -2965,6 +3063,16 @@ public record FleetConfig(
|
||||
+ "auto-launched, labelled) purely by its tab, so every entry must name one.");
|
||||
}
|
||||
});
|
||||
fleet.collaborators().forEach((name, collaborator) -> {
|
||||
if (collaborator == null) {
|
||||
return;
|
||||
}
|
||||
if (collaborator.tab() == null || collaborator.tab().isBlank()) {
|
||||
bad.add("fleet.collaborators." + name + " has no tab: — a collaborator is "
|
||||
+ "recognised purely by its tab, and carries no other field, so every "
|
||||
+ "entry must name one.");
|
||||
}
|
||||
});
|
||||
if (!bad.isEmpty()) {
|
||||
throw new IllegalStateException("refusing to start: " + String.join(" ", bad));
|
||||
}
|
||||
|
||||
@@ -1000,6 +1000,273 @@ class FleetConfigTest {
|
||||
"no primary.terminal pin ⇒ nothing registered, even with fleet.leaders configured");
|
||||
}
|
||||
|
||||
// ── fleetd #669: the collaborators registry ────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* {@code Fleet} is {@code @JsonIgnoreProperties(ignoreUnknown = true)}, so a config naming
|
||||
* {@code fleet.collaborators.<name>.tab} loads with no exception whether or not the key is
|
||||
* ever read into the object model. Asserting only "no exception" would pass both before and
|
||||
* after the real fix, so this asserts the parsed value is actually reachable from the loaded
|
||||
* {@code FleetConfig} — the one thing a vacuous "no exception" test cannot tell apart.
|
||||
*/
|
||||
@Test
|
||||
void collaboratorsBlockIsActuallyParsedNotSilentlyDropped(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("collaborators.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
fleet:
|
||||
collaborators:
|
||||
reviewer-alex:
|
||||
tab: "collab: alex"
|
||||
""");
|
||||
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
assertEquals("collab: alex", cfg.fleet().collaborators().get("reviewer-alex").tab());
|
||||
}
|
||||
|
||||
/** A collaborator carries no field other than {@code tab}, so a blank one is meaningless. */
|
||||
@Test
|
||||
void aCollaboratorWithNoTabRefusesToStart(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("useless-collaborator.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
fleet:
|
||||
collaborators:
|
||||
ghost: {}
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateMembers);
|
||||
assertTrue(e.getMessage().contains("ghost"), "the message must name the useless entry");
|
||||
assertTrue(e.getMessage().contains("tab:"), "the message must say what is missing");
|
||||
}
|
||||
|
||||
/** Control for {@link #aCollaboratorWithNoTabRefusesToStart}: a named tab loads cleanly. */
|
||||
@Test
|
||||
void aCollaboratorWithATabIsAllowed(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("named-collaborator.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
fleet:
|
||||
collaborators:
|
||||
reviewer-alex:
|
||||
tab: "collab: alex"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
assertDoesNotThrow(cfg::validateMembers);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #669: the fleet-wide {@code tabLabel} template can render as a collaborator tab, the
|
||||
* same hazard {@link #aFleetTabLabelTemplateThatCanRenderAsALeadTabRefusesToStart} covers on
|
||||
* the lead side. Drives the fleet-wide branch directly, with no profile override involved.
|
||||
*/
|
||||
@Test
|
||||
void aFleetTabLabelTemplateThatCanRenderAsACollaboratorTabRefusesToStart(@TempDir Path dir)
|
||||
throws Exception {
|
||||
Path f = dir.resolve("collide-template-collaborator.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
fleet:
|
||||
tabLabel: "al{profile}"
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "alpha"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e =
|
||||
assertThrows(IllegalStateException.class, cfg::validateLeadTabPrefixes);
|
||||
assertTrue(e.getMessage().contains("fleet.tabLabel"));
|
||||
assertTrue(e.getMessage().contains("alex"), "the message must name the offending collaborator");
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #669: a member tabLabel that can render as a configured collaborator tab is the same
|
||||
* hazard as the lead case above — a member labelled that way is read back as the collaborator.
|
||||
*/
|
||||
@Test
|
||||
void aProfileTabLabelOverrideMatchingACollaboratorTabRefusesToStart(@TempDir Path dir)
|
||||
throws Exception {
|
||||
Path f = dir.resolve("collide-collaborator.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
tabLabel: "collab-tab"
|
||||
fleet:
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "collab-tab"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e =
|
||||
assertThrows(IllegalStateException.class, cfg::validateLeadTabPrefixes);
|
||||
assertTrue(e.getMessage().contains("gx10"), "the message must name the offending profile");
|
||||
assertTrue(e.getMessage().contains("collab-tab"), "the message must name the offending label");
|
||||
}
|
||||
|
||||
/** Control: a profile tabLabel that cannot render as the collaborator tab is allowed. */
|
||||
@Test
|
||||
void aProfileTabLabelThatCannotRenderAsACollaboratorTabIsAllowed(@TempDir Path dir)
|
||||
throws Exception {
|
||||
Path f = dir.resolve("ok-collaborator.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
tabLabel: "worker-{profile}"
|
||||
fleet:
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "collab-tab"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
assertDoesNotThrow(cfg::validateLeadTabPrefixes);
|
||||
}
|
||||
|
||||
/** fleetd #669: identity is matched on a collaborator's exact tab, so two sharing one are unreachable. */
|
||||
@Test
|
||||
void twoCollaboratorsSharingTheSameExactTabRefusesToStart(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("shared-collaborator-tab.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
fleet:
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "shared tab"
|
||||
sam:
|
||||
tab: "Shared Tab"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e =
|
||||
assertThrows(IllegalStateException.class, cfg::validateLeadTabPrefixes);
|
||||
assertTrue(e.getMessage().contains("alex"), "the message must name one offending collaborator");
|
||||
assertTrue(e.getMessage().contains("sam"), "the message must name the other offending collaborator");
|
||||
}
|
||||
|
||||
/** Control for {@link #twoCollaboratorsSharingTheSameExactTabRefusesToStart}: distinct tabs load cleanly. */
|
||||
@Test
|
||||
void twoCollaboratorsWithDistinctExactTabsAreAllowed(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("distinct-collaborator-tabs.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
fleet:
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "alex tab"
|
||||
sam:
|
||||
tab: "sam tab"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
assertDoesNotThrow(cfg::validateLeadTabPrefixes);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #669: a collaborator tab equal to a lead tab crosses a privilege boundary — the worst
|
||||
* of the three new collisions, since only one of the two identities is ever found.
|
||||
*/
|
||||
@Test
|
||||
void aCollaboratorTabEqualToALeadTabRefusesToStart(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("lead-collaborator-collision.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
fleet:
|
||||
leaders:
|
||||
opus:
|
||||
tab: "shared tab"
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "Shared Tab"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e =
|
||||
assertThrows(IllegalStateException.class, cfg::validateLeadTabPrefixes);
|
||||
assertTrue(e.getMessage().contains("opus"), "the message must name the offending lead");
|
||||
assertTrue(e.getMessage().contains("alex"), "the message must name the offending collaborator");
|
||||
}
|
||||
|
||||
/** Control for {@link #aCollaboratorTabEqualToALeadTabRefusesToStart}: distinct tabs load cleanly. */
|
||||
@Test
|
||||
void aLeadAndACollaboratorWithDistinctTabsAreAllowed(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("lead-collaborator-ok.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
fleet:
|
||||
leaders:
|
||||
opus:
|
||||
tab: "lead tab"
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "collab tab"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
assertDoesNotThrow(cfg::validateLeadTabPrefixes);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #669: a pane-placed member can land in a collaborator's labelled tab exactly as it
|
||||
* can land in a lead's — {@code validatePanePlacementAgainstLeadTabs()} must fire even when
|
||||
* {@code fleet.leaders} is empty, which is the early-return the brief flagged as the bug.
|
||||
*/
|
||||
@Test
|
||||
void aPanePlacedProfileWithACollaboratorTabRefusesToStartEvenWithNoLeaders(@TempDir Path dir)
|
||||
throws Exception {
|
||||
Path f = dir.resolve("pane-hazard-collaborator.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
placement: pane
|
||||
fleet:
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "collab: alex"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class,
|
||||
cfg::validatePanePlacementAgainstLeadTabs);
|
||||
assertTrue(e.getMessage().contains("gx10"), "the message must name the offending profile");
|
||||
}
|
||||
|
||||
/** Control: a pane-placed profile with no lead or collaborator tab configured is allowed. */
|
||||
@Test
|
||||
void aPanePlacedProfileWithNoLeaderOrCollaboratorTabIsAllowed(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("pane-no-tab-at-all.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
placement: pane
|
||||
fleet:
|
||||
collaborators:
|
||||
alex: {}
|
||||
""");
|
||||
|
||||
assertDoesNotThrow(() -> FleetConfig.load(f).validatePanePlacementAgainstLeadTabs(),
|
||||
"a collaborator with no tab feeds nothing into the scanner, so pane placement is safe");
|
||||
}
|
||||
|
||||
// ── CB-548: the architects registry ────────────────────────────────────────────────────────
|
||||
|
||||
@Test
|
||||
@@ -1272,6 +1539,60 @@ class FleetConfigTest {
|
||||
assertEquals(Set.of("sonnet"), cfg.fleet().pool(MemberRole.REVIEWER).keySet());
|
||||
}
|
||||
|
||||
/**
|
||||
* A duplicated name in {@code fleet.collaborators} is refused at parse time, like any other
|
||||
* {@code fleet:} pool. See {@link #duplicateSlotNamesInOnePoolAreRejectedAtParseTime}.
|
||||
*/
|
||||
@Test
|
||||
void duplicateCollaboratorNamesAreRejectedAtParseTime(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("collaborator-dup.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
fleet:
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "collab: alex"
|
||||
alex:
|
||||
tab: "collab: alex, second"
|
||||
""");
|
||||
|
||||
IllegalStateException e =
|
||||
assertThrows(IllegalStateException.class, () -> FleetConfig.load(f));
|
||||
assertTrue(e.getMessage().contains("alex"),
|
||||
"the refusal names the duplicated entry, was: " + e.getMessage());
|
||||
assertTrue(e.getMessage().contains("fleet.collaborators"),
|
||||
"the refusal names the pool the duplicate is in, was: " + e.getMessage());
|
||||
}
|
||||
|
||||
/**
|
||||
* Control for {@link #duplicateCollaboratorNamesAreRejectedAtParseTime}: the same name reused
|
||||
* across the collaborators registry and a member role pool is the role × profile matrix doing
|
||||
* its job in the other pool, not a mistake — only a repeat within one pool loses an entry.
|
||||
*/
|
||||
@Test
|
||||
void theSameNameInCollaboratorsAndAnotherPoolIsNotADuplicate(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("collaborator-cross-pool.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
sonnet:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
fleet:
|
||||
architects:
|
||||
alex:
|
||||
profile: sonnet
|
||||
collaborators:
|
||||
alex:
|
||||
tab: "collab: alex"
|
||||
""");
|
||||
|
||||
FleetConfig cfg = assertDoesNotThrow(() -> FleetConfig.load(f));
|
||||
assertEquals(Set.of("alex"), cfg.fleet().pool(MemberRole.ARCHITECT).keySet());
|
||||
assertEquals(Set.of("alex"), cfg.fleet().collaborators().keySet());
|
||||
}
|
||||
|
||||
@Test
|
||||
void duplicateKeysOutsideTheFleetPoolsAreUnaffected(@TempDir Path dir) throws Exception {
|
||||
// The duplicate check is scoped to the fleet pools — a duplicate elsewhere is not this
|
||||
|
||||
+2
-13
@@ -213,17 +213,6 @@ map_masked_lines() {
|
||||
done < "$file"
|
||||
}
|
||||
|
||||
# Masks every `scheme://user:pass@host` userinfo on one line of text, replacing just that
|
||||
# userinfo with `<redacted>` and leaving the rest of the line untouched, byte for byte. The
|
||||
# pattern stops at the first `/`, whitespace, or `@` reached after `://` — a URI's userinfo
|
||||
# component cannot contain any of those three characters — so a URI with no userinfo, followed
|
||||
# later on the same line by an unrelated `@`, never matches. The `g` flag matters: a line can
|
||||
# carry more than one URI. Shared by every caller that prints a line which may hold a
|
||||
# credentialed URI, so the bound lives in exactly one place.
|
||||
mask_url_userinfo() {
|
||||
printf '%s\n' "$1" | sed -E 's#://[^@/[:space:]]*@#://<redacted>@#g'
|
||||
}
|
||||
|
||||
redact() {
|
||||
local old_file="$1" new_file="$2"
|
||||
local line prefix content indent lead key old_line=0 new_line=0 in_hunk=0
|
||||
@@ -281,7 +270,7 @@ redact() {
|
||||
continue
|
||||
fi
|
||||
fi
|
||||
mask_url_userinfo "$line"
|
||||
printf '%s\n' "$line" | sed -E 's#://[^@]*@#://<redacted>@#g'
|
||||
done
|
||||
[ "$saved_nocasematch" = 1 ] || shopt -u nocasematch
|
||||
}
|
||||
@@ -594,7 +583,7 @@ install_candidate() {
|
||||
# Masks basic-auth userinfo (scheme://user:pass@host) in a daemon verdict line before it reaches
|
||||
# the terminal.
|
||||
mask_verdict_userinfo() {
|
||||
mask_url_userinfo "$1"
|
||||
printf '%s\n' "$1" | sed -E 's#://[^@/[:space:]]*@#://<redacted>@#g'
|
||||
}
|
||||
|
||||
# Prints the literal command the operator (or a test) can run to restore the backup by hand — the
|
||||
|
||||
@@ -233,66 +233,6 @@ test_redaction_holds() {
|
||||
assert_contains "weight" "$RUN_OUTPUT" "a diff must have been demonstrably printed at all"
|
||||
}
|
||||
|
||||
# redact()'s key-name filter only inspects the KEY, so a diff line whose key does not match
|
||||
# TOKEN|SECRET|PASSWORD|PASSWD|PASSPHRASE|CREDENTIAL|URI|_KEY still reaches the final userinfo
|
||||
# sed even when its VALUE holds a credentialed URI. "note" is not a sensitive key name, so this
|
||||
# line must fall all the way through to that sed, not the earlier whole-value branch. The
|
||||
# trailing prose on both sides of the userinfo is a positive control: it proves the line reached
|
||||
# the userinfo sed (which touches only the userinfo) rather than the earlier branch (which would
|
||||
# have replaced the whole value with a bare "<redacted>" and dropped the prose).
|
||||
test_diff_line_userinfo_is_masked_with_positive_control() {
|
||||
local dir
|
||||
dir="$(new_fixture)"
|
||||
|
||||
start_run "$dir" 5 --set '.profiles.sonnet.note=see amqp://alice:wonderland@rabbit.local:5672/vhost for details'
|
||||
sleep 1
|
||||
printf 'config reloaded\n' >> "$dir/fleetd.out"
|
||||
collect_run "$dir"
|
||||
|
||||
assert_equals 0 "$RUN_RC" "diff-userinfo-case reload exit code"
|
||||
assert_not_contains "alice:wonderland" "$RUN_OUTPUT" "the userinfo must never reach the output"
|
||||
assert_contains "amqp://<redacted>@rabbit.local:5672/vhost" "$RUN_OUTPUT" \
|
||||
"the userinfo must be MASKED, not deleted — the rest of the value must survive"
|
||||
assert_contains "note:" "$RUN_OUTPUT" "the key name must still reach the output"
|
||||
assert_contains "see " "$RUN_OUTPUT" "prose BEFORE the userinfo must still reach the output"
|
||||
assert_contains "for details" "$RUN_OUTPUT" "prose AFTER the userinfo must still reach the output"
|
||||
}
|
||||
|
||||
# A diff line can hold a URL with no userinfo, followed later on the same line by an unrelated @
|
||||
# (free text in a string value, for example an email address). The line must pass through the
|
||||
# userinfo sed byte for byte: the match must stop at the end of the URL and must not treat the
|
||||
# later @ as a second userinfo delimiter.
|
||||
test_diff_line_uri_without_userinfo_survives_a_later_at_sign() {
|
||||
local dir
|
||||
dir="$(new_fixture)"
|
||||
|
||||
start_run "$dir" 5 --set '.profiles.sonnet.note2=see https://docs.local/guide and mail ops@example.com'
|
||||
sleep 1
|
||||
printf 'config reloaded\n' >> "$dir/fleetd.out"
|
||||
collect_run "$dir"
|
||||
|
||||
assert_equals 0 "$RUN_RC" "diff-no-userinfo-with-later-at-sign reload exit code"
|
||||
assert_contains "note2: see https://docs.local/guide and mail ops@example.com" "$RUN_OUTPUT" \
|
||||
"a URL with no userinfo plus a later @ on the same line must pass through byte for byte"
|
||||
}
|
||||
|
||||
# Two credentialed URIs on one diff line must both be masked — the g flag matters.
|
||||
test_diff_line_masks_multiple_userinfo_with_g_flag() {
|
||||
local dir
|
||||
dir="$(new_fixture)"
|
||||
|
||||
start_run "$dir" 5 --set '.profiles.sonnet.note3=amqp://u1:p1@host1/vhost1 and amqp://u2:p2@host2/vhost2'
|
||||
sleep 1
|
||||
printf 'config reloaded\n' >> "$dir/fleetd.out"
|
||||
collect_run "$dir"
|
||||
|
||||
assert_equals 0 "$RUN_RC" "diff-two-userinfo-on-one-line reload exit code"
|
||||
assert_not_contains "u1:p1" "$RUN_OUTPUT" "the first userinfo must never reach the output"
|
||||
assert_not_contains "u2:p2" "$RUN_OUTPUT" "the second userinfo must never reach the output"
|
||||
assert_contains "amqp://<redacted>@host1/vhost1" "$RUN_OUTPUT" "the first URI must be masked"
|
||||
assert_contains "amqp://<redacted>@host2/vhost2" "$RUN_OUTPUT" "the second URI must be masked"
|
||||
}
|
||||
|
||||
# ------------------------------------------------------- acceptance criterion 9: forgotten value
|
||||
# `--set .a.b=` is a plausible typo (the value simply forgotten), and it must be refused outright
|
||||
# rather than silently nulling the field — a null numeric field falls back to its default, which
|
||||
@@ -813,12 +753,6 @@ echo "== acceptance criterion 6: the marker works =="
|
||||
test_marker_skips_lines_before_it
|
||||
echo "== acceptance criterion 7 (+13: redaction is proven to have run) =="
|
||||
test_redaction_holds
|
||||
echo "== fleetd #692: a diff line's userinfo is masked, rest of the value survives =="
|
||||
test_diff_line_userinfo_is_masked_with_positive_control
|
||||
echo "== fleetd #692: a diff line's URI with no userinfo survives a later @ in the line =="
|
||||
test_diff_line_uri_without_userinfo_survives_a_later_at_sign
|
||||
echo "== fleetd #692: two userinfo URIs on one diff line are both masked =="
|
||||
test_diff_line_masks_multiple_userinfo_with_g_flag
|
||||
echo "== acceptance criterion 9: a forgotten value refuses and installs nothing =="
|
||||
test_forgotten_value_refuses_and_installs_nothing
|
||||
echo "== acceptance criterion 10: an explicit clear writes a bare null =="
|
||||
|
||||
Reference in New Issue
Block a user