Skip to content

fix(core): bound scan phases + binary-split cascade with hard timeouts - #899

Closed
rayhanadev wants to merge 2 commits into
mainfrom
ray/tyler
Closed

fix(core): bound scan phases + binary-split cascade with hard timeouts#899
rayhanadev wants to merge 2 commits into
mainfrom
ray/tyler

Conversation

@rayhanadev

@rayhanadev rayhanadev commented Jun 19, 2026

Copy link
Copy Markdown
Member

Why

Production traces show genuine pathological hangs the existing per-unit timeouts do not contain — runInspect max 57,912s (~16h), Linter.run max 26,910s (~7.5h), supply-chain fetchScore max 981s despite a 10s per-fetch timeout (undici sockets that ignore abort), and dead-code workers clipping their 120s ceiling. Every CI run and every interactive scan on a repo with one bad dependency socket or one quadratic file is exposed. This is a correctness/robustness fix that caps worst-case wall time without changing the median scan.

Before:

runInspect            p100 = 57,912s (16h)   ← one wedged phase hangs the whole job
Linter.run            p100 = 26,910s (7.5h)  ← binary-split cascade re-waits 60s per level, unbounded
SupplyChain.fetchScore p100 = 981s           ← socket ignores per-fetch abort, run-to-completion
DeadCode worker       max 1,072s             ← unref'd in-worker timer fires late under a starved loop

After:

Bounded, fail-fast into the existing skipped-check / fail-open channels:
- split cascade  → drop to onPartialFailure after a 3-min cumulative budget / depth-8 cap
- supply-chain   → fail open ([]) after a 90s whole-check cap
- dead-code      → skip after a 2.5-min Effect-side cap (worker SIGKILL'd on interruption)
- lint           → skip + null score after a 5-min Effect-side cap
- whole scan     → ScanDeadlineExceeded after a 15-min backstop

What changed

  • Binary-split cascade (spawn-batches.ts): cumulative split-time budget (OXLINT_SPLIT_TOTAL_BUDGET_MS) + recursion-depth cap (OXLINT_SPLIT_MAX_DEPTH) drop remaining files of a pathological batch into the existing onPartialFailureskippedCheckReasons["lint:partial"] channel instead of re-waiting a full spawn timeout at every split level. Both are injectable params (constant defaults) so the bound is deterministically testable.
  • Supply-chain (check-supply-chain.ts): whole-check Effect.timeoutOption(SUPPLY_CHAIN_TOTAL_TIMEOUT_MS) fails open ([]) — the same fail-open contract as the per-fetch orElseSucceed(() => null).
  • Dead-code & lint phases (run-inspect.ts): Effect-side timeoutOption caps fold a timeout into the existing deadCodeFailure / lintFailure Refs (lint→score null, tagged OxlintBatchExceeded for renderer dispatch). The dead-code worker's AbortSignal now SIGKILLs the child on Effect interruption (no orphan child).
  • Overall deadline (run-inspect.ts): Effect.timeout(ScanDeadlineMs) backstops anything not bounded per phase, raising the new ScanDeadlineExceeded reason on the ReactDoctorError union; the channel type stays ReactDoctorError.
  • All caps env-tunable (REACT_DOCTOR_LINT_PHASE_TIMEOUT_MS, REACT_DOCTOR_DEAD_CODE_PHASE_TIMEOUT_MS, REACT_DOCTOR_SCAN_DEADLINE_MS) via Context.Reference; defaults sit above measured p95. Duplicated env-parse logic extracted into a shared readPositiveEnvMs util.

Internal-only — no CLI flag, config-schema, JSON-report, or Action change. Patch changeset included.

Test plan

  • pnpm typecheck — 11/11 ✓
  • pnpm lint — clean ✓ · pnpm format:check — clean ✓ · pnpm smoke:json-report — OK ✓
  • pnpm test (run per-package directly): core 896, api 15, CLI 1837 (+15 skipped), language-server 60 — all pass, including 8 new tests (cascade bound, supply-chain fail-open, dead-code/lint phase caps, overall deadline, dead-code abort→terminate, ScanDeadlineExceeded).
  • Note: 2 check-security-scan tests fail only under a local global ~/.config/git/ignore that ignores .env (a pre-existing environmental trap, file untouched here); they pass in CI and locally with core.excludesFile=/dev/null.

🤖 Generated with Claude Code


Note

Medium Risk
Changes core scan orchestration and subprocess lifecycle; timeouts degrade gracefully via existing skip contracts, but mis-tuned env vars could skip lint/dead-code or fail scans early on legitimately slow repos.

Overview
Adds hard, env-tunable wall-clock caps so pathological scans cannot run for hours. Normal scans should be unchanged; defaults sit above measured p95.

Oxlint binary-split recovery now shares a 3-minute cumulative budget and depth-8 cap across all retries in spawnLintBatches. When exhausted, remaining files go through the existing onPartialFailure / partial-lint skip path instead of re-waiting a full per-batch spawn timeout at every split level.

Supply-chain wraps the concurrent Socket fetches in a 90s whole-check Effect.timeoutOption that fails open to no diagnostics, matching per-fetch skip behavior when sockets ignore abort.

Lint and dead-code get Effect-level caps (5 min / 2.5 min) in runInspect that fold into the existing lint-failure (null score, OxlintBatchExceeded tag) and dead-code skip contracts so the rest of the scan still finishes. AbortSignal is threaded from Effect.tryPromise through the linter into oxlint spawns and the dead-code worker so phase timeouts SIGKILL in-flight children instead of leaving orphans.

Overall scan is wrapped in Effect.timeout(ScanDeadlineMs) (default 15 min), surfacing new ScanDeadlineExceeded on the ReactDoctorError union for wedged phases not individually capped.

New readPositiveEnvMs centralizes env parsing for timeout Context.References (REACT_DOCTOR_*_TIMEOUT_MS, REACT_DOCTOR_SCAN_DEADLINE_MS).

Reviewed by Cursor Bugbot for commit 5fb6222. Bugbot is set up for automated code reviews on this repo. Configure here.

Production traces showed `runInspect` hanging up to 16h and `Linter.run` up
to 7.5h: the existing per-unit timeouts don't contain a wedged dependency
socket, a quadratic file's binary-split cascade, or a dead-code worker whose
unref'd timer fires late under a starved event loop.

Add four bounded layers, each failing fast into the existing skipped-check /
fail-open channels rather than stalling:

- spawnLintBatches: a cumulative split-time budget + recursion-depth cap drop
  pathological files to `onPartialFailure` instead of re-waiting a full spawn
  timeout at every split level.
- checkSupplyChain: a whole-check `Effect.timeoutOption` fails open (`[]`) on a
  many-socket pileup that ignores the per-fetch abort.
- run-inspect: Effect-side lint and dead-code phase caps fold a timeout into
  the existing failure contracts; an overall scan deadline raises the new
  `ScanDeadlineExceeded` reason. The dead-code worker is SIGKILL'd on Effect
  interruption so no orphan child survives.

All caps are env-tunable (`REACT_DOCTOR_*_TIMEOUT_MS` / `_SCAN_DEADLINE_MS`)
with defaults well above measured p95, so only the pathological tail is
affected — no behavior change for normal scans. The duplicated env-parse
logic is extracted into a shared `readPositiveEnvMs` util.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@pkg-pr-new

pkg-pr-new Bot commented Jun 19, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/eslint-plugin-react-doctor@899
npm i https://pkg.pr.new/oxlint-plugin-react-doctor@899
npm i https://pkg.pr.new/react-doctor@899

commit: 5fb6222

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5850024. Configure here.

Comment thread packages/core/src/run-inspect.ts
Comment thread packages/core/src/run-inspect.ts

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread packages/core/src/errors.ts
…ear down oxlint on lint cap

Cursor Bugbot review of #899:

- The overall deadline (10 min) was only ~60s above the sum of the per-phase
  caps (supply-chain 90s + lint 5min + dead-code 2.5min = 9min, run
  sequentially), so a scan legitimately using those budgets could trip the hard
  ScanDeadlineExceeded instead of degrading via the per-phase skips. Raise
  SCAN_TOTAL_DEADLINE_MS to 15 min so the backstop only fires for a genuinely
  unbounded phase (wedged git/IO).

- The lint-phase cap left in-flight oxlint subprocesses running. Because JS
  promises aren't cancellable, spawnLintBatches kept spawning children in the
  background after the Effect fiber was interrupted, so the cap stopped waiting
  but didn't reclaim the work. Thread the Effect.tryPromise AbortSignal through
  Linter -> runOxlint -> spawnLintBatches -> spawnOxlint: on abort the in-flight
  child is SIGKILL'd and queued batches short-circuit before spawning —
  symmetric to the dead-code worker teardown already in this PR.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rayhanadev added a commit that referenced this pull request Jun 20, 2026
…in (#903)

Integrates the 7 large-repo speedup PRs (#894#900) plus the dead-code reworks
done on top of them.

Headline (measured on Sentry, supply-chain on): full scan ~142s → ~40s (3.6x),
with dead-code findings fully intact.

- deslop: skip the analysis passes react-doctor discards — the semantic
  TS-Program pass and the code-quality detectors (duplicate-block/complexity/
  feature-flag/TS-smell/private-type-leak/re-export-cycle), gated behind a new
  `reportCodeQuality` flag (default on for deslop standalone). ~8.5x faster
  dead-code on a large repo, byte-identical consumed findings, locked by a
  parity test.
- Dead-code runs sequentially by default with a file-count-scaled timeout,
  fixing a regression where overlapping it with lint oversubscribed the cores
  and silently dropped all dead-code findings under supply-chain. Opt-in
  `REACT_DOCTOR_DEAD_CODE_OVERLAP=on` now splits the core budget instead of
  doubling it.
- Plus the merged PRs: memory-budgeted worker cap (#896), bounded-phase hard
  timeouts (#899), supply-chain∥lint overlap (#894), per-file lint cache (#900),
  deterministic diagnostics (#897), module-cache dedup (#895).

CI: full test matrix (macOS/Ubuntu 20–26/Windows), build, typecheck, lint, and
react-doctor smoke all green. The 1s "CodeQL" check is the pre-existing
code-scanning-alert gate (main's CodeQL analysis is green); no security-relevant
code changed.
@rayhanadev

Copy link
Copy Markdown
Member Author

Superseded by #903, which integrated this work and squash-merged to main (commit 627f9ca). The changes shipped there — some reworked during integration (e.g. the dead-code overlap became sequential-by-default, and cost-ordering was defaulted off); see the #903 description and the changesets on main. Closing in favor of main.

@rayhanadev rayhanadev closed this Jun 20, 2026
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