Skip to content

Commit 93fc60e

Browse files
wesmclaude
andcommitted
fix: address code review findings across 11 reviews
- install.sh: make checksum verification fail-closed (exit on missing SHA256SUMS/checksum/tool unless AGENTSVIEW_SKIP_CHECKSUM=1) - release.sh: use ${1:-} for set -u safety, remove unused gh check - changelog.sh: handle no-tag repos (include root commit), quote $RANGE to prevent word-splitting - export.go: fix toggle CSS selector to match actual DOM structure (use ~ combinator through header element) - Heatmap.svelte: replace flex centering with margin:0 auto on SVG to preserve scroll origin on narrow viewports - update.go: mark cache-only UpdateInfo so CLI re-fetches full metadata for installs; surface rollback errors on failed install Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 60d0663 commit 93fc60e

7 files changed

Lines changed: 54 additions & 25 deletions

File tree

cmd/agentsview/update.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,19 @@ func runUpdate(args []string) {
5858
if *check {
5959
return
6060
}
61+
// Cache-only results lack download metadata; re-fetch.
62+
if info.NeedsRefetch() {
63+
info, err = update.CheckForUpdate(
64+
version, true, dataDir,
65+
)
66+
if err != nil {
67+
log.Fatalf("checking for updates: %v", err)
68+
}
69+
if info == nil {
70+
fmt.Println("Up to date.")
71+
return
72+
}
73+
}
6174
} else {
6275
fmt.Printf(
6376
"Update available: %s -> %s",

frontend/src/lib/components/analytics/Heatmap.svelte

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,12 +277,11 @@
277277
.heatmap-scroll {
278278
overflow-x: auto;
279279
padding-bottom: 4px;
280-
display: flex;
281-
justify-content: center;
282280
}
283281
284282
.heatmap-svg {
285283
display: block;
284+
margin: 0 auto;
286285
}
287286
288287
.day-label, .month-label {

internal/server/export.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,8 @@ main { max-width: 900px; margin: 0 auto; padding: 16px; }
460460
color: var(--text-primary);
461461
cursor: pointer; font-size: 11px;
462462
}
463-
.toggle-input:checked + .toggle-label {
463+
#thinking-toggle:checked ~ header label[for="thinking-toggle"],
464+
#sort-toggle:checked ~ header label[for="sort-toggle"] {
464465
background: var(--accent-blue); color: #fff;
465466
border-color: var(--accent-blue);
466467
}

internal/update/update.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ type UpdateInfo struct {
5050
Size int64
5151
Checksum string
5252
IsDevBuild bool
53+
// cacheOnly is set when the info came from cache and lacks
54+
// download metadata. The caller must re-fetch for installs.
55+
cacheOnly bool
56+
}
57+
58+
// NeedsRefetch returns true when the info came from cache
59+
// and lacks the download URL/checksum needed for an install.
60+
func (u *UpdateInfo) NeedsRefetch() bool {
61+
return u.cacheOnly
5362
}
5463

5564
// findAssets locates the platform binary and checksums file.
@@ -288,7 +297,12 @@ func installBinaryTo(srcPath, dstPath string) error {
288297
}
289298

290299
if err := copyFile(srcPath, dstPath); err != nil {
291-
_ = os.Rename(backupPath, dstPath)
300+
if restoreErr := os.Rename(backupPath, dstPath); restoreErr != nil {
301+
return fmt.Errorf(
302+
"install: %w (rollback also failed: %v)",
303+
err, restoreErr,
304+
)
305+
}
292306
return fmt.Errorf("install: %w", err)
293307
}
294308

@@ -645,10 +659,14 @@ func checkCache(
645659
latestVersion := strings.TrimPrefix(cached.Version, "v")
646660

647661
if isDevBuild {
662+
// Cache only records the version, not full asset metadata.
663+
// Return nil so the caller re-fetches with full info when
664+
// an install (not just --check) is needed.
648665
return &UpdateInfo{
649666
CurrentVersion: currentVersion,
650667
LatestVersion: cached.Version,
651668
IsDevBuild: true,
669+
cacheOnly: true,
652670
}, true
653671
}
654672

scripts/changelog.sh

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,22 @@ if [ -n "$START_TAG" ] && [ "$START_TAG" != "-" ]; then
1919
else
2020
PREV_TAG=$(git describe --tags --abbrev=0 2>/dev/null || echo "")
2121
if [ -z "$PREV_TAG" ]; then
22-
FIRST_COMMIT=$(git rev-list --max-parents=0 HEAD)
23-
RANGE="$FIRST_COMMIT..HEAD"
22+
RANGE=""
2423
echo "No previous release found. Generating changelog for all commits..." >&2
2524
else
2625
RANGE="$PREV_TAG..HEAD"
2726
echo "Generating changelog from $PREV_TAG to HEAD..." >&2
2827
fi
2928
fi
3029

31-
COMMITS=$(git log $RANGE --pretty=format:"- %s (%h)" --no-merges)
32-
DIFF_STAT=$(git diff --stat $RANGE)
30+
if [ -n "$RANGE" ]; then
31+
COMMITS=$(git log "$RANGE" --pretty=format:"- %s (%h)" --no-merges)
32+
DIFF_STAT=$(git diff --stat "$RANGE")
33+
else
34+
COMMITS=$(git log --pretty=format:"- %s (%h)" --no-merges)
35+
EMPTY_TREE=$(git hash-object -t tree /dev/null)
36+
DIFF_STAT=$(git diff --stat "$EMPTY_TREE" HEAD)
37+
fi
3338

3439
if [ -z "$COMMITS" ]; then
3540
echo "No commits since last release" >&2

scripts/install.sh

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,17 @@ verify_checksum() {
6868
local filename="$3"
6969

7070
if [ ! -f "$checksums_file" ]; then
71-
warn "Checksum file not available, skipping verification"
72-
return 0
71+
if [ "${AGENTSVIEW_SKIP_CHECKSUM:-0}" = "1" ]; then
72+
warn "Checksum file not available, skipping verification (AGENTSVIEW_SKIP_CHECKSUM=1)"
73+
return 0
74+
fi
75+
error "Checksum file not available. Set AGENTSVIEW_SKIP_CHECKSUM=1 to bypass."
7376
fi
7477

7578
local expected
7679
expected=$(awk -v f="$filename" '{gsub(/^\*/, "", $2); if ($2==f) {print $1; exit}}' "$checksums_file")
7780
if [ -z "$expected" ]; then
78-
warn "No checksum found for $filename, skipping verification"
79-
return 0
81+
error "No checksum found for $filename in SHA256SUMS"
8082
fi
8183

8284
local actual
@@ -85,8 +87,7 @@ verify_checksum() {
8587
elif command -v shasum &>/dev/null; then
8688
actual=$(shasum -a 256 "$file" | cut -d' ' -f1)
8789
else
88-
warn "No sha256 tool available, skipping verification"
89-
return 0
90+
error "No sha256 tool available. Install coreutils or set AGENTSVIEW_SKIP_CHECKSUM=1 to bypass."
9091
fi
9192

9293
if [ "$expected" != "$actual" ]; then
@@ -124,11 +125,8 @@ install_from_release() {
124125
return 1
125126
fi
126127

127-
if download "${base_url}/SHA256SUMS" "$tmpdir/SHA256SUMS" 2>/dev/null; then
128-
verify_checksum "$tmpdir/release.tar.gz" "$tmpdir/SHA256SUMS" "$filename"
129-
else
130-
warn "WARNING: Could not download SHA256SUMS -- integrity not verified"
131-
fi
128+
download "${base_url}/SHA256SUMS" "$tmpdir/SHA256SUMS" 2>/dev/null || true
129+
verify_checksum "$tmpdir/release.tar.gz" "$tmpdir/SHA256SUMS" "$filename"
132130

133131
info "Extracting..."
134132
tar -xzf "$tmpdir/release.tar.gz" -C "$tmpdir"

scripts/release.sh

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ set -euo pipefail
33

44
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
55

6-
VERSION="$1"
6+
VERSION="${1:-}"
77
EXTRA_INSTRUCTIONS="${2:-}"
88

99
if [ -z "$VERSION" ]; then
@@ -30,11 +30,6 @@ if ! git diff-index --quiet HEAD --; then
3030
exit 1
3131
fi
3232

33-
if ! command -v gh &> /dev/null; then
34-
echo "Error: gh CLI is required. Install from https://cli.github.com/"
35-
exit 1
36-
fi
37-
3833
# Generate changelog
3934
CHANGELOG_FILE=$(mktemp)
4035
trap 'rm -f "$CHANGELOG_FILE"' EXIT

0 commit comments

Comments
 (0)