WordPress / WordPress/contributor-toolkit

npm runner IPC bridge can drop early output and completion events

Open
#43 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug known-issue post-v1.0
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

Current event ordering

For both APIs, the sequence is:

  1. The renderer calls ipcRenderer.invoke(...).
  2. The main-process IPC handler allocates an ID.
  3. The main process immediately spawns the child.
  4. The main process attaches child stdout, stderr, and close listeners that forward events with event.sender.send(...).
  5. The handler returns the ID.
  6. The preload awaits the IPC response.
  7. 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.
  • currentRunIdRef and 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 on close makes 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.js into a testable seam and running tests in CI; that work would make a deterministic regression test for this race easier to add.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.