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 893e7f4..bb13846 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java @@ -525,17 +525,28 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { charter.toFile().deleteOnExit(); // fleetd #393 follow-up: withArray, not putArray. putArray REPLACES whatever node - // is already at "instructions" — harmless only as long as this block runs first - // against a still-empty root, which is an ordering constraint nothing declared or - // tested. The skills writer just below, and the IDE-rules writer further down, - // both already use withArray (get-or-create) for exactly this reason; this was the - // one straggler. Proven load-bearing on the fleetd #393 merge: flipping this one - // call back to putArray left the whole suite green while silently deleting the - // charter entry whenever skills or IDE rules ran after it — an opencode member - // would launch with no role contract at all, worse than the bug #393 fixed, and - // nothing caught it. See OpenCodeLauncherTest's - // instructionsArrayHoldsCharterThenIdeRulesInOrder and - // instructionsArrayHoldsCharterThenSkillsThenIdeRulesInOrder. + // is already at "instructions"; withArray gets-or-creates. All three writers on + // this array now use withArray so that write order stops being load-bearing for + // whoever adds a fourth. + // + // Be precise about what this particular line is worth, because an earlier version + // of this comment was wrong and claimed too much. THIS call is the one place where + // the two idioms are equivalent, and no test can tell them apart: it runs first, + // against a still-empty root, so there is never an existing node for putArray to + // replace. Measured on the #393 merge: flipping this one call back to putArray + // leaves the whole suite green (1618 tests), and always will. The edit is a + // readability and future-proofing change with no test behind it, and that is not a + // gap anyone can close. + // + // The ordering hazard is real, just not here. It is the LATER writers that can + // destroy earlier entries. Measured on the same merge: flipping the skills writer + // below to putArray deletes this charter entry and fails + // OpenCodeLauncherTest.instructionsArrayHoldsCharterThenSkillsThenIdeRulesInOrder; + // flipping the IDE-rules writer fails that test and + // instructionsArrayHoldsCharterThenIdeRulesInOrder. Deleting this line altogether + // fails instructionsArrayHoldsExactlyTheCharterWhenNothingElseWritesToIt — the + // assertion that a role contract reaches an opencode member at all, which is the + // hole that predates #393 and is what actually let the mutation hide. root.withArray("instructions").add(charter.toAbsolutePath().toString()); }