diff --git a/.changeset/desktop-fixed-port.md b/.changeset/desktop-fixed-port.md new file mode 100644 index 00000000..ebb09104 --- /dev/null +++ b/.changeset/desktop-fixed-port.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": minor +--- + +The desktop app now uses a fixed loopback port so browser-stored settings persist between launches, shows a retry dialog when that port is occupied, and reports updates as unavailable for builds packaged without an update feed. diff --git a/.changeset/question-lease-and-labels.md b/.changeset/question-lease-and-labels.md new file mode 100644 index 00000000..85d88046 --- /dev/null +++ b/.changeset/question-lease-and-labels.md @@ -0,0 +1,5 @@ +--- +'@pymodel/pythinker-code': minor +--- + +Questions now stay open for 30 minutes instead of 60 seconds, and the card warns when the lease is close to its end. An expired question is no longer reported to the agent as a user dismissal, answers carry the question text and the option labels the user saw instead of internal ids, Escape no longer dismisses an open question, and a question that cannot be delivered now fails as a tool error instead of a silent dismissal. diff --git a/apps/desktop/src/host-supervisor.ts b/apps/desktop/src/host-supervisor.ts index 89f525c8..2d40ee7c 100644 --- a/apps/desktop/src/host-supervisor.ts +++ b/apps/desktop/src/host-supervisor.ts @@ -9,6 +9,26 @@ const DEFAULT_SHUTDOWN_TIMEOUT_MS = 5_000 const TASKKILL_TIMEOUT_MS = 5_000 const MAX_STARTUP_OUTPUT_CHARS = 32_768 +export const DESKTOP_PACKAGED_PORT = 24_827 +export const DESKTOP_DEV_PORT = 24_828 + +/** Resolve the fixed Host port, with an optional validated environment override. */ +export function resolveDesktopPort(env: NodeJS.ProcessEnv, isPackaged: boolean): number { + const value = env['PYTHINKER_DESKTOP_PORT'] + if (value === undefined) return isPackaged ? DESKTOP_PACKAGED_PORT : DESKTOP_DEV_PORT + + const port = Number(value) + if (!/^\d+$/u.test(value) || !Number.isInteger(port) || port < 1 || port > 65_535) { + throw new Error(`PYTHINKER_DESKTOP_PORT must be an integer from 1 to 65535; received ${JSON.stringify(value)}`) + } + return port +} + +/** Return whether Host output reports that its fixed port is already occupied. */ +export function isPortInUseError(message: string): boolean { + return /EADDRINUSE|address already in use/iu.test(message) +} + /** Incremental parser for the Web Host's canonical readiness line. */ export interface ReadinessParser { /** @@ -257,6 +277,8 @@ export interface SpawnPythinkerServerOptions { readonly cwd: string /** Frozen environment for the Host process. */ readonly env: NodeJS.ProcessEnv + /** Fixed loopback port for the Host server. */ + readonly port: number /** Run the Electron executable as its bundled Node runtime. */ readonly electronRunAsNode?: boolean } @@ -272,7 +294,7 @@ function streamAdapter(stream: NodeJS.ReadableStream): HostChild['stdout'] { } /** - * Spawn the production Pythinker server on an OS-assigned loopback port. + * Spawn the production Pythinker server on a fixed loopback port. * @param options - Node runtime, built CLI and process environment. * @returns The child handle consumed by {@link createHostSupervisor}. */ @@ -286,7 +308,7 @@ export function spawnPythinkerServer(options: SpawnPythinkerServerOptions): Host 'run', '--foreground', '--port', - '0', + String(options.port), '--log-level', 'error', ], { diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 30522f41..bed37ad7 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -23,7 +23,13 @@ import { setCrashPhase, track, } from '@pymodel/pythinker-telemetry' -import { createHostSupervisor, spawnPythinkerServer, type HostSupervisor } from './host-supervisor' +import { + createHostSupervisor, + isPortInUseError, + resolveDesktopPort, + spawnPythinkerServer, + type HostSupervisor, +} from './host-supervisor' import { createSplashWindow } from './splash' import { checkForUpdatesNow, @@ -302,6 +308,7 @@ async function boot(): Promise { if (bootQuitPromise !== undefined) return initializeDesktopTelemetry() const paths = hostPaths() + const port = resolveDesktopPort(process.env, app.isPackaged) assertHostArtifacts(paths) const splash = createSplashWindow(desktopResources('splash')) const destroySplash = (): void => { @@ -312,26 +319,43 @@ async function boot(): Promise { const trayFrames = loadTrayImages() createTray(trayFrames) stopTrayAnimation = startTrayAnimation(trayFrames) - host = createHostSupervisor({ - spawnHost: () => spawnPythinkerServer({ - ...paths, - env: { - ...process.env, - PYTHINKER_DESKTOP: '1', + for (;;) { + host = createHostSupervisor({ + spawnHost: () => spawnPythinkerServer({ + ...paths, + env: { + ...process.env, + PYTHINKER_DESKTOP: '1', + }, + port, + }), + log: chunk => process.stderr.write(chunk), + onUnexpectedExit: ({ code, signal }) => { + console.error(`desktop Host exited unexpectedly (code ${String(code)}, signal ${String(signal)})`) + void requestAppQuit() }, - }), - log: chunk => process.stderr.write(chunk), - onUnexpectedExit: ({ code, signal }) => { - console.error(`desktop Host exited unexpectedly (code ${String(code)}, signal ${String(signal)})`) - void requestAppQuit() - }, - }) - try { - hostOrigin = await host.start() - track('desktop_server_ready') - } catch (error) { - track('desktop_server_failed') - throw error + }) + try { + hostOrigin = await host.start() + track('desktop_server_ready') + break + } catch (error) { + track('desktop_server_failed') + const message = error instanceof Error ? error.message : String(error) + if (!isPortInUseError(message)) throw error + + const result = await dialog.showMessageBox({ + type: 'error', + buttons: ['Retry', 'Quit'], + defaultId: 0, + cancelId: 1, + title: `${APP_NAME} needs its fixed port`, + message: `${APP_NAME} cannot use port ${String(port)}. It needs this fixed port so settings persist between launches. Free the port, or set PYTHINKER_DESKTOP_PORT to an unused port.`, + }) + if (result.response === 0) continue + await requestAppQuit() + return + } } stopTrayAnimation?.() stopTrayAnimation = undefined diff --git a/apps/desktop/src/updater.ts b/apps/desktop/src/updater.ts index cd6ebd4a..803f625e 100644 --- a/apps/desktop/src/updater.ts +++ b/apps/desktop/src/updater.ts @@ -1,10 +1,11 @@ -import { readFileSync, writeFileSync } from 'node:fs' +import { existsSync, readFileSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { app, type BrowserWindow } from 'electron' import electronUpdater from 'electron-updater' const { autoUpdater } = electronUpdater const UPDATE_SETTINGS_FILE = 'update-settings.json' +const UPDATES_UNAVAILABLE_MESSAGE = 'Updates are not available for this build' const INITIAL_CHECK_DELAY_MS = 10_000 const CHECK_INTERVAL_MS = 4 * 60 * 60 * 1_000 @@ -117,6 +118,20 @@ function clearTimers(): void { checkInterval = undefined } +function hasUpdateConfig(): boolean { + return existsSync(join(process.resourcesPath, 'app-update.yml')) +} + +function disableUpdates(): UpdateState { + updateState({ + status: 'disabled', + message: UPDATES_UNAVAILABLE_MESSAGE, + version: undefined, + percent: undefined, + }) + return state +} + function scheduleChecks(): void { if (checkInterval !== undefined) return checkInterval = setInterval(() => { @@ -181,6 +196,10 @@ export function initUpdater( clearTimers() if (!app.isPackaged) return + if (!hasUpdateConfig()) { + disableUpdates() + return + } try { autoUpdater.autoDownload = settings.autoUpdate @@ -232,6 +251,7 @@ export async function checkForUpdatesNow(): Promise { updateState({ status: 'disabled' }) return state } + if (!hasUpdateConfig()) return disableUpdates() try { autoUpdater.autoDownload = true @@ -251,6 +271,7 @@ export function quitAndInstallNow(): UpdateState { updateState({ status: 'disabled' }) return state } + if (!hasUpdateConfig()) return disableUpdates() try { updateTelemetryTrack('desktop_update_install') diff --git a/apps/desktop/tests/host-supervisor.spec.ts b/apps/desktop/tests/host-supervisor.spec.ts index c347f3dd..ff08b9c0 100644 --- a/apps/desktop/tests/host-supervisor.spec.ts +++ b/apps/desktop/tests/host-supervisor.spec.ts @@ -5,6 +5,7 @@ import { createReadinessParser, type HostChild, } from '../src/host-supervisor' +import * as hostSupervisor from '../src/host-supervisor' vi.mock('node:child_process', { spy: true }) @@ -112,6 +113,34 @@ describe('desktop Host readiness', () => { }) }) +describe('desktop Host port', () => { + it('uses fixed ports for packaged and development builds without an override', () => { + expect(hostSupervisor.DESKTOP_PACKAGED_PORT).toBe(24_827) + expect(hostSupervisor.DESKTOP_DEV_PORT).toBe(24_828) + expect(hostSupervisor.resolveDesktopPort({}, true)).toBe(24_827) + expect(hostSupervisor.resolveDesktopPort({}, false)).toBe(24_828) + }) + + it('uses a valid port override for packaged and development builds', () => { + const env = { PYTHINKER_DESKTOP_PORT: '45231' } + + expect(hostSupervisor.resolveDesktopPort(env, true)).toBe(45_231) + expect(hostSupervisor.resolveDesktopPort(env, false)).toBe(45_231) + }) + + it.each(['not-a-port', '70000'])('rejects an invalid port override: %s', (value) => { + expect(() => hostSupervisor.resolveDesktopPort({ PYTHINKER_DESKTOP_PORT: value }, true)) + .toThrow(new RegExp(`PYTHINKER_DESKTOP_PORT.*${value}`, 'u')) + }) + + it('detects output that reports a port collision', () => { + expect(hostSupervisor.isPortInUseError( + 'listen EADDRINUSE: address already in use 127.0.0.1:24827', + )).toBe(true) + expect(hostSupervisor.isPortInUseError('desktop Host exited before readiness (code 1, signal null)')).toBe(false) + }) +}) + describe('desktop Host supervisor', () => { it('starts one child for concurrent callers and returns its stdout readiness URL', async () => { const child = new FakeHostChild() @@ -296,6 +325,7 @@ describe('desktop Host process', () => { cliEntry: '/Applications/Pythinker.app/Contents/Resources/host/node_modules/@pymodel/pythinker-code/dist/launcher.mjs', cwd: '/Users/tester', env: { PYTHINKER_DESKTOP: '1' }, + port: 24_827, electronRunAsNode: true, }) @@ -307,7 +337,7 @@ describe('desktop Host process', () => { 'run', '--foreground', '--port', - '0', + '24827', '--log-level', 'error', ], @@ -341,6 +371,7 @@ describe('desktop Host process', () => { cliEntry: '/tmp/launcher.mjs', cwd: '/tmp', env: {}, + port: 24_827, }) host.kill('SIGTERM') @@ -379,6 +410,7 @@ describe('desktop Host process', () => { cliEntry: '/tmp/launcher.mjs', cwd: '/tmp', env: {}, + port: 24_827, }) host.kill('SIGTERM') @@ -404,6 +436,7 @@ describe('desktop Host process', () => { cliEntry: '/tmp/launcher.mjs', cwd: '/tmp', env: {}, + port: 24_827, }) host.kill('SIGTERM') diff --git a/apps/desktop/tests/updater.spec.ts b/apps/desktop/tests/updater.spec.ts index 232c50ae..c26899d9 100644 --- a/apps/desktop/tests/updater.spec.ts +++ b/apps/desktop/tests/updater.spec.ts @@ -6,15 +6,26 @@ import { afterEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => ({ app: { isPackaged: false, - getPath: () => '', + getPath: vi.fn(() => ''), + once: vi.fn(), }, })) vi.mock('electron-updater', () => ({ - default: { autoUpdater: {} }, + default: { + autoUpdater: { + on: vi.fn(), + checkForUpdates: vi.fn(), + quitAndInstall: vi.fn(), + }, + }, })) +import { app } from 'electron' +import electronUpdater from 'electron-updater' import { + getUpdateState, + initUpdater, readUpdateSettings, trackUpdateTransition, writeUpdateSettings, @@ -22,11 +33,22 @@ import { } from '../src/updater' const temporaryDirectories: string[] = [] +const resourcesPathDescriptor = Object.getOwnPropertyDescriptor(process, 'resourcesPath') +const { autoUpdater } = electronUpdater afterEach(() => { for (const directory of temporaryDirectories.splice(0)) { rmSync(directory, { recursive: true, force: true }) } + Object.defineProperty(app, 'isPackaged', { configurable: true, value: false }) + vi.mocked(app.getPath).mockReset() + vi.mocked(app.getPath).mockReturnValue('') + if (resourcesPathDescriptor === undefined) { + Reflect.deleteProperty(process, 'resourcesPath') + } else { + Object.defineProperty(process, 'resourcesPath', resourcesPathDescriptor) + } + vi.clearAllMocks() }) function temporaryDirectory(): string { @@ -76,3 +98,23 @@ describe('update telemetry transitions', () => { ]) }) }) + +describe('packaged builds without update metadata', () => { + it('disables updates without wiring updater events', () => { + const directory = temporaryDirectory() + writeUpdateSettings(directory, { autoUpdate: false }) + vi.mocked(app.getPath).mockReturnValue(directory) + Object.defineProperty(app, 'isPackaged', { configurable: true, value: true }) + Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory }) + + initUpdater(() => undefined) + + expect(getUpdateState()).toMatchObject({ + status: 'disabled', + message: 'Updates are not available for this build', + autoUpdate: false, + }) + expect(autoUpdater.on).not.toHaveBeenCalled() + expect(app.once).not.toHaveBeenCalled() + }) +}) diff --git a/apps/pythinker-web/AGENTS.md b/apps/pythinker-web/AGENTS.md index 2f7e0784..7206cfba 100644 --- a/apps/pythinker-web/AGENTS.md +++ b/apps/pythinker-web/AGENTS.md @@ -23,13 +23,13 @@ The browser web UI for Pythinker Code — a peer to the TUI in `apps/pythinker-c - Shared components go in `src/components/`; reusable logic goes in `src/composables/` with a `use` prefix. - There is **no auto-import plugin** and **no path alias** — `#/` and `@/` are intentionally unused. Write relative imports (`../i18n`, `./config`). -## i18n (normative — keeping locales in sync is manual) +## i18n (normative — the app is English-only) - Setup: `src/i18n/index.ts`, vue-i18n in Composition mode (`legacy: false`), fallback `en`. The active locale is persisted in `localStorage` under `pythinker-locale`. -- Locale files: `src/i18n/locales/{en,zh}/.ts`, each `export default { ... } as const`. New namespaces are registered in `src/i18n/locales/index.ts`. +- **`en` is the only locale.** `src/i18n/locales/` contains exactly one directory, and `locales/index.ts` registers only `en`. Do not add a second locale, and do not "restore parity" with one that does not exist. +- Locale files: `src/i18n/locales/en/.ts`, each `export default { ... } as const`. New namespaces are registered in `src/i18n/locales/index.ts`. - Reference with `const { t } = useI18n()` and `t('namespace.key')` (same form in templates). -- **Adding a key:** add it to **both** `en/.ts` and `zh/.ts`. **Adding a namespace:** create the file in both locales **and** register it in `locales/index.ts`. -- There is **no automated missing-key or en/zh parity check**. Keeping the two locales in sync is a manual responsibility — do not leave a key present in only one locale. +- **Adding a key:** add it to `en/.ts`. **Adding a namespace:** create the file under `en/` **and** register it in `locales/index.ts`. ## Commands diff --git a/apps/pythinker-web/src/components/QuestionCard.vue b/apps/pythinker-web/src/components/QuestionCard.vue index 829b4aeb..06eea682 100644 --- a/apps/pythinker-web/src/components/QuestionCard.vue +++ b/apps/pythinker-web/src/components/QuestionCard.vue @@ -30,6 +30,22 @@ const total = computed(() => props.question.questions.length); const hasPreview = computed(() => current.value.options.some((option) => option.preview?.trim()), ); +const now = ref(Date.now()); +const remainingMinutes = computed(() => { + const expiresAt = Date.parse(props.question.expiresAt); + if (Number.isNaN(expiresAt)) return undefined; + return Math.ceil((expiresAt - now.value) / 60_000); +}); +const leaseWarning = computed(() => { + const expiresAt = Date.parse(props.question.expiresAt); + if (Number.isNaN(expiresAt)) return undefined; + const remainingMs = expiresAt - now.value; + if (remainingMs <= 0 || remainingMs >= 5 * 60_000) return undefined; + if (remainingMs < 60_000) return t('question.expiresSoonSeconds'); + const minutes = remainingMinutes.value; + if (minutes === undefined) return undefined; + return t('question.expiresSoon', { minutes }); +}); function goBack(): void { if (step.value > 0) step.value--; @@ -207,17 +223,15 @@ function dismiss(): void { } // --------------------------------------------------------------------------- -// Keyboard: number keys pick options for current question, Enter submit, Esc dismiss +// Keyboard: number keys pick options for the current question and Enter submits. // --------------------------------------------------------------------------- function handleKeydown(e: KeyboardEvent): void { const tag = (document.activeElement?.tagName ?? '').toLowerCase(); if (tag === 'input' || tag === 'textarea') return; - // While minimized the options aren't visible, so don't let number keys pick - // an unseen answer; only Escape (dismiss) stays live. - if (minimized.value && e.key !== 'Escape') return; + // While minimized the options are not visible, so keyboard selection is disabled. + if (minimized.value) return; - if (e.key === 'Escape') { e.preventDefault(); dismiss(); return; } if (e.key === 'Enter') { e.preventDefault(); submit(); return; } const num = parseInt(e.key, 10); @@ -236,8 +250,19 @@ function handleKeydown(e: KeyboardEvent): void { } } -onMounted(() => document.addEventListener('keydown', handleKeydown)); -onUnmounted(() => document.removeEventListener('keydown', handleKeydown)); +let leaseTimer: ReturnType | undefined; + +onMounted(() => { + document.addEventListener('keydown', handleKeydown); + leaseTimer = setInterval(() => { + now.value = Date.now(); + }, 30_000); +}); + +onUnmounted(() => { + document.removeEventListener('keydown', handleKeydown); + if (leaseTimer !== undefined) clearInterval(leaseTimer); +});