Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 9e813ec179 | |||
| 105c065615 | |||
| f84824ee29 | |||
| 7c684e40d3 | |||
| 6938f52155 |
@@ -774,6 +774,19 @@ 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/<name> 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. Every non-hidden subdirectory of this directory is copied
|
||||
# wholesale, with no per-file allowlist — don't park scratch files or drafts alongside the real
|
||||
# skill folders, they will be copied into every provisioned worktree too.
|
||||
# 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)
|
||||
|
||||
@@ -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));
|
||||
|
||||
|
||||
@@ -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.</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p><strong>The denominator, measured on 2026-09-04 (fleetd #330; recounted for fleetd #333).</strong>
|
||||
* {@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
|
||||
* <p><strong>The denominator, measured on 2026-09-04 (fleetd #330; recounted for fleetd #333);
|
||||
* recounted again for fleetd #362.</strong> {@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
|
||||
* <strong>hot</strong> 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<FleetConfig> {
|
||||
* read {@link #COLD_KEYS} and {@link #SPLIT_KEYS}.
|
||||
*/
|
||||
static final Set<String> 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<FleetConfig> {
|
||||
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:
|
||||
|
||||
@@ -106,6 +106,19 @@ 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. Every non-hidden
|
||||
* subdirectory of this directory is copied wholesale, with no per-file
|
||||
* allowlist — do not park scratch files or drafts alongside the real skill
|
||||
* folders, they will be copied into every provisioned worktree too.
|
||||
*/
|
||||
@JsonIgnoreProperties(ignoreUnknown = true)
|
||||
public record FleetConfig(
|
||||
@@ -130,7 +143,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<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) {
|
||||
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<String, Profile> profiles,
|
||||
@@ -1495,7 +1523,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 +2200,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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -92,6 +92,15 @@ 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;
|
||||
/** Extra environment merged into every {@code git} subprocess this instance runs. Always {@code
|
||||
* Map.of()} from every production constructor. Test seam only (fleetd #362 review fix): lets
|
||||
* {@code GitWorktreesTest} point {@code GIT_CONFIG_GLOBAL} at an isolated temp file so it can
|
||||
* drive the real {@link #add} path against a controlled "operator's global git config" and
|
||||
* prove the excludesFile composition below without ever touching the real machine's config. */
|
||||
private final Map<String, String> gitEnv;
|
||||
private final Consumer<String> 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 +134,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 +157,20 @@ public final class GitWorktrees implements Worktrees {
|
||||
|
||||
/** Test seam combining a configurable {@code group} with {@link #afterWorktreeAdded}. */
|
||||
GitWorktrees(String configuredRoot, String group, Consumer<String> 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<String> afterWorktreeAdded,
|
||||
Function<String[], String> 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<String> afterWorktreeAdded,
|
||||
Function<String[], String> shareGroupRunner, Function<String[], String> worktreeAddRunner) {
|
||||
this(configuredRoot, group, afterWorktreeAdded, shareGroupRunner, worktreeAddRunner, null);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -151,11 +180,30 @@ 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<String> afterWorktreeAdded,
|
||||
Function<String[], String> shareGroupRunner, Function<String[], String> worktreeAddRunner) {
|
||||
Function<String[], String> shareGroupRunner, Function<String[], String> worktreeAddRunner,
|
||||
String memberSkillsSource) {
|
||||
this(configuredRoot, group, afterWorktreeAdded, shareGroupRunner, worktreeAddRunner,
|
||||
memberSkillsSource, Map.of());
|
||||
}
|
||||
|
||||
/**
|
||||
* Full test seam, plus {@code gitEnv} (fleetd #362 review fix, verification only): extra
|
||||
* environment merged into every {@code git} subprocess this instance runs, so a test can isolate
|
||||
* something like {@code GIT_CONFIG_GLOBAL} from the real machine while still driving the real
|
||||
* {@link #add} path end to end. Every production constructor above delegates here with {@code
|
||||
* Map.of()}.
|
||||
*/
|
||||
GitWorktrees(String configuredRoot, String group, Consumer<String> afterWorktreeAdded,
|
||||
Function<String[], String> shareGroupRunner, Function<String[], String> worktreeAddRunner,
|
||||
String memberSkillsSource, Map<String, String> gitEnv) {
|
||||
this.configuredRoot = configuredRoot;
|
||||
this.group = (group == null || group.isBlank()) ? null : group;
|
||||
this.memberSkillsSource = (memberSkillsSource == null || memberSkillsSource.isBlank())
|
||||
? null : memberSkillsSource;
|
||||
this.gitEnv = gitEnv == null ? Map.of() : gitEnv;
|
||||
this.afterWorktreeAdded = afterWorktreeAdded == null ? _ -> {} : afterWorktreeAdded;
|
||||
this.shareGroupRunner = shareGroupRunner != null ? shareGroupRunner : this::exec;
|
||||
this.worktreeAddRunner = worktreeAddRunner != null ? worktreeAddRunner : this::exec;
|
||||
@@ -184,6 +232,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 +623,310 @@ public final class GitWorktrees implements Worktrees {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #362: copy each skill folder from the configured {@link #memberSkillsSource} directory
|
||||
* into {@code <worktreePath>/.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 <name>
|
||||
* skill."}; outside a repo carrying its own copy that line was previously a no-op.
|
||||
*
|
||||
* <p><b>No-op — nothing read, nothing written, nothing logged</b> — when {@link
|
||||
* #memberSkillsSource} is null/blank (today's default), the same off-switch shape as
|
||||
* {@link #shareWithGroup}.
|
||||
*
|
||||
* <p><b>Invariant 1 — a repo's own skill wins.</b> A skill folder already present at
|
||||
* {@code <worktreePath>/.claude/skills/<name>} — because the just-checked-out branch commits its
|
||||
* own copy — is left completely untouched: never overwritten, and never even opened.
|
||||
*
|
||||
* <p><b>Invariant 2 — a seeded skill can never end up in a worker's commit.</b> 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/<nonce>/}, 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.
|
||||
*
|
||||
* <p><b>Compose, don't replace.</b> {@code core.excludesFile} is single-valued: the first cut of
|
||||
* this method pointed it at fleetd's own file with {@code --replace-all}, which SHADOWS whatever
|
||||
* the operator's own (global, or repo-local) {@code core.excludesFile} was already resolving to
|
||||
* inside this worktree, rather than adding to it. Measured concretely: this repo's own {@code
|
||||
* .gitignore} does not ignore {@code target/} — only an operator's global excludesFile does — so
|
||||
* every worker's {@code mvn clean install} would otherwise make {@code target/} appear as
|
||||
* untracked, and {@link #hasUncommitted}'s deliberately-untracked-inclusive {@code git status
|
||||
* --porcelain} (CB-576) would then read every such worktree as dirty forever, so it is never
|
||||
* cleaned up. {@link #excludeSeededSkillsFromGitStatus} now reads whatever {@code
|
||||
* core.excludesFile} resolves to BEFORE writing anything (falling back to git's own documented
|
||||
* default, {@code $XDG_CONFIG_HOME/git/ignore} or {@code $HOME/.config/git/ignore}, when the key
|
||||
* is unset entirely — see {@code gitignore(5)}), and writes that content into its OWN exclude
|
||||
* file ahead of the seeded skill patterns, so every operator-configured pattern keeps applying
|
||||
* inside the seeded worktree exactly as it did before seeding ran.
|
||||
*
|
||||
* <p>Instead of using worktree-scoped-config as an add-then-append (a second key does not exist
|
||||
* for {@code core.excludesFile} — it takes exactly one value), an actual second exclude source
|
||||
* was ruled out because git resolves only ONE {@code core.excludesFile}; concatenating the prior
|
||||
* content into fleetd's own file is what "compose" reduces to for a single-valued key.
|
||||
*
|
||||
* <p>Proven the same way as invariant 2's own leak check: {@code
|
||||
* GitWorktreesTest#seedSkillsComposesWithAnAlreadyEffectiveGlobalExcludesFile} isolates a
|
||||
* synthetic "operator's global config" via {@code GIT_CONFIG_GLOBAL} (never the real machine's),
|
||||
* seeds a skill, and asserts {@code git status --porcelain} is still empty for a file matching
|
||||
* that global config's own ignore pattern.
|
||||
*
|
||||
* <p><b>Invariant 3 — best-effort.</b> 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.
|
||||
*
|
||||
* <p>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}.
|
||||
*
|
||||
* <p><b>Claude Code specific by construction, not by a backend check here.</b> Only {@code
|
||||
* .claude/skills/<name>/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<String> seeded = new ArrayList<>();
|
||||
List<String> 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.
|
||||
*
|
||||
* <p><b>Compose, don't replace.</b> {@code core.excludesFile} is single-valued, so pointing it at
|
||||
* fleetd's own file would otherwise SHADOW whatever excludesFile this worktree was already
|
||||
* resolving (an operator's global config, most commonly) rather than add to it — see the
|
||||
* "Compose, don't replace" discussion on {@link #seedSkills}. {@link
|
||||
* #previouslyEffectiveExcludesFileContent} is read BEFORE this method's own {@code --worktree}
|
||||
* write below, so it still sees whatever was effective beforehand; that content is written into
|
||||
* fleetd's own exclude file ahead of the seeded skill patterns, and the worktree-scoped override
|
||||
* then points at that combined file — so every pattern the operator's own configuration already
|
||||
* applied keeps applying, plus the seeded skill paths.
|
||||
*
|
||||
* <p><b>Assumes a fresh worktree — not idempotent.</b> {@link #seedSkills} only ever calls this
|
||||
* from {@link #add}, which always creates a brand-new worktree, so {@code core.excludesFile} is
|
||||
* never already worktree-scoped-set to fleetd's own file when this runs. A hypothetical second
|
||||
* call on the SAME worktree would read fleetd's own already-composed file back as "previously
|
||||
* effective" (worktree scope now wins) and append the seeded patterns a second time — harmless
|
||||
* to {@code git status} (duplicate exclude lines are a no-op), but not something to rely on. No
|
||||
* guard is added for this because the path does not exist today; if a future caller ever seeds
|
||||
* the same worktree twice, it will need one.
|
||||
*/
|
||||
private void excludeSeededSkillsFromGitStatus(String worktreePath, List<String> seededSkillNames) {
|
||||
exec("git", "-C", worktreePath, "config", "extensions.worktreeConfig", "true");
|
||||
String previouslyEffective = previouslyEffectiveExcludesFileContent(worktreePath);
|
||||
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();
|
||||
if (!previouslyEffective.isEmpty()) {
|
||||
patterns.append(previouslyEffective);
|
||||
}
|
||||
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 content of whatever {@code core.excludesFile} resolves to for {@code worktreePath} right
|
||||
* now — BEFORE {@link #excludeSeededSkillsFromGitStatus} points that key at fleetd's own file —
|
||||
* so it can be carried forward instead of shadowed. {@code --type=path} makes git itself perform
|
||||
* {@code ~}/{@code ~user} expansion the same way it would when actually reading the key to build
|
||||
* exclude rules, rather than handing back a raw, unexpanded config string.
|
||||
*
|
||||
* <p>When the key is unset entirely (exit code non-zero), falls back to git's own documented
|
||||
* default excludes file — {@code $XDG_CONFIG_HOME/git/ignore}, or {@code
|
||||
* $HOME/.config/git/ignore} when that variable is unset — per {@code gitignore(5)}: git applies
|
||||
* that file even with no {@code core.excludesFile} configured at all, so skipping it here would
|
||||
* silently drop patterns an operator never had to configure to get.
|
||||
*
|
||||
* <p>Never throws: a missing, unreadable, or unresolvable file is treated as "nothing to carry
|
||||
* forward" (empty string) — this is a best-effort read in service of {@link #seedSkills}'s own
|
||||
* invariant 3, not a new way for skill seeding to fail a spawn.
|
||||
*
|
||||
* <p><b>Review fix, finding 2.</b> The XDG-fallback branch below does not go through {@code git}
|
||||
* at all, so a first cut of it read {@code XDG_CONFIG_HOME}/{@code HOME} straight from the JVM's
|
||||
* own environment ({@link System#getenv} / {@code user.home}) — unlike every other value this
|
||||
* class resolves, which goes through a {@code git} subprocess and therefore already honours
|
||||
* {@link #gitEnv}. That meant no test could make this branch hermetic, and on any machine
|
||||
* carrying a real {@code ~/.config/git/ignore} (this repo's own dev machine does), every
|
||||
* skill-seeding test silently composed with that real file — correct in production, but
|
||||
* machine-dependent in the test suite, and a future broader pattern in that real file could
|
||||
* silently change what a seeded worktree's {@code git status} reports depending on whose home
|
||||
* directory ran the test. {@link #resolveEnv} now checks {@link #gitEnv} first for both
|
||||
* variables, falling back to the JVM's real environment only when the seam does not supply
|
||||
* them — production behaviour (empty {@link #gitEnv}) is unchanged, and a test can now isolate
|
||||
* this branch exactly as it already isolates every {@code git} subprocess call.
|
||||
*
|
||||
* <p><b>Snapshot, not a reference.</b> The content below is read once, at seeding time, and
|
||||
* copied into fleetd's own exclude file. If the operator edits their global excludesFile
|
||||
* afterward, an already-seeded worktree keeps the old copy — acceptable for a worktree's
|
||||
* expected lifetime, but worth knowing before reading a stale pattern as a bug.
|
||||
*
|
||||
* @return the file's content, trailing-newline-normalized, or {@code ""} when there is nothing
|
||||
* to compose with.
|
||||
*/
|
||||
private String previouslyEffectiveExcludesFileContent(String worktreePath) {
|
||||
String resolvedPath;
|
||||
if (exitCode("git", "-C", worktreePath, "config", "--get", "--type=path", "core.excludesFile") == 0) {
|
||||
resolvedPath = exec("git", "-C", worktreePath, "config", "--get", "--type=path",
|
||||
"core.excludesFile").trim();
|
||||
} else {
|
||||
String xdgConfigHome = resolveEnv("XDG_CONFIG_HOME");
|
||||
Path fallback = (xdgConfigHome != null && !xdgConfigHome.isBlank())
|
||||
? Path.of(xdgConfigHome, "git", "ignore")
|
||||
: Path.of(resolveHome(), ".config", "git", "ignore");
|
||||
resolvedPath = fallback.toString();
|
||||
}
|
||||
if (resolvedPath.isBlank()) {
|
||||
return "";
|
||||
}
|
||||
Path file = Path.of(resolvedPath);
|
||||
if (!Files.isRegularFile(file) || !Files.isReadable(file)) {
|
||||
return "";
|
||||
}
|
||||
try {
|
||||
String content = Files.readString(file);
|
||||
return content.isBlank() ? "" : content.stripTrailing() + System.lineSeparator();
|
||||
} catch (IOException e) {
|
||||
log.warn("could not read previously-effective excludesFile {} while seeding skills into "
|
||||
+ "{}: {} — its patterns will not carry forward into the seeded worktree",
|
||||
file, worktreePath, e.getMessage());
|
||||
return "";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve environment variable {@code name} for {@link #previouslyEffectiveExcludesFileContent}'s
|
||||
* XDG fallback, checking {@link #gitEnv} FIRST so a test can isolate this the same way it
|
||||
* already isolates every {@code git} subprocess this class runs, and falling back to the JVM's
|
||||
* real environment only when the seam does not supply it (always the case in production, where
|
||||
* {@link #gitEnv} is {@code Map.of()}).
|
||||
*/
|
||||
private String resolveEnv(String name) {
|
||||
String fromSeam = gitEnv.get(name);
|
||||
return fromSeam != null ? fromSeam : System.getenv(name);
|
||||
}
|
||||
|
||||
/** Same as {@link #resolveEnv(String)}, for {@code HOME} — falls back to {@code user.home}
|
||||
* (rather than {@code System.getenv("HOME")}) when the seam does not supply it, matching this
|
||||
* class's pre-existing behaviour for every other home-directory resolution. */
|
||||
private String resolveHome() {
|
||||
String fromSeam = gitEnv.get("HOME");
|
||||
return fromSeam != null ? fromSeam : System.getProperty("user.home");
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<String> 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);
|
||||
@@ -1084,6 +1437,9 @@ public final class GitWorktrees implements Worktrees {
|
||||
Process p;
|
||||
try {
|
||||
ProcessBuilder pb = new ProcessBuilder(command).redirectErrorStream(true);
|
||||
if (!gitEnv.isEmpty()) {
|
||||
pb.environment().putAll(gitEnv);
|
||||
}
|
||||
if (extraEnv != null && !extraEnv.isEmpty()) {
|
||||
pb.environment().putAll(extraEnv);
|
||||
}
|
||||
@@ -1119,7 +1475,11 @@ public final class GitWorktrees implements Worktrees {
|
||||
private int exitCode(String... command) {
|
||||
Process p;
|
||||
try {
|
||||
p = new ProcessBuilder(command).redirectErrorStream(true).start();
|
||||
ProcessBuilder pb = new ProcessBuilder(command).redirectErrorStream(true);
|
||||
if (!gitEnv.isEmpty()) {
|
||||
pb.environment().putAll(gitEnv);
|
||||
}
|
||||
p = pb.start();
|
||||
} catch (IOException e) {
|
||||
throw new WorktreeException("failed to start " + command[0] + ": " + e.getMessage(), e);
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
+3
-2
@@ -45,8 +45,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
* <p>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;
|
||||
}
|
||||
|
||||
@@ -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,273 @@ 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 <dir>/<skillName>/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);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #362 review fix, finding 2. {@code core.excludesFile}'s XDG-fallback branch
|
||||
* ({@link GitWorktrees#previouslyEffectiveExcludesFileContent}) does not go through a {@code
|
||||
* git} subprocess, so a first cut of it read {@code XDG_CONFIG_HOME}/{@code HOME} straight from
|
||||
* the JVM's real environment — no test could isolate it, and on any machine carrying a real
|
||||
* {@code ~/.config/git/ignore} (this repo's own dev machine does — measured, not assumed), every
|
||||
* seeding test below silently composed with that real file instead of a controlled fixture.
|
||||
* Every test that seeds at least one skill now constructs its {@link GitWorktrees} with this —
|
||||
* an empty, machine-independent {@code XDG_CONFIG_HOME} (so the fallback resolves to a file that
|
||||
* provably does not exist) plus the same {@code GIT_CONFIG_GLOBAL}/{@code GIT_CONFIG_SYSTEM}/
|
||||
* {@code GIT_TERMINAL_PROMPT} isolation the {@link #git}/{@link #gitOutput} helpers already use
|
||||
* for repo setup — so no test in this class can reach the real machine's home directory.
|
||||
*/
|
||||
private static Map<String, String> hermeticGitEnv(Path tmp) {
|
||||
return Map.of(
|
||||
"GIT_CONFIG_GLOBAL", "/dev/null",
|
||||
"GIT_CONFIG_SYSTEM", "/dev/null",
|
||||
"GIT_TERMINAL_PROMPT", "0",
|
||||
"XDG_CONFIG_HOME", tmp.resolve("hermetic-xdg-config-home-" + System.nanoTime()).toString());
|
||||
}
|
||||
|
||||
/** {@link GitWorktrees}'s full test seam, with a {@code memberSkillsSource} and no other
|
||||
* overrides — the shape every seeding test below needs, isolated via {@link #hermeticGitEnv}. */
|
||||
private static GitWorktrees seedingGitWorktrees(Path root, String memberSkillsSource, Path tmp) {
|
||||
return new GitWorktrees(root.toString(), null, _ -> {}, null, null, memberSkillsSource,
|
||||
hermeticGitEnv(tmp));
|
||||
}
|
||||
|
||||
/** 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 = seedingGitWorktrees(tmp.resolve("wts"), skillsSource.toString(), tmp)
|
||||
.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 = seedingGitWorktrees(tmp.resolve("wts"), skillsSource.toString(), tmp)
|
||||
.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 = seedingGitWorktrees(tmp.resolve("wts"), skillsSource.toString(), tmp);
|
||||
// `plain` never seeds anything (memberSkillsSource is null, so seedSkills no-ops before it
|
||||
// ever touches core.excludesFile), so it does not need the hermetic gitEnv seam.
|
||||
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");
|
||||
|
||||
seedingGitWorktrees(tmp.resolve("wts"), skillsSource.toString(), tmp)
|
||||
.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());
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #362 review fix — the "compose, don't replace" invariant, a third direction alongside
|
||||
* {@link #seedSkillsHidesSeededPathsFromGitStatus} and
|
||||
* {@link #seedSkillsExcludeDoesNotLeakIntoASiblingWorktree}. {@code core.excludesFile} is
|
||||
* single-valued: the first cut of {@code excludeSeededSkillsFromGitStatus} pointed it at
|
||||
* fleetd's own exclude file with {@code --replace-all}, which SHADOWS whatever excludesFile the
|
||||
* worktree was already resolving (an operator's global config, most commonly) instead of adding
|
||||
* to it. Concretely, this repo's own {@code .gitignore} does not ignore {@code target/} — only an
|
||||
* operator's global excludesFile does — so every worker's {@code mvn clean install} would
|
||||
* otherwise make {@code target/} appear as untracked, and CB-576's deliberately
|
||||
* untracked-inclusive {@code hasUncommitted} would then read every such worktree as dirty
|
||||
* forever, so {@code SessionManager} never cleans it up.
|
||||
*
|
||||
* <p>A synthetic "operator's global git config" is isolated via {@code GIT_CONFIG_GLOBAL}
|
||||
* pointed at a throwaway temp file, passed to {@link GitWorktrees} through its {@code gitEnv}
|
||||
* test seam — never the real machine's own git config. That global config ignores {@code
|
||||
* target}. A skill is then seeded through the real {@link GitWorktrees#add} path, and a file
|
||||
* named {@code target} is written into the worktree afterward: {@code git status --porcelain}
|
||||
* must still be empty, proving the operator's own global pattern kept applying after seeding.
|
||||
*/
|
||||
@Test
|
||||
void seedSkillsComposesWithAnAlreadyEffectiveGlobalExcludesFile(@TempDir Path tmp) throws Exception {
|
||||
Path globalExcludes = tmp.resolve("operator-global-ignore");
|
||||
Files.writeString(globalExcludes, "target\n");
|
||||
Path globalConfig = tmp.resolve("operator-global.gitconfig");
|
||||
Files.writeString(globalConfig, "[core]\n\texcludesFile = " + globalExcludes + "\n");
|
||||
Map<String, String> gitEnv = Map.of(
|
||||
"GIT_CONFIG_GLOBAL", globalConfig.toString(),
|
||||
"GIT_CONFIG_SYSTEM", "/dev/null",
|
||||
"GIT_TERMINAL_PROMPT", "0",
|
||||
// core.excludesFile is explicitly set above, so the XDG fallback branch is never
|
||||
// reached here — this is belt-and-braces so the test stays hermetic even if that
|
||||
// ever changes, matching every other seeding test in this file.
|
||||
"XDG_CONFIG_HOME", tmp.resolve("unused-xdg-config-home").toString());
|
||||
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
Path skillsSource = tmp.resolve("skills-src");
|
||||
writeSkill(skillsSource, "implementer", "IMPLEMENTER SKILL\n");
|
||||
GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString(), null, _ -> {},
|
||||
null, null, skillsSource.toString(), gitEnv);
|
||||
|
||||
String wt = gitWorktrees.add(repo.toString(), "cb-362-global-compose", "HEAD");
|
||||
assertEquals("IMPLEMENTER SKILL\n",
|
||||
Files.readString(Path.of(wt, ".claude", "skills", "implementer", "SKILL.md")),
|
||||
"fixture check — the skill really was seeded");
|
||||
|
||||
Files.writeString(Path.of(wt, "target"), "build output the operator's global config ignores\n");
|
||||
|
||||
String porcelain = fullStatus(Path.of(wt));
|
||||
assertEquals("", porcelain,
|
||||
"the operator's own global excludesFile pattern ('target') must still apply after "
|
||||
+ "skill seeding ran — got:\n" + porcelain);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #362 review fix, finding 2: pins the XDG-fallback branch of {@link
|
||||
* GitWorktrees#previouslyEffectiveExcludesFileContent}, exercised when {@code core.excludesFile}
|
||||
* is unset entirely (no global, local, or worktree-scoped value at all) — the branch that used to
|
||||
* read {@code XDG_CONFIG_HOME} straight from the JVM's own environment, unreachable by any test
|
||||
* seam, and would silently compose with whatever real {@code ~/.config/git/ignore} happened to
|
||||
* exist on the machine running the suite. {@code GIT_CONFIG_GLOBAL} points at an empty file (so
|
||||
* {@code core.excludesFile} is genuinely unset, forcing the fallback branch to fire — not the
|
||||
* "already configured" branch {@link #seedSkillsComposesWithAnAlreadyEffectiveGlobalExcludesFile}
|
||||
* covers), and {@code XDG_CONFIG_HOME} is isolated through the {@code gitEnv} seam at a throwaway
|
||||
* temp dir carrying a synthetic {@code git/ignore} that ignores {@code xdg-fallback-marker}. A
|
||||
* skill is seeded through the real {@link GitWorktrees#add} path, and a file named {@code
|
||||
* xdg-fallback-marker} is written into the worktree afterward: {@code git status --porcelain}
|
||||
* must still be empty, proving the XDG-default pattern kept applying after seeding.
|
||||
*
|
||||
* <p>Deleting the fallback (so an unset key composes with {@code ""}) turns this test red with:
|
||||
* {@code expected: <> but was: <?? xdg-fallback-marker\n>} — see the PR body for the pasted
|
||||
* failure from actually running that mutation.
|
||||
*/
|
||||
@Test
|
||||
void seedSkillsComposesWithTheXdgDefaultExcludesFileWhenNoneIsConfigured(@TempDir Path tmp) throws Exception {
|
||||
Path xdgConfigHome = tmp.resolve("xdg-config-home");
|
||||
Files.createDirectories(xdgConfigHome.resolve("git"));
|
||||
Files.writeString(xdgConfigHome.resolve("git").resolve("ignore"), "xdg-fallback-marker\n");
|
||||
Path emptyGlobalConfig = tmp.resolve("empty-global.gitconfig");
|
||||
Files.writeString(emptyGlobalConfig, "");
|
||||
Map<String, String> gitEnv = Map.of(
|
||||
"GIT_CONFIG_GLOBAL", emptyGlobalConfig.toString(),
|
||||
"GIT_CONFIG_SYSTEM", "/dev/null",
|
||||
"GIT_TERMINAL_PROMPT", "0",
|
||||
"XDG_CONFIG_HOME", xdgConfigHome.toString());
|
||||
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
Path skillsSource = tmp.resolve("skills-src");
|
||||
writeSkill(skillsSource, "implementer", "IMPLEMENTER SKILL\n");
|
||||
GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString(), null, _ -> {},
|
||||
null, null, skillsSource.toString(), gitEnv);
|
||||
|
||||
String wt = gitWorktrees.add(repo.toString(), "cb-362-xdg-fallback", "HEAD");
|
||||
assertEquals("IMPLEMENTER SKILL\n",
|
||||
Files.readString(Path.of(wt, ".claude", "skills", "implementer", "SKILL.md")),
|
||||
"fixture check — the skill really was seeded");
|
||||
|
||||
Files.writeString(Path.of(wt, "xdg-fallback-marker"),
|
||||
"build output the XDG default ignore file (not core.excludesFile) covers\n");
|
||||
|
||||
String porcelain = fullStatus(Path.of(wt));
|
||||
assertEquals("", porcelain,
|
||||
"the XDG default excludesFile pattern ('xdg-fallback-marker') must still apply "
|
||||
+ "after skill seeding ran — got:\n" + porcelain);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user