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 a8e428564eda..e877b165eb9f 100644 --- a/apps/mobile/src/features/files/AttachmentFileScreen.tsx +++ b/apps/mobile/src/features/files/AttachmentFileScreen.tsx @@ -172,7 +172,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 ? 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 10ece5be6b4a..3e2c05c35058 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"; @@ -43,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", () => { @@ -105,3 +124,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 464e4ab5c912..adb64a1a4a6f 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)); @@ -108,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 @@ -119,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