Merge #366: seed the bridge's worker skills into every provisioned worktree
fleetd #362 item 3. A member spawned against a repo that does not ship its own .claude/skills/ could not load implementer, reviewer or hunter at all. Every brief starts with "Load the <name> skill", and outside this repo that line was silently a no-op. memberSkills: <dir> now copies those folders into each provisioned worktree, skipping any name the target repo already ships. Two review rounds, both about the same hazard: core.excludesFile is single-valued, so pointing it at fleetd's own file would SHADOW the operator's. It now composes instead of replacing, and the XDG default excludes file is carried forward too. Verified on this merge, not taken from the worker's report: mvn clean install -> Tests run: 1402, Failures: 0, Errors: 0, BUILD SUCCESS Two mutations run on merge: drop the XDG fallback -> 1 failure, BUILD FAILURE remove the composition itself -> 2 failures, BUILD FAILURE Both directions are pinned.
This commit is contained in:
Dai Ha
@@ -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,
|
||||
@@ -1501,7 +1529,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) {
|
||||
@@ -2178,9 +2206,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, null));
|
||||
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, null));
|
||||
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, null));
|
||||
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