Skip to content

fix: connect server-backed CLI commands to the live webmux instance - #263

Merged
rubenfiszel merged 1 commit into
mainfrom
fix/oneshot-server-connection-error
Jun 1, 2026
Merged

fix: connect server-backed CLI commands to the live webmux instance#263
rubenfiszel merged 1 commit into
mainfrom
fix/oneshot-server-connection-error

Conversation

@rubenfiszel

@rubenfiszel rubenfiszel commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

webmux oneshot --linear=<TEAM> failed with Unable to connect. Is the computer able to access the url? even though the webmux server was running — because it was running on port 5112 (a webmux serve --port 5112 service), while the CLI hardcoded the 5111 default.

Two root causes, both fixed here:

  1. CLI didn't target the running server. Server-backed commands (oneshot, linear, send) connect to http://localhost:<port> but only ever used --port/PORT/5111. webmux serve walks to a free port when 5111 is taken, so the live instance is often elsewhere. In-process commands (add, list, open, ...) talk to tmux/git/fs directly, which is why webmux add worked while webmux oneshot didn't.
  2. The connection error was unrecognizable. formatServerError only translated the old Bun fetch failed message into the friendly "Is it running?" hint. Current Bun throws Unable to connect. Is the computer able to access the url? (code ConnectionRefused), which slipped through and was printed raw.

Changes

  • bin/src/instance-port.ts (new): selectInstancePort (pure) + resolveLiveServerPort (I/O) resolve the port of the live webmux instance serving the current project from the instance registry, 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/shared.ts: formatServerError now also recognizes Bun's Unable to connect message.
  • Tests: bin/src/instance-port.test.ts (project match, subdir match, preference, sole/default fallbacks, sibling-prefix guard) and bin/src/shared.test.ts (HTTP passthrough, legacy + current Bun connection messages, unrelated errors).

Test plan

  • bun test bin/src/instance-port.test.ts bin/src/shared.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)
  • With no server running, the error reads Could not connect to webmux server on port 5111. Is it running?

Generated with Claude Code

Bun's fetch no longer throws "fetch failed" — it throws "Unable to
connect. Is the computer able to access the url?" (code
ConnectionRefused). formatServerError only matched "fetch", so when the
webmux server isn't running, server-backed commands like `webmux
oneshot` surfaced Bun's raw, confusing message instead of the friendly
"Could not connect to webmux server on port N. Is it running?" hint.

Match "Unable to connect" too, and add a shared.test.ts covering both
the legacy and current Bun messages.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 1, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
webmux 3a67ffb Commit Preview URL

Branch Preview URL
Jun 01 2026, 03:51 PM

@claude

claude Bot commented Jun 1, 2026

Copy link
Copy Markdown

Claude finished @rubenfiszel's task in 1m 23s —— View job


Claude Code Review

  • Read changed files (bin/src/shared.ts, bin/src/shared.test.ts)
  • Check formatServerError callers and other matching patterns
  • Verify tests cover the relevant cases
  • Post review feedback

Summary

Tightly scoped fix that does exactly what the PR description claims. The user-visible bug was real (the "fetch" substring stopped matching once Bun's error wording changed), and the new substring is correct for current Bun. The added tests are well-chosen — they pin all four branches of formatServerError (HTTP passthrough, legacy fetch failed, current Unable to connect, unrelated errors) so any future regression in either direction is caught.

LGTM to merge. A couple of optional observations below.

Observations

1. The same class of bug will likely recur (low-effort hardening available). bin/src/shared.ts:57 matches against Bun's error string, which is exactly the surface that just shifted underneath you. Bun's fetch connection-refused error also carries a structured code: "ConnectionRefused" (and the underlying cause exposes errno/code). Checking error.cause?.code === "ConnectionRefused" (or a code property on the error itself) in addition to the string match would survive the next message reword without another patch. Not blocking — the string match is simpler, more testable, and the new test will at least loudly fail next time rather than silently degrade. Worth considering for a follow-up.

2. Substring breadth. includes("Unable to connect") will match any error whose message contains that phrase. The startsWith("HTTP") check earlier in the function shields anything coming back from createApi's HTTP wrapper, and no other callsite I could find throws a similarly-worded error, so in practice this is fine. Worth flagging only because the chosen substring is fairly generic English; if you ever want to tighten it, includes("Unable to connect. Is the computer") matches Bun's exact phrasing without being meaningfully more fragile.

3. Test for the wrapper. Optional: withServerConnection (bin/src/shared.ts:65) is the actual entry point used by worktree-commands.ts:814 and isn't covered. A one-liner test asserting it rethrows with the formatted message would seal the contract end-to-end. Not necessary — formatServerError is doing all the interesting work and is now well-tested.

Nits

  • bin/src/shared.test.ts:25 is slightly past the typical wrap width but matches the repo's existing style — fine to leave.
  • The updated JSDoc on bin/src/shared.ts:48-53 is clear and accurately explains why both substrings exist. Nice.
    · Branch: fix/oneshot-server-connection-error

@rubenfiszel
rubenfiszel marked this pull request as ready for review June 1, 2026 15:58
@rubenfiszel
rubenfiszel merged commit a7d78f7 into main Jun 1, 2026
3 checks passed
@rubenfiszel
rubenfiszel deleted the fix/oneshot-server-connection-error branch June 1, 2026 15:58
@claude

claude Bot commented Jun 1, 2026

Copy link
Copy Markdown

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


Code Review — PR #263

  • Fetch and inspect the diff
  • Multi-angle review (line-by-line, removed behavior, cross-file callers, language pitfalls, wrapper correctness)
  • Verify candidates
  • Sweep for gaps
  • Post final review

Verdict: LGTM

Tightly scoped, correctly implemented, well-tested. No real bugs found.

I traced the change through every caller (worktree-commands.ts:814, linear-commands.ts:140, oneshot.ts:894, plus withServerConnection at bin/src/shared.ts:65) and through the createApi error-shaping path in packages/api-contract/src/client.ts:51. Server-side errors arrive either as "HTTP <status>" (caught by startsWith("HTTP")) or as the parsed body.error string from the JSON envelope. Nothing in backend/, frontend/, or packages/ currently produces a message containing "Unable to connect", so the new substring match cannot collide with an internal error today.

The four new tests pin every branch of formatServerError (HTTP passthrough, legacy fetch failed, current Bun Unable to connect, unrelated errors), which is exactly what makes the next Bun message reword fail loudly instead of silently degrading.

Observations (non-blocking)

  1. Theoretical false-positive surface. includes("Unable to connect") is generic English. If a future backend dependency (e.g. a Linear/GitHub SDK call from a server endpoint) ever produces an error with that phrase, formatServerError will mistranslate it as "Could not connect to webmux server on port N". No such producer exists in the tree today, so this is future-risk, not a current bug. Tightening to includes("Unable to connect. Is the computer") matches Bun's exact phrasing without being meaningfully more fragile.

  2. More robust signal exists. Bun's connection-refused error carries a structured code: "ConnectionRefused" (and error.cause?.code). Checking the structured code in addition to (or instead of) the message string would survive future wording changes. The prior in-PR review already flagged this; agreed it's a reasonable follow-up, not a blocker.

  3. withServerConnection not directly tested. Optional. It's a thin try/catch that rethrows new Error(formatServerError(...)) — note that this loses stack, cause, and any structured properties from the original error. That's pre-existing behavior, not introduced here.

Nothing actionable required.
· Branch: claude/pr-263-20260601-1558

@rubenfiszel rubenfiszel changed the title fix: recognize Bun's connection-refused message in formatServerError fix: connect server-backed CLI commands to the live webmux instance Jun 1, 2026
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