Skip to content

ci: add surfpool integration workflow#253

Closed
shahan-khatchadourian-anchorage wants to merge 1 commit into
shahankhatch/surfpool_testingfrom
shahankhatch/surfpool-ci-workflow
Closed

ci: add surfpool integration workflow#253
shahan-khatchadourian-anchorage wants to merge 1 commit into
shahankhatch/surfpool_testingfrom
shahankhatch/surfpool-ci-workflow

Conversation

@shahan-khatchadourian-anchorage
Copy link
Copy Markdown
Contributor

Summary

  • Adds .github/workflows/surfpool-solana.yml triggered by the surfpool PR label
  • Runs surfpool_fuzz_all_idls.sh against all 15 embedded IDLs
  • Applies surfpool-failure label on failure, removes it on success
  • Requires HELIUS_API_KEY repo secret for reliable mainnet RPC access

Stacked on #202.

Test plan

  • Verify workflow triggers when surfpool label is applied to a PR
  • Confirm surfpool-failure label is applied/removed based on test outcome
  • Ensure HELIUS_API_KEY secret is configured as a repo secret

🤖 Generated with Claude Code

Runs surfpool_fuzz_all_idls.sh when the 'surfpool' label is applied to a
PR. Applies 'surfpool-failure' label on failure, removes it on success.
Requires HELIUS_API_KEY repo secret for reliable mainnet RPC access.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new GitHub Actions workflow to run Surfpool-based Solana integration fuzzing for PRs labeled surfpool, and automatically manage a surfpool-failure label based on the result.

Changes:

  • Introduces .github/workflows/surfpool-solana.yml to run ./scripts/surfpool_fuzz_all_idls.sh when a PR has the surfpool label.
  • Installs required tooling (Rust toolchain, protoc, surfpool), runs codegen, and executes the surfpool test script.
  • Adds/removes the surfpool-failure label depending on the test step outcome.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/surfpool-solana.yml
Comment thread .github/workflows/surfpool-solana.yml
Comment thread .github/workflows/surfpool-solana.yml
Comment thread .github/workflows/surfpool-solana.yml
@prasanna-anchorage
Copy link
Copy Markdown
Contributor

Dependency check before this lands — none of the workflow's runtime prerequisites currently exist on main:

Dependency Status
Trigger label surfpool (workflow's if: contains(github.event.pull_request.labels.*.name, 'surfpool')) ❌ does not exist on the repo
Script ./scripts/surfpool_fuzz_all_idls.sh ❌ not on main — added by #202 (still draft)
Secret HELIUS_API_KEY ❌ not in repo or org GitHub Actions secrets

Net effect of merging as-is: workflow registers but never runs in practice (label gate). If anyone did hand-add a surfpool label, the job would fail at Run surfpool integration tests (No such file or directory: ./scripts/surfpool_fuzz_all_idls.sh). The continue-on-error: true on that step would mask it as outcome=failure and the next step (Label PR on surfpool failure) would attempt to remove a (also-nonexistent) surfpool-failure label — squashed by || true, so the job goes green. End result: green CI signaling that surfpool ran cleanly, when it actually never ran. That's worse than no workflow at all.

Recommend either:

  1. Land test: surfpool mainnet-fork integration for built-in IDL fuzz path #202 first (which adds the script + integration), then create the surfpool label and add HELIUS_API_KEY to org secrets, then merge this.
  2. Fold this workflow into test: surfpool mainnet-fork integration for built-in IDL fuzz path #202 so the prerequisites land atomically and we never have a window where the workflow lies about its own coverage.

Either way, also worth replacing the continue-on-error: true + manual-label-toggle pattern with explicit failure on the surfpool step itself — silent green is the worst CI failure mode.

shahan-khatchadourian-anchorage added a commit that referenced this pull request May 8, 2026
The workflow toggles a `surfpool-failure` label on PRs. With the prior
`if: contains(labels, 'surfpool')` guard, that toggle re-fires the
workflow on the same PR (since the `surfpool` label is still present
when the `surfpool-failure` label is added). Any other `labeled` event
on a PR that happens to carry the `surfpool` label would also re-trigger
unnecessarily.

Match the pattern used in `.github/workflows/stagex.yml`: on `labeled`
events only run when the label being added is `surfpool`; on other
events (`opened`, `synchronize`, `reopened`) require the label to
already be present.

Addresses copilot review comment on #253 (which proposed the same
guard); folded here so #253 can be closed with the workflow's review
comments resolved on this PR.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@shahan-khatchadourian-anchorage
Copy link
Copy Markdown
Contributor Author

Closing as superseded by #202.

The workflow file is now part of #202 and has gone further than the original here:

No content from this branch is needed.

🤖 Generated with Claude Code

prasanna-anchorage pushed a commit that referenced this pull request May 12, 2026
The workflow toggles a `surfpool-failure` label on PRs. With the prior
`if: contains(labels, 'surfpool')` guard, that toggle re-fires the
workflow on the same PR (since the `surfpool` label is still present
when the `surfpool-failure` label is added). Any other `labeled` event
on a PR that happens to carry the `surfpool` label would also re-trigger
unnecessarily.

Match the pattern used in `.github/workflows/stagex.yml`: on `labeled`
events only run when the label being added is `surfpool`; on other
events (`opened`, `synchronize`, `reopened`) require the label to
already be present.

Addresses copilot review comment on #253 (which proposed the same
guard); folded here so #253 can be closed with the workflow's review
comments resolved on this PR.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
prasanna-anchorage pushed a commit that referenced this pull request May 12, 2026
)

* feat: add solana_test_utils crate with SurfpoolManager

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test: add surfpool mainnet-fork integration test for Solana parser

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

* feat: add idl-meta tool for Python-free IDL metadata extraction

* refactor: replace python3 with idl-meta in fuzz_all_idls.sh

* test: add surfpool_fuzz_all_idls.sh for mainnet-fork IDL testing

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: update SurfpoolManager for current surfpool CLI

- Use `surfpool start` subcommand with correct flags (--port, --ws-port,
  --rpc-url, --no-tui, --ci) instead of obsolete top-level flags
- Remove ledger_path/reset_ledger config (not surfpool concepts)
- Use multi_thread tokio runtime in tests (required by solana-rpc-client)
- All 15 IDLs pass surfpool integration tests

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

* ci: add surfpool fuzz workflow gated on `surfpool` label

Wires the existing `HELIUS_API_KEY` repo secret into a label-gated CI job
that runs `scripts/surfpool_fuzz_all_idls.sh` against all 13 embedded IDLs.
The Solana parser's `SurfpoolConfig::default()` already prefers
`HELIUS_API_KEY` over the public mainnet endpoint, so passing the secret
through the job env is sufficient -- no code changes required.

Modelled on `fuzz-solana.yml` and `proptest-solana.yml`: label-gated to
control RPC quota usage, caches `~/.cargo/bin/surfpool` to skip the
~minute-long install on subsequent runs, and tags the PR with
`surfpool-failure` on a red run.

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

* ci(surfpool): pin surfpool to v1.1.1, skip fork PRs, harden install gate

Addresses code review on the prior commit:

1. Pin surfpool to tag v1.1.1 (matches the version verified locally
   against the Helius mainnet fork) and key the cached binary on the
   pinned version. The previous cache used `src/Cargo.lock` hash, which
   is decoupled from surfpool's upstream and would silently pin CI to
   the first-cached binary indefinitely. Splitting the binary cache from
   the workspace cache also lets each invalidate independently.

2. Skip the job on fork PRs (head repo != base repo). Secrets are not
   passed to fork PRs, so HELIUS_API_KEY would be empty and the labeling
   step would lack `gh pr edit` permissions; both would silently fall
   back to flaky public mainnet and a no-op label step.

3. Use `surfpool --version` instead of `command -v surfpool` to gate
   reinstall. A corrupted cached binary (interrupted previous install,
   ABI mismatch after toolchain bump) is now detected and rebuilt.

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

* fix(surfpool): unblock Tokio worker, surface test diagnostics, fix CI install

Three fixes addressing outstanding review comments and the failing CI:

1. CI install command -- the `surfpool` package was renamed to
   `surfpool-cli` upstream, so `cargo install surfpool --tag v1.1.1`
   fails with "could not find `surfpool` ... with version `*`". Drop the
   package name argument so cargo uses the workspace's `default-members`
   (`crates/cli`), which produces the `surfpool` binary regardless of
   future package renames within the workspace.

2. `wait_ready` no longer blocks a Tokio worker. The synchronous
   `RpcClient::get_version()` is now dispatched via `spawn_blocking` and
   the inter-attempt delay uses `tokio::time::sleep` instead of
   `thread::sleep`. Without this, a stalled RPC could starve other tasks
   on the same multi-thread runtime.

3. `surfpool_fuzz_all_idls.sh` now prints the captured cargo output on
   failure (both the no-summary and `N failed` paths). Previously a red
   IDL row printed only `FAIL (...)` and the diagnostics were silently
   dropped, making CI failures hard to debug.

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

* ci(surfpool): pin to commit SHA instead of tag

Tags are mutable -- a force-push could silently change what we install,
and `--locked` only protects transitive deps (it uses the source's
Cargo.lock; it does not lock the source commit). Pin to the SHA
`v1.1.1` resolved to (d58df4c) so the install target is bit-for-bit
reproducible. Cache key now keys on the rev.

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

* ci(surfpool): cap lints to warn during third-party install

The toolchain action exports `RUSTFLAGS: -D warnings` by default, which
made the `cargo install` step fail on a benign `unused_parens` lint in
upstream `surfpool-gql` v1.1.1. Deny-warnings is the right policy for
our own code but wrong for installing a third-party binary at a pinned
revision -- whether it compiles cleanly under future rustc is not
something we want CI to gate on.

Set `RUSTFLAGS=--cap-lints=warn` for just the install step so upstream
warnings stay warnings without affecting any subsequent step.

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

* ci(surfpool): use upstream prebuilt binary instead of cargo install

Replaces the ~10-minute `cargo install` compile with a ~5-second curl
of the upstream release asset. Pins the tarball's SHA256 in the workflow
so a force-pushed release asset cannot silently change what we install.

The binary is dropped into `~/.local/bin/` (added to `GITHUB_PATH` for
subsequent steps). Cache is keyed on (version, sha256) so a checksum
bump invalidates automatically.

Drops `RUSTFLAGS=--cap-lints=warn` from the install step; that was only
needed to work around upstream lint regressions during compile.

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

* ci(surfpool): guard against label-event re-trigger loops

The workflow toggles a `surfpool-failure` label on PRs. With the prior
`if: contains(labels, 'surfpool')` guard, that toggle re-fires the
workflow on the same PR (since the `surfpool` label is still present
when the `surfpool-failure` label is added). Any other `labeled` event
on a PR that happens to carry the `surfpool` label would also re-trigger
unnecessarily.

Match the pattern used in `.github/workflows/stagex.yml`: on `labeled`
events only run when the label being added is `surfpool`; on other
events (`opened`, `synchronize`, `reopened`) require the label to
already be present.

Addresses copilot review comment on #253 (which proposed the same
guard); folded here so #253 can be closed with the workflow's review
comments resolved on this PR.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

3 participants