WordPress / WordPress/contributor-toolkit
npm runner IPC bridge can drop early output and completion events
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 36
- Forks
- 13
- Avg merge
- 23h 19m
- Merged PRs (30d)
- 72
Description
Summary
The preload bridge registers npm runner event listeners only after awaiting the IPC request that starts the child process. A fast child can emit stdout, stderr, or its completion event before the renderer subscribes, permanently losing those events.
This race affects both dependency installation and npm script execution. It is separate from the renderer bug where the Run first full build button sends output to an unrendered state: even a correctly rendered consumer cannot display an event that the preload bridge missed.
Affected paths
runNpmInstall(): https://github.com/WordPress/experimental-wp-dev-env/blob/trunk/src/preload.js#L9-L23runNpmScript(): https://github.com/WordPress/experimental-wp-dev-env/blob/trunk/src/preload.js#L25-L40- Main-process installation spawn: https://github.com/WordPress/experimental-wp-dev-env/blob/trunk/src/main.js#L506-L545
- Main-process script spawn: https://github.com/WordPress/experimental-wp-dev-env/blob/trunk/src/main.js#L547-L590
Current event ordering
For both APIs, the sequence is:
- The renderer calls
ipcRenderer.invoke(...). - The main-process IPC handler allocates an ID.
- The main process immediately spawns the child.
- The main process attaches child stdout, stderr, and close listeners that forward events with
event.sender.send(...). - The handler returns the ID.
- The preload awaits the IPC response.
- Only after the await resolves does preload call
ipcRenderer.on(...)for log and completion events.
Steps 3–5 can produce events before steps 6–7 finish. IPC does not buffer channel events for listeners that do not yet exist.
Possible symptoms
- The beginning of npm output is missing.
- A fast command failure contains no useful diagnostic text.
- A fast completion event is lost, leaving
installing,building, or terminal-running state stuck indefinitely. - Listener cleanup never runs after a lost completion event.
currentRunIdRefand the main-process running-process maps can disagree with renderer state.- Reproduction appears timing-dependent and may vary across macOS, Windows, Linux, development, and packaged builds.
The macOS report that prompted this investigation involved a command that failed almost immediately with exit 127. In that particular run, manually invoking the command did receive its completion event, so the report does not prove the race fired. The ordering in the production code nevertheless makes the race possible and independently testable.
Steps to reproduce reliably
Add a test runner or fixture that writes a unique stdout marker, writes a stderr marker, and exits immediately. Invoke it repeatedly through the production preload/main bridge.
The test should assert on every iteration that:
- both markers are received;
- the completion callback runs exactly once;
- the exit code is correct;
- all listeners are removed afterward.
Increasing IPC scheduling pressure or running the fixture in a packaged app should make the current ordering easier to expose, but the test should not depend on probabilistic timing once the protocol is refactored.
Expected behavior
The renderer/preload must be subscribed before the main process is permitted to emit the first correlated event. Every started operation must deliver its complete ordered output and exactly one terminal completion result.
Suggested implementation directions
Renderer-generated correlation ID
Generate the operation ID in preload, register channel listeners filtered by that ID, and only then invoke the main process with the ID to start the operation. Main should reject duplicate or invalid IDs.
Two-phase start protocol
Have main allocate an ID without spawning, return it, allow preload to subscribe, and use a second IPC call to start the operation. This requires cancellation and cleanup if the renderer disappears between allocation and start.
Dedicated MessagePort or stream abstraction
Return a dedicated port for each operation and establish it before spawning. This avoids global event channels and naturally scopes cleanup, but is a larger change.
Deferring spawn() with a timer after returning the ID may reduce the likelihood but does not establish a reliable ordering contract and should not be the long-term fix.
Cancellation and lifecycle considerations
- Closing or reloading the renderer should terminate or detach the corresponding operation safely.
- Cancellation must still produce one final result and must not be mistaken for an ordinary failure or retry condition.
- Listener cleanup should happen for success, failure, spawn errors, cancellation, renderer destruction, and rejected IPC calls.
child.on('error')should be represented explicitly; relying only onclosemakes spawn failures harder to distinguish.
Acceptance criteria
- Listeners or a dedicated stream are ready before any child output can be emitted.
- A deterministic fast-process integration test receives all stdout and stderr and exactly one completion event.
- The test covers both install and run-script paths, or both paths share one tested operation primitive.
- Spawn errors, non-zero exits, signals, and user cancellation all complete the operation exactly once.
- Renderer reload/destruction cannot leave unbounded global listeners behind.
- Existing streaming behavior and process cancellation continue to work on macOS, Windows, and Linux.
Related
- #41 tracks the checklist build button writing output to an unrendered state.
- #42 tracks failed/partial dependency installs being misclassified as complete.
- #39 proposes extracting child-process logic from
main.jsinto a testable seam and running tests in CI; that work would make a deterministic regression test for this race easier to add.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read the affected preload paths in src/preload.js and the installation and script spawn handlers in src/main.js, then trace event ordering and cleanup for both operations. Build a deterministic fast-process integration fixture that emits stdout and stderr and exits immediately. Done means both paths deliver all output and exactly one terminal result across failures, cancellation, and renderer teardown.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- desktop, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100