Compare commits

...

6 Commits

Author SHA1 Message Date
Dai Ha 4b9ebda1b3 fleetd #593 CORRECTION 1: allowlist comm=java, not a denylist of shells
CI / shell-tests (pull_request) Failing after 7s
CI / contract (pull_request) Successful in 1m27s
CI / build (pull_request) Successful in 1m48s
The round-1 fix excluded known shell names (sh/bash/zsh/dash/ksh) from
running_pid()'s pgrep candidates. Two holes remained, both the same
false-positive shape the ticket exists to remove:

1. A pid pgrep lists can exit before the following `ps -o comm=` lookup
   runs. On a gone pid, ps prints nothing, comm is empty, and an empty
   string matches no denied shell name -- so a dead pid was still counted.
2. The denylist only knows the shells someone thought to name. ssh, perl,
   python3, ruby, tail -- anything else carrying the pattern in its own
   argv -- was still counted alongside the real daemon. The ticket names
   ssh as a live route.

Both close with one change: allowlist comm=java instead of denying shells.
The daemon is always `java -jar target/fleetd.jar`, so its comm is always
`java`; an empty comm (hole 1) is not `java` either, closing that hole for
free.

Answers the objection in the code comment: an allowlist can under-count if
fleetd ever stops being launched by `java` (a native image, a renamed
launcher). That's a false negative, the worse direction for a guard -- but
it is not a new assumption: PATTERN='target/fleetd.jar' already assumes a
jar run by java, and that pattern breaks before this allowlist would.

Replaces the round-1 "real second process" test (which gave its exec -a
standin an argv[0] holding the pattern, but not comm=java) with one that
forces comm=java via `exec -a java sh -c '...'`. Adds two stubbed
pgrep/ps tests pinning the two holes directly (a non-java, non-shell comm
such as perl; an empty comm from an already-exited pid) -- deterministic
on every platform, unlike a live-process fixture, and immune to the BSD
vs Linux difference in how `comm` is derived from a fabricated process.
Adds a stubbed positive backstop (comm=java is counted).

Confirmed the regression is caught: reverted to the round-1 denylist,
reran the suite, watched the new non-shell-comm test fail at
`set -e`'s first failure, then isolated the exited-pid test separately
and confirmed it also fails against the same broken code. Restored the
fix and reran green.

Branch merged with origin/main (3 commits: hunter role + CLAUDE.md
addendum) before this commit; unrelated, no conflicts.
2026-09-19 15:31:15 +07:00
Dai Ha 1e68d7ee39 Merge origin/main into worker/593-1a8025-5 2026-09-19 15:27:46 +07:00
Dai Ha 42820fbe75 fleetd #593 (pid-count half): running_pid() no longer matches the caller
CI / shell-tests (pull_request) Successful in 9s
CI / contract (pull_request) Successful in 1m18s
CI / build (pull_request) Successful in 1m45s
running_pid() was a bare `pgrep -f "$PATTERN"`, which matches ANY process whose
full command line contains the pattern text -- including a shell that merely
embeds it as literal text (a hand-typed investigation, an ssh-shaped
`sh -c '...; ...'`, or a pipeline) rather than being the daemon. That self-match
turns a working redeploy into a reported "racing supervisor" failure via
assert_single_daemon.

pgrep -c does not exist on BSD/macOS, so this can't be fixed by switching flags.
running_pid() now keeps pgrep to find candidates (portable), then drops any
candidate whose process name (comm) names a shell -- the daemon is always
`java`, so a self-matching wrapper of this shape is always excluded while a
genuine second daemon-shaped process still counts.

assert_single_daemon's die message no longer hands the operator a bare
`pgrep -f "$PATTERN"` as remediation -- that was exactly the self-matching
invocation -- and now says in words that a pattern can match the caller.

Adds three tests: a self-matching wrapper shell must be excluded, a real
second daemon-shaped process must still be found, and the die message must
not recommend the self-matching command. Verified the first test fails
against the pre-fix implementation (confirmed the regression is caught).

Leaves instance 1 (the fleetd.out log source, systemd-only) for a Linux host,
per the ticket's scope split.
2026-09-19 15:18:32 +07:00
Dai Ha d91ff886da #568 follow-up: fix the text defects the hunter-role merge introduced
CI / shell-tests (push) Successful in 6s
CI / contract (push) Successful in 1m17s
CI / build (push) Successful in 1m48s
Found by reading the diff at the merge gate, not reported by the worker.

1. FleetConfig.java: the operator-facing "unknown key" hint read
   "'fleet.architects', 'fleet.developers' or 'fleet.hunters' or
   'fleet.reviewers'" — a double "or". This is text an operator reads at the
   moment their config is already wrong, so it should not itself be wrong.
2. MemberLifecycle.java: javadoc continuation asterisk indented 6 spaces, not 5.
3. MemberRegistry.java: javadoc asterisks moved from column 2 to column 4.
4. CallerResolver.java: a // comment indented one space past its block.

2-4 are the worker mangling alignment while widening enum lists to include
HUNTER. No behaviour changes.

CORRECTION to the #596 merge commit message. It claimed a fifth defect, "two
javadoc lines pushed past the 100-column convention". There is no such
convention in this repo: no checkstyle, no spotless, no .editorconfig, and 2975
of 32728 lines under fleetd/src/main/java already exceed 100 characters. I
asserted the rule before measuring it. Those two lines are untouched.

Verified: built in a scratch worktree, 1790 tests, 0 failures, 0 errors,
0 skipped, counted from the surefire XML.
2026-09-19 15:15:54 +07:00
ltms 386e760a5c Merge #596: fleetd #568 — add the hunter member role
CI / shell-tests (push) Successful in 5s
CI / contract (push) Successful in 49s
CI / build (push) Successful in 2m37s
Verified by the lead before merge, not taken on the worker's report:

- branch contains a639969; 1 ahead, 0 behind — clean fast-forward
- CLAUDE.md change is +2 lines in the Project addendum, NOT the canonical block
- canonical block sync check prints True on main and on this branch
- built in a scratch worktree (never mvn clean in the main clone): 1790 tests,
  0 failures, 0 errors, 0 skipped, counted from the surefire XML. Baseline 1789.
- live fleetd.yaml still loads: the hunters pool is optional

Five text defects found by reading the diff, not reported by the worker. They are
fixed in a follow-up commit on main rather than a round trip:
- FleetConfig.java operator-facing message reads "... 'fleet.developers' or
  'fleet.hunters' or 'fleet.reviewers'" — a double "or"
- misaligned javadoc continuation asterisks in MemberLifecycle and MemberRegistry
- a misaligned // comment in CallerResolver
- two javadoc lines pushed past the 100-column convention

Known gap, tracked separately: fleet.hunters is absent from the live config, so a
hunter cannot spawn on this host until the pool is added after the redeploy. The
role ships correct and inert.
2026-09-19 10:13:26 +02:00
Dai Ha 2e349139e9 #568: add hunter member role
CI / shell-tests (pull_request) Successful in 6s
CI / contract (pull_request) Successful in 1m21s
CI / build (pull_request) Successful in 1m44s
2026-09-19 15:09:07 +07:00
16 changed files with 301 additions and 34 deletions
+23
View File
@@ -0,0 +1,23 @@
---
name: hunter
description: Sweep one assigned scope for defects and report ranked findings without changes.
---
<!-- CB-617: The model comes from fleetd.yaml because the launch flag overrides model here on both backends. -->
You sweep the assigned package or scope for real defects. Read the full assigned scope before you
judge it. Report several ranked findings when the evidence supports them. Change nothing: do not
edit code, commit, push, or open a pull request.
You may run the build or tests to check a finding. Read the complete output and report the real
result. Do not hide failures with a pipe. State only checks you actually ran. The primary's IDE
tools are not yours. A mounted forge tool may use a blocked credential and fail by design.
Do only the assigned scope. Note anything outside it in one line and do not investigate it further.
Use `fleet_ask{question}` only when a decision belongs to the lead, such as an unclear requirement
or two defensible fixes. Do not ask about something you can decide by reading more code.
Your handoff must name the files you read, each ranked finding or `NO FINDINGS`, the checks you ran,
and any caveat for review.
The launcher provides the required bridge reply instructions for every member.
+2
View File
@@ -284,6 +284,8 @@ must obey belongs in the charter, not here.
it for a multi-finding sweep hands the worker two contradictory output contracts. That has
already cost three workers' turns: each wrote a good report to its terminal and ended the turn
with no `fleet_reply`, and the scrape returned the tail of the brief instead.
Spawn `implementer` with role `dev`, `reviewer` with role `reviewer`, and `hunter` with role
`hunter`.
- **Primary-side skills** (not delegation playbooks — a worker cannot use them):
`port-to-opencode` (make an OpenCode session a participant in this workspace),
`fleets-status` (report every fleet that shares one LavinMQ instance),
+12 -6
View File
@@ -560,7 +560,7 @@ placement: weighted
# older keys: `leaders:`, `members:`, `leadScan:` and `defaultProfile:`.
#
# A member is anything a lead spawns, and every member has two INDEPENDENT attributes:
# role — which contract: architect, dev or reviewer. It picks the launch charter, the role
# role — which contract: architect, dev, hunter or reviewer. It picks the launch charter, the role
# file, the playbook skill and the authz row.
# profile — which backend: one of the `profiles:` keys above (model, CLI adapter, cost).
# They vary on their own. A reviewer may run on the same profile as the dev whose diff it reads,
@@ -572,13 +572,13 @@ placement: weighted
#
# Each pool lists the profiles that role MAY run on — these are pools, not identities. That is also
# what replaced `defaultProfile:`: an unqualified spawn names a role, and that role's pool supplies
# the candidates, in definition order. A dev and a reviewer staying anonymous is exactly compatible
# with being listed here; the entry key just names the entry.
# the candidates, in definition order. A dev, hunter and reviewer staying anonymous is exactly
# compatible with being listed here; the entry key just names the entry.
fleet:
# Optional launch-charter text, keyed only by the singular role wire names: architect, dev,
# reviewer. Changes are HOT and reach the next spawn without a daemon restart. Do not put secrets
# here: a later launch step writes this text to a world-readable temp file, and ${ENV} interpolation
# is deliberately not supported.
# hunter, reviewer. Changes are HOT and reach the next spawn without a daemon restart. Do not put
# secrets here: a later launch step writes this text to a world-readable temp file, and ${ENV}
# interpolation is deliberately not supported.
charters:
architect: |-
You are an architect in this fleet. You refine work before anyone builds it:
@@ -589,6 +589,9 @@ fleet:
dev: |-
You implement the one unit you were given, and nothing else. You test it,
commit it, and open your own pull request. You never merge.
hunter: |-
You sweep the assigned scope for real defects. You may run the build or tests
to check a finding. You change nothing, and report several ranked findings.
reviewer: |-
You review the diff you were given. You report bugs, risks and missing tests.
You do not change code.
@@ -666,6 +669,9 @@ fleet:
developers:
gx10:
profile: gx10
# hunters:
# gx10:
# profile: gx10 # a hunt may run checks, but never changes code
# reviewers:
# gx10:
# profile: gx10 # the same backend may serve two roles; that is the point
@@ -221,7 +221,8 @@ public final class CallerResolver {
// The config/live binding names this pane as an architect slot's own. Same
// unforgeable pane mapping; the live binding, never a request argument, decides.
// Check the slot role too: this defence in depth prevents a bad lifecycle bind from
// escalating a dev or reviewer into an architect. Checked before the worker fallback.
// escalating a dev, hunter or reviewer into an architect. Checked before
// the worker fallback.
return Principal.architect(memberSlotNames.apply(slot), c.terminal(), c.pid());
}
return Principal.worker(c.terminal(), c.pid()); // unforgeable; never token-gated
@@ -42,7 +42,8 @@ public interface MemberLifecycle {
* Try to bind a newly spawned {@code terminal} into the role it was granted.
*
* @return the role this session actually holds: {@code role} unchanged for a role with no
* live slot-binding semantics (dev, reviewer), or when the bind succeeded; a fallback
* live slot-binding semantics (dev, hunter, reviewer), or when the bind
* succeeded; a fallback
* role — never {@code role} — when a slot-bound role (architect) could not be bound.
* Callers must record THIS value on the session, never the requested {@code role}, so
* a later roster read never reports a role the session does not hold (CB-619). In
@@ -20,7 +20,8 @@ import java.util.function.Supplier;
*
* <p>Two halves, split by who owns each:
* <ul>
* <li><b>slots</b> — read from {@code fleet.architects}/{@code developers}/{@code reviewers}
* <li><b>slots</b> — read from {@code fleet.architects}/{@code developers}/
* {@code hunters}/{@code reviewers}
* (see {@link #slots()}), each carrying the {@code profile} reference the spawn lifecycle
* reads when it stands the slot up. <strong>Live, since fleetd #424</strong>: {@link #live}
* re-reads {@code fleet:} on every call, through a supplier the same shape as
@@ -322,7 +323,7 @@ public final class MemberRegistry implements MemberLifecycle {
* CB-619 / fleetd #123: refuse an architect acquire before anything spawns when no configured
* slot carries {@code profile} — the config-gap case from the original defect report (a spawn
* asked for {@code role=architect, profile=sonnet}, and {@code fleet.architects} carried only
* {@code opus} and {@code sol}). A dev/reviewer acquire is always a no-op: those pools are
* {@code opus} and {@code sol}). A dev/hunter/reviewer acquire is always a no-op: those pools are
* placement candidates only (see {@code CompositePeerLauncher}), never a live identity binding,
* so there is nothing here to refuse — an explicit profile outside the pool for those roles is a
* documented operator override, not a defect.
@@ -31,7 +31,7 @@ import java.util.function.Supplier;
* {@code placement:}, and an existing profile's {@code weight} / {@code maxLoad}. Both are
* read through a supplier on {@code CompositePeerLauncher}, which is what makes them hot —
* not the fact that they are config. Most of {@code fleet:} — every role pool
* ({@code architects}/{@code developers}/{@code reviewers}), {@code charters}, and
* ({@code architects}/{@code developers}/{@code hunters}/{@code reviewers}), {@code charters}, and
* {@code tabLabel} — is read the same live way, through the same supplier
* ({@code () -> config.get().fleet()}). {@code architects} in particular is hot for
* <strong>two independent consumers</strong> (fleetd #424): {@code CompositePeerLauncher}
@@ -605,7 +605,7 @@ public final class ConfigRef implements Supplier<FleetConfig> {
+ "opened once and needs a restart; the broker URI env-var name kept out of a "
+ "member's environment is read live on every spawn and already applied");
}
// fleetd #333: unlike health/coordinator above, most of `fleet:` (developers, reviewers,
// fleetd #333: unlike health/coordinator above, most of `fleet:` (developers, hunters, reviewers,
// charters, tabLabel) is genuinely hot — ConfigRefTest.aHotChangeIsAppliedAndRead-
// ThroughGet and aCharterChangeIsHotAndReachesTheLiveConfig prove it reaches the live config
// with no restart note. `architects` is hot too, and — since fleetd #424 — hot for BOTH of
@@ -630,7 +630,7 @@ public final class ConfigRef implements Supplier<FleetConfig> {
+ "identity map and to auto-launch leads, and neither is rebuilt on reload, so a "
+ "lead added, removed, or given a new tab: label needs a restart — until then it "
+ "stays unrecognised, and a caller from its new tab resolves as a worker, not a "
+ "lead; the rest of fleet: (developers, reviewers, charters, tabLabel) is read "
+ "lead; the rest of fleet: (developers, hunters, reviewers, charters, tabLabel) is read "
+ "live through the supplier on CompositePeerLauncher, and architects is read "
+ "live through that same supplier for placement AND through a separate supplier "
+ "on MemberRegistry for spawn-time identity — both already applied");
@@ -62,7 +62,7 @@ import java.util.regex.PatternSyntaxException;
* @param fleet who the daemon may run and under which role (CB-557). One block replacing
* the former {@code leaders:}, {@code members:}, {@code leadScan:} and
* {@code defaultProfile:}. Role is the containing key — {@code leaders},
* {@code architects}, {@code developers}, {@code reviewers} — and each entry
* {@code architects}, {@code developers}, {@code hunters}, {@code reviewers} — and each entry
* names the {@code profiles:} backend it runs on. See {@link Fleet}
* @param leadHeartbeat opt-in idle-lead heartbeat (CB-551); {@code null} ⇒ off, and an upgraded
* daemon never nudges an idle lead on its own initiative
@@ -1200,6 +1200,7 @@ public record FleetConfig(
* @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}},
@@ -1210,6 +1211,7 @@ public record FleetConfig(
public record Fleet(Map<String, Leader> leaders,
Map<String, Slot> architects,
Map<String, Slot> developers,
Map<String, Slot> hunters,
Map<String, Slot> reviewers,
Map<String, String> charters,
String tabLabel) {
@@ -1226,6 +1228,7 @@ public record FleetConfig(
leaders = unmodifiableOrEmpty(leaders);
architects = unmodifiableOrEmpty(architects);
developers = unmodifiableOrEmpty(developers);
hunters = unmodifiableOrEmpty(hunters);
reviewers = unmodifiableOrEmpty(reviewers);
charters = unmodifiableOrEmpty(charters);
tabLabel = (tabLabel == null || tabLabel.isBlank()) ? DEFAULT_TAB_LABEL : tabLabel;
@@ -1240,9 +1243,15 @@ public record FleetConfig(
* 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.
*/
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);
}
public Fleet(Map<String, Leader> leaders, Map<String, Slot> architects,
Map<String, Slot> developers, Map<String, Slot> reviewers, String tabLabel) {
this(leaders, architects, developers, reviewers, null, tabLabel);
this(leaders, architects, developers, null, reviewers, null, tabLabel);
}
/**
@@ -1264,6 +1273,7 @@ public record FleetConfig(
return switch (role) {
case ARCHITECT -> architects;
case DEV -> developers;
case HUNTER -> hunters;
case REVIEWER -> reviewers;
};
}
@@ -1872,7 +1882,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", "reviewers");
Set.of("leaders", "architects", "developers", "hunters", "reviewers");
/**
* Reject a {@code fleet:} role pool whose slot names repeat (CB-548, re-homed by CB-557).
@@ -1882,7 +1892,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 four pools <em>directly under the top-level {@code fleet:}</em> are considered,
* <p>Only the five 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.
*
@@ -2049,8 +2059,9 @@ public record FleetConfig(
"defaultProfile", "a role pool under 'fleet:' — an unqualified spawn now names a role,"
+ " and that role's pool supplies the candidate profiles",
"architects", "'fleet.architects'",
"members", "a role pool under 'fleet:' — 'fleet.architects', 'fleet.developers' or"
+ " 'fleet.reviewers'; the role is the containing key, not a 'role:' field",
"members", "a role pool under 'fleet:' — 'fleet.architects', 'fleet.developers',"
+ " 'fleet.hunters' or 'fleet.reviewers'; the role is the containing key, not"
+ " a 'role:' field",
"leaders", "'fleet.leaders'",
"leadScan", "'fleet.leaders.<name>.tabPrefix' and '.scanIntervalSeconds' — lead"
+ " discovery is now configured on the lead it discovers");
@@ -2134,8 +2134,9 @@ public final class FleetMcp {
return tool(FleetTool.SPAWN.wireName(),
"Spawn a new off-subscription member session. A member has two independent attributes: "
+ "role (what it is for) and profile (which backend it runs on). Pass role to pick "
+ "the contract — 'dev' implements a unit and opens its own PR, 'reviewer' reviews a "
+ "diff it did not write, 'architect' refines a ticket before anyone builds it; omit "
+ "the contract — 'dev' implements a unit and opens its own PR, 'hunter' sweeps a "
+ "scope without changing it, 'reviewer' reviews a diff it did not write, 'architect' "
+ "refines a ticket before anyone builds it; omit "
+ "it for 'dev'. Pass profile (from fleet_profiles) to pick the backend, or omit it "
+ "for the default. The two are independent: a reviewer may run on the same profile "
+ "as the dev it reviews. The member opens your current directory by default; pass "
@@ -2152,7 +2153,7 @@ public final class FleetMcp {
+ "one. Returns the member's sessionId (use with fleet_send) and paneId (use with "
+ "fleet_stop).",
objectSchema(Map.of(
"role", stringProp("What the member is for: architect, dev or reviewer (default dev)"),
"role", stringProp("What the member is for: architect, dev, hunter, or reviewer (default dev)"),
"profile", stringProp("Which backend to run it on (omit for the default profile)"),
"cwd", stringProp("Working directory for the member (omit to inherit yours)"),
"worktree", Map.of("type", "string", "description", "'true' or a ticket slug — requests an isolated git worktree"),
@@ -29,8 +29,8 @@ public enum MemberRole {
* <p>Reads the repo and writes analysis. Never commits code and never opens a pull request —
* an architect that starts implementing has stopped doing the job that makes it useful.
*
* <p>Architects are the one member kind declared in config, because a lead addresses the same
* slots across many tickets and needs a stable name for them.
* <p>Architects are the one member kind with live slot binding, because a lead addresses the
* same slots across many tickets and needs a stable name for them.
*/
ARCHITECT,
@@ -43,6 +43,14 @@ public enum MemberRole {
*/
DEV,
/**
* Sweeps an assigned package for defects and reports several ranked findings.
*
* <p>Never changes code, commits, or opens a pull request. A hunt gathers evidence, which can
* include running the build, but leaves every fix to a later implementation unit.
*/
HUNTER,
/**
* Reviews a diff it did not write and reports one structured finding.
*
@@ -59,7 +67,7 @@ public enum MemberRole {
/**
* The {@code fleet:} block that holds this role's pool — {@code architects},
* {@code developers}, {@code reviewers}.
* {@code developers}, {@code hunters}, {@code reviewers}.
*
* <p>Plural, and not always the wire name: the pool of things a {@code dev} may run on reads
* naturally as {@code developers:}. The wire name stays the singular {@code dev}, because that
@@ -69,6 +77,7 @@ public enum MemberRole {
return switch (this) {
case ARCHITECT -> "architects";
case DEV -> "developers";
case HUNTER -> "hunters";
case REVIEWER -> "reviewers";
};
}
@@ -345,7 +345,7 @@ class FleetConfigTest {
IllegalStateException unknownError = assertThrows(IllegalStateException.class,
() -> FleetConfig.load(unknown).validateCharters());
assertTrue(unknownError.getMessage().contains("architetc"));
assertTrue(unknownError.getMessage().contains("[architect, dev, reviewer]"));
assertTrue(unknownError.getMessage().contains("[architect, dev, hunter, reviewer]"));
}
/**
@@ -812,13 +812,17 @@ class FleetConfigTest {
reviewers:
b:
profile: sonnet
hunters:
c:
profile: sonnet
""");
FleetConfig cfg = FleetConfig.load(f);
assertEquals(List.of("sonnet"), cfg.fleet().profilesFor(MemberRole.DEV));
assertEquals(List.of("sonnet"), cfg.fleet().profilesFor(MemberRole.HUNTER));
assertEquals(List.of("sonnet"), cfg.fleet().profilesFor(MemberRole.REVIEWER));
assertTrue(cfg.fleet().profilesFor(MemberRole.ARCHITECT).isEmpty());
assertEquals(List.of(MemberRole.DEV, MemberRole.REVIEWER), cfg.fleet().rolesConfigured());
assertEquals(List.of(MemberRole.DEV, MemberRole.HUNTER, MemberRole.REVIEWER), cfg.fleet().rolesConfigured());
}
/** The case the two axes exist for: one backend, two roles, and neither is a duplicate. */
@@ -1937,7 +1937,7 @@ class FleetMcpTest {
null, null, null, null, null, null);
assertEquals(Boolean.TRUE, res.isError());
assertTrue(textOf(res).contains("architect, dev, reviewer"), textOf(res));
assertTrue(textOf(res).contains("architect, dev, hunter, reviewer"), textOf(res));
}
// ── CB-619 / fleetd #123: a spawn asking for a role its profile has no slot for must be
@@ -556,6 +556,31 @@ class ClaudeCodeLauncherTest {
"no --agent flag when the role has no agent-definition file");
}
@Test
void hunterRoleUsesItsAgentFileAndStopsUsingItWhenRemoved(@TempDir Path cwd) throws Exception {
FakeHerdr herdr = new FakeHerdr();
Path agentFile = Files.createDirectories(cwd.resolve(".claude/agents")).resolve("hunter.md");
Files.writeString(agentFile, "---\nname: hunter\n---\nSweep for defects.");
FleetConfig.Profile cfg = new FleetConfig.Profile(
"sonnet", "http://gx00.gw:8000", null, null, "FLEETD_WORKER_TOKEN",
List.of("claude"), "tab", "fleetd-workers", "w #{n}", null, null, null);
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null);
svc.spawn(new SpawnRequest("sonnet", cwd.toString(), null, null, null, MemberRole.HUNTER));
List<String> args = spawnedArgs(herdr);
int flag = args.indexOf("--agent");
assertTrue(flag >= 0, "the hunter role reaches its agent-definition file: " + args);
assertEquals("hunter", args.get(flag + 1));
Files.delete(agentFile);
svc.spawn(new SpawnRequest("sonnet", cwd.toString(), null, null, null, MemberRole.HUNTER));
assertFalse(spawnedArgs(herdr).contains("--agent"),
"the hunter role no longer gets an agent when its file is removed");
}
private ClaudeCodeLauncher multiProfile(FakeHerdr herdr) {
FleetConfig.Profile gx10 = new FleetConfig.Profile("gx10", "http://gx10.gw:8000", "coder",
null, "FLEETD_WORKER_TOKEN", List.of("claude"), "tab", "fleetd-workers", "w #{n}", null, null, null);
@@ -10,12 +10,13 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
class MemberRoleTest {
@Test
void theThreeRolesAreArchitectDevAndReviewer() {
assertEquals(3, MemberRole.values().length,
void theFourRolesAreArchitectDevHunterAndReviewer() {
assertEquals(4, MemberRole.values().length,
"a new role changes the charter, the role file, the skill and the authz row — "
+ "adding one is a deliberate act, so this count is meant to fail first");
assertEquals("architect", MemberRole.ARCHITECT.wireName());
assertEquals("dev", MemberRole.DEV.wireName());
assertEquals("hunter", MemberRole.HUNTER.wireName());
assertEquals("reviewer", MemberRole.REVIEWER.wireName());
}
@@ -30,6 +31,7 @@ class MemberRoleTest {
void parseIsCaseInsensitiveAndTrimsSurroundingSpace() {
assertSame(MemberRole.ARCHITECT, MemberRole.parse("Architect"));
assertSame(MemberRole.DEV, MemberRole.parse(" DEV "));
assertSame(MemberRole.HUNTER, MemberRole.parse("HuNtEr"));
assertSame(MemberRole.REVIEWER, MemberRole.parse("ReViEwEr"));
}
@@ -38,7 +40,7 @@ class MemberRoleTest {
IllegalArgumentException e =
assertThrows(IllegalArgumentException.class, () -> MemberRole.parse("archtiect"));
assertTrue(e.getMessage().contains("archtiect"), e.getMessage());
assertTrue(e.getMessage().contains("architect, dev, reviewer"),
assertTrue(e.getMessage().contains("architect, dev, hunter, reviewer"),
"a typo in config should be fixable from the message alone: " + e.getMessage());
}
+54 -3
View File
@@ -169,7 +169,55 @@ hash256() {
# on PATH). "absent" must never be the answer for a file that exists — that conflation, on Linux,
# was the whole defect this ticket fixes.
jar_id() { local f="${1:-$JAR}"; [ -f "$f" ] && hash256 "$f" || echo "absent"; }
running_pid() { pgrep -f "$PATTERN" || true; }
# fleetd #593 — `pgrep -f "$PATTERN"` matches ANY process whose full command line CONTAINS the
# pattern text, and that is not the same thing as "is the daemon". A shell that merely embeds the
# pattern as literal text — a human typing this exact investigation by hand, an ssh-shaped
# `sh -c '...; ...'`, a pipeline, or any other non-exec'ing shell that never replaced itself with
# the pattern-holding command — still shows up in that match, and it is the INSTRUMENT, not the
# daemon. Measured live on this Mac: `sh -c 'echo "target/fleetd.jar" >/dev/null; sleep 30' &`
# leaves a real `sh` process alive (it forks for the `sleep`, it does not exec into it) whose own
# `ps -o args` is `sh -c echo "target/fleetd.jar" >/dev/null; sleep 30` — `pgrep -f "$PATTERN"`
# matches that line right alongside the real `java -jar target/fleetd.jar` process. `pgrep -c`
# (an in-one-call count) does not exist on BSD/macOS at all, so this cannot be fixed by switching
# pgrep flags — it has to filter what pgrep already found, after the fact, in a way that still
# runs on BSD.
#
# fleetd #593 CORRECTION 1 — the first cut of this filter kept everything whose `comm` was NOT a
# shell name (a denylist: sh/bash/zsh/dash/ksh). Two holes in that, both the same false-positive
# shape the ticket exists to remove in the first place:
# 1. a pid `pgrep` just listed can exit before the `ps -o comm=` lookup runs; on a gone pid `ps`
# prints nothing, `comm` ends up empty, and an empty string matches none of the denied shell
# names — so a pid that no longer exists was still counted.
# 2. the denylist only knows the shells someone thought to name. `ssh`, `perl`, `python3`,
# `ruby`, `tail` — anything else that carries the pattern in its own argv — was still
# counted right along with the real daemon, and the ticket names `ssh` as a live route.
# Both close with the same change: allowlist `comm = java` instead of denying shells. Measured on
# the live daemon: `pid=30224 comm=java`. An empty comm (hole 1) is not `java` either, so it is
# excluded for free — no separate "is this pid still alive" check needed.
#
# The objection, because it is real: an allowlist can UNDER-count. If fleetd ever stops being
# launched as `java -jar ...` — a native image, a renamed launcher — `running_pid()` silently
# returns nothing and `assert_single_daemon` stops noticing a second daemon at all. For a guard,
# that false-negative direction is the worse one to be wrong in. This is not a new assumption,
# though: `PATTERN='target/fleetd.jar'` two lines up already assumes the daemon is a jar, which
# is only ever run by `java`. If that launch method changes, `PATTERN` stops matching anything
# before this allowlist would ever get the chance to be wrong — the allowlist rides on the same
# assumption that is already load-bearing, it does not add a new one. Whoever changes the launch
# method needs to update both `PATTERN` and this allowlist together.
running_pid() {
local pid comm out=''
for pid in $(pgrep -f "$PATTERN" 2>/dev/null || true); do
comm="$(ps -o comm= -p "$pid" 2>/dev/null || true)"
comm="${comm##*/}"
comm="${comm#-}"
# Allowlist, not a denylist of wrappers — see the CORRECTION 1 comment above. Anything that
# is not literally `java` is excluded, including an empty comm from a pid that already exited.
[ "$comm" = java ] || continue
out="$out$pid"$'\n'
done
printf '%s' "$out"
}
# fleetd #493 — three small, independently testable pieces of "never build into the path a
# running process holds":
@@ -507,8 +555,11 @@ assert_single_daemon() {
die "more than one fleetd process is running after this restart (pids: $(printf '%s' "$pids" | tr '\n' ' ')).
This is the exact failure a racing supervisor produces: the OLD jar was revived by its
supervisor while this script started a NEW copy. Two daemons on one herdr session kill
each other's members. Investigate with 'pgrep -f \"$PATTERN\"' and stop the wrong one by
hand — do not assume either pid is the one you want."
each other's members. Investigate with 'ps -eo pid,comm,args | grep -F \"$PATTERN\"' and
check the COMM column of each hit yourself before acting — a bare 'pgrep -f \"$PATTERN\"'
(fleetd #593) can match the very shell you type it into, not just the daemon, so it is not
safe remediation advice on its own. Stop the wrong one by hand — do not assume either pid
is the one you want."
fi
}
+130
View File
@@ -433,6 +433,130 @@ test_assert_single_daemon_rejects_two_pids() {
printf '%s' "$output" | grep -qF '4343' || fail "refusal message does not list the pids it found"
}
# fleetd #593 instance 2 — `running_pid()` used to be a bare `pgrep -f "$PATTERN"`, which matches
# ANY process whose full command line contains the pattern TEXT, including a shell that merely
# embeds it as literal text rather than being the daemon. Measured live on this Mac: `pgrep -c`
# (a one-call count) does not exist on BSD at all, and `bash -c "<single command>"` execs in place
# so no parent shell survives to hold the pattern — which is exactly why the defect did not
# reproduce from a plain script and needs a wrapper shaped like this instead. A `sh -c '...; ...'`
# with MORE THAN ONE statement does not get that exec-in-place treatment: the shell forks a child
# for the second statement and stays alive itself, holding the whole `-c` string — pattern text
# included — in its own `ps -o args`, for as long as it runs. That is the same shape an
# `ssh host "…; …"` wrapper or a hand-typed pipeline leaves behind. Before the fix this test would
# have found the wrapper's pid in running_pid()'s output; it must not.
test_running_pid_excludes_self_matching_wrapper_shell() {
local before after wrapper_pid
before="$(running_pid)"
sh -c 'echo "target/fleetd.jar" >/dev/null; sleep 20' &
wrapper_pid=$!
sleep 0.3
after="$(running_pid)"
kill "$wrapper_pid" 2>/dev/null || true
wait "$wrapper_pid" 2>/dev/null || true
[ "$after" = "$before" ] \
|| fail "running_pid() counted a self-matching wrapper shell (pid $wrapper_pid, holding the pattern as literal text in its own argv, not the daemon): before=[$before] after=[$after]"
}
# fleetd #593 CORRECTION 1 — the round-1 version of this test gave its standin an argv[0]
# containing the pattern text (via `exec -a`) and left `comm` as whatever that override produced,
# which was never `java`. That was fine for a denylist-of-shells filter, but the allowlist below
# now requires `comm = java` specifically, so the standin here must actually carry that comm, not
# just avoid being a shell. `exec -a java` overrides argv[0] to `java` while the process itself
# stays a genuine, harmless `sh`; combining it with the same non-exec'ing multi-statement shape
# the wrapper-shell test above uses keeps the pattern text in the process's own `ps -o args` for
# as long as it runs. Measured live on this Mac (BSD/macOS: `ps -o comm=` here reflects argv[0]):
# `comm=java`, `args` contains the pattern, `pgrep -f "$PATTERN"` finds it. Copying a real system
# binary into a scratch path and executing it from there was tried first, for a more literal
# stand-in daemon, and the OS killed it outright (SIGKILL, exit 137 — almost certainly a
# code-signing check on a relocated binary); `exec -a` needs no binary of its own and nothing
# under a scratch directory, and it is the technique CORRECTION 1 names as the right one.
#
# This is the one live-process test in this file whose result could differ on Linux: Linux sets
# `comm` from the actually-executed binary's own path, not from `exec -a`'s argv[0] override (BSD
# ties `comm` to argv[0], which is what makes this technique work here) — so on Linux this
# specific fixture might report `comm=sh`, not `comm=java`, even though the REAL daemon (a literal
# `java -jar target/fleetd.jar` process, never fabricated) is unaffected either way. I could not
# verify this fixture's behavior on Linux, so test_running_pid_counts_a_pid_whose_comm_is_java
# below backstops the same claim (the allowlist admits a pid whose comm is `java`) with a stubbed
# `ps`, which is identical bash on every platform and carries no such platform question.
test_running_pid_finds_a_real_java_named_second_process() {
local before after standin_pid
before="$(running_pid)"
( exec -a java sh -c 'echo "target/fleetd.jar" >/dev/null; sleep 20' ) &
standin_pid=$!
sleep 0.3
after="$(running_pid)"
kill "$standin_pid" 2>/dev/null || true
wait "$standin_pid" 2>/dev/null || true
printf '%s\n' "$after" | grep -qxF "$standin_pid" \
|| fail "running_pid() did not find a real second process (pid $standin_pid, comm forced to 'java' via exec -a) whose own argv holds the pattern: before=[$before] after=[$after]"
}
# fleetd #593 CORRECTION 1, hole 2 — the round-1 filter denied known shell names (sh/bash/zsh/
# dash/ksh) and counted everything else. `ssh`, `perl`, `python3`, `ruby`, `tail` — anything not on
# that list, carrying the pattern in its own argv — was still counted right alongside the real
# daemon, and the ticket names `ssh` as a live route. Stubbing `pgrep`/`ps` (rather than spawning a
# real perl/ssh process) pins the exact discriminator this correction is about — comm, not the
# caller's shape — deterministically on every platform, with no dependency on perl/python3/ruby
# being installed in whatever environment runs this suite, and no dependency on how a given OS
# derives `comm` for a fabricated process (see the comment above
# test_running_pid_finds_a_real_java_named_second_process for why that matters here).
test_running_pid_drops_a_pid_whose_comm_is_not_java() {
pgrep() { printf '4242\n'; }
ps() { printf 'perl\n'; }
local found
found="$(running_pid)"
unset -f pgrep ps
[ -z "$found" ] \
|| fail "running_pid() counted pid 4242 whose comm is 'perl', not 'java' — denying known shell names does not exclude a non-shell wrapper such as ssh or perl (fleetd #593 CORRECTION 1): found=[$found]"
}
# fleetd #593 CORRECTION 1, hole 1 — pgrep can list a pid that exits before the following
# `ps -o comm=` lookup runs; on a gone pid `ps` prints nothing, so `comm` comes back empty. Under
# the round-1 denylist an empty string matched none of the denied shell names, so the dead pid was
# still counted — the exact false-positive shape the ticket exists to remove, just rarer. The
# allowlist fixes this for free: an empty comm is not `java` either.
test_running_pid_drops_a_pid_that_exited_before_the_comm_lookup() {
pgrep() { printf '4242\n'; }
ps() { :; } # a pid that no longer exists: the real `ps -p <gone>` prints nothing and this mirrors that
local found
found="$(running_pid)"
unset -f pgrep ps
[ -z "$found" ] \
|| fail "running_pid() counted pid 4242 whose comm lookup came back empty (the pid had already exited before the lookup ran) — an empty comm must not pass the allowlist (fleetd #593 CORRECTION 1): found=[$found]"
}
# The positive backstop for both stubbed tests above, and for
# test_running_pid_finds_a_real_java_named_second_process on whatever platform that live fixture
# does not itself carry comm=java: the allowlist must still ADMIT the one comm value the real
# daemon actually has. Measured on the real, currently-running daemon on this Mac: `comm=java`.
test_running_pid_counts_a_pid_whose_comm_is_java() {
pgrep() { printf '4242\n'; }
ps() { printf 'java\n'; }
local found
found="$(running_pid)"
unset -f pgrep ps
printf '%s\n' "$found" | grep -qxF '4242' \
|| fail "running_pid() did not count pid 4242 whose comm is 'java' — the daemon's own name must pass the allowlist: found=[$found]"
}
# fleetd #593 instance 3 — assert_single_daemon's refusal message used to tell the operator to
# "Investigate with 'pgrep -f \"\$PATTERN\"'", which — typed by hand or over ssh — is precisely the
# self-matching invocation instance 2 above fixes. A source-text check, the same technique
# test_no_error_lines_message_gated_by_drain_state uses: this is prose inside a die() call, never
# reached by sourcing (the SOURCED guard stops before the main flow, and this text only prints
# from inside a call assert_single_daemon makes when it is already refusing).
test_die_message_does_not_recommend_bare_pgrep_as_remediation() {
local src="$ROOT/scripts/redeploy-fleetd.sh" block bad
block="$(grep -A6 -F 'racing supervisor produces' "$src" || true)"
[ -n "$block" ] || fail "could not find the assert_single_daemon refusal message in redeploy-fleetd.sh"
bad="$(printf '%s' "$block" | grep -F "Investigate with 'pgrep -f" || true)"
[ -z "$bad" ] \
|| fail "assert_single_daemon's die message still hands the operator a bare 'pgrep -f \"\$PATTERN\"' as remediation (fleetd #593) — that is exactly the self-matching invocation"
printf '%s' "$block" | grep -qF 'fleetd #593' \
|| fail "assert_single_daemon's die message does not say in words that a pattern can match the caller (fleetd #593)"
}
# fleetd #511 — jar_id()'s no-argument default was unpinned by any test: nothing proved it reports
# $JAR (the live path) rather than $JAR_STAGED. Both halves matter, so this pins both: the bare call
# must hash the live jar, and an explicit path argument must hash THAT file, not fall back to $JAR.
@@ -1894,6 +2018,12 @@ test_require_drivable_supervisor_accepts_known_kinds
test_count_daemon_pids
test_assert_single_daemon_accepts_one_pid
test_assert_single_daemon_rejects_two_pids
test_running_pid_excludes_self_matching_wrapper_shell
test_running_pid_finds_a_real_java_named_second_process
test_running_pid_drops_a_pid_whose_comm_is_not_java
test_running_pid_drops_a_pid_that_exited_before_the_comm_lookup
test_running_pid_counts_a_pid_whose_comm_is_java
test_die_message_does_not_recommend_bare_pgrep_as_remediation
test_jar_id_defaults_to_live_and_reports_explicit_path
test_hash256_computes_a_real_sha256
test_jar_id_reports_absent_for_missing_file