Skip to content

Prepare Cassandra for GitHub Marketplace publishing#44

Merged
menny merged 8 commits intomainfrom
copilot/review-github-actions-docs
Apr 13, 2026
Merged

Prepare Cassandra for GitHub Marketplace publishing#44
menny merged 8 commits intomainfrom
copilot/review-github-actions-docs

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 12, 2026

  • Explore codebase and understand current state
  • Add branding block to action.yml (eye / purple)
  • Add outputs to action.yml: review_file, json_file, approved, review_rationale
  • Add Set Action Outputs step to extract values from JSON and expose them
  • Pin bazel-contrib/setup-bazel to full commit SHA in action.yml only; internal workflows keep major versions
  • Add minimum required permissions documentation to README (including approve-PR note and issues: write)
  • Ensure cassandra_review.yml declares issues: write permission
  • Document REVIEWERS.md and AGENTS.md conventions in README (merged into a single section with directory tree examples for both)
  • Add troubleshooting section to README (4 common failure scenarios; removed stale "No changes found" item since diff is fetched via GitHub API)
  • Create versioned release workflow (.github/workflows/release.yml): Bazel cross-compiles 5 platform binaries via --platforms=@rules_go//go/toolchain:<platform>, creates release with auto-generated changelog via gh release create --generate-notes
  • Update README: simplified copy-paste workflow template (no fetch-depth: 0 or manual base/head needed), outputs table with usage example
  • Merged onto latest main (post-merge of PR feat: fetch diff and commits via GitHub API #42); resolved conflict in cmd/github/main.go keeping the LastIndex fix
  • Run yamlfmt on all changed YAML files
  • Address code review security/robustness findings:
    • [Risk] README example: map review_rationale output through env: to prevent command injection
    • [Risk] release.yml: move github.ref_name to RELEASE_VERSION env var
    • [Observation] action.yml: use openssl rand -hex 16 for unpredictable GITHUB_OUTPUT delimiter
    • [Observation] cmd/github/main.go: fix strings.Fields lockfile path parsing to use LastIndex(" b/") for correctness with spaces in paths
  • release.yml: remove fetch-depth: 0 — shallow clone is sufficient for building
  • release.yml: replace go build loop with Bazel cross-compilation via --platforms=@rules_go//go/toolchain:<platform>
  • README.md: expand guidelines section to document both REVIEWERS.md and AGENTS.md (same discovery logic, merged into one section)
  • README.md: remove stale "No changes found" troubleshooting item; simplified workflow template

- Add branding (eye/purple) to action.yml for Marketplace listing
- Add outputs: review_file, json_file, approved, review_rationale
- Pin bazel-contrib/setup-bazel to full commit SHA in action.yml only
- Add .github/workflows/release.yml: cross-compiles binaries for 5
  platforms and creates a GitHub Release with auto-generated notes
- Update cassandra_review.yml: add issues: write permission
- Update README: outputs table with usage example, required permissions
  section (including approve note), REVIEWERS.md convention docs,
  complete workflow template, and troubleshooting section

Agent-Logs-Url: https://github.com/menny/cassandra/sessions/e89680cd-c332-48a8-b9fd-e79ad64e6c17

Co-authored-by: menny <481272+menny@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 12, 2026

The changes look robust and securely address the Action outputs and cross-compilation requirements. The mitigations for command injection in the workflow example are correct, and replacing the fragile whitespace splitting with strings.LastIndex properly fixes the path extraction bug. Moving to Bazel for the release cross-compilation ensures hermetic builds and is well implemented.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

The PR contains command injection vectors in the workflows and README that need to be resolved before merging.

@menny
Copy link
Copy Markdown
Owner

menny commented Apr 12, 2026

@copilot rebase this branch to latest main, and fix conflicts

menny and others added 3 commits April 12, 2026 03:49
Introduce get-diff, get-files, and get-commits actions to the GitHub tool to allow reviews with shallow clones.
Update the AI reviewer to accept pre-fetched data and unconditionally include commit messages in the review context.

Co-authored-by: menny <481272+menny@users.noreply.github.com>
- Filter lockfiles from raw GitHub diff to prevent token bloat.
- Enforce both --diff-file and --files-list-file in ai_reviewer.
- Improve lockfile path matching precision.
- Truncate commit messages to first line in GitHub tool.
- Export and reuse LockFiles constant across packages.

Co-authored-by: menny <481272+menny@users.noreply.github.com>
- Exclude merge commits from GitHub API results to match local behavior.
- Normalize CRLF line endings and extract only the first line of commit messages.

Co-authored-by: menny <481272+menny@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 12, 2026

@copilot rebase this branch to latest main, and fix conflicts

Done — rebased onto latest main (commit 8d8cce8). No conflicts; the branch now includes the GitHub API diff/commits work from PR #42.

Copilot AI requested a review from menny April 12, 2026 03:49
@menny
Copy link
Copy Markdown
Owner

menny commented Apr 12, 2026

@copilot install baselisk in your environment and run the format target. See AGENTS.md file for details

@github-actions github-actions Bot dismissed their stale review April 12, 2026 03:59

Superseded by a new AI review.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

The rebase and new GitHub API context fetching logic look great, but command injection vulnerabilities and other issues need to be addressed before merging.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 12, 2026

@copilot install baselisk in your environment and run the format target. See AGENTS.md file for details

Done. Ran yamlfmt (v0.14.0, matching the version pinned in multitool.lock.json) on all YAML files changed in this PR. The only change needed was removing 3 blank lines between steps in .github/workflows/release.yml (commit e1121ab).

Note: bazel run //:format itself couldn't execute in this environment because bcr.bazel.build (the Bazel Central Registry) is unreachable. I ran the individual formatters directly instead — yamlfmt for YAML and verified no Go or Starlark files were modified in this PR.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • bcr.bazel.build
    • Triggering command: /home/REDACTED/.cache/bazel/_bazel_REDACTED/install/8d5f50e28a91af5bb7169f1a7a6e173a/embedded_tools/jdk/bin/java bazel(cassandra) --add-opens=java.base/java.lang=ALL-UNNAMED -Xverify:none -Djava.util.logging.config.file=/home/REDACTED/.cache/bazel/_bazel_REDACTED/209127c21b74c9ed478a1f2615011ff3/javalog.properties -Dcom.google.devtools.build.lib.util.LogHandlerQuerier.class=com.google.devtools.build.lib.util.SimpleLogHandler$HandlerQuerier -XX:-MaxFDLimit -Djava.lang.Thread.allowVirtualThreads=true -Djava.library.path=/home/REDACTED/.cache/bazel/_bazel_REDACTED/install/8d5f50e28a91af5bb7169f1a7a6e173a/embedded_tools/jdk/lib:/home/REDACTED/.cache/bazel/_bazel_REDACTED/install/8d5f50e28a91af5bb7169f1a7a6e173a/embedded_tools/jdk/lib/server:/home/REDACTED/.cache/ (dns block)
  • releases.bazel.build
    • Triggering command: /usr/local/lib/node_modules/@bazel/bazelisk/bazelisk-linux_amd64 /usr/local/lib/node_modules/@bazel/bazelisk/bazelisk-linux_amd64 --version (dns block)
    • Triggering command: /usr/local/lib/node_modules/@bazel/bazelisk/bazelisk-linux_amd64 /usr/local/lib/node_modules/@bazel/bazelisk/bazelisk-linux_amd64 version (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@menny
Copy link
Copy Markdown
Owner

menny commented Apr 12, 2026

@copilot read the code review comments and address them if they make sense

- README: map review_rationale output to env var to prevent
  command injection from AI-generated content
- release.yml: move github.ref_name to RELEASE_VERSION env var in
  both Build and Create Release steps to prevent tag injection
- release.yml: add comment explaining why go build is used over Bazel
- action.yml: replace predictable run-id delimiter with
  openssl rand -hex 16 for GITHUB_OUTPUT multiline value
- cmd/github/main.go: replace strings.Fields with LastIndex(" b/")
  to correctly parse diff --git paths containing spaces

Agent-Logs-Url: https://github.com/menny/cassandra/sessions/efef5157-6dd4-47e1-99a1-a675b5c8265a

Co-authored-by: menny <481272+menny@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 12, 2026

@copilot read the code review comments and address them if they make sense

Addressed all 5 actionable findings in commit 449d383:

  • [Risk] README.md: moved review_rationale output into an env: block (REVIEW_RATIONALE) before referencing it in run:, preventing AI-generated content from injecting shell commands.
  • [Risk] release.yml build step: moved github.ref_name to RELEASE_VERSION env var; ldflags now reference $RELEASE_VERSION.
  • [Risk] release.yml release step: same — gh release create now uses $RELEASE_VERSION from the step's env: block.
  • [Maintenance] release.yml: added a comment explaining why go build is preferred over Bazel for cross-compilation in the release workflow.
  • [Observation] action.yml: replaced the predictable cassandra_<run_id>_<run_attempt> delimiter with $(openssl rand -hex 16).
  • [Observation] cmd/github/main.go: replaced strings.Fields (fragile with spaces in paths) with strings.LastIndex(line, " b/") to correctly extract the destination path from diff --git a/... b/... headers.

Comment thread .github/workflows/release.yml Outdated
- name: Check out code
uses: actions/checkout@v4
with:
fetch-depth: 0
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot why do we need the entire history? We just build, thus can be shallow

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed fetch-depth: 0 — a shallow clone is sufficient since we only need the source files to build. Done in commit 924a1f3.

Comment thread .github/workflows/release.yml Outdated

echo "Building cassandra for $GOOS/$GOARCH..."
GOOS="$GOOS" GOARCH="$GOARCH" go build \
-ldflags="-s -w -X main.version=$RELEASE_VERSION" \
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot This is great, but this repo relies on bazel as a build system. Setup bazel and use it for building and cross compilation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replaced the go build loop with Bazel cross-compilation in commit 924a1f3. The step now:

  • Uses setup-bazel (not setup-go)
  • Builds each platform with bazel build --platforms=@rules_go//go/toolchain:<platform>
  • Copies the resulting binaries to dist/ using bazel cquery --output=files

The pure = "on" and static = "on" attributes in the existing go_binary targets ensure hermetic, statically-linked binaries.

Comment thread README.md Outdated
```

If you are using `GITHUB_TOKEN`, you should also ensure the correct permissions:
### REVIEWERS.md — Per-Directory Review Guidelines
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot The ai reviewer will also look for AGENTS.md files using the same logic. Restructure this doc to include (maybe merge for clarity) the two guidelines files

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Merged the two into a single "Per-Directory Review Guidelines — REVIEWERS.md and AGENTS.md" section in commit 924a1f3. It now explains that both files use the same walk-up discovery logic, shows both in the directory tree example, and provides separate content examples for each (reviewer guidance vs. AI assistant instructions).

Comment thread README.md Outdated
pull-requests: write
- uses: actions/checkout@v4
with:
fetch-depth: 0
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot this is no longer needed as the action will use GitHub api to fetch the diff

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed in commit 924a1f3. Also cleaned up the workflow template in the same README section to drop fetch-depth: 0 and the now-unnecessary base/head inputs.

- release.yml: remove fetch-depth: 0 (shallow clone sufficient for build)
- release.yml: replace go build loop with Bazel cross-compilation using
  --platforms=@rules_go//go/toolchain:<platform> for each target
- release.yml: use setup-bazel instead of setup-go
- README: rename and expand REVIEWERS.md section to cover both
  REVIEWERS.md and AGENTS.md (same discovery logic)
- README: remove stale "No changes found" troubleshooting item (diff
  now fetched via GitHub API, no need for fetch-depth: 0)
- README: remove fetch-depth: 0 and base/head from workflow template

Agent-Logs-Url: https://github.com/menny/cassandra/sessions/b9bb0f04-fc28-4a65-ad9d-e6a275fc8e08

Co-authored-by: menny <481272+menny@users.noreply.github.com>
Copilot AI requested a review from menny April 12, 2026 15:32
@menny
Copy link
Copy Markdown
Owner

menny commented Apr 12, 2026

@copilot rebase the branch with the latest main branch. And fix the conflicts.

# Conflicts:
#	cmd/github/main.go

Co-authored-by: menny <481272+menny@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 12, 2026

@copilot rebase the branch with the latest main branch. And fix the conflicts.

Done in commit 8307ba9. There was one conflict in cmd/github/main.go (main had the old strings.Fields path extraction, our branch had the LastIndex fix) — kept our corrected version.

@github-actions github-actions Bot dismissed their stale review April 12, 2026 21:31

Superseded by a new AI review.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

The changes securely address Action outputs and cross-compilation requirements, properly fix the path extraction bug using strings.LastIndex, and successfully implement Bazel for hermetic release builds.

@menny menny merged commit ed60086 into main Apr 13, 2026
2 of 3 checks passed
@menny menny deleted the copilot/review-github-actions-docs branch April 13, 2026 01:25
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