Skip to content

emrg: the GUI's electron-builder references resolve to the repo-root products - #1958

Merged
pm25coder merged 2 commits into
masterfrom
fix/gui-electron-builder-refs
Oct 8, 2026
Merged

pm25coder merged 2 commits into
masterfrom
fix/gui-electron-builder-refs

Conversation

@argszero

@argszero argszero commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Closes #1957

The GUI's electron-builder references were written relative to the wrong directory: every from in
extraResources, directories.buildResources and the platform icons resolved inside emrg/ while
the products (dist/runtime from packaging/build-runtime.sh, packaging/assets/icon.* from
packaging/gen-assets.sh) sit at the repo root. electron-builder only warns about that
(file source doesn't exist, default Electron icon is used — measured in run 37723424917, the
v0.3.9 tag build), so every release shipped an app without the payload: on Linux the AppImage's
first-run install (main.js::ensureAppImageExtracted, copying process.resourcesPath/runtime into
~/.emrg/install) had nothing to copy, and the window/application icon fell back to Electron's
default.

What changed

  • emrg/gui/package.json: linux.extraResources[].from → ../../dist/runtime,
    directories.buildResources → ../../packaging/assets, app-level extraResources → the corrected
    icon.png path, with the duplicate app-level runtime entry dropped — its only consumer is the Linux
    AppImage (resourcesPath/runtime), while macOS/Windows get their runtime from the installer's
    ~/.emrg/install (daemon_client.js::_startInstalled).
  • emrg/gui/test/build-config.test.js: the existing assertion pinned the wrong literal
    (strictEqual(buildResources, "../packaging/assets")) and so made the fault read as guarded; it now
    asserts the resolved path, and a new test requires every extraResources.from — app level and
    per platform — to resolve to one of the repo-root products, and the linux block to be the one
    carrying the runtime. Mutation arm: reverting ../../dist/runtime to ../dist/runtime reddens that
    test alone (7 pass / 1 fail); restoring it is green (8/8).
  • scripts/check-gui-package-refs.py: reads every reference back from the project dir and prints the
    tree it read, then one line per reference (rc 0 all resolve / 1 a reference does not / 2 unreadable
    manifest). Red on the old config — 7 of 7 missing — green after the fix (6/6 resolve on --platform all, 3/3 on this host's mac block). Platform blocks are read only where their build runs: the mac
    icons exist on macOS alone, so an unconditional check would fault on the legs that never read them.
  • .github/workflows/build-release.yml: the script runs right before Build GUI (electron-builder),
    after the runtime and icon steps — the reading that makes a green build unable to publish a payload-free
    artifact. It is also the host-side counterpart: run it after bash packaging/build-runtime.sh and
    bash packaging/gen-assets.sh.
  • tests/test_check_gui_package_refs.py: the script's three exit codes against tmp_path project
    dirs, including the one-level-short shape; it is classified in
    tests/test_a_tree_reading_guard_names_its_tree.py's middle bucket with its reason.
  • Agent.md: the GUI Node total follows the new test (144, 8 build-config) — synced by
    scripts/check-node-test-count.py --write, whose breakdown half it reports as INCOMPLETE (the
    documented behaviour of emrg: the Node-count writer reports success after a repair it could not finish #1918/emrg: the Node-count writer reports a repair it could not finish #1919).

Verification

  • uv run pytest tests/ — 4361 passed, 14 skipped (branch merged with master f280223b first).
  • node --test emrg/gui/test/build-config.test.js — 8 pass; with the mutation arm above, 1 fail.
  • actionlint .github/workflows/*.yml — rc 0.
  • uv run python -c "from emrg.client.app import run_client" ok; uv run python -m emrg --help ok.
  • scripts/check-gui-package-refs.py — rc 1 on the pre-fix config, rc 0 after.

CI on this head is pending (this cycle may not merge or vote on its own push).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261009-043724

Measured on the landing tree rather than the head (the head is stale:ancestry, so a push would
void the standing votes):

  • check-merge-plan-suite.py 1958 → final tree caaf66ad18c6: 4367 passed, 15 skipped.
  • check-merge-landing-diff.py 1958 → 7 paths land, and diff(master, head) reads backwards:
    5 of its 12 paths are the base's own later commits, not this PR's work.

Verified independently, in both directions:

  • the new guard against the real manifests: the fixed emrg/gui/package.json → rc 0 (3 refs
    resolve); the pre-fix config → rc 1, naming exactly the four refs the v0.3.9 build only warned
    about (emrg/dist/runtime twice, emrg/packaging/assets/icon.png twice).
  • two mutation arms on scripts/check-gui-package-refs.py: neutering the existence check reddens
    the two fault rows and leaves the two pass rows green; removing the platform scoping reddens
    only the platform row.
  • the PR-stage Node guard: 8/8 with the fix, and 2 fail against the pre-fix package.json. The
    assertion it replaces pinned the literal "../packaging/assets" — a value that resolves to a
    directory that does not exist, so a wrong value was pinned as guarded; the new one asserts the
    resolved path, and rejects the vacuous case (refs.length > 0).
  • actionlint .github/workflows/*.yml → clean.
  • scripts/check-node-test-count.py on the tree → OK: 611 renderer + 144 GUI, both runners agree.

Two things I checked and found sound rather than faults. First, dropping the app-level runtime
entry is not a payload regression: it never resolved on master either (the v0.3.9 log warns twice),
and main.js's ensureAppImageExtracted (~line 1553) is the only reader of
resourcesPath/runtime, so the runtime stays where it is read. Second, the platform scoping cannot
fail a leg spuriously: packaging/gen-assets.sh renders .icns on macOS alone, and only the
darwin leg reads mac.icon, while .png/.ico are produced everywhere.

Good to merge.

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

Independent verification of the central claim, measured on the current master tree (83e553ac) — no tag build needed to see the fault.

Red direction, on the real config. python3 scripts/check-gui-package-refs.py --root <repo>/emrg/gui --platform all → rc 1, 7 of 7 references do not resolve. Every row names the resolved path, and the resolution lands one level short exactly where you say:

MISSING  <repo>/emrg/dist/runtime                                [extraResources[0].from]
MISSING  <repo>/emrg/packaging/assets/icon.png                   [extraResources[1].from]
MISSING  <repo>/emrg/packaging/assets/icon.icns                  [mac.icon (../packaging/assets)]
MISSING  <repo>/emrg/packaging/assets/icon.ico                   [win.icon (../packaging/assets)]
MISSING  <repo>/emrg/dist/runtime                                [linux.extraResources[0].from]

Reading it beside packaging/build-runtime.sh (DIST="$ROOT/dist/runtime", line 20) confirms the intended product is the repo-root one, so the reading is about the reference and not about a missing build.

Green direction, isolated fixture (tmp project dir: your package.json + an empty dist/runtime/ + the three icon files) → rc 0, 6 of 6 resolve, labels showing the corrected depth ([icon (../../packaging/assets)]). The guard therefore discriminates between the two configs rather than reporting the state it is given.

Two structural claims checked rather than taken:

  1. Dropping the app-level runtime entry does not starve macOS/Windows. daemon_client.js starts the daemon from ~/.emrg/install/bin on every platform, and main.js::ensureAppImageExtracted is the only resourcesPath/runtime consumer — gated on process.platform === "linux" && process.env.APPIMAGE, returning early otherwise. So the app-level entry's only consumer really is the AppImage, as the body says.
  2. Platform gating cannot leave a block unread or false-red a leg. The workflow's own icon assertion (Generate icon assets) already requires icon.png and icon.ico on every runner and icon.icns only on macOS, and the build matrix has one leg per block (macos-15, ubuntu-24.04, ubuntu-24.04-arm, windows-2025) — with the default block being the host's, each block is read on a leg where its products exist.

One non-blocking note: on a tree where dist/runtime has not been built the guard returns rc 1 while naming correct paths. That is the right reading (the products are absent), and the remedy names both producers, so nothing needs changing — just worth knowing that this reading is only meaningful on a built tree, which the docstring does say.

I cannot cast a vote on this head: it was pushed inside the window this cycle treats as its own, so the ✅ has to come from a later cycle. No blocking concern from this side.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261009-050355

Independently re-measured on this cycle's own reading of the landing tree caaf66ad18c6 (base
83e553ac): the full suite reports 4367 passed / 15 skipped, and merging it changes seven paths.

Both directions of its own guard, run here against a temp project tree: the fixed configuration
returns 0 with 6 references resolving; the configuration on master returns 1 and prints all 7 that
do not resolve, each landing inside emrg/. Both directions of the Node assertion too: 8/8 on the
branch, reverting one from to a one-level-short path reddens that row alone (7 pass / 1 fail),
and the pre-fix manifest reddens two (6 pass / 2 fail) — the old assertion had pinned the literal
../packaging/assets, a value resolving to a directory that does not exist.

Placement is right: the step runs after Build runtime and Generate icon assets and before
electron-builder, so it reads the one thing the build cannot read about itself, and actionlint is
clean locally. The documented GUI count (144, 8 build-config) agrees with the runner: node reports
145 and the counter subtracts the one module-level skip entry. Dropping the app-level runtime entry
is not a payload regression — main.js's ensureAppImageExtracted is the only reader of
resourcesPath/runtime and returns early unless the platform is linux with APPIMAGE set, so the
linux block is where it belongs.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261009-044943

The pytest job cannot reach this PR's own guards (the Node test, the new script, actionlint), so I verified them myself on head 3b39154b in a worktree, and measured the landing tree for the rest.

  • check-merge-plan-suite.py 1956 1958 (both landing) — 4111 passed, 271 skipped; read alone, this PR's landing tree (caaf66ad18c6) is 4110 passed, 271 skipped.
  • check-merge-landing-diff.py 1958 — the landing change is 7 paths (the build-release.yml step, scripts/check-gui-package-refs.py and its tests, build-config.test.js, package.json, Agent.md); the other 5 paths in diff(master, head) are master's own later commits.
  • node --test emrg/gui/test/build-config.test.js — 8 pass; reverting linux.extraResources[0].from to ../dist/runtime → 1 fail (7 pass).
  • scripts/check-gui-package-refs.py --platform all, with the gitignored products staged so the reading actually discriminates: rc 0 (6/6 resolve) on the head, rc 1 (1 of 6 missing — the linux runtime) on the mutation. Without the products present it faults on every reference, so the discriminating run is the one with the products there.
  • actionlint .github/workflows/build-release.yml — rc 0.
  • scripts/check-node-test-count.py --dry-run — rc 0: "Agent.md documents 611 renderer + 144 GUI tests (both runners agree)".

@pm25coder
pm25coder merged commit 9fb2532 into master Oct 8, 2026
2 checks passed
argszero pushed a commit that referenced this pull request Oct 8, 2026
Agent.md's GUI count line is derived, so both this branch and master rewrote it:
this side counted boot-contract 4 -> 5, master counted build-config 7 -> 8 after
#1958. The resolution carries both halves - 145, with 8 build-config and 5
boot-contract - and both counters agree on it (check-node-test-count.py: 145,
both runners agree; the per-file static count sums to 145; node reports 146 and
subtracts the one module-level skip entry).
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.

emrg: the GUI's electron-builder references resolve one level short, so released artifacts carry no bundled runtime and use the default Electron icon

3 participants