Skip to content

Commit 8fa81bd

Browse files
Stop stamping changelog watermark without showing notes (CL-5475) (#449)
The OpenTUI path computed whatsNewMarkdown and stamped lastChangelogVersion as a side effect, but never rendered the notes — upgrades were permanently marked seen without display. Stamp first-install only; upgrade stamps only when notesShown is true (currently false until a surface returns).
1 parent 2f4fe7b commit 8fa81bd

4 files changed

Lines changed: 79 additions & 24 deletions

File tree

src/changelog/index.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
parseChangelogText,
1313
parseVersionString,
1414
resolveChangelogPath,
15+
stampVersionAfterStartup,
1516
} from "./index.js";
1617

1718
const SAMPLE = `# Changelog
@@ -96,7 +97,7 @@ describe("decideStartupChangelog", () => {
9697
expect(d.kind).toBe("first_install");
9798
});
9899

99-
test("upgrade shows notes and stamps package version", () => {
100+
test("upgrade yields notes with stampVersion for when shown", () => {
100101
const d = decideStartupChangelog({
101102
entries,
102103
lastChangelogVersion: "0.2.85",
@@ -122,6 +123,43 @@ describe("decideStartupChangelog", () => {
122123
});
123124
});
124125

126+
describe("stampVersionAfterStartup", () => {
127+
const entries = parseChangelogText(SAMPLE);
128+
129+
test("first_install always stamps (quiet, no history dump)", () => {
130+
const d = decideStartupChangelog({
131+
entries,
132+
lastChangelogVersion: undefined,
133+
packageVersion: "0.2.86",
134+
});
135+
expect(stampVersionAfterStartup(d, false)).toBe("0.2.86");
136+
expect(stampVersionAfterStartup(d, true)).toBe("0.2.86");
137+
});
138+
139+
test("upgrade stamps only when notes were shown (CL-5475)", () => {
140+
const d = decideStartupChangelog({
141+
entries,
142+
lastChangelogVersion: "0.2.85",
143+
packageVersion: "0.2.86",
144+
});
145+
expect(d.kind).toBe("upgrade");
146+
// Dead surface / OpenTUI gap: do not consume notes without display.
147+
expect(stampVersionAfterStartup(d, false)).toBeNull();
148+
// When a surface restores and actually shows markdown, stamp.
149+
expect(stampVersionAfterStartup(d, true)).toBe("0.2.86");
150+
});
151+
152+
test("current never stamps", () => {
153+
const d = decideStartupChangelog({
154+
entries,
155+
lastChangelogVersion: "0.2.86",
156+
packageVersion: "0.2.86",
157+
});
158+
expect(stampVersionAfterStartup(d, false)).toBeNull();
159+
expect(stampVersionAfterStartup(d, true)).toBeNull();
160+
});
161+
});
162+
125163
describe("formatStartupChangelog", () => {
126164
test("caps entry count and marks truncated", () => {
127165
const entries = parseChangelogText(SAMPLE);

src/changelog/index.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,11 @@ export type ChangelogDisplayDecision =
163163
/**
164164
* Decide what to show on interactive start.
165165
* - Missing/empty/malformed watermark → first install: stamp package version, no history dump.
166-
* - New versioned sections after watermark → upgrade notes + stamp package version.
166+
* - New versioned sections after watermark → upgrade notes (stamp only once actually shown).
167167
* - Otherwise quiet.
168+
*
169+
* Persistence of the watermark is separate: see {@link stampVersionAfterStartup}.
170+
* Callers must not stamp upgrade notes unless they rendered them (CL-5475).
168171
*/
169172
export function decideStartupChangelog(input: {
170173
entries: ChangelogEntry[];
@@ -201,6 +204,24 @@ export function decideStartupChangelog(input: {
201204
};
202205
}
203206

207+
/**
208+
* Version to persist as `lastChangelogVersion` after this interactive start, or
209+
* `null` to leave the watermark alone.
210+
*
211+
* - first_install: always stamp (quiet; never dump history on later launches).
212+
* - upgrade: stamp only when `notesShown` is true. A dead surface must not
213+
* consume notes by stamping without display (CL-5475).
214+
* - current: no write.
215+
*/
216+
export function stampVersionAfterStartup(
217+
decision: ChangelogDisplayDecision,
218+
notesShown: boolean,
219+
): string | null {
220+
if (decision.kind === "first_install") return decision.stampVersion;
221+
if (decision.kind === "upgrade" && notesShown) return decision.stampVersion;
222+
return null;
223+
}
224+
204225
/**
205226
* Resolve CHANGELOG.md for runtime: package root (dev / npm), then next to the
206227
* executable (binary install), then cwd.

src/config/settings.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ export type Settings = {
105105
// shown. Controls whether subsequent launches show "Welcome to" vs "Welcome back".
106106
onboarded?: boolean;
107107
// Last package version whose release notes were shown (or stamped on first
108-
// interactive install). Drives the one-shot post-upgrade notes banner.
108+
// interactive install). Upgrade stamps only after notes are actually shown
109+
// so a missing surface cannot silently swallow them (CL-5475).
109110
lastChangelogVersion?: string;
110111
// Controls the context-compaction strategy used when the context window fills.
111112
// "llm" (default) generates a structured handoff summary via LLM call.

src/tui/runner.ts

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ import { captureSlashCommand } from "../telemetry/product-events.js";
109109
import { getTelemetry, liveTelemetry, setTelemetry } from "../telemetry/singleton.js";
110110
import { createTelemetryToggleHandler } from "../telemetry/toggle.js";
111111

112-
import { loadStartupChangelogMarkdown } from "../changelog/index.js";
112+
import {
113+
loadStartupChangelogMarkdown,
114+
stampVersionAfterStartup,
115+
} from "../changelog/index.js";
113116
import { scheduleUpgradeNotice } from "../upgrade/index.js";
114117
import pkg from "../../package.json" with { type: "json" };
115118
import { seedPricingMetadataFromCache } from "../cost/pricing-metadata.js";
@@ -1831,30 +1834,22 @@ export async function runTUI(initialConfig: Config): Promise<number> {
18311834
});
18321835
}
18331836

1834-
// Post-upgrade release notes: one-shot banner on a fresh interactive session.
1835-
// Resume skips the banner and does not stamp, so the next fresh session still
1836-
// surfaces notes. First install stamps without dumping history.
1837+
// Post-upgrade release notes watermark policy (CL-5475):
1838+
// - first_install: stamp quietly so later launches do not dump history.
1839+
// - upgrade: stamp only when notes were actually shown. The former Ink
1840+
// whats-new banner is gone on the OpenTUI path, so notesShown is false
1841+
// until a surface is restored — never silently consume upgrade notes.
1842+
// - resume / current: leave the watermark alone.
18371843
const changelogDecision = loadStartupChangelogMarkdown({
18381844
lastChangelogVersion: globalSettingsForOnboarding?.lastChangelogVersion,
18391845
packageVersion: typeof pkg.version === "string" ? pkg.version : "0.0.0",
18401846
});
1841-
let whatsNewMarkdown: string | undefined;
1842-
if (changelogDecision.kind === "upgrade" && !resumeSkipInitialTask) {
1843-
whatsNewMarkdown = changelogDecision.markdown;
1844-
void markLastChangelogVersion(trueGlobalSettingsPath, changelogDecision.stampVersion).catch(
1845-
() => {
1846-
// Best-effort watermark; worst case notes reappear next launch.
1847-
},
1848-
);
1849-
} else if (changelogDecision.kind === "first_install") {
1850-
void markLastChangelogVersion(trueGlobalSettingsPath, changelogDecision.stampVersion).catch(
1851-
() => {
1852-
// Best-effort watermark.
1853-
},
1854-
);
1855-
} else if (changelogDecision.kind === "upgrade" && resumeSkipInitialTask) {
1856-
// Resume with pending notes: leave watermark alone so a future fresh
1857-
// session can show them.
1847+
const notesShown = false;
1848+
const stampVersion = stampVersionAfterStartup(changelogDecision, notesShown);
1849+
if (stampVersion !== null) {
1850+
void markLastChangelogVersion(trueGlobalSettingsPath, stampVersion).catch(() => {
1851+
// Best-effort watermark.
1852+
});
18581853
}
18591854

18601855
const commandContext: CommandContext = {

0 commit comments

Comments
 (0)