Skip to content

fix: target the live server port for server-backed CLI commands - #264

Merged
rubenfiszel merged 1 commit into
mainfrom
fix/cli-live-server-port
Jun 1, 2026
Merged

fix: target the live server port for server-backed CLI commands#264
rubenfiszel merged 1 commit into
mainfrom
fix/cli-live-server-port

Conversation

@rubenfiszel

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #263. That PR fixed the error message when the webmux server is unreachable, but the underlying cause of webmux oneshot --linear=<TEAM> failing with Unable to connect... was a port mismatch: the server was running on 5112 (a webmux serve --port 5112 service) while the CLI hardcoded the 5111 default.

webmux serve walks to a free port when 5111 is taken, but server-backed CLI commands (oneshot, linear, send) only ever used --port/PORT/5111 and never consulted the instance registry — so they connected to a dead port. In-process commands (add, list, open, ...) talk to tmux/git/fs directly, which is why webmux add worked while webmux oneshot didn't.

This resolves the live instance's port from the registry by matching the current project dir when --port/PORT isn't set.

Changes

  • bin/src/instance-port.ts (new): selectInstancePort (pure) + resolveLiveServerPort (I/O) resolve the port of the live webmux instance serving the current project, falling back to the sole live instance, then the default.
  • bin/src/webmux.ts: when --port/PORT isn't set, resolve the live server port before dispatching CLI commands; --debug logs the resolved source. serve is unaffected. Updated --port help text.
  • bin/src/instance-port.test.ts: project match, subdir match, preference over other instances, sole/default fallbacks, and a sibling-prefix guard.

Test plan

  • bun test bin/src/instance-port.test.ts passes
  • With a webmux serve --port 5112 running for the current project, webmux oneshot --prompt 'x' --linear=TEAM (no --port) connects to 5112 instead of failing on 5111
  • webmux --debug oneshot ... logs resolved port 5112 from live instance (project)
  • webmux --port 5111 ... still forces 5111 (explicit port wins)

Generated with Claude Code

`webmux serve` walks to a free port when 5111 is taken (e.g. a service
installed on 5112), but server-backed CLI commands (oneshot, linear,
send) hardcoded the 5111 default and never consulted the instance
registry — so they tried to connect to a port nothing was listening on
and failed. In-process commands (add, list, open, ...) were unaffected,
which is why `webmux add` worked while `webmux oneshot` did not.

Resolve the port from the live instance registry by matching the
current project's dir when --port/PORT isn't set, falling back to the
sole live instance, then the 5111 default.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rubenfiszel
rubenfiszel marked this pull request as ready for review June 1, 2026 21:22
@rubenfiszel
rubenfiszel merged commit 3b0a82f into main Jun 1, 2026
3 of 4 checks passed
@rubenfiszel
rubenfiszel deleted the fix/cli-live-server-port branch June 1, 2026 21:22
@claude

claude Bot commented Jun 1, 2026

Copy link
Copy Markdown

Claude finished @rubenfiszel's task in 4m 12s —— View job


Code Review: PR #264 — fix: target the live server port for server-backed CLI commands

This is a clean, well-scoped fix for a real bug. The pure-function + I/O-wrapper split, the comprehensive test for selectInstancePort, and the WEBMUX_PORT_STRICT interplay are all spot on. A few minor observations below.

✅ Strengths

  • Architecture follows project conventions. selectInstancePort (pure) is isolated from resolveLiveServerPort (I/O), exactly as backend/CLAUDE.md prescribes (“Pure functions for logic, side-effectful functions clearly separated”). The pure half is fully unit-tested; the I/O wrapper is a trivial composition.
  • The sibling-prefix test (bin/src/instance-port.test.ts:79-86) is a real foot-gun avoided — /home/me/windmill-fork no longer false-matches /home/me/windmill. Good defensive test.
  • Backward-compat is preserved. Explicit --port/PORT still wins because the registry lookup is gated on !parsed.portExplicit (bin/src/webmux.ts:358), so existing CI/scripts that pin the port are unaffected.
  • Debug logging is gated correctly. bin/src/webmux.ts:362-364 only logs when source !== "default", so the message is meaningful (it only fires when the registry actually changed the answer).
  • All server-backed surfaces are covered. oneshot, linear, and the worktree send subcommand all flow through effectivePort. I verified worktree-commands.ts:813 (send) and the runtime's controlBaseUrl (backend/src/runtime.ts:63) both consume the port, so passing effectivePort to worktree commands is correct, not just speculative.

🟡 Minor observations

  1. isInside edge case for parent === "/"bin/src/instance-port.ts:19-22:

    const root = parent.endsWith("/") ? parent.slice(0, -1) : parent;
    return child === root || child.startsWith(`${root}/`);

    If entry.projectDir === "/", then root === "" and child.startsWith("/") matches every absolute path — i.e. a root-owned instance would falsely match every CWD. This is unrealistic (no one serves / as a project dir), but a single-line guard (if (root === "") return child === "/";) would make it bulletproof and is easier than reasoning about why it can't happen.

  2. resolveLiveServerPort's registryDir parameter is unusedbin/src/instance-port.ts:48-58 exposes an optional registryDir for testability, but nothing in the test file uses it (the tests target the pure selectInstancePort directly). It's not harmful, but it's dead optionality you could drop until you actually need it.

  3. Per-invocation registry lookup adds a small I/O cost to every non-pinned CLI command, including in-process ones like webmux list. Each entry triggers a process.kill(pid, 0) and dead entries get unlinkSync'd. For users with many historical instances this is negligible but non-zero. The architectural alternative would be to only resolve for the server-backed surfaces (oneshot/linear/send), but the current “resolve once at the top of main” is simpler and consistent — fine as is, just flagging the trade-off.

  4. Symlink/case-sensitivityisInside compares raw strings. If cwd is reached via a symlink (e.g. /tmp/private/tmp on macOS) and entry.projectDir is the canonical form (or vice-versa), the project match silently fails and you'd fall through to sole/default. Not worth normalizing now (you'd need to decide on canonicalization at registry write time too), but worth keeping in your back pocket if someone reports it.

Verification notes

  • I checked the dispatch chain: only oneshot, linear, and the worktree send subcommand actually open HTTP to localhost:${port} (bin/src/oneshot.ts:502/600/646/720, bin/src/linear-commands.ts:129, bin/src/worktree-commands.ts:813). All three receive effectivePort — coverage is complete.
  • serve is correctly excluded — line 402 passes the original parsed.port (not effectivePort) into the backend env, so webmux serve still binds the port the user asked for (or falls back via bindServer() in backend/src/server.ts:2057).
  • backend/CLAUDE.md compliance: explicit return types on all new functions ✅, no any/as ✅, Bun test runner ✅.

Summary

LGTM. The minor items above (especially the parent === "/" guard) are polish, not blockers — feel free to land as-is or fold in a follow-up.

  • Read PR changes
  • Review bin/src/instance-port.ts
  • Review bin/src/instance-port.test.ts
  • Review bin/src/webmux.ts
  • Check CLAUDE.md/AGENTS.md guidelines compliance
  • Post review feedback
    • Branch: claude/pr-264-20260601-2122

@claude

claude Bot commented Jun 1, 2026

Copy link
Copy Markdown

Claude finished @rubenfiszel's task in 2m 59s —— View job


Code Review — PR #264

  • Read the three changed files
  • Read related context (server.ts, instance-registry, runtime, shared)
  • Analyze logic and edge cases
  • Post review findings

Overall: clean fix with good pure/IO separation and solid coverage of the project-matching logic. The fallback chain (project → sole → default) is sensible and the --debug log only firing on non-default sources avoids noise.

Findings

🟡 Worktree cwd does not match the registered projectDir

bin/src/instance-port.ts:50-55 builds candidates from cwd and git rev-parse --show-toplevel, but the backend registers projectDir via projectRoot() in backend/src/adapters/config.ts:539-545, which resolves to the git common dir's parent (i.e. the main repo) — not the worktree path. From a worktree:

  • cwd/home/me/windmill-wt/feature
  • getGitRoot() (--show-toplevel) → /home/me/windmill-wt/feature
  • Registered entry.projectDir/home/me/windmill

Neither candidate is "inside" the entry's projectDir, so the project match fails and we fall through to sole / default. In a multi-instance setup the resolver lands on 5111 — exactly the scenario this PR sets out to fix. For a worktree-management tool this is the common path, not an edge case.

Suggested fix: also include git rev-parse --git-common-dir's parent in candidateDirs (or call the backend's projectRoot() for the canonical root). Worth adding a test for "cwd is a worktree of the registered project".

Fix this →

🟢 cwd parameter is plumbed but ignored for git root

resolveLiveServerPort({ cwd }) at bin/src/instance-port.ts:45-56 accepts a cwd, but getGitRoot() in bin/src/shared.ts:23-27 always runs git rev-parse in process.cwd() with no override. Today the only call site passes process.cwd() so the two coincide, but the API contract is misleading. Either thread cwd into getGitRoot(cwd), or drop the cwd parameter and read process.cwd() directly.

🟢 Pure selectInstancePort is well-tested; I/O wrapper isn't

The seven cases for selectInstancePort cover the important branches (root match, subdir, project preference, sole fallback, default fallback, no instances, sibling-prefix guard). The I/O wrapper resolveLiveServerPort has no test — its integration with createInstanceRegistry and getGitRoot is untested. Optional: pass registryDir to a temp dir + seed entries to exercise the wrapper end-to-end.

🟢 PORT env counts as "explicit" and bypasses live-server resolution

bin/src/webmux.ts:104 treats any set PORT env var as user intent, skipping the resolver. This is documented in the JSDoc and matches the "explicit port wins" behavior called out in the PR description, but worth flagging: a PORT value accidentally exported from a shell config / IDE env will silently disable the new behavior. Not a blocker.

Verified ✓

  • Backend registration path: backend/src/server.ts:2096-2104 registers port: BOUND_PORT (post-walk), so the registry has the right port even after EADDRINUSE fallback.
  • isInside correctly handles trailing slashes and the sibling-prefix case (windmill-fork vs windmill).
  • serve still uses parsed.port (not effectivePort) when binding — line bin/src/webmux.ts:402. Correct.
  • Dynamic import of ./instance-port.ts is gated behind the non-explicit-port branch, so the registry isn't touched when --port is set.
    · claude/pr-264-20260601-2122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant