BOG-246 Trigger crates publish from GitHub releases#8
Conversation
Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
Summary
This PR adds GitHub Release published trigger support to the crates.io publish workflow with comprehensive validation. The implementation includes proper token handling, environment gating, and third-party action pinning as requested.
Critical Issue
Logic Error in Ancestry Validation: The ancestry check on line 107 is inverted and will fail for the intended release workflow. The check currently validates that the tag commit is an ancestor of main, but it should verify that main has the tag commit in its history (i.e., the tag points to a commit that's been merged into main). This will block all legitimate publish attempts following the documented sequence of merge-to-main-then-tag.
Security Review
✓ Token handling is secure - CARGO_REGISTRY_TOKEN is properly gated behind the crates-io environment and only passed to the publish job after all preconditions are validated.
✓ Third-party action remains pinned to reviewed commit SHA 02cc2f1ad653fb25c7d1ff9eb590a8a50d06186b.
✓ Dry-run jobs correctly exclude the registry token.
Recommendation
Fix the ancestry check logic error before merge. The documentation accurately describes the intended workflow, but the implementation will reject it.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| if ! git merge-base --is-ancestor "${tag_commit}" "${main_commit}"; then | ||
| echo "Release tag ${GITHUB_REF_NAME} must point to a commit already reachable from origin/main." >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🛑 Logic Error: The ancestry check validates that the tag commit is an ancestor of the main commit, but semantically you want to ensure the tag points to a commit that has been merged into main. The current logic is inverted - it checks if tag_commit is an ancestor of main_commit, which would only be true if the tag was created before main advanced. This will fail for the intended workflow where you merge to main first, then tag that commit.
The correct check should verify that the tag commit is reachable from main, meaning main contains the tag commit in its history. This requires checking if main_commit is an ancestor of OR equal to tag_commit, or more directly, if they point to the same commit.
| if ! git merge-base --is-ancestor "${tag_commit}" "${main_commit}"; then | |
| echo "Release tag ${GITHUB_REF_NAME} must point to a commit already reachable from origin/main." >&2 | |
| exit 1 | |
| fi | |
| if ! git merge-base --is-ancestor "${main_commit}" "${tag_commit}"; then | |
| echo "Release tag ${GITHUB_REF_NAME} must point to a commit already reachable from origin/main." >&2 | |
| exit 1 | |
| fi |
Summary
publishedtrigger for the crates.io publish workflow.dry-run: true.origin/mainancestry, release event shape,target_commitish, thecrates-ioenvironment, andCARGO_REGISTRY_TOKEN.Verification
git diff --checkruby -e 'require "yaml"; YAML.load_file(".github/workflows/publish-crates.yml"); puts "workflow yaml ok"'cargo package -p upbit-sdk --allow-dirty --listpassed.cargo testpassed:upbit-mock8 tests,upbit-sdk37 tests, and doc-tests with no failures.Notes
actionlintwas not run because it is not installed in the local environment.Review Focus