fleetd #474: ConfigRef.reload() runs the charter tool-surface gate too #475

Closed
agent wants to merge 4 commits from worker/474-charter-check-on-reload-f54a55-17 into main
4 changed files with 262 additions and 19 deletions
Showing only changes of commit d4a2cd720c - Show all commits
+15 -6
View File
@@ -793,12 +793,21 @@ guard:
# .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.
# .claude/skills/<name> is never overwritten — the repo's own copy always wins. 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.
#
# fleetd #393: which member KINDS actually consume this once it is copied. kind: claude-code —
# the Claude Code CLI discovers .claude/skills/ on its own; nothing else is needed. kind: opencode
# — opencode has no such discovery, so OpenCodeLauncher reads whatever landed under
# .claude/skills/ and appends each seeded skill's SKILL.md to the generated instructions[] file
# (opencode's only channel for static guidance text; unlike Claude Code's Skill tool, the content
# is always part of the system prompt, not loaded on demand). Both kinds are covered as of #393 —
# earlier builds copied the files for every kind but only claude-code could read them, and the
# seeding log said "N of M" regardless. Check the per-spawn launcher log (not just the seeding
# log) to see what a given member actually got.
# memberSkills: /path/to/fleetd/checkout/.claude/skills
# Session lifecycle limits (CB-303). All knobs are opt-in; omit or set to null to keep
@@ -331,11 +331,22 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
+ "form — opencode's per-model context limit could not be applied for this profile",
cfg.profile(), cfg.model());
}
// fleetd #393: memberSkills seeding (GitWorktrees#seedSkills) copies skill folders into
// EVERY provisioned worktree's .claude/skills/ regardless of which kind ultimately spawns
// into it — that copy step cannot know the kind, only the caller of GitWorktrees#add does
// (see that method's own javadoc). .claude/skills/ is a Claude Code CLI convention the CLI
// discovers on its own; opencode has no such discovery, so without this, a seeded skill
// never reaches an opencode member even though GitWorktrees logged it as seeded. Read
// whatever landed under <cwd>/.claude/skills/ here — the one place in this launcher that
// knows both the kind (opencode, by construction: this IS OpenCodeLauncher) and the cwd.
List<Path> skillInstructionFiles = skillInstructionFiles(spec.cwd());
// A config file is needed for the bridge MCP mount, a member charter, the IDE MCP (+ its
// guidance overlay, CB-634), a pinned endpoint (CB-508), or a resolvable autoCompactWindow.
// guidance overlay, CB-634), a pinned endpoint (CB-508), a resolvable autoCompactWindow, or
// at least one seeded skill to deliver via instructions[] (fleetd #393).
if (cfg.hasMcp() || cfg.hasIdeMcp() || spec.charter() != null || hasCustomProvider(cfg)
|| wantsContextLimit) {
workerEnv.put("OPENCODE_CONFIG", writeConfig(cfg, spec.charter(), spec.cwd()).toString());
|| wantsContextLimit || !skillInstructionFiles.isEmpty()) {
workerEnv.put("OPENCODE_CONFIG",
writeConfig(cfg, spec.charter(), spec.cwd(), skillInstructionFiles).toString());
}
applyGitToken(workerEnv, cfg);
List<String> argv = argvWithResume(argvWithModel(argvWithAuto(cfg), cfg), spec.resumeSessionId());
@@ -358,6 +369,65 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
return withAgent;
}
/**
* fleetd #393: the {@code SKILL.md} paths under {@code <cwd>/.claude/skills/} this launcher can
* turn into {@code instructions[]} entries, plus the honest log this ticket asks for — emitted
* here, at the one point a skill's fate for THIS spawn is actually known, rather than trusting
* {@code GitWorktrees#seedSkills}'s kind-blind "N of M" line to mean "and it will be read."
*
* <p>Every non-hidden subdirectory of {@code .claude/skills/} is a candidate, whether it got
* there via {@code memberSkills:} seeding or because the target repo ships its own copy — this
* launcher does not care which; it only cares what it can find at spawn time. A candidate with
* a {@code SKILL.md} at its top level (the same shape {@link #writeConfig} already requires for
* the charter and IDE-rules instructions entries) is delivered; anything else is a directory
* this launcher cannot turn into a flat instructions entry, named explicitly in the log rather
* than silently dropped, so a caller sees a real "cannot consume" reason and not just a smaller
* number than {@code GitWorktrees}' own count.
*
* <p>No candidates at all (directory absent or empty) logs nothing — the same
* no-log-when-nothing-to-say shape {@link #hasCustomProvider} and friends already follow, and
* the shape {@code GitWorktrees#seedSkills} itself uses when {@code memberSkills:} is unset.
* A failure to even list the directory is logged and treated as "nothing delivered" — best
* effort, must never fail the spawn, matching {@code GitWorktrees#seedSkills}'s own contract.
*/
private List<Path> skillInstructionFiles(String cwd) {
if (cwd == null || cwd.isBlank()) {
return List.of();
}
Path skillsDir = Path.of(cwd, ".claude", "skills");
if (!Files.isDirectory(skillsDir)) {
return List.of();
}
List<Path> candidates;
try (var listing = Files.list(skillsDir)) {
candidates = listing.filter(Files::isDirectory)
.filter(p -> !p.getFileName().toString().startsWith("."))
.sorted()
.toList();
} catch (IOException e) {
log.warn("could not scan {} for skill folders to deliver to this opencode member: {}",
skillsDir, e.getMessage());
return List.of();
}
if (candidates.isEmpty()) {
return List.of();
}
List<Path> delivered = candidates.stream()
.map(dir -> dir.resolve("SKILL.md"))
.filter(Files::isRegularFile)
.toList();
List<String> undeliverable = candidates.stream()
.filter(dir -> !Files.isRegularFile(dir.resolve("SKILL.md")))
.map(dir -> dir.getFileName().toString())
.toList();
log.info("skill delivery: {} of {} skill folder(s) under {} reached this opencode member via "
+ "instructions[] (opencode does not read .claude/skills/ natively, unlike "
+ "Claude Code){}",
delivered.size(), candidates.size(), skillsDir,
undeliverable.isEmpty() ? "" : "; no SKILL.md, could not be delivered: " + undeliverable);
return delivered;
}
/**
* True when this profile pins its own OpenAI-compatible endpoint (CB-508) rather than using
* whatever provider opencode resolves by default.
@@ -418,8 +488,15 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
* fresh per-spawn directory under {@link #configRoot}, and return the config file's path for
* {@code OPENCODE_CONFIG}. The dir is unique per spawn so concurrent workers never race on it;
* it is best-effort cleaned on JVM exit (worker config is disposable — regenerated every spawn).
*
* @param skillInstructionFiles fleetd #393: absolute {@code SKILL.md} paths from
* {@link #skillInstructionFiles(String)}, appended to
* {@code instructions[]} so a {@code memberSkills:}-seeded skill
* reaches this opencode member the same way the charter and IDE
* rules already do.
*/
private Path writeConfig(FleetConfig.Profile cfg, String charterText, String cwd) {
private Path writeConfig(FleetConfig.Profile cfg, String charterText, String cwd,
List<Path> skillInstructionFiles) {
try {
Path dir = Files.createTempDirectory(configParentDir(), "fleetd-opencode-");
dir.toFile().deleteOnExit();
@@ -450,6 +527,15 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
root.putArray("instructions").add(charter.toAbsolutePath().toString());
}
// fleetd #393: each seeded skill's SKILL.md, delivered as a plain instructions[] entry
// — the only mechanism opencode has for static guidance text. Unlike Claude Code's
// Skill tool, opencode cannot load one of these on demand by name; the content is just
// always part of the system prompt from spawn. That is a real difference in HOW the
// content reaches the member, not a reason to withhold it.
for (Path skillFile : skillInstructionFiles) {
root.withArray("instructions").add(skillFile.toAbsolutePath().toString());
}
if (cfg.hasMcp() || cfg.hasIdeMcp()) {
// One shared mcp node for both servers — putObject would replace the node (and thus
// the other server) on the second call, so build into a single get-or-create node.
@@ -690,14 +690,18 @@ public final class GitWorktrees implements Worktrees {
* {@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.
* <p><b>Kind-blind by construction, not by a backend check here — this used to be a real gap
* (fleetd #393).</b> 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 no
* caller of {@link #add} hands this class a kind to consult. Before fleetd #393, that made the
* log line below a false claim of success for a {@code kind: opencode} member: opencode has no
* built-in discovery of {@code .claude/skills/}, unlike the Claude Code CLI, so a seeded skill
* never reached one. It now does — {@code OpenCodeLauncher#skillInstructionFiles} reads
* whatever this method copied into {@code .claude/skills/} and appends each {@code SKILL.md} to
* the generated {@code instructions[]} — but that delivery, and the log line that honestly
* claims it (kind-aware, unlike this one), happens at the launcher, once the kind is actually
* known, not here.
*/
private void seedSkills(String worktreePath) {
if (memberSkillsSource == null) {
@@ -739,7 +743,15 @@ public final class GitWorktrees implements Worktrees {
if (detail.isEmpty()) {
detail = "no skill folders found under " + source;
}
log.info("skill seeding: {} of {} candidate(s) from {} into {}/.claude/skills — {}",
// fleetd #393: this only claims the copy step, deliberately — it cannot know the member
// kind that will spawn into this worktree (see this method's own javadoc), so it must not
// read as "and the member will act on it." Whether that is true depends on the kind: the
// Claude Code CLI discovers .claude/skills/ on its own; OpenCodeLauncher logs its own
// "skill delivery" line, once the kind is known, naming what it could and could not turn
// into instructions[].
log.info("skill seeding: {} of {} candidate(s) from {} into {}/.claude/skills — {} "
+ "(whether the spawned member can act on this depends on its kind — see "
+ "the launcher's own log for that)",
seeded.size(), seeded.size() + kept.size(), source, worktreePath, detail);
if (seeded.isEmpty()) {
return;
@@ -762,6 +762,142 @@ class OpenCodeLauncherTest {
"no IDE server when ideMcpUrl is unset");
}
// --- fleetd #393: memberSkills seeding must actually reach an opencode member -------------------
//
// Before this fix, GitWorktrees#seedSkills copied skill folders into EVERY provisioned
// worktree's .claude/skills/ and logged "skill seeding: N of M" regardless of which kind ended
// up spawning into that worktree — a claim that held for kind: claude-code (the CLI discovers
// that directory on its own) but was a guaranteed no-op for kind: opencode, which has no such
// discovery. These tests drive the REAL GitWorktrees#add seeding path (not a hand-built
// .claude/skills/ fixture), then spawn an opencode-kind member against the seeded worktree and
// assert on what the member can actually consume — an instructions[] entry — not on the
// seeding log alone. A minimal, non-hermetic git repo is enough here: unlike
// GitWorktreesTest's own seeding tests, nothing in this file cares about core.excludesFile
// composition, only about what lands in .claude/skills/ and whether OpenCodeLauncher reads it.
private static void git(Path cwd, String... args) throws Exception {
Process p = new ProcessBuilder(prepend("git", args)).directory(cwd.toFile())
.redirectErrorStream(true).start();
String out = new String(p.getInputStream().readAllBytes());
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git timed out: git " + String.join(" ", args));
assertEquals(0, p.exitValue(), "git " + String.join(" ", args) + " failed:\n" + out);
}
private static List<String> prepend(String head, String... rest) {
List<String> cmd = new ArrayList<>();
cmd.add(head);
cmd.addAll(List.of(rest));
return cmd;
}
private static Path initRepo(Path dir) throws Exception {
Files.createDirectories(dir);
git(dir, "init", "-q", "-b", "main");
git(dir, "config", "user.email", "test@example.invalid");
git(dir, "config", "user.name", "Test");
Files.writeString(dir.resolve("README.md"), "seed\n");
git(dir, "add", "README.md");
git(dir, "commit", "-q", "-m", "seed");
return dir;
}
@Test
void aSeededSkillReachesTheOpencodeMembersInstructionsArray(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
Path skillsSource = tmp.resolve("skills-src");
Path skillFile = skillsSource.resolve("implementer").resolve("SKILL.md");
Files.createDirectories(skillFile.getParent());
Files.writeString(skillFile, "IMPLEMENTER PROCEDURE\n");
// The real seeding path (fleetd #362), not a hand-built .claude/skills/ fixture — proves
// OpenCodeLauncher reads what GitWorktrees#add actually produced.
dev.ltms.fleet.session.GitWorktrees worktrees =
new dev.ltms.fleet.session.GitWorktrees(tmp.resolve("wts").toString(), null, skillsSource.toString());
String wt = worktrees.add(repo.toString(), "cb-393-opencode", "HEAD");
Path seededSkillMd = Path.of(wt, ".claude", "skills", "implementer", "SKILL.md");
assertTrue(Files.exists(seededSkillMd),
"sanity: the real seeding step must have copied the skill into the worktree");
Logger logger = (Logger) LoggerFactory.getLogger(OpenCodeLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.INFO);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
String cfgPath;
try {
// No mcpUrl, no ideUrl, no fleet (no charter): the seeded skill alone must be enough to
// trigger OPENCODE_CONFIG — proves the gate itself was updated, not only writeConfig's body.
FakeHerdr herdr = new FakeHerdr();
Path configRoot = Files.createDirectory(tmp.resolve("configs"));
service(herdr, configRoot, opencodeIdeCfg(null, null, wt)).spawn();
cfgPath = startEnv(herdr).get("OPENCODE_CONFIG");
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
assertNotNull(cfgPath, "a seeded skill with nothing else configured must still write a config");
JsonNode json = new ObjectMapper().readTree(Path.of(cfgPath).toFile());
List<String> instructions = new ArrayList<>();
json.path("instructions").forEach(n -> instructions.add(n.asText()));
assertTrue(instructions.contains(seededSkillMd.toAbsolutePath().toString()),
"the seeded skill's SKILL.md must be an instructions[] entry — got: " + instructions);
List<String> infos = appender.list.stream()
.filter(e -> e.getLevel() == Level.INFO)
.map(ILoggingEvent::getFormattedMessage)
.toList();
assertTrue(infos.stream().anyMatch(m -> m.contains("skill delivery") && m.contains("1 of 1")),
"the launcher must log, kind-aware, that it delivered the skill — got:\n" + infos);
}
@Test
void aSkillFolderWithoutSkillMdIsNeverDeliveredAndTheLogNamesIt(@TempDir Path tmp) throws Exception {
Path wt = Files.createDirectories(tmp.resolve("wt"));
Path goodSkill = wt.resolve(".claude").resolve("skills").resolve("implementer");
Files.createDirectories(goodSkill);
Files.writeString(goodSkill.resolve("SKILL.md"), "GOOD\n");
Path halfShipped = wt.resolve(".claude").resolve("skills").resolve("half-shipped");
Files.createDirectories(halfShipped);
Files.writeString(halfShipped.resolve("README.md"), "no SKILL.md here\n");
Logger logger = (Logger) LoggerFactory.getLogger(OpenCodeLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.INFO);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
String cfgPath;
try {
FakeHerdr herdr = new FakeHerdr();
Path configRoot = Files.createDirectory(tmp.resolve("configs"));
service(herdr, configRoot, opencodeIdeCfg(null, null, wt.toString())).spawn();
cfgPath = startEnv(herdr).get("OPENCODE_CONFIG");
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
JsonNode json = new ObjectMapper().readTree(Path.of(cfgPath).toFile());
List<String> instructions = new ArrayList<>();
json.path("instructions").forEach(n -> instructions.add(n.asText()));
assertTrue(instructions.contains(goodSkill.resolve("SKILL.md").toAbsolutePath().toString()),
"the well-formed skill is still delivered alongside the malformed one");
assertFalse(instructions.stream().anyMatch(i -> i.contains("half-shipped")),
"a skill folder with no SKILL.md can never become an instructions[] entry");
List<String> infos = appender.list.stream()
.filter(e -> e.getLevel() == Level.INFO)
.map(ILoggingEvent::getFormattedMessage)
.toList();
assertTrue(infos.stream().anyMatch(m -> m.contains("skill delivery") && m.contains("1 of 2")
&& m.contains("half-shipped") && m.contains("could not be delivered")),
"the log must say plainly which folder could not be consumed and why — got:\n" + infos);
}
// --- fleetd #219: config root + discovery root under memberHerdrSocket ------------------------
/** A config with {@code memberHerdrSocket:} set, and optionally {@code worktreeRoot:}/{@code worktreeGroup:}. */