Repository navigation
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
I checked the CLI/client side outside the app. With the configured port held by an unrelated listener, Service.ensure on the base fails after ~21s with a generic "Server process exited with code 1". With this PR it fails after ~16s with a typed PortConflictError carrying the port. The new and nearby client, CLI, desktop and app tests pass, and all four packages type-check. I couldn't run the desktop app, so the conflict screen and save-and-relaunch are untested here, as the description also says. A recording of that flow would help.
The design holds up: recovery is only offered for a conflict reported through the private pipe, never from stderr text; the port is validated both in the IPC schema and in the form; and service set port only writes config. The new tests are small and to the point. Two concerns (inline): failing early can leave the other startup attempt running in the background, where it may still bind the port; and the file-descriptor-3 pipe needs a check on Windows.
| ), | ||
| ) | ||
| yield* Effect.try(() => { | ||
| if (startupPipe) writeSync(3, JSON.stringify({ type: "port-conflict", hostname, port })) |
There was a problem hiding this comment.
This assumes the bundled Bun executable can write to an extra pipe inherited from Electron's spawn as file descriptor 3. That works on Linux, but please confirm it on Windows. If the write fails there, it is silently ignored and users get the generic error page with no recovery. That's no worse than today, but it means the fix quietly doesn't apply on Windows.
There was a problem hiding this comment.
Added an end-to-end smoke check using the actual Node service client to spawn the compiled CLI and assert receipt of PortConflictError. It passes on macOS and is included in the existing Linux/Windows CI smoke checks. Windows verification is still pending; I’m not claiming platform confirmation yet.
Issue for this PR
Type of change
What does this PR do?
How did you verify your code works?
bun run checkpasses.Screenshots / recordings
Trimmed.PR.Screen.recording.mov
Checklist