The binding rule (fleetd #424): config governs what may be bound next; it never - * retroactively unbinds a live session. A slot removed from config while a terminal is - * bound to it keeps that binding — the architect keeps working and keeps its identity — but no new - * spawn can bind to that slot again, because {@link #slotsFor} (which {@link #reserve} and - * {@link #requireSlotFor} both read) stops offering it the moment it drops out of {@link #slots()}. - * To make an already-bound slot survive its own removal from {@link #slots()}, every successful - * {@link #bind} also caches the slot's {@link Entry} into {@link #boundEntries}; {@link #roleForSlot} - * and {@link #nameForSlot} — which {@link CallerResolver#resolve} calls on every request from a - * bound terminal — fall back to that cache when the slot is no longer live. {@link #unbind} clears - * the cache entry at the same time it clears the binding, so a slot that is genuinely free again - * (no live terminal) is never treated as "known" once it also drops out of config. + *
The binding rule (fleetd #424): config governs what a bound slot still grants, as + * well as what may be bound next. Removing a slot from config revokes it — that is the + * ticket's entire point ("Revoking an architect slot does not revoke it"). Revoking it means an + * architect already bound to that slot loses the ARCHITECT privilege on its very next request: + * {@link #roleForSlot} and {@link #nameForSlot} read {@link #slots()} directly, with no cache, so + * the moment a slot drops out of config, {@link CallerResolver#resolve} (which calls both on every + * request from a bound pane, {@code CallerResolver.java:220}) can no longer confirm the pane's slot + * is an architect slot, and the pane falls through to {@code Principal.worker(...)}. What does + * not change is the {@code terminalToSlot} occupancy — the binding created by + * {@link #bind} is untouched by a reload, on purpose: unbinding it here would double-book the slot + * key (a second terminal could then bind to the "freed" key while the first is still the terminal + * the operator actually meant to demote) and would silently break {@link #unbind}'s compare-safe + * contract, which needs the original {@code terminal → slot} pair intact to remove it cleanly. So + * the demoted session keeps occupying its slot — {@link #slotForTerminal} and {@link #snapshot()} + * still name it — it just no longer resolves as an architect through that occupancy, and a fresh + * spawn still cannot bind to the same key while it is occupied ({@link #reserve}/ + * {@link #requireSlotFor} refuse it anyway, since it is gone from {@link #slots()}). The demoted + * session's own turn is unaffected: {@code fleet_reply}'s authorization + * ({@code Authz.Action.REPLY}) is {@code caller.ownsSession(targetSession)} — identity by terminal, + * not by role — so a demoted architect can still end its own turn normally. * *
Spawning/lifecycle is deliberately a separate unit: this class only owns the bindings and
* exposes the map the resolver resolves against plus the profile lookup lifecycle will call.
@@ -79,16 +88,6 @@ public final class MemberRegistry implements MemberLifecycle {
private final Map Every test here drives a REAL {@link ConfigRef#reload()} against a {@code @TempDir} file and
* asserts {@link ConfigRef.Outcome#applied()}, rather than comparing two frozen
@@ -150,40 +155,103 @@ class MemberRegistryLiveTest {
"a slot added by reload must be reservable with no restart");
}
- // ── a bound architect survives its slot's removal by reload (criterion 3) ─────────────────
+ // ── a bound architect is demoted, but the binding itself is not touched (fleetd #424) ───────
+ // The lead's corrected ruling: the PRIVILEGE a slot grants is revoked on the bound session's
+ // very next request, but the terminalToSlot BINDING itself is untouched by a reload — dropping
+ // it would double-book the slot key and break unbind's compare-safe contract. See the class
+ // doc's binding rule.
+
+ /** A caller identity resolving the one canned pane (terminal {@code term_a}) in {@link FakeHerdr}. */
+ private static ConnectionIdentity boundPaneIdentity() {
+ return new ConnectionIdentity(new PaneLocator(new FakeHerdr()), _ -> FakeHerdr.WORKER_PID);
+ }
@Test
- void anArchitectAlreadyBoundToASlotSurvivesTheSlotsRemovalByReload(@TempDir Path dir) throws Exception {
+ void anArchitectAlreadyBoundToASlotIsDemotedByReload(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, yaml(WITH_SONNET_SLOT));
ConfigRef ref = refFor(f);
MemberRegistry registry = MemberRegistry.live(() -> ref.get().fleet());
MemberLifecycle.SlotReservation reservation = registry.reserve(MemberRole.ARCHITECT, "sonnet");
- assertTrue(registry.bind(reservation, "term_designer"));
- assertEquals("architect:designer", registry.slotForTerminal("term_designer"));
+ assertTrue(registry.bind(reservation, "term_a"));
+ assertEquals("architect:designer", registry.slotForTerminal("term_a"));
+
+ // Drive the real caller path, not the roleForSlot seam directly: CallerResolver.resolve is
+ // what a live request actually goes through (CallerResolver.java:220), and a resolver that
+ // ignored roleForSlot entirely would still pass a test that only checked the seam.
+ CallerResolver resolver = CallerResolver.withLeadsAndMembers(
+ boundPaneIdentity(), false, null, Map::of, registry);
+
+ Principal before = resolver.resolve("127.0.0.1", 42, null);
+ assertEquals(Role.ARCHITECT, before.role(), "sanity check: the harness binds term_a as an architect");
+ assertEquals("designer", before.name());
Files.writeString(f, yaml(WITHOUT_ARCHITECT_SLOTS));
ConfigRef.Outcome out = ref.reload();
assertTrue(out.applied(), "the reload must actually take effect: " + out.summary());
- // The binding itself must survive untouched — nothing here may unbind a live session.
- assertEquals("architect:designer", registry.slotForTerminal("term_designer"),
+ Principal after = resolver.resolve("127.0.0.1", 42, null);
+ assertEquals(Role.WORKER, after.role(),
+ "removing the slot from config must demote the bound session to worker on its "
+ + "NEXT request — this is the ticket's whole point");
+ assertEquals("term_a", after.terminal(), "same pane, same terminal — only the role changed");
+ }
+
+ @Test
+ void theOriginalBindingStillOccupiesTheRemovedSlotSoASecondTerminalCannotClaimIt(@TempDir Path dir)
+ throws Exception {
+ Path f = dir.resolve("fleetd.yaml");
+ Files.writeString(f, yaml(WITH_SONNET_SLOT));
+ ConfigRef ref = refFor(f);
+ MemberRegistry registry = MemberRegistry.live(() -> ref.get().fleet());
+
+ MemberLifecycle.SlotReservation reservation = registry.reserve(MemberRole.ARCHITECT, "sonnet");
+ assertTrue(registry.bind(reservation, "term_a"));
+
+ Files.writeString(f, yaml(WITHOUT_ARCHITECT_SLOTS));
+ ConfigRef.Outcome removed = ref.reload();
+ assertTrue(removed.applied(), "the reload must actually take effect: " + removed.summary());
+
+ // The binding survives the removal untouched.
+ assertEquals("architect:designer", registry.slotForTerminal("term_a"),
"a live binding must never be retroactively unbound by a config edit");
- assertEquals(Map.of("term_designer", "architect:designer"), registry.snapshot());
+ assertEquals(Map.of("term_a", "architect:designer"), registry.snapshot());
- // The identity CallerResolver.resolve actually reads off a bound pane must survive too —
- // roleForSlot/nameForSlot going null here is what would silently demote a live architect to
- // a worker the moment its slot is edited out of config.
- assertEquals(MemberRole.ARCHITECT, registry.roleForSlot("architect:designer"),
- "CallerResolver reads roleForSlot to confirm a bound pane is still an architect "
- + "slot — this must not go null just because config removed the slot");
- assertEquals("designer", registry.nameForSlot("architect:designer"));
+ // Bring the slot back into config. If the binding had been silently dropped by the removal
+ // (rather than merely losing the privilege it grants), a second terminal could now claim
+ // the "freed" key — the exact double-booking the class doc's binding rule rules out.
+ Files.writeString(f, yaml(WITH_SONNET_SLOT));
+ ConfigRef.Outcome restored = ref.reload();
+ assertTrue(restored.applied(), "the reload must actually take effect: " + restored.summary());
- // But the removed slot must grant nothing NEW to a different spawn.
+ assertFalse(registry.bind("architect:designer", "term_b"),
+ "the slot is still occupied by term_a — a second terminal must not bind to it");
assertThrows(IllegalArgumentException.class,
- () -> registry.requireSlotFor(MemberRole.ARCHITECT, "sonnet"));
- assertThrows(IllegalArgumentException.class,
- () -> registry.reserve(MemberRole.ARCHITECT, "sonnet"));
+ () -> registry.reserve(MemberRole.ARCHITECT, "sonnet"),
+ "the slot is still occupied by term_a — a fresh reservation must not find it free");
+ assertEquals("architect:designer", registry.slotForTerminal("term_a"),
+ "the original binding is unchanged throughout");
+ }
+
+ @Test
+ void unbindStillSucceedsForTheOriginalTerminalAfterItsSlotIsRemoved(@TempDir Path dir) throws Exception {
+ Path f = dir.resolve("fleetd.yaml");
+ Files.writeString(f, yaml(WITH_SONNET_SLOT));
+ ConfigRef ref = refFor(f);
+ MemberRegistry registry = MemberRegistry.live(() -> ref.get().fleet());
+
+ MemberLifecycle.SlotReservation reservation = registry.reserve(MemberRole.ARCHITECT, "sonnet");
+ assertTrue(registry.bind(reservation, "term_a"));
+
+ Files.writeString(f, yaml(WITHOUT_ARCHITECT_SLOTS));
+ ConfigRef.Outcome out = ref.reload();
+ assertTrue(out.applied(), "the reload must actually take effect: " + out.summary());
+
+ assertTrue(registry.unbind("architect:designer", "term_a"),
+ "unbind must still work for a slot that config has since removed, or a session "
+ + "that outlives its slot's removal could never release it");
+ assertNull(registry.slotForTerminal("term_a"));
+ assertEquals(Map.of(), registry.snapshot());
}
}