Skip to content

ci: notify compatibility tests for LLGo releases - #2366

Open
zhouguangyuan0718 wants to merge 1 commit into
xgo-dev:mainfrom
zhouguangyuan0718:codex/notify-release-benchmarks
Open

ci: notify compatibility tests for LLGo releases#2366
zhouguangyuan0718 wants to merge 1 commit into
xgo-dev:mainfrom
zhouguangyuan0718:codex/notify-release-benchmarks

Conversation

@zhouguangyuan0718

Copy link
Copy Markdown
Contributor

Summary

  • keep the existing llgo-main-updated notification for binary-size data
  • send llgo-tag-released when a stable release or prerelease is published
  • resolve and validate the release tag commit before dispatching it to xgo-dev/benchmarks

This wires future LLGo releases to the open-source compatibility job merged in xgo-dev/benchmarks#29.

Validation

  • parsed the workflow YAML
  • shell syntax checked the dispatch script
  • resolved v1.0.0-pre.3 to cf91afb98fca979c5195751554b67ec75f8c71d6 through the same tag-resolution logic

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review: ci: notify compatibility tests for LLGo releases

This is a clean, well-constructed change. The new release: [published] trigger and the tag→commit resolution are implemented with solid defensive practices:

  • Untrusted github.event.release.tag_name is passed via the RELEASE_TAG env var (line 24) and never inlined as a ${{ }} expression in the run block — the recommended anti-injection pattern.
  • The tag is validated with git check-ref-format (line 35) and the resolved commit is constrained to a 40-hex SHA (line 51) before any dispatch.
  • The JSON payload is built with jq --arg (lines 56–63) rather than string interpolation, so no shell values leak into the JSON.
  • permissions: {} (line 13) and the github.repository == 'xgo-dev/llgo' guard (line 17) keep least privilege and prevent secret exposure to forks.
  • The annotated-vs-lightweight tag handling (prefer refs/tags/X^{}, fall back to refs/tags/X) is correct for the common case, and both refs are requested in a single ls-remote call.

One minor, non-blocking note is inline. No security, performance, or documentation-accuracy issues were found.

llgo_commit="$GITHUB_SHA"
event_type=llgo-main-updated
fi
if [[ ! "$llgo_commit" =~ ^[0-9a-f]{40}$ ]]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: the resolved commit is validated for SHA shape only (40 hex chars), not that it actually refers to a commit object. For the normal case — an annotated release tag on a commit — this is fine, since the refs/tags/X^{} peeled entry yields the commit. In the theoretical case of an annotated tag pointing at a non-commit object, the fallback would pick the tag object's own SHA, which also passes this regex. This is extremely unlikely for GitHub release tags (which require a tag on a commit), so it's an accepted-limitation note rather than an actionable defect.

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

1 participant