Repository navigation
fix(genvm): support genvm-manager, and reuse the harness-built GenVM - #19
Merged
Merged
Conversation
The linter resolved bundles from the retired genvm repo, imported get_schema from a path the v0.3 SDK no longer has, and failed to recognise gl.contract.Contract so every v0.6 contract silently skipped all lint rules.
The E2E harness builds GenVM once and exports GENVM_PREBUILT_DIR, but the linter only read GENVMROOT and its lookup expected unpacked source dirs, so it downloaded its own bundle and validated against a different GenVM than the one under test.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Nothing in CI ran the 93 unit tests, which is why three breaks shipped. Adding the job surfaced that stub generation binds the whole (paths, notes) tuple, so it always raised TypeError on the first path join.
The repo declares select=[E,F,I,W] but has never enforced it, so 26 violations have accumulated across files this branch does not touch. Enforcing it belongs in its own change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits. The first makes the linter work at all against GenVM v0.6; the second stops it downloading its own GenVM when one has already been built for it.
1. The linter was stranded on the retired
genvmrepoThree independent breaks, each verified against source and against a real
genvm-manager v0.6.0-rc1bundle:Bundle source hardcoded. Three URLs pointed at
genlayerlabs/genvm, whose newest non-prerelease isv0.3.0-rc7. That bundle carries py-genlayer hashes1jb45aa8…/1zr6nqk5…but not the v0.6 hash9b8kjyda…, which is why Studio's lint job failed withfilename 'runners/py-genlayer/9b/8kjyda….tar' not found. There was no way to override the repo — only the version — so this could not be fixed by configuration. Now a singleGENVM_REPOconstant (env-overridable) defaulting togenlayerlabs/genvm-manager. Both manager tags are GitHub prereleases (their names contain a hyphen), so prerelease handling and a reachableFALLBACK_VERSIONcame with it.The SDK moved.
from genlayer.py.get_schema import get_schema— the v0.3+ SDK hasgenlayer/_internal/get_schema.pyand nogenlayer.pypackage at all. Now tries the new path and falls back to the old, so both eras load. The guard inspectsexc.nameso a genuinely broken new SDK still raises instead of being masked.Silent false-pass — the worst of the three. v0.6 contracts declare
class Storage(gl.contract.Contract), a nestedast.Attribute. All three subclass detectors requiredbase.valueto be anast.Name, so no v0.6 contract was ever recognised as a contract: rules E011–E022 and the contract-scoped safety rules were skipped entirely andgenvm-lint lintreported clean while checking nothing. Now one sharedlint/ast_utils.pyhelper used at all three sites, matchingContract,gl.Contract,gl.contract.Contract,genlayer.Contract,genlayer.contract.Contract.Also: cached bundles are now namespaced per repository. Versions are not unique across repos and their contents differ, so an unqualified cache let an old-repo bundle satisfy a manager request — which would have silently defeated the repoint for every existing user.
2. It downloaded its own GenVM instead of using the one already built
The E2E harness builds GenVM once per run and exports
GENVM_PREBUILT_DIRinto the linter's subprocess. The linter never read that variable — onlyGENVMROOT. And theGENVMROOTpath was itself dead against modern trees: it looked for unpackedrunners/<type>/srcdirectories, but a real GenVM root containsrunners/<type>/<2>/<rest>.tartarballs. So it silently fell through and fetched its own ~310MB bundle, validating contracts against a different GenVM than the one under test.Adds explicit source modes, precedence prebuilt > release:
prebuiltGENVM_PREBUILT_DIR(orGENVMROOT, kept as an alias) pointing at a usable treereleaseGENVM_SOURCE_MODEselects explicitly. Whenprebuiltis declared and the tree is unusable, the build hard-fails with an actionable message rather than quietly downloading — that is what makes "the harness said prebuilt" verifiable. With the mode unset, behaviour is unchanged: use the tree if usable, else download.Tree resolution handles both on-disk layouts (
runners/…, falling back toexecutor/*/legacy-runners/…), so v0.5-era contracts still resolve from a manager tree. A candidate root is validated the way the harness validates it —bin/genvm-modulesandrunners/— so a half-populated directory is rejected rather than half-used.Verification
93 unit tests pass (86 before). Verified end-to-end against a real GenVM root and a real Studio contract pinned to
9b8kjyda…:✓ Validation passedwith no download at all, exit 0✗ Validation failed … missing bin/genvm-modules, exit 1, no downloadPublishing
publish.ymltriggered only onpushtomain, butmainauto-mirrors the active dev branch (v0.11-dev, version0.10.0) while releases come from the stable branch (v0.11, version0.11.0). A fix merged intov0.11therefore never reachedmainand never published — and had it reachedmain, the0.10.0version there would have been skipped as already-published. Now triggers on the stablev[0-9]+.[0-9]+branches. The already-published guard is kept; it was correct, just fed the wrong branch.No version bump here — choosing the next version is a human call.
Companion
Pairs with genlayerlabs/genlayer-e2e#669, which makes the harness declare
GENVM_SOURCE_MODE=prebuilt. Safe in either order: an older linter ignores the variable, and this linter without the harness change simply uses auto-detect.Supersedes the detection half of #18 (which widens only one of the three sites, and still requires an
ast.Name, so it does not matchgl.contract.Contract). Left open for its author to close.3. Nothing in CI ran the tests
There was no workflow running this repo's own test suite — only branch-policy, retarget, fast-forward and the cross-repo E2E gate. And the E2E scenario runs
pytest tests/test_artifacts.py, whose every case monkeypatchesurllib.request.urlopen/_download_to, so nothing is downloaded, extracted, or linted for real.That is why all three breaks above shipped: no automated check on this repo could have caught them.
Adds a
Testsworkflow on Python 3.10 (therequires-pythonfloor) and 3.12, so a version-specific regression cannot hide behind a single interpreter.Adding it immediately exposed a live crash.
stubs.pybound the whole(paths, notes)tuple returned byextract_sdk_paths— the only one of four call sites that did not unpack it:So
genvm-lint stubsraisedTypeErrorevery time, and the guard meant to catch "no paths" could never fire. Fixed, with a regression test verified to fail against the pre-fix code. This is the VS Code extension's stub-generation path.Deliberately not in this PR
The repo declares
select = ["E", "F", "I", "W"]inpyproject.tomlbut has never enforced it, so 26 violations have accumulated (19E501, 7I001) across files this branch does not touch. I left the lint step out rather than bury a GenVM change under unrelated line-wrapping, or land a red check. Worth its own cleanup PR — the config is already there, it just needs running.