modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

docs(scripts): correct the attribution in the render-smoke margin comment

Browse source 
Copilot round 1. The comment blamed `script(1)` PTY allocation, but this
test's `stub()` spawns `process.execPath` directly — the PTY wrapper
lives in `smoke-tui.mjs`, the caller — so the fixture never pays that
cost and the 66-114ms figure (measured through the wrapper) was not
this path's.

Re-measured without it: Node boot alone is 46-59ms idle, which fits the
old 150ms budget comfortably. That is *why* it read as a flake rather
than a bug — what actually exhausted the budget was scheduling delay
under a full gate run, on both the child's boot and the parent's timer.
The fix is unchanged; only the reasoning was wrong.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-27 22:59:26 -04:00 • committed by GitHub
commit 7083c6c7aa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 18 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

30
scripts/lib/render-smoke.test.mjs
View file

@ -98,20 +98,26 @@ test("a paint landing just under the deadline is not failed by the render timer"
// actual paint < timeoutMs < actual paint + surviveMs
//
// i.e. the render deadline expires while the survival window is still
// open. Both margins are deliberately wide, because `paintAt` is measured
// from the child's first JS tick while `renderTimer` is armed in the
// parent at spawn — so the left margin has to absorb `script(1)` PTY
// allocation plus Node boot, which measured 66-114ms on an *idle* machine
// and goes past 150ms under the load of a full `local:gate` run. These
// constants used to be `paintAt + 150` / `400`, which left ~36ms of slack
// at best and failed deterministically inside the gate (#2177) — with the
// "did not render" diagnostic that is exactly the misreport this test
// exists to catch.
// open. Both margins are deliberately wide, because the two clocks start
// at different moments: `paintAt` is measured from the child's first JS
// tick, while `renderTimer` is armed in the parent at spawn. The left
// margin therefore has to absorb everything in between — child process
// startup, plus whatever scheduling delay the machine is under.
//
// `stub()` spawns `process.execPath` directly, so there is no `script(1)`
// in this path: the PTY wrapper lives in `smoke-tui.mjs`, the caller. Node
// boot alone measured 46-59ms idle here. That fits the old `paintAt + 150`
// budget with room to spare, which is exactly why this read as a flake
// rather than a bug — but under the load of a full `local:gate` run the
// remaining ~90ms of slack is not enough to cover scheduling delay on both
// the child's boot and the parent's timer, and it failed three gate runs
// in a row while passing every time standalone (#2177). The diagnostic it
// failed with, "did not render", is the misreport this test exists to
// catch.
//
// Do not tighten these back up to make the test faster: the overlap is the
// point, not the tightness. Worst-case paint lands near 365ms, well under
// the 1200ms deadline, and 1200ms still falls inside a survival window
// that closes no earlier than ~1750ms.
// point, not the tightness. 1200ms clears any plausible paint time, and
// still falls inside a survival window that cannot close before ~1750ms.
timeoutMs: 1200,
surviveMs: 1500,
});