diff --git a/fleetd/fleetd.example.yaml b/fleetd/fleetd.example.yaml index 5788a06..32b0ced 100644 --- a/fleetd/fleetd.example.yaml +++ b/fleetd/fleetd.example.yaml @@ -774,6 +774,17 @@ guard: # so fleetd falls back to the weaker CB-596 sentinel overlay instead (a WARN names the gap). # worktreeGroup: fleet-workers +# fleetd #362: a directory of skill folders (each a subdirectory holding a SKILL.md, the same +# shape as this repo's own .claude/skills/) copied into every PROVISIONED worktree's +# .claude/skills/, so a member spawned against ANY repo — not only one that already ships its own +# copy — can load a bridge skill (e.g. implementer). Unset (the default): no worktree is touched +# beyond today's behaviour. A skill folder the target repo already carries under +# .claude/skills/ is never overwritten — the repo's own copy always wins. Claude Code +# members only; an opencode member reads a different path (.opencode/agent) this key does not +# touch. Best-effort like worktreeGroup above: a missing/unreadable directory here is logged and +# skipped, never a failed spawn. +# memberSkills: /path/to/fleetd/checkout/.claude/skills + # Session lifecycle limits (CB-303). All knobs are opt-in; omit or set to null to keep # the feature disabled. By default the daemon never reaps, caps, or drains sessions. # idleTtlSeconds → reap READY/DONE sessions idle longer than this (never BUSY/SPAWNING) diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index 6058535..669bda6 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -248,7 +248,8 @@ public final class Fleetd { contextCap = cfg.lifecycle().contextCap(); } boolean clearAfterTurn = cfg.lifecycle() != null && cfg.lifecycle().clearAfterTurn(); - SessionManager sessions = new SessionManager(workers, new GitWorktrees(cfg.worktreeRoot(), cfg.worktreeGroup()), + SessionManager sessions = new SessionManager(workers, + new GitWorktrees(cfg.worktreeRoot(), cfg.worktreeGroup(), cfg.memberSkills()), System::nanoTime, contextCap, clearAfterTurn); liveCountRef.set(profileName -> liveSessionCount(sessions.roster(), profileName)); diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java b/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java index 1c37bd1..7c11ccf 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java @@ -39,10 +39,11 @@ import java.util.function.Supplier; * keeps the old value until a restart: {@code lifecycle:}, {@code leadHeartbeat:}, * {@code spawnReadyTimeoutMs} / {@code spawnReadyPollMs}, {@code quarantineCooldownSeconds} * (CB-578 stage B — baked once into the {@code BackendQuarantine} built at startup), - * {@code guard:}, {@code worktreeRoot:} and {@code worktreeGroup:} (both baked once into the - * {@code GitWorktrees} built at {@code Fleetd.java:251} and never rebuilt — fleetd #323 - * instance 2 found {@code worktreeGroup} missing from this list and from - * {@link #changedDeferredKeys}), {@code primary:} (fleetd #326 — {@code Fleetd.java:506, 519, + * {@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 + * {@code worktreeGroup} missing from this list and from {@link #changedDeferredKeys}; + * {@code memberSkills} (fleetd #362) followed the same shape), {@code primary:} (fleetd #326 — {@code Fleetd.java:506, 519, * 520} read {@code cfg.primary()} only off the startup snapshot to build {@code * PrimaryRegistry} and size {@code ReplyPushLoop}'s reminder cap/backoff, and neither is * rebuilt on reload. Say the consequence exactly: {@code primary.terminal} is DEPRECATED @@ -129,9 +130,10 @@ import java.util.function.Supplier; * five of COLD_KEYS" rather than re-listing them, so prose and set cannot drift again. * * - *

The denominator, measured on 2026-09-04 (fleetd #330; recounted for fleetd #333). - * {@code FleetConfig} has 22 top-level record components: 5 cold, 11 deferred, 3 split, 3 - * hot-excluded. Three of them are named nowhere in this file, and the reason is the same for all + *

The denominator, measured on 2026-09-04 (fleetd #330; recounted for fleetd #333); + * recounted again for fleetd #362. {@code FleetConfig} has 23 top-level record components: + * 5 cold, 12 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 * hot and correctly absent — all three are read live off {@code config.get()} * (placement through the {@code CompositePeerLauncher} supplier the Hot bullet names; @@ -211,7 +213,7 @@ public final class ConfigRef implements Supplier { * read {@link #COLD_KEYS} and {@link #SPLIT_KEYS}. */ static final Set DEFERRED_KEYS = Set.of( - "guard", "worktreeRoot", "worktreeGroup", "primary", "configReload", + "guard", "worktreeRoot", "worktreeGroup", "memberSkills", "primary", "configReload", "leadHeartbeat", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "quarantineCooldownSeconds", "profiles"); @@ -402,6 +404,13 @@ public final class ConfigRef implements Supplier { if (!Objects.equals(old.worktreeGroup(), fresh.worktreeGroup())) { changed.add("worktreeGroup"); } + // fleetd #362: baked into the same GitWorktrees as worktreeRoot/worktreeGroup + // (Fleetd.java:251) and never rebuilt either — a reload that changes only memberSkills + // must be reported the same way, or a newly provisioned worktree keeps seeding from (or + // skipping) the old source directory with nothing telling the operator why. + if (!Objects.equals(old.memberSkills(), fresh.memberSkills())) { + changed.add("memberSkills"); + } // fleetd #326: Fleetd.java:506, 519, 520 read cfg.primary() only off the startup snapshot // (PrimaryRegistry's pinned terminal, ReplyPushLoop's reminder cap and backoff) — neither is // rebuilt on reload, so a changed value needs a restart. Note what it does NOT mean: diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java index ef96eb1..2ac430c 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java @@ -106,6 +106,16 @@ import java.util.regex.PatternSyntaxException; * When {@code memberHerdrSocket} is NOT configured this field is never * consulted at all; fleetd keeps reading its own {@code $SHELL}, exactly as * before this field existed. + * @param memberSkills fleetd #362: nullable directory of skill folders (each a subdirectory + * holding a {@code SKILL.md}, the same shape as this repo's own {@code + * .claude/skills/}) copied into every provisioned worktree's {@code + * .claude/skills/}, so a member spawned against ANY repo — not only one that + * already ships its own copy — can load a bridge skill such as {@code + * implementer}. {@code null}/blank ⇒ off: no worktree is touched beyond + * today's behaviour. A skill folder the target repo already carries is never + * overwritten — see {@link dev.ltms.fleet.session.GitWorktrees}. Claude Code + * members only; an opencode member's equivalent lives under a different path + * ({@code .opencode/agent}) and is not covered by this key. */ @JsonIgnoreProperties(ignoreUnknown = true) public record FleetConfig( @@ -130,7 +140,22 @@ public record FleetConfig( MemberCredentials memberCredentials, Coordinator coordinator, String worktreeGroup, - String memberLoginShell) { + String memberLoginShell, + String memberSkills) { + + /** Back-compat form before the {@code memberSkills} key was added. */ + public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map 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) { + this(bind, herdrSocket, memberHerdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs, + spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, health, placement, auth, + configReload, quarantineCooldownSeconds, memberCredentials, coordinator, worktreeGroup, + memberLoginShell, null); + } /** Back-compat form before the {@code memberLoginShell} key was added. */ public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map profiles, @@ -1495,7 +1520,7 @@ public record FleetConfig( "bind", "herdrSocket", "memberHerdrSocket", "profiles", "guard", "worktreeRoot", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "fleet", "leadHeartbeat", "health", "placement", "auth", "configReload", "quarantineCooldownSeconds", - "memberCredentials", "coordinator", "worktreeGroup", "memberLoginShell"); + "memberCredentials", "coordinator", "worktreeGroup", "memberLoginShell", "memberSkills"); /** Load and validate config from {@code path}. */ public static FleetConfig load(Path path) { @@ -2172,9 +2197,12 @@ public record FleetConfig( // memberLoginShell is left as-is (fleetd #213), like worktreeGroup: null/blank is "not // configured", and there is no sane non-null default — a member's login shell is // operator-specific and only meaningful when memberHerdrSocket is also set. + // memberSkills is left as-is (fleetd #362), like worktreeGroup/memberLoginShell: null/blank + // is "off", and there is no sane non-null default — the daemon may not even run from a + // checkout that ships its own .claude/skills/. 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); + quarantineCooldown, mc, coordinator, worktreeGroup, memberLoginShell, memberSkills); } /** diff --git a/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java b/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java index 1da0f95..0bbf464 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java +++ b/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java @@ -92,6 +92,9 @@ public final class GitWorktrees implements Worktrees { private final String configuredRoot; /** OS group name for {@link #shareWithGroup} (fleetd #185 stage 3); {@code null} ⇒ feature off. */ private final String group; + /** Source directory of skill folders for {@link #seedSkills} (fleetd #362, {@code memberSkills:} + * in config); {@code null} ⇒ feature off, no worktree is touched beyond today's behaviour. */ + private final String memberSkillsSource; private final Consumer afterWorktreeAdded; /** How the initial {@code git worktree add} command runs. Package-private test seam for an * interrupted command after Git has made worktree state. */ @@ -125,7 +128,20 @@ public final class GitWorktrees implements Worktrees { * config); null/blank ⇒ {@link #shareWithGroup} is a no-op. */ public GitWorktrees(String configuredRoot, String group) { - this(configuredRoot, group, _ -> {}); + this(configuredRoot, group, (String) null); + } + + /** + * @param configuredRoot nullable absolute or relative path; null/blank derives a sibling of + * the repo root. + * @param group optional OS group name (fleetd #185 stage 3, {@code worktreeGroup:} + * in config); null/blank ⇒ {@link #shareWithGroup} is a no-op. + * @param memberSkillsSource fleetd #362: optional directory of skill folders ({@code + * memberSkills:} in config) copied into every provisioned worktree's + * {@code .claude/skills/}; null/blank ⇒ {@link #seedSkills} is a no-op. + */ + public GitWorktrees(String configuredRoot, String group, String memberSkillsSource) { + this(configuredRoot, group, _ -> {}, null, null, memberSkillsSource); } /** Test seam for changing a real worktree between its creation and its security check. */ @@ -135,13 +151,20 @@ public final class GitWorktrees implements Worktrees { /** Test seam combining a configurable {@code group} with {@link #afterWorktreeAdded}. */ GitWorktrees(String configuredRoot, String group, Consumer afterWorktreeAdded) { - this(configuredRoot, group, afterWorktreeAdded, null, null); + this(configuredRoot, group, afterWorktreeAdded, null, null, null); } /** Test seam for changing how {@link #shareWithGroup}'s processes run. */ GitWorktrees(String configuredRoot, String group, Consumer afterWorktreeAdded, Function shareGroupRunner) { - this(configuredRoot, group, afterWorktreeAdded, shareGroupRunner, null); + this(configuredRoot, group, afterWorktreeAdded, shareGroupRunner, null, null); + } + + /** Test seam for changing how the initial {@code git worktree add} command runs, with no + * {@code memberSkillsSource} configured. */ + GitWorktrees(String configuredRoot, String group, Consumer afterWorktreeAdded, + Function shareGroupRunner, Function worktreeAddRunner) { + this(configuredRoot, group, afterWorktreeAdded, shareGroupRunner, worktreeAddRunner, null); } /** @@ -151,11 +174,15 @@ public final class GitWorktrees implements Worktrees { * * @param shareGroupRunner {@code null} ⇒ the real {@link #exec(String...)}. * @param worktreeAddRunner {@code null} ⇒ the real {@link #exec(String...)}. + * @param memberSkillsSource {@code null}/blank ⇒ {@link #seedSkills} is a no-op. */ GitWorktrees(String configuredRoot, String group, Consumer afterWorktreeAdded, - Function shareGroupRunner, Function worktreeAddRunner) { + Function shareGroupRunner, Function worktreeAddRunner, + String memberSkillsSource) { this.configuredRoot = configuredRoot; this.group = (group == null || group.isBlank()) ? null : group; + this.memberSkillsSource = (memberSkillsSource == null || memberSkillsSource.isBlank()) + ? null : memberSkillsSource; this.afterWorktreeAdded = afterWorktreeAdded == null ? _ -> {} : afterWorktreeAdded; this.shareGroupRunner = shareGroupRunner != null ? shareGroupRunner : this::exec; this.worktreeAddRunner = worktreeAddRunner != null ? worktreeAddRunner : this::exec; @@ -184,6 +211,7 @@ public final class GitWorktrees implements Worktrees { configureEnvironmentCredentialHelper(repoRoot, wt); configureHttpsUrlRewriteForSshOrigin(repoRoot, wt); isolateToolSurface(wt); + seedSkills(wt); } catch (RuntimeException e) { cleanupAfterAddFailure(repoRoot, wt, branch, e); throw e; @@ -574,6 +602,177 @@ public final class GitWorktrees implements Worktrees { return true; } + /** + * fleetd #362: copy each skill folder from the configured {@link #memberSkillsSource} directory + * into {@code /.claude/skills/}, so a member spawned against ANY repo — not only + * one that already ships its own {@code .claude/skills/} — can load a bridge skill such as + * {@code implementer}. Every brief this fleet sends starts with {@code "Load the + * skill."}; outside a repo carrying its own copy that line was previously a no-op. + * + *

No-op — nothing read, nothing written, nothing logged — when {@link + * #memberSkillsSource} is null/blank (today's default), the same off-switch shape as + * {@link #shareWithGroup}. + * + *

Invariant 1 — a repo's own skill wins. A skill folder already present at + * {@code /.claude/skills/} — because the just-checked-out branch commits its + * own copy — is left completely untouched: never overwritten, and never even opened. + * + *

Invariant 2 — a seeded skill can never end up in a worker's commit. Every path this + * writes is untracked in the target repo (that is the whole reason it is being seeded), so + * {@code git status} would otherwise show each one as a new, addable, committable path. The + * repo-wide {@code .git/info/exclude} is NOT used for this: measured against a real linked + * worktree, that file resolves to the repository's COMMON git dir even from a worktree (the + * same file {@link dev.ltms.fleet.member.ClaudeCodeLauncher#writeIdeOverlay writeIdeOverlay} + * appends {@code CLAUDE.local.md} to), so an entry written there would hide the seeded skill + * from {@code git status} in the PRIMARY's own checkout and every sibling worktree too — not + * only this one. Instead, {@link #excludeSeededSkillsFromGitStatus} points {@code + * core.excludesFile} at a file scoped {@code --worktree} (the same {@code + * extensions.worktreeConfig} mechanism {@link #configureEnvironmentCredentialHelper} already + * relies on) that itself lives under this worktree's own private git dir ({@code + * .git/worktrees//}, OUTSIDE the working tree) — invisible to this worktree's {@code git + * status} and structurally impossible for this worktree to commit, with no effect on any other + * worktree or the primary checkout. Proven with a real {@code git status --porcelain} in + * {@code GitWorktreesTest}, not by reasoning. + * + *

Invariant 3 — best-effort. A missing/unreadable {@link #memberSkillsSource}, or a + * copy/exclude failure, is logged and skipped — it must never fail the spawn, the same contract + * {@link #overlayParity} and {@link #isolateToolSurface} already hold. + * + *

Recorded for the worker itself the same way fleetd #134 records {@code + * fleet.neutralizedConfig}: {@code fleet.seededSkills} (one value per seeded skill folder) and + * {@code fleet.seededSkillsNote}, readable with {@code git config --worktree --get-all + * fleet.seededSkills}. + * + *

Claude Code specific by construction, not by a backend check here. Only {@code + * .claude/skills//SKILL.md} is a path any launcher reads today (opencode's equivalent is a + * different shape under {@code .opencode/agent}, out of scope — see issue #362). This method + * only copies files; like {@link #isolateToolSurface} — which neutralizes BOTH {@code .mcp.json} + * and {@code opencode.json} unconditionally — it runs the same for every worktree regardless of + * which backend ultimately spawns into it, because the backend is not yet chosen at {@link #add} + * time. A seeded {@code .claude/skills/} directory in an opencode member's worktree is simply + * never read by that launcher. + */ + private void seedSkills(String worktreePath) { + if (memberSkillsSource == null) { + return; + } + Path source = Path.of(memberSkillsSource).toAbsolutePath().normalize(); + if (!Files.isDirectory(source)) { + log.warn("memberSkills source '{}' is not a directory — skipping skill seeding for worktree {}", + source, worktreePath); + return; + } + Path skillsRoot = Path.of(worktreePath).resolve(".claude").resolve("skills"); + List seeded = new ArrayList<>(); + List kept = new ArrayList<>(); + try (var candidates = Files.list(source)) { + for (Path candidate : candidates + .filter(Files::isDirectory) + .filter(p -> !p.getFileName().toString().startsWith(".")) + .sorted() + .toList()) { + String name = candidate.getFileName().toString(); + Path dst = skillsRoot.resolve(name); + if (Files.exists(dst)) { + kept.add(name); + continue; + } + copySkillDirectory(candidate, dst); + seeded.add(name); + } + } catch (IOException | RuntimeException e) { + log.warn("failed to seed skills into worktree {} from memberSkills source '{}': {}", + worktreePath, source, e.getMessage()); + return; + } + String detail = seeded.isEmpty() ? "" : "seeded: " + String.join(", ", seeded); + if (!kept.isEmpty()) { + detail += (detail.isEmpty() ? "" : "; ") + "kept the repo's own copy of: " + String.join(", ", kept); + } + if (detail.isEmpty()) { + detail = "no skill folders found under " + source; + } + log.info("skill seeding: {} of {} candidate(s) from {} into {}/.claude/skills — {}", + seeded.size(), seeded.size() + kept.size(), source, worktreePath, detail); + if (seeded.isEmpty()) { + return; + } + try { + excludeSeededSkillsFromGitStatus(worktreePath, seeded); + recordSeededSkillsForWorker(worktreePath, seeded); + } catch (RuntimeException e) { + log.warn("seeded skill(s) {} into {} but could not hide them from git status: {} — " + + "they may show as untracked; never commit them", seeded, worktreePath, e.getMessage()); + } + } + + /** Recursively copy a skill folder ({@code src}) into a fresh destination ({@code dst}) that + * {@link #seedSkills} has already confirmed does not exist, preserving the directory structure + * (e.g. {@code implementer/SKILL.md}, {@code implementer/references/...}). */ + private static void copySkillDirectory(Path src, Path dst) { + try (var walk = Files.walk(src)) { + for (Path path : walk.sorted().toList()) { + Path target = dst.resolve(src.relativize(path).toString()); + if (Files.isDirectory(path)) { + Files.createDirectories(target); + } else { + Files.createDirectories(target.getParent()); + Files.copy(path, target, StandardCopyOption.COPY_ATTRIBUTES); + } + } + } catch (IOException e) { + throw new WorktreeException("cannot copy skill directory " + src + " -> " + dst + ": " + + e.getMessage(), e); + } + } + + /** + * Make every path in {@code seededSkillNames} (each a name under {@code .claude/skills/}) + * invisible to {@code git status} in THIS worktree only — see the invariant-2 discussion on + * {@link #seedSkills}. Sets {@code core.excludesFile} scoped {@code --worktree} to a file + * written under this worktree's own private git dir ({@code git rev-parse + * --absolute-git-dir}), which lives outside the working tree, so the exclude file itself can + * never be committed either. + * + *

This replaces any {@code --worktree}-scoped {@code core.excludesFile} this worktree already + * had — acceptable because a freshly provisioned worktree has none, and worktree-scoped git + * config is already used exclusively for fleetd's own isolation (the credential helper, the SSH + * rewrite) rather than anything an operator sets by hand. + */ + private void excludeSeededSkillsFromGitStatus(String worktreePath, List seededSkillNames) { + exec("git", "-C", worktreePath, "config", "extensions.worktreeConfig", "true"); + String gitDir = exec("git", "-C", worktreePath, "rev-parse", "--absolute-git-dir").trim(); + Path excludeFile = Path.of(gitDir, "fleet-seeded-skills-exclude"); + StringBuilder patterns = new StringBuilder(); + for (String name : seededSkillNames) { + patterns.append("/.claude/skills/").append(name).append('/').append(System.lineSeparator()); + } + try { + Files.writeString(excludeFile, patterns.toString()); + } catch (IOException e) { + throw new WorktreeException("cannot write skills exclude file " + excludeFile + ": " + + e.getMessage(), e); + } + exec("git", "-C", worktreePath, "config", "--worktree", "--replace-all", "core.excludesFile", + excludeFile.toString()); + } + + /** + * The worker-readable half of fleetd #362, mirroring {@link #recordNeutralizedConfigForWorker}: + * record which skill folders were seeded where the worker itself can read it, without a + * working-tree file that would show up in {@code git status}. + */ + private void recordSeededSkillsForWorker(String worktreePath, List seeded) { + exec("git", "-C", worktreePath, "config", "extensions.worktreeConfig", "true"); + for (String name : seeded) { + exec("git", "-C", worktreePath, "config", "--worktree", "--add", "fleet.seededSkills", name); + } + exec("git", "-C", worktreePath, "config", "--worktree", "fleet.seededSkillsNote", + "each fleet.seededSkills value names a skill folder fleetd copied into " + + ".claude/skills/ because this repo did not already ship it; it is excluded " + + "from git status (core.excludesFile, worktree-scoped) and must never be committed"); + } + @Override public void remove(String repoRoot, String worktreePath) { Path p = Path.of(worktreePath); diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelReportingCoverageTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelReportingCoverageTest.java index aea7bba..f08ce0f 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelReportingCoverageTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelReportingCoverageTest.java @@ -107,6 +107,7 @@ class ConfigRefTopLevelReportingCoverageTest { v.put("coordinator", new FleetConfig.Coordinator("amqp://coord-a", null, "self-a", 1)); v.put("worktreeGroup", "group-a"); v.put("memberLoginShell", null); + v.put("memberSkills", "/skills/a"); assertNamesMatchComponents(v); return v; } @@ -147,6 +148,7 @@ class ConfigRefTopLevelReportingCoverageTest { v.put("coordinator", new FleetConfig.Coordinator("amqp://coord-b", null, "self-b", 2)); v.put("worktreeGroup", "group-b"); v.put("memberLoginShell", null); + v.put("memberSkills", "/skills/b"); assertNamesMatchComponents(v); return v; } diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigWithDefaultsPreservesEveryComponentTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigWithDefaultsPreservesEveryComponentTest.java index 1f710a0..9f03f08 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigWithDefaultsPreservesEveryComponentTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigWithDefaultsPreservesEveryComponentTest.java @@ -45,8 +45,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; *

Why this is a valid check for every component, not just some: {@link #withDefaults()}'s own * comments document that it only ever REPLACES a component when the incoming value is {@code null} * (or blank, for {@code placement}) — {@code broker}/{@code primary}/{@code leadHeartbeat}/ - * {@code configReload}/{@code coordinator}/{@code worktreeGroup}/{@code memberLoginShell} are left - * as-is unconditionally, and {@code bind}/{@code guard}/{@code lifecycle}/{@code auth}/ + * {@code configReload}/{@code coordinator}/{@code worktreeGroup}/{@code memberLoginShell}/ + * {@code memberSkills} are left as-is unconditionally, and {@code bind}/{@code guard}/{@code lifecycle}/{@code auth}/ * {@code fleet}/{@code quarantineCooldownSeconds}/{@code memberCredentials}/{@code placement} are * replaced only on null/blank input. A value that is never null or blank going in must therefore * never change coming out, for every current component. No exclusion is needed today. @@ -95,6 +95,7 @@ class FleetConfigWithDefaultsPreservesEveryComponentTest { v.put("coordinator", new FleetConfig.Coordinator("amqp://coord-guard", null, "self-guard", 3)); v.put("worktreeGroup", "group-guard"); v.put("memberLoginShell", "/bin/zsh"); + v.put("memberSkills", "/skills/guard"); assertNamesMatchComponents(v); return v; } diff --git a/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java b/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java index 971c4f8..a2cb113 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java @@ -12,6 +12,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -1469,4 +1470,137 @@ class GitWorktreesTest { assertTrue(reportingAppender.list.isEmpty(), "a null/empty overlay must log nothing, got:\n" + capturedMessages()); } + + // ---- fleetd #362: seedSkills. Drives GitWorktrees#add end-to-end (not a bare worktree) so the + // real memberSkillsSource wiring is exercised, exactly like the credential-helper/origin tests + // above do for their own seams. ---- + + /** Write {@code content} as {@code

//SKILL.md}, creating {@code dir} first. */ + private static void writeSkill(Path dir, String skillName, String content) throws IOException { + Path skillFile = dir.resolve(skillName).resolve("SKILL.md"); + Files.createDirectories(skillFile.getParent()); + Files.writeString(skillFile, content); + } + + /** Acceptance criterion 2 (part 1): a worktree with no {@code .claude/} at all gets the skill + * copied in from the configured {@code memberSkillsSource}, structure and content intact. */ + @Test + void seedSkillsCopiesIntoAWorktreeWithNoClaudeDirAtAll(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + Path skillsSource = tmp.resolve("skills-src"); + writeSkill(skillsSource, "implementer", "IMPLEMENTER SKILL\n"); + + String wt = new GitWorktrees(tmp.resolve("wts").toString(), null, skillsSource.toString()) + .add(repo.toString(), "cb-362-fresh", "HEAD"); + + assertEquals("IMPLEMENTER SKILL\n", + Files.readString(Path.of(wt, ".claude", "skills", "implementer", "SKILL.md"))); + } + + /** Acceptance criterion 2 (part 2) / invariant 1: a repo that already ships its own {@code + * implementer} skill keeps it byte-for-byte — fleetd's copy is never written over it, even + * though the configured source also carries a same-named skill with different content. */ + @Test + void seedSkillsNeverOverwritesAReposOwnSkill(@TempDir Path tmp) throws Exception { + Path repo = tmp.resolve("repo"); + Files.createDirectories(repo); + git(repo, "init", "-q", "-b", "main"); + git(repo, "config", "user.email", "test@example.invalid"); + git(repo, "config", "user.name", "Test"); + writeSkill(repo.resolve(".claude/skills"), "implementer", "REPO OWN SKILL\n"); + git(repo, "add", ".claude"); + git(repo, "commit", "-q", "-m", "repo ships its own implementer skill"); + Path skillsSource = tmp.resolve("skills-src"); + writeSkill(skillsSource, "implementer", "FLEETD SKILL — must never land here\n"); + + String wt = new GitWorktrees(tmp.resolve("wts").toString(), null, skillsSource.toString()) + .add(repo.toString(), "cb-362-repo-own", "HEAD"); + + assertEquals("REPO OWN SKILL\n", + Files.readString(Path.of(wt, ".claude", "skills", "implementer", "SKILL.md")), + "the repo's own committed skill must survive untouched"); + } + + /** Acceptance criterion 2 (part 3) / invariant 3: a misconfigured or missing {@code + * memberSkillsSource} must never fail the spawn — the worktree is still created. */ + @Test + void seedSkillsIsBestEffortWhenSourceDoesNotExist(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + String missingSource = tmp.resolve("does-not-exist").toString(); + + String wt = new GitWorktrees(tmp.resolve("wts").toString(), null, missingSource) + .add(repo.toString(), "cb-362-missing-src", "HEAD"); + + assertTrue(Files.isDirectory(Path.of(wt)), "the spawn must still produce a worktree"); + assertFalse(Files.exists(Path.of(wt, ".claude", "skills")), + "nothing should be seeded when the source directory does not exist"); + assertTrue(capturedMessages().stream().anyMatch(m -> m.contains("is not a directory")), + "expected a warning naming the bad memberSkills source, got:\n" + capturedMessages()); + } + + /** Acceptance criterion 3: prove invariant 2 with a real git command — a freshly seeded skill + * must not appear in {@code git status --porcelain} for the worktree it was seeded into. */ + @Test + void seedSkillsHidesSeededPathsFromGitStatus(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + Path skillsSource = tmp.resolve("skills-src"); + writeSkill(skillsSource, "implementer", "IMPLEMENTER SKILL\n"); + + String wt = new GitWorktrees(tmp.resolve("wts").toString(), null, skillsSource.toString()) + .add(repo.toString(), "cb-362-status", "HEAD"); + + assertEquals("", fullStatus(Path.of(wt)), + "a seeded skill must be invisible to git status, so it can never be staged or committed"); + } + + /** Invariant 2, the other direction: the exclude {@link #seedSkillsHidesSeededPathsFromGitStatus} + * proves is scoped to ONE worktree, not the whole repo. A second worktree of the same repo, + * provisioned with no {@code memberSkillsSource}, still reports an untracked {@code + * .claude/skills/} the ordinary way — proving the exclude did not leak in via the shared + * {@code .git/info/exclude} (which a linked worktree resolves to the repo's COMMON git dir). */ + @Test + void seedSkillsExcludeDoesNotLeakIntoASiblingWorktree(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + Path skillsSource = tmp.resolve("skills-src"); + writeSkill(skillsSource, "implementer", "IMPLEMENTER SKILL\n"); + GitWorktrees seeding = new GitWorktrees(tmp.resolve("wts").toString(), null, skillsSource.toString()); + GitWorktrees plain = new GitWorktrees(tmp.resolve("wts").toString()); + + String seededWt = seeding.add(repo.toString(), "cb-362-scope-a", "HEAD"); + String plainWt = plain.add(repo.toString(), "cb-362-scope-b", "HEAD"); + // Simulate the same untracked shape landing in the sibling worktree by hand, since `plain` + // was never configured with a memberSkillsSource to seed it itself. + writeSkill(Path.of(plainWt, ".claude", "skills"), "implementer", "unrelated untracked content\n"); + + assertEquals("", fullStatus(Path.of(seededWt)), "seeded worktree stays clean"); + assertTrue(porcelainPaths(fullStatus(Path.of(plainWt))).contains(".claude/"), + "an unrelated worktree's own untracked .claude/ must still show up in its status — " + + "the seeded worktree's exclude must not have leaked into it, got:\n" + + fullStatus(Path.of(plainWt))); + } + + /** Criterion 2's log shape, mirroring the {@code overlayParity} log assertions above: the + * denominator, what was seeded, and what was kept because the repo already had it. */ + @Test + void seedSkillsLogsSeededAndKept(@TempDir Path tmp) throws Exception { + reportingLogger.setLevel(Level.INFO); + Path repo = tmp.resolve("repo"); + Files.createDirectories(repo); + git(repo, "init", "-q", "-b", "main"); + git(repo, "config", "user.email", "test@example.invalid"); + git(repo, "config", "user.name", "Test"); + writeSkill(repo.resolve(".claude/skills"), "hunter", "REPO OWN HUNTER\n"); + git(repo, "add", "."); + git(repo, "commit", "-q", "-m", "repo ships hunter only"); + Path skillsSource = tmp.resolve("skills-src"); + writeSkill(skillsSource, "hunter", "FLEETD HUNTER\n"); + writeSkill(skillsSource, "implementer", "FLEETD IMPLEMENTER\n"); + + new GitWorktrees(tmp.resolve("wts").toString(), null, skillsSource.toString()) + .add(repo.toString(), "cb-362-log", "HEAD"); + + assertTrue(capturedMessages().stream().anyMatch(m -> + m.contains("seeded: implementer") && m.contains("kept the repo's own copy of: hunter")), + "expected a summary naming both the seeded and kept skills, got:\n" + capturedMessages()); + } }