fix(scripts): disarm the first-paint deadline once the marker is seen (#2147)

Copilot review round 3.

`done()` is first-caller-wins, and the render deadline stayed armed after
first paint. So a marker landing near `timeoutMs` let that timer fire
*inside* the survival window and settle the run as

    child did not render "MCP Servers" within 15000ms

followed by an output tail that visibly contains "MCP Servers". A TUI that
is merely slow fails intermittently, with a diagnostic naming the one
thing that demonstrably did happen — the same wrong-attribution class this
PR exists to remove, and one that reads as flake.

First paint has happened; its deadline no longer has a question to ask, so
`onData` clears it at the moment the marker is observed.

Also hoists `renderTimer`'s declaration beside `surviveTimer`'s. It was a
`const` below both closures that clear it, safe only because `onData` and
`done` run from events rather than the synchronous body. That already held
for `done`; this fix adds a second reader, so the ordering is now stated
rather than relied upon.

The test paints at 250ms with timeoutMs 400 and surviveMs 400, so the
render deadline expires while the survival window is open — the exact
shape. Verified it fails against the module with the clearTimeout removed
and passes with it: a wider timeoutMs would have been green either way,
which is why the numbers are tight.

Signed-off-by: Cliff Hall <cliff@futurescale.com>
Signed-off-by: cliffhall <cliff@futurescale.com>

Author: cliffhall

scripts/lib/render-smoke.mjs

@@ -146,8 +146,15 @@ export function runRenderSmoke({
     let childExited = false;
     let childClosed = false;
     let markerAt = null;
-    /** @type {ReturnType<typeof setTimeout> | undefined} */
+    /**
+     * Both deadlines. Declared up front because `onData` and `done` each clear
+     * them from closures defined above the `setTimeout` calls that assign them.
+     *
+     * @type {ReturnType<typeof setTimeout> | undefined}
+     */
     let surviveTimer;
+    /** @type {ReturnType<typeof setTimeout> | undefined} */
+    let renderTimer;
 
     const t0 = Date.now();
     const since = () => Date.now() - t0;
@@ -233,6 +240,12 @@ export function runRenderSmoke({
       output += chunk.toString();
       if (markerAt !== null || !output.includes(marker)) return;
       markerAt = since();
+      // Disarm the first-paint deadline. It is first-caller-wins with `done`,
+      // so leaving it armed means a paint landing near `timeoutMs` lets it fire
+      // *during* the survival window and settle the run as "did not render
+      // <marker>" — quoting an output tail that visibly contains the marker.
+      // First paint has happened; its deadline no longer has a question to ask.
+      clearTimeout(renderTimer);
       // First paint is not the verdict — it only starts the clock on the
       // question this smoke actually asks.
       surviveTimer = setTimeout(() => {
@@ -269,7 +282,7 @@ export function runRenderSmoke({
       done(1, `failed to spawn ${command}: ${err.message}`);
     });
 
-    const renderTimer = setTimeout(() => {
+    renderTimer = setTimeout(() => {
       done(
         1,
         () =>

scripts/lib/render-smoke.test.mjs

@@ -57,6 +57,28 @@ test("passes when the child paints the marker and keeps running", async () => {
   assert.match(r.message, /still running 400ms later/);
 });
 
+// The first-paint deadline and the survival window overlap: `done` is
+// first-caller-wins, so an armed render timer firing during the survival window
+// settles the run as "did not render" — while quoting an output tail that
+// visibly contains the marker. A slow-but-fine TUI would fail, intermittently,
+// with a diagnostic pointing at the wrong thing entirely.
+test("a paint landing just under the deadline is not failed by the render timer", async () => {
+  const paintAt = 250;
+  const r = await runRenderSmoke({
+    ...opts,
+    ...stub(
+      `setTimeout(() => { console.log(${JSON.stringify(MARKER)}); ` +
+        `setInterval(() => {}, 1000); }, ${paintAt});`,
+    ),
+    // Deliberately tight: the render deadline expires while the survival window
+    // is still open, which is the whole shape of the bug.
+    timeoutMs: paintAt + 150,
+    surviveMs: 400,
+  });
+  assert.equal(r.code, 0, `expected pass, got: ${r.message}`);
+  assert.doesNotMatch(r.message, /did not render/);
+});
+
 test("fails when the child exits before painting", async () => {
   const r = await runRenderSmoke({
     ...opts,