event loop: count queued concurrent_tasks in is_event_loop_alive() - #36686
Closed
robobun wants to merge 2 commits into
Closed
event loop: count queued concurrent_tasks in is_event_loop_alive()#36686robobun wants to merge 2 commits into
robobun wants to merge 2 commits into
Claude / Claude Code Review
completed
Aug 1, 2026 in 18m 26s
Code review found 2 potential issues
Found 2 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/jsc/VirtualMachine.rs:1042 |
Now-superseded '|| tick_concurrent_with_count() > 0' guards left in place |
| 🟡 Nit | test/regression/issue/11453.test.ts:1-12 |
Regression test carries bug-history comments beyond the issue URL |
Annotations
Check warning on line 1042 in src/jsc/VirtualMachine.rs
claude / Claude Code Review
Now-superseded '|| tick_concurrent_with_count() > 0' guards left in place
nit: the PR description frames the trailing `|| tick_concurrent_with_count() > 0` at `src/runtime/cli/run_command.rs:1535` (and its twin at `src/jsc/web_worker.rs:1152`) as a one-off guard that this fix supersedes — with `!concurrent_tasks.is_empty()` now folded into `is_event_loop_alive()`, that clause no longer flips the condition on the path it was added for. Per REVIEW.md ("when your change supersedes a mechanism, delete the old path in the same PR"), consider removing both clauses here — th
Check warning on line 12 in test/regression/issue/11453.test.ts
claude / Claude Code Review
Regression test carries bug-history comments beyond the issue URL
nit: per REVIEW.md, regression tests get exactly one comment — the issue URL. The 11-line header block at lines 2-12 recounts the edgedb/SCRAM/`postTaskTo` root-cause mechanism (nearly verbatim from the PR description), and further mechanism narration appears at lines 19-20, 59-61, and 94-95. Suggest trimming the header to just `// https://github.com/oven-sh/bun/issues/11453`; the bug history already lives in the PR description where it belongs.
Loading