fix(desktop): support configured server ports

[pascalandr]

Fixes #293

Summary

• read persisted httpPort and httpsPort values from the desktop config
• pass those fixed ports to the bundled server when launcher-started desktop sessions come up
• keep environment variables as the highest-priority override when they are explicitly set

Validation

• npm run bundle:server && cargo build
• visual end-to-end validation of the configured-port flow is still pending

[pascalandr]

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

[codenomadbot]

Gatekeeper review of the latest PR state for fix(desktop): support configured Tauri server ports.

What I reviewed

• Current PR diff against origin/dev
• The changed Rust launcher path in packages/tauri-app/src-tauri/src/cli_manager.rs
• The server-side explicit-port handling in packages/server/src/index.ts
• Validation run: npm run bundle:server && cargo build

What I understood the request to be

• Check the latest PR state for regressions
• Look for stronger implementation alternatives
• Review with a strict gatekeeper mindset

Findings

• No blocking regressions found in the current diff.

Why this looks correct

• The launcher now reads persisted httpPort / httpsPort from the same desktop config source it already uses for listeningMode.
• It only injects --http-port / --https-port when CLI_HTTP_PORT / CLI_HTTPS_PORT are not explicitly set, so environment variables still remain the highest-priority override.
• That matches the server behavior in packages/server/src/index.ts, where explicit CLI args or env vars are what prevent fallback-to-random-port binding.

Residual risk / gap

• I do not see targeted automated coverage for the new precedence path: env var > persisted config > server default/random bind behavior. The implementation is reasonable, but this path is currently protected mainly by manual reasoning and compile validation rather than a focused regression test.

Validation

• npm run bundle:server && cargo build passed on the latest PR state.

Non-blocking suggestion

• The launcher now parses config separately for listeningMode and port resolution. If this area grows further, consolidating startup config resolution into a single read would reduce duplication and drift risk, but I would not block the PR on that.

--
Yours,
CodeNomadBot

[pascalandr]

Follow-up for the bot review, pushed in 3b3ac1d:

• added focused Rust tests for the precedence path
• covered server > preferences port selection
• covered env var > persisted config override behavior
• covered YAML config loading for the desktop wrapper

Validation:

• npm run bundle:server && cargo test on the PR branch

[shantur]

@pascalandr -

• What updates the config with the server port?
• What happens if there is another process listening on the port?
• Is there any UI component to update the port?
• Electron would need to update to handle this feature too.

[pascalandr]

Right, I only looked at the tip of the iceberg. Converting this to draft to rework it properly.

[pascalandr]

@shantur

@pascalandr -

* What updates the config with the server port?

Nothing in this PR. It only honors already-persisted httpPort / httpsPort values.

* What happens if there is another process listening on the port?

Startup fails, same as any explicit fixed-port bind.

* Is there any UI component to update the port?

Not in this PR. Follow-up : Pagecran#9

* Electron would need to update to handle this feature too.

I’ve now added the same behavior to Electron as well.

[github-actions]

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/24835564866

Artifacts expire in 7 days.
Artifacts: (none found on this run)

[pascalandr]

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

[codenomadbot]

Gatekeeper review of the latest PR state for fix(desktop): support configured server ports.

What I understood the request to be

• Re-review the current PR tip, not the earlier version
• Check for regressions
• Look for better implementations
• Review with a strict gatekeeper mindset

What changed since the earlier review

• The PR now includes Electron support in addition to the Tauri launcher path
• Focused precedence tests were added for both desktop paths

Findings

• No blocking findings in the latest diff.

Why the current implementation holds up

• Electron and Tauri now follow the same effective precedence model: env var > persisted server config > persisted preferences config > server defaults/random bind behavior.
• The extracted Electron helper in packages/electron-app/electron/main/cli-config.ts removes duplicated config-path parsing from process-manager.ts and keeps the Tauri/Electron behavior aligned.
• The added tests cover the main correctness points that were previously missing: server > preferences, env var override priority, and YAML config loading.

Validation I ran on the latest PR state

• cargo test in packages/tauri-app/src-tauri
• npm run typecheck in packages/electron-app
• node --test --import tsx electron/main/cli-config.test.ts in packages/electron-app
• npm run build in packages/electron-app

Result

• All of the above passed.

Residual risk / gap

• The functional risk that remains is product-level rather than implementation-level: fixed configured ports will still fail startup if another process already owns the port. That behavior is consistent with explicit port binding and matches the PR discussion, so I am not treating it as a regression in this change.

Non-blocking suggestion

• If this feature is expected to expand, the next quality step is end-to-end desktop coverage for the persisted-port startup path so the launcher/config/server integration is exercised as one flow rather than only by unit-level precedence tests.

--
Yours,
CodeNomadBot

[github-actions]

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/24964803415

Artifacts expire in 7 days.
Artifacts: (none found on this run)

[shantur]

Hey @pascalandr

thanks for working on this.

I would rather have it as complete feature as this can be a breaking change.
this needs to be thought through for edge cases as if user sets a wrong port in settings in UI and that port is taken already, then there is no way for user to change in the UI again as the UI server won't startup due to clashing port.

I understand user can go and manually change in the config file but that's not a good UX.