Skip to content

Commit e9ebda6

Browse files
authored
fix(desktop): show the changelog in the update dialog (#226)
## Related Issue No linked issue — reported from the app: the v0.3.8 release-notes popover read "Pythinker Desktop 0.3.8 (stable channel), built from PyModel/pythinker-code@`<tt>`0f49851`</tt>`.", with the tags printed literally. ## Problem Two separate causes produced that one popover. 1. **The body was a build stamp.** `desktop-release.yml` created the draft with a fixed `--notes "Pythinker Desktop <v> (<channel> channel), built from <commit>."`. Nothing else ever wrote the body, so the updater had nothing to say about a version. 2. **The notes arrive as HTML.** electron-updater's GitHub provider reads the releases Atom feed, whose `<content type="html">` is the body GitHub has already rendered. The renderer treats `releaseNotes` as Markdown, so the tags GitHub emits — `<p>`, `<a>`, `<tt>` — printed as text. Fixing only the body would have left the markup visible, since a real changelog renders to `<ul>`/`<li>`/`<a>` too. ## What changed - **`apps/desktop/scripts/desktop-release.mjs`** — new exported `desktopReleaseNotes()` and a `notes` subcommand. It takes the `## <version>` section of `apps/desktop/CHANGELOG.md`, strips the changesets prefix (`[#225](…) [`sha`](…) Thanks [@user](…)! - `), and returns the bullets plus a `Built from <commit>` footer. - **`.github/workflows/desktop-release.yml`** — the prepare job writes those notes to a file and passes `--notes-file`. A **stable** release whose version has no changelog entry now fails here instead of publishing; preview channels fall back to a one-line description, since a nightly version never appears in the changelog. The source-commit URL stays in the body because the draft-resume check on line 120 gates on `.body | contains($source_url)`. - **`apps/desktop/src/updater.ts`** — `releaseNotesText` reduces HTML notes to text (list items to `- `, block ends to newlines, entities decoded), and leaves notes without markup untouched. - **`AGENTS.md`** — records that changeset text is shipped text: it becomes the release body users read, and a body must never be a build stamp. ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/PyModel/pythinker-code/blob/main/CONTRIBUTING.md) document. - [ ] I have linked a related issue (external PRs: the issue must have a maintainer's `/approve`). - [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. ### Tests `scripts/release/desktop-release.test.mjs` covers extraction, the next-heading boundary, the stable-release gate, the preview fallback, and the required source URL. `updater.spec.ts` asserts the exact HTML from the live v0.3.8 feed comes out as text, and that plain notes are unchanged. `desktop-release-workflow.spec.ts` pins `--notes-file` and forbids the old literal. Reverting both product changes fails exactly 2 tests; 175 + 25 pass as shipped. ### What users will see Instead of the build stamp, the v0.3.9 popover will read: `- Show the changelog for the new version in the update dialog instead of a build stamp with raw HTML tags.` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Update dialogs now display readable changelog content for new versions. * Release notes preserve lists and links as clean, formatted text without raw HTML or script content. * Desktop releases now publish version-specific changelog entries instead of build information. * **Bug Fixes** * Improved handling of plain-text and HTML-formatted release notes. * Prevented empty or malformed release notes from appearing in update prompts. * **Reliability** * Stable releases without valid changelog entries are blocked from publication. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 0f49851 commit e9ebda6

8 files changed

Lines changed: 391 additions & 4 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-desktop": patch
3+
---
4+
5+
Show the changelog for the new version in the update dialog instead of a build stamp with raw HTML tags.

.github/workflows/desktop-release.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,12 +138,18 @@ jobs:
138138
echo "Draft release ${RELEASE_TAG} already exists; resuming it."
139139
exit 0
140140
fi
141+
# The body is what the in-app updater shows users, so it carries the
142+
# changelog entries for this version. A stable release with no entry
143+
# fails here rather than shipping a build nobody can describe.
144+
notes_file="$(mktemp)"
145+
node apps/desktop/scripts/desktop-release.mjs notes \
146+
apps/desktop/CHANGELOG.md "${RELEASE_TAG#v}" "$RELEASE_CHANNEL" "$SOURCE_URL" > "$notes_file"
141147
args=(
142148
"$RELEASE_TAG"
143149
--repo "$RELEASE_REPO"
144150
--draft
145151
--title "$RELEASE_TAG"
146-
--notes "Pythinker Desktop ${RELEASE_TAG#v} (${RELEASE_CHANNEL} channel), built from ${SOURCE_URL}."
152+
--notes-file "$notes_file"
147153
)
148154
if [ "$PRERELEASE" = 'true' ]; then args+=(--prerelease); fi
149155
gh release create "${args[@]}"

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ Gate behind flags. Env: `PYTHINKER_CODE_EXPERIMENTAL_<NAME>` toggles one; `PYTHI
118118
- PR titles: Conventional Commit style (e.g. `chore: remove legacy format commands`).
119119
- Fill in `.github/pull_request_template.md` — link the issue, describe changes. No placeholder text or vague AI-generated PR summaries; the human author must understand the change well enough to explain the code, edge cases, and why the approach fits.
120120
- Run `gen-changesets` skill before submitting PRs. Changesets must strictly follow its rules: one short user-facing sentence stating only what changed; skip any change users cannot perceive. Never decide `major` on your own — stop, explain, and get explicit user confirmation first; default to `minor`, fall back to `patch`.
121+
- Changeset text is shipped text: the desktop release body is generated from `apps/desktop/CHANGELOG.md`, and the in-app updater shows it to users verbatim. A release body must state what changed for users — never a build stamp, a commit hash, or placeholder text. `desktop-release.yml` fails a stable release whose version has no changelog entry.
121122
- Prefer `import ... from '#/...'` (equivalent to `@/...`).
122123
- Do not commit throwaway scratch or exploratory files. Never stage agent working notes or handoff documents (e.g. `HANDOVER-*.md`, `HANDOFF-*.md`, `handoff.md`), or throwaway UI/UX prototypes or design mockups (e.g. `*-designs.html`, `*-mockup.html`, `*-demo(s).html`). The only tracked `.html` files should be Vite `index.html` entrypoints. Put scratch work under `.tmp/` (gitignored).
123124

apps/desktop/scripts/desktop-release.mjs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,45 @@ export function configureDesktopPackage(value, version, channel) {
165165
};
166166
}
167167

168+
const attributionPattern = /^\s*(?:\[[^\]]*\]\([^)]*\)\s*)+(?:Thanks\s+\[[^\]]*\]\([^)]*\)!\s*)?-\s*/u;
169+
170+
function changelogSection(changelog, version) {
171+
if (typeof changelog !== 'string') throw new Error('Desktop changelog must be a string.');
172+
const lines = changelog.split('\n');
173+
const start = lines.findIndex(line => line.trim() === `## ${version}`);
174+
if (start === -1) return [];
175+
const rest = lines.slice(start + 1);
176+
const end = rest.findIndex(line => line.startsWith('## '));
177+
return end === -1 ? rest : rest.slice(0, end);
178+
}
179+
180+
/**
181+
* Users read the release body in the updater, so it carries the changelog
182+
* entries and nothing else. Changesets prefixes every entry with its PR link,
183+
* commit link, and a thanks line; those are noise in an update dialog.
184+
*/
185+
export function desktopReleaseNotes(options) {
186+
const version = desktopVersion(options?.version);
187+
const channel = desktopChannel(options?.channel);
188+
const sourceUrl = options?.sourceUrl;
189+
if (typeof sourceUrl !== 'string' || sourceUrl.length === 0) {
190+
throw new Error('Desktop release notes require the source commit URL.');
191+
}
192+
const entries = [];
193+
for (const line of changelogSection(options?.changelog ?? '', version)) {
194+
if (!line.startsWith('- ')) continue;
195+
const text = line.slice(2).replace(attributionPattern, '').trim();
196+
if (text.length > 0) entries.push(`- ${text}`);
197+
}
198+
if (entries.length === 0) {
199+
if (channel === 'stable') {
200+
throw new Error(`apps/desktop/CHANGELOG.md has no entries for ${version}; a stable release must tell users what changed.`);
201+
}
202+
entries.push(`- Preview build of the ${channel} channel.`);
203+
}
204+
return `${entries.join('\n')}\n\n---\n\nBuilt from ${sourceUrl}.\n`;
205+
}
206+
168207
function writeOutputs(result) {
169208
const lines = [
170209
`version=${result.version}`,
@@ -201,14 +240,24 @@ function main() {
201240
}));
202241
return;
203242
}
243+
if (command === 'notes' && args.length === 4) {
244+
const [changelogPath, version, channel, sourceUrl] = args;
245+
process.stdout.write(desktopReleaseNotes({
246+
changelog: readFileSync(resolve(changelogPath), 'utf8'),
247+
version,
248+
channel,
249+
sourceUrl,
250+
}));
251+
return;
252+
}
204253
if (command === 'configure' && args.length === 3) {
205254
const [path, version, channel] = args;
206255
const packagePath = resolve(path);
207256
const configured = configureDesktopPackage(JSON.parse(readFileSync(packagePath, 'utf8')), version, channel);
208257
writeFileSync(packagePath, `${JSON.stringify(configured, null, 2)}\n`, 'utf8');
209258
return;
210259
}
211-
throw new Error('Usage: desktop-release.mjs resolve <event> <package-version> <channel> <tag> <commit-count> <publish-nightly> | configure <package-json> <version> <channel>');
260+
throw new Error('Usage: desktop-release.mjs resolve <event> <package-version> <channel> <tag> <commit-count> <publish-nightly> | notes <changelog> <version> <channel> <source-url> | configure <package-json> <version> <channel>');
212261
}
213262

214263
if (process.argv[1] !== undefined && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {

apps/desktop/src/updater.ts

Lines changed: 136 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,144 @@ export function writeUpdateSettings(dir: string, value: UpdateSettings): void {
109109
writeFileSync(join(dir, UPDATE_SETTINGS_FILE), `${JSON.stringify(value, null, 2)}\n`, 'utf8')
110110
}
111111

112+
const HTML_MARKUP_PATTERN = /<\/?[a-z][^>]*>/iu
113+
const NAMED_ENTITIES: Readonly<Record<string, string>> = {
114+
amp: '&',
115+
apos: "'",
116+
gt: '>',
117+
lt: '<',
118+
nbsp: ' ',
119+
quot: '"',
120+
}
121+
122+
function decodeEntities(value: string): string {
123+
return value.replaceAll(/&(#x[0-9a-f]+|#\d+|[a-z]+);/giu, (match, entity: string) => {
124+
if (!entity.startsWith('#')) return NAMED_ENTITIES[entity.toLowerCase()] ?? match
125+
const code = entity.startsWith('#x') || entity.startsWith('#X')
126+
? Number.parseInt(entity.slice(2), 16)
127+
: Number.parseInt(entity.slice(1), 10)
128+
return Number.isSafeInteger(code) && code > 0 && code <= 0x10ffff ? String.fromCodePoint(code) : match
129+
})
130+
}
131+
132+
const BLOCK_BREAK_TAGS: ReadonlySet<string> = new Set([
133+
'blockquote',
134+
'div',
135+
'h1',
136+
'h2',
137+
'h3',
138+
'h4',
139+
'h5',
140+
'h6',
141+
'li',
142+
'ol',
143+
'p',
144+
'pre',
145+
'table',
146+
'tr',
147+
'ul',
148+
])
149+
150+
const RAW_TEXT_TAGS: ReadonlySet<string> = new Set(['script', 'style'])
151+
152+
const TAG_NAME_CHARACTER = /[a-z0-9]/iu
153+
154+
interface HtmlTag {
155+
name: string
156+
closing: boolean
157+
end: number
158+
}
159+
160+
function readTag(value: string, start: number): HtmlTag | undefined {
161+
const end = value.indexOf('>', start)
162+
if (end < 0) return undefined
163+
const closing = value[start + 1] === '/'
164+
const nameStart = start + (closing ? 2 : 1)
165+
let cursor = nameStart
166+
while (cursor < end && TAG_NAME_CHARACTER.test(value[cursor] ?? '')) cursor += 1
167+
return { name: value.slice(nameStart, cursor).toLowerCase(), closing, end }
168+
}
169+
170+
function skipRawTextElement(value: string, name: string, from: number): number {
171+
let index = from
172+
while (index < value.length) {
173+
const next = value.indexOf('<', index)
174+
if (next < 0) return value.length
175+
const tag = readTag(value, next)
176+
if (tag === undefined) return value.length
177+
if (tag.closing && tag.name === name) return tag.end + 1
178+
index = tag.end + 1
179+
}
180+
return value.length
181+
}
182+
183+
/**
184+
* Decoding runs after the scan, so `&lt;script&gt;` — which is how GitHub
185+
* renders a literal tag an author typed — turns back into `<script>` once the
186+
* scan can no longer see it. The update dialog renders this value with a
187+
* Markdown component that does render raw HTML, so the decoded text has to
188+
* leave here inert. Escaping the angle brackets keeps it readable: a Markdown
189+
* renderer prints `&lt;` as `<` text rather than opening an element.
190+
*/
191+
function escapeMarkupStarts(value: string): string {
192+
return value.replaceAll('<', '&lt;').replaceAll('>', '&gt;')
193+
}
194+
195+
/**
196+
* The GitHub provider reads the releases Atom feed, whose `<content>` is the
197+
* body GitHub has already rendered to HTML. The renderer shows these notes as
198+
* Markdown, so the tags would print literally — `PyModel/pythinker-code@<tt>`
199+
* and the rest. Reduce the markup to text here, at the boundary that already
200+
* owns this field.
201+
*
202+
* This walks the input once and copies out only the text it passes, rather
203+
* than deleting tags from the string. Deletion is what lets `<scr<x>ipt>`
204+
* close back up into markup; a scan that never re-reads what it emitted cannot
205+
* produce a tag that was not already there. Markup the scan cannot terminate
206+
* ends the walk, so an unclosed `<` is dropped with the rest of the tail.
207+
*/
208+
function plainReleaseNotes(value: string): string {
209+
let text = ''
210+
let index = 0
211+
while (index < value.length) {
212+
const next = value.indexOf('<', index)
213+
if (next < 0) {
214+
text += value.slice(index)
215+
break
216+
}
217+
text += value.slice(index, next)
218+
const tag = readTag(value, next)
219+
if (tag === undefined) break
220+
if (!tag.closing && RAW_TEXT_TAGS.has(tag.name)) {
221+
index = skipRawTextElement(value, tag.name, tag.end + 1)
222+
continue
223+
}
224+
if (tag.name === 'li' && !tag.closing) text += '\n- '
225+
else if (tag.name === 'br') text += '\n'
226+
else if (tag.closing && BLOCK_BREAK_TAGS.has(tag.name)) text += '\n'
227+
index = tag.end + 1
228+
}
229+
return escapeMarkupStarts(decodeEntities(text))
230+
.replaceAll(/[^\S\n]+\n/gu, '\n')
231+
.replaceAll(/\n{3,}/gu, '\n\n')
232+
.trim()
233+
}
234+
235+
function normalizedNote(value: string): string {
236+
return HTML_MARKUP_PATTERN.test(value) ? plainReleaseNotes(value) : value.trim()
237+
}
238+
112239
function releaseNotesText(value: UpdateInfo['releaseNotes']): string | undefined {
113-
if (typeof value === 'string') return value
240+
if (typeof value === 'string') {
241+
const note = normalizedNote(value)
242+
return note.length > 0 ? note : undefined
243+
}
114244
if (!Array.isArray(value)) return undefined
115-
const notes = value.flatMap(item => typeof item.note === 'string' ? [item.note] : [])
245+
const notes = value.flatMap(item => {
246+
if (typeof item.note !== 'string') return []
247+
const note = normalizedNote(item.note)
248+
return note.length > 0 ? [note] : []
249+
})
116250
return notes.length > 0 ? notes.join('\n\n') : undefined
117251
}
118252

apps/desktop/tests/desktop-release-workflow.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ describe('desktop release workflow', () => {
3636
expect(workflow).toContain('verify-update-manifest.ts win')
3737
})
3838

39+
it('gives users the changelog as the release body instead of a build stamp', () => {
40+
expect(workflow).toContain('scripts/desktop-release.mjs notes')
41+
expect(workflow).toContain('--notes-file "$notes_file"')
42+
expect(workflow).not.toContain('--notes "Pythinker Desktop')
43+
})
44+
3945
it('never publishes a manual unsigned build to the release feed', () => {
4046
expect(workflow).toContain("publish: ${{ steps.resolve.outputs.publish }}")
4147
expect(workflow).toContain("if: needs.prepare.outputs.publish == 'true'")

apps/desktop/tests/updater.spec.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,114 @@ describe('update prompt receipts', () => {
888888
expect(readLocalUpdateSettings(directory).pendingInstallVersion).toBeUndefined()
889889
})
890890

891+
it('renders GitHub HTML release notes as text', async () => {
892+
vi.resetModules()
893+
const directory = temporaryDirectory()
894+
writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
895+
const { app: localApp } = await import('electron')
896+
const localElectronUpdater = (await import('electron-updater')).default
897+
const {
898+
getUpdateState: getLocalUpdateState,
899+
initUpdater: initLocalUpdater,
900+
} = await import('../src/updater')
901+
const localAutoUpdater = localElectronUpdater.autoUpdater
902+
vi.mocked(localApp.getPath).mockReturnValue(directory)
903+
Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
904+
Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
905+
906+
initLocalUpdater(() => undefined)
907+
const available = vi.mocked(localAutoUpdater.on).mock.calls
908+
.find(([event]) => event === 'update-available')?.[1] as ((info: { version: string, releaseNotes?: string }) => void) | undefined
909+
available?.({
910+
version: '1.2.3',
911+
releaseNotes: '<ul>\n<li>Install Windows updates in the background.</li>\n</ul>\n<hr>\n'
912+
+ '<p>Built from <a class="commit-link" href="https://example.com/commit/f27686a">PyModel/pythinker-code@<tt>f27686a</tt></a>.</p>',
913+
})
914+
915+
expect(getLocalUpdateState().releaseNotes).toBe(
916+
'- Install Windows updates in the background.\n\nBuilt from PyModel/pythinker-code@f27686a.',
917+
)
918+
})
919+
920+
it('keeps plain release notes untouched', async () => {
921+
vi.resetModules()
922+
const directory = temporaryDirectory()
923+
writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
924+
const { app: localApp } = await import('electron')
925+
const localElectronUpdater = (await import('electron-updater')).default
926+
const {
927+
getUpdateState: getLocalUpdateState,
928+
initUpdater: initLocalUpdater,
929+
} = await import('../src/updater')
930+
const localAutoUpdater = localElectronUpdater.autoUpdater
931+
vi.mocked(localApp.getPath).mockReturnValue(directory)
932+
Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
933+
Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
934+
935+
initLocalUpdater(() => undefined)
936+
const available = vi.mocked(localAutoUpdater.on).mock.calls
937+
.find(([event]) => event === 'update-available')?.[1] as ((info: { version: string, releaseNotes?: string }) => void) | undefined
938+
available?.({ version: '1.2.3', releaseNotes: '- One fix\n- Another fix' })
939+
940+
expect(getLocalUpdateState().releaseNotes).toBe('- One fix\n- Another fix')
941+
})
942+
943+
it('cannot be made to emit markup by nesting or truncating tags', async () => {
944+
vi.resetModules()
945+
const directory = temporaryDirectory()
946+
writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
947+
const { app: localApp } = await import('electron')
948+
const localElectronUpdater = (await import('electron-updater')).default
949+
const {
950+
getUpdateState: getLocalUpdateState,
951+
initUpdater: initLocalUpdater,
952+
} = await import('../src/updater')
953+
const localAutoUpdater = localElectronUpdater.autoUpdater
954+
vi.mocked(localApp.getPath).mockReturnValue(directory)
955+
Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
956+
Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
957+
958+
initLocalUpdater(() => undefined)
959+
const available = vi.mocked(localAutoUpdater.on).mock.calls
960+
.find(([event]) => event === 'update-available')?.[1] as ((info: { version: string, releaseNotes?: string }) => void) | undefined
961+
available?.({
962+
version: '1.2.3',
963+
releaseNotes: '<p>Real note.</p><scr<span>ipt>x</script><script>hidden()</script><b>Kept</b><spa',
964+
})
965+
966+
const notes = getLocalUpdateState().releaseNotes ?? ''
967+
expect(notes).not.toContain('<')
968+
expect(notes).toContain('Real note.')
969+
expect(notes).toContain('Kept')
970+
expect(notes).not.toContain('hidden()')
971+
})
972+
973+
it('does not let encoded markup decode back into tags', async () => {
974+
vi.resetModules()
975+
const directory = temporaryDirectory()
976+
writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
977+
const { app: localApp } = await import('electron')
978+
const localElectronUpdater = (await import('electron-updater')).default
979+
const {
980+
getUpdateState: getLocalUpdateState,
981+
initUpdater: initLocalUpdater,
982+
} = await import('../src/updater')
983+
const localAutoUpdater = localElectronUpdater.autoUpdater
984+
vi.mocked(localApp.getPath).mockReturnValue(directory)
985+
Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
986+
Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
987+
988+
initLocalUpdater(() => undefined)
989+
const available = vi.mocked(localAutoUpdater.on).mock.calls
990+
.find(([event]) => event === 'update-available')?.[1] as ((info: { version: string, releaseNotes?: string }) => void) | undefined
991+
available?.({
992+
version: '1.2.3',
993+
releaseNotes: '<p>Note</p>&lt;script&gt;payload&lt;/script&gt;',
994+
})
995+
996+
expect(getLocalUpdateState().releaseNotes).toBe('Note\n&lt;script&gt;payload&lt;/script&gt;')
997+
})
998+
891999
it('reports a pending install that did not take effect as an error', async () => {
8921000
vi.resetModules()
8931001
const directory = temporaryDirectory()

0 commit comments

Comments
 (0)