Skip to content

fix: bound configurable deadlines to the Node timer range - #212

Merged
steipete merged 1 commit into
mainfrom
fix/phase-five-timeout-bounds
Sep 12, 2026
Merged

fix: bound configurable deadlines to the Node timer range#212
steipete merged 1 commit into
mainfrom
fix/phase-five-timeout-bounds

Conversation

@steipete

Copy link
Copy Markdown
Contributor

What Problem This Solves

A configured timeout above 2,147,483,647 ms overflows Node's timer range and becomes a one-millisecond deadline. Provider, validation, and PR-publishing operations could therefore be killed immediately despite an apparently long timeout.

User Impact

Out-of-range timeout overrides use the existing safe default. Valid fractional delays are truncated, and provider-specific settings still take precedence over the global provider timeout.

Why This Change Was Made

Share one bounded timer parser across provider checks/execution, validation, PR publishing, Go discovery, and Windows process cleanup. This keeps the previously bounded Go/Windows behavior and prevents other execution paths from diverging.

Evidence

  • Five new boundary regressions fail on the previous implementation.
  • Live before: a real Node child configured with 2147483648 emits TimeoutOverflowWarning, gets a 1 ms deadline, and exits 124.
  • Live after: the same compiled command runner uses its 2000 ms default and the child prints ready with exit 0.
  • pnpm typecheck, pnpm lint, provider/publishing/process/Go tests (38 passed, 2 existing platform skips), and pnpm build pass.
  • Isolated Codex autoreview: scoped-clean at P0–P2.

@steipete
steipete requested a review from a team as a code owner September 12, 2026 23:31
@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix: bound configurable deadlines to the Node timer range This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@steipete
steipete merged commit fa57be4 into main Sep 12, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant