Compare commits
12 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| af4c88d54b | |||
| b540a1744b | |||
| e7b33fe3a0 | |||
| 799014e99d | |||
| 69e09b10fa | |||
| b9d09e044e | |||
| fd8650cda4 | |||
| 11050e24ed | |||
| 6f71f40047 | |||
| fb36c5238f | |||
| cc9cdc938b | |||
| 5fede82468 |
@@ -877,3 +877,32 @@ guard:
|
||||
# terminal: term_65619bd6174568
|
||||
# pushReminders: 5
|
||||
# pushBackoffMs: 15000
|
||||
|
||||
# Central allow-list of models any profiles: entry may name. Nothing checked a profile's model:
|
||||
# value before this block existed — it was a free-form string handed straight to the backend
|
||||
# adapter, and a withdrawn or misspelled name failed silently instead of at config load (opencode
|
||||
# falls back to a default model rather than erroring on an unknown -m).
|
||||
#
|
||||
# Absent, or present with an empty allow:, is OFF: no profile's model: is checked, exactly like
|
||||
# before this block existed. fleetd.yaml is gitignored on every host, so an upgrade must not force
|
||||
# every operator to enumerate their models before the daemon will start.
|
||||
#
|
||||
# The list is the authority; profiles: is checked against it, never the reverse — adding or
|
||||
# editing a profiles: entry cannot, by itself, widen what is permitted here.
|
||||
#
|
||||
# Enforcement is at CONFIG LOAD only (a bad model: fails the daemon at startup, naming both the
|
||||
# model and the profile). There is no spawn-time enforcement, no runtime on/off switch, and no
|
||||
# interaction with BackendQuarantine — those are separate, later units.
|
||||
#
|
||||
# allow → the permitted models. Each entry is its own block (not a bare string) so a later unit
|
||||
# can add an on/off state or a load limit per model without changing this shape.
|
||||
# model → the model id exactly as a profiles: entry's model: field would write it. One flat,
|
||||
# opaque-string namespace: a bare Claude id (claude-sonnet-5) and an opencode
|
||||
# provider-prefixed id (openai/gpt-5.6-terra) both fit here unchanged — the check is a
|
||||
# plain string match, never a parse of the provider prefix or a branch on kind:.
|
||||
# models:
|
||||
# allow:
|
||||
# - model: claude-sonnet-5
|
||||
# - model: claude-opus-5
|
||||
# - model: openai/gpt-5.6-terra
|
||||
# - model: amazon.nova-pro-v1:0
|
||||
|
||||
@@ -144,19 +144,16 @@ public final class Fleetd {
|
||||
SubscriptionGuard guard = new SubscriptionGuard(cfg.guard().hostSet());
|
||||
guard.assertPrimaryClean(System.getenv());
|
||||
|
||||
// CB-501: refuse to start if the bind is wider than the auth mode can defend. Under
|
||||
// loopback-trust, "not a known worker" means "the primary" — sound only because the OS
|
||||
// refuses remote connections to a loopback socket. This throws rather than warns so the
|
||||
// dangerous configuration cannot be reached by ignoring a log line.
|
||||
cfg.validateAuthExposure();
|
||||
cfg.validateLeadTabPrefixes();
|
||||
// CB-542: a subscription:true profile whose env: reseats ANTHROPIC_BASE_URL/AUTH_TOKEN would
|
||||
// reach an unguarded endpoint (the launcher skips SubscriptionGuard for it). Refuse at load.
|
||||
cfg.validateSubscriptionProfiles();
|
||||
cfg.validateCharters();
|
||||
// CB-548: every architect slot must name a configured workers: profile — the strong-model
|
||||
// backend the future spawn lifecycle would read. A stale reference dies here, not later.
|
||||
cfg.validateMembers();
|
||||
// Every FleetConfig.validateXxx() the operator's config can fail — CB-501's auth-exposure
|
||||
// check, CB-531's lead-tab-prefix check, CB-542's subscription-profile check, the charter
|
||||
// and member-slot checks, and the "central allow-list of usable models" check — must run
|
||||
// here, at load, before anything below opens a socket or spawns a member. fleetd ticket
|
||||
// "central allow-list of usable models" follow-up: mutation testing found six individual
|
||||
// calls here with nothing proving any of them still ran (deleting one left the full suite
|
||||
// green). validateAll() replaces them with the one call that FleetConfigValidateAllTest
|
||||
// and the Fleetd-startup tests actually pin — see FleetConfig#validateAll's javadoc for
|
||||
// why a name-by-name list here would have the same defect it replaces.
|
||||
cfg.validateAll();
|
||||
|
||||
Path socket = cfg.herdrSocket() != null && !cfg.herdrSocket().isBlank()
|
||||
? Path.of(cfg.herdrSocket())
|
||||
|
||||
@@ -43,6 +43,12 @@ import java.util.function.Supplier;
|
||||
* running daemon keeps whatever this was at startup regardless of a later edit),
|
||||
* {@code spawnReadyTimeoutMs} / {@code spawnReadyPollMs}, {@code quarantineCooldownSeconds}
|
||||
* (CB-578 stage B — baked once into the {@code BackendQuarantine} built at startup),
|
||||
* {@code models:} (fleetd ticket "central allow-list of usable models" — {@link
|
||||
* FleetConfig#validateModels()} re-runs against the fresh config in {@link #reload()}
|
||||
* (via {@link FleetConfig#validateAll()}), so a
|
||||
* models.allow: edit that would refuse to boot still refuses the reload; a change that
|
||||
* passes has nothing built at startup to rebuild, so it is reported deferred rather than
|
||||
* silently accepted with no report at all),
|
||||
* {@code guard:}, {@code worktreeRoot:}, {@code worktreeGroup:} and {@code memberSkills:}
|
||||
* (all three of the latter baked once into the {@code GitWorktrees} built at
|
||||
* {@code Fleetd.java:251} and never rebuilt — fleetd #323 instance 2 found
|
||||
@@ -135,8 +141,9 @@ import java.util.function.Supplier;
|
||||
* </ul>
|
||||
*
|
||||
* <p><strong>The denominator, measured on 2026-09-04 (fleetd #330; recounted for fleetd #333);
|
||||
* recounted again for fleetd #362, and again after {@code idleSleepGuard:} was added.</strong>
|
||||
* {@code FleetConfig} has 24 top-level record components: 5 cold, 13 deferred, 3 split, 3
|
||||
* recounted again for fleetd #362, again after {@code idleSleepGuard:} was added, and again after
|
||||
* {@code models:} was added.</strong>
|
||||
* {@code FleetConfig} has 25 top-level record components: 5 cold, 14 deferred, 3 split, 3
|
||||
* hot-excluded. Three of them are named nowhere in this file, and the reason is the same for all
|
||||
* three: {@code placement}, {@code memberCredentials} and {@code memberLoginShell} are
|
||||
* <strong>hot</strong> and correctly absent — all three are read live off {@code config.get()}
|
||||
@@ -219,7 +226,7 @@ public final class ConfigRef implements Supplier<FleetConfig> {
|
||||
static final Set<String> DEFERRED_KEYS = Set.of(
|
||||
"guard", "worktreeRoot", "worktreeGroup", "memberSkills", "primary", "configReload",
|
||||
"leadHeartbeat", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs",
|
||||
"quarantineCooldownSeconds", "profiles", "idleSleepGuard");
|
||||
"quarantineCooldownSeconds", "profiles", "idleSleepGuard", "models");
|
||||
|
||||
private final Path path;
|
||||
private final AtomicReference<FleetConfig> current;
|
||||
@@ -322,12 +329,12 @@ public final class ConfigRef implements Supplier<FleetConfig> {
|
||||
fresh = FleetConfig.load(path);
|
||||
// The same gate startup runs. A config that would have refused to boot must not be able
|
||||
// to slip in through a reload — that is how a daemon ends up in a state it could never
|
||||
// have started in, which is the hardest kind to debug.
|
||||
fresh.validateAuthExposure();
|
||||
fresh.validateLeadTabPrefixes();
|
||||
fresh.validateSubscriptionProfiles();
|
||||
fresh.validateCharters();
|
||||
fresh.validateMembers();
|
||||
// have started in, which is the hardest kind to debug. fleetd ticket "central allow-list
|
||||
// of usable models" follow-up: this used to be six individual validateXxx() calls, and
|
||||
// mutation testing found two of the six unpinned here even though startup pinned nothing
|
||||
// at all — see FleetConfig#validateAll's javadoc for why the fix is one reflective call,
|
||||
// not a longer hand-maintained list.
|
||||
fresh.validateAll();
|
||||
} catch (RuntimeException e) {
|
||||
String msg = e.getMessage() == null ? e.toString() : e.getMessage();
|
||||
log.warn("config reload from {} refused, keeping the running config: {}", path, msg);
|
||||
@@ -439,6 +446,15 @@ public final class ConfigRef implements Supplier<FleetConfig> {
|
||||
if (!Objects.equals(old.idleSleepGuard(), fresh.idleSleepGuard())) {
|
||||
changed.add("idleSleepGuard");
|
||||
}
|
||||
// fleetd ticket "central allow-list of usable models": validateModels() runs again in
|
||||
// reload() above (via validateAll()), so a bad edit is already refused as cold-adjacent
|
||||
// (the whole reload is refused via the catch block, never partially applied). A GOOD edit
|
||||
// to the allow-list
|
||||
// itself has nothing built at startup to rebuild — it only ever mattered to the validation
|
||||
// call that already ran — so report it deferred rather than silently swallowing the change.
|
||||
if (!Objects.equals(old.models(), fresh.models())) {
|
||||
changed.add("models");
|
||||
}
|
||||
if (!Objects.equals(old.spawnReadyTimeoutMs(), fresh.spawnReadyTimeoutMs())
|
||||
|| !Objects.equals(old.spawnReadyPollMs(), fresh.spawnReadyPollMs())) {
|
||||
changed.add("spawnReady*");
|
||||
|
||||
@@ -16,11 +16,15 @@ import org.slf4j.LoggerFactory;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.UncheckedIOException;
|
||||
import java.lang.reflect.InvocationTargetException;
|
||||
import java.lang.reflect.Method;
|
||||
import java.lang.reflect.Modifier;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.Comparator;
|
||||
import java.util.HashSet;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
@@ -124,6 +128,13 @@ import java.util.regex.PatternSyntaxException;
|
||||
* under a member's long turn. {@code null} (the block omitted) behaves the
|
||||
* same as an explicit {@code enabled: true}; set {@code enabled: false} to
|
||||
* turn it off. See {@link dev.ltms.fleet.power.IdleSleepGuard}.
|
||||
* @param models central allow-list of models any {@code profiles:} entry may name. {@code
|
||||
* null} or an empty {@code allow:} ⇒ off: {@link #validateModels()} checks
|
||||
* nothing and every existing config keeps working exactly as it does today.
|
||||
* When non-empty, a profile whose {@code model:} is not one of {@link
|
||||
* Models#ids()} fails config load, naming both the model and the profile —
|
||||
* see {@link #validateModels()}. This block only decides what may be
|
||||
* CONFIGURED; nothing here enforces it at spawn time. See {@link Models}.
|
||||
*/
|
||||
@JsonIgnoreProperties(ignoreUnknown = true)
|
||||
public record FleetConfig(
|
||||
@@ -150,7 +161,22 @@ public record FleetConfig(
|
||||
String worktreeGroup,
|
||||
String memberLoginShell,
|
||||
String memberSkills,
|
||||
IdleSleepGuard idleSleepGuard) {
|
||||
IdleSleepGuard idleSleepGuard,
|
||||
Models models) {
|
||||
|
||||
/** Back-compat form before the {@code models:} block was added. */
|
||||
public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map<String, Profile> profiles,
|
||||
Guard guard, String worktreeRoot, Lifecycle lifecycle, Integer spawnReadyTimeoutMs,
|
||||
Integer spawnReadyPollMs, Broker broker, Primary primary, Fleet fleet,
|
||||
LeadHeartbeat leadHeartbeat, Health health, String placement, Auth auth,
|
||||
ConfigReload configReload, Integer quarantineCooldownSeconds,
|
||||
MemberCredentials memberCredentials, Coordinator coordinator, String worktreeGroup,
|
||||
String memberLoginShell, String memberSkills, IdleSleepGuard idleSleepGuard) {
|
||||
this(bind, herdrSocket, memberHerdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs,
|
||||
spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, health, placement, auth,
|
||||
configReload, quarantineCooldownSeconds, memberCredentials, coordinator, worktreeGroup,
|
||||
memberLoginShell, memberSkills, idleSleepGuard, null);
|
||||
}
|
||||
|
||||
/** Back-compat form before the {@code idleSleepGuard:} block was added. */
|
||||
public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map<String, Profile> profiles,
|
||||
@@ -1318,6 +1344,72 @@ public record FleetConfig(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Central allow-list of models any {@code profiles:} entry may name (fleetd ticket: "a central
|
||||
* allow-list of usable models"). Nothing before this block checked a profile's {@code model:}
|
||||
* against anything — it was a free-form string handed straight to the backend adapter, and a
|
||||
* withdrawn or misspelled name failed silently (opencode falls back to a default model rather
|
||||
* than erroring on an unknown {@code -m}) rather than at config load, where a mistake is cheap.
|
||||
*
|
||||
* <p><b>Absent or empty {@code allow:} is "off"</b>, on purpose: this is a large deployment
|
||||
* with gitignored {@code fleetd.yaml} on more than one host, and a change that forced every
|
||||
* operator to enumerate their models before the daemon would start would break every one of
|
||||
* them on upgrade. See {@link FleetConfig#validateModels()}, which is where the allow-list is
|
||||
* actually enforced, at config load.
|
||||
*
|
||||
* <p><b>The list is the authority; a {@code profiles:} entry is checked against it, never the
|
||||
* other way around.</b> Adding or editing a {@code profiles:} entry cannot, by itself, widen
|
||||
* the set of permitted models — only editing {@code models.allow:} itself can. This is the
|
||||
* invariant the ticket asked for: the two blocks are validated in one direction only.
|
||||
*
|
||||
* <p><b>Out of scope here, deliberately:</b> nothing in this block is read at spawn time —
|
||||
* enforcing it against a live spawn, an on/off runtime switch, and any interaction with {@code
|
||||
* BackendQuarantine} are separate units. This block is config-load validation only.
|
||||
*
|
||||
* @param allow the permitted models, each its own {@link ModelEntry} rather than a bare
|
||||
* string — see that record's javadoc for why. {@code null}/empty ⇒ the block is
|
||||
* treated as absent: {@link FleetConfig#validateModels()} checks nothing.
|
||||
*/
|
||||
@JsonIgnoreProperties(ignoreUnknown = true)
|
||||
public record Models(List<ModelEntry> allow) {
|
||||
|
||||
public Models {
|
||||
allow = (allow == null) ? List.of() : List.copyOf(allow);
|
||||
}
|
||||
|
||||
/**
|
||||
* One permitted model, named as a record rather than a bare string on purpose: a later unit
|
||||
* needs to hang an on/off state and a load-limit state off each entry, and a bare {@code
|
||||
* List<String>} cannot grow those fields without changing the YAML shape underneath every
|
||||
* operator who already wrote one. {@link #model()} is intentionally a single flat,
|
||||
* opaque-string namespace — a bare Claude id ({@code claude-sonnet-5}) and an opencode
|
||||
* provider-prefixed id ({@code openai/gpt-5.6-terra}) both fit it unchanged, because
|
||||
* {@link FleetConfig#validateModels()} only ever compares a profile's {@code model:} value
|
||||
* against this string for exact equality; it never parses a provider prefix or branches on
|
||||
* a profile's {@code kind:}.
|
||||
*
|
||||
* @param model the model id exactly as a {@code profiles:} entry's {@code model:} field
|
||||
* would name it
|
||||
*/
|
||||
@JsonIgnoreProperties(ignoreUnknown = true)
|
||||
public record ModelEntry(String model) {
|
||||
public ModelEntry {
|
||||
model = (model == null || model.isBlank()) ? null : model.trim();
|
||||
}
|
||||
}
|
||||
|
||||
/** {@link #allow}'s model ids, as a set for membership checks. Blank/null entries are dropped. */
|
||||
public Set<String> ids() {
|
||||
Set<String> ids = new java.util.LinkedHashSet<>();
|
||||
for (ModelEntry e : allow) {
|
||||
if (e != null && e.model() != null) {
|
||||
ids.add(e.model());
|
||||
}
|
||||
}
|
||||
return Collections.unmodifiableSet(ids);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The terminal → lead-name map seeded from the legacy singular {@code primary:} pin (CB-530).
|
||||
*
|
||||
@@ -1569,7 +1661,7 @@ public record FleetConfig(
|
||||
"lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "fleet",
|
||||
"leadHeartbeat", "health", "placement", "auth", "configReload", "quarantineCooldownSeconds",
|
||||
"memberCredentials", "coordinator", "worktreeGroup", "memberLoginShell", "memberSkills",
|
||||
"idleSleepGuard");
|
||||
"idleSleepGuard", "models");
|
||||
|
||||
/** Load and validate config from {@code path}. */
|
||||
public static FleetConfig load(Path path) {
|
||||
@@ -2254,10 +2346,15 @@ public record FleetConfig(
|
||||
// enabled: true — see its javadoc), so defaulting the block here would change nothing a
|
||||
// reader observes and would only obscure that "block omitted" and "block present and
|
||||
// enabled" are deliberately the same outcome.
|
||||
// models is left as-is, like broker/primary/coordinator above: null/empty is "off", and an
|
||||
// absent block must validate nothing (see Models's javadoc) — defaulting it here to an
|
||||
// empty Models would be a no-op for validateModels() either way, since an empty allow-list
|
||||
// already means "check nothing", so there is nothing to gain and one more null check to
|
||||
// avoid by leaving it exactly as configured.
|
||||
return new FleetConfig(b, herdrSocket, memberHerdrSocket, profiles, g, worktreeRoot, l, timeout, pollMs,
|
||||
broker, primary, f, leadHeartbeat, health, placementOrDefault, a, configReload,
|
||||
quarantineCooldown, mc, coordinator, worktreeGroup, memberLoginShell, memberSkills,
|
||||
idleSleepGuard);
|
||||
idleSleepGuard, models);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -2477,6 +2574,118 @@ public record FleetConfig(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Reject a {@code profiles:} entry whose {@code model:} is not on the configured {@link
|
||||
* #models} allow-list.
|
||||
*
|
||||
* <p>Absent or empty {@code models.allow:} validates nothing — see {@link Models}'s javadoc:
|
||||
* an existing config with no such block must keep working exactly as it does today. Once the
|
||||
* operator declares at least one entry, every profile's {@code model:} (when set — a profile
|
||||
* may legitimately leave it {@code null}, e.g. a {@code subscription: true} profile relying on
|
||||
* the account's own default) must equal one of {@link Models#ids()} exactly. The comparison is
|
||||
* a flat string match: a bare Claude id and an opencode {@code provider/model} id are both
|
||||
* just opaque strings here, so nothing here needs to know which {@code kind:} a profile runs.
|
||||
*
|
||||
* <p>The check runs one direction only, by construction: it reads {@link #profiles} and
|
||||
* {@link #models}, and only ever adds to {@code bad} when a profile's model is missing from
|
||||
* the allow-list. Nothing here can be satisfied by widening a {@code profiles:} entry — only
|
||||
* editing {@code models.allow:} itself changes what passes. That is the invariant the ticket
|
||||
* asked for: the list is the authority, profiles are checked against it.
|
||||
*
|
||||
* @throws IllegalStateException when any profile names a model outside the configured
|
||||
* allow-list, naming both the model and the profile that wanted it
|
||||
*/
|
||||
public void validateModels() {
|
||||
if (models == null || models.allow().isEmpty()) {
|
||||
return;
|
||||
}
|
||||
Set<String> allowed = models.ids();
|
||||
List<String> bad = new ArrayList<>();
|
||||
profiles.forEach((name, p) -> {
|
||||
String model = p.model();
|
||||
if (model != null && !model.isBlank() && !allowed.contains(model)) {
|
||||
bad.add("profile '" + name + "' names model '" + model + "', which is not in "
|
||||
+ "models.allow: (have: " + allowed + ").");
|
||||
}
|
||||
});
|
||||
if (!bad.isEmpty()) {
|
||||
throw new IllegalStateException("refusing to start: " + String.join(" ", bad));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs every validator this class declares — found by reflection, not by name.
|
||||
*
|
||||
* <p>fleetd ticket "central allow-list of usable models", follow-up: mutation testing found
|
||||
* that although each of the six validators above was well pinned on its own, nothing proved
|
||||
* either real caller ({@code Fleetd.main} and {@link ConfigRef#reload()}) still
|
||||
* invoked it — deleting a call site left the full suite green. The fix is not a seventh test
|
||||
* per caller; a hand-maintained list of six names here would have the exact same defect its
|
||||
* own javadoc would warn against: the seventh validator someone adds next month has no reason
|
||||
* to be added to it. So this method does not name any validator. It sweeps {@link
|
||||
* #getClass()}'s own public, no-argument, {@code void} methods whose name starts with {@code
|
||||
* "validate"} (excluding itself) and invokes every one it finds, via {@link
|
||||
* #invokeAllValidators}. A new {@code validateXxx()} method is therefore wired into both
|
||||
* callers the moment it is written — there is no second step to forget, and so no state in
|
||||
* which it silently never runs.
|
||||
*
|
||||
* <p>{@code Fleetd.main} and {@link ConfigRef#reload()} each call this one method instead of
|
||||
* the six individually — see the comments at those two call sites for why
|
||||
* each must run it.
|
||||
*
|
||||
* <p>Methods run in a fixed (alphabetical) order, so a config with more than one violation
|
||||
* always names the same one first, on every run.
|
||||
*
|
||||
* @throws IllegalStateException (or whatever unchecked exception a validator itself throws),
|
||||
* propagated unchanged from the first validator, in that order,
|
||||
* that finds a problem
|
||||
*/
|
||||
public void validateAll() {
|
||||
invokeAllValidators(this);
|
||||
}
|
||||
|
||||
/**
|
||||
* The reflective sweep behind {@link #validateAll()}, kept as its own method — taking any
|
||||
* {@code target}, not just {@code this} — so a test can prove the MECHANISM is generic (it
|
||||
* would sweep a seventh {@code validateXxx()} method added to any class, not just something
|
||||
* special-cased to today's six on {@link FleetConfig}) without needing to add a real, unwanted
|
||||
* seventh validator to this class just to exercise that claim. See {@code
|
||||
* FleetConfigValidateAllTest} for that proof.
|
||||
*
|
||||
* @param target an object whose public, no-argument, {@code void} methods named {@code
|
||||
* validateXxx} (any name starting with {@code "validate"}, excluding {@code
|
||||
* validateAll} itself) should all run, in alphabetical-by-name order
|
||||
*/
|
||||
static void invokeAllValidators(Object target) {
|
||||
List<Method> methods = new ArrayList<>();
|
||||
for (Method m : target.getClass().getMethods()) {
|
||||
if (Modifier.isPublic(m.getModifiers())
|
||||
&& m.getParameterCount() == 0
|
||||
&& m.getReturnType() == void.class
|
||||
&& m.getName().startsWith("validate")
|
||||
&& !m.getName().equals("validateAll")) {
|
||||
methods.add(m);
|
||||
}
|
||||
}
|
||||
methods.sort(Comparator.comparing(Method::getName));
|
||||
for (Method m : methods) {
|
||||
try {
|
||||
m.invoke(target);
|
||||
} catch (InvocationTargetException e) {
|
||||
Throwable cause = e.getCause();
|
||||
if (cause instanceof RuntimeException re) {
|
||||
throw re;
|
||||
}
|
||||
if (cause instanceof Error err) {
|
||||
throw err;
|
||||
}
|
||||
throw new IllegalStateException("validator " + m.getName() + " failed", cause);
|
||||
} catch (IllegalAccessException e) {
|
||||
throw new IllegalStateException("cannot invoke validator " + m.getName(), e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** True for the loopback addresses and the unspecified-but-local forms we treat as same-host. */
|
||||
private static boolean isLoopbackBind(String host) {
|
||||
if (host == null || host.isBlank()) {
|
||||
|
||||
@@ -369,8 +369,7 @@ public final class CompositePeerLauncher implements PeerLauncher {
|
||||
// CB-547a: route the chosen profile but keep the caller's session identity — dropping it
|
||||
// here would silently sever the resume handle on every policy-routed spawn. CB-557: the
|
||||
// role rides along for the same reason, or a routed spawn would be labelled as a dev.
|
||||
SpawnRequest routedReq = new SpawnRequest(chosen.profile(), req.requestedCwd(), req.callerCwd(),
|
||||
req.sessionName(), req.resumeSessionId(), req.role());
|
||||
SpawnRequest routedReq = req.withProfile(chosen.profile());
|
||||
try {
|
||||
PeerHandle handle = d.spawn(routedReq);
|
||||
spawnedBy.put(handle.id(), d);
|
||||
|
||||
@@ -13,6 +13,7 @@ import java.nio.file.attribute.PosixFilePermissions;
|
||||
import java.time.Duration;
|
||||
import java.time.Instant;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.stream.Stream;
|
||||
@@ -28,24 +29,46 @@ import java.util.stream.Stream;
|
||||
* control lacked: herdr applies that overlay BEFORE the shell starts, so any sourced file can undo
|
||||
* it — and did.
|
||||
*
|
||||
* <p><b>Which file is last depends on the platform, so the scrub runs from two of them.</b> zsh
|
||||
* <p><b>zsh reads its four startup files under three different conditions, so no single file is
|
||||
* guaranteed to run — the scrub has to cover the gap between them, not just the platforms.</b> zsh
|
||||
* reads {@code .zshenv} always, {@code .zprofile} and {@code .zlogin} only for a LOGIN shell, and
|
||||
* {@code .zshrc} only for an INTERACTIVE one. herdr does not open the same kind of shell
|
||||
* everywhere — measured on herdr 0.8.0: macOS panes run {@code -zsh} (login, so {@code .zlogin}
|
||||
* runs), Linux panes run a plain {@code /usr/bin/zsh} (interactive but NOT login, so
|
||||
* {@code .zlogin} never runs at all). A scrub in {@code .zlogin} alone is therefore a control that
|
||||
* silently does nothing on Linux — the exact failure this class exists to remove, one platform
|
||||
* over.
|
||||
* {@code .zshrc} only for an INTERACTIVE one. A pane shell that is at least one of login or
|
||||
* interactive is covered by sourcing the scrub from {@code .zshrc} and {@code .zlogin} (below), but
|
||||
* a pane shell that is NEITHER reads only {@code .zshenv} and stops — fleetd #388, measured: a herdr
|
||||
* pane can be neither login nor interactive, and such a pane read {@code .zshenv}, never reached
|
||||
* {@code scrub.zsh}, and left no report at all. A bare {@code argv[0]} of {@code /usr/bin/zsh}
|
||||
* proves the shell is NOT a login shell; it says nothing about whether it is interactive, so it
|
||||
* must never be read as "therefore interactive" — that wrong inference is what let #388 ship.
|
||||
*
|
||||
* <p>So both {@code .zshrc} and {@code .zlogin} source the same generated {@code scrub.zsh} after
|
||||
* sourcing their {@code $HOME} counterpart. On Linux only the first fires; on macOS both do, and
|
||||
* the second pass is deliberate rather than merely harmless — it re-scrubs anything the operator's
|
||||
* own {@code ~/.zlogin} exported after {@code .zshrc} had finished. Re-running is idempotent: a
|
||||
* name already blank is blanked again, and the report is rewritten with the same counts.
|
||||
* <p>So {@code .zshenv} carries a THIRD pass, guarded by the exact condition that defines the gap:
|
||||
* {@code [[ ! -o login && ! -o interactive ]]}. That guard is why this pass cannot double-scrub a
|
||||
* pane that {@code .zshrc} or {@code .zlogin} will also cover — one of {@code -o login}/
|
||||
* {@code -o interactive} is always true there, so the {@code .zshenv} pass never fires for them, and
|
||||
* their own unconditional sourcing is untouched. The guard also carries a sentinel
|
||||
* ({@value #SCRUB_SENTINEL}) so it fires once per PANE and not once per PROCESS: {@code .zshenv} is
|
||||
* read by every zsh a member's own tooling forks (a plain {@code zsh -c '...'} for a single
|
||||
* command is itself neither login nor interactive), and those children inherit variables their
|
||||
* parent deliberately set for them (git hooks get {@code GIT_DIR}, a venv gets
|
||||
* {@code VIRTUAL_ENV}, a build tool gets {@code NODE_OPTIONS} or {@code JAVA_TOOL_OPTIONS}).
|
||||
* Re-scrubbing every such child would blank all of that, and would also make the pane's own
|
||||
* {@code scrub-report.txt} — rewritten on every pass — describe whichever child exited last
|
||||
* instead of the pane. The sentinel is exported only AFTER {@code scrub.zsh} runs, so the pass
|
||||
* that sets it never sees it and cannot blank it; it must also be on the scrub's own allow-list
|
||||
* (see {@link #generate(Path, Set)}) so a later pass, in the same pane, cannot blank it back to
|
||||
* empty — an exported-but-empty sentinel reads as unset to the {@code -z} guard and would silently
|
||||
* re-enable scrubbing for every subsequent child of that pane.
|
||||
*
|
||||
* <p>So all three of {@code .zshenv} (gap only, guarded), {@code .zshrc}, and {@code .zlogin}
|
||||
* source the same generated {@code scrub.zsh} after sourcing their {@code $HOME} counterpart. A
|
||||
* login-and-interactive pane runs the {@code .zshrc} and {@code .zlogin} passes, and the second is
|
||||
* deliberate rather than merely harmless — it re-scrubs anything the operator's own
|
||||
* {@code ~/.zlogin} exported after {@code .zshrc} had finished. A pane that is neither runs only the
|
||||
* {@code .zshenv} pass. Re-running is idempotent: a name already blank is blanked again, and the
|
||||
* report is rewritten with the same counts.
|
||||
*
|
||||
* <p>Each generated file sources its {@code $HOME} counterpart FIRST, so {@code PATH} and every
|
||||
* toolchain binary still resolve exactly as the operator configured them; only afterwards does
|
||||
* {@code .zlogin} run the scrub: every EXPORTED variable not on the derived allow-list is re-exported
|
||||
* toolchain binary still resolve exactly as the operator configured them; only afterwards does the
|
||||
* scrub run: every EXPORTED variable not on the derived allow-list is re-exported
|
||||
* blank. Blank, not credential-shaped-pattern-filtered: a pattern list ({@code *TOKEN*}, …) is an
|
||||
* enumeration and misses what it did not think of — a username is the other half of a credential and
|
||||
* is shaped like none. Credential-SHAPED names among the blanked set go to the WARN log only,
|
||||
@@ -63,7 +86,11 @@ public final class EnvAllowListScrub {
|
||||
/** Name of the report file written into the generated directory by the scrub itself. */
|
||||
static final String REPORT_FILE = "scrub-report.txt";
|
||||
|
||||
/** The scrub body, generated once and sourced from both {@code .zshrc} and {@code .zlogin}. */
|
||||
/**
|
||||
* The scrub body, generated once and sourced from {@code .zshrc} and {@code .zlogin}
|
||||
* unconditionally, and from {@code .zshenv} when the pane shell is neither login nor
|
||||
* interactive (fleetd #388) — see the class javadoc.
|
||||
*/
|
||||
static final String SCRUB_FILE = "scrub.zsh";
|
||||
|
||||
/** Prefix of every generated directory — also what {@link #reapOrphans} matches on. */
|
||||
@@ -79,6 +106,28 @@ public final class EnvAllowListScrub {
|
||||
private static final String SOURCE_SCRUB =
|
||||
"source \"$ZDOTDIR/" + SCRUB_FILE + "\"\n";
|
||||
|
||||
/**
|
||||
* fleetd #388: marks a pane, not a process, as already scrubbed. Set only by the guarded
|
||||
* {@code .zshenv} pass (see {@link #NEITHER_LOGIN_NOR_INTERACTIVE_SCRUB}) after
|
||||
* {@code scrub.zsh} has run, so it must also be folded into that pass's own allow-list — see
|
||||
* the class javadoc's "must also be on the scrub's own allow-list" paragraph.
|
||||
*/
|
||||
static final String SCRUB_SENTINEL = "_CB633_SCRUBBED";
|
||||
|
||||
/**
|
||||
* Appended to {@code .zshenv}, after its {@code $HOME} source: the third pass, guarded on the
|
||||
* exact condition that defines the gap {@code .zshrc}/{@code .zlogin} do not cover — a shell
|
||||
* that is neither login nor interactive. The sentinel export happens only once the scrub has
|
||||
* already run, and only for as long as the current pane's environment has not been rebuilt from
|
||||
* scratch (a fresh {@code env -i} child would not inherit it — that is out of scope here, since
|
||||
* such a child is no longer running under the pane's own environment at all).
|
||||
*/
|
||||
private static final String NEITHER_LOGIN_NOR_INTERACTIVE_SCRUB =
|
||||
"if [[ ! -o login && ! -o interactive && -z \"${" + SCRUB_SENTINEL + ":-}\" ]]; then\n"
|
||||
+ " " + SOURCE_SCRUB
|
||||
+ " export " + SCRUB_SENTINEL + "=1\n"
|
||||
+ "fi\n";
|
||||
|
||||
private EnvAllowListScrub() {
|
||||
}
|
||||
|
||||
@@ -105,11 +154,16 @@ public final class EnvAllowListScrub {
|
||||
reapOrphans(parentDir);
|
||||
Path dir = Files.createTempDirectory(parentDir, DIR_PREFIX);
|
||||
dir.toFile().deleteOnExit();
|
||||
// The report is written by zsh, after these hooks are registered, so register its path
|
||||
// too — otherwise the directory is non-empty at JVM exit and cannot be removed at all.
|
||||
dir.resolve(REPORT_FILE).toFile().deleteOnExit();
|
||||
write(dir, SCRUB_FILE, scrubScript(allowedNames));
|
||||
write(dir, ".zshenv", homeSourcingFile(".zshenv"));
|
||||
// zsh truncates this pre-created receipt after these hooks are registered. Register its
|
||||
// path too — otherwise the directory is non-empty at JVM exit and cannot be removed.
|
||||
Files.createFile(dir.resolve(REPORT_FILE)).toFile().deleteOnExit();
|
||||
// fleetd #388: scrub.zsh's OWN allow-list must also keep SCRUB_SENTINEL, or a later
|
||||
// pass in the same pane blanks it back to empty and the .zshenv guard below thinks it
|
||||
// was never scrubbed — see the class javadoc.
|
||||
Set<String> namesForScrubScript = new HashSet<>(allowedNames);
|
||||
namesForScrubScript.add(SCRUB_SENTINEL);
|
||||
write(dir, SCRUB_FILE, scrubScript(namesForScrubScript));
|
||||
write(dir, ".zshenv", homeSourcingFile(".zshenv") + NEITHER_LOGIN_NOR_INTERACTIVE_SCRUB);
|
||||
write(dir, ".zprofile", homeSourcingFile(".zprofile"));
|
||||
write(dir, ".zshrc", homeSourcingFile(".zshrc") + SOURCE_SCRUB);
|
||||
write(dir, ".zlogin", homeSourcingFile(".zlogin") + SOURCE_SCRUB);
|
||||
@@ -147,12 +201,11 @@ public final class EnvAllowListScrub {
|
||||
* into it: owner keeps full access, {@code group} gets traverse+read on the directory ({@code
|
||||
* rwxr-x---}, so a member process — a login shell reading it via {@code ZDOTDIR}, or another
|
||||
* process simply opening a file under it — running under that group can find and read the
|
||||
* files) and read-only on each file ({@code rw-r-----}) — deliberately no group WRITE anywhere,
|
||||
* since a member never needs to add or change what fleetd generated. (For the ZDOTDIR scrub
|
||||
* specifically, this also means the scrub script's own report write inside the pane fails
|
||||
* closed rather than open — see {@code scrub.zsh}'s trailing {@code 2>/dev/null} — which {@link
|
||||
* dev.ltms.fleet.member.HerdrPeerLauncher#releaseZdotdir} already treats as "cannot be
|
||||
* confirmed to have run" rather than success.)
|
||||
* files) and read-only on each file ({@code rw-r-----}), except the pre-created ZDOTDIR
|
||||
* {@code scrub-report.txt}. That receipt gets group write ({@code rw-rw----}), so
|
||||
* {@code scrub.zsh} can truncate and write it without granting group write on the directory.
|
||||
* If its optional permission change fails, the member cannot write a receipt and the launcher
|
||||
* keeps its existing WARN rather than failing the spawn.
|
||||
*
|
||||
* <p>Package-private and named generically on purpose: fleetd #213 built this for the ZDOTDIR
|
||||
* scrub directory, and fleetd #219 reuses it verbatim for {@link
|
||||
@@ -167,7 +220,15 @@ public final class EnvAllowListScrub {
|
||||
setGroupAndPermissions(dir, principal, "rwxr-x---");
|
||||
try (Stream<Path> entries = Files.list(dir)) {
|
||||
for (Path file : entries.toList()) {
|
||||
setGroupAndPermissions(file, principal, "rw-r-----");
|
||||
if (REPORT_FILE.equals(file.getFileName().toString())) {
|
||||
try {
|
||||
setGroupAndPermissions(file, principal, "rw-rw----");
|
||||
} catch (IOException | UnsupportedOperationException ignored) {
|
||||
// The receipt is optional. Its absence keeps the existing WARN path.
|
||||
}
|
||||
} else {
|
||||
setGroupAndPermissions(file, principal, "rw-r-----");
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (IOException e) {
|
||||
@@ -245,11 +306,13 @@ public final class EnvAllowListScrub {
|
||||
}
|
||||
return """
|
||||
# generated by fleetd (CB-633 memberCredentials policy=allow-list) — do not edit.
|
||||
# Sourced from .zshrc and again from .zlogin, each time AFTER that file has sourced
|
||||
# its $HOME counterpart — so this runs after everything the operator sourced, on a
|
||||
# login shell (macOS panes) and on a plain interactive one (Linux panes) alike.
|
||||
# Running twice is idempotent and deliberate: the second pass catches anything
|
||||
# ~/.zlogin exported after ~/.zshrc had finished.
|
||||
# Sourced unconditionally from .zshrc and again from .zlogin, each time AFTER that
|
||||
# file has sourced its $HOME counterpart — so this runs after everything the
|
||||
# operator sourced, on any pane that is login and/or interactive. Running twice is
|
||||
# idempotent and deliberate: the second pass catches anything ~/.zlogin exported
|
||||
# after ~/.zshrc had finished. Also sourced, once, from a guarded pass in .zshenv
|
||||
# (fleetd #388) when the pane shell is NEITHER login nor interactive — the one gap
|
||||
# those two files do not cover.
|
||||
|
||||
typeset -A _cb633_allowed
|
||||
for _cb633_n in %s; do _cb633_allowed[$_cb633_n]=1; done
|
||||
|
||||
@@ -29,4 +29,9 @@ public record SpawnRequest(String profileName, String requestedCwd, String calle
|
||||
String sessionName, String resumeSessionId) {
|
||||
this(profileName, requestedCwd, callerCwd, sessionName, resumeSessionId, null);
|
||||
}
|
||||
|
||||
/** Return a copy of this request with {@code profileName} replaced by {@code profile}. */
|
||||
public SpawnRequest withProfile(String profile) {
|
||||
return new SpawnRequest(profile, requestedCwd, callerCwd, sessionName, resumeSessionId, role);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
package dev.ltms.fleet;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/**
|
||||
* Proves the one thing no test proved before this ticket's follow-up: that {@code Fleetd.main}
|
||||
* ITSELF — not a copy of its logic, not the validator called directly — still refuses to start on
|
||||
* a bad config. Mutation testing found that deleting {@code cfg.validateAll();} (née six
|
||||
* individual {@code cfg.validateXxx();} calls) from {@code Fleetd.main} left the full 1478-test
|
||||
* suite green; every existing test called a validator directly and none exercised {@code
|
||||
* Fleetd.main} as the caller. See {@code FleetConfigValidateAllTest} for why the fix collapses
|
||||
* those six calls into one reflective {@link FleetConfig#validateAll()}, and {@code
|
||||
* ConfigRefTest} for the equivalent proof on the {@link
|
||||
* dev.ltms.fleet.config.ConfigRef#reload()} path.
|
||||
*
|
||||
* <p>This is deliberately the real {@code static void main(String[] args)} — package-private, so
|
||||
* only a test in this package can call it, which is exactly what makes this proof strong: it is
|
||||
* not a helper extracted for testability, it is the literal method {@code java -jar fleetd.jar}
|
||||
* invokes. Every fixture below is otherwise-valid and fails exactly one validator, and — because
|
||||
* {@link FleetConfig#validateAll()} runs immediately after {@code SubscriptionGuard.
|
||||
* assertPrimaryClean}, before {@code main} opens the herdr socket, binds Javalin, or touches
|
||||
* anything else with a real side effect — calling {@code Fleetd.main} with one of these configs is
|
||||
* safe: it is guaranteed to throw before reaching any of that, precisely because the config is
|
||||
* deliberately invalid.
|
||||
*/
|
||||
class FleetdStartupValidationTest {
|
||||
|
||||
private static void assertMainRefuses(Path dir, String fileName, String yaml,
|
||||
String mustContain) throws Exception {
|
||||
Path f = dir.resolve(fileName);
|
||||
Files.writeString(f, yaml);
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class,
|
||||
() -> Fleetd.main(new String[]{f.toString()}),
|
||||
fileName + ": Fleetd.main must refuse this config before doing anything else");
|
||||
assertTrue(e.getMessage().contains(mustContain),
|
||||
fileName + ": expected message to contain \"" + mustContain + "\" but was: "
|
||||
+ e.getMessage());
|
||||
}
|
||||
|
||||
@Test
|
||||
void mainRefusesANonLoopbackBindWithoutTokenMode(@TempDir Path dir) throws Exception {
|
||||
assertMainRefuses(dir, "auth-exposure.yaml", """
|
||||
bind:
|
||||
host: 0.0.0.0
|
||||
port: 8765
|
||||
""", "auth.mode: token");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mainRefusesALeadTabPrefixCollision(@TempDir Path dir) throws Exception {
|
||||
assertMainRefuses(dir, "lead-tab-prefixes.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
fleet:
|
||||
tabLabel: "lead: {role} {profile}"
|
||||
leaders:
|
||||
opus:
|
||||
tab: "lead: opus"
|
||||
""", "fleet.tabLabel");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mainRefusesASubscriptionProfileThatReseatsAnthropicBaseUrl(@TempDir Path dir)
|
||||
throws Exception {
|
||||
assertMainRefuses(dir, "subscription-profiles.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
profiles:
|
||||
sonnet:
|
||||
subscription: true
|
||||
argv: ["ccs", "sonnet"]
|
||||
env:
|
||||
ANTHROPIC_BASE_URL: http://anything-not-on-the-allowlist
|
||||
""", "ANTHROPIC_BASE_URL");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mainRefusesAnUnknownCharterKey(@TempDir Path dir) throws Exception {
|
||||
assertMainRefuses(dir, "charters.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
fleet:
|
||||
charters:
|
||||
architetc: text
|
||||
""", "architetc");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mainRefusesAnArchitectSlotNamingAnUnconfiguredProfile(@TempDir Path dir) throws Exception {
|
||||
assertMainRefuses(dir, "members.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
profiles:
|
||||
gx10:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
fleet:
|
||||
architects:
|
||||
lead-designer:
|
||||
profile: sonnet
|
||||
""", "lead-designer");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mainRefusesAProfileNamingAModelOutsideTheAllowList(@TempDir Path dir) throws Exception {
|
||||
assertMainRefuses(dir, "models.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
profiles:
|
||||
sonnet:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
model: claude-sonnet-5
|
||||
rogue:
|
||||
baseUrl: http://gx11.gw:8000
|
||||
model: claude-opus-9000
|
||||
models:
|
||||
allow:
|
||||
- model: claude-sonnet-5
|
||||
""", "rogue");
|
||||
}
|
||||
}
|
||||
@@ -109,6 +109,8 @@ class ConfigRefTopLevelReportingCoverageTest {
|
||||
v.put("memberLoginShell", null);
|
||||
v.put("memberSkills", "/skills/a");
|
||||
v.put("idleSleepGuard", new FleetConfig.IdleSleepGuard(true));
|
||||
v.put("models", new FleetConfig.Models(
|
||||
List.of(new FleetConfig.Models.ModelEntry("model-a"))));
|
||||
assertNamesMatchComponents(v);
|
||||
return v;
|
||||
}
|
||||
@@ -151,6 +153,8 @@ class ConfigRefTopLevelReportingCoverageTest {
|
||||
v.put("memberLoginShell", null);
|
||||
v.put("memberSkills", "/skills/b");
|
||||
v.put("idleSleepGuard", new FleetConfig.IdleSleepGuard(false));
|
||||
v.put("models", new FleetConfig.Models(
|
||||
List.of(new FleetConfig.Models.ModelEntry("model-b"))));
|
||||
assertNamesMatchComponents(v);
|
||||
return v;
|
||||
}
|
||||
|
||||
@@ -2683,4 +2683,222 @@ class FleetConfigTest {
|
||||
assertTrue(cfg.idleSleepGuard().isEnabled(),
|
||||
"unlike ConfigReload/Health, this block defaults to ON even when present but empty");
|
||||
}
|
||||
|
||||
// --- models: central allow-list of usable models -----------------------------------------
|
||||
|
||||
/**
|
||||
* An absent {@code models:} block is today's behaviour exactly: no profile's {@code model:} is
|
||||
* checked against anything, whatever it says. This is the "existing config keeps working"
|
||||
* invariant — an operator on a gitignored {@code fleetd.yaml} that predates this feature must
|
||||
* not be broken by upgrading the daemon.
|
||||
*/
|
||||
@Test
|
||||
void absentModelsBlockValidatesNothing(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
model: totally-unheard-of-model-xyz
|
||||
""");
|
||||
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
assertDoesNotThrow(cfg::validateModels);
|
||||
}
|
||||
|
||||
/**
|
||||
* {@code models:} present but with an empty (or absent) {@code allow:} must behave exactly like
|
||||
* the block being absent — an operator adding the block for the first time with nothing in it
|
||||
* yet must not be surprised by every profile suddenly refusing to start.
|
||||
*/
|
||||
@Test
|
||||
void modelsBlockPresentButEmptyValidatesNothing(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
model: whatever-the-operator-typed
|
||||
models:
|
||||
allow: []
|
||||
""");
|
||||
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
assertDoesNotThrow(cfg::validateModels);
|
||||
}
|
||||
|
||||
/**
|
||||
* The core of the ticket: a profile naming a model outside the configured allow-list fails
|
||||
* config load, naming both the model and the profile that wanted it.
|
||||
*/
|
||||
@Test
|
||||
void aProfileNamingAModelOutsideTheAllowListRefusesToStart(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
sonnet:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
model: claude-sonnet-5
|
||||
rogue:
|
||||
baseUrl: http://gx11.gw:8000
|
||||
model: claude-opus-9000
|
||||
models:
|
||||
allow:
|
||||
- model: claude-sonnet-5
|
||||
- model: claude-haiku-5
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateModels);
|
||||
assertTrue(e.getMessage().contains("rogue"), "the refusal names the offending profile");
|
||||
assertTrue(e.getMessage().contains("claude-opus-9000"), "the refusal names the offending model");
|
||||
assertFalse(e.getMessage().contains("'sonnet'"),
|
||||
"the profile whose model IS allowed must not be reported");
|
||||
}
|
||||
|
||||
/** A profile whose {@code model:} is on the allow-list loads fine. */
|
||||
@Test
|
||||
void aProfileNamingAModelOnTheAllowListLoadsFine(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
sonnet:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
model: claude-sonnet-5
|
||||
models:
|
||||
allow:
|
||||
- model: claude-sonnet-5
|
||||
""");
|
||||
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
assertDoesNotThrow(cfg::validateModels);
|
||||
}
|
||||
|
||||
/**
|
||||
* A profile that never sets {@code model:} (a {@code subscription: true} profile relying on the
|
||||
* account's own default is the live example) must not be refused just because an allow-list is
|
||||
* active — there is nothing to check it against.
|
||||
*/
|
||||
@Test
|
||||
void aProfileWithNoModelPassesEvenWithAnActiveAllowList(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
opus:
|
||||
subscription: true
|
||||
models:
|
||||
allow:
|
||||
- model: claude-sonnet-5
|
||||
""");
|
||||
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
assertDoesNotThrow(cfg::validateModels);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reproduces the live config shape this ticket measured: a mix of {@code amazon-bedrock},
|
||||
* {@code opencode} and {@code openai}-backed profiles, some naming a bare Claude id and some a
|
||||
* provider-prefixed opencode id, in ONE allow-list. Both forms are just opaque strings compared
|
||||
* for exact equality — proves the shape decision holds against the shape actually seen live,
|
||||
* not just against a synthetic single-provider example.
|
||||
*/
|
||||
@Test
|
||||
void aBareClaudeIdAndAnOpencodeProviderPrefixedIdBothFitOneAllowList(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
sonnet:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
model: claude-sonnet-5
|
||||
terra:
|
||||
kind: opencode
|
||||
model: openai/gpt-5.6-terra
|
||||
nova:
|
||||
kind: opencode
|
||||
model: amazon-bedrock/amazon.nova-pro-v1:0
|
||||
models:
|
||||
allow:
|
||||
- model: claude-sonnet-5
|
||||
- model: openai/gpt-5.6-terra
|
||||
- model: amazon-bedrock/amazon.nova-pro-v1:0
|
||||
""");
|
||||
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
assertDoesNotThrow(cfg::validateModels);
|
||||
}
|
||||
|
||||
/** The same live shape, but one opencode profile's model is missing from the allow-list. */
|
||||
@Test
|
||||
void anUnlistedOpencodeProviderPrefixedModelRefusesToStart(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
sonnet:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
model: claude-sonnet-5
|
||||
terra:
|
||||
kind: opencode
|
||||
model: openai/gpt-5.6-terra-withdrawn
|
||||
models:
|
||||
allow:
|
||||
- model: claude-sonnet-5
|
||||
- model: openai/gpt-5.6-terra
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateModels);
|
||||
assertTrue(e.getMessage().contains("terra"), "the refusal names the offending profile");
|
||||
assertTrue(e.getMessage().contains("openai/gpt-5.6-terra-withdrawn"),
|
||||
"the refusal names the offending model, with its provider prefix intact");
|
||||
}
|
||||
|
||||
/**
|
||||
* Editing a {@code profiles:} entry alone must never be able to widen what is permitted — only
|
||||
* editing {@code models.allow:} itself can. This is the invariant the ticket states explicitly;
|
||||
* this test pins it by giving a profile a plausible-looking model that was never added to the
|
||||
* allow-list and confirming it is still refused.
|
||||
*/
|
||||
@Test
|
||||
void addingAProfileCannotWidenTheAllowListByItself(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
sonnet:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
model: claude-sonnet-5
|
||||
brand-new:
|
||||
baseUrl: http://gx12.gw:8000
|
||||
model: claude-sonnet-6-preview
|
||||
models:
|
||||
allow:
|
||||
- model: claude-sonnet-5
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateModels);
|
||||
assertTrue(e.getMessage().contains("brand-new"));
|
||||
assertTrue(e.getMessage().contains("claude-sonnet-6-preview"));
|
||||
}
|
||||
|
||||
/** {@code models} is a brand-new top-level key and must be recognized, not WARN-ed as unknown. */
|
||||
@Test
|
||||
void modelsIsAKnownTopLevelKey() {
|
||||
assertTrue(FleetConfig.KNOWN_TOP_LEVEL_KEYS.contains("models"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,358 @@
|
||||
package dev.ltms.fleet.config;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
|
||||
import java.lang.reflect.Method;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.TreeSet;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/**
|
||||
* The gap this class exists to close: mutation testing on the fleetd ticket "central allow-list
|
||||
* of usable models" found that although {@link FleetConfig#validateModels()}'s own logic was well
|
||||
* pinned, nothing proved either real caller ({@code Fleetd.main} and {@link ConfigRef#reload()})
|
||||
* still invoked it — deleting the call site left the full suite green (1478/0/0/0). A follow-up
|
||||
* measurement (same technique — remove one call site, run the suite, not read the code) found the
|
||||
* SAME gap for all five of {@link FleetConfig}'s other validators at startup, and for four of the
|
||||
* six inside {@link ConfigRef#reload()}. This is a class of gap, not one line's mistake: every one
|
||||
* of those thirteen tests called the validator itself directly, never the real caller that was
|
||||
* supposed to.
|
||||
*
|
||||
* <p>The fix replaces the six individual {@code cfg.validateXxx()} calls at each of the two real
|
||||
* call sites with one {@link FleetConfig#validateAll()}, which reaches every validator by
|
||||
* reflection rather than by a hand-maintained list of names. A hand-maintained list of six names
|
||||
* would have exactly the defect it replaces: the seventh validator someone adds next month has no
|
||||
* reason to be added to it, and nothing would say so. This class proves TWO separate claims, and
|
||||
* keeps them separate on purpose:
|
||||
*
|
||||
* <ol>
|
||||
* <li>{@link #theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames()} and its neighbours
|
||||
* prove the reflective sweep itself ({@link FleetConfig#invokeAllValidators}) is a general
|
||||
* mechanism — it runs whatever public, no-arg, void {@code validateXxx()} methods a class
|
||||
* happens to declare today, including a class with more of them than {@link FleetConfig}
|
||||
* has right now. This is the proof that a future, real seventh validator on {@link
|
||||
* FleetConfig} would be swept automatically, without needing to add a real (unwanted)
|
||||
* seventh validator just to exercise the claim.</li>
|
||||
* <li>{@link #validateAllReachesEveryOneOfTodaysSixValidators()} proves {@link
|
||||
* FleetConfig#validateAll()} itself is wired to that same generic mechanism and genuinely
|
||||
* reaches each of today's six real validators — reusing the exact minimal failing
|
||||
* configurations {@code FleetConfigTest} already established for each one directly, so a
|
||||
* single call to {@code validateAll()} is shown to reproduce every one of those six
|
||||
* failures.</li>
|
||||
* </ol>
|
||||
*
|
||||
* <p>Together with the direct-{@code Fleetd.main}-invocation tests in {@code
|
||||
* FleetdStartupValidationTest} (which prove the real startup call site still calls {@code
|
||||
* validateAll()}) and the {@code ConfigRefTest} reload tests (which prove the same for {@link
|
||||
* ConfigRef#reload()}), removing {@code cfg.validateAll();} from either real call site now fails
|
||||
* a test in this module.
|
||||
*
|
||||
* <p><b>What is NOT pinned, measured rather than assumed.</b> Reverting {@link
|
||||
* FleetConfig#validateAll()} to a hardcoded list of today's six method calls leaves the whole
|
||||
* suite green (measured at review: 1491 tests, 0 failures). Nothing ties {@code validateAll()} to
|
||||
* the generic sweep — claim 1 proves {@link FleetConfig#invokeAllValidators} is generic, and claim
|
||||
* 2 proves {@code validateAll()} reaches today's six, and a hardcoded list satisfies both. So the
|
||||
* reflective sweep is a convenience, not the guarantee. The guarantee is {@link
|
||||
* #fleetConfigDeclaresExactlyTheseSixValidatorsToday()}: it fails the moment a seventh validator
|
||||
* is declared, which forces whoever adds it to look at this file.
|
||||
*/
|
||||
class FleetConfigValidateAllTest {
|
||||
|
||||
// ── Claim 1: the reflective sweep is a general mechanism, not six names in disguise ──────────
|
||||
|
||||
/**
|
||||
* A throwaway fixture class, unrelated to {@link FleetConfig} in every way except shape: three
|
||||
* public, no-arg, void methods named {@code validateXxx}. Proves the sweep works on ANY class
|
||||
* with this shape, not on something special-cased to {@link FleetConfig}.
|
||||
*/
|
||||
static class ThreeValidators {
|
||||
final List<String> ran = new ArrayList<>();
|
||||
|
||||
public void validateAlpha() {
|
||||
ran.add("validateAlpha");
|
||||
}
|
||||
|
||||
public void validateBeta() {
|
||||
ran.add("validateBeta");
|
||||
}
|
||||
|
||||
public void validateGamma() {
|
||||
ran.add("validateGamma");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames() {
|
||||
ThreeValidators target = new ThreeValidators();
|
||||
FleetConfig.invokeAllValidators(target);
|
||||
assertEquals(List.of("validateAlpha", "validateBeta", "validateGamma"), target.ran,
|
||||
"every validateXxx() method on this unrelated class must run, in alphabetical "
|
||||
+ "order — the sweep reads the class's own shape, not a name FleetConfig "
|
||||
+ "happens to know about");
|
||||
}
|
||||
|
||||
/**
|
||||
* The core of the "self-maintaining" requirement: the exact same class shape as {@link
|
||||
* ThreeValidators}, plus one more method — standing in for "a developer adds a validator next
|
||||
* month". Nothing about the sweep changes to pick it up; the new method is invoked purely
|
||||
* because it exists and matches the shape. This is what makes adding a seventh real validator
|
||||
* to {@link FleetConfig} safe without touching {@link FleetConfig#validateAll()} or either
|
||||
* call site — there is no "wire it in" step left to forget.
|
||||
*/
|
||||
static class FourValidators {
|
||||
final List<String> ran = new ArrayList<>();
|
||||
|
||||
public void validateAlpha() {
|
||||
ran.add("validateAlpha");
|
||||
}
|
||||
|
||||
public void validateBeta() {
|
||||
ran.add("validateBeta");
|
||||
}
|
||||
|
||||
public void validateGamma() {
|
||||
ran.add("validateGamma");
|
||||
}
|
||||
|
||||
public void validateDelta() {
|
||||
ran.add("validateDelta");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void addingAFourthValidatorMethodGetsSweptWithNoOtherChange() {
|
||||
FourValidators target = new FourValidators();
|
||||
FleetConfig.invokeAllValidators(target);
|
||||
assertEquals(List.of("validateAlpha", "validateBeta", "validateDelta", "validateGamma"),
|
||||
sorted(target.ran),
|
||||
"the fourth method must be reached automatically — proving a class can grow the "
|
||||
+ "set of things it validates with no change to the sweep itself");
|
||||
}
|
||||
|
||||
private static List<String> sorted(List<String> in) {
|
||||
List<String> copy = new ArrayList<>(in);
|
||||
copy.sort(String::compareTo);
|
||||
return copy;
|
||||
}
|
||||
|
||||
@Test
|
||||
void aFailingValidatorStopsTheSweepAndPropagatesUnchanged() {
|
||||
class OneFails {
|
||||
@SuppressWarnings("unused")
|
||||
public void validateOk() {
|
||||
// passes
|
||||
}
|
||||
|
||||
public void validateBoom() {
|
||||
throw new IllegalStateException("refusing to start: boom");
|
||||
}
|
||||
}
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class,
|
||||
() -> FleetConfig.invokeAllValidators(new OneFails()));
|
||||
assertEquals("refusing to start: boom", e.getMessage(),
|
||||
"the real exception must propagate unchanged, not be wrapped or swallowed");
|
||||
}
|
||||
|
||||
/**
|
||||
* Every rule the sweep's filter applies, proven independently: only public, no-arg, void
|
||||
* methods whose name starts with {@code "validate"} run, {@code validateAll} itself is
|
||||
* excluded (so a class that declares one of its own — as {@link FleetConfig} does — cannot
|
||||
* recurse into itself), and a same-shaped-but-wrongly-named or wrongly-shaped method never
|
||||
* runs. A reader who "simplifies" the filter in {@link FleetConfig#invokeAllValidators} in a
|
||||
* way that widens or narrows it breaks one of these.
|
||||
*/
|
||||
static class FilterEdgeCases {
|
||||
final List<String> ran = new ArrayList<>();
|
||||
|
||||
public void validateReal() {
|
||||
ran.add("validateReal");
|
||||
}
|
||||
|
||||
/** Wrong name — must not run. */
|
||||
public void checkSomething() {
|
||||
ran.add("checkSomething");
|
||||
}
|
||||
|
||||
/** Wrong shape — takes an argument. */
|
||||
public void validateWithArg(String ignored) {
|
||||
ran.add("validateWithArg");
|
||||
}
|
||||
|
||||
/** Wrong shape — returns something. */
|
||||
public boolean validateReturnsBoolean() {
|
||||
ran.add("validateReturnsBoolean");
|
||||
return true;
|
||||
}
|
||||
|
||||
/** Excluded by name on purpose, so the sweep cannot call itself. */
|
||||
public void validateAll() {
|
||||
ran.add("validateAll");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void onlyPublicNoArgVoidValidateNamedMethodsRun() {
|
||||
FilterEdgeCases target = new FilterEdgeCases();
|
||||
FleetConfig.invokeAllValidators(target);
|
||||
assertEquals(List.of("validateReal"), target.ran,
|
||||
"checkSomething (wrong name), validateWithArg (wrong shape), "
|
||||
+ "validateReturnsBoolean (wrong shape), and validateAll (excluded by "
|
||||
+ "name) must all be skipped");
|
||||
}
|
||||
|
||||
// ── Claim 2: FleetConfig.validateAll() is wired to that mechanism and reaches all six today ──
|
||||
|
||||
/**
|
||||
* Reflectively enumerates {@link FleetConfig}'s own public, no-arg, void {@code validateXxx()}
|
||||
* methods (excluding {@code validateAll} itself) — the exact same filter {@link
|
||||
* FleetConfig#invokeAllValidators} applies. This is not the mechanism proof (that is claim 1,
|
||||
* above, on an unrelated class) — it is a visible denominator: today there are six, named
|
||||
* here, so a reader adding a seventh sees this assertion name the new count rather than a
|
||||
* silent pass at the old one.
|
||||
*/
|
||||
@Test
|
||||
void fleetConfigDeclaresExactlyTheseSixValidatorsToday() {
|
||||
Set<String> names = new TreeSet<>();
|
||||
for (Method m : FleetConfig.class.getMethods()) {
|
||||
if (java.lang.reflect.Modifier.isPublic(m.getModifiers())
|
||||
&& m.getParameterCount() == 0
|
||||
&& m.getReturnType() == void.class
|
||||
&& m.getName().startsWith("validate")
|
||||
&& !m.getName().equals("validateAll")) {
|
||||
names.add(m.getName());
|
||||
}
|
||||
}
|
||||
assertEquals(new TreeSet<>(Set.of("validateAuthExposure", "validateLeadTabPrefixes",
|
||||
"validateSubscriptionProfiles", "validateCharters", "validateMembers",
|
||||
"validateModels")), names,
|
||||
"FleetConfig's public validate*() methods changed. Do TWO things, in this "
|
||||
+ "order. First confirm validateAll() still delegates to "
|
||||
+ "invokeAllValidators(this) — a hardcoded list there passes every other "
|
||||
+ "test in this class, so this assertion is the only place that will ever "
|
||||
+ "make you check. Only then update the expected set to match.");
|
||||
}
|
||||
|
||||
/** A minimal, otherwise-valid file — same shape FleetConfigTest and ConfigRefTest use. */
|
||||
private static String minimalValidYaml() {
|
||||
return """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
profiles:
|
||||
sonnet:
|
||||
baseUrl: http://gx00.gw:8000
|
||||
model: sonnet
|
||||
""";
|
||||
}
|
||||
|
||||
@Test
|
||||
void aFullyValidConfigPassesValidateAll(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, minimalValidYaml());
|
||||
assertDoesNotThrow(() -> FleetConfig.load(f).validateAll());
|
||||
}
|
||||
|
||||
/**
|
||||
* The heart of claim 2: for each of today's six real validators, a minimal file that fails
|
||||
* ONLY that one — the exact fixtures {@code FleetConfigTest} uses to test each validator
|
||||
* directly — must also fail through {@link FleetConfig#validateAll()}. If a future edit to
|
||||
* {@code validateAll()} silently dropped one validator from the sweep (e.g. a typo'd name
|
||||
* filter), exactly one of these six would start passing when it must not.
|
||||
*/
|
||||
@Test
|
||||
void validateAllReachesEveryOneOfTodaysSixValidators(@TempDir Path dir) throws Exception {
|
||||
// validateAuthExposure: a non-loopback bind without token mode.
|
||||
assertValidateAllRefuses(dir, "auth-exposure.yaml", """
|
||||
bind:
|
||||
host: 0.0.0.0
|
||||
port: 8765
|
||||
""", "auth.mode: token");
|
||||
|
||||
// validateLeadTabPrefixes: a fleet-wide tabLabel that starts with a lead's own tabPrefix.
|
||||
assertValidateAllRefuses(dir, "lead-tab-prefixes.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
fleet:
|
||||
tabLabel: "lead: {role} {profile}"
|
||||
leaders:
|
||||
opus:
|
||||
tab: "lead: opus"
|
||||
""", "fleet.tabLabel");
|
||||
|
||||
// validateSubscriptionProfiles: subscription: true with env: reseating ANTHROPIC_BASE_URL.
|
||||
assertValidateAllRefuses(dir, "subscription-profiles.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
profiles:
|
||||
sonnet:
|
||||
subscription: true
|
||||
argv: ["ccs", "sonnet"]
|
||||
env:
|
||||
ANTHROPIC_BASE_URL: http://anything-not-on-the-allowlist
|
||||
""", "ANTHROPIC_BASE_URL");
|
||||
|
||||
// validateCharters: a charter key that is not a role wire name.
|
||||
assertValidateAllRefuses(dir, "charters.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
fleet:
|
||||
charters:
|
||||
architetc: text
|
||||
""", "architetc");
|
||||
|
||||
// validateMembers: an architect slot naming an unconfigured profile.
|
||||
assertValidateAllRefuses(dir, "members.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
profiles:
|
||||
gx10:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
fleet:
|
||||
architects:
|
||||
lead-designer:
|
||||
profile: sonnet
|
||||
""", "lead-designer");
|
||||
|
||||
// validateModels: a profile naming a model outside the configured allow-list.
|
||||
assertValidateAllRefuses(dir, "models.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
profiles:
|
||||
sonnet:
|
||||
baseUrl: http://gx10.gw:8000
|
||||
model: claude-sonnet-5
|
||||
rogue:
|
||||
baseUrl: http://gx11.gw:8000
|
||||
model: claude-opus-9000
|
||||
models:
|
||||
allow:
|
||||
- model: claude-sonnet-5
|
||||
""", "rogue");
|
||||
}
|
||||
|
||||
private static void assertValidateAllRefuses(Path dir, String fileName, String yaml,
|
||||
String mustContain) throws Exception {
|
||||
Path f = dir.resolve(fileName);
|
||||
Files.writeString(f, yaml);
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateAll,
|
||||
fileName + ": validateAll() must refuse this config");
|
||||
assertTrue(e.getMessage().contains(mustContain),
|
||||
fileName + ": expected message to contain \"" + mustContain + "\" but was: "
|
||||
+ e.getMessage());
|
||||
}
|
||||
}
|
||||
+2
@@ -97,6 +97,8 @@ class FleetConfigWithDefaultsPreservesEveryComponentTest {
|
||||
v.put("memberLoginShell", "/bin/zsh");
|
||||
v.put("memberSkills", "/skills/guard");
|
||||
v.put("idleSleepGuard", new FleetConfig.IdleSleepGuard(true));
|
||||
v.put("models", new FleetConfig.Models(
|
||||
List.of(new FleetConfig.Models.ModelEntry("model-guard"))));
|
||||
assertNamesMatchComponents(v);
|
||||
return v;
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ import java.util.concurrent.ScheduledFuture;
|
||||
import java.util.concurrent.ScheduledThreadPoolExecutor;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicLong;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.function.BiConsumer;
|
||||
import java.util.function.LongSupplier;
|
||||
|
||||
@@ -277,6 +278,58 @@ class FleetHealthMonitorTest {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #386 follow-up, added on merge. The fix carries a PER-MEMBER drift baseline, so drift
|
||||
* from a sleep that happened BEFORE a member went busy is never charged to that member. The
|
||||
* two tests shipped with the fix both start with the member already BUSY, so a single global
|
||||
* baseline passes them — this one fails without the per-member map.
|
||||
*
|
||||
* <p>Order matters: the host sleeps while nothing is busy, and only then does a member take a
|
||||
* turn. Its stall clock must start at zero.
|
||||
*/
|
||||
@Test void driftFromASleepBeforeAMemberWentBusyIsNotChargedToThatMember() {
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(FleetHealthMonitor.class);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
try {
|
||||
AtomicLong mono = new AtomicLong(0);
|
||||
AtomicLong real = new AtomicLong(0);
|
||||
AtomicReference<List<MemberSession>> roster = new AtomicReference<>(List.of());
|
||||
FakeHerdr herdr = new FakeHerdr().withAgent("busy", "term_busy", "pane_busy", "tab_busy");
|
||||
var scheduler = Executors.newSingleThreadScheduledExecutor();
|
||||
AgentControl agents = new AgentControl(herdr);
|
||||
FleetHealthMonitor monitor = new FleetHealthMonitor(agents, roster::get,
|
||||
new MessageService(agents, new Injector(agents), new Rendezvous(), new InMemoryReplyInbox()),
|
||||
scheduler, mono::get, real::get, 60, 600, (_, _) -> { });
|
||||
|
||||
monitor.tick(); // baseline, no members yet
|
||||
|
||||
// The host sleeps for 700s with nobody busy: the monotonic clock stands still.
|
||||
real.set(TimeUnit.SECONDS.toNanos(700));
|
||||
monitor.tick();
|
||||
|
||||
// Awake again. Only NOW does a member start a turn, with a fresh activity stamp taken
|
||||
// from the monotonic clock. Both clocks advance together from here.
|
||||
mono.set(TimeUnit.SECONDS.toNanos(10));
|
||||
real.set(TimeUnit.SECONDS.toNanos(710));
|
||||
roster.set(List.of(member("term_busy", MemberSession.State.BUSY,
|
||||
0, TimeUnit.SECONDS.toNanos(10))));
|
||||
monitor.tick();
|
||||
|
||||
mono.set(TimeUnit.SECONDS.toNanos(20));
|
||||
real.set(TimeUnit.SECONDS.toNanos(720));
|
||||
monitor.tick();
|
||||
monitor.stop();
|
||||
|
||||
assertEquals(0, appender.list.stream().filter(event -> event.getFormattedMessage()
|
||||
.contains("member=term_busy state=STALL_SUSPECTED")).count(),
|
||||
"the member has been busy for 10s, not 710s — the earlier sleep is not its stall");
|
||||
} finally {
|
||||
logger.detachAppender(appender);
|
||||
}
|
||||
}
|
||||
|
||||
private static FleetHealthMonitor monitorWithClocks(FakeHerdr herdr, List<MemberSession> roster,
|
||||
java.util.concurrent.ScheduledExecutorService scheduler, LongSupplier clock,
|
||||
LongSupplier realtimeClock, long intervalSeconds, long workingSuspectAfterSeconds,
|
||||
|
||||
@@ -8,6 +8,7 @@ import java.nio.charset.StandardCharsets;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
@@ -45,6 +46,15 @@ class EnvAllowListScrubTest {
|
||||
/** Env var names appearing in command output; anything else (prompts, wrapped lines) is noise. */
|
||||
private static final Pattern ENV_NAME = Pattern.compile("^([A-Za-z_][A-Za-z0-9_]*)$");
|
||||
|
||||
/**
|
||||
* fleetd #388: the sentinel name the generated {@code .zshenv} guard uses, kept here as a
|
||||
* literal rather than referencing {@link EnvAllowListScrub#SCRUB_SENTINEL} — the two tests that
|
||||
* use it must still compile and run against the pre-fix production class (which has no such
|
||||
* constant), so the revert-and-prove-it-fails step exercises a real assertion instead of a
|
||||
* compilation error.
|
||||
*/
|
||||
private static final String SCRUB_SENTINEL_NAME = "_CB633_SCRUBBED";
|
||||
|
||||
/**
|
||||
* The equality test. Expected survivors = baseline exports ∩ allowed — i.e. every survivor is
|
||||
* allowed AND every allowed name that existed survives. The operator's own secret-store exports
|
||||
@@ -108,6 +118,31 @@ class EnvAllowListScrubTest {
|
||||
"allowed N of M with N <= M — the denominator is always reported");
|
||||
}
|
||||
|
||||
/** A group-shared ZDOTDIR still lets the member truncate and write its pre-created receipt. */
|
||||
@Test
|
||||
void groupSharedScrubWritesAndReadsItsReport(@TempDir Path tmp) throws Exception {
|
||||
assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here");
|
||||
Set<String> allowed = MemberEnvAllowList.derive(List.of());
|
||||
Path zdotdir = EnvAllowListScrub.generate(tmp, allowed, currentUserGroup());
|
||||
|
||||
Map<String, String> cleanParent = Map.of(
|
||||
"HOME", System.getProperty("user.home"),
|
||||
"PATH", "/usr/bin:/bin",
|
||||
"SHELL", "/bin/zsh");
|
||||
exportedNamesFromCleanParent(cleanParent, zdotdir);
|
||||
|
||||
EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(zdotdir);
|
||||
assertNotNull(report, "a group-shared completed login shell must leave a report behind");
|
||||
assertTrue(report.allowed() >= 0 && report.total() >= report.allowed(),
|
||||
"allowed N of M with N <= M — the denominator is always reported");
|
||||
assertEquals("rw-rw----", java.nio.file.attribute.PosixFilePermissions.toString(
|
||||
Files.getPosixFilePermissions(zdotdir.resolve(EnvAllowListScrub.REPORT_FILE))),
|
||||
"the pre-created receipt must be group-writable");
|
||||
assertEquals("rwxr-x---", java.nio.file.attribute.PosixFilePermissions.toString(
|
||||
Files.getPosixFilePermissions(zdotdir)),
|
||||
"group sharing must not make the ZDOTDIR directory group-writable");
|
||||
}
|
||||
|
||||
/** Report parsing is lenient: absent file → null (no measurement), not an exception. */
|
||||
@Test
|
||||
void readReportReturnsNullForADirectoryWithoutOne(@TempDir Path dir) {
|
||||
@@ -224,6 +259,140 @@ class EnvAllowListScrubTest {
|
||||
+ "shell. A difference here means the scrub is dead on Linux.");
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #388: the actual gap. zsh reads {@code .zshenv} always, {@code .zprofile}/
|
||||
* {@code .zlogin} only for a LOGIN shell, and {@code .zshrc} only for an INTERACTIVE one — so a
|
||||
* shell that is NEITHER (a bare {@code /bin/zsh} reading a script off a non-tty stdin, no
|
||||
* {@code -l}, no {@code -i}) reads only {@code .zshenv} and stops. Before this fix, that shell
|
||||
* never reached {@code scrub.zsh} at all: the decoy secret below would survive untouched. This
|
||||
* test injects that decoy directly into the process environment (not via a sourced dotfile,
|
||||
* since the whole point of the gap is that {@code .zshenv} is normally close to empty) so the
|
||||
* test does not depend on any real {@code ~/.zshrc} content existing on the host.
|
||||
*/
|
||||
@Test
|
||||
void scrubRunsInAShellThatIsNeitherLoginNorInteractive(@TempDir Path tmp) throws Exception {
|
||||
assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here");
|
||||
Set<String> allowed = MemberEnvAllowList.derive(List.of());
|
||||
Path zdotdir = EnvAllowListScrub.generate(tmp, allowed);
|
||||
|
||||
Map<String, String> cleanParent = new HashMap<>(Map.of(
|
||||
"HOME", System.getProperty("user.home"),
|
||||
"PATH", "/usr/bin:/bin",
|
||||
"SHELL", "/bin/zsh",
|
||||
"USER", System.getProperty("user.name", "nobody"),
|
||||
"TMPDIR", tmp.toString()));
|
||||
cleanParent.put("FLEETD_TEST_DECOY_SECRET", "x"); // not on any allow-list; must be blanked
|
||||
|
||||
List<String> neither = List.of(); // no -l, no -i; stdin is a pipe (never a tty) either way
|
||||
Set<String> baseline = exportedNamesFromCleanParent(cleanParent, null, neither);
|
||||
Set<String> scrubbed = exportedNamesFromCleanParent(cleanParent, zdotdir, neither);
|
||||
|
||||
Set<String> expected = new TreeSet<>();
|
||||
for (String name : baseline) {
|
||||
if (MemberEnvAllowList.keeps(allowed, name)) {
|
||||
expected.add(name);
|
||||
}
|
||||
}
|
||||
assertTrue(baseline.contains("FLEETD_TEST_DECOY_SECRET"),
|
||||
"sanity: the decoy must actually reach the un-scrubbed baseline, or this test proves "
|
||||
+ "nothing");
|
||||
expected.add("ZDOTDIR"); // the harness set it and it is infrastructure, so it must survive
|
||||
expected.add(SCRUB_SENTINEL_NAME); // set by the new .zshenv guard once scrubbed
|
||||
assertEquals(expected, scrubbed,
|
||||
"a pane shell that is NEITHER login nor interactive must still be scrubbed — its "
|
||||
+ "surviving exported names must EQUAL baseline ∩ allow-list, plus the "
|
||||
+ "sentinel the guard sets once it has run. FLEETD_TEST_DECOY_SECRET surviving "
|
||||
+ "here means the gap is still open.");
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #388 invariants 3 and 4, which a name-set equality cannot show: a member's own tooling
|
||||
* forks plain, non-login, non-interactive zsh processes for a single command (the same shape as
|
||||
* the pane shell itself), and such a child must (a) keep whatever its parent deliberately set
|
||||
* for it, never (b) re-run the scrub and blank it, and never (c) overwrite the pane's own
|
||||
* {@code scrub-report.txt} with a description of itself instead of the pane. All three can only
|
||||
* be shown by actually running a child process from within the scrubbed pane shell.
|
||||
*
|
||||
* <p>The pane and the child both report presence via {@code ${NAME:+present}} — empty when a
|
||||
* name is unset OR blanked (exported empty), {@code present} when it is set and non-empty. No
|
||||
* value is ever printed, only these two shapes and the literal word {@code set}/{@code unset}
|
||||
* for the sentinel.
|
||||
*/
|
||||
@Test
|
||||
void neitherShellChildKeepsParentVariablesAndReceiptStillDescribesThePane(@TempDir Path tmp) throws Exception {
|
||||
assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here");
|
||||
Set<String> allowed = MemberEnvAllowList.derive(List.of());
|
||||
Path zdotdir = EnvAllowListScrub.generate(tmp, allowed);
|
||||
|
||||
Map<String, String> paneEnv = new HashMap<>(Map.of(
|
||||
"HOME", System.getProperty("user.home"),
|
||||
"PATH", "/usr/bin:/bin",
|
||||
"SHELL", "/bin/zsh",
|
||||
"USER", System.getProperty("user.name", "nobody"),
|
||||
"TMPDIR", tmp.toString()));
|
||||
paneEnv.put("FLEETD_TEST_DECOY_SECRET", "x"); // not allow-listed; the pane must blank it
|
||||
paneEnv.put("ZDOTDIR", zdotdir.toAbsolutePath().toString());
|
||||
|
||||
// The pane's own script reports what IT sees, then forks a plain non-login, non-interactive
|
||||
// child — the shape a member's own tooling uses — carrying a variable the "parent" (this
|
||||
// pane) deliberately set for it, the way git sets GIT_DIR for a hook.
|
||||
String outerScript = """
|
||||
print -r -- "PANE_SENTINEL=${%1$s:+set}"
|
||||
print -r -- "PANE_DECOY=${FLEETD_TEST_DECOY_SECRET:+present}"
|
||||
FLEETD_TEST_TOOL_VAR=keep /bin/zsh <<'CHILD'
|
||||
print -r -- "CHILD_LOGIN=$([[ -o login ]] && echo yes || echo no)"
|
||||
print -r -- "CHILD_INTERACTIVE=$([[ -o interactive ]] && echo yes || echo no)"
|
||||
print -r -- "CHILD_TOOL_VAR=${FLEETD_TEST_TOOL_VAR:+present}"
|
||||
print -r -- "CHILD_DECOY=${FLEETD_TEST_DECOY_SECRET:+present}"
|
||||
print -r -- "CHILD_SENTINEL=${%1$s:+set}"
|
||||
CHILD
|
||||
exit
|
||||
""".formatted(SCRUB_SENTINEL_NAME);
|
||||
|
||||
ProcessBuilder pb = new ProcessBuilder("/bin/zsh"); // no -l, no -i: the pane's own shape
|
||||
pb.environment().clear();
|
||||
pb.environment().putAll(paneEnv);
|
||||
pb.redirectError(ProcessBuilder.Redirect.DISCARD);
|
||||
Process zsh = pb.start();
|
||||
zsh.getOutputStream().write(outerScript.getBytes(StandardCharsets.UTF_8));
|
||||
zsh.getOutputStream().flush();
|
||||
zsh.getOutputStream().close();
|
||||
String stdout = new String(zsh.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
|
||||
assertTrue(zsh.waitFor(60, java.util.concurrent.TimeUnit.SECONDS),
|
||||
"the pane+child probe did not exit within 60s");
|
||||
assertTrue(zsh.exitValue() == 0, "probe zsh exited non-zero: " + stdout);
|
||||
|
||||
Map<String, String> reported = new HashMap<>();
|
||||
for (String line : stdout.split("\n")) {
|
||||
int eq = line.indexOf('=');
|
||||
if (eq > 0) {
|
||||
reported.put(line.substring(0, eq).trim(), line.substring(eq + 1).trim());
|
||||
}
|
||||
}
|
||||
|
||||
assertEquals("set", reported.get("PANE_SENTINEL"),
|
||||
"the pane itself is neither login nor interactive, so the .zshenv guard must have "
|
||||
+ "run the scrub and exported the sentinel");
|
||||
assertEquals("", reported.get("PANE_DECOY"),
|
||||
"the pane must blank a non-allow-listed name — invariant 1");
|
||||
assertEquals("no", reported.get("CHILD_LOGIN"), "sanity: the child must also be non-login");
|
||||
assertEquals("no", reported.get("CHILD_INTERACTIVE"), "sanity: the child must also be non-interactive");
|
||||
assertEquals("present", reported.get("CHILD_TOOL_VAR"),
|
||||
"invariant 3: a variable the pane deliberately set for its child must survive — a "
|
||||
+ "child that re-ran the scrub would have blanked it");
|
||||
assertEquals("", reported.get("CHILD_DECOY"),
|
||||
"a name already blanked by the pane must stay blanked in the child, never resurrected");
|
||||
assertEquals("set", reported.get("CHILD_SENTINEL"),
|
||||
"the child must inherit the sentinel from the pane's environment, or it would re-scrub");
|
||||
|
||||
EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(zdotdir);
|
||||
assertNotNull(report, "the pane's own scrub pass must leave a report behind");
|
||||
assertTrue(report.blanked().stream().noneMatch(n -> n.startsWith("FLEETD_TEST_TOOL_VAR")),
|
||||
"invariant 4: the receipt must still describe the PANE, not the child — a child that "
|
||||
+ "re-ran the scrub would have rewritten this file to list its own "
|
||||
+ "FLEETD_TEST_TOOL_VAR as blanked");
|
||||
}
|
||||
|
||||
/**
|
||||
* Run {@code /bin/zsh -l -i} from a clean parent and return the NAMES it has exported by prompt
|
||||
* time. With {@code zdotdir} non-null, {@code ZDOTDIR} points at a generated scrub directory, so
|
||||
|
||||
+144
@@ -0,0 +1,144 @@
|
||||
package dev.ltms.fleet.peer;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import java.lang.reflect.Constructor;
|
||||
import java.lang.reflect.RecordComponent;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
import java.util.TreeSet;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
|
||||
/**
|
||||
* Fleetd #382, the same "defect factory" #357 and #358 guarded on {@code FleetConfig.withDefaults()}
|
||||
* and {@link dev.ltms.fleet.session.MemberSession}'s rebuild sites — reproduced here on
|
||||
* {@link SpawnRequest#withProfile(String)}.
|
||||
*
|
||||
* <p>{@code SpawnRequest} has back-compat constructors at arity 3 and 5 alongside its canonical
|
||||
* arity-6 constructor. Before this ticket, {@code CompositePeerLauncher} routed a profile by
|
||||
* building a fresh {@code SpawnRequest} from a literal {@code new SpawnRequest(...)} call listing
|
||||
* six of the original request's own accessors. That call is only correct because it happens to
|
||||
* name exactly six arguments today — add a 7th component and the established back-compat pattern
|
||||
* (a new constructor at the old, now-shorter arity) and a call one argument short of the new
|
||||
* canonical arity would silently rebind to that back-compat constructor, dropping the new
|
||||
* component on every profile-routed spawn without any compile error. {@link #withProfile} replaces
|
||||
* that literal call, so this test guards the ONE rebuild site instead of a call site scattered
|
||||
* through a launcher.
|
||||
*
|
||||
* <p>The check below builds a {@link SpawnRequest} through the TRUE canonical constructor —
|
||||
* resolved by the record's own component types via {@code getDeclaredConstructor}, never by
|
||||
* argument count, so it can never itself land on a back-compat overload — with a real, distinctive,
|
||||
* non-null value in every component, calls {@link SpawnRequest#withProfile(String)}, and asserts
|
||||
* every component the method is not documented to change survives unchanged, while {@code
|
||||
* profileName} comes back as the new value it was given. A component that comes back anything else
|
||||
* was silently dropped — the shape of the defect this test exists to catch.
|
||||
*
|
||||
* <p>{@link #EXCLUDED_FROM_SURVIVAL_CHECK} is kept deliberately empty and size-pinned by
|
||||
* {@link #exclusionListSizeIsPinned()}, for the same reason the other two guards pin theirs at
|
||||
* zero: a checker whose escape hatch can grow to silence a failure is not a checker. Every one of
|
||||
* {@link SpawnRequest}'s 6 current components has a real, non-null value here and none is excluded.
|
||||
*/
|
||||
class SpawnRequestWithProfilePreservesEveryComponentTest {
|
||||
|
||||
private static final RecordComponent[] COMPONENTS = SpawnRequest.class.getRecordComponents();
|
||||
|
||||
/** Deliberately empty today; grow it only with a matching justification, and re-pin the size. */
|
||||
private static final Set<String> EXCLUDED_FROM_SURVIVAL_CHECK = Set.of();
|
||||
|
||||
/** One real, distinctive, non-null value per component — none of the 6 is excluded. */
|
||||
private static Map<String, Object> baseValues() {
|
||||
Map<String, Object> v = new LinkedHashMap<>();
|
||||
v.put("profileName", "profile-guard");
|
||||
v.put("requestedCwd", "/wt/requested-guard");
|
||||
v.put("callerCwd", "/wt/caller-guard");
|
||||
v.put("sessionName", "session-guard");
|
||||
v.put("resumeSessionId", "resume-guard");
|
||||
v.put("role", MemberRole.REVIEWER);
|
||||
assertNamesMatchComponents(v);
|
||||
return v;
|
||||
}
|
||||
|
||||
/**
|
||||
* Guards {@link #baseValues()} itself against drifting from the record's real shape — forgetting
|
||||
* to add a new component here fails this assertion by name, rather than silently checking one
|
||||
* component fewer than the record has.
|
||||
*/
|
||||
private static void assertNamesMatchComponents(Map<String, Object> values) {
|
||||
Set<String> names = new TreeSet<>();
|
||||
for (RecordComponent rc : COMPONENTS) {
|
||||
names.add(rc.getName());
|
||||
}
|
||||
assertEquals(names, new TreeSet<>(values.keySet()),
|
||||
"this test's value map has drifted from SpawnRequest's actual components — "
|
||||
+ "update baseValues() alongside the record");
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds a {@link SpawnRequest} through the TRUE canonical constructor — resolved by the
|
||||
* record's own component types, not by argument count — so this never accidentally exercises a
|
||||
* back-compat overload the way a literal {@code new SpawnRequest(...)} call risks doing.
|
||||
*/
|
||||
private static SpawnRequest requestOf(Map<String, Object> values) throws ReflectiveOperationException {
|
||||
Class<?>[] types = Arrays.stream(COMPONENTS).map(RecordComponent::getType).toArray(Class<?>[]::new);
|
||||
Object[] args = Arrays.stream(COMPONENTS).map(rc -> values.get(rc.getName())).toArray();
|
||||
Constructor<SpawnRequest> ctor = SpawnRequest.class.getDeclaredConstructor(types);
|
||||
return ctor.newInstance(args);
|
||||
}
|
||||
|
||||
@Test
|
||||
void exclusionListSizeIsPinned() {
|
||||
assertEquals(0, EXCLUDED_FROM_SURVIVAL_CHECK.size(),
|
||||
"EXCLUDED_FROM_SURVIVAL_CHECK grew from 0 — every entry needs a justification in "
|
||||
+ "this test class's javadoc AND this assertion re-pinned to the new size; a "
|
||||
+ "growing exclusion list that silences failures on its own is not a guard");
|
||||
}
|
||||
|
||||
@Test
|
||||
void withProfilePreservesEveryOtherComponent() throws ReflectiveOperationException {
|
||||
Map<String, Object> base = baseValues();
|
||||
SpawnRequest request = requestOf(base);
|
||||
SpawnRequest result = request.withProfile("profile-updated");
|
||||
|
||||
Map<String, Object> expectedOverrides = Map.of("profileName", "profile-updated");
|
||||
|
||||
List<String> dropped = new ArrayList<>();
|
||||
int checked = 0;
|
||||
for (RecordComponent rc : COMPONENTS) {
|
||||
String name = rc.getName();
|
||||
if (EXCLUDED_FROM_SURVIVAL_CHECK.contains(name)) {
|
||||
continue;
|
||||
}
|
||||
checked++;
|
||||
Object expected = expectedOverrides.containsKey(name) ? expectedOverrides.get(name) : base.get(name);
|
||||
Object actual;
|
||||
try {
|
||||
actual = rc.getAccessor().invoke(result);
|
||||
} catch (ReflectiveOperationException e) {
|
||||
throw new RuntimeException("failed to read SpawnRequest." + name + "()", e);
|
||||
}
|
||||
if (!Objects.equals(expected, actual)) {
|
||||
dropped.add(String.format(Locale.ROOT,
|
||||
"%s: withProfile() was expected to carry (%s) for '%s' but returned %s — a "
|
||||
+ "component silently dropped by withProfile(), the shape of the defect "
|
||||
+ "this test exists to catch (its final \"return new SpawnRequest(...)\" "
|
||||
+ "call binding to a back-compat constructor instead of the true "
|
||||
+ "canonical one)",
|
||||
name, expected, name, actual));
|
||||
}
|
||||
}
|
||||
|
||||
System.out.printf(Locale.ROOT,
|
||||
"SpawnRequest.withProfile() component-survival coverage — %d components, %d checked, "
|
||||
+ "%d excluded, %d survived%n",
|
||||
COMPONENTS.length, checked, EXCLUDED_FROM_SURVIVAL_CHECK.size(), checked - dropped.size());
|
||||
assertEquals(List.of(), dropped,
|
||||
"withProfile() silently dropped these components: " + dropped);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user