diff --git a/fleetd/fleetd.example.yaml b/fleetd/fleetd.example.yaml index 08e97f9..7a0dfd8 100644 --- a/fleetd/fleetd.example.yaml +++ b/fleetd/fleetd.example.yaml @@ -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/ 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/ 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 diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java index e7330dc..c1aed92 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java @@ -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 /.claude/skills/ here — the one place in this launcher that + // knows both the kind (opencode, by construction: this IS OpenCodeLauncher) and the cwd. + List 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 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 /.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." + * + *

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. + * + *

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 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 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 delivered = candidates.stream() + .map(dir -> dir.resolve("SKILL.md")) + .filter(Files::isRegularFile) + .toList(); + List 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 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. diff --git a/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java b/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java index af80e13..71898d2 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java +++ b/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java @@ -690,14 +690,18 @@ public final class GitWorktrees implements Worktrees { * {@code fleet.seededSkillsNote}, readable with {@code git config --worktree --get-all * fleet.seededSkills}. * - *

Claude Code specific by construction, not by a backend check here. Only {@code - * .claude/skills//SKILL.md} is a path any launcher reads today (opencode's equivalent is a - * different shape under {@code .opencode/agent}, out of scope — see issue #362). This method - * only copies files; like {@link #isolateToolSurface} — which neutralizes BOTH {@code .mcp.json} - * and {@code opencode.json} unconditionally — it runs the same for every worktree regardless of - * which backend ultimately spawns into it, because the backend is not yet chosen at {@link #add} - * time. A seeded {@code .claude/skills/} directory in an opencode member's worktree is simply - * never read by that launcher. + *

Kind-blind by construction, not by a backend check here — this used to be a real gap + * (fleetd #393). 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; diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java index f0d700a..755e350 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java @@ -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 prepend(String head, String... rest) { + List 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 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 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 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 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 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 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:}. */