Skip to content

The closed-read-end case in run-dev-unbuilt-workspace.e2e.test.ts still uses a fixed wall-clock bound as its oracle #14716

Description

@os-trump

Found while fixing #14648 (PR #14715). Deliberately not folded into that PR — the correct shape is arguable rather than mechanical, and guessing at it is how the sibling assertion got its constant in the first place.

What

packages/cli/test/run-dev-unbuilt-workspace.e2e.test.ts, the case right after the one #14648 is about:

it('a CLOSED read end is released at once, not held for the bound (EPIPE reaches the callback)', () => {
  expect(closedEnd.signal).toBeNull();
  expect(closedEnd.elapsedMs).toBeLessThan(STALL_MS);     // STALL_MS = 10_000
});

The property it means to pin is real and worth pinning: when the reader is gone rather than idle, write()'s callback fires with EPIPE and the shim's 15 s no-progress bound is never paid. But the way it asks the question is a fixed wall-clock comparison — the same class of oracle that ejected five PRs from the merge queue through the neighbouring case.

STALL_MS is also borrowed rather than derived here: it is case 4's parent-stall duration, and it has no relationship to what this case is discriminating.

Measurements

Measured on a 4-core container while working #14648, same child, same --import hook:

condition closedEnd elapsed
heavy-verify lock held 1536 / 1572 / 1415 ms

So the margin is 10 s over a ~1.5 s measurement, and the measured term is fully elastic — unlike the sibling case, none of its budget is spent on the shim's wall-clock bound. Roughly 6.7x of headroom against a term whose idle-to-contended spread bin/run-dev.js:36-37 records at 6.9x for the closely related pre-drain phase.

Why it was left alone rather than fixed in #14715

The obvious repair — scale the bound with the same in-band calibration #14715 introduces — makes this case weaker under exactly the load it is supposed to survive, which is the wrong direction:

  • ceiling max(STALL_MS, calibration), idle: max(10 000, ~7 400) = 10 000 ms. A regression that paid the bound would land at ~16 500 ms, so it is still caught.
  • the same ceiling under 8x contention: calibration ~22 600 ms, so the ceiling is 22 600 ms — while a regression that paid the bound lands at ~4 200 + 15 000 = ~19 200 ms and passes.

The discriminating property is "did NOT pay the 15 s bound", and nothing the parent can observe separates that from "the runner was slow", because the parent deliberately holds no reference run of the closed-reader path. Making the oracle load-robust here needs a different observation, not a different constant — which makes it a decision rather than a mechanical edit.

Priority note

Nothing has been ejected by this case; the whole 24 h ejection history on this file is the sibling assertion. This is filed so the class is not re-discovered from a queue build. Unassigned and unlabelled for triage.

https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions