diff --git a/clients/web/src/components/views/InspectorView/InspectorView.tsx b/clients/web/src/components/views/InspectorView/InspectorView.tsx index 6e845dba..43749729 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.tsx @@ -105,7 +105,7 @@ import { } from "../../../utils/correlateTransportErrors"; import { applySchemaConstants, - collectSchemaDefaults, + seedSchemaValues, toFormSchema, } from "../../../utils/jsonUtils"; import { MONITOR_COLUMN_ANIM_MS } from "./monitorColumnAnimation"; @@ -1049,10 +1049,13 @@ export function InspectorView({ // link disagreeing with one would otherwise auto-open with a hidden value // contradicting the shape on screen. const appFormSchema = toFormSchema(target.inputSchema) ?? {}; - const formValues = applySchemaConstants(appFormSchema, { - ...collectSchemaDefaults(appFormSchema, deepLink.appArgs ?? {}), - ...deepLink.appArgs, - }); + const formValues = applySchemaConstants( + appFormSchema, + // Merged per level, not with one shallow spread: a nested object in the + // args would otherwise replace the whole seeded object, discarding the + // nested defaults the form goes on displaying. + seedSchemaValues(appFormSchema, deepLink.appArgs ?? {}), + ); // Seed the selection directly rather than routing through // AppsScreen.handleSelect. This deliberately bypasses handleSelect's // no-input-app auto-launch: a deep link must never invoke a tool against diff --git a/clients/web/src/utils/jsonUtils.test.ts b/clients/web/src/utils/jsonUtils.test.ts index 228f81f3..62d0e123 100644 --- a/clients/web/src/utils/jsonUtils.test.ts +++ b/clients/web/src/utils/jsonUtils.test.ts @@ -7,6 +7,7 @@ import { collectSchemaDefaults, hasMissingRequiredFields, applySchemaConstants, + seedSchemaValues, } from "./jsonUtils"; import type { InspectorFormSchema } from "./jsonUtils"; @@ -488,6 +489,65 @@ describe("root composition (#2123)", () => { ).toEqual({ config: { kind: "email" } }); }); + describe("seedSchemaValues", () => { + const NESTED: InspectorFormSchema = { + type: "object", + properties: { + config: { + type: "object", + anyOf: [ + { + type: "object", + properties: { + kind: { type: "string", const: "email" }, + retries: { type: "number", default: 1 }, + }, + }, + { + type: "object", + properties: { + kind: { type: "string", const: "sms" }, + retries: { type: "number", default: 3 }, + }, + }, + ], + }, + }, + }; + + it("keeps a nested branch's defaults beside the supplied values", () => { + // A shallow spread would replace the whole `config` object, so the SMS + // branch's `retries` would be displayed by the form and never submitted. + expect(seedSchemaValues(NESTED, { config: { kind: "sms" } })).toEqual({ + config: { kind: "sms", retries: 3 }, + }); + }); + + it("lets the supplied value win where the two meet", () => { + expect( + seedSchemaValues(NESTED, { config: { kind: "sms", retries: 9 } }), + ).toEqual({ config: { kind: "sms", retries: 9 } }); + }); + + it("does not treat an array as a nested object", () => { + const withArray: InspectorFormSchema = { + type: "object", + properties: { items: { type: "array" } }, + }; + expect(seedSchemaValues(withArray, { items: [1, 2] })).toEqual({ + items: [1, 2], + }); + }); + + it("keeps a supplied argument named __proto__", () => { + const seeded = seedSchemaValues( + { type: "object", properties: {} }, + Object.fromEntries([["__proto__", "kept"]]), + ); + expect(Object.hasOwn(seeded, "__proto__")).toBe(true); + }); + }); + it("re-applies a nested object's constants", () => { // The overlay replaces the whole nested object rather than merging into it, // so a link naming `{ config: { kind: "sms" } }` would otherwise slip past diff --git a/clients/web/src/utils/jsonUtils.ts b/clients/web/src/utils/jsonUtils.ts index acf5f0a7..5293c697 100644 --- a/clients/web/src/utils/jsonUtils.ts +++ b/clients/web/src/utils/jsonUtils.ts @@ -194,6 +194,60 @@ export function collectSchemaDefaults( return result; } +/** + * Seed a schema's defaults underneath values a caller already holds, merging + * the two **per level** rather than with one shallow spread. + * + * The App deep link is the caller: it holds `appArgs` and needs the defaults + * the form would otherwise display. A shallow `{ ...defaults, ...args }` + * replaces a nested object wholesale, so `{ config: { kind: "sms" } }` discards + * the nested SMS branch's own defaults — which the form then shows while the + * submitted arguments omit them (#2123). Recursing also lets each nested level + * pick its branch from the nested values, rather than from the top-level ones. + * + * The supplied value always wins where the two meet; only what it does not + * mention is seeded. + */ +export function seedSchemaValues( + schema: InspectorFormSchema, + suppliedValues: Record, +): Record { + const { base, branches } = resolveRootUnion(schema); + const selected = selectBranchIndex(branches, suppliedValues) ?? 0; + const effective = branches[selected]?.schema ?? base; + const properties = effective.properties ?? {}; + + const merged: Record = { + ...collectSchemaDefaults(effective, suppliedValues), + }; + for (const [name, value] of Object.entries(suppliedValues)) { + const fieldSchema = normalizeNullableUnion(properties[name] ?? {}); + const mergedValue = + isPlainObject(value) && + fieldSchema.type === "object" && + declaresAnyFields(fieldSchema) + ? seedSchemaValues( + fieldSchema, + // The nested defaults are re-derived by the recursion, so only the + // supplied half is passed down. + value, + ) + : value; + Object.defineProperty(merged, name, { + value: mergedValue, + writable: true, + enumerable: true, + configurable: true, + }); + } + return merged; +} + +/** Whether a value is a plain object a nested schema could describe. */ +function isPlainObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + /** * Overwrite every `const`-pinned field with the value its schema fixes. *