Conversation
Adds scripts/install/install.sh, which installs brush from official GitHub releases: picks the target (glibc >= 2.34 or the static musl build on Linux), resolves "latest" to a tag once, verifies the SHA-256 checksum and, when gh >= 2.68 is available, the build attestation (--require-attestation makes that mandatory). The script never touches gh's auth state; if gh reports it has no credentials (exit status 4), verification is skipped with a note pointing out the stronger check an authenticated gh would provide. Installs to $XDG_BIN_HOME if set, else ~/.local/bin. Stages to a mktemp name next to the destination, checks the binary runs, then renames atomically. Notes when the install dir isn't on PATH or is shadowed. Includes a pytest suite exercising real releases with tool shims, run in CI (via uvx, pytest pinned to 8.x) across Linux/macOS plus Alpine and AlmaLinux 8 containers and weekly, plus README instructions. The CD workflow gains a comment listing everything the installer depends on, including the glibc floor. Assisted-By: Claude Fable 5.1
The test asserted no note at all, but on hosts without gh the installer always emits the attestation note. Only PATH-related notes matter here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4iK6r1LQ9y3fX53cyqRR8
Performance Benchmark ReportCode Coverage Report: Only Changed Files listed
Minimum allowed coverage is Test Summary: bash-completion test suite
|
Test Results 5 files 49 suites 22m 58s ⏱️ Results for commit 7008210. ♻️ This comment has been updated with latest results. |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4iK6r1LQ9y3fX53cyqRR8
There was a problem hiding this comment.
🟡 Changes recommended
The documented curl pipelines can mask download failures, and glibc target selection lacks positive-path coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a release-binary installer with checksum and optional attestation verification.
Changes:
- Adds portable target detection and atomic installation.
- Adds cross-platform pytest coverage and CI.
- Documents curl-based installation and release dependencies.
File summaries
| File | Description |
|---|---|
scripts/install/install.sh |
Implements secure release installation. |
scripts/install/tests/test_install.py |
Tests installation and failure paths. |
.github/workflows/install-script.yaml |
Runs installer tests across platforms. |
.github/workflows/cd.yaml |
Documents installer/release coupling. |
README.md |
Adds installation instructions. |
Review details
Suppressed comments (2)
README.md:147
- As written, a failed
curlcan still make the overall pipeline succeed because POSIX pipelines report the finalshprocess's status; empty input exits successfully. Please replace this with an invocation that propagates download failures so scripted installs cannot silently do nothing.
curl --proto '=https' --tlsv1.2 -fsSL https://raw.githubusercontent.com/reubeno/brush/main/scripts/install/install.sh | sh
scripts/install/install.sh:134
- The suite only asserts the old-glibc fallback; the normal Linux install tests do not assert which archive was selected. A regression that sends glibc 2.34+ systems to the musl build would still pass because that binary also runs there. Add a
getconfshim returning 2.34 or newer and assert that the GNU asset is downloaded.
if printf '2.34\n%s\n' "${glibc_version}" | sort -CV 2>/dev/null; then
target="${arch}-unknown-linux-gnu"
else
target="${arch}-unknown-linux-musl"
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ### Quick start: | ||
|
|
||
| ```console | ||
| $ curl --proto '=https' --tlsv1.2 -fsSL https://raw.githubusercontent.com/reubeno/brush/main/scripts/install/install.sh | sh |
| # Installs brush from official GitHub releases of reubeno/brush. | ||
| # | ||
| # Usage: | ||
| # curl --proto '=https' --tlsv1.2 -fsSL https://raw.githubusercontent.com/reubeno/brush/main/scripts/install/install.sh | sh |
The suite only asserted the musl fallback, so a broken or inverted version comparison that sent every Linux install to the musl build would have passed unnoticed. Shim getconf to report exactly glibc 2.34 and assert the gnu archive is chosen; skip where the host glibc can't run it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4iK6r1LQ9y3fX53cyqRR8
Adds scripts/install/install.sh, which installs brush from official GitHub releases: picks the target (glibc >= 2.34 or the static musl build on Linux), resolves "latest" to a tag once, verifies the SHA-256 checksum and, when gh >= 2.68 is available, the build attestation (--require-attestation makes that mandatory). The script never touches gh's auth state; if gh reports it has no credentials (exit status 4), verification is skipped with a note pointing out the stronger check an authenticated gh would provide. Installs to $XDG_BIN_HOME if set, else ~/.local/bin. Stages to a mktemp name next to the destination, checks the binary runs, then renames atomically. Notes when the install dir isn't on PATH or is shadowed.
Includes a pytest suite exercising real releases with tool shims, run in CI (via uvx, pytest pinned to 8.x) across Linux/macOS plus Alpine and AlmaLinux 8 containers and weekly, plus README instructions. The CD workflow gains a comment listing everything the installer depends on, including the glibc floor.
Assisted-By: Claude Fable 5.1