Skip to content

ci(electron): streamline release dependency setup (2/8) - #10325

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.50from
backryun:perf/electron-release-setup
Aug 16, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.50from
backryun:perf/electron-release-setup

Conversation

@backryun

Copy link
Copy Markdown
Contributor

Part of #10321 (stage 2).

What changed

  • remove the redundant root node_modules cache restored immediately before npm ci
  • install Electron dependencies with npm ci --no-audit --no-fund
  • synchronize electron/package-lock.json so clean installs work on every release runner
  • copy the prepared standalone runtime into resources/app once instead of mapping its node_modules subtree twice
  • add workflow and packaging regression guards

Impact

The release matrix avoids unnecessary node_modules cache transfer/extraction, uses deterministic dependency installation for both package trees, and eliminates duplicate packaging traversal of the standalone dependency tree.

Validation

  • Electron-focused unit tests: 127 passed
  • npm run test:vitest: 357 passed
  • npm ci --dry-run --ignore-scripts --no-audit --no-fund in electron/: passed
  • electron-release.yml YAML parse: passed
  • npm run check:file-size: passed

Comparable GitHub Actions timing is not available until this workflow runs; the tracking issue leaves that measurement open rather than treating a local dry-run as release-runner timing.

@backryun
backryun requested a review from diegosouzapw as a code owner August 14, 2026 00:29
@backryun
backryun force-pushed the perf/electron-release-setup branch 5 times, most recently from 06b1939 to 7d63019 Compare August 14, 2026 15:54
@backryun backryun changed the title ci(electron): streamline release dependency setup ci(electron): streamline release dependency setup (2/8) Aug 14, 2026
@backryun
backryun force-pushed the perf/electron-release-setup branch 7 times, most recently from 0d77aed to 13e2669 Compare August 15, 2026 07:20
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks for this — clean, well-scoped Stage 2 of the #10321 release-efficiency work.

I re-verified this independently rather than just trusting the PR description:

  • Ran both touched test files against your branch head: tests/unit/electron-packaging.test.ts and the new tests/unit/electron-release-efficiency.test.ts — 5/5 pass.
  • Confirmed the pre-fix base branch really does have the duplicate app / app/node_modules extraResources mapping, so the updated packaging test is a genuine regression guard, not a tautology.
  • Ran npm ci --dry-run --ignore-scripts --no-audit --no-fund in electron/ against your updated lockfile — resolved cleanly (284 packages, no drift), confirming the lockfile resync is actually in sync with package.json.
  • Parsed the workflow YAML to confirm it's still valid after the cache-step removal.

All CI checks are green and the merge state is clean. I don't have any blocking items — nice, surgical fix that matches exactly what the tracking issue's Stage 2 checklist describes. The only thing left unmeasured (actual release-runner timing) is something only a real release run can produce, and you've already flagged that honestly instead of guessing from the local dry-run, which I appreciate.

@backryun
backryun force-pushed the perf/electron-release-setup branch from 13e2669 to a2122e7 Compare August 15, 2026 13:08
@backryun
backryun force-pushed the perf/electron-release-setup branch from a2122e7 to 901bd7f Compare August 15, 2026 17:16
@diegosouzapw
diegosouzapw merged commit 47f53f3 into cheaper-inference:release/v3.8.50 Aug 16, 2026
16 checks passed
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Validated in local merge-train .claude/worktrees/merge-train-20260816-000002-suite.log on devbox-vm-06-dev002 @ 8be62df65f61cc4cf29cdc3e6bc38173ffafa4e3 (FAST gates green: static + changed tests + vitest; daily full-suite run still required). Merged under the batch merge-train protocol (merge-gates §7). ⚠️ base-red inherited: #10523 (ServiceSupervisor order-dependent test, not touched by this batch).

@backryun
backryun deleted the perf/electron-release-setup branch August 16, 2026 03:25
diegosouzapw pushed a commit that referenced this pull request Aug 25, 2026
…e2e assertion and 10 integration reds

Electron Package Smoke — a packaging defect that had been hidden behind another
packaging defect for nine days. Once the loginHeaderCapture fix let the main process
start, the server underneath died on 'Cannot find module next': resources/app/server.js
shipped without resources/app/node_modules.

electron-builder discards the ROOT node_modules in code, not by configuration —
app-builder-lib/out/util/filter.js:42 has a hard-coded `if (relative === "node_modules")
return false` that runs before any filter pattern. The second extraResources entry
pointing INTO ../.build/electron-standalone/node_modules is what sidesteps it, because
those relative paths are never equal to "node_modules". #10325 removed that entry as an
apparent duplicate and flipped the test to assert "exactly once", freezing the
regression as if it were the contract. Restored, and the unit guard now pins both
entries — proven by mutation: reverting package.json to the post-#10325 shape fails the
guard 3/4, restoring it passes 4/4.

group-b-quota-plans-config — the assertion was impossible to satisfy on ANY route, and
the page was never broken. layout.tsx hands the whole message catalogue to
NextIntlClientProvider, React serialises that prop into the RSC payload, and en.json
carries "Internal Server Error" twice, so page.content() always contains it: probing
/dashboard, /dashboard/costs, /dashboard/settings and /login showed the string present
with every page rendering fine, and a pageerror probe on the failing run captured zero
client exceptions. This is the same trap that killed the sibling not.toContain("500")
in fc77100 ("raw HTML is unreliable") — that one was removed, this one was kept.
Now asserts on rendered text, which still catches a real error boundary. The pageerror
capture stays: the CI failure carried no stack trace, which is why it was misread twice.

Integration — 10 of the 14 shard-2 reds, all sibling-test gaps behind security fixes:
monitoring health now takes a Request and requires management auth (GHSA-mvf8-qc78-5mxm);
the OAuth import routes moved to requireManagementAuth (GHSA-mg76) — the test accepts
both guard shapes and gained a stronger anchor that every exported handler awaits a
guard on its own request, mutation-verified; skill tool names are derived from
encodeSkillToolName() and the fake upstream now returns the encoded name so
decodeSkillToolName() is exercised too; previous_response_id now fails closed (#10262);
proxy_logs persist as an async batch (#11182) so the test flushes first;
providerQuotaOverrides joined GET /api/resilience (#9871); the reasoning fixture used a
model that stopped being thinking-incompatible, replaced and pinned with a premise
assert so it cannot rot silently again.

A vacuous assert.ok(true, "all 10 streams completed without hanging") was replaced with
real anchors — content must arrive on every stream and the active Timeout count must not
grow.

Four are deliberately left red rather than aligned, each now tracked: #11551 (the
/v1/models after() wiring is dead — the route passes a third argument to a two-parameter
function and catalogCache never imports after, so the #8728 contract is unimplemented),
#11552 (~27% of requests emit an extra discarded upstream call; the delivered
distribution is exactly 0.70, so weighted routing is correct and the waste is the real
finding), the fixed-account combo pin (aligning it would destroy the per-step attribution
the test exists for), and the web_search fallback already tracked as #11524.

Package Artifact — the provenance stamp I added last round used git rev-parse HEAD, which
under pull_request is the ephemeral merge commit and therefore never an ancestor of the
release branch. Now takes the PR head sha.

Refs #10692
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…e2e assertion and 10 integration reds

Electron Package Smoke — a packaging defect that had been hidden behind another
packaging defect for nine days. Once the loginHeaderCapture fix let the main process
start, the server underneath died on 'Cannot find module next': resources/app/server.js
shipped without resources/app/node_modules.

electron-builder discards the ROOT node_modules in code, not by configuration —
app-builder-lib/out/util/filter.js:42 has a hard-coded `if (relative === "node_modules")
return false` that runs before any filter pattern. The second extraResources entry
pointing INTO ../.build/electron-standalone/node_modules is what sidesteps it, because
those relative paths are never equal to "node_modules". cheaper-inference#10325 removed that entry as an
apparent duplicate and flipped the test to assert "exactly once", freezing the
regression as if it were the contract. Restored, and the unit guard now pins both
entries — proven by mutation: reverting package.json to the post-cheaper-inference#10325 shape fails the
guard 3/4, restoring it passes 4/4.

group-b-quota-plans-config — the assertion was impossible to satisfy on ANY route, and
the page was never broken. layout.tsx hands the whole message catalogue to
NextIntlClientProvider, React serialises that prop into the RSC payload, and en.json
carries "Internal Server Error" twice, so page.content() always contains it: probing
/dashboard, /dashboard/costs, /dashboard/settings and /login showed the string present
with every page rendering fine, and a pageerror probe on the failing run captured zero
client exceptions. This is the same trap that killed the sibling not.toContain("500")
in feb0709 ("raw HTML is unreliable") — that one was removed, this one was kept.
Now asserts on rendered text, which still catches a real error boundary. The pageerror
capture stays: the CI failure carried no stack trace, which is why it was misread twice.

Integration — 10 of the 14 shard-2 reds, all sibling-test gaps behind security fixes:
monitoring health now takes a Request and requires management auth (GHSA-mvf8-qc78-5mxm);
the OAuth import routes moved to requireManagementAuth (GHSA-mg76) — the test accepts
both guard shapes and gained a stronger anchor that every exported handler awaits a
guard on its own request, mutation-verified; skill tool names are derived from
encodeSkillToolName() and the fake upstream now returns the encoded name so
decodeSkillToolName() is exercised too; previous_response_id now fails closed (cheaper-inference#10262);
proxy_logs persist as an async batch (cheaper-inference#11182) so the test flushes first;
providerQuotaOverrides joined GET /api/resilience (cheaper-inference#9871); the reasoning fixture used a
model that stopped being thinking-incompatible, replaced and pinned with a premise
assert so it cannot rot silently again.

A vacuous assert.ok(true, "all 10 streams completed without hanging") was replaced with
real anchors — content must arrive on every stream and the active Timeout count must not
grow.

Four are deliberately left red rather than aligned, each now tracked: cheaper-inference#11551 (the
/v1/models after() wiring is dead — the route passes a third argument to a two-parameter
function and catalogCache never imports after, so the cheaper-inference#8728 contract is unimplemented),
cheaper-inference#11552 (~27% of requests emit an extra discarded upstream call; the delivered
distribution is exactly 0.70, so weighted routing is correct and the waste is the real
finding), the fixed-account combo pin (aligning it would destroy the per-step attribution
the test exists for), and the web_search fallback already tracked as cheaper-inference#11524.

Package Artifact — the provenance stamp I added last round used git rev-parse HEAD, which
under pull_request is the ephemeral merge commit and therefore never an ancestor of the
release branch. Now takes the PR head sha.

Refs cheaper-inference#10692
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.

2 participants