From c0b2abc90b6f47087b5c09bb4123c7f838165b67 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 31 Jul 2026 01:03:56 +0200 Subject: [PATCH] fix(contracts): decode growing config unions forward-compatibly Nightly 0.0.32 added two keybinding commands and 0.0.31 clients rejected the whole serverGetConfig payload during connect, so they never reached the connected state. The root cause is that clients validate server-owned, growing literal unions (keybinding commands, config issue kinds, editor ids) as closed sets, and one unknown member fails the entire config decode and takes down the connection. Introduce ForwardCompatibleArray: a wire codec that drops array elements the current build cannot decode instead of failing the payload, and use it for resolved keybindings, server config issues, and available editors. Encoding is unchanged, authoring/upsert inputs stay strict, and all client dispatch is by string equality so dropped rules are inert. Servers can now grow these sets without per-client-version projections on the wire (the approach previously attempted in #5018). Co-Authored-By: Claude Fable 5 --- .../client-runtime/src/rpc/session.test.ts | 34 ++++++++++ packages/contracts/src/baseSchemas.ts | 25 +++++++ packages/contracts/src/keybindings.test.ts | 65 +++++++++++++++++++ packages/contracts/src/keybindings.ts | 11 +++- packages/contracts/src/server.test.ts | 26 +++++++- packages/contracts/src/server.ts | 9 ++- 6 files changed, 165 insertions(+), 5 deletions(-) diff --git a/packages/client-runtime/src/rpc/session.test.ts b/packages/client-runtime/src/rpc/session.test.ts index 71649ad94dd..751453f82f6 100644 --- a/packages/client-runtime/src/rpc/session.test.ts +++ b/packages/client-runtime/src/rpc/session.test.ts @@ -287,6 +287,40 @@ describe("RpcSessionFactory", () => { }), ); + it.effect("reaches ready when a newer server sends unknown config members", () => + Effect.gen(function* () { + const { factory, sockets } = yield* makeFactory(); + const session = yield* factory.connect(PREPARED); + const readyFiber = yield* Effect.forkChild(session.ready); + const socket = yield* awaitSocket(sockets); + socket.open(); + + const shortcut = { + key: "p", + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + modKey: true, + }; + yield* completeInitialConfig(socket, { + ...ENCODED_SERVER_CONFIG, + keybindings: [ + { command: "someFuture.toggle", shortcut }, + { command: "terminal.toggle", shortcut }, + ], + issues: [{ kind: "keybindings.future-issue", message: "From a newer server" }], + availableEditors: ["some-future-editor", "zed"], + }); + yield* Fiber.join(readyFiber); + + const config = yield* session.initialConfig; + expect(config.keybindings).toEqual([{ command: "terminal.toggle", shortcut }]); + expect(config.issues).toEqual([]); + expect(config.availableEditors).toEqual(["zed"]); + }), + ); + it.effect("uses the legacy config RPC for probes when the server lacks the capability", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/contracts/src/baseSchemas.ts b/packages/contracts/src/baseSchemas.ts index a8fa565cef4..9a63f22c9ef 100644 --- a/packages/contracts/src/baseSchemas.ts +++ b/packages/contracts/src/baseSchemas.ts @@ -1,4 +1,5 @@ import * as Effect from "effect/Effect"; +import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; import * as SchemaTransformation from "effect/SchemaTransformation"; @@ -20,6 +21,30 @@ export const PortSchema = Schema.Int.check(Schema.isBetween({ minimum: 1, maximu export const IsoDateTime = Schema.String; export type IsoDateTime = typeof IsoDateTime.Type; +/** + * Wire codec for server→client arrays whose element unions grow over time + * (new literal members, new struct variants). Decoding drops elements the + * current build cannot decode instead of failing the whole payload — a client + * has to keep decoding configs sent by servers newer than itself, and + * rejecting the payload would take down the connection over data the client + * couldn't act on anyway. Encoding is the plain array encoding. + */ +export const ForwardCompatibleArray = (element: Element) => { + const decodeElement = Schema.decodeUnknownOption(element as never); + return Schema.Array(Schema.Unknown).pipe( + Schema.decodeTo( + Schema.Array(element), + SchemaTransformation.transform, ReadonlyArray>({ + decode: (values) => + values.filter((value) => Option.isSome(decodeElement(value))) as ReadonlyArray< + Element["Encoded"] + >, + encode: (values) => values, + }), + ), + ); +}; + /** * Construct a branded identifier. Enforces non-empty trimmed strings */ diff --git a/packages/contracts/src/keybindings.test.ts b/packages/contracts/src/keybindings.test.ts index 4e8ea953770..ec8c839be95 100644 --- a/packages/contracts/src/keybindings.test.ts +++ b/packages/contracts/src/keybindings.test.ts @@ -20,6 +20,7 @@ const decode = ( >; const decodeResolvedRule = Schema.decodeUnknownEffect(ResolvedKeybindingRule as never); +const encodeResolvedKeybindings = Schema.encodeEffect(ResolvedKeybindingsConfig); it.effect("parses keybinding rules", () => Effect.gen(function* () { @@ -185,6 +186,70 @@ it.effect("parses resolved keybindings arrays", () => }), ); +const shortcut = { + key: "p", + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + modKey: true, +}; + +it.effect("drops resolved rules with commands this build does not know", () => + Effect.gen(function* () { + const parsed = yield* decode(ResolvedKeybindingsConfig, [ + { command: "terminal.toggle", shortcut }, + { command: "someFuture.toggle", shortcut }, + { command: "filePicker.toggle", shortcut }, + ]); + assert.deepEqual( + parsed.map((rule) => rule.command), + ["terminal.toggle", "filePicker.toggle"], + ); + }), +); + +it.effect("drops resolved rules with unknown when-node types", () => + Effect.gen(function* () { + const parsed = yield* decode(ResolvedKeybindingsConfig, [ + { + command: "terminal.toggle", + shortcut, + whenAst: { type: "xor", left: 1, right: 2 }, + }, + { command: "terminal.split", shortcut }, + ]); + assert.deepEqual( + parsed.map((rule) => rule.command), + ["terminal.split"], + ); + }), +); + +it.effect("drops malformed resolved rule entries", () => + Effect.gen(function* () { + const parsed = yield* decode(ResolvedKeybindingsConfig, [ + "garbage", + { command: "terminal.toggle", shortcut }, + null, + ]); + assert.deepEqual( + parsed.map((rule) => rule.command), + ["terminal.toggle"], + ); + }), +); + +it.effect("encodes resolved keybindings to the plain wire shape", () => + Effect.gen(function* () { + const rules = [{ command: "terminal.toggle" as const, shortcut }]; + const encoded = yield* encodeResolvedKeybindings(rules); + assert.deepEqual(encoded, rules); + const roundTripped = yield* decode(ResolvedKeybindingsConfig, encoded); + assert.deepEqual(roundTripped, rules); + }), +); + it.effect("drops unknown fields in resolved keybinding rules", () => decodeResolvedRule({ command: "terminal.toggle", diff --git a/packages/contracts/src/keybindings.ts b/packages/contracts/src/keybindings.ts index 14a2a9a4b30..88867bd8a9c 100644 --- a/packages/contracts/src/keybindings.ts +++ b/packages/contracts/src/keybindings.ts @@ -1,5 +1,5 @@ import * as Schema from "effect/Schema"; -import { TrimmedString } from "./baseSchemas.ts"; +import { ForwardCompatibleArray, TrimmedString } from "./baseSchemas.ts"; export const MAX_KEYBINDING_VALUE_LENGTH = 64; export const MAX_KEYBINDING_WHEN_LENGTH = 256; @@ -155,7 +155,14 @@ export const ResolvedKeybindingRule = Schema.Struct({ }).annotate({ parseOptions: { onExcessProperty: "ignore" } }); export type ResolvedKeybindingRule = typeof ResolvedKeybindingRule.Type; -export const ResolvedKeybindingsConfig = Schema.Array(ResolvedKeybindingRule).check( +/** + * The command set grows over time, so a client may receive rules it cannot + * represent (a command or `when` node added after that client shipped). + * Decoding drops those rules instead of failing the whole payload — + * rejecting the config would take down the connection over a shortcut the + * client couldn't dispatch anyway. + */ +export const ResolvedKeybindingsConfig = ForwardCompatibleArray(ResolvedKeybindingRule).check( Schema.isMaxLength(MAX_KEYBINDINGS_COUNT), ); export type ResolvedKeybindingsConfig = typeof ResolvedKeybindingsConfig.Type; diff --git a/packages/contracts/src/server.test.ts b/packages/contracts/src/server.test.ts index c906f86f4dc..078e9fcbf33 100644 --- a/packages/contracts/src/server.test.ts +++ b/packages/contracts/src/server.test.ts @@ -1,9 +1,11 @@ import * as Schema from "effect/Schema"; import { describe, expect, it } from "vite-plus/test"; -import { ServerProvider } from "./server.ts"; +import { ServerConfig, ServerProvider, ServerUpsertKeybindingResult } from "./server.ts"; const decodeServerProvider = Schema.decodeUnknownSync(ServerProvider); +const decodeUpsertKeybindingResult = Schema.decodeUnknownSync(ServerUpsertKeybindingResult); +const decodeAvailableEditors = Schema.decodeUnknownSync(ServerConfig.fields.availableEditors); describe("ServerProvider", () => { it("defaults capability arrays when decoding provider snapshots", () => { @@ -72,3 +74,25 @@ describe("ServerProvider", () => { expect(parsed.continuation?.groupKey).toBe("codex:home:/Users/julius/.codex"); }); }); + +describe("server config forward compatibility", () => { + it("drops config issues with kinds this build does not know", () => { + const parsed = decodeUpsertKeybindingResult({ + keybindings: [], + issues: [ + { kind: "keybindings.invalid-entry", message: "Bad entry", index: 2 }, + { kind: "keybindings.future-issue", message: "From a newer server" }, + ], + }); + + expect(parsed.issues).toEqual([ + { kind: "keybindings.invalid-entry", message: "Bad entry", index: 2 }, + ]); + }); + + it("drops editor ids this build does not know", () => { + const parsed = decodeAvailableEditors(["zed", "some-future-editor", "vscode"]); + + expect(parsed).toEqual(["zed", "vscode"]); + }); +}); diff --git a/packages/contracts/src/server.ts b/packages/contracts/src/server.ts index e083523bbdf..334116794f8 100644 --- a/packages/contracts/src/server.ts +++ b/packages/contracts/src/server.ts @@ -3,6 +3,7 @@ import * as Schema from "effect/Schema"; import { ExecutionEnvironmentDescriptor, ServerSelfUpdateMethod } from "./environment.ts"; import { ServerAuthDescriptor } from "./auth.ts"; import { + ForwardCompatibleArray, IsoDateTime, NonNegativeInt, PositiveInt, @@ -38,7 +39,9 @@ export const ServerConfigIssue = Schema.Union([ ]); export type ServerConfigIssue = typeof ServerConfigIssue.Type; -const ServerConfigIssues = Schema.Array(ServerConfigIssue); +// Issue kinds grow over time; older clients must not fail the whole config +// decode over a kind they cannot render. +const ServerConfigIssues = ForwardCompatibleArray(ServerConfigIssue); export const ServerProviderState = Schema.Literals(["ready", "warning", "error", "disabled"]); export type ServerProviderState = typeof ServerProviderState.Type; @@ -417,7 +420,9 @@ export const ServerConfig = Schema.Struct({ keybindings: ResolvedKeybindingsConfig, issues: ServerConfigIssues, providers: ServerProviders, - availableEditors: Schema.Array(EditorId), + // Editor ids grow over time; drop ones this build does not know rather than + // failing the whole config decode. + availableEditors: ForwardCompatibleArray(EditorId), observability: ServerObservability, settings: ServerSettings, /** Whether shell subscriptions can emit an opt-in catch-up completion marker. */