Skip to content

fix(server): keep older clients connected after keybinding additions - #5018

Closed
PixPMusic wants to merge 2 commits into
pingdotgg:mainfrom
PixPMusic:fix/keybinding-wire-compat
Closed

fix(server): keep older clients connected after keybinding additions#5018
PixPMusic wants to merge 2 commits into
pingdotgg:mainfrom
PixPMusic:fix/keybinding-wire-compat

Conversation

@PixPMusic

@PixPMusic PixPMusic commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What Changed

  • Negotiate the supported keybinding command set once per WebSocket connection.
  • Keep filePicker.toggle and projectSearch.toggle out of config and mutation responses for clients that predate the negotiation.
  • Preserve the full keybinding list for current web, desktop, and mobile clients.
  • Cover initial config, streamed config updates, and keybinding upsert/remove responses.

Why

Nightly 0.0.32-nightly.20260730.957 began returning two new default keybinding commands. The 0.0.31 client validates those commands against a closed schema while establishing its connection, so it rejects the initial server config and never reaches the connected state.

Current clients now advertise support through the WebSocket URL. Older clients omit the capability and receive a legacy-safe wire projection without changing the persisted or in-memory keybinding configuration. Older servers ignore the additional query parameter.

Verification

  • vp test run packages/client-runtime/src/rpc/session.test.ts apps/server/src/server.test.ts — 121 tests passed
  • vp run --filter @t3tools/contracts --filter @t3tools/client-runtime --filter t3 typecheck
  • targeted lint and formatting checks for all changed files
  • git diff --check origin/main...HEAD

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I added regression coverage

Model: GPT-5.6 Sol | Harness: Codex in T3 Code


Note

Medium Risk
Wire compatibility logic on the hot WebSocket path; mistakes could hide keybindings from current clients or still break legacy connects, but scope is limited to response projection with strong test coverage.

Overview
Adds WebSocket keybinding command-set negotiation so nightly servers can expose filePicker.toggle and projectSearch.toggle without breaking 0.0.31 clients that validate keybindings against a smaller schema during connect.

Contracts define WS_KEYBINDING_COMMAND_SET_QUERY_PARAM and CURRENT_KEYBINDING_COMMAND_SET_VERSION. Client runtime appends that query param when opening the WebSocket. Server reads it on /ws upgrade and, for connections without a current version, filters those two commands from keybinding payloads on serverGetConfig, upsert/remove responses, and subscribeServerConfig streams—without changing persisted config.

Regression tests cover unnegotiated, legacy (0), current, and future client versions across config, streams, and mutations.

Reviewed by Cursor Bugbot for commit 8785f3d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Filter keybinding commands for older clients after new keybinding additions

  • Adds WS_KEYBINDING_COMMAND_SET_QUERY_PARAM and CURRENT_KEYBINDING_COMMAND_SET_VERSION (1) to rpc.ts as a versioning mechanism for keybinding command sets.
  • Clients advertise support by appending keybindingCommandSet=<version> to the WebSocket URL in session.ts.
  • The server in ws.ts inspects this query param and, for clients not advertising the current version, strips filePicker.toggle and projectSearch.toggle from keybindings in serverGetConfig, serverUpsertKeybinding, serverRemoveKeybinding, and subscribeServerConfig responses.
  • Behavioral Change: older or unversioned clients now receive a filtered keybinding set that omits the two new commands.

Macroscope summarized 8785f3d.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 54035889-0c45-499e-b0a8-f7d09bcb58c8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jul 30, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jul 30, 2026
@macroscopeapp

macroscopeapp Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This backwards-compatibility fix modifies what keybindings are sent to websocket clients based on version negotiation. While the change is well-scoped and thoroughly tested, it touches core server-client communication in ws.ts and the author is not a frequent contributor to this file, warranting human verification.

You can customize Macroscope's approvability policy. Learn more.

@macroscopeapp
macroscopeapp Bot dismissed their stale review July 30, 2026 18:29

Dismissing prior approval to re-evaluate 8785f3d

@NikAtNight

Copy link
Copy Markdown

Confirmed this exact regression against production T3 Connect. With Nightly .958, both the iOS app (T3Code/18) and app.t3.codes successfully authenticated and upgraded the WebSocket, then disconnected immediately after server.getConfig. Rolling the desktop environment back to .953 restored connectivity for both clients. This matches the closed-schema/keybinding compatibility failure described here.

@juliusmarminge

Copy link
Copy Markdown
Member

this feels like the wrong solution to the problem

@PixPMusic

Copy link
Copy Markdown
Contributor Author

@juliusmarminge main concern was keeping the nightly server compatible with the released mobile client, as App Store review time can take a while.

If you don't care about nightlies being compatible--or want to start doing CI nightlies for Android at least?--then we can move in another direction.

@juliusmarminge

Copy link
Copy Markdown
Member

I do care about nightlies being compatible, but we've already submitted a new version for review and I'm not sure if we can do a new OTA right now ( I think build versions are in a rough state...) so we should just fix it properly

@juliusmarminge

Copy link
Copy Markdown
Member

#5055

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

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants