Repository navigation
emrg: the GUI's electron-builder references resolve to the repo-root products - #1958
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ 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 treecaaf66ad18c6: 4367 passed, 15 skipped.check-merge-landing-diff.py 1958→ 7 paths land, anddiff(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/runtimetwice,emrg/packaging/assets/icon.pngtwice). - 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.pyon 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.
|
Independent verification of the central claim, measured on the current master tree ( Red direction, on the real config. Reading it beside Green direction, isolated fixture ( Two structural claims checked rather than taken:
One non-blocking note: on a tree where 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
left a comment
There was a problem hiding this comment.
✅ 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
left a comment
There was a problem hiding this comment.
✅ 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 (thebuild-release.ymlstep,scripts/check-gui-package-refs.pyand its tests,build-config.test.js,package.json,Agent.md); the other 5 paths indiff(master, head)are master's own later commits.node --test emrg/gui/test/build-config.test.js— 8 pass; revertinglinux.extraResources[0].fromto../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)".
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).
Closes #1957
The GUI's electron-builder references were written relative to the wrong directory: every
frominextraResources,directories.buildResourcesand the platform icons resolved insideemrg/whilethe products (
dist/runtimefrompackaging/build-runtime.sh,packaging/assets/icon.*frompackaging/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, thev0.3.9 tag build), so every release shipped an app without the payload: on Linux the AppImage's
first-run install (
main.js::ensureAppImageExtracted, copyingprocess.resourcesPath/runtimeinto~/.emrg/install) had nothing to copy, and the window/application icon fell back to Electron'sdefault.
What changed
emrg/gui/package.json:linux.extraResources[].from→../../dist/runtime,directories.buildResources→../../packaging/assets, app-levelextraResources→ the correctedicon.pngpath, with the duplicate app-level runtime entry dropped — its only consumer is the LinuxAppImage (
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 nowasserts the resolved path, and a new test requires every
extraResources.from— app level andper platform — to resolve to one of the repo-root products, and the
linuxblock to be the onecarrying the runtime. Mutation arm: reverting
../../dist/runtimeto../dist/runtimereddens thattest 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 thetree 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'smacblock). Platform blocks are read only where their build runs: the macicons 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 beforeBuild 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.shandbash packaging/gen-assets.sh.tests/test_check_gui_package_refs.py: the script's three exit codes againsttmp_pathprojectdirs, 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 byscripts/check-node-test-count.py --write, whose breakdown half it reports as INCOMPLETE (thedocumented 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 masterf280223bfirst).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 --helpok.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).