Skip to content

fix(contracts): keep Space shortcuts after saving keybindings - #32008

Closed
voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:t3/fix-keybinding-settings-save
Closed

voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:t3/fix-keybinding-settings-save

Conversation

@voltcrash

Copy link
Copy Markdown
Contributor

Problem

Saving a shortcut containing Space, such as Super+Space for dictation, writes the correct file but leaves Settings and the active shortcut using the default. The resolved-keybinding schema trims the literal Space key to an empty string during encoding; clients silently drop the invalid rule and restore the default.

Closes #32007

Change

Preserve resolved key strings without trimming, while keeping the nonempty and maximum-length checks. Shortcut configuration text still uses the existing trimmed schema. Add one regression test for encoding and decoding the resolved Super+Space dictation rule.

Scope and approval

This qualifies for the very small, focused obvious-bug exception in CONTRIBUTING.md: it fixes an existing shortcut's serialization without adding a feature, changing defaults, or changing UI code. The shared contract covers server/client messages, cached configuration, and forwarded preview shortcuts. No provider-specific changes are needed.

Verification

  • Added the regression test before the fix; it failed because shortcut.key encoded as "" instead of " ". It passes with the fix.
  • vp test run packages/contracts/src/keybindings.test.ts apps/server/src/keybindings.test.ts apps/web/src/components/settings/KeybindingsSettings.logic.test.ts: 39 tests passed.
  • vp run check:changed: format, lint, both knip checks, affected typechecks, and adjacent tests passed.
  • Isolated development client, seeded from a read-only snapshot: recorded Meta+Space in Settings and clicked Save. The file stores meta+space; after a full page reload Settings displays Meta+Space. Reading the live client bindings and matching both chords returned:
Meta+Space    → composer.dictation
Ctrl+Shift+D  → null

The verification covers persistence, serialization, display, and shortcut resolution. Actual microphone capture/transcription and native mobile were not exercised; this is a shared-contract fix with no UI layout changes.

Reviewed with review-loop: 1 round with GPT-6.1 Sol on xhigh. Reviewer verdict: approved.

Created with GPT-6.1 Sol in the Codex harness.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Nov 6, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 1e82c17

Macroscope's review found this PR approvable — Resolved Space shortcuts now survive serialization after saving keybindings, while existing length limits and configuration-text validation remain intact. This is a bounded compatibility fix with no product-default changes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The resolved keybinding schema now preserves literal Space characters. A regression test checks that a Space shortcut with metaKey: true and modKey: false is unchanged after encoding and decoding.

Changes: Space keybinding round-trip

Layer / File(s) Summary
Resolved shortcut schema and round-trip test
packages/contracts/src/keybindings.ts, packages/contracts/src/keybindings.test.ts
KeybindingShortcut.key accepts non-empty strings up to 64 characters without trimming. The test checks that encoding and decoding preserve a resolved Space shortcut.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

  • pingdotgg/t3code#30645: Both PRs remove trimming from KeybindingShortcut.key while retaining the non-empty and maximum-length checks.

Suggested labels: size:S

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to 1e82c

The change is intended to preserve Space shortcuts after saving and reloading. No merge-blocking risk was identified.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly describes the Space shortcut fix.
Description check Passed The description covers the problem, change, scope, verification results, and unchecked behavior. It also identifies the model and harness.
Linked Issues check Passed [#32007] KeybindingShortcut.key preserves the literal Space while retaining nonempty and length checks. The regression test verifies encoding and decoding a Meta+Space dictation rule. The author rep…
Out of Scope Changes check Passed [#32007] The shared schema change and its regression test address Space shortcut serialization. The change does not alter shortcut configuration text or unrelated code.
Full details: Linked Issues check

Explanation

[#32007] KeybindingShortcut.key preserves the literal Space while retaining nonempty and length checks. The regression test verifies encoding and decoding a Meta+Space dictation rule. The author reports that the saved shortcut persists, displays, and resolves after reload, replacing the default.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 6577c6b and 1e82c17.

Files selected for processing: 2

  • packages/contracts/src/keybindings.test.ts
  • packages/contracts/src/keybindings.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed labels Nov 6, 2026
@voltcrash voltcrash closed this Nov 7, 2026
@juliusmarminge juliusmarminge removed the pr:gate-failed A land gate failed label Nov 7, 2026
@voltcrash
voltcrash deleted the t3/fix-keybinding-settings-save branch November 10, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop size:XS 0-9 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.

[Bug]: Space keybindings save to disk but revert to defaults in clients

2 participants