Skip to content
12 changes: 12 additions & 0 deletions apps/mobile/src/features/threads/git/GitOverviewSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 <GitOverviewSheetContent {...props} />;
}

function GitOverviewSheetContent(props: GitOverviewSheetProps) {
const { layout } = useAdaptiveWorkspaceLayout();
const navigation = useNavigation();
const insets = useSafeAreaInsets();
Expand Down
4 changes: 2 additions & 2 deletions apps/web/src/lib/composerContextRecords.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ describe("composerContextRecords", () => {
expect(
isSameComposerContextPayload(base, {
...base,
elements: base.elements?.map((element) => ({
elements: base.elements!.map((element) => ({
...element,
htmlPreview: '<button id="pay">Changed</button>',
})),
Expand All @@ -523,7 +523,7 @@ describe("composerContextRecords", () => {
expect(
isSameComposerContextPayload(base, {
...base,
elements: base.elements?.map((element) => ({
elements: base.elements!.map((element) => ({
...element,
source: {
functionName: "Checkout",
Expand Down
32 changes: 29 additions & 3 deletions packages/contracts/src/baseSchemas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))([
Expand All @@ -59,9 +60,34 @@ 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("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);
// 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);
});
});

describe("ForwardCompatibleUnion", () => {
Expand Down
20 changes: 19 additions & 1 deletion packages/contracts/src/baseSchemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,31 @@ export const ForwardCompatibleArray = <Element extends Schema.Top>(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. Aborts, so a
// later check on the array never sees a hole.
Schema.makeFilter(
(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" },
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
true,
),
),
SchemaTransformation.transform<
ReadonlyArray<Element["Type"]>,
ReadonlyArray<Element["Type"] | undefined>
>({
decode: (values) => values.filter((value) => value !== undefined),
encode: (values) => values,
// 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),
}),
),
) as unknown as ForwardCompatibleArray<Element>;
Expand Down
41 changes: 41 additions & 0 deletions packages/contracts/src/composerContext.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -197,6 +199,45 @@ 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("sends a message without the records the wire cannot carry", () => {
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 },
},
{ ...knownRecords["review-comment"], contextId: "ctx_3", fenceLanguage: undefined },
// 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"]);
});

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,
Expand Down
29 changes: 19 additions & 10 deletions packages/contracts/src/composerContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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,
Expand All @@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -231,6 +234,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.
Expand All @@ -241,12 +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 {
const encoded = JSON.stringify(payload);
return encoded !== undefined && encoded.length <= 64_000;
encoded = JSON.stringify(payload);
} catch {
// 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
// undefined field) fails this record alone instead of the whole message.
return encoded !== undefined && encoded.length <= 64_000 && isJson(payload);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}),
),
});
Expand Down
Loading