Fix site creation assigning port 0 on first site#3422
Merged
Conversation
SiteServer.create registered placeholder details with port 0 and never updated it. CLI assigned a real port but only reported back id and running via reportKeyValuePair, so the port stayed 0 in the desktop process and leaked to the renderer (visible as http://localhost:0) until the next app launch when fetchAll() reread cli.json. Most visible on a fresh machine creating the first site. CLI now emits port as a keyValuePair, the IPC schema accepts it, and SiteServer.create writes the real port (and updated url) into details before returning.
Guards against regressing to the placeholder port 0 leak: - SiteServer.create copies CLI port into details and computes url when the site is running. - createSiteViaCli accepts the new port keyValuePair and rejects when the CLI succeeds without reporting a real (non-zero) port or id.
Collaborator
📊 Performance Test ResultsComparing 013ef3d vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
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
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 issues
How AI was used in this PR
Used an AI assistant to help trace the data flow between desktop main process and the CLI subprocess (
SiteServer.create→createSiteViaCli→reportKeyValuePair), draft the fix, and scaffold the test cases. I reviewed all changes manually, ran the local test suite, and verified the typecheck for the affected workspaces.Proposed Changes
SiteServer.createregistered a placeholderSiteDetailswithport: 0and never updated it after the CLI assigned a real port. The CLI only reportedidandrunningback viareportKeyValuePair, so the placeholder leaked all the way to the renderer (visible ashttp://localhost:0) until the next app launch whenSiteServer.fetchAll()rereadcli.json. On a fresh machine, creating the very first site exposed the bug immediately.portas akeyValuePairalongsideidandrunning(apps/cli/commands/site/create.ts).portkey, parses it, and the success branch rejects when the CLI succeeds without a real (non-zero) port (apps/studio/src/modules/cli/lib/cli-site-creator.ts).SiteServer.createwrites the real port intodetailsand, when the site is running, transitions toStartedSiteDetailswith a correcthttp://localhost:<port>url, also updatingserver.server.url(apps/studio/src/site-server.ts).Testing Instructions
Reproduce the bug on
trunk(pre-fix):mv ~/.studio ~/.studio.bak.trunk, install, and run:npm install && npm start.Confirm the fix:
rm -rf ~/.studio(or move the backup aside).npm install && npm start.http://localhost:8881), and opening the site in the browser works on the first try, with no restart needed.updateSiteUrlcall atipc-handlers.ts:1058no longer seeslocalhost:0).mv ~/.studio.bak ~/.studio.Before
port-zero.mp4
After
port-fix.mp4
Pre-merge Checklist