Skip to content

fix(extensions): the runtime harness no longer races a test-local 3 s startup deadline - #1436

Merged
Juliusolsson05 merged 3 commits into
mainfrom
fix/extensions-managed-runtime-load
Sep 27, 2026
Merged

Juliusolsson05 merged 3 commits into
mainfrom
fix/extensions-managed-runtime-load

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Fixes #1334.

Plan: docs/plans/2026-09-27-extension-runtime-harness-startup-deadline.md, the first commit.

Problem

electron.system.test.ts › "keeps one managed runtime…" failed under full-suite load and passed alone. The failure text was never captured.

Evidence (reproduced)

  • The test. It runs scripts/check-extension-frames.mjs --runtime-only, a bundled runtimeHarness.ts in a hidden Electron app. Alone it takes about 8.6 s.
  • The failure. In 12 parallel runs, with a temporary probe logging the cold-command results, 6 failed at the first assertion with 0 !== 32. Every cold call was rejected with Extension runtime did not finish starting before the deadline. The passing runs logged 32 fulfilled.
  • The cause. The harness built ExtensionRuntimeService with startupTimeoutMs: 3000, tighter than the product default of 10 s. The deadline itself is tested by the hung-engine case, which now runs at the product value. Under load, a cold Electron activation takes longer than 3 s, the runtime is retired, and all queued cold commands are rejected.

Change

The harness drops the startupTimeoutMs override and uses the product's deadline. In the bounded-cold case, the deadline is not what is under test, and the 3 s value raced a cold Electron start there.

  • invocationTimeoutMs: 1500 is kept, because the stalled-engine case needs it to fire fast.
  • until's 3 s poll budget was never observed failing, so it is left alone.

Review a (5ede7157)

  • In-flight view request case. The handler slept 80 ms, and under 24-way load it could finish before the detach (1 of 24). It now waits for an explicit releaseSlow command, sent after the detach and before the awaited rejection, which is delivered when the result reaches the closed connection.
  • Activation waits. Under 24-way load "updated startup runtime" timed out at 3 s (2 of 24). until takes a budget, and the two waits that include a cold activation use the product's own startup deadline, now exported as EXTENSION_RUNTIME_STARTUP_TIMEOUT_MS.
  • Comment corrected. The startup deadline IS tested (hung-engine).

Verification

  • With the first change: 24 of 24 parallel runs passed (two rounds of 12, load average about 40).
  • At 5ede7157: 48 of 48 runs at 24-way parallel load (load average up to 103). src/main/extensions: 23 files, 346 tests. npx tsc -b clean.
  • Without it: 6 of 12 failed at the same load.
  • There is no new test. The reproduction is the existing check under parallel load; the command is in the plan.

🤖 Generated with Claude Code

Juliusolsson05 and others added 2 commits September 27, 2026 07:22
…line under load (#1334)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e, not a racing 3 s one (#1334)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… by the product deadline (review a)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Disposition for review a (head 5ede7157)

  • P2: a load race in the in-flight view request case. Fixed.
    • slowIdentity no longer sleeps 80 ms. It waits for an explicit releaseSlow command, which the harness sends after detachOwner and before awaiting the rejection. The rejection is delivered when the handler's result reaches the closed connection, so this is the exact order the case checks.
    • Your storage-flag variant could not work: a running extension does not see main-side storage writes. That is why the release goes through a command.
  • Found while verifying: a third race. Fixed.
    • Under 24-way load, "updated startup runtime" timed out in 2 of 24 runs. It is an until wait with a 3 s budget that includes a cold re-activation.
    • until now takes a budget. The two waits that include an activation use ACTIVATION_WAIT_MS, which is the product's own startup deadline, now exported as EXTENSION_RUNTIME_STARTUP_TIMEOUT_MS from runtimeService.ts, and no longer a literal. Ordinary polls keep 3 s.
  • P3: the comment wrongly said no assertion tests the startup deadline. Fixed. hung-engine does, at the product's 10 s, which adds about 7 s to that case. The plan and body are corrected.
  • Verification: 48 of 48 runs at 24-way parallel load, at load average up to 103. src/main/extensions passes (346 tests), and npx tsc -b is clean.

@Juliusolsson05

Copy link
Copy Markdown
Owner Author

READY (W1). Head 5ede7157. Exact-head CI is green. Every reviewer is MERGE-READY: a (verify-report-a), b (report-b), c (report-c). The disposition for review a is posted above. Review c's non-blocking notes are follow-ups: the zero-slack ACTIVATION_WAIT_MS edge, and the now-unused startupTimeoutMs injection. merge-gate.sh --member: GATE PASS. Mergeable and clean.

Note (manager q117): the load evidence was gathered with 12- and 24-way parallel Electron runs, which contributed to host PTY exhaustion (#1437). Any re-verification should run at most 4 at a time.

Juliusolsson05 added a commit that referenced this pull request Sep 27, 2026
@Juliusolsson05
Juliusolsson05 merged commit cb29223 into main Sep 27, 2026
2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the fix/extensions-managed-runtime-load branch September 27, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

class:C9-tests Test hygiene sev:P3 Minor type:bug Something works wrong

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(extensions): electron.system managed-runtime test fails under full-suite load

1 participant