From 1752aa1ea0f52ff2c936c4d5e80628f3d186c333 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 23:55:30 -0700 Subject: [PATCH 1/5] fix(contracts): a context record that cannot be encoded no longer fails the send #16300 made ForwardCompatibleArray send an element it cannot encode as a hole. Context records sit behind an `Array(Unknown)` bound, and the RPC JSON codec rejects `undefined` there, so one record with a blank field failed the whole message send instead of dropping that record. Holes are now dropped before the wire. A decoded array without holes is also required again, so `Schema.is` and `make` reject `[undefined]` as they did before #16300. The mobile git sheet treats a blank deep-link ID as missing, like the other thread screens. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/threads/git/GitOverviewSheet.tsx | 12 +++++++++++ packages/contracts/src/baseSchemas.test.ts | 20 ++++++++++++++++--- packages/contracts/src/baseSchemas.ts | 10 +++++++++- .../contracts/src/composerContext.test.ts | 11 ++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx b/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx index 828c8838e31a..31ad85873d1c 100644 --- a/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx +++ b/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx @@ -55,6 +55,18 @@ type GitOverviewSheetProps = StaticScreenProps<{ }; export function GitOverviewSheet(props: GitOverviewSheetProps) { + const navigation = useNavigation(); + const { environmentId, threadId } = props.route.params; + // A hand-typed deep link can carry a blank ID, which the branded IDs reject. + const isBlankLink = environmentId.trim().length === 0 || threadId.trim().length === 0; + useEffect(() => { + if (isBlankLink) navigation.goBack(); + }, [isBlankLink, navigation]); + if (isBlankLink) return null; + return ; +} + +function GitOverviewSheetContent(props: GitOverviewSheetProps) { const { layout } = useAdaptiveWorkspaceLayout(); const navigation = useNavigation(); const insets = useSafeAreaInsets(); diff --git a/packages/contracts/src/baseSchemas.test.ts b/packages/contracts/src/baseSchemas.test.ts index 3e2c05c35058..f36425131299 100644 --- a/packages/contracts/src/baseSchemas.test.ts +++ b/packages/contracts/src/baseSchemas.test.ts @@ -48,8 +48,9 @@ describe("ForwardCompatibleArray", () => { ]); }); - it("sends an element it cannot encode as a hole instead of failing the array", () => { - const Named = ForwardCompatibleArray(Schema.Struct({ name: TrimmedNonEmptyString })); + const Named = ForwardCompatibleArray(Schema.Struct({ name: TrimmedNonEmptyString })); + + it("drops an element it cannot encode instead of failing the array", () => { const wire = JSON.parse( JSON.stringify( Schema.encodeUnknownSync(Schema.toCodecJson(Named))([ @@ -59,9 +60,22 @@ describe("ForwardCompatibleArray", () => { ]), ), ); - expect(wire).toEqual([{ name: "a" }, null, { name: "b" }]); + expect(wire).toEqual([{ name: "a" }, { name: "b" }]); expect(fromWire(Named)(wire)).toEqual([{ name: "a" }, { name: "b" }]); }); + + it("drops it too when a wrapper reads the encoded array as JSON values", () => { + // How context records are bounded before forward-compatible decoding. + const Wrapped = Schema.Array(Schema.Unknown).pipe(Schema.decodeTo(Named)); + expect( + Schema.encodeUnknownSync(Schema.toCodecJson(Wrapped))([{ name: "a" }, { name: " " }]), + ).toEqual([{ name: "a" }]); + }); + + it("does not accept holes as a decoded value", () => { + expect(Schema.is(Named)([undefined])).toBe(false); + expect(Schema.is(Named)([{ name: "a" }])).toBe(true); + }); }); describe("ForwardCompatibleUnion", () => { diff --git a/packages/contracts/src/baseSchemas.ts b/packages/contracts/src/baseSchemas.ts index adb64a1a4a6f..297cbb5d3eb5 100644 --- a/packages/contracts/src/baseSchemas.ts +++ b/packages/contracts/src/baseSchemas.ts @@ -140,13 +140,21 @@ export const ForwardCompatibleArray = (element: Elem Schema.UndefinedOr(Schema.toType(element)).pipe( Schema.catchEncoding(() => Effect.succeedSome(undefined)), ), + ).check( + // The holes above are an encoding detail: a decoded value has none, so + // `Schema.is` and `make` still reject an array that does. + Schema.makeFilter((values) => values.every((value) => value !== undefined), { + expected: "an array without holes", + }), ), SchemaTransformation.transform< ReadonlyArray, ReadonlyArray >({ decode: (values) => values.filter((value) => value !== undefined), - encode: (values) => values, + // Dropped before the wire, so a wrapper that sees the encoded array as + // JSON values never meets a hole. + encode: (values) => values.filter((value) => value !== undefined), }), ), ) as unknown as ForwardCompatibleArray; diff --git a/packages/contracts/src/composerContext.test.ts b/packages/contracts/src/composerContext.test.ts index 87aa403cd6f7..9ca06684f026 100644 --- a/packages/contracts/src/composerContext.test.ts +++ b/packages/contracts/src/composerContext.test.ts @@ -197,6 +197,17 @@ describe("OrchestrationMessageContext", () => { ).toThrow(); }); + it("sends a message without a record it cannot encode", () => { + const wire = Schema.encodeUnknownSync(Schema.toCodecJson(OrchestrationMessageContext))({ + version: 1, + records: [ + decodeContext({ version: 1, records: [knownRecords.terminal] }).records[0], + { ...knownRecords.terminal, contextId: "ctx_2", terminalLabel: " " }, + ], + }); + expect(decodeContext(wire).records.map((record) => record.contextId)).toEqual(["ctx_1"]); + }); + it("normalizes decoded record identifiers", () => { const context = decodeContext({ version: 1, From 350108e8c64d6c942b26c949c07972bf4815edeb Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 6 Oct 2026 02:11:41 -0700 Subject: [PATCH 2/5] fix(contracts): unknown context payloads must be JSON; holes abort the check An unknown-kind context record only checked that its payload stringified, so a Date, NaN or undefined field passed and then failed the whole message's JSON encode. Payloads must now be JSON values, so such a record is dropped alone. The hole check now aborts, so a later check on the array (the context's contextId uniqueness filter) never sees a hole when every issue is collected. A test pins that null holes from servers on #16300 still decode. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/contracts/src/baseSchemas.test.ts | 7 +++++ packages/contracts/src/baseSchemas.ts | 16 ++++++----- .../contracts/src/composerContext.test.ts | 27 +++++++++++++++++++ packages/contracts/src/composerContext.ts | 12 ++++----- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/packages/contracts/src/baseSchemas.test.ts b/packages/contracts/src/baseSchemas.test.ts index f36425131299..fbc992677898 100644 --- a/packages/contracts/src/baseSchemas.test.ts +++ b/packages/contracts/src/baseSchemas.test.ts @@ -72,6 +72,13 @@ describe("ForwardCompatibleArray", () => { ).toEqual([{ name: "a" }]); }); + it("still drops the null holes a server on an earlier build sends", () => { + expect(fromWire(Named)([{ name: "a" }, null, { name: "b" }])).toEqual([ + { name: "a" }, + { name: "b" }, + ]); + }); + it("does not accept holes as a decoded value", () => { expect(Schema.is(Named)([undefined])).toBe(false); expect(Schema.is(Named)([{ name: "a" }])).toBe(true); diff --git a/packages/contracts/src/baseSchemas.ts b/packages/contracts/src/baseSchemas.ts index 297cbb5d3eb5..62479500ef10 100644 --- a/packages/contracts/src/baseSchemas.ts +++ b/packages/contracts/src/baseSchemas.ts @@ -142,18 +142,22 @@ export const ForwardCompatibleArray = (element: Elem ), ).check( // The holes above are an encoding detail: a decoded value has none, so - // `Schema.is` and `make` still reject an array that does. - Schema.makeFilter((values) => values.every((value) => value !== undefined), { - expected: "an array without holes", - }), + // `Schema.is` and `make` still reject an array that does. Aborts, so a + // later check on the array never sees a hole. + Schema.makeFilter( + (values) => values.every((value) => value !== undefined), + { expected: "an array without holes" }, + true, + ), ), SchemaTransformation.transform< ReadonlyArray, ReadonlyArray >({ decode: (values) => values.filter((value) => value !== undefined), - // Dropped before the wire, so a wrapper that sees the encoded array as - // JSON values never meets a hole. + // An element that fails its own checks is dropped before the wire, so + // a wrapper that reads the encoded array as JSON values never meets + // the hole it left. encode: (values) => values.filter((value) => value !== undefined), }), ), diff --git a/packages/contracts/src/composerContext.test.ts b/packages/contracts/src/composerContext.test.ts index 9ca06684f026..c359e6d0a22f 100644 --- a/packages/contracts/src/composerContext.test.ts +++ b/packages/contracts/src/composerContext.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from "vite-plus/test"; +import * as Cause from "effect/Cause"; +import * as Exit from "effect/Exit"; import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; @@ -208,6 +210,31 @@ describe("OrchestrationMessageContext", () => { expect(decodeContext(wire).records.map((record) => record.contextId)).toEqual(["ctx_1"]); }); + it("sends a message without an unknown-kind record whose payload is not JSON", () => { + const wire = Schema.encodeUnknownSync(Schema.toCodecJson(OrchestrationMessageContext))({ + version: 1, + records: [ + decodeContext({ version: 1, records: [knownRecords.terminal] }).records[0], + { + ...base, + contextId: "ctx_2", + kind: "future-kind", + label: "x", + payload: { count: Number.NaN }, + }, + ], + }); + expect(decodeContext(wire).records.map((record) => record.contextId)).toEqual(["ctx_1"]); + }); + + it("reports a hole as a schema issue, even when collecting every issue", () => { + const result = Schema.decodeUnknownExit(Schema.toType(OrchestrationMessageContext))( + { version: 1, records: [undefined] }, + { errors: "all" }, + ); + expect(Exit.isFailure(result) && Cause.hasFails(result.cause)).toBe(true); + }); + it("normalizes decoded record identifiers", () => { const context = decodeContext({ version: 1, diff --git a/packages/contracts/src/composerContext.ts b/packages/contracts/src/composerContext.ts index 6d8975a0100d..66d584e6cf5f 100644 --- a/packages/contracts/src/composerContext.ts +++ b/packages/contracts/src/composerContext.ts @@ -231,6 +231,8 @@ export const ThreadContextRecord = Schema.Struct({ }); export type ThreadContextRecord = typeof ThreadContextRecord.Type; +const isJson = Schema.is(Schema.Json); + /** * Catch-all for kinds this build does not know. Known discriminators are excluded so a * malformed known record fails its own schema instead of sliding through unchecked. @@ -241,12 +243,10 @@ export const UnknownContextRecord = Schema.Struct({ kind: ComposerContextKind.check(Schema.isPattern(KNOWN_KIND_PATTERN)), payload: Schema.Unknown.check( Schema.makeFilter((payload) => { - try { - const encoded = JSON.stringify(payload); - return encoded !== undefined && encoded.length <= 64_000; - } catch { - return false; - } + // Only JSON values, so a payload the wire cannot carry (a Date, NaN, an + // undefined field) fails this record alone instead of the whole message. + if (!isJson(payload)) return false; + return JSON.stringify(payload).length <= 64_000; }), ), }); From cefe23ccea8f651886fb3c6ef3fd2181b161b329 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 6 Oct 2026 04:24:30 -0700 Subject: [PATCH 3/5] fix(contracts): a context record with an undefined optional field is dropped alone Optional context record fields are now optionalKey, so a record holding an explicit undefined fails its own check instead of failing the whole send. On the real wire path null was already rejected, since records are bounded as JSON values before they decode, so this changes nothing for decoding. The unknown-kind payload check stringifies inside a try again: a payload nested too deep to stringify threw and killed the whole decode instead of dropping its record. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/lib/composerContextRecords.test.ts | 4 +-- .../contracts/src/composerContext.test.ts | 10 ++++++- packages/contracts/src/composerContext.ts | 29 ++++++++++++------- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/apps/web/src/lib/composerContextRecords.test.ts b/apps/web/src/lib/composerContextRecords.test.ts index e5a83030a5fa..3634cea6d448 100644 --- a/apps/web/src/lib/composerContextRecords.test.ts +++ b/apps/web/src/lib/composerContextRecords.test.ts @@ -514,7 +514,7 @@ describe("composerContextRecords", () => { expect( isSameComposerContextPayload(base, { ...base, - elements: base.elements?.map((element) => ({ + elements: base.elements!.map((element) => ({ ...element, htmlPreview: '', })), @@ -523,7 +523,7 @@ describe("composerContextRecords", () => { expect( isSameComposerContextPayload(base, { ...base, - elements: base.elements?.map((element) => ({ + elements: base.elements!.map((element) => ({ ...element, source: { functionName: "Checkout", diff --git a/packages/contracts/src/composerContext.test.ts b/packages/contracts/src/composerContext.test.ts index c359e6d0a22f..77f4d31b4753 100644 --- a/packages/contracts/src/composerContext.test.ts +++ b/packages/contracts/src/composerContext.test.ts @@ -210,7 +210,7 @@ describe("OrchestrationMessageContext", () => { expect(decodeContext(wire).records.map((record) => record.contextId)).toEqual(["ctx_1"]); }); - it("sends a message without an unknown-kind record whose payload is not JSON", () => { + it("sends a message without the records the wire cannot carry", () => { const wire = Schema.encodeUnknownSync(Schema.toCodecJson(OrchestrationMessageContext))({ version: 1, records: [ @@ -222,6 +222,14 @@ describe("OrchestrationMessageContext", () => { label: "x", payload: { count: Number.NaN }, }, + { ...knownRecords["review-comment"], contextId: "ctx_3", fenceLanguage: undefined }, + { + ...base, + contextId: "ctx_4", + kind: "future-kind", + label: "deep", + payload: Array.from({ length: 20_000 }).reduce((inner: unknown) => [inner], 0), + }, ], }); expect(decodeContext(wire).records.map((record) => record.contextId)).toEqual(["ctx_1"]); diff --git a/packages/contracts/src/composerContext.ts b/packages/contracts/src/composerContext.ts index 66d584e6cf5f..86618bbc0599 100644 --- a/packages/contracts/src/composerContext.ts +++ b/packages/contracts/src/composerContext.ts @@ -155,6 +155,9 @@ export const ElementContextRecord = Schema.Struct({ }); export type ElementContextRecord = typeof ElementContextRecord.Type; +// Optional record fields are `optionalKey`: a record holding an explicit +// `undefined` cannot be sent as JSON, so it fails its own check and is dropped +// alone instead of failing the whole message. export const PreviewAnnotationContextRecord = Schema.Struct({ ...recordBase, kind: Schema.Literal("preview-annotation"), @@ -165,14 +168,14 @@ export const PreviewAnnotationContextRecord = Schema.Struct({ targetSummary: ShortString, styleChanges: Schema.Array(ShortString).check(Schema.isMaxLength(200)), /** Picked elements inside the annotation, with the detail the agent needs to find them. */ - elements: Schema.optional(Schema.Array(ElementContextDetails).check(Schema.isMaxLength(50))), + elements: Schema.optionalKey(Schema.Array(ElementContextDetails).check(Schema.isMaxLength(50))), /** Original target ids and edits allow pasted annotations to retain exact style changes. */ - elementIds: Schema.optional(Schema.Array(ShortString).check(Schema.isMaxLength(50))), + elementIds: Schema.optionalKey(Schema.Array(ShortString).check(Schema.isMaxLength(50))), /** Region and stroke geometry is lossy on purpose, but their counts feed the target summary, so a pasted annotation still says what it marked. */ - regionCount: Schema.optional(NonNegativeInt), - strokeCount: Schema.optional(NonNegativeInt), - styleChangeDetails: Schema.optional( + regionCount: Schema.optionalKey(NonNegativeInt), + strokeCount: Schema.optionalKey(NonNegativeInt), + styleChangeDetails: Schema.optionalKey( Schema.Array( Schema.Struct({ targetId: ShortString, @@ -184,7 +187,7 @@ export const PreviewAnnotationContextRecord = Schema.Struct({ ).check(Schema.isMaxLength(200)), ), /** The screenshot travels as its own image record; this links the two. */ - screenshotContextId: Schema.optional(ComposerContextId), + screenshotContextId: Schema.optionalKey(ComposerContextId), }); export type PreviewAnnotationContextRecord = typeof PreviewAnnotationContextRecord.Type; @@ -199,8 +202,8 @@ export const ReviewCommentContextRecord = Schema.Struct({ rangeLabel: ShortString, text: BoundedString(COMPOSER_CONTEXT_REVIEW_TEXT_MAX_CHARS), diff: BoundedString(COMPOSER_CONTEXT_REVIEW_DIFF_MAX_CHARS), - fenceLanguage: Schema.optional(BoundedString(64)), - pullRequest: Schema.optional(PullRequestContextMetadata), + fenceLanguage: Schema.optionalKey(BoundedString(64)), + pullRequest: Schema.optionalKey(PullRequestContextMetadata), }).check(Schema.makeFilter((record) => record.endIndex >= record.startIndex)); export type ReviewCommentContextRecord = typeof ReviewCommentContextRecord.Type; @@ -243,10 +246,16 @@ export const UnknownContextRecord = Schema.Struct({ kind: ComposerContextKind.check(Schema.isPattern(KNOWN_KIND_PATTERN)), payload: Schema.Unknown.check( Schema.makeFilter((payload) => { + let encoded: string | undefined; + try { + encoded = JSON.stringify(payload); + } catch { + // Cycles, bigints and payloads nested too deep to stringify. + return false; + } // Only JSON values, so a payload the wire cannot carry (a Date, NaN, an // undefined field) fails this record alone instead of the whole message. - if (!isJson(payload)) return false; - return JSON.stringify(payload).length <= 64_000; + return encoded !== undefined && encoded.length <= 64_000 && isJson(payload); }), ), }); From 98a398432054ba05cc86b15b99212b499b88b481 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 6 Oct 2026 05:56:29 -0700 Subject: [PATCH 4/5] test(contracts): cover the unstringifiable payload with a bigint, not stack depth How deep a payload must be before JSON.stringify throws depends on the engine and thread, so the deep row passed only on some runtimes. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/contracts/src/composerContext.test.ts | 9 ++------- packages/contracts/src/composerContext.ts | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/contracts/src/composerContext.test.ts b/packages/contracts/src/composerContext.test.ts index 77f4d31b4753..b43e59c2840b 100644 --- a/packages/contracts/src/composerContext.test.ts +++ b/packages/contracts/src/composerContext.test.ts @@ -223,13 +223,8 @@ describe("OrchestrationMessageContext", () => { payload: { count: Number.NaN }, }, { ...knownRecords["review-comment"], contextId: "ctx_3", fenceLanguage: undefined }, - { - ...base, - contextId: "ctx_4", - kind: "future-kind", - label: "deep", - payload: Array.from({ length: 20_000 }).reduce((inner: unknown) => [inner], 0), - }, + // JSON.stringify throws on a bigint on every engine. + { ...base, contextId: "ctx_4", kind: "future-kind", label: "y", payload: { n: 1n } }, ], }); expect(decodeContext(wire).records.map((record) => record.contextId)).toEqual(["ctx_1"]); diff --git a/packages/contracts/src/composerContext.ts b/packages/contracts/src/composerContext.ts index 86618bbc0599..116c141c9dfb 100644 --- a/packages/contracts/src/composerContext.ts +++ b/packages/contracts/src/composerContext.ts @@ -250,7 +250,7 @@ export const UnknownContextRecord = Schema.Struct({ try { encoded = JSON.stringify(payload); } catch { - // Cycles, bigints and payloads nested too deep to stringify. + // Cycles, bigints, and payloads nested deeper than this engine's stack. return false; } // Only JSON values, so a payload the wire cannot carry (a Date, NaN, an From ef743e01926806d8f044500610e7d45bfc22d5a8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 6 Oct 2026 16:13:54 -0700 Subject: [PATCH 5/5] test(contracts): pin that sparse arrays count as holes Effect's array parser already turns a sparse array's missing index into `undefined`, so `Schema.is` and `make` rejected `new Array(1)` before this. The hole check now walks every index itself instead of relying on that, and a test pins sparse arrays alongside explicit `undefined`. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/contracts/src/baseSchemas.test.ts | 5 +++++ packages/contracts/src/baseSchemas.ts | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/contracts/src/baseSchemas.test.ts b/packages/contracts/src/baseSchemas.test.ts index fbc992677898..d1bcd4c153a0 100644 --- a/packages/contracts/src/baseSchemas.test.ts +++ b/packages/contracts/src/baseSchemas.test.ts @@ -81,6 +81,11 @@ describe("ForwardCompatibleArray", () => { it("does not accept holes as a decoded value", () => { expect(Schema.is(Named)([undefined])).toBe(false); + // A sparse array's missing index is a hole too. + const sparse: Array<{ name: string }> = [{ name: "a" }]; + sparse.length = 2; + expect(Schema.is(Named)(sparse)).toBe(false); + expect(() => Named.make(sparse)).toThrow(); expect(Schema.is(Named)([{ name: "a" }])).toBe(true); }); }); diff --git a/packages/contracts/src/baseSchemas.ts b/packages/contracts/src/baseSchemas.ts index 62479500ef10..3ca88df66b19 100644 --- a/packages/contracts/src/baseSchemas.ts +++ b/packages/contracts/src/baseSchemas.ts @@ -145,7 +145,13 @@ export const ForwardCompatibleArray = (element: Elem // `Schema.is` and `make` still reject an array that does. Aborts, so a // later check on the array never sees a hole. Schema.makeFilter( - (values) => values.every((value) => value !== undefined), + (values) => { + // Every index, not `every`, which skips the holes of a sparse array. + for (let index = 0; index < values.length; index++) { + if (values[index] === undefined) return false; + } + return true; + }, { expected: "an array without holes" }, true, ),