Skip to content

feat(e2e): add apps-e2e shared workflow + composite actions#43

Open
MaximusHaximus wants to merge 1 commit into
mainfrom
feat/apps-e2e-shared-workflow
Open

feat(e2e): add apps-e2e shared workflow + composite actions#43
MaximusHaximus wants to merge 1 commit into
mainfrom
feat/apps-e2e-shared-workflow

Conversation

@MaximusHaximus

@MaximusHaximus MaximusHaximus commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Consumer-side kurtosis-pos e2e harness, factored into 0xPolygon/pipelines so adopters call one shared implementation instead of vendoring copies that drift. lst-api migrates onto it in a stacked PR (0xPolygon/lst-api#125); matic.js and proof-generation-api are the next candidates.

  • apps-e2e.yml — reusable workflow. Snapshot mode (default, ~30s) restores a published GHCR image via the e2e-snapshot-restore composite; cold-start mode (~10m) runs kurtosis from source via e2e-cold-start for kurtosis-pos refs that haven't been snapshotted. Runs the consumer's e2e_command, dumps devnet state on failure, uploads a 7-day artefact. Optional REGISTRY_USERNAME / REGISTRY_PASSWORD secrets let a cross-org consumer pull a private snapshot with its own credentials, falling back to the workflow actor + GITHUB_TOKEN when unset.
  • actions/e2e-snapshot-restore — pull + restore a snapshot image (extract volumes, patch the pos-anvil host port, replay anvil state). Bundles its restore.sh next to action.yml, referenced via ${{ github.action_path }}.
  • actions/e2e-cold-start — install kurtosis-cli, scale EL resource limits down to fit ubuntu-latest, run the enclave from source. Exports E2E_KURTOSIS_ENCLAVE so a consumer can write a single env-driven adapter across both modes.
  • actions/e2e-dump — capture devnet state on failure (compose logs or kurtosis enclave dump). A composite rather than an inline run: block or a .github/scripts/ helper, because a reusable workflow can't reach a repo-local script when it runs in a consumer's checkout.
  • apps-e2e-trigger.yml — canonical thin trigger for consumers to copy, guarded against firing on pipelines' own PRs.
  • README — workflow + composite tables and usage notes.

use_snapshot boolean handling

The snapshot/cold-start switch resolves via toJSON(inputs.use_snapshot) == 'false'. GitHub Actions' ==/!= use loose equality where null == false evaluates to true, so a naive inputs.use_snapshot == false would also match the null inputs that pull_request events carry and silently drop label-triggered PR runs into the slow cold-start path. Stringifying via toJSON first ("null" / "false" / "true") makes the comparison unambiguous; snapshot stays the strong default and only an explicit boolean false flips to cold-start.

Review feedback (all resolved)

Addressing the API review on this PR plus the automated review:

  • Trigger guarded with github.repository != '0xPolygon/pipelines' so a run-e2e label here doesn't spawn a run with no pnpm run e2e.
  • e2e-snapshot-restore uses docker create --entrypoint /bin/true so the addresses-sidecar extraction overrides any ENTRYPOINT a future snapshot declares.
  • Cold-start verifies the EL resource scale-down actually applied (greps the override marker, fails loud) rather than silently OOM-killing on a ref that renamed the vars.
  • Temp files in the anvil-state replay fold into the cleanup trap (no leak on a curl failure).
  • Registry secrets surfaced + forwarded (cross-org private-snapshot pull).
  • secrets: inherit recommendation removed (prohibited by Team Standards); the dual-path adapter contract and the named-secrets-only limitation are documented on the e2e_command input.

Deferred as design polish (tracked for follow-up, not blocking adoption): runner / checkout_fetch_depth inputs, jq-over-python3, install-after-composite ordering, enclave-name parameterisation, required snapshot_image. Generic suite-secret injection stays out — named secrets only.

Out of scope

The snapshot publisher stays in lst-api against its GHCR namespace; moving and renaming it is a separate refactor once a second consumer hits a publisher-side bug.

Validation

Validated cross-repo against lst-api#125 before this branch was cleaned up:

  • Snapshot mode via run-e2e label — restore + suite green end-to-end.
  • Cold-start mode via dispatch — e2e-cold-start brought kurtosis up cleanly; the suite failure there is the documented pre-block-256 eth_getLogs limitation (consumer-side), not the workflow.
  • Failure path — on-failure dump produced a devnet-dump-<run-id> artefact.

Cross-repo refs were pinned to this feature branch during validation, then reverted to @main. History has since been squashed to a single commit on top of current main.

Comment on lines +83 to +88
run: |
# The consuming repo's e2e suite reads this env var to switch from the
# kurtosis CLI path to the sidecar JSON path. Writing to GITHUB_ENV
# propagates it to every subsequent step in the calling job without
# the consumer having to know the magic name.
echo "E2E_SNAPSHOT_ADDRESSES_JSON=${ADDRS_PATH}" >> "$GITHUB_ENV"
- name: Restore from snapshot
if: env.E2E_USE_SNAPSHOT == '1'
id: snapshot
uses: 0xPolygon/pipelines/.github/actions/e2e-snapshot-restore@main

- name: Cold-start kurtosis
if: env.E2E_USE_SNAPSHOT == '0'
uses: 0xPolygon/pipelines/.github/actions/e2e-cold-start@main
Comment thread .github/workflows/apps-e2e.yml Fixed
Comment thread .github/workflows/apps-e2e.yml Fixed
Comment thread .github/workflows/apps-e2e.yml Fixed
Comment thread .github/workflows/apps-e2e.yml Fixed
Comment thread .github/workflows/apps-e2e.yml Fixed
Comment thread .github/workflows/apps-e2e.yml Fixed
Comment thread .github/workflows/apps-e2e.yml Fixed
@sonarqubecloud

sonarqubecloud Bot commented May 1, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
3 Security Hotspots
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@MaximusHaximus MaximusHaximus marked this pull request as ready for review May 1, 2026 16:06
@claude

claude Bot commented May 1, 2026

Copy link
Copy Markdown

Code Review

2 issues found. Inline comments posted on .github/workflows/apps-e2e.yml.

Issue Location Type
Comment recommends secrets: inherit to consumers apps-e2e.yml L14-17 CLAUDE.md / Team Standards
Non-trivial inline shell in dump step apps-e2e.yml L134-144 Team Standards

No bugs found. Shell scripts are well-written with proper error handling, quoting, and cleanup traps.

@claude

claude Bot commented May 1, 2026

Copy link
Copy Markdown

Code Review - Inline Details

Issue 1: secrets: inherit recommended in comment (CLAUDE.md / Team Standards violation)

Location: apps-e2e.yml L14-17

The comment at line 16-17 instructs consumers to call this workflow with secrets: inherit. This is explicitly prohibited by Team Standards and CLAUDE.md. Team Standards: Never secrets: inherit -- always pass by name. CLAUDE.md: Declare all required secrets in the workflow_call: secrets: block so callers pass them explicitly. The workflow needs no caller-supplied secrets (GITHUB_TOKEN flows automatically via packages: read). Fix: remove the secrets: inherit phrase from this comment.

Issue 2: Non-trivial inline shell in workflow YAML (Team Standards violation)

Location: apps-e2e.yml L134-144

The "Dump devnet on failure" step has ~10 lines of branching shell logic (nested if/else with multiple docker compose and kurtosis commands) directly in the YAML run: block. Team Standards: Keep workflow YAML thin -- put logic in scripts. Any non-trivial shell logic (more than a single command) must live under .github/scripts/ or .github/actions/<name>/ so it can be run and debugged locally. The rest of the workflow correctly delegates to shell scripts (restore.sh, cold-start.sh); this dump step should follow the same pattern. Fix: extract to a script (e.g. .github/scripts/dump-devnet.sh) and invoke from this step as a single command.

No bugs found. Shell scripts are well-written with proper error handling, quoting, and cleanup traps.

# they live upstream and we'd rather pin to a known kurtosis-pos commit than
# vendor copies that silently drift from the snapshot they're paired with.
echo "[restore] cloning kurtosis-pos@${KURTOSIS_POS_REF} into ${WORK_DIR}"
git clone --depth 1 --branch "$KURTOSIS_POS_REF" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

--branch wouldn't work with a commit SHA

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Neat

@MaximusHaximus

Copy link
Copy Markdown
Contributor Author

API review: is it sane, and flexible enough for proof-generation-api / pos-airdrop / matic.js?

Reviewed the workflow + composite interfaces against the lst-api consumer side (#125) and the three candidate adopters. The call surface is sane and the right shape, but it's currently sized exactly for lst-api's existing adapter. Two flexibility gaps block clean matic.js / pos-airdrop adoption, plus a handful of correctness/footgun issues worth fixing in the same PR.

Correctness bugs

  1. The canonical trigger fires on pipelines's own PRs. apps-e2e-trigger.yml ships with a live pull_request: types: [labeled, synchronize, reopened] block. Apply the run-e2e label to any pipelines PR (or push to a labelled one) and it spawns an apps-e2e.yml@main run that has no pnpm run e2e to execute against pipelines. Unlike the other apps-*-trigger.yml files, this one has no legitimate use against pipelines itself. Either relocate it to docs / apps-team-ts-template/.github/workflows/, or guard with if: github.repository != '0xPolygon/pipelines'.

  2. restore.sh step 3 — docker create "$IMAGE" /bin/true won't reliably run /bin/true if the snapshot image declares an ENTRYPOINT — the args get appended as CMD to the entrypoint instead of overriding it. Use docker create --entrypoint /bin/true "$IMAGE". Silently breaks the addresses-sidecar extraction on any future snapshot rebuilt with an entrypoint.

  3. cold-start.sh resource scale-down can silently no-op. The sed -i.bak patch of MAX_CPU/MAX_MEM in src/el/shared.star does nothing if a future kurtosis-pos ref renames the file or the vars. Since kurtosis_pos_ref is overridable, an unexpected ref could bypass the scale-down and OOM-kill the runner. After the sed, grep for the override-marker comment and exit 1 if it's absent.

  4. Stale comment in apps-e2e-trigger.yml. "Compare against the boolean AND the string explicitly..." — the actual expression only compares the toJSON string. The logic is correct (null / true / false / "true" / "false" all resolve right), but the comment misdescribes it. The second paragraph already explains the real reason; drop the first.

  5. restore.sh step 6 leaks body_file on curl failure — it's only rm'd after curl returns. Fold it into the cleanup trap.

Missing functionality (the real flexibility gaps)

  1. No secrets: block on apps-e2e.yml. The composite exposes registry_username / registry_password, but the reusable workflow doesn't surface either. Consequence: matic.js (in maticnetwork) cannot pull ghcr.io/0xpolygon/lst-api-e2e-snapshot with its own org's token if the package is private — they'd have to fork the workflow. Add REGISTRY_USERNAME / REGISTRY_PASSWORD (both required: false) and forward into the snapshot composite.

  2. No env / secret injection for the suite step. The team-standard CI composite passes secrets via job.env: in the trigger; that pattern doesn't work here because env: on the calling job doesn't cross the workflow_call boundary. With no secrets: block, no extra_env input, and no secrets: inherit opt-in, a consumer's e2e suite can't read anything from secrets. Fine for lst-api (emulator + kurtosis, no externals). A blocker for proof-generation-api (RPC tokens), pos-airdrop (real Firestore project / Sequence keys), or any matic.js test using non-public RPC. Accept named secrets explicitly, or support secrets: inherit + an env_secret_names input.

  3. The e2e_command contract has a hidden requirement. The consumer's suite must read E2E_SNAPSHOT_ADDRESSES_JSON in snapshot mode and query the kurtosis CLI directly in cold-start mode (the cold-start composite intentionally exports nothing). So every consumer needs a dual-path adapter modeled on lst-api's packages/e2e/scripts/up.ts — that's a non-trivial port, not a thin trigger, and the API doesn't say so. Document it on the e2e_command input. Better: have the cold-start composite emit something symmetric (e.g. E2E_KURTOSIS_ENCLAVE=pos) so consumers can write a single-path adapter.

Design issues

  1. Generic-named workflow defaults to one consumer's image. snapshot_image: ghcr.io/0xpolygon/lst-api-e2e-snapshot:latest means "forgot to set the input" silently uses lst-api's image, which rots when the publisher moves (deferred per this PR). Consider required: true until the publisher migrates, so consumers consciously opt in.

  2. python3 for both the YAML patch (step 4) and the JSON-RPC response check (step 6). jq is already near-universal in CI and removes the runtime dep on python3 + PyYAML.

  3. pnpm install --frozen-lockfile runs before both composites, including the ~10m cold-start — ~30s wasted on a cold-start failure. Run it after, or in parallel.

  4. Hardcoded runs-on: ubuntu-latest. No override for consumers needing a larger runner (matic.js sometimes does). Add a runner input defaulting to ubuntu-latest.

  5. Hardcoded checkout (depth 1, no submodules). Add a checkout_fetch_depth input — matters for adapters that want git describe or repos using submodule fixtures.

  6. Enclave/volume names (pos, name=^pos-) are hardcoded in both restore.sh and the cold-start composite. Fine while in lockstep, but unparameterised.

  7. docker compose --file "$COMPOSE_FILE" ps --quiet | grep -q . as the "previous restore exists" check is fragile when the compose file doesn't exist yet (compose exits non-zero, swallowed by 2>/dev/null). Gate with [ -f "$COMPOSE_FILE" ] first.

  8. L2_RPC_URL / ANVIL_RPC_URL localhost defaults would clash for two parallel runs on one self-hosted runner. Theoretical on ubuntu-latest (fresh VM per run), but worth a comment that host-network single-tenant is the assumption.

What I'd fix before merge

  • Bug 1 (trigger fires on pipelines PRs) — relocate or guard.
  • Gap 6 (secrets: block) + gap 7 (secret/env injection). Without these, "matic.js next" is aspirational.
  • Document the e2e-adapter contract on e2e_command (gap 8).
  • Bug 2 (--entrypoint) and bug 3 (post-sed verification).
  • Drop the stale comment paragraph (bug 4).

Per-consumer fitness

  • lst-api (current): fits exactly. ✓
  • pos-airdrop: fits if e2e stays self-contained (emulator + kurtosis env only). Blocked by gap 7 the moment it needs a real Firestore project ID or external secret. Must still write a kurtosis adapter (gap 8 — hidden work, not a hard blocker).
  • proof-generation-api: no e2e suite exists yet; fits when written if it follows lst-api's adapter pattern. Same secret-injection caveat.
  • matic.js: blocked today by gap 6 (cross-org private GHCR pull) and gap 7 if any test needs RPC tokens. Fits once both land.

MaximusHaximus added a commit that referenced this pull request Jun 3, 2026
Correctness:
- Guard apps-e2e-trigger.yml against firing on pipelines' own PRs
  (`github.repository != '0xPolygon/pipelines'`) — pipelines has no
  `pnpm run e2e` to run. No-op in any consumer, so still copy-verbatim.
- restore.sh: `docker create --entrypoint /bin/true` so the addresses
  sidecar extraction overrides (not appends to) any ENTRYPOINT a future
  snapshot image declares.
- cold-start.sh: verify the EL resource scale-down sed actually applied
  (grep the override marker, exit 1 if the file moved or vars renamed on
  an unexpected kurtosis_pos_ref) instead of silently OOM-killing.
- restore.sh: fold state_hex_file / body_file into the cleanup trap so
  the ~2 MB temps don't leak when curl or the result check exits non-zero.
- Drop the stale "compare boolean AND string" comment in the trigger
  (the expression only compares the toJSON string).

Flexibility / standards:
- Add REGISTRY_USERNAME / REGISTRY_PASSWORD (required: false) secrets to
  apps-e2e.yml and forward into the snapshot composite, falling back to
  the actor + GITHUB_TOKEN when unset. Unblocks a cross-org consumer
  (matic.js) pulling a private snapshot with its own credentials.
- Remove the `secrets: inherit` recommendation from the header comment
  (prohibited by Team Standards).
- Extract the on-failure dump shell into a new e2e-dump composite action
  (bundled script via github.action_path) rather than inline YAML — a
  reusable workflow can't reach a .github/scripts/ helper cross-repo.
- Document the dual-path adapter contract and the secret-injection
  limitation on the e2e_command input; cold-start composite now exports
  E2E_KURTOSIS_ENCLAVE=pos for a symmetric, single-path consumer adapter.

Deferred (design polish, tracked for follow-up): runner/fetch-depth
inputs, jq-over-python, install-after-composite, enclave-name
parameterisation, required snapshot_image. Generic suite-secret
injection stays out — named secrets only, no secrets: inherit.
# keeps this YAML thin and the shell locally runnable.
- name: Dump devnet on failure
if: failure()
uses: 0xPolygon/pipelines/.github/actions/e2e-dump@main
@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
4 Security Hotspots
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Consumer-side kurtosis-pos e2e harness, shared so adopters (lst-api
today, matic.js / proof-generation-api next) call one implementation
instead of vendoring copies that drift.

- apps-e2e.yml — reusable workflow. Snapshot mode (default, ~30s)
  restores a published GHCR image via the e2e-snapshot-restore
  composite; cold-start mode (~10m) runs kurtosis from source via the
  e2e-cold-start composite for unsnapshotted kurtosis-pos refs. The
  snapshot/cold-start switch uses `toJSON(inputs.use_snapshot) == 'false'`
  because Actions' loose equality treats `null == false` as true, which
  would otherwise drop null-input pull_request runs into cold-start.
  REGISTRY_USERNAME / REGISTRY_PASSWORD (optional) let a cross-org
  consumer pull a private snapshot with its own credentials, falling
  back to the actor + GITHUB_TOKEN when unset.
- actions/e2e-snapshot-restore — pull + restore a snapshot image
  (extract volumes, patch the pos-anvil host port, replay anvil state).
- actions/e2e-cold-start — install kurtosis-cli, scale EL resource
  limits down for ubuntu-latest (verified, fails loud if the patch
  no-ops on an unexpected ref), run the enclave; exports
  E2E_KURTOSIS_ENCLAVE for a symmetric consumer adapter.
- actions/e2e-dump — capture devnet state on failure (compose logs or
  enclave dump). A composite, not an inline run: block or a
  .github/scripts/ helper, because a reusable workflow can't reach a
  repo-local script cross-repo.
- apps-e2e-trigger.yml — canonical thin trigger, guarded against firing
  on pipelines' own PRs.
- README — workflow + composite tables and usage notes.

Out of scope: the snapshot publisher stays in lst-api against its
GHCR namespace; moving + renaming it is a separate refactor. Generic
suite-secret injection is documented as a known limitation — named
secrets only, never `secrets: inherit` (Team Standards).
@MaximusHaximus MaximusHaximus force-pushed the feat/apps-e2e-shared-workflow branch from 02f6294 to baa6cfe Compare June 3, 2026 23:43
@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
4 Security Hotspots
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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