Skip to content

Commit cb2ecdc

Browse files
authored
fix(desktop): sign the macOS DMG and explain server conflicts (#121)
## Related Issue No issue — both problems were found while producing the first signed macOS release from a new Developer ID certificate. ## Problem **1. The DMG was not signed.** Only `mac.notarize` was set, which covers the `.app`. `dmg.sign` kept its documented default of `false`, so the disk image itself shipped unsigned: ``` Pythinker-0.1.3-arm64.dmg: rejected source=no usable signature stapler: does not have a ticket stapled to it ``` Signing and stapling it by hand afterwards fixed Gatekeeper but broke auto-update, because those operations mutate the DMG after electron-builder has already written its checksum into `latest-mac.yml`, which `electron-updater` verifies downloads against: | field | latest-mac.yml | actual file | |---|---|---| | size | 166113403 | 166125225 | | sha512 | `vXP6moUrffhA…` | `lQbTRViFFbuS…` | **2. A server conflict was unreadable.** The server lock is global rather than per-port, so a CLI server on any port stops the desktop app from starting. The user saw only: ``` Host exited before readiness (code 1, signal null) Host output: server already running (pid=78405, port=58700, started=…) ``` which names no way out. ## What changed **DMG signing.** Enable `dmg.sign` so signing happens while electron-builder builds the image — before it hashes it. Notarizing and stapling still must follow the artifact, so a finalize step runs after electron-builder returns, then rewrites `latest-mac.yml` from the finished file and deletes the stale blockmap so a differential update falls back to a full download rather than corrupting. This is deliberately not an `afterAllArtifactBuild` hook: that fires after hashing and would reintroduce the same drift. The install window also gets an explicit icon layout. **Server conflict.** Parse the canonical conflict line and show a Retry/Quit dialog naming the process, port and start time. It does **not** stop the other server — it may be serving live sessions, and that is the user's call. `server run` also now honors an error's declared exit code, so `ServerLockedError`'s `2` survives instead of being flattened to `1`. The single-instance rule itself is kept deliberately. Two servers sharing one Pythinker home would race on session `state.json` writes, truncate each other's `wire.jsonl` rewrites, and both claim the same next sequence in the shared event journal. Refusing to start is correct; only the diagnosis was broken. ## Verification - `cd apps/desktop && npx vitest run` — 97 passed (was 87) - `pnpm --filter @pymodel/pythinker-desktop run typecheck` — clean - `pnpm --filter @pymodel/pythinker-code run typecheck` — clean - `pnpm run lint` — no `: error ` lines - The metadata rewrite was run against the real stale `latest-mac.yml` from the broken build and reproduced the correct checksum and size exactly. - The new parser tests were mutation-checked: breaking the regex fails them. Not covered: no full `dist:mac` has been run with this code, so the `xcrun` orchestration is exercised only through an injected command runner. ## Checklist - [x] I have read the CONTRIBUTING 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.
1 parent 0871533 commit cb2ecdc

12 files changed

Lines changed: 488 additions & 2 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-code': patch
3+
---
4+
5+
Say why the desktop app cannot start when another Pythinker server is already running. It now names the process, port and start time and offers Retry or Quit, in place of an exit code that explained nothing. Stopping the other server stays the user's choice.

.changeset/mac-dmg-signing.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@pymodel/pythinker-code': patch
3+
---
4+
5+
Sign, notarize and staple the macOS disk image, so a downloaded desktop build no longer opens with a Gatekeeper warning, and keep the update metadata in step with the finished file. The install window also gets a deliberate icon layout in place of the stock one.

apps/desktop/package.json

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,28 @@
6666
"dir"
6767
]
6868
},
69+
"dmg": {
70+
"sign": true,
71+
"title": "Pythinker ${version}",
72+
"window": {
73+
"width": 660,
74+
"height": 400
75+
},
76+
"iconSize": 128,
77+
"contents": [
78+
{
79+
"x": 180,
80+
"y": 200,
81+
"type": "file"
82+
},
83+
{
84+
"x": 480,
85+
"y": 200,
86+
"type": "link",
87+
"path": "/Applications"
88+
}
89+
]
90+
},
6991
"win": {
7092
"icon": "build/icon.png",
7193
"target": [
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
/** Notarize and staple built DMGs, then repair their update metadata. */
2+
3+
import { spawnSync } from 'node:child_process'
4+
import { createHash } from 'node:crypto'
5+
import {
6+
existsSync,
7+
readFileSync,
8+
readdirSync,
9+
statSync,
10+
unlinkSync,
11+
writeFileSync,
12+
} from 'node:fs'
13+
import { basename, join } from 'node:path'
14+
import { resolveNotarizationCredentials } from './release-preflight'
15+
16+
export interface CommandResult {
17+
readonly status: number | null
18+
readonly stderr: string
19+
readonly stdout: string
20+
}
21+
22+
export type CommandRunner = (command: string, args: readonly string[]) => CommandResult
23+
24+
export interface FinalizeMacArtifactsOptions {
25+
readonly distDir: string
26+
readonly env: NodeJS.ProcessEnv
27+
readonly log?: (message: string) => void
28+
readonly runCommand?: CommandRunner
29+
}
30+
31+
function requiredValue(env: NodeJS.ProcessEnv, name: string): string {
32+
return env[name]!.trim()
33+
}
34+
35+
/** Build the notarytool credential arguments selected by the release preflight. */
36+
export function buildNotarytoolArguments(env: NodeJS.ProcessEnv): readonly string[] {
37+
switch (resolveNotarizationCredentials(env)) {
38+
case 'api-key':
39+
return [
40+
'--key', requiredValue(env, 'APPLE_API_KEY'),
41+
'--key-id', requiredValue(env, 'APPLE_API_KEY_ID'),
42+
'--issuer', requiredValue(env, 'APPLE_API_ISSUER'),
43+
]
44+
case 'apple-id':
45+
return [
46+
'--apple-id', requiredValue(env, 'APPLE_ID'),
47+
'--password', requiredValue(env, 'APPLE_APP_SPECIFIC_PASSWORD'),
48+
'--team-id', requiredValue(env, 'APPLE_TEAM_ID'),
49+
]
50+
case 'keychain-profile': {
51+
const args = ['--keychain-profile', requiredValue(env, 'APPLE_KEYCHAIN_PROFILE')]
52+
const keychain = env['APPLE_KEYCHAIN']?.trim()
53+
if (keychain !== undefined && keychain !== '') args.push('--keychain', keychain)
54+
return args
55+
}
56+
}
57+
}
58+
59+
function yamlScalar(value: string): string {
60+
const trimmed = value.trim()
61+
if (
62+
(trimmed.startsWith("'") && trimmed.endsWith("'"))
63+
|| (trimmed.startsWith('"') && trimmed.endsWith('"'))
64+
) return trimmed.slice(1, -1)
65+
return trimmed
66+
}
67+
68+
/** Update all checksum and size fields associated with one DMG. */
69+
export function rewriteLatestMacYaml(
70+
yaml: string,
71+
filename: string,
72+
sha512: string,
73+
size: number,
74+
): string {
75+
const lines = yaml.split('\n')
76+
let fileEntryIndent: number | undefined
77+
let topLevelPathMatches = false
78+
let checksumUpdates = 0
79+
let sizeUpdates = 0
80+
81+
for (let index = 0; index < lines.length; index += 1) {
82+
const line = lines[index]!
83+
const indentation = line.search(/\S|$/)
84+
const url = line.match(/^(\s*)-\s+url:\s*(.+?)\s*$/)
85+
if (url !== null) {
86+
fileEntryIndent = yamlScalar(url[2]!) === filename ? url[1]!.length : undefined
87+
continue
88+
}
89+
90+
if (fileEntryIndent !== undefined) {
91+
if (line.trim() !== '' && indentation <= fileEntryIndent) {
92+
fileEntryIndent = undefined
93+
} else {
94+
const checksum = line.match(/^(\s*)sha512:\s*.*$/)
95+
if (checksum !== null) {
96+
lines[index] = `${checksum[1]}sha512: ${sha512}`
97+
checksumUpdates += 1
98+
continue
99+
}
100+
const artifactSize = line.match(/^(\s*)size:\s*.*$/)
101+
if (artifactSize !== null) {
102+
lines[index] = `${artifactSize[1]}size: ${String(size)}`
103+
sizeUpdates += 1
104+
continue
105+
}
106+
}
107+
}
108+
109+
if (topLevelPathMatches) {
110+
if (line.startsWith('sha512:')) {
111+
lines[index] = `sha512: ${sha512}`
112+
checksumUpdates += 1
113+
continue
114+
}
115+
if (line.startsWith('size:')) {
116+
lines[index] = `size: ${String(size)}`
117+
sizeUpdates += 1
118+
continue
119+
}
120+
if (line.trim() !== '' && indentation === 0) topLevelPathMatches = false
121+
}
122+
123+
const path = line.match(/^path:\s*(.+?)\s*$/)
124+
if (path !== null) topLevelPathMatches = yamlScalar(path[1]!) === filename
125+
}
126+
127+
if (checksumUpdates === 0 || sizeUpdates === 0) {
128+
throw new Error(`latest-mac.yml does not contain complete metadata for ${filename}`)
129+
}
130+
return lines.join('\n')
131+
}
132+
133+
function defaultCommandRunner(command: string, args: readonly string[]): CommandResult {
134+
const result = spawnSync(command, args, { encoding: 'utf8' })
135+
if (result.error !== undefined) throw result.error
136+
return { status: result.status, stderr: result.stderr, stdout: result.stdout }
137+
}
138+
139+
/** Finalize every DMG in the supplied desktop distribution directory. */
140+
export function finalizeMacArtifacts(options: FinalizeMacArtifactsOptions): void {
141+
const runCommand = options.runCommand ?? defaultCommandRunner
142+
const log = options.log ?? console.log
143+
const dmgs = readdirSync(options.distDir, { withFileTypes: true })
144+
.filter(entry => entry.isFile() && entry.name.endsWith('.dmg'))
145+
.map(entry => entry.name)
146+
.sort()
147+
if (dmgs.length === 0) throw new Error(`No DMG artifacts found in ${options.distDir}`)
148+
149+
const metadataPath = join(options.distDir, 'latest-mac.yml')
150+
let metadata = readFileSync(metadataPath, 'utf8')
151+
const credentialArgs = buildNotarytoolArguments(options.env)
152+
153+
for (const filename of dmgs) {
154+
const dmgPath = join(options.distDir, filename)
155+
const notarization = runCommand('xcrun', [
156+
'notarytool', 'submit', dmgPath, '--wait', '--output-format', 'json', ...credentialArgs,
157+
])
158+
const notaryOutput = [notarization.stdout.trim(), notarization.stderr.trim()].filter(Boolean).join('\n')
159+
if (notaryOutput !== '') log(notaryOutput)
160+
if (notarization.status !== 0) {
161+
throw new Error(`notarytool failed for ${filename} with status ${String(notarization.status)}:\n${notaryOutput}`)
162+
}
163+
164+
let status: unknown
165+
try {
166+
status = (JSON.parse(notarization.stdout) as { readonly status?: unknown }).status
167+
} catch {
168+
throw new Error(`notarytool returned invalid JSON for ${filename}:\n${notaryOutput}`)
169+
}
170+
if (status !== 'Accepted') {
171+
throw new Error(`notarytool did not accept ${filename} (status: ${String(status)}):\n${notaryOutput}`)
172+
}
173+
174+
const stapling = runCommand('xcrun', ['stapler', 'staple', dmgPath])
175+
const staplerOutput = [stapling.stdout.trim(), stapling.stderr.trim()].filter(Boolean).join('\n')
176+
if (staplerOutput !== '') log(staplerOutput)
177+
if (stapling.status !== 0) {
178+
throw new Error(`stapler failed for ${filename} with status ${String(stapling.status)}:\n${staplerOutput}`)
179+
}
180+
181+
const size = statSync(dmgPath).size
182+
const sha512 = createHash('sha512').update(readFileSync(dmgPath)).digest('base64')
183+
metadata = rewriteLatestMacYaml(metadata, filename, sha512, size)
184+
185+
const blockmapPath = `${dmgPath}.blockmap`
186+
if (existsSync(blockmapPath)) {
187+
unlinkSync(blockmapPath)
188+
log(`Removed stale ${basename(blockmapPath)} because stapling changed the DMG; electron-updater will use a full download.`)
189+
}
190+
}
191+
192+
writeFileSync(metadataPath, metadata)
193+
}

apps/desktop/scripts/release-mac.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { spawnSync } from 'node:child_process'
44
import { dirname, resolve } from 'node:path'
55
import { fileURLToPath } from 'node:url'
6+
import { finalizeMacArtifacts } from './finalize-mac-artifacts'
67
import { adaptMacReleaseEnvironment, assertMacReleaseReady } from './release-preflight'
78

89
const RELEASE_VARIABLES = [
@@ -50,6 +51,10 @@ export function releaseMac(): void {
5051
'exec', 'electron-builder', '--mac', 'dmg',
5152
'--config.forceCodeSigning=true', '--config.mac.notarize=true',
5253
], desktopRoot, releaseEnvironment)
54+
finalizeMacArtifacts({
55+
distDir: resolve(desktopRoot, 'dist'),
56+
env: releaseEnvironment,
57+
})
5358
}
5459

5560
const invokedPath = process.argv[1]

apps/desktop/scripts/release-preflight.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ function resolveCredentialGroup(
112112
return source
113113
}
114114

115-
function resolveNotarizationCredentials(env: NodeJS.ProcessEnv): NotarizationCredentialSource {
115+
export function resolveNotarizationCredentials(env: NodeJS.ProcessEnv): NotarizationCredentialSource {
116116
const appleId = resolveCredentialGroup(
117117
env,
118118
['APPLE_ID', 'APPLE_APP_SPECIFIC_PASSWORD', 'APPLE_TEAM_ID'],

apps/desktop/src/host-supervisor.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,30 @@ export function isPortInUseError(message: string): boolean {
2929
return /EADDRINUSE|address already in use/iu.test(message)
3030
}
3131

32+
/** The live server described by the Host's single-instance conflict line. */
33+
export interface RunningServerConflict {
34+
readonly pid: number
35+
readonly port: number
36+
readonly startedAt: string
37+
}
38+
39+
/**
40+
* Recognize the Host's single-instance lock conflict.
41+
*
42+
* The server refuses to start while another one holds the lock at
43+
* `<PYTHINKER_CODE_HOME>/server/lock`, and that lock is global rather than
44+
* per-port, so a CLI server on any port blocks the desktop Host. Without this
45+
* the conflict surfaces only as a generic non-zero exit, which tells the user
46+
* nothing about which process to stop.
47+
* @param message - Host output, including the diagnostic appended on failure.
48+
* @returns The conflicting server's details, or undefined for other failures.
49+
*/
50+
export function parseRunningServerConflict(message: string): RunningServerConflict | undefined {
51+
const match = /server already running \(pid=(\d+), port=(\d+), started=([^)]*)\)/u.exec(message)
52+
if (match === null) return undefined
53+
return { pid: Number(match[1]), port: Number(match[2]), startedAt: match[3]! }
54+
}
55+
3256
/** Incremental parser for the Web Host's canonical readiness line. */
3357
export interface ReadinessParser {
3458
/**

apps/desktop/src/main.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
import {
2828
createHostSupervisor,
2929
isPortInUseError,
30+
parseRunningServerConflict,
3031
resolveDesktopPort,
3132
spawnPythinkerServer,
3233
type HostSupervisor,
@@ -345,6 +346,22 @@ async function boot(): Promise<void> {
345346
} catch (error) {
346347
track('desktop_server_failed')
347348
const message = error instanceof Error ? error.message : String(error)
349+
350+
const conflict = parseRunningServerConflict(message)
351+
if (conflict !== undefined) {
352+
const conflicted = await dialog.showMessageBox({
353+
type: 'error',
354+
buttons: ['Retry', 'Quit'],
355+
defaultId: 0,
356+
cancelId: 1,
357+
title: `${APP_NAME} cannot start its server`,
358+
message: `Another Pythinker server is already running (process ${String(conflict.pid)} on port ${String(conflict.port)}, started ${conflict.startedAt}). Only one server can run at a time, because they would share the same session files. Stop that server, then retry.`,
359+
})
360+
if (conflicted.response === 0) continue
361+
await requestAppQuit()
362+
return
363+
}
364+
348365
if (!isPortInUseError(message)) throw error
349366

350367
const result = await dialog.showMessageBox({

0 commit comments

Comments
 (0)