debugger: wait for target startup before initialization

[Archkon]

Trying to fix the test error that triggered at previous pr when run github action ci/cd

https://github.com/nodejs/node/actions/runs/28730483133/job/85195106458?pr=64301

Fixes: #64116
Fixes: #61762
Fixes: #64005

[trivikr]

Several previous attempts of deflaking were unsuccessful.
Please refer #64116 for prior discussions and links to other PRs.

[codecov]

Codecov Report

❌ Patch coverage is 96.49123% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.31%. Comparing base (c18fd90) to head (aa1cfd8).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/debugger/inspect_helpers.js	95.23%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #64304      +/-   ##
==========================================
- Coverage   90.31%   90.31%   -0.01%     
==========================================
  Files         759      759              
  Lines      247646   248350     +704     
  Branches    46697    46871     +174     
==========================================
+ Hits       223654   224288     +634     
- Misses      15463    15473      +10     
- Partials     8529     8589      +60     

Files with missing lines	Coverage Δ
lib/internal/debugger/inspect_probe.js	82.05% <100.00%> (-0.09%)	⬇️
lib/internal/debugger/inspect_repl.js	91.36% <100.00%> (+0.05%)	⬆️
lib/internal/debugger/inspect_helpers.js	97.00% <95.23%> (-0.41%)	⬇️

... and 40 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[joyeecheung]

I think at the minimum the commit should explain why the previous flakes happen, and why the change is supposed to fix it?

[inoway46]

@Archkon

Thanks for working on this!

Could you clarify why the timeout needs to be increased? If waitForDebugger() addresses the race condition, I would expect the timeout increase to be unnecessary.

Also, I think keeping the original timeout would make the stress test results more convincing, since otherwise it’s difficult to tell whether the improvement comes from waitForDebugger() or simply from the increased timeout.

[inoway46]

@Archkon
Thanks for clarifying. If Debugger.paused was emitted but not received by the JavaScript layer within 15 seconds, could you share a failing run or trace showing that?

Because the current stress runs include both changes, it is difficult to tell whether waitForDebugger() alone resolves the issue.

[inoway46]

@Archkon
Could you remove this change from the PR?

It appears to be needed only for the --repeat 1000 -j16 stress run, where concurrent instances of this test share the same node.cpuprofile. It is unrelated to the debugger startup race and is not needed in the normal CI run.

If it is needed for stress validation, it can remain only in the stress-test branch or workflow.

[inoway46]

@Archkon
Thanks. My concern is the PR scope, not the -j4 setting.

The linked run still executes this test 1000 times. Normal CI executes it only once, so the node.cpuprofile collision shown there is specific to the stress run. I think this change should be removed from this PR and kept only in the stress-test setup if needed.

Also, please leave this thread unresolved while this concern is still under discussion.