Skip to content

Commit d7a5db0

Browse files
committed
fix: surface why an automatic update failed
A background install that failed left no trace of why. The installer was spawned with stdio: 'ignore', so its error output was discarded and lastFailure recorded only an exit code; the TUI then told the user to restart and, on the next launch, showed the ordinary update prompt with no sign that two attempts had already failed. Diagnosing a broken installer meant reproducing the spawn by hand. - Pipe the installer's stderr and store its tail in lastFailure.message. - Show that message on the update prompt when it belongs to the offered version. - Pin the target version for native installs on macOS and Linux with '-s -- --version', the guarantee PYTHINKER_VERSION already gave on Windows; unpinned, the script installed whatever the CDN called latest. - Tell the user to open a new terminal, which is what actually applies a native update, rather than to restart the CLI.
1 parent dc1c5cf commit d7a5db0

5 files changed

Lines changed: 246 additions & 26 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pythoughts/pythinker-code": patch
3+
---
4+
5+
Report why an automatic update failed instead of failing silently: the installer's error output is now recorded and shown on the next update prompt, native installs on macOS and Linux pin the version the rollout picked, and update messages tell you to open a new terminal to apply the update.

apps/pythinker-code/src/cli/update/preflight.ts

Lines changed: 91 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { spawn } from 'node:child_process';
22
import { randomUUID } from 'node:crypto';
33
import { homedir } from 'node:os';
4+
import type { Readable } from 'node:stream';
45

56
import { gte, valid } from 'semver';
67

@@ -10,6 +11,7 @@ import type { TelemetryProperties } from '@pythoughts/pythinker-telemetry';
1011
import {
1112
NATIVE_INSTALL_COMMAND_UNIX,
1213
NATIVE_INSTALL_COMMAND_WIN,
14+
PYTHINKER_CODE_INSTALL_SH_URL,
1315
} from '#/constant/app';
1416
import { loadTuiConfig } from '#/tui/config';
1517

@@ -163,7 +165,18 @@ export function spawnForSource(
163165
// would look like a successful update. `pipefail` makes the pipeline
164166
// surface curl's non-zero status so installUpdate() rejects and we warn
165167
// instead of printing "Updated …".
166-
return { cmd: 'bash', args: ['-c', `set -o pipefail; ${NATIVE_INSTALL_COMMAND_UNIX}`] };
168+
//
169+
// `-s -- --version` pins the install to the version this preflight
170+
// decided on, the same guarantee PYTHINKER_VERSION gives on Windows.
171+
// Without it the script installs whatever the CDN currently calls
172+
// latest, which can differ from the rollout's target.
173+
return {
174+
cmd: 'bash',
175+
args: [
176+
'-c',
177+
`set -o pipefail; curl -fsSL ${PYTHINKER_CODE_INSTALL_SH_URL} | bash -s -- --version ${version}`,
178+
],
179+
};
167180
case 'unsupported':
168181
throw new Error('unsupported install source cannot be auto-installed');
169182
}
@@ -207,7 +220,10 @@ export function renderManualUpdateMessage(
207220
}
208221

209222
export function renderInstallSuccessMessage(target: UpdateTarget): string {
210-
return `Updated ${NPM_PACKAGE_NAME} to ${target.version}. Restart the CLI to use the new version.\n`;
223+
return (
224+
`Updated ${NPM_PACKAGE_NAME} to ${target.version}. ` +
225+
'Close this terminal and open a new one to use the new version.\n'
226+
);
211227
}
212228

213229
function renderBackgroundInstallSuccessNotice(version: string): string {
@@ -548,16 +564,31 @@ async function promptInstall(
548564
target: UpdateTarget,
549565
source: InstallSource,
550566
installCommand: string,
567+
previousFailure: string | undefined,
551568
): Promise<InstallPromptChoiceValue> {
552569
const options: InstallPromptOptions = {
553570
currentVersion,
554571
target,
555572
installSource: source,
556573
installCommand,
574+
previousFailure,
557575
};
558576
return promptForInstallChoice(options);
559577
}
560578

579+
/**
580+
* A recorded failure is only worth showing when it is about the version the
581+
* prompt is offering — an older version's failure is stale noise.
582+
*/
583+
function failureMessageFor(
584+
state: UpdateInstallState,
585+
target: UpdateTarget,
586+
): string | undefined {
587+
const failure = state.lastFailure;
588+
if (failure === null || failure.version !== target.version) return undefined;
589+
return failure.message;
590+
}
591+
561592
export async function installUpdate(
562593
source: InstallSource,
563594
version: string,
@@ -581,6 +612,41 @@ export async function installUpdate(
581612
});
582613
}
583614

615+
/** Keep the tail only: installers can be chatty, and the state file is small. */
616+
const INSTALLER_STDERR_TAIL_CHARS = 2000;
617+
618+
/**
619+
* Buffer the installer's stderr so a failed background install records why it
620+
* failed. Discarding it (the previous `stdio: 'ignore'`) left `lastFailure`
621+
* with nothing but an exit code, which made a broken installer script
622+
* impossible to diagnose without reproducing the spawn by hand.
623+
*/
624+
function captureStderrTail(child: ReturnType<typeof spawn>): () => string | undefined {
625+
// Typed `Readable | null`, but absent entirely when stderr was not piped.
626+
const stream: Readable | null | undefined = child.stderr;
627+
if (stream === null || stream === undefined) return () => undefined;
628+
let tail = '';
629+
stream.setEncoding('utf8');
630+
stream.on('data', (chunk: string) => {
631+
tail = (tail + chunk).slice(-INSTALLER_STDERR_TAIL_CHARS);
632+
});
633+
// A detached installer outliving this process must not crash it, and the
634+
// pipe must not hold the event loop open on the way out. `child.stderr` is
635+
// typed as a plain Readable, but the pipe is a Socket at runtime and that is
636+
// where unref lives.
637+
stream.on('error', () => {});
638+
(stream as Readable & { unref?: () => void }).unref?.();
639+
return () => {
640+
const trimmed = tail.trim();
641+
return trimmed.length === 0 ? undefined : trimmed;
642+
};
643+
}
644+
645+
function describeChildExit(cmd: string, code: number | null, signal: NodeJS.Signals | null): string {
646+
const detail = signal !== null ? `signal ${signal}` : `code ${String(code)}`;
647+
return `${cmd} exited with ${detail}`;
648+
}
649+
584650
async function waitForChildSpawn(child: ReturnType<typeof spawn>): Promise<void> {
585651
await new Promise<void>((resolve, reject) => {
586652
const onSpawn = (): void => {
@@ -750,16 +816,18 @@ async function startBackgroundInstall(
750816
// the terminal outcome until the handler is "ready".
751817
let ready = false;
752818
let settled = false;
753-
let pendingOutcome: boolean | undefined;
819+
let pendingOutcome: { succeeded: boolean; reason: string } | undefined;
754820

755-
const finish = async (succeeded: boolean): Promise<void> => {
821+
const finish = async (succeeded: boolean, reason: string): Promise<void> => {
756822
if (!ready) {
757-
pendingOutcome ??= succeeded;
823+
pendingOutcome ??= { succeeded, reason };
758824
return;
759825
}
760826
if (settled) return;
761827
settled = true;
762828
const attempts = failureAttemptsFor(startedState, target, 'install') + 1;
829+
const stderrTail = readStderrTail();
830+
const message = stderrTail === undefined ? reason : `${reason}: ${stderrTail}`;
763831

764832
const nextState: UpdateInstallState = succeeded
765833
? {
@@ -780,6 +848,7 @@ async function startBackgroundInstall(
780848
failedAt: nowIso(),
781849
attempts,
782850
operation: 'install',
851+
message,
783852
},
784853
};
785854
try {
@@ -804,6 +873,7 @@ async function startBackgroundInstall(
804873
targetVersion: target.version,
805874
source,
806875
attempts,
876+
message,
807877
});
808878
} finally {
809879
await lock.release().catch(() => {});
@@ -815,11 +885,16 @@ async function startBackgroundInstall(
815885
// A detached child gets its own console window on Windows regardless
816886
// of stdio; stdio: 'ignore' alone does not suppress it.
817887
windowsHide: platform === 'win32',
818-
stdio: 'ignore',
888+
// stdout stays discarded (install progress is noise); stderr is piped so
889+
// a failure records the installer's own error text.
890+
stdio: ['ignore', 'ignore', 'pipe'],
819891
env: env === undefined ? undefined : { ...process.env, ...env },
820892
});
821-
child.once('error', () => { void finish(false); });
822-
child.once('exit', (code) => { void finish(code === 0); });
893+
const readStderrTail = captureStderrTail(child);
894+
child.once('error', (error) => { void finish(false, formatErrorMessage(error)); });
895+
child.once('exit', (code, signal) => {
896+
void finish(code === 0, describeChildExit(cmd, code, signal));
897+
});
823898
if (child.pid !== undefined && child.pid > 0) {
824899
const stateWithPid: UpdateInstallState = {
825900
...startedState,
@@ -841,7 +916,7 @@ async function startBackgroundInstall(
841916
child.unref();
842917
finalizerOwnsLock = true;
843918
ready = true;
844-
if (pendingOutcome !== undefined) void finish(pendingOutcome);
919+
if (pendingOutcome !== undefined) void finish(pendingOutcome.succeeded, pendingOutcome.reason);
845920
return true;
846921
// When startup failed before handoff, release the lock here; the
847922
// finalizer releases it once the terminal state write completes.
@@ -1196,7 +1271,13 @@ export async function runUpdatePreflight(
11961271
return 'continue';
11971272
}
11981273

1199-
const choice = await promptInstall(currentVersion, userVisibleTarget, source, installCommand);
1274+
const choice = await promptInstall(
1275+
currentVersion,
1276+
userVisibleTarget,
1277+
source,
1278+
installCommand,
1279+
failureMessageFor(installState, userVisibleTarget),
1280+
);
12001281
if (choice === 'skip') return 'continue';
12011282

12021283
try {

apps/pythinker-code/src/cli/update/prompt.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,30 @@ export interface InstallPromptOptions {
2828
readonly target: UpdateTarget;
2929
readonly installCommand: string;
3030
readonly installSource: InstallSource;
31+
/**
32+
* Why the last automatic install of this same version failed. A silently
33+
* failed background install otherwise looks identical to one that never
34+
* started, and the prompt is where the user decides what to do about it.
35+
*/
36+
readonly previousFailure?: string;
3137
readonly input?: NodeJS.ReadStream;
3238
readonly output?: NodeJS.WriteStream;
3339
}
3440

41+
/** Keep the prompt readable: one line, the first line of the recorded error. */
42+
const FAILURE_SUMMARY_MAX_CHARS = 120;
43+
44+
export function summarizeInstallFailure(message: string): string | undefined {
45+
const firstLine = message
46+
.split('\n')
47+
.map((line) => line.trim())
48+
.find((line) => line.length > 0);
49+
if (firstLine === undefined) return undefined;
50+
return firstLine.length > FAILURE_SUMMARY_MAX_CHARS
51+
? `${firstLine.slice(0, FAILURE_SUMMARY_MAX_CHARS - 1)}…`
52+
: firstLine;
53+
}
54+
3555
const INSTALL_HINT = 'Install update now';
3656
const SKIP_HINT = 'Continue with current version';
3757

@@ -78,10 +98,25 @@ function renderInstallPrompt(
7898
`${label('Target ')} ${targetVersion}`,
7999
`${label('Source ')} ${sourceLabel}`,
80100
`${label('Command')} ${command}`,
101+
];
102+
103+
const failureSummary =
104+
options.previousFailure === undefined
105+
? undefined
106+
: summarizeInstallFailure(options.previousFailure);
107+
if (failureSummary !== undefined) {
108+
lines.push(
109+
'',
110+
chalk.hex(UPDATE_PROMPT_WARNING).bold('The last automatic update failed'),
111+
chalk.hex(UPDATE_PROMPT_MUTED)(failureSummary),
112+
);
113+
}
114+
115+
lines.push(
81116
'',
82117
chalk.hex(UPDATE_PROMPT_MUTED)('↑↓ choose · Enter confirm · Esc continue'),
83118
'',
84-
];
119+
);
85120

86121
for (let i = 0; i < choices.length; i++) {
87122
const choice = choices[i];

apps/pythinker-code/src/tui/commands/info.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -314,18 +314,18 @@ export async function handleUpdateCommand(
314314
host.showNotice(
315315
`Updating to v${result.version}`,
316316
result.installOnRestart
317-
? 'Preparing with Homebrew in the background. Once ready, restart the CLI to install it.'
318-
: 'Installing in the background — restart the CLI when it completes.',
317+
? 'Preparing with Homebrew in the background. Once ready, close this terminal and open a new one to install it.'
318+
: 'Installing in the background — close this terminal and open a new one to apply the update.',
319319
);
320320
return;
321321
case 'in-progress':
322322
host.showNotice(
323323
`Update to v${result.version} already in progress`,
324324
result.installOnRestart
325325
? result.readyToInstall
326-
? 'Restart the CLI to install it.'
327-
: 'Restart after the current update operation finishes.'
328-
: 'Restart the CLI once it completes.',
326+
? 'Close this terminal and open a new one to install it.'
327+
: 'Close this terminal and open a new one after the current update operation finishes.'
328+
: 'Close this terminal and open a new one once it completes.',
329329
);
330330
return;
331331
case 'manual':

0 commit comments

Comments
 (0)