[dannyward630]

Fixes #427

The integration step ignored every loom optimize error with || true and treated missing output as a skip. This runs the command under strict shell error handling, writes its output into a fresh temporary directory, and requires a nonempty artifact. Optimizer failures and missing output now fail the step, and stale files on self-hosted runners cannot create a false pass.

The CLI supports .wat input directly, so the workflow does not need a runner-dependent skip.

Validation:

• cargo fmt --all -- --check passed.
• The workflow YAML parsed successfully; manual Bash checks confirmed command failures and missing output exit nonzero.
• cargo test --all could not complete on Windows because z3-sys bindgen could not find libclang.dll; the Linux CI suite should provide the full test result.

Assisted by Codex.

[avrabe]

Thanks for this — it fixes the defect as filed, and it fixes it in the right place.

I verified it rather than eyeballing it, because #427 is itself a case of something that looked fine going unchecked.

The claim that could not be known before

|| true is exactly what prevented anyone learning whether the wrapped command works. Your PR body says the CLI takes .wat directly; that needed proof, because if it were false this PR would correctly turn a vacuous green into a red, and that would be a finding in its own right.

Ran your script verbatim against a release build:

🔧 LOOM Optimizer v1.4.1
Input: test.wat
📄 Parsing WAT text format...
✓ Parsed in 1.344ms
⚡ Optimizing...   [19 passes]
✓ Optimized in 25 ms
📦 Encoding to WASM binary...
✅ CLI optimization successful
exit 0

Confirmed: .wat input works, output is non-empty, the step passes honestly. This converts a vacuous pass into a real one, which is the outcome worth having.

Can it go red? (three negative controls)

A gate that only ever goes green is the bug being fixed, so:

scenario	this PR	current main
happy path	exit 0 ✅	exit 0
optimizer fails (malformed input)	exit 1 ✅	exit 0 ❌
zero-byte output file	exit 1 ✅	passes — -f is true for an empty file ❌
binary absent entirely	exit 1 ✅	exit 0, prints ⚠️ CLI test skipped ❌

That last row is the cleanest statement of #427: with the binary missing altogether, the old step reports a skip and goes green.

Details I want to call out as right, not incidental

• test -s rather than -f. -f is true for a zero-byte file, so a truncated or empty encode would have passed. Caught above as control 2.
• mktemp -d instead of a fixed filename. These jobs run on self-hosted runners with a persistent workspace, so a leftover test_optimized.wasm from an earlier run could have produced a pass with no optimizer involvement at all. That is a second, independent false-pass path the original had and this removes.
• set -euo pipefail. Worth noting this lands on a real gap rather than being boilerplate: run: steps in this repo execute under /usr/bin/bash -e {0} with no pipefail, so a pipeline's left-hand failure is invisible. Documented in Gate-integrity sweep of all 6 workflows: a job that prints a false finding every run, and one that goes green without its validator #429 as a class-level issue; this step is now explicitly covered.
• Dropping the "wat tool not available" branch. That message was inaccurate — no external tool was ever involved; the CLI parses .wat itself.

One non-blocking follow-up

test -s proves the file is non-empty, not that it is valid WebAssembly. Given this project's "provably correct" standard, validating the output would be stronger — but wasm-tools is not installed in this job, ci.yml already validates output elsewhere, and adding it here would widen the PR past the filed defect. Better as a separate change; flagging it so it is a decision rather than an oversight.

Status

Workflows show action_required: this is a fork PR and the jobs run on self-hosted runners, so running them needs a maintainer's approval. That approval and the merge are the maintainer's call, not mine — I am leaving both.

For whoever makes it: the diff is one commit, one file, +8/−8, read in full, and contains no network access, no secret use, and no writes outside mktemp -d and the workspace test.wat it cleans up via trap.

Related: #429 is the exhaustive sweep of this pattern across all six workflow files. It leaves this job to this PR and has no overlap with it.

[dannyward630]

Thanks for running the release build and the three negative controls, especially verifying the failure paths against main. I appreciate the careful confirmation of the .wat CLI path.

[dannyward630]

I replaced the sole commit with a signed equivalent so the PR meets the verified-signature rule. The source tree and diff are unchanged; GitHub now verifies commit 82ab6f9. Workflows still need maintainer approval.

[avrabe]

Confirmed the re-signed commit carries the same content, so the verification above still stands and does not need redoing:

1295fe44  tree=e3f23d39377eedf3d7dc0c659bd14d81818849b3  verified=false (unsigned)  parent=3e8ebb8d
82ab6f98  tree=e3f23d39377eedf3d7dc0c659bd14d81818849b3  verified=true  (valid)     parent=3e8ebb8d

Identical tree SHA, identical parent. A git tree hash is a Merkle root over the whole source tree, so matching trees is proof of byte-identical content rather than evidence for it — stronger than re-diffing, and it means the three negative controls I ran against 1295fe44 transfer to 82ab6f98 unchanged. Signature now verifies.

Checks have been approved and are running on the signed head.

[codecov]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

[avrabe]

Merged, thanks. All three runs passed on the signed head. In CI, Integration Testing now parses, optimizes and writes the module before it passes, which makes it the first green from that job that actually tests something.

For the record: the Verification Gate's first attempt failed on stats::tests::record_and_read. That's a race in a test on main (1 failure in 400 runs, reproduced without this PR), fixed separately in #431. It had nothing to do with this change.