Skip to content

fix(web): updating the desktop app only warns about interrupted work when something is running - #28187

Closed
1337Crakker wants to merge 1 commit into
pingdotgg:mainfrom
1337Crakker:fix/skip-idle-update-confirm
Closed

1337Crakker wants to merge 1 commit into
pingdotgg:mainfrom
1337Crakker:fix/skip-idle-update-confirm

Conversation

@1337Crakker

Copy link
Copy Markdown

Problem

Installing a downloaded desktop update always asks "Install update X and restart T3 Code? Any running tasks will be interrupted." This happens even when nothing is running, so the prompt gets clicked through out of habit, which makes it less useful when work really is running.

Change

The prompt is skipped only when every backend the install stops (the local server plus any WSL servers the app runs) is known to be idle:

  • its thread list is live, not a cached copy or mid-resync;
  • no thread matches isLiveSubagentShell, meaning no live run, pending approval or question, or pending background tasks;
  • its terminal list has loaded and no terminal reports hasRunningSubprocess.

Anything unknown keeps today's prompt: the catalog isn't ready, an environment is disconnected or disabled, terminals haven't loaded, or the desktop's backend list (getLocalEnvironmentBootstraps) can't be read or names a backend with no idle environment. Work started from a phone or app.t3.codes on this machine runs on its server, so it counts. SSH-launched servers run under nohup and survive the restart, so they don't count.

Both entry points (the sidebar update button and Settings → About) share useConfirmDesktopUpdateInstall. Its atom is mounted only while an install is offered and re-renders only when the set of idle backends changes. Update everything, remote server updates, and mobile are unchanged.

Scope and approval

This changes product behavior, so I opened #28185 for direction and scope. Maintainer approval is still pending. It overlaps with #26424, which edits the same prompt. Known limits, also listed in #28185:

  • Threads archived while waiting on an approval or with background tasks running aren't in the shell; fix(server): reject archiving threads with active turns #23451 covers that on the server.
  • Work that starts in the moment between the click and the shutdown can still be interrupted, as it can after Confirm today. Terminal commands are seen through the server's subprocess poll: about once a second in packaged builds, backing off to 60s if process snapshots keep failing.

Verification

  • desktopUpdate.logic.test.ts covers the idle rule (waiting thread, terminal command, non-live shell, unloaded terminals) and the backend cross-check (unknown, missing, unreadable, or empty backend lists). vp run check:changed passes.
  • I ran the real desktop app (dev:desktop, isolated state) with a temporary renderer shim that reported a downloaded update, which was not committed. Results:
    • Sidebar button, idle: no prompt, and the install was invoked. Dev refuses the install, hence the "Update downloaded" toast.
    • Sidebar button, during a replay turn: prompt shown.
    • Sidebar button, with sleep 600 in a terminal: prompt shown.
    • After the turn ended and the command was stopped: no prompt.
    • Settings → About: no prompt when idle (install invoked), and a prompt while a terminal command ran.
  • I couldn't check a real packaged install, WSL backends, or the mobile or app.t3.codes paths against a desktop host.
Before: idle, still asks After: idle, installs After: thread running After: terminal command running
before idle after idle after busy thread after busy terminal

Proved by running the real desktop dev app before and after the change.

Reviewed with sol-loop: 1 round with gpt-6.1-sol. Sol verdict: approved

Created with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

…pted

Installing a downloaded update always asked to confirm that running tasks
would be interrupted, even with nothing running. Skip the prompt only when
every backend the install stops is known idle: its thread list is live, no
thread has live work, and no terminal is running a command. Anything
unknown still asks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 31, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — Idle desktop updates will install and restart without the previously mandatory confirmation; active or unknown work still triggers the warning. Removing that default confirmation requires a human product decision.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 15fe283 and 8b95f4e.

Files selected for processing: 5
  • apps/web/src/components/desktopUpdate.logic.test.ts
  • apps/web/src/components/desktopUpdate.logic.ts
  • apps/web/src/components/settings/SettingsPanels.tsx
  • apps/web/src/components/sidebar/SidebarUpdatePill.tsx
  • apps/web/src/state/desktopHostedWork.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The change adds hosted-environment idleness checks and a shared install-confirmation hook. Settings and sidebar update controls use the hook to skip or show confirmation based on idle and running desktop backend IDs.

Changes: Desktop update confirmation

Layer / File(s) Summary
Determine hosted-environment idleness
apps/web/src/components/desktopUpdate.logic.ts, apps/web/src/state/desktopHostedWork.ts, apps/web/src/components/desktopUpdate.logic.test.ts
The idle check requires a live shell, loaded terminals, no live subagent shell, and no running terminal subprocess. State derives sorted backend IDs only when the catalog and local environments satisfy the required checks. Tests cover idle and non-idle cases.
Decide whether to confirm
apps/web/src/components/desktopUpdate.logic.ts, apps/web/src/state/desktopHostedWork.ts
The confirmation logic requires a dialog when backend IDs are unknown, no running backend is reported, or a running backend is not idle. The hook reads running IDs from the desktop bridge and shows the dialog when required.
Use confirmation in update entry points
apps/web/src/components/settings/SettingsPanels.tsx, apps/web/src/components/sidebar/SidebarUpdatePill.tsx
The settings and sidebar update controls use the shared confirmation hook before installation. The sidebar enables confirmation only for install actions outside the update-everything flow.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant UpdateControl
  participant ConfirmationHook
  participant IdleBackendIds
  participant DesktopBridge
  participant ConfirmationDialog
  UpdateControl->>ConfirmationHook: request install confirmation
  ConfirmationHook->>IdleBackendIds: read idle backend IDs
  ConfirmationHook->>DesktopBridge: read running backend IDs
  ConfirmationHook->>ConfirmationDialog: show dialog when IDs require confirmation
Loading

Suggested reviewers: saphid, t3dotgg, juliusmarminge

Merge Risk: Minimal · up to 8b95f

Updates can restart without a warning when backends appear idle. No merge-blocking issue was established.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the main change: the desktop update install prompt now warns about interrupted work only when something is running. It matches the raw summary, the objectives, and the description. Its…
Description check Passed The description follows the template's sections: Problem, Change, Scope and approval, and Verification. The Problem and Change sections are clear and specific. The Scope and approval section links iss…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: the desktop update install prompt now warns about interrupted work only when something is running. It matches the raw summary, the objectives, and the description. Its conventional commit form also matches the template's example. The title is 94 characters, which is longer than the usual short-title guidance, but it stays specific and readable.

Full details: Description check

Explanation

The description follows the template's sections: Problem, Change, Scope and approval, and Verification. The Problem and Change sections are clear and specific. The Scope and approval section links issue #28185, notes that maintainer approval is pending, and lists known limits and overlap with #26424. The Verification section lists focused unit tests, the vp run check:changed result, manual desktop dev-app checks with observed results, and items that were not verified. A before/after image table is included. The template asks for a conventional commit title in the header comment, which the title satisfies. The description does not give the approval comment link that the template asks for, and it relies on pending approval, so a reviewer must confirm that the scope requirement is met before merge. The description also ends with an AI-attribution line that the template asks for. These gaps are not critical, so the check passes.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@t3dotgg

t3dotgg commented Nov 1, 2026

Copy link
Copy Markdown
Member

Note

🤖 GPT-6.1 Terra [1m] responding on behalf of Theo

Closing because there is no prior maintainer approval for silently restarting the desktop app when affected backends appear idle. Obtain agreement on that scope in Ideas before requesting reconsideration under the contribution policy.

@t3dotgg t3dotgg closed this Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants