Skip to content

ci: fix registry notification workflow (PLT-846) - #331

Merged
alexander-sei merged 1 commit into
mainfrom
alexanderh/plt-846-ci-hygiene
Aug 18, 2026
Merged

ci: fix registry notification workflow (PLT-846)#331
alexander-sei merged 1 commit into
mainfrom
alexanderh/plt-846-ci-hygiene

Conversation

@alexander-sei

Copy link
Copy Markdown
Contributor

Summary

  • remove the unreachable registry-path push trigger so notifications only follow successful Release workflow runs
  • upgrade peter-evans/repository-dispatch from v1 to v4, declare its required permission, and remove the unused checkout
  • complete the remaining PLT-846 CI hygiene work after feat!: migrate the monorepo to Bun and Biome #327 added PR checks, concurrency, scoped pushes, and consistent runtimes

Test plan

  • go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/*.yml
  • YAML syntax validation
  • git diff --check

Made with Cursor

Remove the unreachable push path and update repository dispatch so registry notifications only follow successful releases.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.82%. Comparing base (5675fcf) to head (532b603).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #331   +/-   ##
=======================================
  Coverage   83.82%   83.82%           
=======================================
  Files          64       64           
  Lines        4390     4390           
=======================================
  Hits         3680     3680           
  Misses        710      710           
Flag Coverage Δ
mcp-server 77.28% <ø> (ø)
precompiles 100.00% <ø> (ø)
registry 100.00% <ø> (ø)
sei-global-wallet 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Clean, well-scoped CI hygiene change to .github/workflows/notify.yml: the removed push trigger was provably unreachable, the if simplification is correct, the v1→v4 action bump is input-compatible, and contents: write is the right minimum permission. No blockers; the main open question is whether the dispatch this workflow sends actually reaches a listener.

Findings: 0 blocking | 6 non-blocking | 2 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • Verified the removed push trigger was genuinely dead: the old if: github.event_name == 'workflow_run' && ... guard meant push-triggered runs always skipped the job, so this is behavior-preserving cleanup, not a lost code path. workflows: ["Release"] also matches the actual name: Release in release.yml.
  • Sibling workflows touched by #327 (checks.yml, release.yml, uci-stale-check.yml) all declare a concurrency group; notify.yml still does not. With a 90s sleep plus a dispatch, overlapping Release runs could stack duplicate notifications. Low impact, but adding one would finish the PLT-846 hygiene pass consistently.
  • The Cursor second-opinion review file (cursor-review.md) was empty — that pass produced no output. Codex reported no material issues, which matches my read.
  • No changeset needed here: this is CI-only and touches no published @sei-js/* package (REVIEW_GUIDELINES.md §3).
  • 2 suggestion(s)/nit(s) flagged inline on specific lines.


- name: Update registry API
uses: peter-evans/repository-dispatch@v1
uses: peter-evans/repository-dispatch@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[suggestion] The v1→v4 bump itself is safe — token, event-type, and client-payload kept the same names across those majors, and this moves off the deprecated Node runtime.

Worth confirming while you're in here, though, whether this step actually notifies anything today. Two things stack up:

  1. No repository input is set, so the dispatch defaults to sei-protocol/sei-js itself — and nothing in .github/workflows/ subscribes to repository_dispatch (types: [workflow_completed]).
  2. GitHub does not start workflow runs from repository_dispatch events created with secrets.GITHUB_TOKEN (line 25). Reaching an external repo would need a PAT anyway, since GITHUB_TOKEN is scoped to this repo.

Both predate this PR, so not a blocker on the diff. But given the PR title is "fix registry notification workflow," it seems worth checking that the registry API is really receiving these — otherwise the fix is polishing a no-op.

- 'packages/registry/**'

permissions:
contents: write

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[nit] contents: write is the correct minimum for the repository-dispatch REST endpoint — good call declaring it explicitly. Since there's only one job, you could scope it under jobs.notify.permissions instead, so any future job added to this file starts from the default read-only token rather than inheriting write.

@alexander-sei
alexander-sei merged commit eff761b into main Aug 18, 2026
15 checks passed
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.

2 participants