feat: add automatic update check notification for binary users#1039
Merged
feat: add automatic update check notification for binary users#1039
Conversation
Cache-based update check triggered by CLI commands and Web UI page load. Fetches latest release from GitHub API with 24h cache staleness and 3s timeout. CLI prints one-liner to stderr (suppressed by --quiet, skipped for source builds). Web UI shows pulsing badge in TopNav linking to release page. Also fixes release skill asset count (6 -> 7).
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds an update-check feature: a cached GitHub release checker (24h TTL) with version comparison, CLI notice for bundled binaries, server API Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Process
participant Cache as Local Cache
participant GH as GitHub API
participant Server as Server
participant Web as Web Client
rect rgba(100, 150, 200, 0.5)
Note over CLI: CLI Update Check Flow
CLI->>Cache: getCachedUpdateCheck(currentVersion)
alt Cache exists & fresh
Cache-->>CLI: cached UpdateCheckResult
else Cache stale/missing
CLI->>GH: GET /releases/latest (3s timeout)
GH-->>CLI: release payload (tag_name, html_url)
CLI->>Cache: write update-check.json
Cache-->>CLI: write result (errors ignored)
CLI-->>CLI: isNewerVersion -> UpdateCheckResult
end
CLI->>CLI: printUpdateNotice if updateAvailable
end
rect rgba(150, 200, 100, 0.5)
Note over Web,Server: Web Update Check Flow
Web->>Server: GET /api/update-check
Server->>Cache: getCachedUpdateCheck(appVersion)
alt Cache exists & fresh
Cache-->>Server: cached UpdateCheckResult
else Cache stale/missing
Server->>GH: GET /releases/latest (3s timeout)
GH-->>Server: release payload
Server->>Cache: write update-check.json
end
Server-->>Web: UpdateCheckResult
Web->>Web: display pulsing link if updateAvailable (refetch hourly)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
PR Review SummaryCritical Issues (2 found)
Important Issues (5 found)
Suggestions (6 found)
Documentation Issues
Strengths
Verdict: NEEDS FIXESTwo critical issues must be addressed before merge:
Recommended Actions
|
- Add BUNDLED_IS_BINARY guard to /api/update-check server route to prevent unintended GitHub API calls from source/dev builds - Replace hand-crafted UpdateCheckResult interface with generated OpenAPI type (components['schemas']['UpdateCheckResponse']) - Add staleness + checkedAt validation to getCachedUpdateCheck, matching readCache behavior - Add debug-level logging to all bare catch blocks in update-check.ts for --verbose diagnostics - Add releaseUrl guard in TopNav to prevent empty href links - Fix SKILL.md: correct CI scope claim (Step 10 only, not 10-11) and clarify merge commit sync note - Add tests: non-200 HTTP response, stale cache for getCachedUpdateCheck, missing checkedAt, and cache content verification - Document /api/update-check endpoint and update-check.json cache file in CLAUDE.md and docs-web - Regenerate api.generated.d.ts with UpdateCheckResponse schema
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/paths/src/update-check.ts (1)
27-43: Consider extracting shared cache validation logic.
readCache()andgetCachedUpdateCheck()(lines 138-153) both parse and validate the cache JSON with similar logic. The only difference is staleness handling. Consider extracting the common validation into a helper to reduce duplication and ensure consistent validation.♻️ Suggested refactor
+function parseCache(raw: string): UpdateCheckCache | null { + try { + const data = JSON.parse(raw) as UpdateCheckCache; + if (!data.latestVersion || !data.releaseUrl || typeof data.checkedAt !== 'number') { + return null; + } + return data; + } catch { + return null; + } +} + function readCache(): UpdateCheckCache | null { try { const cachePath = getCachePath(); if (!existsSync(cachePath)) return null; const raw = readFileSync(cachePath, 'utf-8'); - const data = JSON.parse(raw) as UpdateCheckCache; - if (!data.latestVersion || !data.releaseUrl || typeof data.checkedAt !== 'number') { - return null; - } + const data = parseCache(raw); + if (!data) return null; if (Date.now() - data.checkedAt > STALENESS_MS) { return null; } return data; } catch { return null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paths/src/update-check.ts` around lines 27 - 43, readCache() and getCachedUpdateCheck() duplicate JSON parse and validation logic; extract a helper (e.g., parseAndValidateCache or validateCache) that takes raw JSON or parsed object and an optional maxAgeMs (use STALENESS_MS for readCache and undefined/Infinity for getCachedUpdateCheck) and verifies latestVersion, releaseUrl and that checkedAt is a number and not stale when maxAgeMs provided; update readCache and getCachedUpdateCheck to call this helper, handle parse errors centrally, and return null on failure to keep behavior identical.packages/paths/src/update-check.test.ts (1)
3-3: Remove unused importunlinkSync.The
unlinkSyncimport is not used in this file.🧹 Fix unused import
-import { mkdirSync, writeFileSync, existsSync, rmSync, unlinkSync } from 'fs'; +import { mkdirSync, writeFileSync, existsSync, rmSync } from 'fs';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paths/src/update-check.test.ts` at line 3, The import list at the top of update-check.test.ts includes an unused symbol unlinkSync; remove unlinkSync from the named import list (leaving mkdirSync, writeFileSync, existsSync, rmSync) so there are no unused imports reported and the test file only imports symbols that are actually referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/release/SKILL.md:
- Around line 199-201: Update the note that currently says the `update-homebrew`
CI job handles "Steps 10-11" to correctly state it only handles Step 10;
specifically, edit the text referencing the `update-homebrew` job (the line
mentioning "Steps 10-11" in SKILL.md) so it reads that the CI job only updates
the main formula and pushes to dev (Step 10) and does not sync the Homebrew tap
(Step 11), and add a short reminder to check Actions and perform the manual tap
sync (Step 11: syncing coleam00/homebrew-archon) if the CI job does not cover
it.
---
Nitpick comments:
In `@packages/paths/src/update-check.test.ts`:
- Line 3: The import list at the top of update-check.test.ts includes an unused
symbol unlinkSync; remove unlinkSync from the named import list (leaving
mkdirSync, writeFileSync, existsSync, rmSync) so there are no unused imports
reported and the test file only imports symbols that are actually referenced.
In `@packages/paths/src/update-check.ts`:
- Around line 27-43: readCache() and getCachedUpdateCheck() duplicate JSON parse
and validation logic; extract a helper (e.g., parseAndValidateCache or
validateCache) that takes raw JSON or parsed object and an optional maxAgeMs
(use STALENESS_MS for readCache and undefined/Infinity for getCachedUpdateCheck)
and verifies latestVersion, releaseUrl and that checkedAt is a number and not
stale when maxAgeMs provided; update readCache and getCachedUpdateCheck to call
this helper, handle parse errors centrally, and return null on failure to keep
behavior identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82a47870-1fa2-4bdf-87c5-d31db443942f
📒 Files selected for processing (9)
.claude/skills/release/SKILL.mdpackages/cli/src/cli.tspackages/paths/src/index.tspackages/paths/src/update-check.test.tspackages/paths/src/update-check.tspackages/server/src/routes/api.tspackages/server/src/routes/schemas/system.schemas.tspackages/web/src/components/layout/TopNav.tsxpackages/web/src/lib/api.ts
- Deduplicate getCachedUpdateCheck by delegating to readCache - Extract shared noUpdate fallback object in server route - Move guard clause outside try block in printUpdateNotice - Fix cachePath variable scoping in readCache catch block
Merged
This was referenced Apr 13, 2026
Tyone88
pushed a commit
to Tyone88/Archon
that referenced
this pull request
Apr 16, 2026
…m00#1039) * feat: add automatic update check notification for binary users Cache-based update check triggered by CLI commands and Web UI page load. Fetches latest release from GitHub API with 24h cache staleness and 3s timeout. CLI prints one-liner to stderr (suppressed by --quiet, skipped for source builds). Web UI shows pulsing badge in TopNav linking to release page. Also fixes release skill asset count (6 -> 7). * fix: address review findings for update check notification - Add BUNDLED_IS_BINARY guard to /api/update-check server route to prevent unintended GitHub API calls from source/dev builds - Replace hand-crafted UpdateCheckResult interface with generated OpenAPI type (components['schemas']['UpdateCheckResponse']) - Add staleness + checkedAt validation to getCachedUpdateCheck, matching readCache behavior - Add debug-level logging to all bare catch blocks in update-check.ts for --verbose diagnostics - Add releaseUrl guard in TopNav to prevent empty href links - Fix SKILL.md: correct CI scope claim (Step 10 only, not 10-11) and clarify merge commit sync note - Add tests: non-200 HTTP response, stale cache for getCachedUpdateCheck, missing checkedAt, and cache content verification - Document /api/update-check endpoint and update-check.json cache file in CLAUDE.md and docs-web - Regenerate api.generated.d.ts with UpdateCheckResponse schema * refactor: simplify update check code - Deduplicate getCachedUpdateCheck by delegating to readCache - Extract shared noUpdate fallback object in server route - Move guard clause outside try block in printUpdateNotice - Fix cachePath variable scoping in readCache catch block
joaobmonteiro
pushed a commit
to joaobmonteiro/Archon
that referenced
this pull request
Apr 26, 2026
…m00#1039) * feat: add automatic update check notification for binary users Cache-based update check triggered by CLI commands and Web UI page load. Fetches latest release from GitHub API with 24h cache staleness and 3s timeout. CLI prints one-liner to stderr (suppressed by --quiet, skipped for source builds). Web UI shows pulsing badge in TopNav linking to release page. Also fixes release skill asset count (6 -> 7). * fix: address review findings for update check notification - Add BUNDLED_IS_BINARY guard to /api/update-check server route to prevent unintended GitHub API calls from source/dev builds - Replace hand-crafted UpdateCheckResult interface with generated OpenAPI type (components['schemas']['UpdateCheckResponse']) - Add staleness + checkedAt validation to getCachedUpdateCheck, matching readCache behavior - Add debug-level logging to all bare catch blocks in update-check.ts for --verbose diagnostics - Add releaseUrl guard in TopNav to prevent empty href links - Fix SKILL.md: correct CI scope claim (Step 10 only, not 10-11) and clarify merge commit sync note - Add tests: non-200 HTTP response, stale cache for getCachedUpdateCheck, missing checkedAt, and cache content verification - Document /api/update-check endpoint and update-check.json cache file in CLAUDE.md and docs-web - Regenerate api.generated.d.ts with UpdateCheckResponse schema * refactor: simplify update check code - Deduplicate getCachedUpdateCheck by delegating to readCache - Extract shared noUpdate fallback object in server route - Move guard clause outside try block in printUpdateNotice - Fix cachePath variable scoping in readCache catch block
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
Label Snapshot
risk: lowsize: Mpaths,cli,server,web,skillspaths:update-check,cli:notification,server:api,web:topnavChange Metadata
featuremultiLinked Issue
Validation Evidence (required)
bun run validate # All passSecurity Impact (required)
/repos/coleam00/Archon/releases/latest(public, no auth, 60 req/hr limit)~/.archon/update-check.jsoncache fileCompatibility / Migration
Human Verification (required)
scripts/build-binaries.sh)Side Effects / Blast Radius (required)
--quietflag suppresses notice entirely.Rollback Plan (required)
--quietsuppresses CLI notice. Source builds (BUNDLED_IS_BINARY === false) skip entirely.Risks and Mitigations
Summary by CodeRabbit
New Features
Documentation
Tests