Enable Linux nightly AppImage auto-update in the System settings UI - #999
Conversation
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe desktop updater adds a Nightly channel, synchronizes status rendering and polling, supports acknowledged channel changes and restart actions, and forwards diagnostics. Tailwind builds now use the project dependency graph installed by CI. ChangesUpdater settings flow
Reproducible Tailwind builds
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AppUpdateUI
participant updateApi
participant renderFrom
AppUpdateUI->>updateApi: setChannel(stored)
updateApi-->>AppUpdateUI: acknowledged updater result
AppUpdateUI->>renderFrom: render status
renderFrom->>AppUpdateUI: update status line and action mode
AppUpdateUI->>updateApi: checkNow() or apply()
updateApi-->>AppUpdateUI: checking, downloading, or settled status
AppUpdateUI->>updateApi: getStatus() while busy
updateApi-->>AppUpdateUI: settled updater status
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
d2808a7 to
5228f18
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@static/js/settings.js`:
- Around line 448-460: Update the channel-switch flow around
updateApi.setChannel and the feedBack-update-channel localStorage key to track
the last acknowledged channel, persist the newly selected value only after
setChannel succeeds, and restore the acknowledged channel in both the dropdown
and localStorage when it fails. Keep the successful render behavior intact and
ensure failed switches cannot leave the UI or persisted state ahead of the
updater.
- Around line 260-271: Update the channel selector state handling in
showLinuxFallback and the related checking, downloading, and setChannel flows so
channel switching is disabled while an update is active, including checking and
downloading states. Preserve the selector-enabled behavior for Linux unsupported
states, and ensure busy-state transitions consistently apply the disabled state.
- Around line 294-296: Update the Linux auto-update guard in the settings flow
so only s.status === 'unsupported' or an actually unsupported Linux build enters
showLinuxFallback. Do not use s.platform === 'linux' alone as the condition;
supported Nightly AppImage states must continue through progress, polling,
Restart now rendering, and action-button handling.
🪄 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 Plus
Run ID: 96eeb413-e29a-4f28-aaae-da0c5bada435
⛔ Files ignored due to path filters (1)
static/tailwind.min.cssis excluded by!**/*.min.css
📒 Files selected for processing (2)
static/js/settings.jsstatic/v3/index.html
| if (s.status === 'unsupported' || s.platform === 'linux') { | ||
| showLinuxFallback('Auto-update requires the AppImage build on the Nightly channel.'); | ||
| return; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not classify every Linux status as unsupported.
s.platform === 'linux' also matches supported Nightly AppImage states, bypassing progress, polling, and “Restart now” rendering while disabling the action button.
Proposed fix
- if (s.status === 'unsupported' || s.platform === 'linux') {
+ if (s.status === 'unsupported') {
showLinuxFallback('Auto-update requires the AppImage build on the Nightly channel.');
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (s.status === 'unsupported' || s.platform === 'linux') { | |
| showLinuxFallback('Auto-update requires the AppImage build on the Nightly channel.'); | |
| return; | |
| if (s.status === 'unsupported') { | |
| showLinuxFallback('Auto-update requires the AppImage build on the Nightly channel.'); | |
| return; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@static/js/settings.js` around lines 294 - 296, Update the Linux auto-update
guard in the settings flow so only s.status === 'unsupported' or an actually
unsupported Linux build enters showLinuxFallback. Do not use s.platform ===
'linux' alone as the condition; supported Nightly AppImage states must continue
through progress, polling, Restart now rendering, and action-button handling.
5228f18 to
2c1d34b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@static/js/settings.js`:
- Around line 457-472: Update the channelSelect change handler to prevent stale
overlapping setChannel() completions from updating state. Use a request sequence
or disable the selector until each request settles, ensuring only the latest
acknowledged switch updates _appUpdateAckedChannel, localStorage, and renderFrom
while preserving the existing validation and error behavior.
- Line 254: Update the acknowledged-channel tracking around
_appUpdateAckedChannel so stored preferences are not treated as authoritative
during initialization. Assign it only after setChannel() succeeds or after an
authoritative getStatus() response, including the corresponding logic near the
additional referenced flow, and preserve failure handling without reverting to
an unacknowledged channel.
- Around line 402-416: The pollWhileBusy status loop currently allows
overlapping requests and synchronous getStatus throws to escape handling.
Replace the setInterval flow with recursive setTimeout scheduling that invokes
updateApi.getStatus inside a promise callback, renders each settled response,
stops on terminal status or rejection, and schedules the next poll only after
the current request settles.
- Around line 489-498: Update the status-error branch in the updater apply flow
to restore the restart action after renderFrom() handles a resolved `{status:
'error'}` response. Ensure checkBtn displays “Restart now” and is enabled,
matching the catch path, while preserving the existing failure message
rendering.
- Around line 356-374: Update the diagnostics contribution in the settings
rendering flow to use a dedicated ID for this snapshot instead of audio_engine.
Keep the existing payload and rendering behavior unchanged, ensuring the
snapshot no longer overwrites the real audio diagnostics contribution.
🪄 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 Plus
Run ID: 84534ac8-f17f-411c-8ad6-adfe790b948c
⛔ Files ignored due to path filters (1)
static/tailwind.min.cssis excluded by!**/*.min.css
📒 Files selected for processing (2)
static/js/settings.jsstatic/v3/index.html
🚧 Files skipped from review as they are similar to previous changes (1)
- static/v3/index.html
| try { storedRaw = localStorage.getItem('feedBack-update-channel') || localStorage.getItem('slopsmith-update-channel'); } catch (_) { /* fall through */ } | ||
| const stored = APP_UPDATE_CHANNELS.includes(storedRaw) ? storedRaw : 'stable'; | ||
| channelSelect.value = stored; | ||
| _appUpdateAckedChannel = stored; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Only initialize the acknowledged channel from an authoritative success.
Line 254 treats the stored preference as main-acknowledged before setChannel() succeeds. If initial synchronization rejects—or storage failed after a previous successful switch—later failures revert the UI to a channel main never acknowledged. Assign this tracker only after successful setChannel() or an authoritative getStatus() response.
Also applies to: 442-447
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@static/js/settings.js` at line 254, Update the acknowledged-channel tracking
around _appUpdateAckedChannel so stored preferences are not treated as
authoritative during initialization. Assign it only after setChannel() succeeds
or after an authoritative getStatus() response, including the corresponding
logic near the additional referenced flow, and preserve failure handling without
reverting to an unacknowledged channel.
| // Live structured snapshot (overwrites, not a log) via the existing | ||
| // diagnostics contribute() API — 'audio_engine' is feedBack-desktop's | ||
| // own registered plugin id, so the server's diagnostics export won't | ||
| // filter it out. Always current, no scrolling through console history | ||
| // needed to answer "what does the app think is going on right now." | ||
| try { | ||
| window.feedBack?.diagnostics?.contribute('audio_engine', { | ||
| update: { | ||
| channel: s.channel || channelSelect.value, | ||
| status: s.status, | ||
| currentVersion: s.currentVersion ?? null, | ||
| lastChecked: s.lastChecked ?? null, | ||
| percent: typeof s.percent === 'number' ? s.percent : null, | ||
| message: s.message ?? null, | ||
| rendered: line, | ||
| ts: Date.now(), | ||
| }, | ||
| }); | ||
| } catch (_) { /* diagnostics.js not loaded — never let this break rendering */ } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
diag="$(fd -a '^diagnostics\.js$' . | head -n1)"
test -n "$diag" || { echo "diagnostics.js not found"; exit 1; }
ast-grep outline "$diag" --items all --match 'contribute|audio_engine'
rg -n -C4 '\bcontribute\s*\(|audio_engine' "$diag" static/js/settings.jsRepository: got-feedBack/feedBack
Length of output: 1958
Use a dedicated diagnostics ID here contribute() stores by pluginId (contributions[pluginId] = payload), so writing this snapshot under audio_engine will overwrite the real audio diagnostics entry.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@static/js/settings.js` around lines 356 - 374, Update the diagnostics
contribution in the settings rendering flow to use a dedicated ID for this
snapshot instead of audio_engine. Keep the existing payload and rendering
behavior unchanged, ensuring the snapshot no longer overwrites the real audio
diagnostics contribution.
| function pollWhileBusy() { | ||
| if (_appUpdatePollTimer) return; | ||
| _appUpdatePollTimer = setInterval(() => { | ||
| void Promise.resolve(updateApi.getStatus()).then((s) => { | ||
| renderFrom(s); | ||
| const st = s && s.status; | ||
| if (st !== 'downloading' && st !== 'checking') { | ||
| clearInterval(_appUpdatePollTimer); | ||
| _appUpdatePollTimer = null; | ||
| } | ||
| }).catch(() => { | ||
| clearInterval(_appUpdatePollTimer); | ||
| _appUpdatePollTimer = null; | ||
| }); | ||
| }, 1500); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Serialize status polling instead of overlapping getStatus() calls.
A slow request can overlap later ticks, allowing stale responses to overwrite a terminal status and restart polling. Also, Line 405 evaluates getStatus() before Promise.resolve, so synchronous throws bypass the rejection handler. Use recursive setTimeout after each request settles and invoke the bridge inside a promise callback.
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 403-415: React's useState should not be directly called
Context: setInterval(() => {
void Promise.resolve(updateApi.getStatus()).then((s) => {
renderFrom(s);
const st = s && s.status;
if (st !== 'downloading' && st !== 'checking') {
clearInterval(_appUpdatePollTimer);
_appUpdatePollTimer = null;
}
}).catch(() => {
clearInterval(_appUpdatePollTimer);
_appUpdatePollTimer = null;
});
}, 1500)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@static/js/settings.js` around lines 402 - 416, The pollWhileBusy status loop
currently allows overlapping requests and synchronous getStatus throws to escape
handling. Replace the setInterval flow with recursive setTimeout scheduling that
invokes updateApi.getStatus inside a promise callback, renders each settled
response, stops on terminal status or rejection, and schedules the next poll
only after the current request settles.
| if (r?.status === 'error') { | ||
| console.warn('[updater] apply returned error:', r.message || 'unknown'); | ||
| renderFrom(r, 'Restart failed.'); | ||
| } | ||
| // On success the app quits + relaunches — nothing to render. | ||
| } catch (e) { | ||
| console.warn('[updater] apply failed:', e); | ||
| statusEl.textContent = `Restart failed: ${e?.message || e}`; | ||
| checkBtn.textContent = 'Restart now'; | ||
| checkBtn.disabled = false; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve “Restart now” after resolved apply errors.
The rejection path restores restart mode, but {status: 'error'} goes through renderFrom() and switches the button back to “Check for updates.” Keep the staged-update restart action available consistently after either failure form.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@static/js/settings.js` around lines 489 - 498, Update the status-error branch
in the updater apply flow to restore the restart action after renderFrom()
handles a resolved `{status: 'error'}` response. Ensure checkBtn displays
“Restart now” and is enabled, matching the catch path, while preserving the
existing failure message rendering.
2c1d34b to
35192c9
Compare
…ttings
Fixes the Settings → System "App updates" panel so it actually works on
Linux, and adds Nightly as a selectable channel — previously missing
entirely, so Linux self-update couldn't be reached from this UI at all.
- The channel dropdown no longer gets permanently disabled the moment
the desktop bridge reports 'unsupported', which is the normal state
whenever the channel isn't Nightly on Linux. It stays enabled so the
user can switch to Nightly, the only way out of that state.
- Shows live download progress ("Downloading update… N%") and an
explicit button state machine (Check → grayed out while busy →
Restart now once staged), instead of a frozen "Checking…" during the
~1.5GB background download.
- Renders every status update from the triggering action's own return
value (checkNow()/setChannel()'s result) rather than a separate
follow-up getStatus() call, which can race against other state
changes and show a stale result even after a real success.
- setupAppUpdates() no longer re-syncs the channel to the backend on
every Settings-panel re-render — only once per page load — so a
redundant sync can no longer stomp an in-flight download's state.
- Routes update-flow events into the existing diagnostics.js
console-capture + contribute() snapshot API, so the user's existing
"Export Diagnostics" button now captures the full update decision
trace end to end — no new UI or log file. This diagnostic tracing is
what actually root-caused the bugs above, from real on-device
captures rather than guesswork.
Companion PR in feedBack-desktop (the underlying update engine).
Verified end-to-end on a Steam Deck: channel switch → check → live
download progress → restart button → relaunch onto the new build,
confirmed via a real Export Diagnostics capture showing a clean,
fully-accounted-for trace.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
35192c9 to
c991a92
Compare
…edupe diag log - Extract the status→UI state machine from renderFrom into a pure, exported _appUpdateStatusView() (DOM-free) and cover it with tests/js — settings.js's large module graph made importing it for a full harness impractical, so the pure function is the testable seam. Behavior-preserving; renderFrom applies the returned shape to the DOM exactly as before. - Dedupe the [update-diag] renderFrom console line so the ~1.5s download poll no longer floods the diagnostics ring buffer with byte-identical entries; every real state/percent change still logs, and the structured contribute() snapshot stays unconditional. Left the 'audio_engine' diagnostics key as-is: the server export filters client contributions to loaded plugin ids (diagnostics_bundle.py path-traversal guard), so a dedicated key would be silently dropped from the bundle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
Summary
Fixes the Settings → System "App updates" panel so it actually works on Linux, and adds Nightly as a selectable channel — previously missing entirely, so Linux self-update couldn't be reached from this UI at all.
unsupported, which is the normal state whenever the channel isn't Nightly on Linux. It stays enabled so the user can switch to Nightly, the only way out of that state.checkNow()/setChannel()'s result) rather than a separate follow-upgetStatus()call, which can race against other state changes and show a stale result even after a real success.setupAppUpdates()no longer re-syncs the channel to the backend on every Settings-panel re-render — only once per page load — so a redundant sync can no longer stomp an in-flight download's state.diagnostics.jsconsole-capture +contribute()snapshot API, so the user's existing "Export Diagnostics" button now captures the full update decision trace end to end — no new UI or log file. This diagnostic tracing is what actually root-caused the bugs above, from real on-device captures rather than guesswork.Companion PR in feedBack-desktop (the underlying update engine): got-feedBack/feedBack-desktop#119
Test plan
window.feedBackDesktop.updateandwindow.feedBack.diagnostics) reproducing the exact bug sequence before confirming the fix, for each of the fixes above.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Build & Maintenance