Skip to content

Faster Cloudflare share QR + tighter QR + keybar keyboard toggle#293

Merged
kshivang merged 4 commits into
masterfrom
feat/share-faster-qr-and-keybar
Jun 9, 2026
Merged

Faster Cloudflare share QR + tighter QR + keybar keyboard toggle#293
kshivang merged 4 commits into
masterfrom
feat/share-faster-qr-and-keybar

Conversation

@kshivang

@kshivang kshivang commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Three session-sharing / mobile-viewer improvements (one commit each).

1. Pre-warm + keep the Cloudflare tunnel warm — instant QR (SessionShareManager.kt)

The public Cloudflare QR appeared ~5-15s after clicking Share, because the tunnel was established and verified-routable on the click path (awaitUrl + awaitReady). This keeps that safety but moves the cost off the critical path:

  • Pre-warm: when session sharing is enabled with a remote provider, bind the engine + establish the tunnel at app start, so the verified public URL is already published when the user clicks Share.
  • Keep-warm: don't tear the engine/tunnel down on the last unshare while sharing stays enabled → re-shares are instant. Disable-sharing / mode→off / quit still tear down.
  • Auto-respawn: if a kept-warm tunnel exits on its own, re-establish after a 2s backoff. The remoteOp token is the authority — every cooperative teardown bumps the op before killing, so our own kills never respawn, and a persistent failure (FellBack, no adopted process) can't hot-loop.
  • Port-collision fix pre-warm exposed: the BossTerm MCP server (loopback, 7676-7685) and the share server (7677+) overlap. Started concurrently at launch under port pressure, both could land on the same port — MCP on 127.0.0.1, share on the 0.0.0.0 wildcard — and cloudflared (dials 127.0.0.1) then hit the MCP server's DNS-rebinding guard (403 "not a loopback target") instead of the viewer. Pre-warm now waits for the MCP server to claim its port first (bounded, only when MCP enabled), and the port loop skips the MCP server's running port.

Accepted trade-off: once sharing has been enabled (default mode cloudflare), each launch spawns a quick tunnel at startup and keeps it for the session. It points at a token-gated server (serves nothing without a ?t= token), and Disable session sharing turns it off.

2. Tighter QR rendering (ShareWindow.kt)

Asking ZXing for a fixed 256px image made it integer-scale + center the code, baking the slack in as a fat white border. Encode at size 1 (tight 1px-per-module matrix, single-module quiet zone) then upscale by an integer factor → the QR fills the box and stays crisp; the 10dp white frame stays as the scan quiet zone.

3. Keybar toggles the soft keyboard (viewer.js)

The on-screen keyboard button only summoned the keyboard. Now it toggles — blurs the focused textarea to dismiss when up, focuses to summon when down (inside the tap gesture so iOS honours it). "Up" = keyboard inset OR textarea focused, reliable across iOS/Android. wireKeyButton gained an optional tap action; the key-row buttons are unchanged.

Testing

Verified on the dev build + iPhone over a Cloudflare relay: tunnel pre-warms at launch (reachable via cloudflare: … before any share); the port-collision guard fires (Session-sharing port 7679 is the BossTerm MCP port, trying next → bound 7680); the QR loads the viewer (no 403); stop→re-share is instant; the button shows and hides the keyboard.

Generated with Claude Code

kshivang added 3 commits June 8, 2026 22:32
…instant

The public Cloudflare QR appeared ~5-15s after clicking Share: the tunnel was
established + verified-routable only on the click path. Move that cost off the
critical path.

- Pre-warm: when session sharing is enabled with a remote provider, bind the
  engine and establish the tunnel at app start (settings observer), so the
  verified public URL is already published when the user clicks Share.
- Keep-warm: don't tear the engine/tunnel down on the last unshare while sharing
  stays enabled, so re-shares are instant. Disable-sharing / mode→off / quit
  still tear down.
- Auto-respawn: if a kept-warm Cloudflare tunnel exits on its own, re-establish
  it after a 2s backoff. The remoteOp token is the authority — every cooperative
  teardown bumps the op before killing, so our own kills never respawn, and a
  persistent failure (FellBack, no adopted process) can't hot-loop.
- Fix a port collision pre-warm exposed: the BossTerm MCP server (loopback,
  7676-7685) and the share server (7677+) overlap. Started concurrently at
  launch under port pressure, both could land on the same port — MCP on
  127.0.0.1, share on the 0.0.0.0 wildcard — and cloudflared (dials 127.0.0.1)
  then hit the MCP server's DNS-rebinding guard (403) instead of the viewer.
  Pre-warm now waits for the MCP server to claim its port first (bounded, only
  when MCP is enabled), and the port loop skips the MCP server's running port.

Generated with [Claude Code](https://claude.com/claude-code)
Asking ZXing for a fixed 256px image made it scale the code by an integer module
multiple and center it, baking the leftover slack in as a fat white border (on
top of the quiet zone, then the 10dp modifier padding) — the QR looked small in
a large white box. Encode at size 1 instead, which yields a tight 1px-per-module
matrix (just the MARGIN=1 single-module quiet zone, no slack), then upscale by an
integer factor so module edges stay crisp. The QR now fills the box; the 10dp
white frame remains as the scan-friendly quiet zone.

Generated with [Claude Code](https://claude.com/claude-code)
The on-screen keyboard button only summoned the soft keyboard (focused the hidden
textarea). Make it a toggle: when the keyboard is up it blurs the textarea to
dismiss it, when down it focuses to summon it — running inside the button's tap
gesture so iOS honours the focus. "Up" is detected as a keyboard inset OR the
textarea already being the focused element, so it's reliable across iOS/Android.
wireKeyButton now takes an optional tap action; the key-row buttons keep their
"always refocus to stay up" behavior unchanged — only the ⌨ button toggles.

Generated with [Claude Code](https://claude.com/claude-code)
@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review — Faster Cloudflare share QR + tighter QR + keybar toggle

Read through all three changes against the surrounding SessionShareManager concurrency model. This is careful, well-documented work — the op-token reasoning in particular holds up. A few observations, none blocking.

✅ What's solid

  • Op-token discipline in registerRespawn. The claim that "only an unattended death respawns" checks out: every cooperative teardown (stopEngineLocked, applyRemoteMode, shutdown) bumps the op before destroyForcibly(), so the captured op is already stale when the resulting onExit fires → self-cancels. The post-delay re-check closes the teardown-during-backoff window too. One handler is registered per successful establishRemote, so no duplicate-handler accumulation. Good.
  • Keep-warm gating. keepWarm() mirrors the pre-warm condition exactly, and unshare only skips teardown when it's true — disable / mode→off / quit all still tear down. Consistent.
  • QR encode-at-1-then-upscale. Correct: ZXing clamps the requested size up to qrWidth (modules + 2*MARGIN) when you pass 1, so n is the tight matrix incl. the 1-module quiet zone, and integer scale keeps module edges sharp. The nested dy/dx fill is indexed correctly (matrix[mx,my]setRGB(mx*scale+dx, my*scale+dy)).
  • Security boundary preserved. Checked serveViewer: the ws is rejected for any unknown/expired token, and after unshare sharesByToken is empty — so a kept-warm tunnel exposes only the anonymous static viewer (no session data) until a real share re-populates the token map. The continuous public endpoint is opt-in (sessionSharingEnabled defaults false) and documented as an accepted trade-off. Fine.

Minor concerns

  1. Stale URL during respawn (UX). When a kept-warm tunnel dies on its own, teardownRemoteAccess isn't called, so remoteUrl / _remoteUrlFlow keep publishing the dead link for the full delay(2000) + re-establish window. Viewers hitting it in that gap get Cloudflare errors. For an idle warm tunnel this is harmless, but registerRespawn could optimistically push RemoteState(Starting/Verifying) (or null the URL) the moment it decides to respawn, the way establishRemote does, so the dialog reflects reconnecting rather than a soon-to-be-dead URL.

  2. No respawn ceiling for a flapping tunnel. A tunnel that goes routable → dies after ~1s repeatedly will respawn every ~2s indefinitely while sharing stays enabled. The 2s backoff prevents a hot-loop (as the comment says), but there's no MAX_REFRESHES-style cap like establishCloudflareVerified has, so a persistently flapping cloudflared quietly respawns forever in the background. Consider a bounded consecutive-failure counter that gives up to FellBack.

  3. toggleKeyboard edge: inset up but no textarea. If softKeyboardUp() is true while activeTextarea() returns null (no current pane / not yet wired), up is true but the if (ta) guard makes it a no-op, leaving the keyboard up with no way to dismiss via the button. Narrow, but worth a fallback (e.g. document.activeElement && document.activeElement.blur()).

  4. MCP-port skip covers only the bound port. if (port == McpTerminalRegistry.runningPort.value) guards the steady state; the genuine startup race (both servers probing before either binds) is still handled only by portBindable's loopback probe + prewarmRemote's first { it != null } wait. The comment is honest about this. Note the withTimeoutOrNull(5000) wait only runs through prewarmRemote — an actual share that reaches ensureEngineLocked directly doesn't wait, though by then MCP is normally bound. Acceptable as-is; just flagging the asymmetry.

Tests

No automated coverage for this module (consistent with the repo). The op-token state machine — especially the new respawn path interacting with teardown — is exactly the kind of concurrency logic that's easy to regress silently. A few unit tests around keepWarm() and op-supersession (even with a fake Process) would be worth their weight here, if the project ever adds test infra for share/.

Nice work overall — the diff is small, the comments carry the reasoning, and the risky parts (op token, leak-free tunnel adoption) are handled correctly.

🤖 Generated with Claude Code

…the fold

After the keyboard is raised, entering a TUI (e.g. Claude Code) drops the prompt
to the bottom of the screen — but the keyboard push was only computed at
open/close, so the input hid behind the keyboard until you toggled it.

Re-add a narrow cursor-follow: followCursor only ever INCREASES the upward push,
and only when the cursor sits below the visible fold. A thinking TUI with a
visible cursor writes nothing to the transform (no churn), so it doesn't
reintroduce the dismiss bug — which came from the renderStage innerHTML="" detach
and the autofit re-render (both fixed separately), not the transform write.
Coalesced to one check per frame.

Generated with [Claude Code](https://claude.com/claude-code)
@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review — Faster Cloudflare share QR + tighter QR + keybar keyboard toggle

Read through all four commits against master. Careful, well-documented work — the op-token concurrency reasoning is genuinely solid and the comments capture the why well. Notes below are mostly minor; nothing blocking.

1. Pre-warm / keep-warm / auto-respawn (SessionShareManager.kt)

The op-token discipline holds up. I traced every cooperative teardown — stopEngineLocked, applyRemoteMode, refreshRemoteLink, shutdown, stopAll — and each bumps the op via claimRemoteOp() BEFORE killing the process, so a destroyForcibly() we initiated leaves the captured op stale and registerRespawns onExit self-cancels. Only an unattended death respawns, and a FellBack attempt registers no onExit so it cannot hot-loop. At most one respawn is armed at a time (registration only happens after establishRemote publishes Active). 👍

The MCP port-collision fix is the right shape. BossTermMcpManager calls registry.setRunning(port) right after engine.start(wait = false) — i.e. before CIO has actually bound the socket — so the portBindable() loopback probe alone can race (it sees the port free in the gap before MCP binds). The explicit if (port == McpTerminalRegistry.runningPort.value) continue guard closes exactly that window, which is why it is not redundant with the probe. The bounded withTimeoutOrNull(5000) gated on mcpEnabled avoids stalling when MCP is off.

Minor — dead parameter: registerRespawn(proc, port, op) never uses port; it re-reads boundPort inside the coroutine (val p = boundPort ?: return@launch), which is the safer choice anyway. Drop the port param and the , port at the call site (line 535) to remove the misleading signal.

Worth flagging (you already acknowledge it) — always-on tunnel at launch: with the default cloudflare mode, every launch now spawns a public Cloudflare tunnel for the whole session even when nothing is shared, and auto-respawn keeps it alive. It is token-gated and Disable session sharing tears it down, so the trade-off is reasonable — but a publicly-routable endpoint now exists for the entire session lifetime rather than only while a tab is actively shared, and if a quick tunnel is flappy the respawn loop keeps re-dialing indefinitely (2s backoff each). The exposure window and background network cost are strictly larger than before. Consider making keep-warm opt-in for users who enable sharing but rarely share.

Minor — no single-flight on pre-warm: the settings observer launches prewarmRemote() on every (enabled, mode) change; rapid off→cloudflare→off→cloudflare would launch several. ensureEngineLocked is idempotent so this is harmless, but unlike prefetchJob there is no guard. Fine to leave.

2. Tighter QR (ShareWindow.kt)

Correct approach — encoding at 1x1 makes QRCodeWriter emit the natural module matrix (with the MARGIN=1 quiet zone), then integer-upscaling keeps module edges crisp. Degrades gracefully: a very long E2E link yields a large n, and (target / n).coerceAtLeast(1) caps the floor at 1px/module rather than dividing by zero. The nested scale-by-scale fill is O(px^2) but at 512px that is trivial and only runs when the dialog opens.

3 & 4. Keybar toggle + cursor-follow (viewer.js)

The followCursor invariant (only ever increases the push, only when the cursor is below the fold) is what keeps a thinking TUI from churning the transform and dropping the keyboard — coalescing to one check per frame via followRaf is a nice touch. Two small observations:

  • followCursor is monotonic-up only, so when a TUI later moves the cursor back up (or exits), the body stays pushed up with empty space below until the keyboard is toggled. You note this; confirming it is the intended trade-off vs. risking a blur on the way down.
  • toggleKeyboard double-tap edge case: up = softKeyboardUp() || (ta && document.activeElement === ta). If the textarea is focused but the keyboard is not visually up (hardware keyboard, or a transient focus-without-inset state), the first tap blurs and a second is needed to summon. Likely rare on the target path, but worth knowing.

Tests

No tests added, and there are none for these files today. The logic is hard to unit-test (process lifecycle, soft-keyboard insets, ZXing rendering), so I would not block on it — but the QR matrix-to-bitmap mapping and the port-skip predicate are pure enough to cover cheaply if you want a regression net (e.g. assert qrImageBitmap returns a bitmap whose dimensions are an integer multiple of the module count, and that the port loop skips the MCP port).

Overall: ship it after trimming the unused registerRespawn param. The teardown-vs-respawn race analysis is the load-bearing part and it checks out.

🤖 Reviewed with Claude Code

@kshivang kshivang merged commit 80096d8 into master Jun 9, 2026
5 checks passed
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