From f0d3916b41ac083a8aa43e2a020e1bfbcf79e587 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 00:12:35 +0000 Subject: [PATCH 1/2] test(ui): render AgentActionButtons inside act so the first paint cannot race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `render()` called `createRoot(container)` and `root.render(...)` outside this suite's `act()` helper, then relied on a later `flushReact()` to catch up. `flushReact()` is a single fixed pass — one microtask plus one `setTimeout(0)` — so the first paint depends on the scheduler getting its turn inside that one window. On a CPU-starved runner it does not, and the assertion reads `container.textContent === ""`: AssertionError: expected '' to contain 'Pause' ui/src/components/AgentActionButtons.test.tsx:175 That ejected the merge group for #1787 on 2026-09-21 with no defect in the PR under test. Merge groups were ejecting at 17/33 on that day, and the ejectors are timing-sensitive assertions across unrelated suites — the signature of hypervisor CPU steal (BLO-35090), not of any diff. This suite is the outlier: Inbox, Routines, CompanyInvites and ProjectDetail all render inside `act` already. The local `act()` wraps `flushSync`, so moving the render inside it flushes the first paint before `render()` returns and the assertion no longer races the scheduler. Same class of fix as #1962, which replaced a wall-clock deadline with an attempt budget. Honest limit: the flake reproduces only under contention, so this is verified as "3/3 pass, no new tsc errors, and the scheduling dependency is removed by construction" — not by reproducing the red locally and watching it go green. Co-Authored-By: Claude Opus 5 (1M context) --- ui/src/components/AgentActionButtons.test.tsx | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/ui/src/components/AgentActionButtons.test.tsx b/ui/src/components/AgentActionButtons.test.tsx index 6a156c0ceed0..1d8553f39505 100644 --- a/ui/src/components/AgentActionButtons.test.tsx +++ b/ui/src/components/AgentActionButtons.test.tsx @@ -124,17 +124,28 @@ describe("AgentActionButtons", () => { vi.clearAllMocks(); }); - function render(agent: Agent) { - root = createRoot(container); - root.render( - - - , - ); + // `root.render` must run INSIDE `act`, which is what every other UI suite + // here does (Inbox, Routines, CompanyInvites, ProjectDetail). Rendering + // outside it and relying on a later `flushReact()` to catch up makes the + // first paint depend on the scheduler getting a turn within that helper's + // single microtask + one `setTimeout(0)`. On a CPU-starved runner it does + // not, and the assertion reads `container.textContent === ""` — the + // `expected '' to contain 'Pause'` failure that ejected the merge group for + // #1787 on 2026-09-21. Inside `act`, the render is flushed before this + // returns, so the assertion no longer races the scheduler. + async function render(agent: Agent) { + await act(async () => { + root = createRoot(container); + root.render( + + + , + ); + }); } it("replaces the pause slot with Clear error for error agents", async () => { - render(makeAgent({ status: "error" })); + await render(makeAgent({ status: "error" })); await flushReact(); expect(container.textContent).toContain("Clear error"); @@ -150,7 +161,7 @@ describe("AgentActionButtons", () => { }); it("calls clearError and refreshes agent-related queries", async () => { - render(makeAgent({ status: "error" })); + await render(makeAgent({ status: "error" })); await flushReact(); await act(async () => { @@ -169,7 +180,7 @@ describe("AgentActionButtons", () => { }); it("keeps the normal pause action for non-error agents", async () => { - render(makeAgent({ status: "active" })); + await render(makeAgent({ status: "active" })); await flushReact(); expect(container.textContent).toContain("Pause"); From e91f74b2a5dac706854857b9ea5579ace5f367ad Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 00:44:28 +0000 Subject: [PATCH 2/2] test(mcp-gateway): stop the healthy upstream sharing the unhealthy one's 150ms budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GatewayState.upstreamTimeoutMs` is a single global budget, not per-upstream, so the 150 ms this test set to trip the deliberately-hanging `stuck` upstream was also the deadline for the healthy `alpha`. With `failureThreshold: 1`, one `alpha` response slower than 150 ms opens its breaker for the rest of the test and `tools/list` returns `[]`: AssertionError: expected [] to deeply equal [ 'alpha__search' ] packages/mcp-gateway/src/server.test.ts:1445 That ejected the merge group for #1952 on 2026-09-21. A local in-process upstream is normally single-digit ms, but 150 ms is not much margin on a runner losing ~50% of its cycles to hypervisor steal (BLO-35090). Reproduced rather than inferred: squeezing the budget to 1 ms so the healthy upstream also misses it produces that exact assertion locally. `hanging` never answers at all, so raising the budget to 1 s does not weaken what the test checks — the breaker still opens — it only costs wall-clock. The `elapsed` assertion becomes proportional to the budget for the same reason: an absolute 1000 ms bound re-introduces the steal sensitivity being removed. Verified: 52/52 in the file. Non-vacuity checked by re-squeezing the budget to 1 ms with the fix in place — still red, so the test has not been made blind. Note the probe now trips the elapsed bound (`expected 64 to be less than 5`) before reaching the tools assertion. Co-Authored-By: Claude Opus 5 (1M context) --- packages/mcp-gateway/src/server.test.ts | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/mcp-gateway/src/server.test.ts b/packages/mcp-gateway/src/server.test.ts index 31d929926dc3..dbe549a1a662 100644 --- a/packages/mcp-gateway/src/server.test.ts +++ b/packages/mcp-gateway/src/server.test.ts @@ -1417,12 +1417,25 @@ describe("mcp gateway lifecycle compatibility", () => { it("keeps aggregate initialize available when one upstream is unhealthy", async () => { const alpha = await createStrictMcpUpstream([{ name: "search", description: "Alpha search" }]); const hanging = await createHangingUpstream(); + // `GatewayState.upstreamTimeoutMs` is ONE global budget, not per-upstream, + // so this deadline applies to the healthy `alpha` as much as to `stuck`. + // With `failureThreshold: 1`, a single `alpha` response slower than the + // budget opens its breaker for the rest of the test and `tools/list` + // returns `[]`. At the previous 150 ms that left a local in-process + // upstream almost no margin: on a CPU-starved runner it lost the race and + // this test failed `expected [] to deeply equal [ 'alpha__search' ]`, + // ejecting the merge group for #1952 on 2026-09-21. + // + // `hanging` never answers at all, so raising the budget does not weaken + // what this test checks — it only costs wall-clock. Reproduced directly: + // squeezing this to 1 ms reproduces that exact assertion locally. + const UPSTREAM_TIMEOUT_MS = 1_000; const gateway = await createAggregateGateway( { alpha: { url: alpha.url, credentialHeaders: [] }, stuck: { url: hanging.url, credentialHeaders: [] }, }, - { timeoutMs: 150, failureThreshold: 1 }, + { timeoutMs: UPSTREAM_TIMEOUT_MS, failureThreshold: 1 }, ); const start = Date.now(); @@ -1432,7 +1445,11 @@ describe("mcp gateway lifecycle compatibility", () => { expect(initialize.status).toBe(200); expect(clientSessionId).toBeTruthy(); - expect(elapsed).toBeLessThan(1000); + // Proportional to the budget above, not an absolute wall-clock number: + // what this asserts is "initialize returns near the timeout rather than + // hanging on `stuck`", and an absolute bound re-introduces exactly the + // steal sensitivity this change removes. + expect(elapsed).toBeLessThan(UPSTREAM_TIMEOUT_MS * 5); expect(gateway.state.breaker.stateOf("stuck")).toBe("open"); const list = await postJson(