test(bundle-runner): fix SIGTERM-handler registration race (1s timeout flake) by koala73 · Pull Request #3618 · koala73/worldmonitor

koala73 · 2026-05-07T12:04:27Z

Summary

Fixes the timeout emits terminal reason BEFORE SIGTERM/SIGKILL grace test flake that hit PR #3617's post-merge run on main (run #25494120215, 1 of 8114 tests failing).

The fixture's first line is:

process.on('SIGTERM', () => {}); console.log('hung'); setInterval(...);

On a cold/loaded CI runner Node's startup (parse + import + execute that line) can exceed the test's 1s timeout. SIGTERM then arrives before process.on registered the ignore-handler, so the child dies via Node's default SIGTERM behaviour and never reaches console.log('hung'). Total elapsed lands at ~1.1s instead of the expected ~11s (1s timeout + 10s SIGKILL grace), the [HANG] hung assertion fails, and the test never exercises the SIGKILL escalation it's actually here to validate.

Fix: bump TIMEOUT_MS from 1000 → 3000. Typical Node startup is 50–200ms; even loaded GitHub-hosted runners shouldn't exceed 1–2s, so 3s gives comfortable cold-start margin while still exercising the same timeout → SIGTERM → 10s grace → SIGKILL flow. The test's existing 20s elapsedMs cap remains comfortably above the new worst-case (3s + 10s grace + overhead ≈ 14s).

Also relaxed the regex /timeout after 1s — sending SIGTERM/ to /timeout after \d+s — sending SIGTERM/ so a future timeout bump doesn't require a coordinated regex update — the assertion's purpose is "Failed line names the timeout-after-N pattern", not the literal N.

Why it was a real bug, not "slow runner"

The 1s value had no contract that the fixture's user code had run before the timer fired. The fixture's SIGTERM handler MUST be registered before SIGTERM arrives for the test's "ignores SIGTERM, must SIGKILL" contract to be exercised. The 1s window didn't guarantee that — the test was structurally flaky. 3s makes the contract reliably testable on real-world runner conditions.

Test plan

npx tsx --test tests/bundle-runner.test.mjs — ran 5×, all 9 tests pass each run, no flakes
npm run typecheck — clean
Post-merge: Test workflow on the new main commit goes green

…t flake) The `timeout emits terminal reason BEFORE SIGTERM/SIGKILL grace` test flaked on PR #3617's post-merge run on main (run #25494120215). The fixture's first line is: process.on('SIGTERM', () => {}); console.log('hung'); setInterval(...); On a cold/loaded CI runner Node's startup (parse + import + this line's execution) can exceed the test's 1s timeout. SIGTERM then arrives BEFORE process.on registered the ignore-handler, so the child dies via Node's default SIGTERM behaviour and never reaches console.log('hung'). Total elapsed lands at ~1.1s instead of the expected ~11s (1s timeout + 10s SIGKILL grace), the `[HANG] hung` assertion fails, and the test never exercises the SIGKILL escalation it's actually here to validate. Fix: bump TIMEOUT_MS from 1000 to 3000. Typical Node startup is 50-200ms and even loaded GitHub-hosted runners shouldn't exceed 1-2s, so 3s gives comfortable cold-start margin while still exercising the same timeout → SIGTERM → 10s grace → SIGKILL flow the test validates. The test's existing 20s elapsedMs cap remains comfortably above the new worst-case (3s + 10s grace + overhead ~= 14s). Also relaxed the regex `/timeout after 1s — sending SIGTERM/` to `/timeout after \d+s — sending SIGTERM/` so a future timeout bump doesn't require a coordinated regex update — the assertion's purpose is "Failed line names the timeout-after-N pattern", not the literal N. Verification: ran tests/bundle-runner.test.mjs 5 times locally, all 9 tests pass each run, no flakes. The 1s value was a real timing bug, not just a slow runner — it was flaky because there's no contract that user code in the fixture has run before the timer fires. The fixture's SIGTERM handler MUST be registered before SIGTERM arrives for the test's "ignores SIGTERM, must SIGKILL" contract to be exercised, and the 1s window didn't guarantee that.

vercel · 2026-05-07T12:04:32Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
worldmonitor	Ready	Preview, Comment	May 7, 2026 0:07am

greptile-apps · 2026-05-07T12:06:11Z

Greptile Summary

Addresses a documented flake in the timeout emits terminal reason BEFORE SIGTERM/SIGKILL grace test by increasing TIMEOUT_MS from 1 s to 3 s and loosening the assertion regex from a hardcoded 1s literal to \d+s. The root cause was a genuine structural race: on a cold/loaded CI runner, Node.js startup could take longer than 1 s, so SIGTERM arrived before the fixture's process.on('SIGTERM', () => {}) handler registered — making the test exercise the wrong (default-SIGTERM) path.

Timeout bump (1000 → 3000 ms): Gives Node at least 3 s to parse and execute the handler registration before SIGTERM fires; the existing 20 s upper-bound cap remains well above the new ~13 s worst-case (3 s timeout + 10 s SIGKILL grace + overhead).
Regex loosened to \d+s: Decouples the assertion from the literal timeout value so future bumps don't require a coordinated regex update; the semantic check ("Failed line names the timeout-after-N pattern") is preserved.

Confidence Score: 5/5

Safe to merge; the change is confined to a single test file and corrects a real structural flake without weakening any meaningful assertion.

The timeout bump from 1 s to 3 s directly addresses the documented race between Node startup and SIGTERM delivery, and the 20 s upper-bound cap still gives comfortable headroom above the new ~13 s worst-case path. The regex relaxation preserves the test's semantic intent while removing a brittle literal. No production code is touched.

No files require special attention.

Important Files Changed

Filename	Overview
tests/bundle-runner.test.mjs	Bumps TIMEOUT_MS from 1000→3000 and relaxes the SIGTERM regex to fix a documented CI flake; no logic regressions.

Sequence Diagram

sequenceDiagram
    participant Test as Test (bundle-runner.test.mjs)
    participant Runner as _bundle-runner.mjs
    participant Child as _bundle-fixture-hang.mjs

    Test->>Runner: "runBundleWith([HANG, timeoutMs=3000])"
    Runner->>Child: spawn (Node.js startup)
    Note over Child: ~50-200ms: parse + register process.on('SIGTERM', () => {})
    Child-->>Runner: stdout "hung"
    Runner-->>Test: [HANG] hung (streamed live)

    Note over Runner: 3000ms timeout fires
    Runner->>Child: SIGTERM
    Note over Child: Handler installed - SIGTERM ignored
    Runner-->>Test: Failed after Xs: timeout after 3s - sending SIGTERM

    Note over Runner: 10s SIGKILL grace window
    Runner->>Child: SIGKILL
    Runner-->>Test: Did not exit on SIGTERM - SIGKILL
    Runner-->>Test: exit code 1

    Note over Test: assert elapsedMs < 20_000 (~13s actual)

_{Reviews (1): Last reviewed commit: "test(bundle-runner): fix SIGTERM-handler..." | Re-trigger Greptile}

…t flake) (koala73#3618) The `timeout emits terminal reason BEFORE SIGTERM/SIGKILL grace` test flaked on PR koala73#3617's post-merge run on main (run #25494120215). The fixture's first line is: process.on('SIGTERM', () => {}); console.log('hung'); setInterval(...); On a cold/loaded CI runner Node's startup (parse + import + this line's execution) can exceed the test's 1s timeout. SIGTERM then arrives BEFORE process.on registered the ignore-handler, so the child dies via Node's default SIGTERM behaviour and never reaches console.log('hung'). Total elapsed lands at ~1.1s instead of the expected ~11s (1s timeout + 10s SIGKILL grace), the `[HANG] hung` assertion fails, and the test never exercises the SIGKILL escalation it's actually here to validate. Fix: bump TIMEOUT_MS from 1000 to 3000. Typical Node startup is 50-200ms and even loaded GitHub-hosted runners shouldn't exceed 1-2s, so 3s gives comfortable cold-start margin while still exercising the same timeout → SIGTERM → 10s grace → SIGKILL flow the test validates. The test's existing 20s elapsedMs cap remains comfortably above the new worst-case (3s + 10s grace + overhead ~= 14s). Also relaxed the regex `/timeout after 1s — sending SIGTERM/` to `/timeout after \d+s — sending SIGTERM/` so a future timeout bump doesn't require a coordinated regex update — the assertion's purpose is "Failed line names the timeout-after-N pattern", not the literal N. Verification: ran tests/bundle-runner.test.mjs 5 times locally, all 9 tests pass each run, no flakes. The 1s value was a real timing bug, not just a slow runner — it was flaky because there's no contract that user code in the fixture has run before the timer fires. The fixture's SIGTERM handler MUST be registered before SIGTERM arrives for the test's "ignores SIGTERM, must SIGKILL" contract to be exercised, and the 1s window didn't guarantee that.

vercel Bot deployed to Preview May 7, 2026 12:07 View deployment

koala73 merged commit 6f7b99e into main May 7, 2026
12 checks passed

koala73 mentioned this pull request May 16, 2026

fix(feeds): #3715 review — server parity + drop 0-item Google News placeholders #3717

Merged

5 tasks

koala73 deleted the fix/bundle-runner-stdout-timing-flake branch June 25, 2026 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(bundle-runner): fix SIGTERM-handler registration race (1s timeout flake)#3618

test(bundle-runner): fix SIGTERM-handler registration race (1s timeout flake)#3618
koala73 merged 1 commit into
mainfrom
fix/bundle-runner-stdout-timing-flake

koala73 commented May 7, 2026

Uh oh!

vercel Bot commented May 7, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

koala73 commented May 7, 2026

Summary

Why it was a real bug, not "slow runner"

Test plan

Uh oh!

vercel Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented May 7, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented May 7, 2026 •

edited

Loading