Skip to content

Commit 44efbc7

Browse files
authored
fix(update): make the update flow report the truth and stop wedging (#38)
## Related Issue No issue — reported directly. The problem is described below. ## Problem A user's terminal showed `↑ Update available — v0.11.0` on the banner while `/update` answered `Update to v0.10.0 already in progress`, and nothing ever changed. Investigating it found a whole class of defects behind that one screen: - The manifest advertised a **version**, so the client had to guess a GitHub asset URL and poll for it. When the guess was wrong it polled for ~6 minutes, on every launch. - No installer network call had a timeout, so one hung request wedged every update path with no expiry. - A live pid held an install lease forever, with no ceiling, so a recycled pid wedged updates permanently. - The background installer's outcome was written by the parent process, and the product tells the user to close that terminal — so failures went unrecorded, the attempt counter never advanced, and a version that could not succeed was retried on every launch. - `install.sh` rendered a progress bar only on a TTY; in the background it blocked in a single silent `curl` and reported nothing at all, which is why an update in flight looked identical to a wedged one. - Two foreground install paths ignored the lock entirely and could run while a detached installer was writing the same executable. - The banner chip and `/update` read different files, which is how they came to disagree. ## What changed Fourteen commits, each with tests, in the order they were verified. Grouped: **The channel tells the truth.** `latest.json` now carries the resolved per-platform artifact (`url` + `sha256`), copied from the release's own native manifest, and a native client requires an entry for its platform before it will advertise or install anything. npm-family sources are exempt — the published version *is* their artifact — and that exemption is the case the tests protect hardest. `minRequiredVersion` lets a release bypass the staged rollout when a client cannot skip it. **Progress is visible.** The installer emits newline-terminated machine progress on stderr — the stream the parent already pipes — and the parent records it on the install record. The footer status row under the prompt renders `↑ v0.11.0`, `↓ v0.11.0 ▰▰▰▱▱▱▱▱ 42%`, `↑ v0.11.0 restart to apply`, reusing the context gauge's own bar glyphs. An unknown download size drops the bar rather than inventing a percentage. **Nothing wedges.** Every installer fetch has a connect bound, a per-attempt ceiling and a stall guard (`--retry` is deliberately absent: it resets `--max-time`). One `lease.ts` states the lease rule once, with a ceiling on live pids. Startup reconciles an abandoned install into a recorded failure so a doomed version parks. Both foreground paths hold the lock and write their outcome. **It says what is happening.** `/update` reports the installing version *and* the newer target that follows, and reports a parked version's attempt count and recorded reason instead of a bare command. Deletions rather than additions where the shape allowed: the plain-text `/latest` fallback (it carried no platform data, so it reported an unverifiable target as verified), the duplicate `install.sh`/`install.ps1` under `apps/site/public/`, the duplicated `isProcessRunning` and four lease constants, the banner's update chip and its per-frame `readFileSync`, and one of the two update decisions per launch. Two scope decisions worth flagging: killing an in-flight installer to switch targets is **not** implemented — the lease ceiling and reconciliation make the wait finite, and honest reporting fixes what the user saw. And a writability precheck was dropped in favour of surfacing the installer's own recorded error, which covers EACCES, network faults and disk-full alike. Base is `fix/release-cdn-version-truth` (#37) because the manifest generator work stacks on it. ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/Pythoughts-labs/pythinker-code/blob/main/CONTRIBUTING.md) document. - [x] I have linked a related issue, or explained the problem above. - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset. - [x] Ran `gen-docs` skill, or this PR needs no doc update. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Update status now appears beneath the prompt, including availability, download progress, required updates, waiting, and failures. - Updates can enforce a minimum supported version. - Platform-specific update availability is validated before installation. - **Bug Fixes** - Prevented conflicting or abandoned installations from blocking future updates. - Added clearer failure messages and retry information. - Installers now use connection, metadata, and download timeouts to avoid hanging. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 12069a8 commit 44efbc7

37 files changed

Lines changed: 3664 additions & 2326 deletions
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+
Stop offering an update with no build for the running platform, give every installer network call a timeout, expire a stale install lease instead of blocking updates forever, and say which version is installing and why a failed one stopped retrying.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pythoughts/pythinker-code": minor
3+
---
4+
5+
Let a release declare a minimum supported version, so a client below it is offered the update without waiting for its staged rollout batch.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pythoughts/pythinker-code": minor
3+
---
4+
5+
Show update availability and live download progress in the status row under the prompt, replacing the startup banner chip that was computed once and never refreshed.

apps/pythinker-code/src/cli/sub/upgrade.ts

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,16 @@ import { log, type Logger } from '@pythoughts/pythinker-code-sdk';
22
import { track as trackTelemetry, type TelemetryProperties } from '@pythoughts/pythinker-telemetry';
33

44
import { refreshUpdateCache } from '#/cli/update/refresh';
5-
import { selectUpdateTarget } from '#/cli/update/select';
5+
import { tryAcquireUpdateInstallLock } from '#/cli/update/install-lock';
6+
import type { UpdateInstallLockHandle, UpdateInstallLockRequest } from '#/cli/update/install-lock';
7+
import {
8+
emptyUpdateInstallState,
9+
failureAttemptsFor,
10+
hasFreshActiveInstall,
11+
readUpdateInstallState,
12+
writeUpdateInstallState,
13+
} from '#/cli/update/install-state';
14+
import { isTargetInstallable, selectUpdateTarget } from '#/cli/update/select';
615
import { detectInstallSource } from '#/cli/update/source';
716
import {
817
canAutoInstall,
@@ -20,6 +29,8 @@ import {
2029
NPM_PACKAGE_NAME,
2130
type InstallSource,
2231
type UpdateCache,
32+
type UpdateInstallState,
33+
type UpdateTarget,
2334
} from '#/cli/update/types';
2435

2536
interface WritableLike {
@@ -40,6 +51,11 @@ export interface UpgradeDeps {
4051
readonly promptForInstallChoice: (
4152
options: InstallPromptOptions,
4253
) => Promise<InstallPromptChoiceValue>;
54+
readonly readUpdateInstallState: () => Promise<UpdateInstallState>;
55+
readonly writeUpdateInstallState: (state: UpdateInstallState) => Promise<void>;
56+
readonly tryAcquireUpdateInstallLock: (
57+
request: UpdateInstallLockRequest,
58+
) => Promise<UpdateInstallLockHandle | null>;
4359
readonly platform: NodeJS.Platform;
4460
readonly stdout: WritableLike;
4561
readonly stderr: WritableLike;
@@ -85,6 +101,20 @@ export async function handleUpgrade(
85101
}
86102

87103
const source = await deps.detectInstallSource().catch(() => 'unsupported' as const);
104+
// A native install consumes the manifest's platform artifact; without one
105+
// the update cannot succeed, so take the same exit as being up to date.
106+
if (!isTargetInstallable(source, cache.manifest)) {
107+
trackUpgradeEvent(deps.track, 'upgrade_command_no_update', {
108+
current_version: currentVersion,
109+
});
110+
logUpgradeInfo(deps.logger, 'manual upgrade no update', {
111+
currentVersion,
112+
});
113+
deps.stdout.write(
114+
`${formatDisplayVersion(target.version)} is published but has no build for this platform yet.\n`,
115+
);
116+
return 0;
117+
}
88118
const installCommand = installCommandFor(source, target.version, deps.platform);
89119
if (!canAutoInstall(source, deps.platform) || !deps.isInteractive) {
90120
trackUpgradeEvent(deps.track, 'upgrade_command_manual_command', {
@@ -131,13 +161,36 @@ export async function handleUpgrade(
131161
return 0;
132162
}
133163

164+
// The foreground install holds the update-install lock for its whole run:
165+
// another live installer (usually a detached background one) must never be
166+
// raced by this path, which writes the same executable. A fresh active
167+
// record or a held lock means an install is already in flight — refuse.
168+
const installState = await deps.readUpdateInstallState().catch(() => emptyUpdateInstallState());
169+
if (hasFreshActiveInstall(installState)) {
170+
return refuseForegroundInstall(deps, currentVersion, target, source, installState.active?.version);
171+
}
172+
const lock = await deps.tryAcquireUpdateInstallLock({ version: target.version });
173+
if (lock === null) {
174+
return refuseForegroundInstall(deps, currentVersion, target, source, undefined);
175+
}
176+
134177
try {
135178
trackUpgradeEvent(deps.track, 'upgrade_command_install_selected', {
136179
current_version: currentVersion,
137180
target_version: target.version,
138181
source,
139182
});
140183
await deps.installUpdate(source, target.version, deps.platform);
184+
await deps.writeUpdateInstallState({
185+
...installState,
186+
active: null,
187+
lastFailure: null,
188+
lastSuccess: {
189+
version: target.version,
190+
installedAt: nowIso(),
191+
notifiedAt: null,
192+
},
193+
}).catch(() => {});
141194
trackUpgradeEvent(deps.track, 'upgrade_command_succeeded', {
142195
current_version: currentVersion,
143196
target_version: target.version,
@@ -151,6 +204,18 @@ export async function handleUpgrade(
151204
deps.stdout.write(renderInstallSuccessMessage(target));
152205
return 0;
153206
} catch (error) {
207+
const attempts = failureAttemptsFor(installState, target, 'install') + 1;
208+
await deps.writeUpdateInstallState({
209+
...installState,
210+
active: null,
211+
lastFailure: {
212+
version: target.version,
213+
failedAt: nowIso(),
214+
attempts,
215+
operation: 'install',
216+
message: formatErrorMessage(error),
217+
},
218+
}).catch(() => {});
154219
trackUpgradeEvent(deps.track, 'upgrade_command_failed', {
155220
current_version: currentVersion,
156221
target_version: target.version,
@@ -169,6 +234,8 @@ export async function handleUpgrade(
169234
`${formatErrorMessage(error)}\n`,
170235
);
171236
return 1;
237+
} finally {
238+
await lock.release().catch(() => {});
172239
}
173240
}
174241

@@ -178,6 +245,9 @@ function createDefaultUpgradeDeps(overrides: Partial<UpgradeDeps>): UpgradeDeps
178245
detectInstallSource: overrides.detectInstallSource ?? (() => detectInstallSource()),
179246
installUpdate: overrides.installUpdate ?? installUpdateForeground,
180247
promptForInstallChoice: overrides.promptForInstallChoice ?? promptForInstallChoice,
248+
readUpdateInstallState: overrides.readUpdateInstallState ?? (() => readUpdateInstallState()),
249+
writeUpdateInstallState: overrides.writeUpdateInstallState ?? writeUpdateInstallState,
250+
tryAcquireUpdateInstallLock: overrides.tryAcquireUpdateInstallLock ?? tryAcquireUpdateInstallLock,
181251
platform: overrides.platform ?? process.platform,
182252
stdout: overrides.stdout ?? process.stdout,
183253
stderr: overrides.stderr ?? process.stderr,
@@ -191,6 +261,39 @@ function formatDisplayVersion(version: string): string {
191261
return version.startsWith('v') ? version : `v${version}`;
192262
}
193263

264+
function nowIso(): string {
265+
return new Date().toISOString();
266+
}
267+
268+
/**
269+
* Refuse the foreground install because another install is already in
270+
* flight. The active-record case names the version being installed; the
271+
* lock-held case cannot know it, so the message stays generic.
272+
*/
273+
function refuseForegroundInstall(
274+
deps: UpgradeDeps,
275+
currentVersion: string,
276+
target: UpdateTarget,
277+
source: InstallSource,
278+
activeVersion: string | undefined,
279+
): number {
280+
trackUpgradeEvent(deps.track, 'upgrade_command_failed', {
281+
current_version: currentVersion,
282+
target_version: target.version,
283+
source,
284+
stage: 'install',
285+
reason: 'another update install is already in progress',
286+
});
287+
const suffix = activeVersion === undefined
288+
? ''
289+
: ` (${formatDisplayVersion(activeVersion)})`;
290+
deps.stderr.write(
291+
`error: another update install is already in progress${suffix}; ` +
292+
'try again once it finishes.\n',
293+
);
294+
return 1;
295+
}
296+
194297
function formatErrorMessage(error: unknown): string {
195298
return error instanceof Error ? error.message : String(error);
196299
}
Lines changed: 92 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { valid } from 'semver';
1+
import { lt, valid } from 'semver';
22
import { z } from 'zod';
33

4-
import { PYTHINKER_CODE_CDN_LATEST_JSON_URL, PYTHINKER_CODE_CDN_LATEST_URL } from '#/constant/app';
4+
import { PYTHINKER_CODE_CDN_LATEST_JSON_URL } from '#/constant/app';
55

66
import type { UpdateManifest } from './types';
77

@@ -12,27 +12,59 @@ const RolloutBatchSchema = z.object({
1212
delaySeconds: z.number().int().min(0),
1313
});
1414

15+
const UpdateManifestPlatformSchema = z.object({
16+
url: z
17+
.string()
18+
.refine(
19+
(value) => {
20+
try {
21+
const url = new URL(value);
22+
return url.protocol === 'http:' || url.protocol === 'https:';
23+
} catch {
24+
return false;
25+
}
26+
},
27+
{ error: 'invalid url' },
28+
),
29+
sha256: z.string().regex(/^[a-f0-9]{64}$/u),
30+
});
31+
1532
/**
1633
* CDN `latest.json` wire format. Deliberately NOT `.strict()` — unknown
1734
* fields are ignored so future manifest additions never break shipped
18-
* clients (the plain-text `/latest` taught us that hard-failing on
19-
* unexpected content bricks the update path forever).
35+
* clients. Hard-failing on unexpected content bricks the update path for
36+
* every already-installed client, which is unrecoverable from our side.
2037
*/
2138
export const UpdateManifestSchema = z.object({
2239
version: z.string().refine((value) => valid(value) !== null, { error: 'invalid semver' }),
2340
publishedAt: z
2441
.string()
2542
.refine((value) => Number.isFinite(Date.parse(value)), { error: 'invalid timestamp' }),
2643
rollout: z.array(RolloutBatchSchema).readonly().default([]),
44+
/**
45+
* Resolved per-platform artifacts, keyed `<platform>-<arch>`. A malformed
46+
* value drops only this field via `.catch(undefined)` so `version` and
47+
* `publishedAt` still parse — failing the whole manifest would cost the
48+
* client its update over one unreadable field.
49+
*/
50+
platforms: z
51+
.record(z.string(), UpdateManifestPlatformSchema)
52+
.readonly()
53+
.optional()
54+
.catch(undefined),
55+
/**
56+
* Lowest version that can still work against the current services. A
57+
* malformed value drops only this field via `.catch(undefined)` so
58+
* `version` and `publishedAt` still parse — a client below the floor must
59+
* not lose its update because the declaration is unreadable.
60+
*/
61+
minRequiredVersion: z
62+
.string()
63+
.refine((value) => valid(value) !== null, { error: 'invalid semver' })
64+
.optional()
65+
.catch(undefined),
2766
});
2867

29-
export interface FetchLatestResult {
30-
/** Raw newest version — what `pythinker upgrade` installs, never rollout-gated. */
31-
readonly latest: string;
32-
/** Null when the JSON manifest was unavailable and we fell back to plain text. */
33-
readonly manifest: UpdateManifest | null;
34-
}
35-
3668
async function fetchWithTimeout(fetchImpl: typeof fetch, input: string): Promise<Response> {
3769
const controller = new AbortController();
3870
const timeout = setTimeout(() => {
@@ -46,52 +78,68 @@ async function fetchWithTimeout(fetchImpl: typeof fetch, input: string): Promise
4678
}
4779

4880
/**
49-
* Fetch the latest published Pythinker Code version from the CDN.
81+
* Fetch the CDN update manifest — the client's only source of update truth.
5082
*
51-
* **Throws** on any failure (network error, non-2xx, empty body, non-semver
52-
* text). Callers must catch — `refreshUpdateCache` deliberately lets the
53-
* error propagate so the existing cache stays intact instead of being
54-
* overwritten with a null `latest` on a transient blip.
83+
* **Throws** on any failure (network error, non-2xx, unparseable body). Callers
84+
* must catch: `refreshUpdateCache` deliberately lets the error propagate so the
85+
* existing cache stays intact instead of being overwritten on a transient blip.
86+
*
87+
* There is deliberately no fallback to the plain-text `/latest` endpoint, which
88+
* still exists for `install.sh`. That endpoint carries no per-platform artifact
89+
* data, so falling back to it turns "cannot verify this platform has a build"
90+
* into "verified" and re-opens the hole `platforms` exists to close. It also
91+
* cannot fail independently: both files come from the same generator in the same
92+
* deploy, and the manifest schema already tolerates unknown fields and a
93+
* malformed `platforms` value without failing the parse.
5594
*
5695
* `fetchImpl` is injectable for tests; defaults to the global `fetch`.
5796
*/
58-
export async function fetchLatestVersionFromCdn(
97+
export async function fetchUpdateManifest(
5998
fetchImpl: typeof fetch = fetch,
60-
): Promise<string> {
61-
const response = await fetchWithTimeout(fetchImpl, PYTHINKER_CODE_CDN_LATEST_URL);
62-
if (!response.ok) {
63-
throw new Error(`CDN /latest returned HTTP ${response.status}`);
64-
}
65-
const raw = (await response.text()).trim();
66-
if (valid(raw) === null) {
67-
throw new Error(`CDN /latest returned invalid semver: ${JSON.stringify(raw)}`);
68-
}
69-
return raw;
70-
}
71-
72-
async function fetchUpdateManifestFromCdn(fetchImpl: typeof fetch): Promise<UpdateManifest> {
99+
): Promise<UpdateManifest> {
73100
const response = await fetchWithTimeout(fetchImpl, PYTHINKER_CODE_CDN_LATEST_JSON_URL);
74101
if (!response.ok) {
75102
throw new Error(`CDN /latest.json returned HTTP ${response.status}`);
76103
}
77104
return UpdateManifestSchema.parse(JSON.parse(await response.text()));
78105
}
79106

107+
export type ArtifactAvailability = 'available' | 'unavailable';
108+
80109
/**
81-
* Fetch the rollout manifest, falling back to the plain-text `/latest` when
82-
* `latest.json` is unavailable or malformed. The fallback removes any
83-
* deployment-order coupling between client releases and the CDN file, and a
84-
* null manifest means "fully rolled out" — exactly the pre-rollout behavior.
85-
*
86-
* **Throws** only when both sources fail; callers must catch (see above).
110+
* Whether the manifest advertises an artifact for `target`. Unknown — a
111+
* null manifest or one that predates artifact addressing — resolves to
112+
* 'available': a CDN blip must never stop a working update, while a
113+
* manifest that explicitly omits the target platform is a definitive
114+
* denial.
87115
*/
88-
export async function fetchLatestFromCdn(
89-
fetchImpl: typeof fetch = fetch,
90-
): Promise<FetchLatestResult> {
91-
const manifest = await fetchUpdateManifestFromCdn(fetchImpl).catch(() => null);
92-
if (manifest !== null) {
93-
return { latest: manifest.version, manifest };
116+
export function manifestArtifactAvailability(
117+
manifest: UpdateManifest | null,
118+
target: string = `${process.platform}-${process.arch}`,
119+
): ArtifactAvailability {
120+
if (manifest === null) {
121+
return 'available';
122+
}
123+
if (manifest.platforms === undefined) {
124+
return 'available';
94125
}
95-
const latest = await fetchLatestVersionFromCdn(fetchImpl);
96-
return { latest, manifest: null };
126+
return Object.hasOwn(manifest.platforms, target) ? 'available' : 'unavailable';
127+
}
128+
129+
/**
130+
* Whether the running version is below the manifest's declared floor, which
131+
* makes its update mandatory rather than merely available: the staged rollout
132+
* delay exists for ordinary releases, not for one a client cannot skip.
133+
*
134+
* An absent, unreadable or non-semver floor answers false — a declaration we
135+
* cannot understand must not escalate an update on its own.
136+
*/
137+
export function isBelowMinRequiredVersion(
138+
manifest: UpdateManifest | null,
139+
currentVersion: string,
140+
): boolean {
141+
const floor = manifest?.minRequiredVersion;
142+
if (floor === undefined) return false;
143+
if (valid(currentVersion) === null || valid(floor) === null) return false;
144+
return lt(currentVersion, floor);
97145
}

0 commit comments

Comments
 (0)