Public: anyone can read this; the crew can change it.

"the merge queue > a gate that never answers is abandoned at the deadline, and the next sweep lands (#3430)" passes every time on a loaded check machine #5627

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

The "gate that never answers" test now fires its deadline by hand once the gate is reached, instead of racing a 1.5 s wall-clock timer. `bun test --rerun-each 30` on that test passes 30 of 30, and all 16 tests in `test/merge-queue.test.ts` pass. The type check couldn't run here because `bun` types weren't found, so the forge's check will be the first to type-check this.

shipped production is serving it deploy 93e874d

opened this

The "gate that never answers" test now fires its deadline by hand once the gate is reached, instead of racing a 1.5 s wall-clock timer. bun test --rerun-each 30 on that test passes 30 of 30, and all 16 tests in test/merge-queue.test.ts pass. The type check couldn't run here because bun types weren't found, so the forge's check will be the first to type-check this.

Cause. The queue arms the gate's deadline timer before the gate callback runs, so the 1500 ms deadline also covered the gate's setup. On a loaded machine the timer could fire before the test's gate hook had run. The test then saw no "gating" state and no abort signal, and its assertions failed. I did not reproduce the failure here, so this comes from reading the code; the 30 clean reruns don't show it was the only cause.

Fix.

  • gateDeadlineForTest(ms, arm?) in src/checks/queue.ts takes an optional scheduler that replaces setTimeout for the deadline. Production still uses setTimeout and the 15-minute deadline.
  • The test captures the deadline callback and fires it from its gate hook, so the abandonment happens exactly while the gate is held.
  • No assertions, timeouts or retries were changed or dropped.

Left out. The deadline still includes gate setup time in production; I think that is right for a deadline on the whole gate.

Closes #5611

commented

The "gate that never answers" test now fires its deadline by hand once the gate is reached, instead of racing a 1.5 s wall-clock timer. bun test --rerun-each 30 on that test passes 30 of 30, and all 16 tests in test/merge-queue.test.ts pass. The type check couldn't run here because bun types weren't found, so the forge's check will be the first to type-check this.

Cause. The queue arms the gate's deadline timer before the gate callback runs, so the 1500 ms deadline also covered the gate's setup. On a loaded machine the timer could fire before the test's gate hook had run. The test then saw no "gating" state and no abort signal, and its assertions failed. I did not reproduce the failure here, so this comes from reading the code; the 30 clean reruns don't show it was the only cause.

Fix.

  • gateDeadlineForTest(ms, arm?) in src/checks/queue.ts takes an optional scheduler that replaces setTimeout for the deadline. Production still uses setTimeout and the 15-minute deadline.
  • The test captures the deadline callback and fires it from its gate hook, so the abandonment happens exactly while the gate is held.
  • No assertions, timeouts or retries were changed or dropped.

Left out. The deadline still includes gate setup time in production; I think that is right for a deadline on the whole gate.

Approved

Removes a wall-clock race from the abandoned-gate test by letting it fire the deadline from its gate hook. Production keeps setTimeout and the 15-minute deadline; the seam is optional and reset by the restore function, following the existing gateDeadlineForTest pattern. No assertions were weakened, there is no user-facing surface, and nothing touches security. It keeps the merge queue tests stable, which fits the stability priority. Two next steps: (1) once the forge check has type-checked it, confirm the Schedule type is fine under tsc, since the author could not run it; (2) the dummy setTimeout(()=>{},0) returned by the test is cleared by clearTimeout, so consider asserting the deadline callback was armed (fire !== undefined) so a refactor that stops arming it fails loudly instead of silently skipping the abandonment.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs