fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fleetd #393: deliver memberSkills to opencode members, and stop overclaiming seeding success #471

Closed
agent wants to merge 0 commits from worker/393-opencode-skill-seeding-71854b-13 into main
pull from: worker/393-opencode-skill-seeding-71854b-13
merge into: fleet:main
Conversation 2 Commits - Files Changed -
agent commented 2026-09-10 14:37:28 +02:00
Member
Copy Link
Copy Source

Closes #393.

What changed

GitWorktrees.seedSkills copies memberSkills:-seeded skill folders into every provisioned worktree's .claude/skills/ and logged skill seeding: N of M as if that were success. .claude/skills/ is a Claude Code CLI convention; opencode has no such discovery, so a kind: opencode member never actually read a seeded skill even though the log said N of M succeeded.

Two changes, both required per the ticket:

  1. Deliver it. OpenCodeLauncher.skillInstructionFiles scans <cwd>/.claude/skills/*/SKILL.md at spawn time (the one point the launcher knows both the kind and the cwd) and appends each to the generated opencode.json's instructions[] array, the same channel already used for the member charter and IDE rules. A skill folder with no SKILL.md is named and skipped rather than silently dropped. The config-file gate in buildLaunch is updated so a seeded skill alone (no MCP, no charter, no custom provider) is enough to trigger OPENCODE_CONFIG.
  2. Stop claiming it where the claim can't be verified. GitWorktrees.seedSkills's log now says explicitly that consumption depends on the member's kind and points at the launcher's own log. OpenCodeLauncher logs its own kind-aware skill delivery: M of N ... line once the kind is actually known, naming any folder it could not turn into an instructions[] entry.

fleetd.example.yaml's memberSkills: doc previously claimed "Claude Code members only; an opencode member reads a different path (.opencode/agent) this key does not touch" — false as of this fix (the ticket asked me to check for and fix exactly this). Corrected to name both kinds and how each consumes it.

ClaudeCodeLauncher is untouched — its native .claude/skills/ discovery already worked and is explicitly out of scope per the ticket.

Tests

OpenCodeLauncherTest gains two cases, both driving the real GitWorktrees#add seeding path (not a hand-built .claude/skills/ fixture) into an opencode-kind spawn:

  • aSeededSkillReachesTheOpencodeMembersInstructionsArray — a seeded skill's SKILL.md lands in instructions[], plus the honest skill delivery: 1 of 1 log line.
  • aSkillFolderWithoutSkillMdIsNeverDeliveredAndTheLogNamesIt — a well-formed skill is still delivered alongside a malformed one; the malformed one is named in the log and excluded from instructions[].

Break-and-restore

Delivery half — commented out the instructions[].add(...) loop in writeConfig. Both new tests failed:

  • aSeededSkillReachesTheOpencodeMembersInstructionsArray: the seeded skill's SKILL.md must be an instructions[] entry — got: [] ==> expected: <true> but was: <false>
  • aSkillFolderWithoutSkillMdIsNeverDeliveredAndTheLogNamesIt: the well-formed skill is still delivered alongside the malformed one ==> expected: <true> but was: <false>

Restored, re-ran, green.

Logging half — commented out the log.info("skill delivery: ...") call in skillInstructionFiles. Both new tests failed:

  • aSeededSkillReachesTheOpencodeMembersInstructionsArray: the launcher must log, kind-aware, that it delivered the skill — got: [] ==> expected: <true> but was: <false>
  • aSkillFolderWithoutSkillMdIsNeverDeliveredAndTheLogNamesIt: the log must say plainly which folder could not be consumed and why — got: [] ==> expected: <true> but was: <false>

Restored, re-ran, green.

Full suite

mvn -B clean test
...
Tests run: 1603, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
exit=0

Beyond scope (not fixed, reporting only — the lead asked for this list)

A read-only sweep of src/main/java for the same shape (a log claims success for work whose downstream consumer may not actually be able to use it), most confident first:

  1. GitWorktrees.java:1005 — "parity overlay: copied {} of {} candidates" — copies operator-configured files (default .env) into every worktree regardless of member kind or whether anything downstream reads them; weaker than #393 though, since .env/.envrc are plain filesystem config any shell/tool reads uniformly, not gated by member kind the way .claude/skills/ is.
  2. HerdrPeerLauncher.java:1454 — "memberCredentials policy=allow-list: profile={} generated ZDOTDIR {}" — logs the ZDOTDIR scrub as generated/applied without checking the member's actual login shell is zsh; a non-zsh member's .zlogin-based scrub silently never runs.
  3. HerdrPeerLauncher.java:1578/1668 — "member credentials: allowed {} of {}" — counts names allowed into the pane env; doesn't distinguish whether the member's actual runtime (opencode vs claude-code) even reads a given var, so "allowed" isn't the same claim as "used."

Already-honest examples checked, not new instances: HerdrPeerLauncher.java:317 ("context reset is unsupported for peer kind {}") and HerdrPeerLauncher.java:1892 (memberCredentials gap warning) both explicitly name the limiting condition rather than overclaiming.

Caveats for review

  • The delivery mechanism is genuinely different in kind, not just channel: opencode's instructions[] is static system-prompt text present from spawn, not an invokable Skill-tool resource the way Claude Code's .claude/skills/ is. The content reaches both kinds; the mechanism by which a member acts on it differs, and that's inherent to opencode's design, not something this PR can close further.
  • I did not touch ClaudeCodeLauncher per the ticket's explicit boundary.
  • fleetd/fleetd.yaml is gitignored and not in my worktree; I did not touch it or attempt to reproduce fleet01's live shape beyond what the tests need.

Follow-up: instructions[] writer-ordering hazard (fixed in this PR)

After the delivery/logging halves above were accepted, the fleet01 lead found — and my lead
independently verified on this branch's merge — a separate, previously-undetected hazard in
OpenCodeLauncher.writeConfig: the instructions[] array has three writers (charter, seeded
skills, IDE rules). The charter writer used ObjectNode.putArray (create-or-REPLACE) instead
of withArray (get-or-create), which only "worked" because it happened to run first, against a
still-empty array — an undeclared ordering dependency nothing tested.

Proof it was live-load-bearing: switching the skills writer from withArray to putArray
left the entire 1603-test suite green while silently deleting the charter entry — an opencode
member would launch with no role contract at all, worse than the bug this ticket fixed.

Fix (this commit): the charter writer's putArray -> withArray — a one-word production
change, behavior-identical today. Added three tests in OpenCodeLauncherTest that assert
instructions[] content as an exact ordered list (not size — a putArray mutation can
replace N entries with a different N, so a size check cannot tell them apart):

  • instructionsArrayHoldsExactlyTheCharterWhenNothingElseWritesToIt — charter only
  • instructionsArrayHoldsCharterThenIdeRulesInOrder — charter + IDE rules
  • instructionsArrayHoldsCharterThenSkillsThenIdeRulesInOrder — charter + IDE rules + seeded
    skills (the realistic shape on a host where weighted placement makes opencode the default for
    most members, per fleet01)

Mutation testing, each writer flipped to putArray individually and restored after:

  • charter writer (the one this commit fixes): not independently detectable by any test — it
    structurally always runs first in writeConfig, against an empty array, so putArray and
    withArray are equivalent there. Full suite stayed at 59/59 green in
    OpenCodeLauncherTest under this mutation. This is not a gap in the tests; it is the same fact
    that makes the fix itself "behavior-identical today."
  • skills writer: fails instructionsArrayHoldsCharterThenSkillsThenIdeRulesInOrder by name (1
    failure, 58 other OpenCodeLauncherTest tests still pass).
  • IDE-rules writer: fails two tests by name —
    instructionsArrayHoldsCharterThenIdeRulesInOrder and
    instructionsArrayHoldsCharterThenSkillsThenIdeRulesInOrder (2 failures, 57 others pass).

Full suite with the fix restored: Tests run: 1606, Failures: 0, Errors: 0, Skipped: 0,
BUILD SUCCESS, exit code 0.

What I did not verify: what opencode actually does at runtime with N instructions[] entries
(order of concatenation, separators, whether a later entry can override an earlier one). I have
not run a live opencode session against a multi-entry config and have no observed behavior to
report here — this stays an open question rather than a guess.

Credit: the writer-ordering hazard itself was found by the fleet01 lead and independently
verified by my lead on this branch's merge, not by me. My original delivery and logging work
(above) held up under that verification unmodified; this section is a distinct hazard neither of
those tests could see, because none of them combined all three writers in one config.

Closes #393. ## What changed `GitWorktrees.seedSkills` copies `memberSkills:`-seeded skill folders into every provisioned worktree's `.claude/skills/` and logged `skill seeding: N of M` as if that were success. `.claude/skills/` is a Claude Code CLI convention; opencode has no such discovery, so a `kind: opencode` member never actually read a seeded skill even though the log said N of M succeeded. Two changes, both required per the ticket: 1. **Deliver it.** `OpenCodeLauncher.skillInstructionFiles` scans `<cwd>/.claude/skills/*/SKILL.md` at spawn time (the one point the launcher knows both the kind and the cwd) and appends each to the generated `opencode.json`'s `instructions[]` array, the same channel already used for the member charter and IDE rules. A skill folder with no `SKILL.md` is named and skipped rather than silently dropped. The config-file gate in `buildLaunch` is updated so a seeded skill alone (no MCP, no charter, no custom provider) is enough to trigger `OPENCODE_CONFIG`. 2. **Stop claiming it where the claim can't be verified.** `GitWorktrees.seedSkills`'s log now says explicitly that consumption depends on the member's kind and points at the launcher's own log. `OpenCodeLauncher` logs its own kind-aware `skill delivery: M of N ...` line once the kind is actually known, naming any folder it could not turn into an `instructions[]` entry. `fleetd.example.yaml`'s `memberSkills:` doc previously claimed "Claude Code members only; an opencode member reads a different path (`.opencode/agent`) this key does not touch" — false as of this fix (the ticket asked me to check for and fix exactly this). Corrected to name both kinds and how each consumes it. `ClaudeCodeLauncher` is untouched — its native `.claude/skills/` discovery already worked and is explicitly out of scope per the ticket. ## Tests `OpenCodeLauncherTest` gains two cases, both driving the *real* `GitWorktrees#add` seeding path (not a hand-built `.claude/skills/` fixture) into an opencode-kind spawn: - `aSeededSkillReachesTheOpencodeMembersInstructionsArray` — a seeded skill's `SKILL.md` lands in `instructions[]`, plus the honest `skill delivery: 1 of 1` log line. - `aSkillFolderWithoutSkillMdIsNeverDeliveredAndTheLogNamesIt` — a well-formed skill is still delivered alongside a malformed one; the malformed one is named in the log and excluded from `instructions[]`. ### Break-and-restore **Delivery half** — commented out the `instructions[].add(...)` loop in `writeConfig`. Both new tests failed: - `aSeededSkillReachesTheOpencodeMembersInstructionsArray`: `the seeded skill's SKILL.md must be an instructions[] entry — got: [] ==> expected: <true> but was: <false>` - `aSkillFolderWithoutSkillMdIsNeverDeliveredAndTheLogNamesIt`: `the well-formed skill is still delivered alongside the malformed one ==> expected: <true> but was: <false>` Restored, re-ran, green. **Logging half** — commented out the `log.info("skill delivery: ...")` call in `skillInstructionFiles`. Both new tests failed: - `aSeededSkillReachesTheOpencodeMembersInstructionsArray`: `the launcher must log, kind-aware, that it delivered the skill — got: [] ==> expected: <true> but was: <false>` - `aSkillFolderWithoutSkillMdIsNeverDeliveredAndTheLogNamesIt`: `the log must say plainly which folder could not be consumed and why — got: [] ==> expected: <true> but was: <false>` Restored, re-ran, green. ### Full suite ``` mvn -B clean test ... Tests run: 1603, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS exit=0 ``` ## Beyond scope (not fixed, reporting only — the lead asked for this list) A read-only sweep of `src/main/java` for the same shape (a log claims success for work whose downstream consumer may not actually be able to use it), most confident first: 1. `GitWorktrees.java:1005` — `"parity overlay: copied {} of {} candidates"` — copies operator-configured files (default `.env`) into every worktree regardless of member kind or whether anything downstream reads them; weaker than #393 though, since `.env`/`.envrc` are plain filesystem config any shell/tool reads uniformly, not gated by member kind the way `.claude/skills/` is. 2. `HerdrPeerLauncher.java:1454` — `"memberCredentials policy=allow-list: profile={} generated ZDOTDIR {}"` — logs the ZDOTDIR scrub as generated/applied without checking the member's actual login shell is zsh; a non-zsh member's `.zlogin`-based scrub silently never runs. 3. `HerdrPeerLauncher.java:1578`/`1668` — `"member credentials: allowed {} of {}"` — counts names allowed into the pane env; doesn't distinguish whether the member's actual runtime (opencode vs claude-code) even reads a given var, so "allowed" isn't the same claim as "used." Already-honest examples checked, not new instances: `HerdrPeerLauncher.java:317` (`"context reset is unsupported for peer kind {}"`) and `HerdrPeerLauncher.java:1892` (memberCredentials gap warning) both explicitly name the limiting condition rather than overclaiming. ## Caveats for review - The delivery mechanism is genuinely different in kind, not just channel: opencode's `instructions[]` is static system-prompt text present from spawn, not an invokable Skill-tool resource the way Claude Code's `.claude/skills/` is. The *content* reaches both kinds; the *mechanism* by which a member acts on it differs, and that's inherent to opencode's design, not something this PR can close further. - I did not touch `ClaudeCodeLauncher` per the ticket's explicit boundary. - `fleetd/fleetd.yaml` is gitignored and not in my worktree; I did not touch it or attempt to reproduce fleet01's live shape beyond what the tests need. --- ## Follow-up: instructions[] writer-ordering hazard (fixed in this PR) After the delivery/logging halves above were accepted, the fleet01 lead found — and my lead independently verified on this branch's merge — a separate, previously-undetected hazard in `OpenCodeLauncher.writeConfig`: the `instructions[]` array has three writers (charter, seeded skills, IDE rules). The charter writer used `ObjectNode.putArray` (create-or-**REPLACE**) instead of `withArray` (get-or-create), which only "worked" because it happened to run first, against a still-empty array — an undeclared ordering dependency nothing tested. Proof it was live-load-bearing: switching the **skills** writer from `withArray` to `putArray` left the entire 1603-test suite green while silently deleting the charter entry — an opencode member would launch with **no role contract at all**, worse than the bug this ticket fixed. **Fix (this commit):** the charter writer's `putArray` -> `withArray` — a one-word production change, behavior-identical today. Added three tests in `OpenCodeLauncherTest` that assert `instructions[]` **content** as an exact ordered list (not size — a `putArray` mutation can replace N entries with a different N, so a size check cannot tell them apart): - `instructionsArrayHoldsExactlyTheCharterWhenNothingElseWritesToIt` — charter only - `instructionsArrayHoldsCharterThenIdeRulesInOrder` — charter + IDE rules - `instructionsArrayHoldsCharterThenSkillsThenIdeRulesInOrder` — charter + IDE rules + seeded skills (the realistic shape on a host where weighted placement makes opencode the default for most members, per fleet01) **Mutation testing, each writer flipped to `putArray` individually and restored after:** - charter writer (the one this commit fixes): **not independently detectable** by any test — it structurally always runs first in `writeConfig`, against an empty array, so `putArray` and `withArray` are equivalent there. Full suite stayed at 59/59 green in `OpenCodeLauncherTest` under this mutation. This is not a gap in the tests; it is the same fact that makes the fix itself "behavior-identical today." - skills writer: fails `instructionsArrayHoldsCharterThenSkillsThenIdeRulesInOrder` by name (1 failure, 58 other `OpenCodeLauncherTest` tests still pass). - IDE-rules writer: fails **two** tests by name — `instructionsArrayHoldsCharterThenIdeRulesInOrder` and `instructionsArrayHoldsCharterThenSkillsThenIdeRulesInOrder` (2 failures, 57 others pass). **Full suite with the fix restored:** `Tests run: 1606, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, exit code 0. **What I did not verify:** what opencode actually does at runtime with N `instructions[]` entries (order of concatenation, separators, whether a later entry can override an earlier one). I have not run a live opencode session against a multi-entry config and have no observed behavior to report here — this stays an open question rather than a guess. **Credit:** the writer-ordering hazard itself was found by the fleet01 lead and independently verified by my lead on this branch's merge, not by me. My original delivery and logging work (above) held up under that verification unmodified; this section is a distinct hazard neither of those tests could see, because none of them combined all three writers in one config.
agent added 1 commit 2026-09-10 14:37:29 +02:00
fleetd #393: deliver memberSkills to opencode members, and stop overclaiming seeding success
CI / contract (pull_request) Successful in 49s
Details
CI / build (pull_request) Successful in 2m46s
Details
d4a2cd720c 
GitWorktrees.seedSkills copies memberSkills:-seeded skill folders into every
provisioned worktree's .claude/skills/ and logged "skill seeding: N of M" as
if that were success — but .claude/skills/ is a Claude Code CLI convention.
opencode has no such discovery, so a kind: opencode member never actually
read a seeded skill even though the log said N of M succeeded.

Two changes, both required:

1. Deliver it. OpenCodeLauncher.skillInstructionFiles scans
   <cwd>/.claude/skills/*/SKILL.md at spawn time (the one point the launcher
   knows both the kind and the cwd) and appends each to the generated
   opencode.json's instructions[] array, the same channel already used for
   the member charter and IDE rules. A skill folder with no SKILL.md is
   named and skipped rather than silently dropped.

2. Stop claiming it where the claim can't be verified. GitWorktrees.seedSkills'
   log now says explicitly that consumption depends on the member's kind and
   points at the launcher's own log; OpenCodeLauncher logs its own kind-aware
   "skill delivery: M of N ..." line once the kind is actually known, naming
   any folder it could not turn into an instructions[] entry.

fleetd.example.yaml's memberSkills: doc previously claimed "Claude Code
members only; an opencode member reads a different path (.opencode/agent)
this key does not touch" — false as of this fix, corrected to name both
kinds and how each consumes it.

Tests: OpenCodeLauncherTest gains two cases driving the real
GitWorktrees#add seeding path (not a hand-built fixture) into an
opencode-kind spawn — one asserting a seeded skill's SKILL.md lands in
instructions[] plus the honest log line, one covering a skill folder
without SKILL.md (delivered skills still flow, the malformed one is named
in the log and excluded from instructions[]). ClaudeCodeLauncher is
untouched — its native .claude/skills/ discovery already worked and is out
of scope.

mvn -B clean test: Tests run: 1603, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS
ltms commented 2026-09-10 14:49:38 +02:00
Owner
Copy Link
Copy Source

Adjudicated. The change is accepted and I have asked for one small follow-up on this branch before I merge.

Battery run on the merge commit (56d2890, tree 917ae82), not on the branch. Control, unmutated: Tests run: 1603, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS, rc=0, 0 compile-error blocks.

What holds

Both halves of the change are genuinely pinned — I re-ran the PR's break-and-restore proofs independently rather than taking them, and they reproduce. The delivery half and the kind-aware logging half each fail by name when broken.

The test design is the right call and worth naming: both new tests drive the real GitWorktrees#add seeding path instead of a hand-built .claude/skills/ fixture. So they prove OpenCodeLauncher reads what GitWorktrees actually produces, not what the author imagined it produces. That is the difference between a test of the seam and a test of the pipeline, and it was not asked for in my brief.

The PR is also honest about the limit that matters most: opencode's instructions[] is static system-prompt text present from spawn, not an invokable skill resource the way Claude Code's .claude/skills/ is. The content now reaches both kinds; how a member can act on it still differs. Saying that plainly is better than the change would have been with a claim of parity.

What does not hold — an ordering constraint nothing declares and nothing tests

This is context the worker never had. The fleet01 lead sent it to me after the brief went out; I verified their mechanism in my own clone, then measured the consequence here.

There are now three writers to instructions[], and they are not the same operation:

527:  root.putArray("instructions").add(charter...)      // CREATES-OR-REPLACES
536:  root.withArray("instructions").add(skillFile...)   // gets-or-creates  <- this PR
565:  root.withArray("instructions").add(rules...)       // gets-or-creates

putArray replaces the node; withArray gets-or-creates it. The charter at :527 gets away with putArray only because it runs first, while the array is still empty. This PR used withArray and placed it after the charter, which is correct. The hazard is that nothing holds that arrangement in place.

Two cells, both aimed at the ordering rather than at this PR's own code:

Cell Mutation Result
M1 the skills writer switches to putArray — destroys the charter entry above it SURVIVED — 1603 green, rc=0
M2 the IDE-rules writer switches to putArray — it runs last, so it destroys charter and skills SURVIVED — 1603 green, rc=0

Both directions unpinned. In M1's state an opencode member launches with no role contract at all, and the full suite passes.

That is a worse failure than the bug this ticket fixed, and it is reachable by a one-word edit from anyone who adds a fourth writer without reading :478's comment. The count of writers went from two to three here, so the odds of a fourth are now higher than they were.

Why the existing tests cannot catch it: both new tests are skills-focused, and they pass under either idiom. The defect needs two or more writers active at once, so member kind is the wrong axis for it — a kind-parameterised test with only a charter present passes whichever operation the code uses.

The follow-up I asked for

Sent to the same worker, to push to this branch:

  1. Convert :527 to withArray. Behaviour is identical today, and afterwards the order of the three writers stops mattering. The whole value is deleting an unwritten rule.
  2. A test that parameterises the combination of writers — charter only, charter+ide, charter+ide+skills — asserting array contents, not size. A size assertion passes when putArray swaps two entries for two different ones, which is precisely this failure mode.

Both the fix and the test shape are fleet01's recommendation, passed on as theirs. The three-writer cell is the realistic configuration on their host, where weighted placement makes opencode the default for essentially every member.

Also accepted

The sweep for other kind-blind success logs is useful and correctly ranked, including the two cases checked and reported as not instances (HerdrPeerLauncher:317 and :1892 already name their limiting condition). Reporting a non-finding is worth as much as reporting a finding here. Those three candidates are noted and not fixed in this PR, which is the right boundary.

The fleetd.example.yaml correction matters more than its size suggests: the old text actively told the reader opencode reads .opencode/agent and that this key does not touch it. A false statement in the example config is worse than an omission, because it is what an operator reads while deciding.

Note on these numbers

They describe tree 917ae82. The follow-up will change the tree, so I will re-measure on the final merge before pushing rather than carrying these forward.

Adjudicated. The change is accepted and I have asked for one small follow-up on this branch before I merge. Battery run on the **merge commit** (`56d2890`, tree `917ae82`), not on the branch. Control, unmutated: `Tests run: 1603, Failures: 0, Errors: 0, Skipped: 0` — `BUILD SUCCESS`, rc=0, 0 compile-error blocks. ## What holds Both halves of the change are genuinely pinned — I re-ran the PR's break-and-restore proofs independently rather than taking them, and they reproduce. The delivery half and the kind-aware logging half each fail by name when broken. The test design is the right call and worth naming: both new tests drive the **real** `GitWorktrees#add` seeding path instead of a hand-built `.claude/skills/` fixture. So they prove `OpenCodeLauncher` reads what `GitWorktrees` actually produces, not what the author imagined it produces. That is the difference between a test of the seam and a test of the pipeline, and it was not asked for in my brief. The PR is also honest about the limit that matters most: opencode's `instructions[]` is static system-prompt text present from spawn, not an invokable skill resource the way Claude Code's `.claude/skills/` is. The content now reaches both kinds; how a member can act on it still differs. Saying that plainly is better than the change would have been with a claim of parity. ## What does not hold — an ordering constraint nothing declares and nothing tests This is context the worker never had. The fleet01 lead sent it to me after the brief went out; I verified their mechanism in my own clone, then measured the consequence here. There are now **three** writers to `instructions[]`, and they are not the same operation: ``` 527: root.putArray("instructions").add(charter...) // CREATES-OR-REPLACES 536: root.withArray("instructions").add(skillFile...) // gets-or-creates <- this PR 565: root.withArray("instructions").add(rules...) // gets-or-creates ``` `putArray` replaces the node; `withArray` gets-or-creates it. The charter at :527 gets away with `putArray` only because it runs first, while the array is still empty. **This PR used `withArray` and placed it after the charter, which is correct.** The hazard is that nothing holds that arrangement in place. Two cells, both aimed at the ordering rather than at this PR's own code: | Cell | Mutation | Result | |---|---|---| | M1 | the skills writer switches to `putArray` — destroys the charter entry above it | **SURVIVED** — 1603 green, rc=0 | | M2 | the IDE-rules writer switches to `putArray` — it runs last, so it destroys charter **and** skills | **SURVIVED** — 1603 green, rc=0 | Both directions unpinned. In M1's state an opencode member launches with **no role contract at all**, and the full suite passes. That is a worse failure than the bug this ticket fixed, and it is reachable by a one-word edit from anyone who adds a fourth writer without reading :478's comment. The count of writers went from two to three here, so the odds of a fourth are now higher than they were. Why the existing tests cannot catch it: both new tests are skills-focused, and they pass under either idiom. The defect needs two or more writers active at once, so member kind is the wrong axis for it — a kind-parameterised test with only a charter present passes whichever operation the code uses. ## The follow-up I asked for Sent to the same worker, to push to this branch: 1. Convert :527 to `withArray`. Behaviour is identical today, and afterwards the order of the three writers stops mattering. The whole value is deleting an unwritten rule. 2. A test that parameterises the **combination** of writers — charter only, charter+ide, charter+ide+skills — asserting array **contents**, not size. A size assertion passes when `putArray` swaps two entries for two different ones, which is precisely this failure mode. Both the fix and the test shape are fleet01's recommendation, passed on as theirs. The three-writer cell is the realistic configuration on their host, where weighted placement makes opencode the default for essentially every member. ## Also accepted The sweep for other kind-blind success logs is useful and correctly ranked, including the two cases checked and reported as **not** instances (`HerdrPeerLauncher:317` and `:1892` already name their limiting condition). Reporting a non-finding is worth as much as reporting a finding here. Those three candidates are noted and not fixed in this PR, which is the right boundary. The `fleetd.example.yaml` correction matters more than its size suggests: the old text actively told the reader opencode reads `.opencode/agent` and that this key does not touch it. A false statement in the example config is worse than an omission, because it is what an operator reads while deciding. ## Note on these numbers They describe tree `917ae82`. The follow-up will change the tree, so I will re-measure on the final merge before pushing rather than carrying these forward.
agent added 1 commit 2026-09-10 14:58:28 +02:00
fleetd #393 follow-up: remove the instructions[] writer-ordering hazard
CI / contract (pull_request) Successful in 1m22s
Details
CI / build (pull_request) Successful in 2m11s
Details
9e4e423ad6 
OpenCodeLauncher.writeConfig has three writers into the instructions[]
array (charter, seeded skills, IDE rules). The charter writer used
putArray (create-or-REPLACE) instead of withArray (get-or-create), which
"worked" only because it happened to run first against a still-empty
array — an undeclared ordering dependency nothing tested. Found by the
fleet01 lead and verified on this branch's merge: flipping the skills
writer to putArray left the full 1603-test suite green while silently
deleting the charter entry, which would launch an opencode member with
no role contract at all.

Fix: charter's putArray -> withArray (one-word change, behavior-identical
today). Add three tests asserting instructions[] CONTENT as an exact
ordered list (not size) across writer combinations: charter only,
charter + IDE rules, and charter + IDE rules + seeded skills. Mutation
testing (see PR body) shows the skills and IDE-rules writers are each
independently detectable by name; the charter writer's own mutation is
not detectable by any test, because it structurally always runs first
against an empty array, so putArray and withArray are equivalent there.
agent commented 2026-09-10 15:01:54 +02:00
Author
Member
Copy Link
Copy Source

Re-posting the beyond-scope research as a standalone comment for visibility (this was already researched directly — no fork was delegated for it — and is part of the PR description above; surfacing it here too since it was asked for as a comment):

Beyond scope (not fixed, reporting only — the lead asked for this list)

A read-only sweep of src/main/java for the same shape (a log claims success for work whose downstream consumer may not actually be able to use it), most confident first:

  1. GitWorktrees.java:1005 — "parity overlay: copied {} of {} candidates" — copies operator-configured files (default .env) into every worktree regardless of member kind or whether anything downstream reads them; weaker than #393 though, since .env/.envrc are plain filesystem config any shell/tool reads uniformly, not gated by member kind the way .claude/skills/ is.
  2. HerdrPeerLauncher.java:1454 — "memberCredentials policy=allow-list: profile={} generated ZDOTDIR {}" — logs the ZDOTDIR scrub as generated/applied without checking the member's actual login shell is zsh; a non-zsh member's .zlogin-based scrub silently never runs.
  3. HerdrPeerLauncher.java:1578/1668 — "member credentials: allowed {} of {}" — counts names allowed into the pane env; doesn't distinguish whether the member's actual runtime (opencode vs claude-code) even reads a given var, so "allowed" isn't the same claim as "used."

Already-honest examples checked, not new instances: HerdrPeerLauncher.java:317 ("context reset is unsupported for peer kind {}") and HerdrPeerLauncher.java:1892 (memberCredentials gap warning) both explicitly name the limiting condition rather than overclaiming.

Re-posting the beyond-scope research as a standalone comment for visibility (this was already researched directly — no fork was delegated for it — and is part of the PR description above; surfacing it here too since it was asked for as a comment): ## Beyond scope (not fixed, reporting only — the lead asked for this list) A read-only sweep of `src/main/java` for the same shape (a log claims success for work whose downstream consumer may not actually be able to use it), most confident first: 1. `GitWorktrees.java:1005` — `"parity overlay: copied {} of {} candidates"` — copies operator-configured files (default `.env`) into every worktree regardless of member kind or whether anything downstream reads them; weaker than #393 though, since `.env`/`.envrc` are plain filesystem config any shell/tool reads uniformly, not gated by member kind the way `.claude/skills/` is. 2. `HerdrPeerLauncher.java:1454` — `"memberCredentials policy=allow-list: profile={} generated ZDOTDIR {}"` — logs the ZDOTDIR scrub as generated/applied without checking the member's actual login shell is zsh; a non-zsh member's `.zlogin`-based scrub silently never runs. 3. `HerdrPeerLauncher.java:1578`/`1668` — `"member credentials: allowed {} of {}"` — counts names allowed into the pane env; doesn't distinguish whether the member's actual runtime (opencode vs claude-code) even reads a given var, so "allowed" isn't the same claim as "used." Already-honest examples checked, not new instances: `HerdrPeerLauncher.java:317` (`"context reset is unsupported for peer kind {}"`) and `HerdrPeerLauncher.java:1892` (memberCredentials gap warning) both explicitly name the limiting condition rather than overclaiming.
ltms closed this pull request 2026-09-10 15:19:03 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 1m22s
Details
CI / build (pull_request) Successful in 2m11s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#471
Delete Branch "worker/393-opencode-skill-seeding-71854b-13"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?