ci(codeql): run analysis on beta - #146
Merged
Merged
Conversation
The "Main Branch Protection" ruleset covers main and beta, and its code_scanning rule requires CodeQL results. CodeQL only triggered on main, so PRs into beta were blocked waiting on an analysis that never ran.
|
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: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
pepebndc
approved these changes
Aug 27, 2026
0xisk
added a commit
that referenced
this pull request
Sep 1, 2026
* ci(codeql): run analysis on beta (#146) The "Main Branch Protection" ruleset covers main and beta, and its code_scanning rule requires CodeQL results. CodeQL only triggered on main, so PRs into beta were blocked waiting on an analysis that never ran. * Bump deps to prerelease, add ecdsa tests (#145) * add tmp runtime dep * update to 0.18 runtime * wire sim to 0.18-rc/ledger-9 stack * fix fmt * add ecdsa tests * add pre-release compact install * fix(simulator): clear the #145 review comments (#147) * fix(simulator): pin simulated block time to zero compact-runtime 0.18 defaults `createCircuitContext`'s `time` to wall-clock, where the hand-rolled 0.16 `QueryContext` left it at 0. Any suite reading `kernel.blockTime()` silently became non-reproducible. Threads an explicit `time` from `BaseSimulatorOptions` through to the runtime factory, defaulting to 0 to restore the previous behaviour. * refactor(simulator): drop the dead circuit-context helpers `useCircuitContext` and `useCircuitContextSender` had no callers and were never exported from the barrel, so they were unreachable before this migration and got hand-ported to the 0.18 call-tree shape untested. * fix(simulator): guard context reads before init() 0.18 split construction from state building, so a caller that skipped `init()` got `Cannot read properties of undefined (reading 'callContext')`. Both `CircuitContextManager.context` and the simulator's `contractAddress` now throw a named error instead. * fix(simulator): declare initialState async on IMinimalContract The canonical interface still promised the sync-only 0.16 return type, so anything typed against it got the wrong shape and `CircuitContextManager` carried a private redeclaration to compensate. * fix(simulator): typecheck the test tree `tsconfig.types.json` compiled `src/**` only, and its `exclude` named a `tests` directory that does not exist, so nothing verified the test tree. Adding a project that covers it surfaced 74 errors, of which the load-bearing one is in `src/`: 0.18 circuits return `Promise<CircuitResults>`, which the `ContextlessCircuits` / `AsyncCircuits` conditionals did not match, collapsing every entry on `circuits.pure` and `circuits.impure` to `never`. * src: match the promised circuit shape; keep the anonymous class's address field underscore-public for declaration emit * test: give relative imports the `.js` extension nodenext requires * wiring: run both projects under `yarn types`, and let turbo see the test tree so a regression is not cached away * fix(simulator): build the setContext fixture as a real 0.18 context The replacement context was a hand-rolled literal in the pre-0.18 flat shape, so `setContext`'s only test asserted against a context the runtime would reject. `toEqual` compared two plain objects and passed anyway. Rebuilds it through `createCircuitContext` and asserts identity plus the two fields the test is about. Exporting `BackendDeps` is required for the same reason: it appears in `create`/`_create`, so an exported binding assigned from `createSimulator(...)` could not name it (TS4023). * fix(ci): make the prerelease compiler input functional and verified * `COMPACTC_VERSION` is now exported to `$GITHUB_ENV`. `VER` was step-scoped, so `test/setup.ts` never saw it: overriding the input installed one toolchain while the fixtures compiled with the hardcoded default. * The archive is checked against a pinned SHA-256 before it is unpacked and executed. The release publishes no checksum file, so the digests are pinned per platform here; overriding the version requires passing its digest. * fix(simulator): key the fixture cache on the compiler The staleness check compared artifact mtime against source mtime only, so an existing clone kept its 0.31-compiled artifacts and hit a load-time rejection against the 0.18 runtime. CI never saw it: the `test` turbo task is uncached and the checkout is clean. Artifacts now carry a stamp of the compiler version and the zkir flag, and recompile when it does not match. The flag also moves onto the fixture list, replacing a `secp256k1` regex over the source that a mention in a comment was enough to trip. * test(simulator): collapse the field-to-bytes helpers Two near-identical copies existed in the test tree. The one in `SampleZOwnable.test.ts` silently truncated where the runtime helper it replaced threw, so a wrong value would read as a passing assertion. Keeps one in the shared test utils, with the range check restored, and derives `zeroUint8Array` from it. `Signers.ts` keeps its own copy: `src` cannot import from `test`. * test(simulator): consume the generated secp256k1 types The `Ecdsa.compact` re-export is live: the artifact's `index.d.ts` declares both `Secp256k1EcdsaSignature` and `Secp256k1Point`. The simulator hand-rolled the signature struct and took the point from the runtime instead, so a change to the generated shape would not surface here. * docs(simulator): correct the sync-era comments The async migration left the docs describing a synchronous engine: the dry primitive called "synchronous", `DryBackend` said it wraps a synchronous result in a resolved promise and preserves dry behaviour "byte-for-byte", and the `SyncSimulator` name now describes nothing. Renaming the exported type is a breaking change, so it keeps the name with a `@remarks` note. Also records that `copyCircuitContext` is `@internal` upstream. * ci(turbo): declare COMPACTC_VERSION on the test task The setup action exports it and `test/setup.ts` reads it, but the task never declared it. Pass-through happens to work on turbo 2.9, so this only pins the behaviour against a stricter env mode. * fix(ci): compile fixtures before the type check `yarn types` now covers `test/**`, which imports the generated artifacts, but `checks.yml` runs it before `yarn test` — the only thing that compiled them. It passed locally only because the artifacts were already on disk. Lifts fixture compilation into its own turbo task that `types` depends on. `test/setup.ts` gains a direct entry point so the task does not have to go through vitest; vitest's `globalSetup` still calls the same function. * ci: add a beta release channel (#150) * ci: add a beta release channel Prerelease versions publish under the `beta` npm dist-tag, stable ones under `latest`. The publish workflow derives the tag from the version string alone, so a prerelease cannot take over `latest` no matter which ref triggered the run. Yarn's `version` has no `--preid`, so a pre* strategy off a stable version lands a bare counter (0.3.1 -> 0.3.2-0). The bump step relabels that once to `-beta.0`; yarn carries the identifier forward on every later prerelease. * docs: add a beta graduation cycle example * build: upgrade to compact toolchain 0.34.0 (#152) 0.34.0 is the first stable release carrying the secp256k1 primitives and ledger 9, so the prerelease-compiler bridge in the setup action is no longer needed. `setup-compact-action` passes the version straight to `compact update`, which serves 0.34.0. Generated artifacts assert the runtime minor at load time, so the compact-runtime pin moves 0.18.0-rc.1 -> 0.19.0 in lockstep with the compiler. midnight-js goes to 5.0.0-beta.7 to stay on the current beta line. 0.34.0 makes `Secp256k1Point` an internally handled type that a contract can no longer re-export. The Ecdsa fixture drops it from its export list and `EcdsaSimulator` sources the type from compact-runtime, which is where the generated artifact now references it. * fix(simulator): async proxy types and live address binding (#154) * fix(simulator): type circuit proxies as promise-returning The 0.18 migration made every circuit proxy async but left ContextlessCircuits mapping to the bare result, so a consumer typed `boolean` received `Promise<boolean>` and any truthiness check passed vacuously. The cast on the Proxy return masked the mismatch from tsc. * Circuit.ts: map ContextlessCircuits to `Promise<R>`; AsyncCircuits becomes an alias so the two cannot drift. * create-contract.type-test.ts: pin the promise mapping for both the 0.18 promise shape and the 0.16 sync shape; fails on the old mapping. * createDrySimulator.ts: narrow the init guard to `=== undefined` and note why the private-state cast is sound. * AbstractSimulator.ts: same cast note. * fix(simulator): bind live pure evaluation to deployed address In live mode the local evaluator was built before the LiveContext resolved, so it kept the dummy address in its circuit context while the public simulator reported the deployed one. Resolve the context first, seed the evaluator with the deployed address, and reject an explicit contractAddress that disagrees with it. The deployed address now reaches the runtime, so a live harness must supply a parseable one; the mock worlds in the tests move from a made-up string to dummyContractAddress(). * docs(release): hyphenate and align prerelease wording --------- Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com>
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.
Types of changes
CodeQL only triggered on
main, but the "Main Branch Protection" ruleset coversmainandbeta, and itscode_scanningrule requires CodeQL results.Every PR into
betatherefore sits atBLOCKEDwaiting on an analysis thatnever runs. #145 is the current case: semgrep and Run Checks pass,
Analyzeissimply absent.
checks.ymlwas unaffected because itspull_requesttrigger has no branchfilter.
PR Checklist
Further comments
branches: [main, beta]rather than dropping the filter, so analysis scopetracks the ruleset instead of running on every branch.
Merging this does not retroactively unblock #145:
pull_requestruns use theworkflow from the merge commit, so #145 needs an "Update branch" (or any push)
afterwards for
Analyzeto fire.Release workflows need no equivalent change.
release.ymlis dispatch-only andbases everything on
github.ref_name, andpull_request.branchesfilters thebase ref, so
release/*heads are already covered by whichever base they target.