From 4c05382b479a5e5d74506936677e07ffd0309e13 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 18:46:39 -0700 Subject: [PATCH 1/3] fix(contracts): trimmed IDs round-trip TrimmedNonEmptyString checked non-emptiness on the untrimmed value, but TrimmedString only trims while decoding and encoding. So ThreadId.make(" ") and the other branded IDs accepted a whitespace-only string, encoded it to "", and then could not decode it again. A row written that way would make its projection unreadable. The non-empty check now trims before it tests, so make, Schema.is, decode and encode all reject whitespace-only values, and anything accepted encodes to a value that decodes back. Valid values encode exactly as before, and the JSON Schema output of every contracts export is unchanged. Property tests built on Effect's Arbitrary and @effect/vitest it.prop cover TrimmedNonEmptyString and all 29 makeEntityId brands, using whitespace-padded and whitespace-only inputs. Before the fix, 91 of them fail. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/contracts/src/baseSchemas.test.ts | 129 +++++++++++++++++++++ packages/contracts/src/baseSchemas.ts | 13 ++- 2 files changed, 141 insertions(+), 1 deletion(-) diff --git a/packages/contracts/src/baseSchemas.test.ts b/packages/contracts/src/baseSchemas.test.ts index 10ece5be6b4a..bf21077d5d60 100644 --- a/packages/contracts/src/baseSchemas.test.ts +++ b/packages/contracts/src/baseSchemas.test.ts @@ -1,13 +1,17 @@ import { describe, expect, it } from "@effect/vitest"; +import * as Arbitrary from "effect/Arbitrary"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import * as Schema from "effect/Schema"; +import * as BaseSchemas from "./baseSchemas.ts"; import { ForwardCompatibleArray, ForwardCompatibleUnion, ForwardCompatibleUnionArray, isUnknownUnionMember, + TrimmedNonEmptyString, UnknownUnionMember, } from "./baseSchemas.ts"; @@ -105,3 +109,128 @@ describe("ForwardCompatibleUnionArray", () => { expect(encode([square])).toEqual([square]); }); }); + +describe("trimmed non-empty strings", () => { + const entityIds = [ + "ThreadId", + "ProjectId", + "EnvironmentId", + "CommandId", + "EventId", + "MessageId", + "TurnId", + "RunId", + "RunAttemptId", + "NodeId", + "AuthSessionId", + "ProviderItemId", + "ProviderSessionId", + "ProviderThreadId", + "ProviderTurnId", + "RuntimeSessionId", + "RuntimeItemId", + "TurnItemId", + "RuntimeRequestId", + "RuntimeTaskId", + "ScheduledTaskId", + "ApprovalRequestId", + "CheckpointRef", + "CheckpointId", + "CheckpointScopeId", + "ContextHandoffId", + "ContextTransferId", + "RawEventId", + "PlanId", + ] as const; + const schemas = [ + ["TrimmedNonEmptyString", TrimmedNonEmptyString], + ...entityIds.map((name) => [name, BaseSchemas[name]] as const), + ] as const; + + // Schema-derived strings are rarely padded, so pad them with every kind of + // whitespace `trim()` removes, and include whitespace-only values. + const whitespace = Arbitrary.schema( + Schema.Literals([" ", "\t", "\n", "\r\n", "\v", "\f", "\u00a0", "\u2028", "\u3000", "\ufeff"]), + ); + const padding = Arbitrary.array(whitespace, { maxLength: 4 }).pipe( + Arbitrary.map((parts) => parts.join("")), + ); + const paddedString = Arbitrary.all([padding, Arbitrary.schema(Schema.String), padding]).pipe( + Arbitrary.map(([before, value, after]) => before + value + after), + ); + const options = { arbitrary: { runs: 500 } }; + + for (const [name, schema] of schemas) { + const make = schema.makeOption; + const encode = Schema.encodeUnknownExit(schema); + const decode = Schema.decodeExit(schema); + + it.prop( + `${name}: whatever make accepts encodes to something that decodes back`, + [paddedString], + ([input]) => { + const made = make(input); + if (made._tag === "None") { + expect(input.trim()).toBe(""); + return; + } + const encoded = encode(made.value); + expect(encoded).toStrictEqual(Exit.succeed(input.trim())); + expect(decode(input.trim())).toStrictEqual(Exit.succeed(input.trim())); + }, + options, + ); + + it.prop( + `${name}: decoding then encoding is stable`, + [paddedString], + ([input]) => { + const decoded = decode(input); + if (Exit.isFailure(decoded)) { + expect(input.trim()).toBe(""); + expect(Exit.isFailure(encode(input))).toBe(true); + return; + } + expect(decoded.value).toBe(input.trim()); + expect(encode(decoded.value)).toStrictEqual(Exit.succeed(decoded.value)); + }, + options, + ); + + it.prop( + `${name}: generated values round-trip`, + [schema], + ([value]) => { + const encoded = encode(value); + expect(Exit.isSuccess(encoded)).toBe(true); + if (Exit.isSuccess(encoded)) + expect(decode(encoded.value)).toStrictEqual(Exit.succeed(value.trim())); + }, + options, + ); + } + + const isThreadId = Schema.is(BaseSchemas.ThreadId); + const encodeThreadId = Schema.encodeUnknownExit(BaseSchemas.ThreadId); + const decodeThreadId = Schema.decodeExit(BaseSchemas.ThreadId); + + it("rejects whitespace-only values everywhere", () => { + for (const value of [" ", "\t\n", "\u00a0\u3000"]) { + expect(() => BaseSchemas.ThreadId.make(value)).toThrow(); + expect(isThreadId(value)).toBe(false); + expect(Exit.isFailure(encodeThreadId(value))).toBe(true); + expect(Exit.isFailure(decodeThreadId(value))).toBe(true); + } + }); + + it("keeps the encoded form and JSON Schema of valid values", () => { + expect(encodeThreadId(BaseSchemas.ThreadId.make("thread-1"))).toStrictEqual( + Exit.succeed("thread-1"), + ); + expect(encodeThreadId(" a b ")).toStrictEqual(Exit.succeed("a b")); + expect(Schema.toJsonSchemaDocument(Schema.toType(BaseSchemas.ThreadId)).schema).toEqual({ + type: "string", + minLength: 1, + }); + }); +}); diff --git a/packages/contracts/src/baseSchemas.ts b/packages/contracts/src/baseSchemas.ts index 00cb38a572a8..e1a6e08ca2f8 100644 --- a/packages/contracts/src/baseSchemas.ts +++ b/packages/contracts/src/baseSchemas.ts @@ -13,7 +13,18 @@ export const TrimmedString = Schema.String.pipe( }), ), ); -export const TrimmedNonEmptyString = TrimmedString.check(Schema.isNonEmpty()); +/** + * Non-empty once trimmed. A `TrimmedString` only trims when decoding or + * encoding, so `make` and encode see the untrimmed value: a plain + * `isNonEmpty` would accept `" "` there and encode it to `""`, which no + * longer decodes. + */ +const isNonBlank = Schema.makeFilter((value: string) => value.trim().length > 0, { + expected: "a non-blank string", + toJsonSchema: () => [{ minLength: 1 }, true], + arbitraryConstraint: { minLength: 1 }, +}); +export const TrimmedNonEmptyString = TrimmedString.check(isNonBlank); export const NonNegativeInt = Schema.Int.check(Schema.isGreaterThanOrEqualTo(0)); export const PositiveInt = Schema.Int.check(Schema.isGreaterThanOrEqualTo(1)); From a4719c4070067906ed80a1ccf09857e973426178 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 21:22:07 -0700 Subject: [PATCH 2/3] fix(contracts): a blank value in a forward-compatible array costs only its element Rejecting blank IDs and trimmed strings on encode meant one bad element (a whitespace-only skill description, model name or usage bucket) failed the whole server config or usage payload, where it used to drop just that element on the client. ForwardCompatibleArray now sends such an element as a hole, which every client already drops. Mobile route params that are blank (a hand-typed deep link) are treated as missing instead of reaching a branded ID's make and the error screen. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/devices/DevicePreviewRouteScreen.tsx | 11 +++++++++-- .../src/features/files/AttachmentFileScreen.tsx | 2 +- .../src/features/files/ThreadFilesRouteScreen.tsx | 9 ++++----- .../terminal/ThreadTerminalRouteScreen.tsx | 9 ++++----- .../src/features/threads/ThreadRouteScreen.tsx | 9 ++++----- apps/mobile/src/state/use-thread-selection.ts | 9 ++++----- packages/contracts/src/baseSchemas.test.ts | 15 +++++++++++++++ packages/contracts/src/baseSchemas.ts | 11 +++++++++-- 8 files changed, 50 insertions(+), 25 deletions(-) diff --git a/apps/mobile/src/features/devices/DevicePreviewRouteScreen.tsx b/apps/mobile/src/features/devices/DevicePreviewRouteScreen.tsx index d3b960844efb..874fa33320c5 100644 --- a/apps/mobile/src/features/devices/DevicePreviewRouteScreen.tsx +++ b/apps/mobile/src/features/devices/DevicePreviewRouteScreen.tsx @@ -54,10 +54,17 @@ type DevicePreviewRouteScreenProps = StaticScreenProps<{ export function DevicePreviewRouteScreen({ route }: DevicePreviewRouteScreenProps) { const navigation = useNavigation(); const onClose = useCallback(() => navigation.goBack(), [navigation]); + const { environmentId, threadId } = 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) onClose(); + }, [isBlankLink, onClose]); + if (isBlankLink) return null; return ( ); diff --git a/apps/mobile/src/features/files/AttachmentFileScreen.tsx b/apps/mobile/src/features/files/AttachmentFileScreen.tsx index 23adc5cddb12..c13d68134fad 100644 --- a/apps/mobile/src/features/files/AttachmentFileScreen.tsx +++ b/apps/mobile/src/features/files/AttachmentFileScreen.tsx @@ -164,7 +164,7 @@ export function AttachmentFileScreen(props: AttachmentFileScreenProps) { const iconColor = useUniwindTheme()["--color-icon"]; const isAndroid = Platform.OS === "android"; const params = props.route.params; - const environmentId = params.environmentId ? EnvironmentId.make(params.environmentId) : null; + const environmentId = params.environmentId?.trim() ? EnvironmentId.make(params.environmentId) : null; const sizeBytes = Number.parseInt(params.sizeBytes, 10) || 0; const draftKey = params.draftKey ?? null; const draft = useComposerDraft(draftKey); diff --git a/apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx b/apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx index 06b6664431b5..c216adbc6f0c 100644 --- a/apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx +++ b/apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx @@ -171,12 +171,11 @@ function FileHeader(props: { type FileViewMode = "preview" | "source"; +// A blank param (a hand-typed deep link) is treated as missing, since branded +// IDs reject whitespace-only values. function firstRouteParam(value: string | string[] | undefined): string | null { - if (Array.isArray(value)) { - return value[0] ?? null; - } - - return value ?? null; + const first = Array.isArray(value) ? value[0] : value; + return first === undefined || first.trim().length === 0 ? null : first; } function normalizeRoutePath(value: string | string[] | undefined): string | null { diff --git a/apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx b/apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx index c1a0d9ee65c1..6b711d8fec98 100644 --- a/apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx +++ b/apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx @@ -188,12 +188,11 @@ type TerminalToolbarAction = readonly modifier: PendingModifier; }; +// A blank param (a hand-typed deep link) is treated as missing, since branded +// IDs reject whitespace-only values. function firstRouteParam(value: string | string[] | undefined): string | null { - if (Array.isArray(value)) { - return value[0] ?? null; - } - - return value ?? null; + const first = Array.isArray(value) ? value[0] : value; + return first === undefined || first.trim().length === 0 ? null : first; } function inferHostPlatform(environmentLabel: string | null): HostPlatform { diff --git a/apps/mobile/src/features/threads/ThreadRouteScreen.tsx b/apps/mobile/src/features/threads/ThreadRouteScreen.tsx index 72d868677ba8..b7ebc569cedd 100644 --- a/apps/mobile/src/features/threads/ThreadRouteScreen.tsx +++ b/apps/mobile/src/features/threads/ThreadRouteScreen.tsx @@ -200,12 +200,11 @@ function InspectorPaneRoleActivation() { return null; } +// A blank param (a hand-typed deep link) is treated as missing, since branded +// IDs reject whitespace-only values. function firstRouteParam(value: string | string[] | undefined): string | null { - if (Array.isArray(value)) { - return value[0] ?? null; - } - - return value ?? null; + const first = Array.isArray(value) ? value[0] : value; + return first === undefined || first.trim().length === 0 ? null : first; } function OpeningThreadLoadingScreen() { diff --git a/apps/mobile/src/state/use-thread-selection.ts b/apps/mobile/src/state/use-thread-selection.ts index c967bb0da5f9..3624caebb076 100644 --- a/apps/mobile/src/state/use-thread-selection.ts +++ b/apps/mobile/src/state/use-thread-selection.ts @@ -38,12 +38,11 @@ type ThreadSelectionRouteParams = { readonly threadId?: string | string[]; }; +// A blank param (a hand-typed deep link) is treated as missing, since branded +// IDs reject whitespace-only values. function firstRouteParam(value: string | string[] | undefined): string | null { - if (Array.isArray(value)) { - return value[0] ?? null; - } - - return value ?? null; + const first = Array.isArray(value) ? value[0] : value; + return first === undefined || first.trim().length === 0 ? null : first; } function latestUserMessageAt( diff --git a/packages/contracts/src/baseSchemas.test.ts b/packages/contracts/src/baseSchemas.test.ts index bf21077d5d60..3e2c05c35058 100644 --- a/packages/contracts/src/baseSchemas.test.ts +++ b/packages/contracts/src/baseSchemas.test.ts @@ -47,6 +47,21 @@ describe("ForwardCompatibleArray", () => { { name: "a", count: 7 }, ]); }); + + it("sends an element it cannot encode as a hole instead of failing the array", () => { + const Named = ForwardCompatibleArray(Schema.Struct({ name: TrimmedNonEmptyString })); + const wire = JSON.parse( + JSON.stringify( + Schema.encodeUnknownSync(Schema.toCodecJson(Named))([ + { name: "a" }, + { name: " " }, + { name: "b" }, + ]), + ), + ); + expect(wire).toEqual([{ name: "a" }, null, { name: "b" }]); + expect(fromWire(Named)(wire)).toEqual([{ name: "a" }, { name: "b" }]); + }); }); describe("ForwardCompatibleUnion", () => { diff --git a/packages/contracts/src/baseSchemas.ts b/packages/contracts/src/baseSchemas.ts index e1a6e08ca2f8..5df29132d1a7 100644 --- a/packages/contracts/src/baseSchemas.ts +++ b/packages/contracts/src/baseSchemas.ts @@ -119,7 +119,7 @@ export const OmittedWhenNull = (value: Value) => { * * Decoding runs each value through its own schema, so transformations (dates, * trimming, decoding defaults) apply as usual. Only values that schema - * rejects are dropped; encoding is the plain encoding. + * rejects are dropped, on either side. * * For a tagged union, prefer {@link ForwardCompatibleUnion}: it drops only * values whose tag this build does not know, so a known member with a broken @@ -130,10 +130,17 @@ export const ForwardCompatibleArray = (element: Elem Schema.UndefinedOr(element).pipe( // An element this build cannot read becomes a hole, filtered out below. Schema.catchDecoding(() => Effect.succeedSome(undefined)), + // Likewise an element that cannot be encoded is sent as a hole, so one + // bad element costs only itself rather than the whole payload. + Schema.catchEncoding(() => Effect.succeedSome(undefined)), ), ).pipe( Schema.decodeTo( - Schema.Array(Schema.toType(element)), + Schema.Array( + Schema.UndefinedOr(Schema.toType(element)).pipe( + Schema.catchEncoding(() => Effect.succeedSome(undefined)), + ), + ), SchemaTransformation.transform< ReadonlyArray, ReadonlyArray From fa5e65a03f386987747d39693bb745b283d17e17 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 5 Oct 2026 21:31:03 -0700 Subject: [PATCH 3/3] style(mobile): format attachment screen Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/mobile/src/features/files/AttachmentFileScreen.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/mobile/src/features/files/AttachmentFileScreen.tsx b/apps/mobile/src/features/files/AttachmentFileScreen.tsx index c13d68134fad..0b79f2e14542 100644 --- a/apps/mobile/src/features/files/AttachmentFileScreen.tsx +++ b/apps/mobile/src/features/files/AttachmentFileScreen.tsx @@ -164,7 +164,9 @@ export function AttachmentFileScreen(props: AttachmentFileScreenProps) { const iconColor = useUniwindTheme()["--color-icon"]; const isAndroid = Platform.OS === "android"; const params = props.route.params; - const environmentId = params.environmentId?.trim() ? EnvironmentId.make(params.environmentId) : null; + const environmentId = params.environmentId?.trim() + ? EnvironmentId.make(params.environmentId) + : null; const sizeBytes = Number.parseInt(params.sizeBytes, 10) || 0; const draftKey = params.draftKey ?? null; const draft = useComposerDraft(draftKey);