fix(desktop): pin the Host port and stop reporting non-updatable builds as errors - #87
Merged
Merged
Conversation
…ds as errors Two defects reported from the packaged app. The Host was started with --port 0, so the window loaded a different http://127.0.0.1:<port> origin on every launch. localStorage is keyed by origin, so the web UI's persisted state was discarded each time: the first-run onboarding dialog reappeared, and theme, colour scheme, UI font size, permission mode, thinking level, plan mode, starred models, unread state and the active workspace all reset. Packaged builds now use port 24827 and development uses 24828, with a validated PYTHINKER_DESKTOP_PORT override. There is deliberately no fallback port. A fallback would reintroduce the same silent data loss on exactly the machines most likely to hit a collision, so a bind failure now shows a dialog naming the port and the override, offering Retry or Quit. A locally packed build (electron-builder --dir) sets isPackaged but ships no app-update.yml, so the first update check threw ENOENT and Settings showed a red error. The updater now checks for that file first and reports the calm 'disabled' state instead.
|
Warning Review limit reached
Next review available in: 6 minutes Limit details: You’ve used all 3 included reviews currently available under your plan. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Comment |
commit: |
5 tasks
elkaix
added a commit
that referenced
this pull request
Aug 16, 2026
## Related Issue Follow-up to #87, which fixed the desktop update *client*. This fixes the update *channel*. ## Problem Auto-update is broken in shipped desktop builds, independently of the client-side bugs fixed in #87. `electron-updater`'s public GitHub provider resolves the repository's newest release. This repository publishes CLI releases continuously, so `https://github.com/PyModel/pythinker-code/releases/latest` resolves to a CLI release — currently `@pymodel/pythinker-code@0.19.0` — which contains no desktop assets and no `latest-mac.yml`. Every desktop update check therefore 404s. One repository was publishing two unrelated release streams into one "latest" channel. ## What changed - **Desktop releases now publish to `PyModel/pythinker-desktop-releases`**, a repository that holds desktop artifacts and update metadata only, so "latest release" unambiguously means the desktop app. - **`releaseType: "release"`** is set explicitly. `electron-publish`'s GitHub publisher otherwise creates a **draft**, and a draft is invisible to the updater — this would have failed quietly. - **Both release jobs mint a GitHub App installation token** for the publish step. `GITHUB_TOKEN` is scoped to the repository running the workflow and cannot publish into another one. - **Both jobs now assert `app-update.yml` exists inside the packaged application.** A build without it cannot self-update, and after #87 the app reports itself as non-updatable rather than erroring — so shipping such a build would be a silent regression. The check is the last command in its step, so a missing file fails the job. ### Required before the next desktop release Two repository secrets, from a GitHub App installed on `PyModel/pythinker-desktop-releases` with **Contents: write**: - `DESKTOP_RELEASES_APP_ID` - `DESKTOP_RELEASES_APP_PRIVATE_KEY` There is deliberately **no fallback** to `GITHUB_TOKEN`. A fallback would publish to the wrong repository and silently recreate this bug; a missing secret must fail the release loudly instead. Nothing breaks before then — this workflow only runs on `desktop-v*` tags. ### Alternatives considered and rejected - **Generic provider against the `code.pythinker.com` CDN.** Needs its own credential *and* atomic publishing, cache-control correctness, and availability monitoring that the GitHub path gets free. - **One repository with `allowPrerelease` and a `desktop` channel.** This can be made to work with semver-valid tags, since the provider scans the Atom feed and skips invalid tags. Rejected because Atom retention is not a channel contract: enough CLI releases can push every desktop entry out of the feed, and the failure returns silently. ### Migration note Clients on the existing `v0.1.0` pre-release cannot learn about the new repository and will need one manual reinstall. That cost is already sunk — their update path is broken today regardless — and it only grows with the installed base, which is why this moves now rather than later. ## Verification - `pnpm --filter @pymodel/pythinker-desktop exec vitest run` — **80 passed**, including a new assertion that the publish target is the releases repo with `releaseType: release`. Recorded red against the old config first. - `pnpm run lint` — exit 0. - Workflow shape asserted mechanically: the app-token step present in both jobs, no publish step left on `GITHUB_TOKEN`, and a packaged-feed guard in both jobs. ## 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. — `@pymodel/pythinker-desktop` is private and changeset-ignored. - [ ] Ran `gen-docs` skill, or this PR needs no doc update.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
No issue — both defects were reported directly from the packaged app.
Problem
1. Every launch was a fresh browser profile.
spawnPythinkerServerstarted the Host with--port 0, so the window loadedhttp://127.0.0.1:<random>on every launch.localStorageis keyed by origin, so the web UI read anempty store each time.
The visible symptom was the first-run onboarding dialog appearing on every launch, but it was never
an onboarding bug — every persisted preference was being discarded: theme, colour scheme, UI font
size, permission mode, thinking level, plan mode, dynamic-workflow and goal mode, starred models,
unread state, and the active workspace.
2. Settings showed a red update error.
A locally packed build (
electron-builder --dir) setsapp.isPackaged = truebut carries noapp-update.yml, so the first check threw and surfaced as a failure. A build that simply cannotself-update should say so calmly.
What changed
PYTHINKER_DESKTOP_PORToverride that fails loudly on a bad value rather than silently reverting to a default.
the machines most likely to hit a collision. A bind failure now shows a dialog naming the port and
the override, offering Retry (rebuilds the supervisor and retries the same port, so the user can
free it and continue) or Quit.
during normal development, which is the divergence that produced this report.
initUpdater,checkForUpdatesNowandquitAndInstallNowall check forapp-update.ymlunderresourcesPathfirst and reportdisabledwhen it is absent. No listenersare wired and no timers are scheduled on such a build.
Design alternatives considered and rejected
These were argued out before implementing, and the reasons are recorded here so they are not
re-litigated later:
localStorageinto desktop-owned storage via IPC. Fixes today's keys,but leaves the next
localStorageuse silently broken and forks behaviour between the browser andElectron.
app://scheme.apps/pythinker-web/src/api/config.tsderivesboth the HTTP base and the WebSocket URL from
window.location.origin, so this forces endpointinjection plus a CORS and WebSocket-origin story into a client that has none — an architecture
change to fix a storage bug. Worth revisiting only if fixed ports prove to fail in the field.
Verification
Run in this branch's worktree:
pnpm --filter @pymodel/pythinker-desktop exec vitest run— 85 passed (79 on the base plus 6new). Each new test was watched failing against the old behaviour before the source changed.
pnpm --filter @pymodel/pythinker-desktop run typecheck— exit 0.pnpm run lint— exit 0.New coverage:
resolveDesktopPortdefaults per build type, a valid override winning, invalidoverrides (non-numeric and out-of-range) throwing rather than falling back,
isPortInUseErrorclassification, the port reaching the spawned argv, and
initUpdaterstayingdisabledwith noevents wired when
app-update.ymlis absent.Known follow-up (not in this PR)
Auto-update is still broken in shipped builds for a separate reason:
pythinker-codepublishesCLI releases continuously, so
/releases/latestresolves to a CLI release with nolatest-mac.yml,and electron-updater's public GitHub provider follows exactly that. The fix is to publish desktop
releases to their own repository; that lands separately because it needs a cross-repo publishing
credential.
Checklist
gen-changesetsskill, or this PR needs no changeset. —@pymodel/pythinker-desktopis private and changeset-ignored.gen-docsskill, or this PR needs no doc update.