fleetd #449: say what the timing fix actually proved, not what it assumed
CI / contract (push) Successful in 1m22s
CI / build (push) Successful in 1m33s

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.
This commit is contained in:
Dai Ha
2026-09-10 17:36:22 +07:00
parent 9011c59b9f
commit 20c1094cbf
@@ -18,12 +18,24 @@ import static org.junit.jupiter.api.Assumptions.assumeTrue;
* the throwaway space down.
*
* <p>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.
*
* <p><strong>What was measured, and what was not.</strong> 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.
*
* <p>{@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.
*
* <p>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 {