Skip to content

Add a manual CI build for the patched planegcs glue - #32

Closed
MakerViking wants to merge 5 commits into
mainfrom
ci/planegcs-glue
Closed

Add a manual CI build for the patched planegcs glue#32
MakerViking wants to merge 5 commits into
mainfrom
ci/planegcs-glue

Conversation

@MakerViking

Copy link
Copy Markdown
Owner

Adds .github/workflows/planegcs-glue.yml. Manual trigger only (workflow_dispatch); it produces an artifact plus provenance and changes nothing in the app.

Why

The 2D constraint solver needs 'unsafe-eval' only because planegcs is an emscripten/embind module: embind's new_ helper hands source text to the Function constructor. Closure specialises it to var a=Function; ... a.apply(c,b), which is why grepping for new Function( finds nothing and is not evidence of absence — that false negative is what kept this misdiagnosed while four reporters were told to update a runtime that was never the problem.

Upstream PR Salusoft89/planegcs#12 proposes the same one-line flag but is still open, and its branch is based on 1.2.0 — taking it would silently upgrade the solver alongside the CSP fix. This applies -s DYNAMIC_EXECUTION=0 to the exact commit that produced the 1.1.7 artifact in package-lock.json (951ba1a5, proven via the npm packument's gitHead).

The control build is the point

Building with the flag proves nothing on its own. The job first builds the source unmodified and requires byte-identity with the published npm artifact — that is what establishes this toolchain is the maintainer's, and therefore that the flag is the only variable. Two further gates follow: the wasm must be unchanged (or it is not the glue-only change it claims to be), and the patched glue must actually shed Function references (or the flag did not take effect). If the control does not reproduce, no patched artifact is emitted.

Building in CI rather than on a workstation is deliberate. Vendoring a binary into a public AGPL repo needs provenance a local build cannot give: this leaves public logs, the pinned commit, the toolchain image and checksums beside the artifact. It also keeps third-party native install scripts — tree-sitter needs them to generate bindings.cpp, which upstream stopped committing — in a disposable runner.

Verified before pushing

  • both published checksums in the workflow match the installed artifact
  • the sed applies exactly once to the pinned CMakeLists.txt
  • bindings.cpp is generated, not committed (upstream dropped it in 0cc67f3), so build:bindings must run first — a local build failed at cmake without it

Not in this PR

Vendoring the artifact and tightening the CSP are separate, deliberate steps. tauri dev serves without the CSP, so tightening it without the patched glue would ship a dead solver that looks perfect in development.

Builds planegcs at the commit that produced the 1.1.7 artifact in
package-lock.json, with -s DYNAMIC_EXECUTION=0 so the constraint solver
initialises without 'unsafe-eval' in the CSP.

The control build is the point: the job first builds the source UNMODIFIED
and requires byte-identity with the published npm artifact. That establishes
this toolchain is the maintainer's, and therefore that the flag is the only
variable between the two outputs. Two further gates follow - the wasm must be
unchanged, and the patched glue must actually shed Function references.

Manual trigger only, and it produces an artifact plus provenance rather than
changing anything in the app. Vendoring the result and tightening the CSP are
deliberately separate steps: tauri dev serves without the CSP, so tightening
it without the patched glue would ship a dead solver that looks perfect in
development.
workflow_dispatch is only available once the file is on the default branch,
and a push to main here runs build -> release and publishes a rolling beta.
Spending a published beta to prove a CI job runs is the wrong trade, so this
adds a push trigger scoped to this branch to get the first real run, with a
default ref so a push event (which carries no inputs) still builds the pinned
commit.

Delete the push trigger before merging: workflow_dispatch is the only trigger
this should keep.
The first run refused to emit an artifact when the patched wasm turned out to
differ from the control, which is right as a gate and useless as evidence:
nothing survived to say WHAT differed, so the finding could not be
investigated without rebuilding.

The gate still fails the job. It just no longer takes the artifacts down with
it, and both wasm files ride along now, since they are the thing in question.
Two runs of this job at the SAME pinned commit in the SAME container
disagreed about the wasm. Run 1: control == published, patched != control,
which reads as 'the flag changed the solver'. Run 2: the exact opposite,
control != published and patched == control, which reads as 'it did not'.
The JS glue reproduced byte for byte in both, so the non-determinism is in
the wasm compile and parallel codegen is the prime suspect.

A byte comparison across a non-reproducible build is worthless in both
directions, and worse than worthless here: it produced a confident and wrong
conclusion about solver risk on the first run. -j 1 costs build time and buys
a comparison that means something.
Three runs at the same pinned commit in the same container disagreed about
the wasm: control == published then patched != control; then the exact
opposite; then, single-threaded, neither matching. The JS glue reproduced
byte for byte in all three, so the wasm compile is simply not reproducible
here and comparing its bytes is worthless in both directions. On the first
run that comparison produced a confident and wrong conclusion about solver
risk, which is the strongest argument for removing it.

This does not weaken anything, because the wasm is NOT vendored. The
Function-constructor sink is in the glue, and the app loads the published
wasm by ?url from the npm package. Shipping only the glue leaves the solver
binary exactly as it is today - a stronger guarantee than a checksum in this
job could give.

What remains to be argued is behavioural and belongs on the vendoring commit,
not here: that the patched glue drives the published wasm correctly. That is
what the solver suites are for.
@MakerViking

Copy link
Copy Markdown
Owner Author

Superseded, not abandoned: 54f40ce (now on main) carries this workflow with the corrections that came out of actually running it.

Two sessions wrote this file independently — 21b49d8 on the todo-sweep branch and 2027cbc here — and only this lineage learned what happened on a real run. Those lessons are what landed:

  • the wasm hashes are recorded with a warning instead of gated, because three runs at the same pinned commit in the same container disagreed (once control matched the published binary and patched did not, once the exact opposite, once neither). The JS glue reproduced byte for byte every time, so the non-determinism is in the wasm compile and a byte comparison across it proves nothing in either direction;
  • both builds run -j 1 with that determinism note;
  • DEFAULT_REF so a workflow_dispatch with no input works;
  • artifacts upload on always(), because a failed gate is exactly when they are worth having;
  • the temporary push trigger is gone.

This branch is also based on an older main and merging it would have reverted 14 lines of build.yml. Closing rather than merging; nothing here is lost.

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