[tanav29]

Problem

Fixes #32007. A saved shortcut such as meta+space resolves to the literal Space key (" "), but KeybindingShortcut trims it to "" during serialization. Clients drop the invalid rule and restore the default shortcut.

Change

Preserve resolved key values with a non-trimming, non-empty string schema, retaining the existing 64-character limit. Add a regression test through the resolved-keybindings JSON codec and extend the malformed-rule case with empty and oversized keys.

Scope and approval

This qualifies as a very small, focused fix for an obvious bug under CONTRIBUTING.md: the shared wire schema corrupts an already-supported keyboard key. The fix applies to server config delivery and desktop shortcut IPC. Textual shortcut parsing, provider behavior, and settings defaults are unchanged.

Verification

Windows x64, based on upstream main at cec8d73a1f0f.

• Added the regression before the fix: it failed with expected key: " ", received key: "". After the fix, all 3 contract tests pass: vp test run src/keybindings.test.ts --config ../../vite.config.ts from packages/contracts.
• Existing server keybinding tests: 21 passed (vp test run src/keybindings.test.ts from apps/server). Existing web keybinding tests: 44 passed (same command from apps/web).
• Runtime check compiled a meta+space rule, encoded and decoded it across JSON, merged client defaults, and matched a Space keyboard event. It reported:

{
  "input": "meta+space",
  "encodedKey": " ",
  "receivedRuleCount": 1,
  "effectiveDictationRuleCount": 1,
  "matchesSpace": true,
  "defaultRestored": false
}

• Changed-file formatting, lint, git diff --check, and the contracts typecheck passed. Both Knip passes (vp run knip:check:files and vp run knip:check:exports) passed. Direct typechecks for all 37 packages selected by the changed-file runner passed.
• vp run check:changed cec8d73a1 --concurrency 2 was attempted but interrupted during its ignored-file cache scan: Git follows Windows dependency junctions recursively and produces Filename too long warnings. Its checks were run directly instead, including vp test run src/provider/AgentContextBudget.test.ts from apps/server (2 passed).

Interactive client verification was not performed.

Implemented with GPT-6.1 Sol through the Codex harness in T3 Code.

[macroscopeapp]

Approvability

Verdict: Approved at dad51fd

Macroscope's review found this PR approvable — The schema now preserves literal Space keys in serialized shortcuts while retaining nonempty and maximum-length validation. The change leaves shortcut assignments and product defaults unchanged and presents limited risk.

Notes:

• All code in this push has already been reviewed. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The resolved keybinding schema now preserves literal Space keys while enforcing non-empty values and a maximum length. Tests cover Space round-tripping and filtering of empty and oversized keys.

Changes: Space keybinding serialization

Layer / File(s)	Summary
Shortcut schema and regression tests packages/contracts/src/keybindings.ts, packages/contracts/src/keybindings.test.ts	KeybindingShortcut.key now uses a non-empty string schema with a maximum length and no trimming. Tests cover Space round-tripping and malformed keys.

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

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

• pingdotgg/t3code#30645: Changes the shortcut key schema to accept non-empty, untrimmed values and tests Space round-tripping.
• pingdotgg/t3code#32008: Removes trimming from the shortcut key schema and tests Space round-tripping.

Suggested labels: bugfix, size:S

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to dad51

Space shortcuts are preserved during serialization, with no material merge risk identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the fix: preserving Space keybindings during serialization.
Description check		The description covers the problem, change, scope and approval rationale, and focused verification results. It also states that interactive client verification was not performed.
Linked Issues check		[#32007] KeybindingShortcut.key now accepts non-empty strings without trimming and retains the 64-character limit. The JSON codec regression test verifies that a resolved Space key survives encoding…
Out of Scope Changes check		The changes are limited to the resolved-keybinding schema and its contract tests. Both changes directly support [#32007].

Full details: Linked Issues check

Explanation

[#32007] KeybindingShortcut.key now accepts non-empty strings without trimming and retains the 64-character limit. The JSON codec regression test verifies that a resolved Space key survives encoding and decoding. The malformed-rule test verifies that empty and 65-character keys are dropped. The reported runtime check also confirms that the Space rule remains effective and matches a Space event.

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

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 633438d
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: f3c694c4-ad62-4add-8212-b584766e2036

Commits

Reviewing files that changed from the base of the PR and between 633438d and dad51fd.

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]

Thanks, @tanav29, for the fix and regression coverage. #30645 by @Jo2234 already addresses the same Space-key serialization defect with the same schema change and regression coverage, so I’m closing this as a duplicate of that earlier, smaller PR.