From 20c1094cbfccc14af232eaa0fea19a894501ab4a Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 10 Sep 2026 17:36:22 +0700 Subject: [PATCH] fleetd #449: say what the timing fix actually proved, not what it assumed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The polling fix that landed in #452 is right, but its javadoc named a mechanism nobody measured: that input typed before the shell's prompt was swallowed by the shell's own startup. I mutated the settle poll away — SHELL_READY_TIMEOUT_MS = 0, so input is typed at once with no wait — and the test passed 3 of 3. So waitForText is the load-bearing half, and the proven cause is the old 800ms READ deadline, not the 1000ms write delay. The direction is the point: typing at 0ms works where typing at 1000ms failed. If early input were swallowed, 0ms would be worse than 1000ms. It is better, so the swallow explanation is unsupported. waitUntilSettled stays as cheap insurance, now labelled as insurance rather than as the fix. Comment-only; AgentControlContractTest still green. --- .../fleet/herdr/AgentControlContractTest.java | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/fleetd/src/test/java/dev/ltms/fleet/herdr/AgentControlContractTest.java b/fleetd/src/test/java/dev/ltms/fleet/herdr/AgentControlContractTest.java index 333efe0..a480f14 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/herdr/AgentControlContractTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/herdr/AgentControlContractTest.java @@ -18,12 +18,24 @@ import static org.junit.jupiter.api.Assumptions.assumeTrue; * the throwaway space down. * *

The seed shell's own startup (restoring its session, printing its banner) is asynchronous - * and its length is not a fleetd contract — measured here at ~2.5s on one host (fleetd #449). A - * fixed sleep before typing raced that startup: input typed before the shell reached its prompt - * was swallowed by the shell's own startup, and the pane showed the typed line followed by the - * startup banner with no command output at all — indistinguishable, at a glance, from the env - * map never reaching the shell. So this polls for a real signal (the pane's visible text - * settling, then the expected output appearing) instead of guessing a sleep length. + * and its length is not a fleetd contract — measured here at ~2.5s on one host (fleetd #449). + * The old version used two fixed sleeps: 1000ms before typing, then 800ms before reading. It + * failed, and the pane showed the typed line followed by the startup banner with no command + * output at all — which looks, at a glance, exactly like the env map never reaching the shell. + * So this polls for a real signal instead of guessing a sleep length. + * + *

What was measured, and what was not. Polling fixes it: 5 standalone runs + * green. The load-bearing half is {@link #waitForText}. With {@link #SHELL_READY_TIMEOUT_MS} + * set to 0 — so input is typed at once, with no settle wait at all — the test still passed 3 of + * 3. So the proven cause is the 800ms READ deadline being too short, not the 1000ms write delay. + * Note the direction, because it matters: typing at 0ms works where typing at 1000ms failed. The + * earlier explanation for this test — that input typed before the prompt is swallowed by the + * shell's startup — is therefore NOT supported by any measurement here. Please do not repeat it + * as the reason; if it were true, 0ms would be worse than 1000ms, and it is better. + * + *

{@link #waitUntilSettled} is kept as cheap insurance against that swallow case, not because + * anyone showed it was needed. If you want to delete it, the honest test is whether you can make + * this test fail by typing early. Nobody has managed that yet. * *

Tagged {@code contract}; run with {@code mvn test -Pcontract}. */ @@ -48,8 +60,9 @@ class AgentControlContractTest { /** * Poll {@code pane.read} until two consecutive reads come back identical — the shell's own * startup output (restore banner, prompt) has stopped changing — or {@code timeoutMs} elapses. - * Never asserts by itself; the caller's own assertion is what actually verifies the outcome, - * this only avoids sending input into a shell still mid-startup. + * Never asserts by itself; the caller's own assertion is what actually verifies the outcome. + * Setting {@code timeoutMs} to 0 skips the wait entirely and the test still passes here, so + * treat this as insurance rather than as the fix — see the class javadoc. */ private static String waitUntilSettled(UnixSocketHerdrClient herdr, String paneId, long timeoutMs) throws InterruptedException {