fix(desktop): make doctor installs retryable with per-runtime progress and auth status#1765
Merged
Merged
Conversation
wpfleger96
added a commit
that referenced
this pull request
Jul 12, 2026
wpfleger96
added a commit
that referenced
this pull request
Jul 12, 2026
Collaborator
Author
|
🤖 Updated Doctor state screenshots from the current branch: Authenticated
Not authenticated, with login hint
Config error diagnostic
Node.js required callout
Failed install with Retry
Retry succeedsClicking Retry re-runs the install; the error clears and the success banner renders. |
…s and auth status Fresh-install failures on stock Macs had four root causes: the login-shell PATH was cached for the app's entire lifetime via OnceLock (installing Node.js mid-session had no effect until restart); nvm-managed node/npm was invisible to the command resolver and agent spawn PATH because nvm only initializes in interactive shells; a single shared React mutation overwrote the in-flight indicator when two runtimes installed concurrently; and the Doctor panel never verified that CLI-based agents (claude, codex) were actually authenticated. Review-round hardening: probe_auth_status's inner-thread timeout leaked a blocked thread and a live CLI child on expiry — replaced with a process-level timeout (spawn → drain threads → kill-on-expiry) mirroring run_install_command. The login-shell cache now uses double-checked locking with a named LoginShellPath enum so the shell spawn happens outside the lock. nvm alias tags are validated (is_safe_nvm_tag) before path joins so an attacker-writable alias file cannot steer agent spawn to arbitrary binaries; both the default tag and one-hop alias are checked. auth_probe_args moved onto KnownAcpRuntime (removing the probe_args_for side registry), and the never-hitting 60s auth-probe TTL cache was removed. A five-state Doctor screenshots spec (logged-in/logged-out/config-error badges, node-required callout, install-failure Retry) doubles as a regression guard for the new UI logic. Rebased over #1750 (codex-acp 1.x swap): its adapter_outdated Doctor branch now passes the required hasError prop, its make_codex_runtime test helper gained the auth_probe_args/login_hint fields, and the AdapterOutdated version-gate is composed into the two-phase discover_acp_runtimes.
wpfleger96
force-pushed
the
wpfleger96/doctor-install-reliability
branch
from
d16bf95 to
ea82454
Compare
wpfleger96
pushed a commit
that referenced
this pull request
Jul 12, 2026
* origin/main: fix(desktop): correct provider and model handling in agent config dialogs (#1764) fix(desktop): cascade persona deletes and restart agents on global config save (#1766) fix(desktop): make doctor installs retryable with per-runtime progress and auth status (#1765) fix(codex): swap to @agentclientprotocol/codex-acp 1.x + detect outdated adapter (#1750) Fix oversized mobile Manage channel sheet (#1774) fix(desktop): show thread replies loader (#1773) fix(build): allow vendored opus to configure under CMake 4 (#1763) fix(desktop): restore multi-image mosaic galleries (#1769) Group channel membership events (#1713) Fix desktop launch motion and reaction spacing (#1717) fix(desktop): align sidebar search across themes (#1712) fix(desktop): Buzz theme flicker, white bar & accent-picker motion (#1681) fix(mobile): highlight full multi-word mentions (#1762) [BOT-1264] fix(mobile): #channel tags not tappable (#1695) feat(desktop): surface Team Instructions as distinct observer section (#1759) # Conflicts: # desktop/playwright.config.ts
wpfleger96
pushed a commit
that referenced
this pull request
Jul 12, 2026
* origin/main: fix(desktop): correct provider and model handling in agent config dialogs (#1764) fix(desktop): cascade persona deletes and restart agents on global config save (#1766) fix(desktop): make doctor installs retryable with per-runtime progress and auth status (#1765) fix(codex): swap to @agentclientprotocol/codex-acp 1.x + detect outdated adapter (#1750) Fix oversized mobile Manage channel sheet (#1774) fix(desktop): show thread replies loader (#1773) fix(build): allow vendored opus to configure under CMake 4 (#1763) fix(desktop): restore multi-image mosaic galleries (#1769) Group channel membership events (#1713) Fix desktop launch motion and reaction spacing (#1717) fix(desktop): align sidebar search across themes (#1712) fix(desktop): Buzz theme flicker, white bar & accent-picker motion (#1681) fix(mobile): highlight full multi-word mentions (#1762) [BOT-1264] fix(mobile): #channel tags not tappable (#1695) feat(desktop): surface Team Instructions as distinct observer section (#1759) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> # Conflicts: # desktop/playwright.config.ts
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.
Stack: this PR → #1767
Fixes the Doctor (harness install) failures hit on a fresh Mac with no dev tools (OSS desktop build):
PATHwas cached in aOnceLockfor the app's lifetime, so installing Node.js after a failed install could never help until relaunch. The cache is now a refreshable three-stateLoginShellPathenum — every install/retry and Doctor Re-run re-fetches it (refresh_login_shell_path()), the login shell spawns outside the cache lock so a concurrent agent spawn never blocks on it, and failed install rows show a Retry button with the error kept visible until retried.~/.zshrc(interactive shells only), so login shells never see it andcodex-acp-style#!/usr/bin/env nodeshims failed at launch withenv: node: No such file or directory. Command resolution, CLI probes, and the agent-spawnPATHnow include nvm's default version bin dir (find_nvm_default_bin: readsalias/defaultwith one alias hop, falling back to the highest installed version under~/.nvm/versions/node/). Alias tags are validated before any path join (is_safe_nvm_tagrejects absolute paths,..traversal, and non-nvm characters) —PathBuf::joinreplaces its base when handed an absolute path, so an unvalidated~/.nvm/alias/defaultcould otherwise steer the agent-spawnPATHto arbitrary binaries.npm install -gbut neithernodenornpmresolves, the runtime row hides the Install button and shows a "Node.js is required" callout linking to nodejs.org (a sharedNodeRequiredOrInstallcomponent backs both theadapter_missingandnot_installedstates), instead of failing mid-install withzsh:1: command not found: npm.variables, so starting a second install stopped the first's spinner (backend installs already ran concurrently). Progress is now tracked per runtime id.claude auth status/codex login status, declared per runtime viaauth_probe_argsonKnownAcpRuntime) and render "Authenticated", "Not authenticated" with a login hint, or "Config error: ". Probes run in parallel with a 10-second process-level timeout that kills the probe child on expiry, so a hung CLI cannot accumulate orphaned processes across Re-runs. Every discovery probes fresh — there is no result cache. Runtimes without a login step (goose, buzz-agent) show nothing.The Doctor states (auth badges, node-required callout, retry flow) are covered by a Playwright screenshot spec (
doctor-states.spec.ts) driving them through configurable bridge mocks, including a fail-then-succeed install sequence that exercises Retry end to end.