Skip to content

Commit 601c36a

Browse files
committed
fix(update): compare installed previews by base version on latest channel
Takeover of PR #871's fix. An installed preview such as 2.8.2-preview.20260731 made parseStable(current) return null on the latest channel, so isNewer() returned false and the GUI reported already_latest with one-click update disabled. The current side now falls back to the preview's major.minor.patch core; the target side stays strict (a preview registry target is never accepted on latest). Same-base stays not-newer, mirroring the preview channel's O3 rule — semver §9 tension and the respin-format known limitation are recorded in devlog/_plan/260802_wt1_update_path_star_prompt/011.
1 parent fd72782 commit 601c36a

4 files changed

Lines changed: 101 additions & 3 deletions

File tree

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# Cycle 1 (wp1, Bug A #871) — P-phase re-verification record
2+
3+
## Stale check vs pre-written 010 doc (2026-08-02, worktree codex/wt1-update-path @ dev tip)
4+
5+
- `src/update/notify.ts` `isNewer` confirmed unfixed on dev: latest channel does
6+
`const c = parseStable(current)``null` for `2.8.2-preview.20260731` → returns
7+
`false` → GUI reports `already_latest`. Bug present, doc not stale.
8+
- Consumers confirmed: `src/update/badge.ts:69` (`updateAvailable: isNewer(cache.latest_version, current, channel)`),
9+
`src/update/job.ts:317` `checkForUpdate` (GUI one-click path), `src/update/job.ts:1377`.
10+
- Existing tests: `tests/update-notify.test.ts` has latest/preview channel tables;
11+
`tests/update-job.test.ts` has `checkForUpdate("latest", ...)` fixtures with
12+
injectable `currentVersion`/`detectInstall`/`latestVersion` deps — the PR's test
13+
slots fit without new fixtures.
14+
15+
## Semantics decision (aligned with PR #871 diff)
16+
17+
- Current side, latest channel: `parseStable(current) ?? parsePreview(current)?.slice(0, 3)`
18+
an installed preview compares by its `major.minor.patch` core.
19+
- Target side stays strict: `parseStable(latest)` only — a preview registry target is
20+
never accepted on the latest channel (parity with codex-rs, existing doc comment).
21+
- Same-base case: `2.9.1` vs installed `2.9.1-preview.N` → NOT newer. Although semver
22+
precedence says stable > its prerelease, the product rule matches the preview
23+
channel's existing O3 decision (same base = no nag); the release train promotes a
24+
preview to the same-base stable, so the update is content-lateral. This mirrors
25+
PR #871's test expectations exactly; deviating would fork behavior from the
26+
contributor PR under review.
27+
28+
## External verification (sol-medium lane, cxc-search)
29+
30+
| Claim | Result |
31+
|-------|--------|
32+
| semver: prerelease < associated release (2.9.1-preview.N < 2.9.1) | verified — SemVer §9/§11.3 (semver.org) |
33+
| semver: numeric prerelease identifiers compare numerically | verified — SemVer §11.4.1 |
34+
| npm: bare install resolves the `latest` dist-tag; preview train belongs on its own tag | verified — npm-dist-tag docs (Description/Purpose/Caveats) |
35+
| npm: `2.9.1-preview.N` does NOT satisfy `^2.9.1` | verified — node-semver Prerelease Tags + Caret Ranges |
36+
37+
Tension resolved: strict semver says same-base stable (2.9.1) IS newer than
38+
2.9.1-preview.N, but this repo's comparator deliberately treats same-base as
39+
not-newer on BOTH channels (preview-channel O3 rule predates this fix; PR #871
40+
encodes the same expectation for the latest channel). Rationale: the release
41+
train promotes a preview to its same-base stable, so the update is
42+
content-lateral and offering it is a nag. This is a product decision, recorded
43+
here so a future "strict semver" refactor can find it.
44+
45+
## Known limitation (audit blocker 1, folded)
46+
47+
Respin preview tags exist in the wild: `v2.7.9-preview.20260712.1` / `.2`
48+
i.e. `x.y.z-preview.YYYYMMDD[.r]`. `parsePreview`'s
49+
`/^(\d+)\.(\d+)\.(\d+)-preview\.(\d+)$/` rejects the trailing `.r`, so installs
50+
on a respin preview remain stuck at `already_latest` even after this fix. This
51+
gap predates the fix on BOTH channels and expanding the comparator would fork
52+
behavior from PR #871, so it stays out of scope here. Follow-up candidate:
53+
widen `parsePreview` to `(\d+)(?:\.(\d+))?$` and treat the respin counter as an
54+
extra `gt` tuple element (needs its own cycle + tests).
55+
56+
## Implementation delta (diff-level)
57+
58+
- MODIFY `src/update/notify.ts` — one line in `isNewer` latest-channel branch:
59+
`const c = parseStable(current) ?? parsePreview(current)?.slice(0, 3);`
60+
plus doc-comment update naming the preview-core rule.
61+
- MODIFY `tests/update-notify.test.ts` — add latest-channel case:
62+
`isNewer("2.9.1", "2.8.2-preview.20260731", "latest") === true`,
63+
`isNewer("2.9.1", "2.9.1-preview.20260731", "latest") === false`.
64+
- MODIFY `tests/update-job.test.ts` — add `checkForUpdate("latest", ...)` case:
65+
older preview → `updateAvailable: true, canUpdate: true`; same-base preview →
66+
both false (`already_latest`).
67+
68+
## Activation scenarios (C)
69+
70+
1. Red: new tests fail on unmodified tree (preview current → `already_latest`).
71+
2. Green: all three files' tests pass after the one-line change.
72+
3. No regression: full `bun run test` + `bun run typecheck`.

src/update/notify.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,20 @@ function gt(a: number[], b: number[]): boolean {
8383

8484
/**
8585
* Channel-aware "is latest newer than current?".
86-
* - latest channel: compare maj.min.pat only; prereleases are never "newer"
87-
* (parity with codex-rs), so stable users are not pushed onto previews.
86+
* - latest channel: compare maj.min.pat only; prerelease TARGETS are never
87+
* "newer" (parity with codex-rs), so stable users are not pushed onto
88+
* previews. An installed preview CURRENT compares by its maj.min.pat core,
89+
* so a stable release with a strictly higher base is offered (same base is
90+
* content-lateral promotion and stays not-newer, mirroring the preview
91+
* channel's O3 rule).
8892
* - preview channel: preview-vs-preview compares the trailing -preview.N; a
8993
* stable release with a strictly higher base counts as newer (O3), while a
9094
* stable release with the same base as the current preview does not.
9195
*/
9296
export function isNewer(latest: string, current: string, channel: Channel): boolean {
9397
if (channel === "latest") {
9498
const l = parseStable(latest);
95-
const c = parseStable(current);
99+
const c = parseStable(current) ?? parsePreview(current)?.slice(0, 3);
96100
if (!l || !c) return false;
97101
return gt(l, c);
98102
}

tests/update-job.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,24 @@ describe("GUI update check", () => {
8585
expect(result.canUpdate).toBe(false);
8686
expect(result.reason).toBe("already_latest");
8787
});
88+
89+
test("offers a stable update from an older preview but not the same base", () => {
90+
const olderPreview = checkForUpdate("latest", {
91+
currentVersion: () => "2.8.2-preview.20260731",
92+
detectInstall: () => "npm",
93+
latestVersion: () => "2.9.1",
94+
});
95+
expect(olderPreview.updateAvailable).toBe(true);
96+
expect(olderPreview.canUpdate).toBe(true);
97+
98+
const sameBasePreview = checkForUpdate("latest", {
99+
currentVersion: () => "2.9.1-preview.20260731",
100+
detectInstall: () => "npm",
101+
latestVersion: () => "2.9.1",
102+
});
103+
expect(sameBasePreview.updateAvailable).toBe(false);
104+
expect(sameBasePreview.canUpdate).toBe(false);
105+
});
88106
});
89107

90108
describe("GUI update execution decisions", () => {

tests/update-notify.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ describe("isNewer — latest channel", () => {
3838
test("prereleases are ignored on the stable channel", () => {
3939
expect(isNewer("2.7.0-preview.1", "2.6.4", "latest")).toBe(false);
4040
});
41+
test("stable releases compare against a preview current by its base version", () => {
42+
expect(isNewer("2.9.1", "2.8.2-preview.20260731", "latest")).toBe(true);
43+
expect(isNewer("2.9.1", "2.9.1-preview.20260731", "latest")).toBe(false);
44+
});
4145
});
4246

4347
describe("isNewer — preview channel", () => {

0 commit comments

Comments
 (0)