ci: add release workflow + install script #2
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: rodin/review-bot#2
Reference in New Issue
Block a user
Delete Branch "ci/release-workflow"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
.gitea/workflows/release.yml): triggers onv*tags, builds 4 binaries (linux/darwin × amd64/arm64), creates Gitea release with assetsinstall.sh): curl-pipe-bash installer that fetches latest release binaryUsage
To release:
To install in CI:
Or in gargoyle workflow:
Summary
Useful release workflow and installer additions, plus ldflags-ready version variable. However, the installer does not verify downloaded binaries and the release workflow assumes jq is present; also the LLM client silently changes temperature behavior. Address these before merging.
Findings
install.sh.gitea/workflows/release.ymlinstall.shinstall.shllm/client.gollm/client.goRecommendation
REQUEST_CHANGES — Overall the direction is good, but please address the security and robustness issues first. Specifically: 1) In install.sh, verify the downloaded artifact before installation (e.g., fetch checksums.txt from the same release, validate the binary’s SHA-256, and fail if it doesn’t match). Consider also verifying the checksum file itself (e.g., via release signatures) if available. 2) In release.yml, add an explicit step to install jq (or avoid jq by parsing with a POSIX-safe method) so the release creation is reliable. 3) In install.sh, either check for and handle non-writable INSTALL_DIR (falling back to ~/.local/bin and adding it to PATH or instructing the user) or update the documentation to require sudo. 4) Clarify the change to LLM temperature: if the intent is to rely on server defaults, document it and consider making temperature configurable with a sensible default; otherwise restore an explicit value. After these changes, the PR should be ready to merge.
Summary
The PR adds a release workflow, an install script, and a version variable, but introduces at least one functional regression in the LLM client and multiple issues in the release workflow that will cause failures (wrong Gitea URL and missing jq dependency). Please address the major issues before merging.
Findings
llm/client.gollm/client.gojson:"temperature,omitempty". This combined with the removed assignment means the temperature will be omitted when 0.0; if the intent was to always send a non-zero temperature, restore the value. The tag change itself is not necessarily wrong but has behavioural implications..gitea/workflows/release.yml.gitea/workflows/release.ymljq -r '.id') but the runner does not install jq. This will cause the job to fail. Either install jq in the job (apt-get install -y jq) or parse JSON without jq (e.g. use a small Go/Python one-liner or use curl's --silent and a robust tool)..gitea/workflows/release.ymlinstall.shcmd/review-bot/main.govar version = "dev"for ldflags injection — that's fine. There's an extra blank line before the import block and some whitespace changes; these are harmless but could be cleaned up for readability..gitea/workflows/ci.ymlRecommendation
REQUEST_CHANGES — Request changes. Fix the functional regression and workflow problems before merging:
llm/client.go: Restore an explicit temperature value in the ChatRequest (e.g. Temperature: 0.1) if that was the intended behaviour, or document the intentional change clearly and update callers/tests. If you keep the omitempty tag, be explicit about default behaviour.
.gitea/workflows/release.yml:
install.sh: Consider using a more robust JSON parsing approach (or require jq) to fetch the latest tag, or use the releases/tags API to fetch a particular tag. Add graceful error messages and consider supporting more architectures if needed.
CI adjustments: Verify that changing/removing REVIEWER_NAME and changing LLM_MODEL values are intentional and compatible with the application; update code or workflow envs accordingly.
After addressing the above, re-run CI. Once the LLM temperature regression and release workflow issues are fixed and tests pass (go test ./... and go vet ./...), this change can be approved.
Summary
Useful additions (release workflow, installer, LLM temperature) but there are blocking issues that will likely break builds and releases. Specifically, an unused package-level version variable in main will fail compilation, the release workflow parses the release ID unreliably and may use the wrong context, and the install script will fail on macOS due to sha256sum.
Findings
cmd/review-bot/main.govar version = "dev"that is never used; in Go, unused variables cause a compile error. This will breakgo buildand the release workflow..gitea/workflows/release.yml.gitea/workflows/release.yml${{ github.server_url }}and${{ github.repository }}in a Gitea Actions workflow. On Gitea, the canonical context isgitea.*; relying ongithub.*may not be supported on all instances and can break the release step. Use${{ gitea.server_url }}and${{ gitea.repository }}.install.shsha256sum, which is not available by default on macOS. Since darwin binaries are built, macOS users will see the script fail. Add a fallback toshasum -a 256when OS is darwin.llm/client.goWithTemperaturelacks tests. Per project conventions, all exported functions should have tests..gitea/workflows/ci.ymlRecommendation
REQUEST_CHANGES — Address the following before merging: (1) Use the injected version variable in cmd/review-bot/main.go (e.g., print on startup or provide a -version flag) or remove it to avoid a compile error. (2) Make the release workflow robust: use the Gitea context variables (
gitea.server_url,gitea.repository) and fetch the release ID reliably (e.g., call GET /api/v1/repos/{repo}/releases/tags/{tag} and parse that single object, or at least grep for the top-level"id"key). (3) Update install.sh to support macOS checksum verification by usingshasum -a 256when OS is darwin. Additionally, add tests for the new exportedWithTemperaturemethod to adhere to project conventions. Once these issues are resolved, the release and installation flow should be reliable and the feature is a solid enhancement.Summary
The PR adds a release workflow, installer script, and temperature plumbing for the LLM client. Functional changes are mostly reasonable, but there is a critical build break in cmd/review-bot/main.go (a missing import), plus several smaller issues and robustness improvements to address before merging.
Findings
cmd/review-bot/main.gollm/client.gollm/client.gollm/client.goinstall.sh.gitea/workflows/release.yml.gitea/workflows/ci.ymlcmd/review-bot/main.goRecommendation
REQUEST_CHANGES — Request changes. The blocker is the missing strconv import in cmd/review-bot/main.go; add strconv to the import list (run go build/go test to verify). After fixing that, run go test ./... and go vet ./... locally to validate. Also:
Once the import/build issue and the suggested hardening items are addressed and CI passes, this can be approved.
- .gitea/actions/review/action.yml: composite action with caching Consumers just use: uses: https://gitea.weiker.me/rodin/review-bot/.gitea/actions/review@v0.1.0 No Go toolchain needed, binary cached by version tag. - Remove install.sh (replaced by composite action) - CI workflow: use matrix strategy to parallelize reviews - Self-review still builds from source (pre-release)Summary
Overall, the release workflow and temperature plumbing look sound and align with the repository conventions. The changes are mostly additive and well-scoped, with CI passing. There are a few minor robustness and process issues worth addressing.
Findings
llm/client.gollm/client.gollm/client.go.gitea/actions/review/action.yml.gitea/workflows/release.yml.gitea/actions/review/action.yml.gitea/actions/review/action.ymlcmd/review-bot/main.goRecommendation
APPROVE — The changes are solid and CI is green. I recommend merging, with a follow-up to: add a small test for Client.WithTemperature, document the intended temperature defaults, and harden the composite action by verifying release checksums and handling OS/arch detection. If feasible, also make the release ID parsing slightly more robust, and consider writing binaries to a guaranteed-writable path in actions. None of these block merging but will improve reliability and security.
Summary
The PR adds CI/release workflows, an installer action, and LLM temperature support. However, there is a compile-breaking issue in cmd/review-bot (missing strconv import for the new envOrDefaultFloat) and a few other problematic/fragile spots that should be addressed before merging.
Findings
cmd/review-bot/main.gocmd/review-bot/main.gocmd/review-bot/main.gollm/client.go.gitea/actions/review/action.yml.gitea/actions/review/action.yml.gitea/workflows/release.yml.gitea/workflows/ci.ymlRecommendation
REQUEST_CHANGES — Fix the compilation error first by importing strconv in cmd/review-bot/main.go (add "strconv" to the import block). Run go build, go test ./..., and go vet ./... locally to catch any other issues. For robustness: (1) consider validating the llm temperature flag value range and documenting that 0 means server default; (2) avoid caching directly to /usr/local/bin — use a temporary, user-writable cache directory and move the binary to /usr/local/bin after restore; (3) make the release creation/upload step more robust (avoid parsing JSON with sed); and (4) verify that GitHub Actions-specific constructs in .gitea/ files are intentional and compatible with your CI runner. After these changes and a successful CI run, the PR can be approved.
Summary
Overall the release workflow and composite action look reasonable, and the LLM temperature support is a useful addition. However, an exported method was added without corresponding tests per the repository conventions, and there are a few robustness and reusability issues in the workflows.
Findings
llm/client.go.gitea/actions/review/action.yml.gitea/actions/review/action.yml.gitea/actions/review/action.yml.gitea/workflows/release.ymlcmd/review-bot/main.go.gitea/workflows/ci.ymlRecommendation
REQUEST_CHANGES — Please add unit tests for the new exported method llm.Client.WithTemperature in accordance with the repository's testing conventions. In the composite action, remove the hard-coded repository path and use the provided repo input (or github.repository) both when determining the latest version and when downloading the binary; also consider detecting OS/architecture to download the correct asset and adjust the cache key. For the JSON parsing in both the action and the release workflow, replace the brittle sed-based extraction with a more robust approach (e.g., jq if available on the runner, or more precise grep/sed anchored to the specific fields), to avoid misparsing. Additionally, tidy up minor style issues (indentation in main.go) and clarify the CI matrix names/models. Once tests are added and these adjustments are made, the changes should be good to merge.
Summary
The changes add release and CI workflows and LLM temperature handling, which are reasonable additions, but there is at least one correctness issue in the Go code that will break builds (missing import) and a few smaller style/clarity issues to address before merging.
Findings
cmd/review-bot/main.gocmd/review-bot/main.gocmd/review-bot/main.gollm/client.gollm/client.go.gitea/actions/review/action.ymlRecommendation
REQUEST_CHANGES — Request changes: fix the build-blocking issue by ensuring strconv is imported in cmd/review-bot/main.go (or verify it was already present), run gofmt and go vet, and consider the minor clarity/style improvements listed above. After addressing these, re-run CI (go test ./... and go vet ./...) to ensure everything builds and tests pass before merging.
Summary
Solid improvements to release automation and the LLM client, but a new exported API lacks tests and a few workflow robustness issues remain. Address these to align with repository conventions and improve reliability.
Findings
llm/client.go.gitea/actions/review/action.yml.gitea/actions/review/action.yml.gitea/actions/review/action.ymlllm/client.gocmd/review-bot/main.go.gitea/workflows/release.ymlRecommendation
REQUEST_CHANGES — Please add table-driven tests covering the new exported WithTemperature method in the llm package (including behavior when temperature is unset, set within range, and edge cases). In the composite action, replace the sed-based version detection with a more robust JSON parsing approach (similar to the Python usage in release.yml) and verify downloaded binary checksums prior to execution. Also, either remove or wire up the 'repo' input to avoid hardcoding 'rodin/review-bot' in the Determine version and Install steps. Document the intended change in default temperature behavior (server default vs fixed 0.1) and ensure existing tests reflect this. After these changes, the PR should be ready to merge.
Summary
The changes add a release workflow, a composite action, and temperature support for the LLM. However, the Go changes introduce a build break (missing import) and a behavioral change to LLM temperature handling that should be called out and (if unintended) reverted. Please fix the compile error and address the minor issues below.
Findings
cmd/review-bot/main.gollm/client.gollm/client.goomitempty). This means that unless a non-zero temperature is explicitly set, the temperature field will be omitted and server default used — different from previous 0.1 default. Ensure this behavioral change is intended and documented..gitea/workflows/release.yml.gitea/actions/review/action.ymlRecommendation
REQUEST_CHANGES — Please add the missing import for strconv in cmd/review-bot/main.go (and run go build/go test/go vet locally) so the project compiles. Confirm whether the change to the LLM default temperature (removing the previous 0.1 default and using omission to defer to server defaults) is intentional; if not, restore the previous default or document the change. Tighten the WithTemperature comment for clarity. For the release workflow, consider avoiding a hard dependency on python3 for JSON parsing or explicitly document that python3 is required on the runner. Finally, prefer more robust JSON parsing (jq or the API) instead of sed when determining the latest release in the composite action. After addressing the compile error and these suggestions, re-run CI to ensure all checks pass.
Summary
The changes add release automation, a reusable Gitea composite action, and support for LLM temperature in the client and CLI. Code and tests are consistent and CI passed; only small improvements and clarifications are recommended.
Findings
cmd/review-bot/main.goversion = "dev"was added for ldflags injection. This is fine, but consider adding a short comment explaining its purpose and intended ldflags usage so future maintainers understand it's expected to be overwritten at build time.cmd/review-bot/main.gocmd/review-bot/main.gollm/client.gotemperature,omitempty. This behaves correctly (omits zero), but ensure callers understand that 0 means 'omit' and not an explicit 0.0 sent to the server — the comment on WithTemperature helps, but consider documenting this behavior on the Client type as well.llm/client.gollm/client.gollm/client_test.go.gitea/workflows/release.yml.gitea/actions/review/action.ymlRecommendation
APPROVE — Approve: the implementation is sound, tests were added to cover the new temperature feature, and CI passed. Please address the minor items: add a brief comment for the version variable, consider a non-fatal validation pattern to keep main logic testable, add documentation/comments clarifying how temperature=0 is treated (omit vs explicit 0), tighten floating-point assertions in tests (use epsilon), and either include the install.sh mentioned in the PR or update the description. Also consider authenticating API calls where appropriate for private repositories (or documenting the public-repo requirement). These changes are small and non-blocking.
Summary
Solid addition of a release workflow and LLM temperature handling, plus a useful composite action for running reviews. Code and tests are straightforward and adhere to conventions; CI has passed.
Findings
.gitea/actions/review/action.yml.gitea/actions/review/action.yml.gitea/actions/review/action.ymlRecommendation
APPROVE — Overall, this PR is well-executed: the release workflow builds four target binaries, includes checksum generation, and robustly creates or reuses a Gitea release. The LLM client’s temperature handling is idiomatic with an omitempty JSON field, a chainable setter, and tests covering both inclusion and omission. The composite action for installation and execution is pragmatic and includes checksum verification. Please consider aligning the default repo behavior with its description in the composite action and hardening the cache/permissions behavior. Otherwise, this is good to merge.
Summary
Solid addition of a release workflow, a reusable composite action, and LLM temperature handling with tests. Changes are coherent and CI passed; no correctness or design blockers found.
Findings
.gitea/actions/review/action.yml.gitea/actions/review/action.yml.gitea/workflows/release.ymlllm/client_test.goRecommendation
APPROVE — The PR is well-structured and aligns with project conventions. The release workflow builds and uploads multi-arch binaries, the composite action provides a convenient installer with checksum verification, and the LLM temperature feature is implemented cleanly with tests ensuring correct omission/inclusion. Address the minor notes to improve portability and robustness (e.g., support for other runner architectures, stricter bash settings, explicit CGO settings in cross-builds, and a small test refinement). Otherwise, this is good to merge.
Summary
The changes add CI release automation, an install action, a temperature option for the LLM client, and update prompts — overall implementations look correct and CI is reported as passing. A few minor improvements and robustness suggestions are noted below.
Findings
.gitea/actions/review/action.yml.gitea/actions/review/action.yml.gitea/workflows/release.ymlcmd/review-bot/main.gollm/client.goRecommendation
APPROVE — Approve — the PR is well-structured and CI is reported as passing. The changes are small and focused: release workflow, install action, LLM temperature support, and prompt improvements. Address the minor robustness suggestions: make the action's JSON parsing and checksum verification more defensive (or document dependencies), surface parse errors from envOrDefaultFloat (or at least warn), and consider documenting or validating temperature ranges in the LLM client. None of these require blocking the merge.
Summary
Overall the release workflow, installer action, and code changes are well-structured and covered by tests; CI passed. However, the composite action conflates the PR repository input with the binary download repository, which will break valid use cases and needs to be corrected before merging.
Findings
.gitea/actions/review/action.yml.gitea/actions/review/action.yml.gitea/workflows/release.yml.gitea/workflows/release.ymlRecommendation
REQUEST_CHANGES — Please decouple the binary download repository from the PR target repository in the composite action. Introduce a separate input (e.g., 'bot-repo') defaulting to 'rodin/review-bot' and use that for determining the version and downloading assets, while keeping 'repo' for the PR under review. Also, update the installer step to select the correct asset based on runner OS/arch and adjust the cache key accordingly. In the release workflow, add a 'go vet ./...' step to conform to repository conventions, and consider making asset uploads idempotent to handle reruns without failure. With these changes, the PR should be safe to merge.
Summary
The changes add a release workflow, an install action, LLM temperature support, and richer prompts while updating clients and tests. The code is well integrated, tests were extended accordingly, and I found no blocking defects; only minor suggestions and nits to improve robustness and clarity.
Findings
gitea/client.gocmd/review-bot/main.goversionis declared but never referenced in the CLI. This is fine for ldflags injection, but consider using it (e.g. expose --version) so the compiled binary can report its version at runtime.llm/client.goomitempty(good). However, floating-point zero values can be subtle; the current approach relies on zero meaning 'omit'. If callers may explicitly want temperature=0.0, document this behavior clearly (or use *float64 to distinguish unset vs. set-to-zero)..gitea/actions/review/action.ymlreview/prompt.goRecommendation
APPROVE — Approve: the PR is well-structured and the new functionality (release workflow, installer, version variable, LLM temperature support, expanded prompts, and corresponding tests) integrates cleanly. Address the minor robustness items: conditionally set Authorization headers when token is empty in gitea client, consider exposing the injected version in the CLI, document the runner prerequisites used by the composite action, and optionally clarify temperature semantics (or use a pointer) if you need to distinguish unset vs explicit zero. None of these changes are blocking; run the usual
go test ./...andgo vet ./...(CI already passed) and consider the small improvements above in a follow-up commit.Summary
Well-structured addition of release/workflow files and LLM/PR enhancements. Code compiles logically and tests have been updated to exercise new functionality; changes are idiomatic and conform to the repo conventions. I have a few minor suggestions to improve robustness and correctness edge-cases.
Findings
cmd/review-bot/main.gogitea/client.gogitea/client.gollm/client.gollm/client_test.go.gitea/workflows/ci.yml.gitea/actions/review/action.ymlRecommendation
APPROVE — Approve these changes. The new workflows and installer script are well integrated and the Go code updates add useful capabilities (version injection, fetching file contexts, patterns, and LLM temperature control). Please consider addressing the minor robustness suggestions: URL-escape file paths/refs in client methods, avoid sending empty Authorization header, and optionally tighten the tests by type-asserting interface values before numeric comparisons. Also add a short comment explaining the package-level 'version' variable is for ldflags. These are small improvements and not blockers.
Summary
Overall, this PR cleanly adds a release workflow, a composite install/run action, and extends the CLI and clients with patterns support and temperature handling. The implementations are correct and tests cover the new exported functions. I have a few minor suggestions to strengthen robustness and alignment with repo conventions.
Findings
.gitea/workflows/release.ymlgo vet ./.... Repository conventions require vet to pass before commit; consider adding a vet step to the release workflow as well to keep parity with CI..gitea/actions/review/action.ymlreleases?limit=1and assumes the API returns the newest first. If the API order changes or differs, this could select the wrong release. Consider explicitly ensuring sort order or handling pagination to robustly pick the latest tag.cmd/review-bot/main.gogitea/client.goRecommendation
APPROVE — The changes are well-structured and adhere to the repository's conventions. The new features (release workflow, composite action installer, patterns loading, and LLM temperature control) are implemented cleanly, with good test coverage for newly exported functions. Please consider the minor improvements: add
go vetto the release workflow, make the latest-version detection in the composite action more robust, use an explicit language for file-context code fences, and refine error handling inGetAllFilesInPathto avoid masking non-404 errors. None of these are blockers; approving as-is.Summary
Well-structured addition of release workflow, composite action, installer integration points, and LLM/PR context enhancements. Changes are consistent, tests updated accordingly, and CI passed.
Findings
.gitea/actions/review/action.ymlllm/client.gogitea/client.go.gitea/workflows/release.ymlcmd/review-bot/main.goRecommendation
APPROVE — Approve the changes. The new workflows, CLI flags, LLM temperature support, and expanded Gitea client functionality are coherent and well-tested. Address the minor recommendations: (1) ensure required runner tools (python3, sha256sum) are present or documented in the composite action, (2) consider small optimization in PostReview to avoid an extra allocation, (3) make release asset uploads idempotent by handling existing assets, and (4) optionally expose the embedded version at runtime for easier debugging/troubleshooting. None of these are blockers for merging.
Summary
Well-structured addition of a release workflow, composite install/action, and enhancements to support patterns, file context, and LLM temperature. Code is idiomatic, tests cover new functionality, and CI has passed.
Recommendation
APPROVE — The release workflow, composite action, and code changes look solid and consistent with the repository conventions. New functionality (file context, external patterns, and temperature handling) is implemented cleanly with corresponding tests. Since CI has passed and no major issues were found, this is good to merge.
rodin referenced this pull request2026-05-12 16:59:58 +00:00