Skip to content

Commit 01bbd69

Browse files
wpfleger96npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
andauthored
fix(onboarding): show real install errors and fix concurrent install state (#2658)
Signed-off-by: Will Pfleger <[email protected]> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
1 parent 89a2e97 commit 01bbd69

7 files changed

Lines changed: 482 additions & 112 deletions

File tree

‎desktop/src/features/onboarding/ui/RuntimeErrorTooltip.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ export function RuntimeErrorTooltip({
4242
side="bottom"
4343
sideOffset={12}
4444
>
45-
<span className="leading-4">{detail}</span>
45+
<span className="block max-h-48 overflow-y-auto overflow-x-hidden break-words whitespace-pre-line leading-4">
46+
{detail}
47+
</span>
4648
</TooltipContent>
4749
</Tooltip>
4850
);

‎desktop/src/features/onboarding/ui/SetupStep.tsx‎

Lines changed: 47 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -463,19 +463,56 @@ function RuntimeAuthError({ runtime }: { runtime: AcpRuntimeCatalogEntry }) {
463463
}
464464

465465
function RuntimeCard({
466-
installError,
467-
isInstalling,
468-
onInstall,
466+
installResults,
467+
onInstallResultsChange,
469468
runtime,
470469
}: {
471-
installError: string | null;
472-
isInstalling: boolean;
473-
onInstall: () => void;
470+
installResults: InstallResultsState;
471+
onInstallResultsChange: React.Dispatch<
472+
React.SetStateAction<InstallResultsState>
473+
>;
474474
runtime: AcpRuntimeCatalogEntry;
475475
}) {
476+
// Each card owns its own mutation instance so concurrent installs on
477+
// different cards each track their own isPending state and callbacks
478+
// independently (react-query v5 per-mutate callbacks only fire for the
479+
// latest mutate() call on a shared instance, silently dropping earlier ones).
480+
const installMutation = useInstallAcpRuntimeMutation();
481+
const installError = installResults[runtime.id]?.error ?? null;
482+
const isInstalling = installMutation.isPending;
476483
const isAvailable = runtime.availability === "available";
477484
const isReady = runtimeIsReadyForOnboarding(runtime);
478485

486+
function handleInstall() {
487+
onInstallResultsChange((current) => ({
488+
...current,
489+
[runtime.id]: { error: null, success: false },
490+
}));
491+
492+
installMutation.mutate(runtime.id, {
493+
onSuccess: (result) => {
494+
onInstallResultsChange((current) => ({
495+
...current,
496+
[runtime.id]: result.success
497+
? { error: null, success: true }
498+
: {
499+
error: getInstallErrorMessage(result.steps),
500+
success: false,
501+
},
502+
}));
503+
},
504+
onError: (error) => {
505+
onInstallResultsChange((current) => ({
506+
...current,
507+
[runtime.id]: {
508+
error: error instanceof Error ? error.message : "Install failed.",
509+
success: false,
510+
},
511+
}));
512+
},
513+
});
514+
}
515+
479516
return (
480517
<Card
481518
className={cn(
@@ -499,7 +536,7 @@ function RuntimeCard({
499536
<RuntimeStatus
500537
installError={installError}
501538
isInstalling={isInstalling}
502-
onInstall={onInstall}
539+
onInstall={handleInstall}
503540
runtime={runtime}
504541
/>
505542
{!isAvailable && runtimeDetailText(runtime) ? (
@@ -517,7 +554,7 @@ function RuntimeCard({
517554
{installError ? (
518555
<RuntimeErrorTooltip
519556
className="absolute inset-x-3 bottom-2 flex min-w-0 items-center justify-center gap-1.5 overflow-hidden whitespace-nowrap text-xs leading-4 text-destructive"
520-
detail="Installation couldn’t be completed. Try again."
557+
detail={installError}
521558
label="Installation failed"
522559
showIcon
523560
testId={`onboarding-runtime-error-${runtime.id}`}
@@ -560,34 +597,6 @@ function RuntimeProvidersSection({
560597
}) {
561598
const { errorMessage, isChecking, items } = runtimeProviders;
562599
const orderedItems = getVisibleOnboardingRuntimes(items);
563-
const installMutation = useInstallAcpRuntimeMutation();
564-
565-
function handleInstall(runtimeId: string) {
566-
onInstallResultsChange((current) => ({
567-
...current,
568-
[runtimeId]: { error: null, success: false },
569-
}));
570-
571-
installMutation.mutate(runtimeId, {
572-
onSuccess: (result) => {
573-
onInstallResultsChange((current) => ({
574-
...current,
575-
[runtimeId]: result.success
576-
? { error: null, success: true }
577-
: { error: getInstallErrorMessage(result.steps), success: false },
578-
}));
579-
},
580-
onError: (error) => {
581-
onInstallResultsChange((current) => ({
582-
...current,
583-
[runtimeId]: {
584-
error: error instanceof Error ? error.message : "Install failed.",
585-
success: false,
586-
},
587-
}));
588-
},
589-
});
590-
}
591600

592601
return (
593602
<section className="flex min-h-full w-full flex-col items-center">
@@ -606,13 +615,9 @@ function RuntimeProvidersSection({
606615
<div className="grid min-w-0 w-full max-w-[592px] grid-cols-1 gap-4 md:grid-cols-2">
607616
{orderedItems.map((runtime) => (
608617
<RuntimeCard
609-
installError={installResults[runtime.id]?.error ?? null}
610-
isInstalling={
611-
installMutation.isPending &&
612-
installMutation.variables === runtime.id
613-
}
618+
installResults={installResults}
614619
key={runtime.id}
615-
onInstall={() => handleInstall(runtime.id)}
620+
onInstallResultsChange={onInstallResultsChange}
616621
runtime={runtime}
617622
/>
618623
))}

‎desktop/src/features/settings/ui/DoctorSettingsPanel.tsx‎

Lines changed: 53 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -279,22 +279,56 @@ function RuntimeHeader({
279279
}
280280

281281
function RuntimeRow({
282-
installError,
283-
installSuccess,
284-
isInstalling,
285-
onInstall,
282+
resetEpoch,
286283
runtime,
287284
}: {
288-
installError: string | null;
289-
installSuccess: boolean;
290-
isInstalling: boolean;
291-
onInstall: () => void;
285+
resetEpoch: number;
292286
runtime: AcpRuntimeCatalogEntry;
293287
}) {
294288
const [terminalLaunchMethodId, setTerminalLaunchMethodId] = React.useState<
295289
string | null
296290
>(null);
297291
const [isUpdateWarningOpen, setIsUpdateWarningOpen] = React.useState(false);
292+
// Each row owns its mutation instance so concurrent installs each track
293+
// their own isPending / result state independently.
294+
const installMutation = useInstallAcpRuntimeMutation();
295+
const [installResult, setInstallResult] = React.useState<{
296+
success: boolean;
297+
error: string | null;
298+
} | null>(null);
299+
// Clear stale install results when the parent triggers a catalog refresh
300+
// (Check again) — the runtime may now be healthy and stale failure state
301+
// would linger because keyed rows don't remount on refetch.
302+
// biome-ignore lint/correctness/useExhaustiveDependencies: resetEpoch is an intentional trigger only; its value is not consumed in the effect body
303+
React.useEffect(() => {
304+
setInstallResult(null);
305+
}, [resetEpoch]);
306+
const isInstalling = installMutation.isPending;
307+
const installError = installResult?.error ?? null;
308+
const installSuccess = installResult?.success ?? false;
309+
310+
function handleInstall() {
311+
setInstallResult(null);
312+
installMutation.mutate(runtime.id, {
313+
onSuccess: (result) => {
314+
if (result.success) {
315+
setInstallResult({ success: true, error: null });
316+
} else {
317+
setInstallResult({
318+
success: false,
319+
error: getInstallErrorMessage(result.steps),
320+
});
321+
}
322+
},
323+
onError: (error) => {
324+
setInstallResult({
325+
success: false,
326+
error: error instanceof Error ? error.message : "Install failed.",
327+
});
328+
},
329+
});
330+
}
331+
298332
const canConnectAccount =
299333
runtime.availability === "available" &&
300334
runtime.authStatus.status === "logged_out";
@@ -352,7 +386,7 @@ function RuntimeRow({
352386
setIsUpdateWarningOpen(true);
353387
return;
354388
}
355-
onInstall();
389+
handleInstall();
356390
}}
357391
runtime={runtime}
358392
/>
@@ -372,7 +406,10 @@ function RuntimeRow({
372406
</p>
373407
) : null}
374408
{installError ? (
375-
<p className="mt-2 whitespace-pre-line rounded-lg border border-destructive/30 bg-destructive/10 px-3 py-1.5 text-sm text-destructive">
409+
<p
410+
className="mt-2 whitespace-pre-line rounded-lg border border-destructive/30 bg-destructive/10 px-3 py-1.5 text-sm text-destructive"
411+
data-testid={`doctor-runtime-install-error-${runtime.id}`}
412+
>
376413
{installError}
377414
</p>
378415
) : null}
@@ -410,7 +447,7 @@ function RuntimeRow({
410447
<AlertDialogFooter>
411448
<AlertDialogCancel>Cancel</AlertDialogCancel>
412449
<AlertDialogAction
413-
onClick={onInstall}
450+
onClick={handleInstall}
414451
data-testid={`doctor-runtime-confirm-update-${runtime.id}`}
415452
>
416453
Update
@@ -491,59 +528,9 @@ export function DoctorSettingsPanel() {
491528
[runtimesQuery.data],
492529
);
493530
const isRefreshing = runtimesQuery.isFetching;
494-
const installMutation = useInstallAcpRuntimeMutation();
495-
const [installResults, setInstallResults] = React.useState<
496-
Record<string, { success: boolean; error: string | null }>
497-
>({});
498-
// Per-runtime installing state: tracks which runtime IDs have an in-flight
499-
// install so concurrent installs each show their own spinner correctly.
500-
const [installingIds, setInstallingIds] = React.useState<Set<string>>(
501-
new Set(),
502-
);
503-
504-
function handleInstall(runtimeId: string) {
505-
// Clear any previous result for this runtime before retrying.
506-
setInstallResults((prev) => ({
507-
...prev,
508-
[runtimeId]: { success: false, error: null },
509-
}));
510-
setInstallingIds((prev) => new Set(prev).add(runtimeId));
511-
512-
installMutation.mutate(runtimeId, {
513-
onSuccess: (result) => {
514-
if (result.success) {
515-
setInstallResults((prev) => ({
516-
...prev,
517-
[runtimeId]: { success: true, error: null },
518-
}));
519-
} else {
520-
setInstallResults((prev) => ({
521-
...prev,
522-
[runtimeId]: {
523-
success: false,
524-
error: getInstallErrorMessage(result.steps),
525-
},
526-
}));
527-
}
528-
},
529-
onError: (error) => {
530-
setInstallResults((prev) => ({
531-
...prev,
532-
[runtimeId]: {
533-
success: false,
534-
error: error instanceof Error ? error.message : "Install failed.",
535-
},
536-
}));
537-
},
538-
onSettled: () => {
539-
setInstallingIds((prev) => {
540-
const next = new Set(prev);
541-
next.delete(runtimeId);
542-
return next;
543-
});
544-
},
545-
});
546-
}
531+
// Incremented each time the user clicks "Check again" so RuntimeRow
532+
// useEffect clears stale install results from before the refresh.
533+
const [resetEpoch, setResetEpoch] = React.useState(0);
547534

548535
return (
549536
<section
@@ -558,7 +545,7 @@ export function DoctorSettingsPanel() {
558545
<Button
559546
disabled={isRefreshing}
560547
onClick={() => {
561-
setInstallResults({});
548+
setResetEpoch((e) => e + 1);
562549
void runtimesQuery.refetch();
563550
void gitBashQuery.refetch();
564551
}}
@@ -598,11 +585,8 @@ export function DoctorSettingsPanel() {
598585
<div className="space-y-3" data-testid="doctor-runtime-list">
599586
{runtimes.map((runtime) => (
600587
<RuntimeRow
601-
installError={installResults[runtime.id]?.error ?? null}
602-
installSuccess={installResults[runtime.id]?.success ?? false}
603-
isInstalling={installingIds.has(runtime.id)}
604588
key={runtime.id}
605-
onInstall={() => handleInstall(runtime.id)}
589+
resetEpoch={resetEpoch}
606590
runtime={runtime}
607591
/>
608592
))}

‎desktop/src/shared/lib/installError.test.mjs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,47 @@ test("getInstallErrorMessage: failed step with empty stderr falls back to stdout
6767
]);
6868
assert.match(message, /some stdout output/);
6969
});
70+
71+
test("getInstallErrorMessage: hint and step detail are separated by double newline for whitespace-pre-line rendering", () => {
72+
const hint = "Git Bash is required. Install it from git-scm.com.";
73+
const message = getInstallErrorMessage([
74+
{
75+
step: "shell",
76+
command: "bash -l -c 'npm install'",
77+
success: false,
78+
stdout: "",
79+
stderr: "bash: command not found",
80+
exitCode: 127,
81+
hint,
82+
},
83+
]);
84+
assert.ok(
85+
message.includes("\n\n"),
86+
"hint and step detail should be separated by a blank line",
87+
);
88+
assert.ok(message.startsWith(hint));
89+
});
90+
91+
test("getInstallErrorMessage: only reports the last (failing) step when multiple steps present", () => {
92+
const message = getInstallErrorMessage([
93+
{
94+
step: "node",
95+
command: "node --version",
96+
success: true,
97+
stdout: "v20.0.0",
98+
stderr: "",
99+
exitCode: 0,
100+
},
101+
{
102+
step: "adapter",
103+
command: "npm install -g @agentclientprotocol/claude-code-acp",
104+
success: false,
105+
stdout: "",
106+
stderr: "npm ERR! code E404",
107+
exitCode: 1,
108+
},
109+
]);
110+
assert.match(message, /Step "adapter" failed:/);
111+
assert.match(message, /npm ERR! code E404/);
112+
assert.doesNotMatch(message, /Step "node"/);
113+
});

0 commit comments

Comments
 (0)