Skip to content

Stop the matchstick run from executing a cached wasm - #292

Merged
thedavidmeister merged 1 commit into
mainfrom
2026-08-25-issue-230
Aug 26, 2026
Merged

Stop the matchstick run from executing a cached wasm#292
thedavidmeister merged 1 commit into
mainfrom
2026-08-25-issue-230

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Closes #230.

Matchstick 0.6.0 compiles each suite to tests/.bin/<suite>.wasm and then reuses that file. Compiler::execute (src/compiler/mod.rs at tag 0.6.0) compiles only when

matches.is_present("recompile") || !Path::new(&out_file).exists() || is_source_modified(&in_file, &out_file)

and is_source_modified (src/compiler/sources.rs) is an mtime comparison — the test file or a transitively imported file has to be strictly NEWER than the wasm. An mtime is not a fact about content, so a tree where nothing happens to land newer than the cached binary runs that binary and reports its results as the suite's.

subgraph/tests/.bin/metaboard.wasm was tracked in git and last written 2024-07-31. subgraph/tests and subgraph/src have changed as recently as 2026-08-20. strings on the committed blob shows it still exports generated/metaboard0/MetaBoard/MetaBoard#hash and links matchstick-as/index/MockedFunctionMetaBoard.hash() and its subgraph mocks were deleted in e6e191c (#106). So the binary a fresh checkout landed was a build of tests that no longer exist. Today CI is defused by accident, not by design: graph codegen writes subgraph/generated/ after checkout, the test file imports ../generated/..., and that fresh mtime forces a recompile. Nothing guarantees that ordering.

What changed

Two independent things have to hold for the cached path to be unreachable, and neither implies the other, so the issue's "and/or" is taken as and.

  • Nothing to reuse. subgraph/tests/.bin/metaboard.wasm is removed from git and tests/.bin added to subgraph/.gitignore. A fresh checkout now has no wasm at all, so the !out_file.exists() arm forces compilation whatever the mtimes say — and a local run no longer dirties the tree with an opaque 45KB binary that the next git add -A re-commits. Deleting without the ignore rule leaves the churn; ignoring without the deletion changes nothing, because ignore rules do not apply to tracked paths.
  • Nothing is reused anyway. subgraph/docker-compose.yml sets ARGS: -r, so the docker run never consults an mtime. rainix's subgraph-test is npm ci && docker compose up --abort-on-container-exit from subgraph/, and the pinned image config for rainprotocol/matchstick:main is ENV ARGS= plus CMD ["/bin/sh","-c","/binary-linux-22 ${ARGS}"]ARGS is the extension point the image exists to offer, so this needs no command: override. -r is matchstick's short spelling of --recompile ("Force-recompiles the tests", src/cli.rs).
  • test/subgraph/SubgraphMatchstick.t.sol pins both, in the rainix-sol lane. Deliberately not a check inside the matchstick suite: the question is whether that suite executes the current sources at all, and a check shipped inside the cached binary is answered by the stale build along with everything else. It needs no docker, so it runs on every push. The compose file is read as parsed YAML over yq/vm.ffi for the reason SubgraphManifest.t.sol gives — a commented-out # ARGS: -r satisfies a text search while the compose parser never sees it, which is exactly how this protection would be lost.
  • foundry.toml and CLAUDE.md name git as a third vm.ffi binary, so "nothing else here shells out" stays true. git and yq are both in rainix's sol-build-inputs, i.e. on PATH for the forge test the rainix-sol lane runs.

The now-absent subgraph/tests/.bin directory is created on demand: matchstick shells out to <lib>/assemblyscript/bin/asc --outFile tests/.bin/<suite>.wasm, and assemblyscript@0.19.23 (the version subgraph/package-lock.json resolves at the top level) does if (!fs.existsSync(dirPath)) mkdirp(dirPath) in writeFileNode.

Relation to the other open work on this suite

#282 (issue #206) changes subgraph/src/metaBoard.ts and three files under subgraph/tests/. This PR touches none of those, and #282 touches none of these, so there is no textual conflict in either merge order.

They meet at exactly one point, and it is the point this PR is about. #282's QA notes that subgraph/tests/.bin/metaboard.wasm "is a tracked matchstick build artifact last written in 2024 and already stale against several test changes since. My runs rewrote it; I restored it rather than commit a rebuild." That restore was the right call — committing a rebuild would have swapped one opaque binary for another — but it is a manual step that every agent and every human who runs matchstick locally has to remember, and forgetting it commits a binary nobody can review. After this, there is nothing to restore. If #282 merges first, this deletes the same unchanged blob and applies cleanly; if this merges first, #282's local runs write into an ignored directory and its diff is unaffected.

#289 (issue #229) and #291 (issue #227) are also in subgraph/tests/ and subgraph/src/, and likewise share no file with this. #229 in particular is about mutants the matchstick suite would not kill; this is about the suite not being run against the current sources at all, which is upstream of any question about what it covers. Neither is a fix for the other.

QA

  • Not run: matchstick. subgraph-test is docker and there is no docker here. I also did not run the matchstick binary directly, for the specific reason that doing so writes subgraph/tests/.bin/metaboard.wasm — the artifact this PR removes — and the run would have had to be undone by hand, which is the failure mode being fixed. The MetaBoard Subgraph CI lane on this branch is the authority for the matchstick half, and it is the run that proves the two claims I could not execute locally: that the missing tests/.bin is created rather than fatal, and that ARGS: -r reaches the binary without breaking the run. It has since run green on this branch and its log settles both — 💬 Compiling metaboard... then All 10 tests passed! 😎, with no skipped! line, against a checkout that contains no wasm.
  • Run locally, with forge from the nix store and yq/git on PATH: forge test --match-path 'test/subgraph/*' — 6 passed (2 new + the 4 existing SubgraphManifestTest, which shares the vm.ffi surface this PR extends). forge fmt --check clean. Not run: the rest of the forge suite (5096-run fuzz, nothing here is Solidity behavior) and the rust lanes.
  • Discriminating tests: testMatchstickBinariesAreNotInGit, testMatchstickRunForcesRecompilation — each fails on base, verified by restoring main's exact state for the file it is about and re-running: M1 restores the tracked wasm and M3 restores main's docker-compose.yml verbatim, below.
  • Mutations applied, each a full re-run of test/subgraph/SubgraphMatchstick.t.sol, baseline 2 passed:
    • M1 git checkout HEAD -- subgraph/tests/.bin/metaboard.wasm, i.e. main's exact index state -> killed by testMatchstickBinariesAreNotInGit, git ls-files returning the wasm alongside the test source. This is the mutation that IS the issue.
    • M2 drop tests/.bin from subgraph/.gitignore, keeping the deletion -> killed by the same test on git check-ignore --no-index exiting 1. Confirms the two assertions are not redundant.
    • M3 remove the environment: block from docker-compose.yml, i.e. main's exact file -> killed by testMatchstickRunForcesRecompilation on keyExistsJson.
    • M4 ARGS: -c — a real matchstick flag that is not --recompile -> killed on -c != -r. A presence-only check would have survived this.
    • M5 comment the block out as # environment: / # ARGS: -r -> killed. This is the mutation the parsed-YAML read exists for: a text search for ARGS: -r finds it and passes.
  • Oracle: matchstick's own source at tag 0.6.0 for when it recompiles and what -r means; the registry image config for rainprotocol/matchstick:main for how ARGS reaches the binary; rainix's flake.nix for what subgraph-test runs and for git/yq being in sol-build-inputs. The test's expected values are spelled as literals, not derived from the files they are checked against.
  • The wasm deletion changes no source and no manifest, so crates/, subgraph/subgraph.yaml and the published interface are untouched. REUSE.toml already blankets subgraph/**, so removing a file there needs no licensing change.
  • Category check: the issue asks two things — "whether tests/.bin should be gitignored and the committed wasm removed, and/or ARGS=-r set for the docker matchstick run". Both are done, argued above as independent rather than alternative. The issue's remaining paragraphs are the analysis, not further asks.

🤖 Generated with Claude Code

Closes #230.

`subgraph/tests/.bin/metaboard.wasm` was a tracked matchstick build,
last written in 2024, still exporting a `MetaBoard#hash` entry point
deleted from the sources in 2026. Matchstick 0.6.0 reuses that file
unless `--recompile` is passed, the wasm is absent, or an mtime says a
source is newer, so a checkout where no import lands newer than the
committed binary runs the 2024 build instead of the current mappings.

The binary is untracked and `tests/.bin` ignored, so there is nothing to
reuse and a local run cannot stage one back. `docker-compose.yml` passes
`-r` through the image's `ARGS`, so the decision never consults an mtime.

`test/subgraph/SubgraphMatchstick.t.sol` holds both, in the sol lane
rather than inside the suite a stale binary would answer for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 29 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 28c3c285-9c64-4480-9dbc-297c75fb9505

📥 Commits

Reviewing files that changed from the base of the PR and between c5a1cb0 and e318f8a.

⛔ Files ignored due to path filters (1)
  • subgraph/tests/.bin/metaboard.wasm is excluded by !**/*.wasm
📒 Files selected for processing (5)
  • CLAUDE.md
  • foundry.toml
  • subgraph/.gitignore
  • subgraph/docker-compose.yml
  • test/subgraph/SubgraphMatchstick.t.sol

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thedavidmeister
thedavidmeister merged commit e5dbf5f into main Aug 26, 2026
11 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment:

S/M/L PR Classification Guidelines:

This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed.

Small (S)

Characteristics:

  • Simple bug fixes, typos, or minor refactoring
  • Single-purpose changes affecting 1-2 files
  • Documentation updates
  • Configuration tweaks
  • Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

  • Fix typo in variable name
  • Update README with new instructions
  • Adjust configuration values
  • Simple one-line bug fixes
  • Import statement cleanup

Medium (M)

Characteristics:

  • Feature additions or enhancements
  • Refactoring that touches multiple files but maintains existing behavior
  • Breaking changes with backward compatibility
  • Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

  • Add new feature or component
  • Refactor common utility functions
  • Update dependencies with minor breaking changes
  • Add new component with tests
  • Performance optimizations
  • More complex bug fixes

Large (L)

Characteristics:

  • Major feature implementations
  • Breaking changes or API redesigns
  • Complex refactoring across multiple modules
  • New architectural patterns or significant design changes
  • Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

  • Complete new feature with frontend/backend changes
  • Protocol upgrades or breaking changes
  • Major architectural refactoring
  • Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

  • Test coverage impact: More comprehensive test changes lean toward larger classification
  • Risk level: Changes to critical systems bump up a size category
  • Team familiarity: Novel patterns or technologies increase complexity

Notes:

  • the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
  • the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
  • do not include any additional text, only the size classification
  • your assessment comment must not include tips or additional sections
  • do NOT tag me or anyone else on your comment

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.

subgraph: compiled matchstick binary tests/.bin/metaboard.wasm is committed; matchstick cache invalidation is mtime-based

1 participant