Skip to content

fix(runtime): make every child process settle, so the loading notice ends - #15

Merged
endrix merged 1 commit into
mainfrom
fix/bound-child-processes
Aug 25, 2026
Merged

fix(runtime): make every child process settle, so the loading notice ends#15
endrix merged 1 commit into
mainfrom
fix/bound-child-processes

Conversation

@endrix

@endrix endrix commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Fixes the "Model loading in progress" notification that never goes away.

What decides its lifetime

Exactly one promise. RequestModelActionHandler starts the notification, awaits loadSourceModel, ends it after — layout, bounds and rendering all happen after the clear, so nothing slow there can pin it. That narrows the bug to a single question: what can make the load never settle.

The answer: nothing bounded any child process

All six spawns awaited close with no deadline. A child that starts and never exits resolves neither close nor error, so the load hangs forever — and the notification is created without cancellable, so there is no cancel button and the only escape is closing the tab.

This is not exotic for these runtimes: acquisition imports the user's own module, so anything that module does at import time — a sleep, a socket, input(), a lock — hangs the child. Two sites (sidecar-invoker, the send command) had also never listened for error, where an unstartable command is both an uncaught exception in the extension host and a promise that never settles. Every diagram edit goes through the invoker.

One place owns it

runChildProcess — deadline, whole-process-group kill (SIGTERM → SIGKILL), settle on exit when a grandchild holds the pipes open, resolved result when the command never started. Never rejects, so no caller can be stranded by an error path it forgot.

Three details that decide whether it works:

  • The process group, not the child. Killing the direct child leaves a grandchild alive holding the pipes we are waiting on — close still never comes.
  • exit with a grace period, not only close. Same reason, from the other side. There is a test for exactly this shape.
  • A deadline miss is not a crash. It is reported as "no response after Ns", not as a failure of the command, because what the reader should do about it is different.

The pattern is lifted from mlir-viewer's mlir-opt.ts, which already does this correctly.

The deadline is raisable

120s default for graph loads (acquisition runs user code and can be honestly slow — this is here to catch the child that will NEVER answer, not to police slow ones), 20s for the openability probe, which only decides whether a file can open and falls back to a source scan. <namespace>.graphLoadTimeoutSeconds raises it, and the message that reports a miss names the setting: a limit the reader cannot see or change is just a different way to be stuck. Every unusable value — zero, negative, a typo — falls back to the default rather than to "no deadline".

Also: the latent version of the same bug

The polling-refresh suppression was handler state read after an await, and the suppressed finish returned without ending the monitor a real load had created. Today's dispatcher queue serializes external dispatches so I could not reproduce it live — but a reentrant request runs inline, and the failure mode is the exact symptom this PR is about. Suppression now travels with the monitor, so finishing cannot be overruled by another request. The new test fails against the old logic (monitor ended 0 times instead of 1).

Verification

Build, 128 test files across all four workspaces, check:neutrality 4/4. The runChildProcess tests drive real processes rather than mocks — what is under test is Node's own event behaviour (error vs exit vs close, signals, process groups), which a fake would only restate.

Not covered: the setting is read but not contributed to either shell's package.json, so it does not appear in the Settings UI yet — one line in wfpy-ide and streamblocks-ide, and their own PRs.

…ends

"Model loading in progress" lives exactly as long as the model load: the
handler starts it, awaits the load, and ends it after. Every child process
in that path awaited `close` with no deadline, so a command that started
and never exited left a promise pending forever — and the notification has
no cancel button, so the only way out was closing the tab.

Acquisition imports the user's own module, so import-time code (a sleep, a
socket, `input()`, a lock) hangs the child. Two call sites had also never
listened for 'error', where an unstartable command is both an uncaught
exception in the host and a promise that never settles.

`runChildProcess` now owns the contract for all six spawns: a deadline that
kills the whole process group (SIGTERM, then SIGKILL), settling on `exit`
when a grandchild holds the pipes open, and a resolved result for a command
that never started. The deadline is raisable per product via
`<namespace>.graphLoadTimeoutSeconds`, and the message that reports a miss
names it — a limit the reader cannot change would just be a different way
to be stuck.

Also removes a latent version of the same bug: the polling-refresh
suppression was handler state read after an await, so a refresh in flight
could talk a real load out of ending the monitor it had started. What is
suppressed now travels with the monitor, so finishing cannot be overruled
by another request.
@endrix
endrix merged commit e86730b into main Aug 25, 2026
endrix added a commit that referenced this pull request Aug 25, 2026
fix(build): restore the import that #15 merged without
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