Refactor: rig bridge readme updates/install scripts refactor

[ceotjoe]

What does this PR do?

Based on feedback in FB there are some flaws regarding the install scripts and documentation. This PR will address that.

Overview

This PR significantly enhances the reliability, cross-platform compatibility, and user experience of the Rig Bridge installation and update logic. The installer scripts across Windows, macOS, and Linux have been structurally refactored to securely handle dynamic configurations, safely flush out stale code bases during updates, and aggressively guarantee users are surfaced with the proper documentation requirements before booting the Setup UI.
Target Branch: staging

Installer Logic Enhancements

• Dynamic Config Handling: Installer scripts (server/routes/rig-bridge.js) no longer rely on localhost:5555. Everything is now explicitly extracted straight out of the rig-bridge-config.json file. It dynamically loads the port, tls protocols, and custom bindAddress IPs to properly formulate networking connections.
• Fool-proof Updates: Process shutdown/kills now explicitly target whatever dynamic port is configured rather than blindly striking 5555.
• Stale File Deletion: Remedied major filesystem logic bugs that left outdated files permanently stranded in local repositories. Directories are now firmly wiped using deeply-recursing cleanup tasks (rmdir /S /Q on Windows and find -mindepth 1 -maxdepth 1 ... -exec rm -rf {} + on Mac/Linux) prior to cloning the updated Git repository.
• Interactive Browser Halting: Stripped out the primitive methods that raced the browser Setup UI prior to the Node backend launching. Installers now safely run the Rig Bridge Node process in the console background, gracefully await active TCP connection handshakes locally, output a critical interaction warning forcing users to acknowledge reading rig-bridge/README.md, and completely automatically open the browser directly afterwards.

Documentation Updates

• Updated Steps 4 & 5 within the "Getting Started" segment of rig-bridge/README.md effectively clarifying the new automatic installer UI-prompting behavior.
• Cleaned up miscellaneous notes surrounding GitHub repository operations and Node requirements.

Related Issues/Files Changed

• server/routes/rig-bridge.js
• rig-bridge/README.md

Verification

• Verified directory extraction (.bat/.sh) behavior locally to confirm parent variables are rigorously structured against expansion/recursion threats.
• Verified terminal halting behaviour and dynamic browser triggering loops on all three main operating systems.

[accius]

Review — Rig Bridge installer refactor (draft)

Good direction. The dynamic port/proto/bind-address extraction and the aggressive stale-file cleanup are real improvements. A few things before moving out of draft:

Correctness

1. No timeout on "wait for listening" loops. If node rig-bridge.js fails to bind (permission, port in use, syntax error in the built JS), the shell loop while ! lsof -ti tcp:$RB_PORT …; do sleep 1; done spins forever, and the Windows :waitloop does the same. Please bound these (e.g. 30 iterations / 30s) and bail with a readable error + tail of node output.

2. Windows: node is launched with start /b (background, attached to console), so the user sees the "Press Enter" prompt, but there is no capture of the node PID. If node has already died by the time netstat check runs, :waitloop spins forever. Same timeout concern as #1.

3. Windows :nodeloopport at end polls every 2s while the port is still listening and exits when it stops — but there is no kill path for node on Ctrl-C, and nothing actually waits on node as a process (unlike the POSIX wait $NODE_PID). If the user closes the window the node child may be orphaned. Consider using start /wait or simply node rig-bridge.js in the foreground after the browser opens.

4. Linux/macOS: open detection after xdg-open is fine, but some minimal Linux distros ship an unrelated open (from util-linux) that will not open a browser. Prefer uname-gated detection: [[ "$(uname)" == "Darwin" ]] && open ….

5. Three separate PowerShell invocations for port/proto/bind in the .bat — each spawns a process (~500ms). Combine into one ConvertFrom-Json call returning all three values, then parse into env vars.

6. grep -Eo "[0-9]+$" to recover RB_PORT from SETUP_URL is brittle if the URL ever gains a path. Have the node one-liner print PORT\nURL (two lines) and read both.

7. Config path fallback: $HOME/.config/openhamclock/rig-bridge-config.json then $HOME/openhamclock-rig-bridge/…. On macOS the canonical path is $HOME/Library/Application Support/openhamclock/… — is that used anywhere? If so, add it to the fallback chain.

8. bindAddress fallback to localhost is correct for browser-open, but the 0.0.0.0 replacement happens only in the shell variant, not the .bat. Parity bug: Windows still builds http://0.0.0.0:5555 when bindAddress is 0.0.0.0, which does not open in a browser.

Docs

9. README "e.g., http://localhost:5555" — good. Consider also noting TLS case ("or https://… if you enabled TLS").

10. Prerequisite block is much clearer than before — nice.

Nits

11. "ACK" variable set but never read; comment-only prompt. Fine, just note.
12. The README language ("absolutely vital", "IMPORTANT") is a bit heavy — a single "Before continuing, please read rig-bridge/README.md for radio-specific requirements." is plenty.

Happy to re-review once the timeout/parity fixes land.

[ceotjoe]

Thanks for the thorough review — all points addressed in the latest commit.

Correctness

1 — Shell wait loop timeout
Replaced the unbounded while ! lsof with a counter loop (max 30 iterations / 30 s).
Each iteration also checks process liveness via kill -0 $NODE_PID so a crashed node
exits immediately with "check the output above" rather than spinning to the timeout.

2 & 3 — Windows node lifecycle
Dropped start /b, :waitloop, and :nodeloopport entirely. The new pattern is:

start %RB_PROTO%://%RB_HOST%:%RB_PORT%
node rig-bridge.js
pause

node runs in the foreground, so Ctrl-C kills it cleanly, there is no orphaned child,
and the window stays open on unexpected exit (via pause). The browser opens a second
or two before the port is ready, which is acceptable given it eliminates all the
lifecycle complexity.

4 — open on minimal Linux
Replaced command -v open with [[ "$(uname)" == "Darwin" ]] so the macOS open is
only called when we know we are on macOS.

5 — Three PowerShell invocations
Collapsed into one call that emits PORT|PROTO|HOST on a single line, then parsed with
for /f "tokens=1,2,3 delims=|". One process spawn instead of three.

6 — Brittle grep -Eo "[0-9]+$"
The node one-liner now writes PORT URL space-separated on one line.
read -r RB_PORT SETUP_URL reads both fields, no grep needed.

7 — macOS config path
Added $HOME/Library/Application Support/openhamclock/rig-bridge-config.json as the
second candidate in the fallback chain (before the legacy ~/openhamclock-rig-bridge/
path).

8 — bindAddress 0.0.0.0 in Windows URL
Covered by fix #5: the single PowerShell call normalises 0.0.0.0 → localhost before
RB_HOST is set, so %RB_PROTO%://%RB_HOST%:%RB_PORT% is always browser-openable.

Docs

9 — TLS note in README
Added https://localhost:5555 alongside the http:// example in step 5.

Nits

10 — Heavy language
Replaced the IMPORTANT / absolutely vital banner block in both scripts with a single
plain line: Before continuing, read rig-bridge/README.md for radio-specific setup requirements.

The unused ACK variable was left as-is per your note.

[accius]

Re-review — Rig Bridge installer refactor (still draft)

Good round of fixes. Most of the blockers from the prior pass are resolved.

Addressed well

• Item 1, 2, 3 (timeouts / orphan processes / Ctrl-C):

  • Shell: 30-iteration bounded loop, kill -0 "$NODE_PID" liveness check, wait $NODE_PID for clean foreground attachment — exactly right.
  • Windows: the whole :waitloop / :nodeloopport pair is replaced with start <url> + foreground node rig-bridge.js. Clean, no orphan risk, Ctrl-C kills node naturally. Much simpler.

• Item 4 (open on minimal Linux): xdg-open checked first, then [[ "$(uname)" == "Darwin" ]] && open. Correct.

• Item 6 (brittle grep for port): read -r RB_PORT SETUP_URL < <(node -e …) returns both fields in one shot. Much better.

• Item 7 (macOS config path): $HOME/Library/Application Support/openhamclock/… added to the fallback chain.

• Item 8 (bindAddress 0.0.0.0 parity): normalisation now happens inside the PowerShell call too, so Windows opens localhost not 0.0.0.0.

• Item 9 (TLS note): README mentions https://localhost:5555 alongside the HTTP example.

• Item 12 (banner tone): softened to a single "Before continuing, read rig-bridge/README.md for radio-specific setup requirements." — nicely done.

Item 5 — reverted, with reason

The combined PowerShell call was tried and reverted because | inside for /f "usebackq" gets intercepted by CMD even inside double-quoted -Command strings. The revert commit captures this correctly. Three invocations × ~500 ms is acceptable for a one-time installer; flagging this as "not worth fighting CMD quoting" is the right call.

New concerns / follow-up

1. read -rp pause blocks non-interactive flows. If a user runs the installer from SSH via a deploy script or pipes it into bash, the Press Enter prompt will hang. Consider skipping it when [ ! -t 0 ] (stdin not a TTY) or behind an env var like NONINTERACTIVE=1.

2. Process-substitution < <(…) requires bash. The installer is .sh — if a user's shell is dash/ash, this breaks. Given the rest of the script already uses [[ ]], the dependency is consistent, but worth either #!/usr/bin/env bash shebang explicitly (if not already) or a POSIX-compatible rewrite using a temp file.

3. Windows: 0.0.0.0 fallback in .bat. The PowerShell expression now normalises '' or '0.0.0.0' to localhost, but the -or $b -eq '0.0.0.0' uses PowerShell string comparison which is case-insensitive and fine. Just double-check that a null bindAddress (not a string '') also falls into the IsNullOrEmpty branch — it does (IsNullOrEmpty($null) returns true).

4. Timeout error message could include last N lines of node output. Currently echo "ERROR: Rig Bridge did not start within 30 seconds." with no diagnostic. Since node is running with inherited stdout, errors are visible above the message — so this is only a nit.

Small doc note

The Prerequisites block has grown quite long for an inline > blockquote. Consider moving it to its own small Prerequisites ### section to keep the "Option A" steps skimmable. Cosmetic, not a blocker.

Ready for un-draft?

From my side the installer is in good shape to come out of draft once you're happy with the SSH / non-interactive behaviour. The only other thing to verify is full round-trip on each OS (install fresh, update, config-port change, TLS case).

[ceotjoe]

Thanks again — all actionable points addressed.

read -rp blocking non-interactive flows
Wrapped the prompt in if [ -t 0 ] && [ "${NONINTERACTIVE:-0}" != "1" ].
Piped installs and deploy scripts skip it automatically; the env var gives
an explicit opt-out for anything else.

Process substitution / bash dependency
Shebang changed from #!/bin/bash to #!/usr/bin/env bash, making the
dependency explicit and consistent with the [[ ]] and < <(…) usage
already in the script.

Windows bindAddress null check
Confirmed — [string]::IsNullOrEmpty($null) returns $true, so a null
bindAddress falls into the localhost branch correctly. No change needed.

Timeout diagnostic
Agreed it's a nit. Since node inherits stdout the errors are already
visible above the message, so left as-is.

Prerequisites block
Promoted from an inline blockquote inside Option A to its own
#### Prerequisites section between the Step 1 heading and Option A,
keeping the numbered steps skimmable.