Skip to content

M2: package and publish platform-specific shim artifacts#9

Merged
tazarov merged 4 commits into
mainfrom
codex/m2-release-artifacts
Feb 21, 2026
Merged

M2: package and publish platform-specific shim artifacts#9
tazarov merged 4 commits into
mainfrom
codex/m2-release-artifacts

Conversation

@tazarov

@tazarov tazarov commented Feb 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a new release workflow that triggers on v* tags and manual dispatch
  • build release Rust shim artifacts on Linux/macOS/Windows and package them as stable chroma-go-shim-<os>-<arch>.tar.gz assets
  • generate per-archive SHA256 files and publish a combined chroma-go-shim_SHA256SUMS.txt with release assets
  • document artifact names, OS-to-library mapping, and checksum verification in README

Testing

  • make test
  • make lint

Closes #3

@tazarov

tazarov commented Feb 20, 2026

Copy link
Copy Markdown
Contributor Author

@claude review this PR

@claude

claude Bot commented Feb 20, 2026

Copy link
Copy Markdown

Claude finished @tazarov's task in 1m 50s —— View job


PR Review: M2 - Package and publish platform-specific shim artifacts

Tasks:

  • Gather context and understand request
  • Read changed files (release.yml, README.md)
  • Analyze workflow for correctness, security, and best practices
  • Post review feedback

Summary

This PR adds a solid release workflow for cross-platform Rust shim builds and documents the artifact layout. The workflow is well-structured and consistent with the existing ci.yml patterns. A few things worth addressing before merging:


.github/workflows/release.yml

Security: Action versions not pinned to commit SHAs

All actions are pinned to mutable version tags (e.g. @v4, @v2) rather than immutable SHA hashes. For a release workflow that has contents: write permissions and publishes release assets, supply-chain attacks against these actions are a real risk.

Affected lines:

  • actions/checkout@v4 (line 26)
  • Swatinem/rust-cache@v2 (line 40)
  • arduino/setup-protoc@v3 (line 45)
  • actions/upload-artifact@v4 (line 95)
  • actions/download-artifact@v4 (line 113)
  • softprops/action-gh-release@v2 (line 127) — highest risk, this publishes release assets

Recommendation: pin each to a specific commit SHA. For example:

uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683  # v4.2.2

Fix this →


Minor: Redundant contents: write permission

contents: write is declared at both the workflow level (line 9-10) and again on the publish-release job (line 108-109). The job-level declaration is redundant since the workflow-level already grants it.

A cleaner approach would be to restrict the workflow-level permissions (e.g. permissions: {} or contents: read) and only grant contents: write on the specific job that needs to publish releases (publish-release). The build-artifacts job doesn't need write access at all.

Fix this →


Observation: workflow_dispatch does not publish (intentional?)

When triggered via workflow_dispatch (without a tag), the build-artifacts job runs on all 3 platforms but publish-release is skipped (due to if: startsWith(github.ref, 'refs/tags/')). This appears intentional as a dry-run mechanism.

Consider adding a brief comment in the YAML to clarify this intent, e.g.:

# publish-release only runs on tag pushes; workflow_dispatch is for testing the build matrix
if: startsWith(github.ref, 'refs/tags/')

Observation: arm64 targets

The matrix covers ubuntu-latest, macos-latest, windows-latest, all of which run on X64. The runner.arch == 'ARM64' branch in the PowerShell switch is forward-looking (for self-hosted runners or future GitHub-provided ARM64 runners), which is good. Consider adding a note in the README that only amd64 artifacts are published for now. GitHub does offer macos-14 (M1/ARM64) as a runner — if native arm64 macOS builds are wanted, a separate matrix entry could be added.


README.md

Minor: macOS checksum verification is incomplete

The macOS example (line ~54 in the diff) shows how to verify a single archive with shasum but doesn't show how to verify against the combined SHA256SUMS.txt. Add an example using shasum -a 256 -c for completeness:

# macOS - verify all archives
shasum -a 256 -c chroma-go-shim_SHA256SUMS.txt

What looks good

  • fail-fast: false in the build matrix — one platform failure won't cancel others
  • PowerShell for the packaging step — consistent with ci.yml and works cross-platform
  • cargo build --locked --release--locked ensures reproducibility against Cargo.lock
  • set -euo pipefail in the bash step in publish-release
  • generate_release_notes: true — auto-populates release notes from PR/commit history
  • merge-multiple: true in download-artifact cleanly flattens all platform artifacts into one dist/ dir
  • The combined SHA256SUMS.txt approach is clean — one file to verify all archives
  • The README documentation is clear and well-structured with a table and copy-paste examples

Verdict

The workflow is functionally correct. The main concern worth addressing before merging is action pinning in the release workflow, particularly for softprops/action-gh-release which handles actual release publishing. The permission scoping is a lower-priority improvement.

@tazarov

tazarov commented Feb 20, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all review items from the Claude feedback:

  1. Pinned release workflow actions to immutable commit SHAs (including dtolnay/rust-toolchain).
  2. Tightened permissions to least privilege (contents: read at workflow level, contents: write only on publish job).
  3. Added an explicit YAML note that workflow_dispatch is for build verification and publishing runs only on tags.
  4. Updated macOS checksum docs to use shasum -a 256 -c chroma-go-shim_SHA256SUMS.txt.
  5. Added architecture coverage note in README clarifying <arch> is runner-derived and ARM64 requires ARM64 runners.

Validation rerun after changes:

  • make test
  • make lint

@tazarov

tazarov commented Feb 20, 2026

Copy link
Copy Markdown
Contributor Author

@claude review please

@claude

claude Bot commented Feb 20, 2026

Copy link
Copy Markdown

Claude finished @tazarov's task in 1m 51s —— View job


PR Review: M2 - Package and publish platform-specific shim artifacts (re-review)

Tasks:

  • Gather context and understand request
  • Read changed files (release.yml, README.md)
  • Verify previous feedback was addressed
  • Check for new issues
  • Post review feedback

All five items from the previous review have been addressed. The workflow and docs are in good shape. A couple of minor remaining observations:


Addressed ✓

Previous concern Status
Action versions pinned to mutable tags Fixed — all 6 actions now pinned to SHA hashes
contents: write at wrong scope Fixed — workflow-level is contents: read, job-level contents: write only on publish-release
workflow_dispatch intent unclear Fixed — comment added at line 106
macOS checksum docs incomplete Fixed — shasum -a 256 -c example added
ARM64 not documented Fixed — architecture note added to README

Minor remaining observations

1. SHA comments don't include specific version tags

The pinned SHAs use abbreviated comments (# v4, # v2, # stable) rather than the specific release that was pinned (e.g., # v4.2.2). Without a specific version in the comment, you can't easily audit whether the SHA matches the intended version without looking it up.

Affected lines: 26, 37, 40, 45, 95, 114, 128.

Example of the pattern that's easier to audit:

uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.2.2

This is a style point, not a security concern — the SHA itself is what matters.


2. Windows checksum verification missing from README

The verify section covers Linux and macOS but leaves Windows users without an example. Consider adding:

# Windows PowerShell (requires PowerShell 5.1+ / Windows 10+)
Get-Content chroma-go-shim_SHA256SUMS.txt | ForEach-Object {
    $expected, $file = $_ -split '  '
    $computed = (Get-FileHash -Algorithm SHA256 $file).Hash.ToLowerInvariant()
    if ($computed -eq $expected) { "OK: $file" } else { throw "MISMATCH: $file" }
}

3. ci.yml has unpinned actions (out of scope, just noting)

The existing ci.yml still uses @v4, @v5, @v2, etc. for all its actions. That's pre-existing and outside this PR's scope, but worth a follow-up issue.


Verdict

The workflow is functionally correct and the previous security concern (action pinning) has been resolved. Items 1 and 2 above are style/docs improvements — neither is blocking. This is ready to merge if the approach looks good to you.

@tazarov

tazarov commented Feb 20, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining non-blocking items as requested:

  1. Updated pinned-action comments in release.yml to exact versions where tag-resolved (for example # v4.3.1, # v2.8.2, # v2.5.0).
  2. Added Windows checksum verification example in README using PowerShell + Get-FileHash.
  3. Opened a follow-up issue to pin actions in ci.yml: Security: pin GitHub Actions by SHA in ci.yml #10.

Validation rerun:

  • make test
  • make lint

@tazarov

tazarov commented Feb 20, 2026

Copy link
Copy Markdown
Contributor Author

@claude let's review

@claude

claude Bot commented Feb 20, 2026

Copy link
Copy Markdown

Claude finished @tazarov's task in 2m 11s —— View job


PR Review: M2 - Package and publish platform-specific shim artifacts (3rd review)

Tasks:

  • Gather context and understand request
  • Read changed files (release.yml, README.md, ci.yml)
  • Verify latest round of feedback was addressed
  • Post review feedback

All items from the second review have been addressed. Here's the verification:

Addressed ✓

Previous concern Status
SHA comments lacked specific version tags Fixed — all 7 pinned actions now include exact version in comment (e.g. # v4.3.1, # v2.8.2)
Windows checksum verification missing from README Fixed — PowerShell block added, with a nice extra improvement: guards against empty lines with if (-not $_)
ci.yml unpinned actions Fixed via follow-up issue #10 (correctly out of scope for this PR)

One remaining minor observation

dtolnay/rust-toolchain has no toolchain input

ci.yml uses dtolnay/rust-toolchain@stable with components: clippy. The release workflow pins the same action to a SHA but omits the toolchain input entirely:

uses: dtolnay/rust-toolchain@631a55b12751854ce901bb631d5902ceb48146f7 # stable branch commit (no semver tag)

Without a rust-toolchain.toml file in the repo, this defaults to stable — so it's functionally correct. Adding toolchain: stable explicitly would make the intent unambiguous and match the ci.yml pattern:

uses: dtolnay/rust-toolchain@631a55b12751854ce901bb631d5902ceb48146f7 # stable branch commit (no semver tag)
with:
  toolchain: stable

This is a style point only. Not blocking.


What looks good

  • All 7 action SHAs pinned with exact version tags in comments — auditable and clear
  • Permissions: contents: read at workflow level, contents: write scoped only to publish-release — least privilege
  • workflow_dispatch dry-run intent documented at line 106
  • macOS shasum -a 256 -c example present
  • Windows PowerShell verification improved beyond the original suggestion (handles empty lines, uses split limit of 2)
  • ARM64 architecture note in README is accurate
  • Set-Content -Encoding ascii avoids BOM issues for checksum files — good attention to detail
  • BSD two-space separator format in .sha256 files matches what all three verification commands expect

Verdict

The workflow is functionally correct, security-conscious, and well-documented. All substantive feedback from the previous two rounds has been resolved. The one remaining point (toolchain: stable) is a minor style improvement with no functional impact.

This is ready to merge.

@tazarov tazarov merged commit 24e537e into main Feb 21, 2026
4 checks passed
@tazarov tazarov deleted the codex/m2-release-artifacts branch February 21, 2026 05:36
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.

M2: Package and publish platform-specific shim artifacts

1 participant