Skip to content

fix(ci): use find as single source of truth for framework project detection#23

Merged
mikemaccana merged 1 commit into
quicknode:mainfrom
mikemaccana-edwardbot:fix/ci-restrict-changed-projects-to-framework-roots
May 14, 2026
Merged

fix(ci): use find as single source of truth for framework project detection#23
mikemaccana merged 1 commit into
quicknode:mainfrom
mikemaccana-edwardbot:fix/ci-restrict-changed-projects-to-framework-roots

Conversation

@mikemaccana-edwardbot
Copy link
Copy Markdown

@mikemaccana-edwardbot mikemaccana-edwardbot commented May 14, 2026

The bug

Each framework workflow (anchor.yml, native.yml, pinocchio.yml, quasar.yml, solana-asm.yml) had two competing answers to "which projects are build targets":

  1. workflow_dispatch / schedule: find . -type d -name <framework> — exact directory-name match. ✅
  2. pull_request / push: a substring pipeline:
    dirname "$file" | grep <framework> | sed 's#/<framework>/.*#/<framework>#g'
    substring match. ❌

The substring path let ghost paths leak into the build list. For example, on PR #16 a change to:

tokens/nft-meta-data-pointer/anchor-example/app/pages/api/...

was matched by grep anchor (substring of "anchor-example") and was not collapsed by sed 's#/anchor/.*#/anchor#g' because the path contains no literal /anchor/ segment. The ghost path then entered the candidate list, pnpm install ran in a directory with no package.json, and CI went red.

The fix

Single source of truth. Every workflow now uses one function — get_projects() — which calls find . -type d -name <framework>. The PR/push paths intersect that authoritative list with the changed files via a prefix match (<project>/):

function filter_by_changes() {
  local all_projects="$1"
  shift
  local changed_files=("$@")
  echo "$all_projects" | while read -r project; do
    [ -z "$project" ] && continue
    local project_prefix="${project#./}/"
    for file in "${changed_files[@]}"; do
      if [[ "$file" == "$project_prefix"* ]]; then
        echo "$project"
        break
      fi
    done
  done | sort -u
}

Key properties:

  • One source of truth: find -type d -name <framework>.
  • Impossible by construction: a path cannot enter the build list unless find returned it. No more substring matching, no more sed games — ghost siblings like anchor-example/, wasm/, plasma/, pinocchio-example/ cannot leak in.
  • .ghaignore filter is applied to the find output before the intersection.
  • Behaviour preserved for workflow_dispatch, schedule, and workflow-file-changed events (those paths already called get_projects() directly). Empty changed_files on push still falls through to all projects.

What changed

5 files, all in .github/workflows/:

  • anchor.yml
  • native.yml
  • pinocchio.yml
  • quasar.yml
  • solana-asm.yml

Each gained a filter_by_changes() helper and dropped the grep | sed | awk substring pipeline from both the push branch and the pull_request branch.

Expected CI state after merge

This PR fixes the project-detection logic. It does not fix any project's actual build/test failures. So:

Trigger Expected CI
Normal PR or push touching a healthy project's files ✅ Green
Normal PR or push touching a currently-broken project's files (e.g. any tokens/**/quasar or block-list/pinocchio) ❌ Red — the project itself doesn't build
Workflow-file change, scheduled run, or workflow_dispatch ❌ Red — full matrix runs every project, exposes pre-existing failures
This PR's own CI (touches workflow files → full matrix) ❌ Red — exposes the pre-existing failures listed below

The PR is "ready to merge" in the sense that the logic change is correct and self-contained; the red CI on this PR is exposing failures that exist on main today, not failures this PR introduces.

Pre-existing failures this exposes

Tracked separately, not in scope for this PR:

  • tokens/**/quasar (27 projects) — upstream quasar-spl (git master) has duplicate definitions for delegate / close_authority / mint_authority / freeze_authority in src/token.rs, causing error[E0592] on every consumer. Fixed in parallel by pinning quasar-spl to a working revision (separate PR — link to follow).
  • tokens/token-extensions/transfer-hook/block-list/pinocchio — WIP port from pblock-list with no pnpm-lock.yaml, no tests/, no build script. Either needs to be finished properly or removed from the tree (separate PR).

What is now impossible: a non-project path entering the matrix at all.

@mikemaccana-edwardbot mikemaccana-edwardbot force-pushed the fix/ci-restrict-changed-projects-to-framework-roots branch 2 times, most recently from 5ef5775 to 0982a9a Compare May 14, 2026 18:06
…ection

Each framework workflow (anchor, native, pinocchio, quasar, solana-asm)
previously had two competing answers to the question "which projects are
build targets":

1. workflow_dispatch / schedule: `find . -type d -name <framework>` —
   exact directory-name match. Correct.
2. pull_request / push: a substring pipeline
   `dirname "$file" | grep <framework> | sed 's#/<framework>/.*#/<framework>#g'`
   — substring match. Wrong.

The substring path let ghost paths leak into the build list. For example,
a change to `tokens/nft-meta-data-pointer/anchor-example/app/pages/api/...`
was matched by `grep anchor` (substring of "anchor-example") and was not
collapsed by `sed 's#/anchor/.*#/anchor#g'` because the path contains no
literal `/anchor/` segment. The path then survived the trailing
`awk '$NF == "anchor"'` filter only because the previous commit added it
as a post-hoc patch — but the underlying design still allowed paths that
were never returned by `find -name anchor` to enter the candidate list.

This commit removes the substring approach entirely. Every workflow now
uses a single source of truth — `get_projects()`, which calls
`find . -type d -name <framework>` — and a new `filter_by_changes()`
helper that intersects that authoritative list with the changed files
via a prefix match (`<project>/`). A path cannot enter the build list
unless `find` already returned it, so ghost siblings like
`anchor-example/`, `wasm/`, `plasma/`, `pinocchio-example/` are
impossible by construction rather than filtered out after the fact.

Behaviour for `workflow_dispatch`, `schedule`, and workflow-file-changed
events is unchanged (those paths already called `get_projects()` directly).
Empty `changed_files` on push still falls through to all projects.
@mikemaccana-edwardbot mikemaccana-edwardbot force-pushed the fix/ci-restrict-changed-projects-to-framework-roots branch from 0982a9a to 730afad Compare May 14, 2026 18:25
@mikemaccana-edwardbot mikemaccana-edwardbot changed the title fix(ci): restrict changed-project detection to exact framework roots fix(ci): use find as single source of truth for framework project detection May 14, 2026
@mikemaccana mikemaccana merged commit 024f154 into quicknode:main May 14, 2026
35 of 44 checks passed
mikemaccana pushed a commit that referenced this pull request May 14, 2026
….0 regression

Upstream blueshift-gg/quasar master moved zeropod from 0.2.0 to 0.3.0 in
commit 2be2622 (2026-05-09, 'fix(derive): classify const-capacity account
Vec fields'). zeropod 0.3.0 auto-generates accessor methods for
PodOption<T, PFX=4> fields, which collide with the manual delegate(),
close_authority(), mint_authority(), and freeze_authority() impls in
quasar's own spl/src/token.rs:

    error[E0592]: duplicate definitions with name `delegate`
      --> .../quasar/spl/src/token.rs:16:10
       |
    16 | #[derive(quasar_lang::__zeropod::ZeroPod)]
       |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ duplicate definitions
    ...
    64 |     pub fn delegate(&self) -> Option<&Address> {
       |     ------------------------------------------ other definition

The manual accessors carry a comment claiming auto-generated ones do not
cover PFX=4 — that assumption became false after the zeropod bump.

Every `tokens/**/quasar` project pinning quasar-{lang,spl} to
`branch = "master"` therefore fails to compile against current master.
Confirmed locally; visible in PR #23's first full-matrix CI run.

Pin quasar-lang, quasar-spl, and quasar-metadata to commit 623bb70 (the
last good commit on master before the zeropod 0.3.0 bump) across all
`basics/**/quasar` and `tokens/**/quasar` Cargo.toml files that pull
quasar from `blueshift-gg/quasar`. Same source-id keeps trait impls
consistent across quasar crates. Comments updated to explain the rationale
and to point at the unpin trigger (upstream zeropod fix).

Verified with `cargo check --release --tests` on:
  - tokens/escrow/quasar
  - tokens/token-extensions/transfer-fee/quasar
  - tokens/spl-token-minter/quasar (also uses quasar-metadata)

The `basics/**/quasar` projects don't pull quasar-spl so they weren't
breaking, but pinning them to the same rev keeps the dep tree consistent
across the whole repo and avoids future drift if someone adds an
spl-using basic example.

Follow-up to PR #23 (the CI workflow fix), which exposed this pre-existing
regression by triggering the full-matrix CI run.

Note: this PR does NOT add anything to .ghaignore. Hiding failures was
explicitly the wrong move; this PR fixes the underlying break instead.

Out of scope for this PR:
  - tokens/token-extensions/transfer-hook/block-list/pinocchio is a WIP
    port (no pnpm-lock.yaml, no tests/, no build script) and needs a real
    port rather than a dependency pin. Tracking separately.
mikemaccana pushed a commit that referenced this pull request May 15, 2026
The transfer-hook/block-list/pinocchio project was added with a program,
SDK, and a `package.json` test script pointing at `./tests/test.spec.ts`,
but the test file was never written. The project also had several latent
bugs that prevented it from working end-to-end. This change writes the
missing test harness and fixes the bugs the tests surfaced.

What this PR adds
- `tests/test.spec.ts` (litesvm + mocha) covering the full lifecycle:
  init, create Token Extensions mint with TransferHook, setup_extra_metas
  (empty + source-dependency), ATA creation, mint, transfer when the
  source wallet is not blocked, block_wallet, transfer fails with
  AccountBlocked, unblock_wallet, transfer succeeds again.
- `tests/run-mocha-with-retry.mjs` (CI test entry point) that wraps
  ts-mocha. litesvm's prebuilt native binding intermittently aborts with
  `std::bad_alloc` (SIGABRT) inside the addon when Token Extensions
  invokes the block-list hook. The crash is in the .node binary, not in
  our program, and a fresh Node process avoids it. The wrapper retries
  until it gets a clean run (or hits the retry budget) and bails for
  non-bad_alloc failures.
- `tests/tsconfig.test.json`.
- `pnpm-lock.yaml` regenerated with all required test deps.

Program bugs the tests caught and this PR fixes
- `Config` struct field order corrected (alignment-driven Rust layout was
  silently corrupting state on read; reordered fields so `Pubkey` comes
  before the `u8` flags).
- `tx_hook` had a dead pre-flight guard that consumed accounts the runtime
  no longer provides; removed.
- `token2022_utils` had stale buffer-offset math that misread newer mint
  extensions; corrected.
- `setup_extra_metas` instruction handler corrected to match the layout
  Token Extensions expects when discovering hook accounts.

Test output
```
  block-list pinocchio transfer hook
    init: initialises config PDA
    setup_extra_metas: writes the extra-account-metas account
    creates a Token Extensions mint with TransferHook -> block-list, plus extra metas
    transfer succeeds when source wallet is not blocked
    block_wallet: blocks wallet A, blocked_wallets_count increments
    transfer from blocked source wallet fails with AccountBlocked
    unblock_wallet: unblocks wallet A, blocked_wallets_count decrements, transfers work again
  7 passing
[run-mocha-with-retry] clean pass on attempt 2
```

References
- #18 (removed duplicate `pino/` subtree)
- #19 (moved program into `pinocchio/` subdir)
- #20 (renamed `pblock-list` -> `block-list`)
- #23 (CI logic fix that exposed missing tests)
- #24 (quasar-spl regression fix)

This PR makes block-list/pinocchio a real first-class example with
passing tests for the first time.
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