Compare commits
8 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 780cb342ad | |||
| 5c563f02c8 | |||
| ad593c9bb9 | |||
| 05244a82b3 | |||
| edbd8d816a | |||
| 9425a9b696 | |||
| d0688c8a60 | |||
| c6430d8edd |
@@ -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));
|
||||
}
|
||||
|
||||
@@ -83,6 +83,18 @@ public final class FleetApp {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The second gate for {@code POST /sessions/{id}/message}: checked only when {@code turnId}
|
||||
* is present and non-blank, against {@link Authz.Action#ANSWER}. A request with no {@code
|
||||
* turnId} passes this gate unconditionally, without consulting {@code permit} at all, having
|
||||
* already cleared the coarse {@link Authz.Action#SEND} grant checked ahead of it.
|
||||
*
|
||||
* @param permit reports whether the caller holds the named grant
|
||||
*/
|
||||
static boolean answerGatePasses(String turnId, Predicate<Authz.Action> permit) {
|
||||
return turnId == null || turnId.isBlank() || permit.test(Authz.Action.ANSWER);
|
||||
}
|
||||
|
||||
/** Default blocking window for a message; kept under typical HTTP idle timeouts. */
|
||||
private static final long DEFAULT_MESSAGE_TIMEOUT_MS = 25_000;
|
||||
private static final long MAX_MESSAGE_TIMEOUT_MS = 120_000;
|
||||
@@ -625,27 +637,31 @@ public final class FleetApp {
|
||||
*
|
||||
* <p>Two call shapes share this route, exactly as {@code fleet_send} does over MCP (see
|
||||
* {@code FleetMcp#sendAction}): a plain delivery to {@code id}, and -- when the body carries
|
||||
* {@code turnId} -- resolving a worker's blocked question. The body is parsed before the
|
||||
* authorization check so the right one of {@link Authz.Action#SEND}/{@link Authz.Action#ANSWER}
|
||||
* reaches the gate; a body that fails to parse is treated as the plain shape for that check
|
||||
* alone, and is rejected afterward exactly as before.
|
||||
* {@code turnId} -- resolving a worker's blocked question. The coarse {@link
|
||||
* Authz.Action#SEND} grant is checked first, before the body is read at all; only once that
|
||||
* passes is the body parsed, and a present {@code turnId} is then checked again against
|
||||
* {@link Authz.Action#ANSWER}. A body that fails to parse is rejected with 400 and reaches
|
||||
* neither {@code messages.answer} nor {@code messages.send}.
|
||||
*/
|
||||
private void sendMessage(Context ctx) {
|
||||
String id = ctx.pathParam("id");
|
||||
if (!allow(ctx, routeAction("POST /sessions/{id}/message"), id)) {
|
||||
return;
|
||||
}
|
||||
JsonNode body;
|
||||
try {
|
||||
body = mapper.readTree(ctx.body());
|
||||
} catch (Exception e) {
|
||||
body = null;
|
||||
}
|
||||
String turnId = body == null ? null : body.path("turnId").asText(null);
|
||||
if (!allow(ctx, routeAction("POST /sessions/{id}/message", turnId), id)) {
|
||||
return;
|
||||
}
|
||||
if (body == null) {
|
||||
ctx.status(400).json(Map.of("error", "bad_request", "detail", "body must be JSON"));
|
||||
return;
|
||||
}
|
||||
String turnId = body.path("turnId").asText(null);
|
||||
if (!answerGatePasses(turnId, action -> allow(ctx, action, id))) {
|
||||
return;
|
||||
}
|
||||
String content = body.path("content").asText("");
|
||||
long timeout = body.path("timeoutMs").asLong(DEFAULT_MESSAGE_TIMEOUT_MS);
|
||||
boolean wait = body.path("wait").asBoolean(true); // default: block for the reply (CB-104)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
package dev.ltms.fleet.rest;
|
||||
|
||||
import ch.qos.logback.classic.Level;
|
||||
import ch.qos.logback.classic.spi.ILoggingEvent;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import dev.ltms.fleet.auth.CallerResolver;
|
||||
import dev.ltms.fleet.auth.Authz;
|
||||
import dev.ltms.fleet.auth.MemberRegistry;
|
||||
@@ -18,6 +21,7 @@ import dev.ltms.fleet.msg.Rendezvous;
|
||||
import dev.ltms.fleet.session.FakeWorktrees;
|
||||
import dev.ltms.fleet.session.SessionManager;
|
||||
import dev.ltms.fleet.member.ClaudeCodeLauncher;
|
||||
import dev.ltms.fleet.testing.CapturedLog;
|
||||
import io.javalin.Javalin;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
@@ -28,10 +32,13 @@ import java.net.http.HttpRequest;
|
||||
import java.net.http.HttpResponse;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.ArrayList;
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.function.Predicate;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.stream.Collectors;
|
||||
@@ -144,6 +151,38 @@ class FleetAppAuthTest {
|
||||
assertEquals(Authz.Action.ANSWER, FleetApp.routeAction("POST /sessions/{id}/message", "turn-1"));
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #689: {@code answerGatePasses} is the second, conditional gate behind {@code
|
||||
* sendMessage}'s coarse {@link Authz.Action#SEND} check. With the {@code ANSWER} grant denied,
|
||||
* a {@code turnId}-bearing request is refused while a plain one still passes — and the denied
|
||||
* permit is queried only for the {@code turnId} case, never for the plain one, which is what
|
||||
* proves this is a genuinely separate, conditional check rather than the {@code SEND} check
|
||||
* renamed or an unconditional call whose result is ignored. Flipping only the {@code ANSWER}
|
||||
* grant to allowed then flips only the {@code turnId} shape's outcome.
|
||||
*/
|
||||
@Test
|
||||
void answerGatePassesOnlyWhenTurnIdAbsentOrAnswerGranted() {
|
||||
List<Authz.Action> queried = new ArrayList<>();
|
||||
Predicate<Authz.Action> denyAnswer = action -> {
|
||||
queried.add(action);
|
||||
return false;
|
||||
};
|
||||
|
||||
assertFalse(FleetApp.answerGatePasses("turn-1", denyAnswer),
|
||||
"ANSWER denied ⇒ the turnId shape is refused");
|
||||
assertEquals(List.of(Authz.Action.ANSWER), queried,
|
||||
"the ANSWER grant, specifically, must be the one consulted");
|
||||
|
||||
queried.clear();
|
||||
assertTrue(FleetApp.answerGatePasses(null, denyAnswer),
|
||||
"no turnId ⇒ the plain shape passes even though ANSWER is denied");
|
||||
assertTrue(FleetApp.answerGatePasses(" ", denyAnswer), "a blank turnId is treated as absent");
|
||||
assertEquals(List.of(), queried, "the plain shape must never consult the permit at all");
|
||||
|
||||
assertTrue(FleetApp.answerGatePasses("turn-1", action -> true),
|
||||
"flipping only the ANSWER grant to allowed flips only the turnId shape's outcome");
|
||||
}
|
||||
|
||||
private static Set<String> routesTheServerRegisters() {
|
||||
try {
|
||||
String source = Files.readString(REST_SOURCE).lines()
|
||||
@@ -223,6 +262,92 @@ class FleetAppAuthTest {
|
||||
"resolving another session's blocked question would be a worker escalating too");
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #689: a caller refused the coarse {@link Authz.Action#SEND} grant is refused on
|
||||
* {@code SEND} specifically, even on the {@code turnId}-bearing shape that otherwise raises
|
||||
* the check to {@link Authz.Action#ANSWER} — proving {@code turnId} was never read from the
|
||||
* body before the refusal (reading it would have changed which action is named in the 403).
|
||||
* The same caller refused with no body at all gets the identical detail, which could not hold
|
||||
* if the decision depended on anything read from the body. Control: a caller who IS granted
|
||||
* reaches past the gate and the body is used normally.
|
||||
*/
|
||||
@Test
|
||||
void aDeniedCallerIsRefusedOnSendEvenWithATurnIdBodyAndNeverReadsTheBody() throws Exception {
|
||||
int workerPort = start(FakeHerdr.WORKER_PID, false, null); // denied: not primary/architect
|
||||
|
||||
HttpResponse<String> withTurnId = send(workerPort, "POST", "/sessions/term_b/message",
|
||||
"{\"turnId\":\"turn-1\",\"content\":\"hi\"}", null);
|
||||
assertEquals(403, withTurnId.statusCode());
|
||||
assertTrue(withTurnId.body().contains("may not SEND"),
|
||||
"the SEND check must be the one that fired, not ANSWER — ANSWER would only be "
|
||||
+ "reachable by having already read turnId out of the body");
|
||||
|
||||
HttpResponse<String> noBody = send(workerPort, "POST", "/sessions/term_b/message", null, null);
|
||||
assertEquals(403, noBody.statusCode());
|
||||
assertTrue(noBody.body().contains("may not SEND"),
|
||||
"refused identically with no body at all — the refusal cannot depend on body content");
|
||||
|
||||
// Control: a primary IS granted SEND, so the same turnId body is read and acted on —
|
||||
// reaching messages.answer, which reports this unknown turnId as a stale one.
|
||||
int primaryPort = start(999_999, false, null);
|
||||
HttpResponse<String> granted = send(primaryPort, "POST", "/sessions/term_b/message",
|
||||
"{\"turnId\":\"turn-1\",\"content\":\"hi\"}", null);
|
||||
assertEquals(409, granted.statusCode());
|
||||
assertTrue(granted.body().contains("stale_turn"), "a granted caller's body IS read and acted on");
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #689 (ticket comment 18353): the only place {@code sendMessage}'s call to {@code
|
||||
* answerGatePasses} is observable is the audit trail — {@code allow()} logs an {@code
|
||||
* "allowed"} entry for every granted action except {@code READ}/{@code METRICS}/{@code
|
||||
* TASK_READ}, and {@code ANSWER} is none of those. A granted {@code turnId} request must
|
||||
* therefore log both a {@code SEND} and an {@code ANSWER} entry; a granted plain request must
|
||||
* log {@code SEND} alone. A unit test of the extracted helper pins the helper; this pins the
|
||||
* call site — deleting the {@code answerGatePasses} call from {@code sendMessage} leaves the
|
||||
* helper's own test green but turns this one red.
|
||||
*/
|
||||
@Test
|
||||
void aGrantedTurnIdRequestAuditsBothSendAndAnswerButAPlainRequestAuditsSendAlone() throws Exception {
|
||||
int port = start(999_999, false, null); // primary: granted both SEND and ANSWER
|
||||
ObjectMapper mapper = new ObjectMapper();
|
||||
|
||||
try (CapturedLog audit = CapturedLog.at("audit", Level.INFO)) {
|
||||
send(port, "POST", "/sessions/term_b/message",
|
||||
"{\"turnId\":\"turn-1\",\"content\":\"hi\"}", null);
|
||||
|
||||
List<String> allowed = allowedActions(audit, mapper);
|
||||
assertTrue(allowed.contains("SEND"),
|
||||
"a turnId request must still clear the coarse SEND grant first");
|
||||
assertTrue(allowed.contains("ANSWER"),
|
||||
"a turnId request must ALSO clear the ANSWER grant — this is the call site itself");
|
||||
}
|
||||
|
||||
try (CapturedLog audit = CapturedLog.at("audit", Level.INFO)) {
|
||||
send(port, "POST", "/sessions/term_b/message",
|
||||
"{\"content\":\"hi\",\"timeoutMs\":50}", null);
|
||||
|
||||
List<String> allowed = allowedActions(audit, mapper);
|
||||
assertEquals(List.of("SEND"), allowed,
|
||||
"a plain request must log SEND and nothing else — ANSWER is conditional on "
|
||||
+ "turnId, not something every request happens to log");
|
||||
}
|
||||
}
|
||||
|
||||
private static List<String> allowedActions(CapturedLog audit, ObjectMapper mapper) {
|
||||
return audit.events().stream()
|
||||
.map(ILoggingEvent::getFormattedMessage)
|
||||
.map(line -> {
|
||||
try {
|
||||
return mapper.readTree(line);
|
||||
} catch (Exception e) {
|
||||
throw new AssertionError("audit line is not valid JSON: " + line, e);
|
||||
}
|
||||
})
|
||||
.filter(n -> "allowed".equals(n.path("outcome").asText()))
|
||||
.map(n -> n.path("action").asText())
|
||||
.toList();
|
||||
}
|
||||
|
||||
// --- token mode ---------------------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
|
||||
@@ -675,6 +675,26 @@ class FleetAppTest {
|
||||
assertEquals(400, postMessage(port, "{}").statusCode());
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #689: a body that fails to parse is rejected with 400 before {@code turnId} is ever
|
||||
* read from it, so it reaches neither {@code messages.answer} (which needs a {@code turnId})
|
||||
* nor {@code messages.send} — confirmed here for {@code send} by the fake agent's idle status,
|
||||
* which would otherwise make an immediate {@code agent.prompt} delivery observable.
|
||||
*/
|
||||
@Test
|
||||
void malformedBodyReturns400AndNeverReachesSendOrAnswer() throws Exception {
|
||||
FakeHerdr herdr = new FakeHerdr().agentStatus("idle"); // idle ⇒ send would deliver right away if reached
|
||||
int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw"));
|
||||
|
||||
HttpResponse<String> res = postMessage(port, "not json at all");
|
||||
assertEquals(400, res.statusCode());
|
||||
JsonNode err = mapper.readTree(res.body());
|
||||
assertEquals("bad_request", err.get("error").asText());
|
||||
assertEquals("body must be JSON", err.get("detail").asText());
|
||||
assertFalse(herdr.called("agent.prompt"),
|
||||
"a malformed body must never reach messages.send's delivery");
|
||||
}
|
||||
|
||||
@Test
|
||||
void sessionStatusReportsLiveAgentStatus() throws Exception {
|
||||
FakeHerdr herdr = new FakeHerdr().agentStatus("blocked");
|
||||
|
||||
Reference in New Issue
Block a user