fix(terminal): report a failed reconnect instead of silently repainting #73

Merged

lz merged 1 commit from fix/reconnect-feedback into main

2026-07-16 22:16:53 +02:00

lz commented

2026-07-16 21:36:27 +02:00

Owner

Targets fix/reconnect (#71), not main — it builds on that fix. Merge #71 first and this retargets to main cleanly.

Follow-up to the caveat raised in #71.

The bug

A reconnect the server rejects (workspace stopped → 409, vault locked → 401) left the overlay reading exactly what it read before the click: disconnected. Before and after were byte-identical, so a rejected attempt was indistinguishable from a click that never landed — the button read as broken twice over, first because it genuinely was (#71), then because nothing it did was observable.

The fix

The overlay now carries the attempt's outcome:

state	overlay
link dropped on its own	`disconnected`
attempt in flight	`reconnecting…` (button disabled)
attempt rejected	`reconnect failed` + `the workspace may not be running`

Why the hint is hedged rather than naming the cause. The server rejects with a real status and body (upgrade.ts reject() — 401 locked, 409 container <state>), but a browser exposes neither to JS: a rejected handshake surfaces as CloseEvent code 1006 with an empty reason. Verified in-browser rather than assumed:

{ event: 'close', code: 1006, reason: '', reasonLength: 0, wasClean: false }

So naming a cause would be a guess. Reporting the real reason requires a separate probe of worker state — deliberately out of scope; happy to do it as a follow-up if you want the overlay to say "workspace is stopped" outright.

Two decisions worth reviewing

One 4-state link replacing the disconnected boolean, not two more booleans. connecting and failed are mutually exclusive; three booleans would encode 8 states of which half are nonsense. It starts 'up', so a fresh pane still shows no overlay while its first socket opens — unchanged behaviour.

The transition is derived from each socket's own opened flag, not from the current link. A rejected handshake fires both onerror and onclose. A state-derived transition (link === 'connecting' ? 'failed' : 'down') clobbers 'failed' back to 'down' on the second event and reprints "disconnected" — reintroducing the exact bug. The opened flag makes it order-independent and idempotent. The recorded transition below is what proves it.

The disabled style is scoped to the overlay. app.css styles only .btn.primary:disabled; the Reconnect button is .btn sm, so disabling it without a style would leave it looking clickable but inert — the very failure this overlay exists to report. Adding .btn:disabled globally would silently restyle every other plain disabled button (e.g. Workers.svelte:182), so the rule stays scoped.

Verification

The suite is environment: 'node' with no DOM, so this is browser-verified. Transitions recorded with a MutationObserver against a stopped workspace:

disconnected | btn:enabled                                              <- passive drop
reconnecting… | btn:disabled                                            <- click registers
reconnect failed | the workspace may not be running | btn:enabled       <- terminal, persists

A passive drop still reads plain disconnected (no regression).
A fresh pane shows no overlay while connecting (no regression).
Repeat attempts re-cycle reconnecting… → reconnect failed.
The scoped disabled rule actually resolves: opacity: 0.4, cursor: not-allowed.
725 tests pass, lint clean, typecheck 0 errors.

Noted, not touched

app.css has no .btn:disabled rule at all — only .btn.primary:disabled. So any plain .btn elsewhere in the app that gets disabled is inert-but-looks-enabled. Out of scope here; worth a look separately.

**Targets `fix/reconnect` (#71), not `main`** — it builds on that fix. Merge #71 first and this retargets to `main` cleanly. Follow-up to the caveat raised in #71. ## The bug A reconnect the server rejects (workspace stopped → 409, vault locked → 401) left the overlay reading exactly what it read *before* the click: `disconnected`. Before and after were byte-identical, so a rejected attempt was indistinguishable from a click that never landed — the button read as broken twice over, first because it genuinely was (#71), then because nothing it did was observable. ## The fix The overlay now carries the attempt's outcome: | state | overlay | |---|---| | link dropped on its own | `disconnected` | | attempt in flight | `reconnecting…` (button disabled) | | attempt rejected | `reconnect failed` + `the workspace may not be running` | **Why the hint is hedged rather than naming the cause.** The server rejects with a real status and body (`upgrade.ts` `reject()` — `401 locked`, `409 container <state>`), but a browser exposes **neither** to JS: a rejected handshake surfaces as `CloseEvent` code `1006` with an empty reason. Verified in-browser rather than assumed: ``` { event: 'close', code: 1006, reason: '', reasonLength: 0, wasClean: false } ``` So naming a cause would be a guess. Reporting the *real* reason requires a separate probe of worker state — deliberately out of scope; happy to do it as a follow-up if you want the overlay to say "workspace is stopped" outright. ## Two decisions worth reviewing **One 4-state `link` replacing the `disconnected` boolean, not two more booleans.** `connecting` and `failed` are mutually exclusive; three booleans would encode 8 states of which half are nonsense. It starts `'up'`, so a fresh pane still shows no overlay while its first socket opens — unchanged behaviour. **The transition is derived from each socket's own `opened` flag, not from the current `link`.** A rejected handshake fires **both** `onerror` and `onclose`. A state-derived transition (`link === 'connecting' ? 'failed' : 'down'`) clobbers `'failed'` back to `'down'` on the second event and reprints "disconnected" — reintroducing the exact bug. The `opened` flag makes it order-independent and idempotent. The recorded transition below is what proves it. **The disabled style is scoped to the overlay.** `app.css` styles only `.btn.primary:disabled`; the Reconnect button is `.btn sm`, so disabling it without a style would leave it looking clickable but inert — the very failure this overlay exists to report. Adding `.btn:disabled` globally would silently restyle every other plain disabled button (e.g. `Workers.svelte:182`), so the rule stays scoped. ## Verification The suite is `environment: 'node'` with no DOM, so this is browser-verified. Transitions recorded with a `MutationObserver` against a stopped workspace: ``` disconnected | btn:enabled <- passive drop reconnecting… | btn:disabled <- click registers reconnect failed | the workspace may not be running | btn:enabled <- terminal, persists ``` - A passive drop still reads plain `disconnected` (no regression). - A fresh pane shows **no** overlay while connecting (no regression). - Repeat attempts re-cycle `reconnecting… → reconnect failed`. - The scoped disabled rule actually resolves: `opacity: 0.4`, `cursor: not-allowed`. - 725 tests pass, lint clean, typecheck 0 errors. ## Noted, not touched `app.css` has no `.btn:disabled` rule at all — only `.btn.primary:disabled`. So any plain `.btn` elsewhere in the app that gets disabled is inert-but-looks-enabled. Out of scope here; worth a look separately.

lz added 1 commit

2026-07-16 21:36:28 +02:00

fix(terminal): report a failed reconnect instead of silently repainting

ci / nexus (pull_request) Successful in 6m30s

Details

ci / images (pull_request) Successful in 8m0s

Details

pr-image-cleanup / delete-pr-images (pull_request) Successful in 11s

Details

3951d7a81e

A reconnect that the server rejects (workspace stopped -> 409, vault locked
-> 401) left the overlay reading exactly what it read before the click:
"disconnected". Both the before and after states were identical, so a
rejected attempt was indistinguishable from a click that never landed — the
button read as broken twice over.

The overlay now carries the attempt's outcome:

  disconnected                                   (link dropped on its own)
  reconnecting...                                (attempt in flight, btn disabled)
  reconnect failed / the workspace may not be running

Why the hint is hedged rather than naming the cause: the server rejects with
a real status and body (upgrade.ts reject()), but a browser exposes NEITHER
to JS — a rejected handshake surfaces as CloseEvent code 1006 with an empty
reason (verified in-browser, not assumed). Naming a cause would be a guess.
Reporting the real reason needs a separate probe of worker state; deliberately
out of scope here.

Modelled as one 4-state `link` replacing the `disconnected` boolean rather
than adding two more booleans: 'connecting' and 'failed' are mutually
exclusive, and three booleans would encode 8 states of which half are
nonsense. It starts 'up' so a fresh pane still shows no overlay while its
first socket opens — unchanged behaviour.

The transition is deliberately derived from each socket's own `opened` flag,
not from the current `link`: a rejected handshake fires BOTH onerror and
onclose, so a state-derived transition clobbers 'failed' back to 'down' on
the second event and reprints "disconnected" — the exact bug being fixed.

The disabled style is scoped to the overlay because app.css styles only
`.btn.primary:disabled`; a disabled `.btn sm` would otherwise look identical
to an enabled one. Adding `.btn:disabled` globally would restyle every other
plain disabled button in the app.

Verified in a real browser (the suite is node-only, no DOM). Recorded
transitions via MutationObserver, against a stopped workspace:
  disconnected | btn:enabled
  reconnecting... | btn:disabled
  reconnect failed | the workspace may not be running | btn:enabled
A passive drop still reads plain "disconnected"; repeat attempts re-cycle;
the scoped disabled rule resolves (opacity 0.4, cursor not-allowed).
725 tests pass, lint clean, typecheck 0 errors.

lz changed target branch from fix/reconnect to main

2026-07-16 22:15:25 +02:00

lz merged commit 0525962eaa into main

2026-07-16 22:16:53 +02:00

lz deleted branch fix/reconnect-feedback

2026-07-16 22:16:53 +02:00

lz referenced this pull request from a commit

2026-07-16 22:16:56 +02:00

Merge pull request 'fix(terminal): report a failed reconnect instead of silently repainting' (#73) from fix/reconnect-feedback into main

lz referenced this pull request from a commit

2026-07-17 17:18:42 +02:00

fix(ui): base disabled affordance for plain .btn