From 035ea9c8563dd15919134c56fde8d7808bacf08a Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 5 Jul 2026 18:55:09 +0200 Subject: [PATCH] fix: remove redundant record guards --- .../src/harness/session/jsonl-storage.ts | 52 ++++++++++-------- packages/ai/src/utils/validation.ts | 42 +++++---------- ...penai-responses-tool-result-images.test.ts | 54 +++++++++---------- packages/coding-agent/src/core/keybindings.ts | 11 ++-- 4 files changed, 71 insertions(+), 88 deletions(-) diff --git a/packages/agent/src/harness/session/jsonl-storage.ts b/packages/agent/src/harness/session/jsonl-storage.ts index 54ad9eb03..4091a436c 100644 --- a/packages/agent/src/harness/session/jsonl-storage.ts +++ b/packages/agent/src/harness/session/jsonl-storage.ts @@ -40,10 +40,6 @@ function generateEntryId(byId: { has(id: string): boolean }): string { return uuidv7(); } -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null; -} - function invalidSession(filePath: string, message: string, cause?: Error): SessionError { return new SessionError("invalid_session", `Invalid JSONL session file ${filePath}: ${message}`, cause); } @@ -63,24 +59,27 @@ function parseHeaderLine(line: string, filePath: string): SessionHeader { } catch (error) { throw invalidSession(filePath, "first line is not a valid session header", toError(error)); } - if (!isRecord(parsed)) throw invalidSession(filePath, "first line is not a valid session header"); - if (parsed.type !== "session") throw invalidSession(filePath, "first line is not a valid session header"); - if (parsed.version !== 3) throw invalidSession(filePath, "unsupported session version"); - if (typeof parsed.id !== "string" || !parsed.id) throw invalidSession(filePath, "session header is missing id"); - if (typeof parsed.timestamp !== "string" || !parsed.timestamp) { + if (typeof parsed !== "object" || parsed === null) { + throw invalidSession(filePath, "first line is not a valid session header"); + } + const header = parsed as Partial; + if (header.type !== "session") throw invalidSession(filePath, "first line is not a valid session header"); + if (header.version !== 3) throw invalidSession(filePath, "unsupported session version"); + if (typeof header.id !== "string" || !header.id) throw invalidSession(filePath, "session header is missing id"); + if (typeof header.timestamp !== "string" || !header.timestamp) { throw invalidSession(filePath, "session header is missing timestamp"); } - if (typeof parsed.cwd !== "string" || !parsed.cwd) throw invalidSession(filePath, "session header is missing cwd"); - if (parsed.parentSession !== undefined && typeof parsed.parentSession !== "string") { + if (typeof header.cwd !== "string" || !header.cwd) throw invalidSession(filePath, "session header is missing cwd"); + if (header.parentSession !== undefined && typeof header.parentSession !== "string") { throw invalidSession(filePath, "session header parentSession must be a string"); } return { type: "session", version: 3, - id: parsed.id, - timestamp: parsed.timestamp, - cwd: parsed.cwd, - parentSession: parsed.parentSession, + id: header.id, + timestamp: header.timestamp, + cwd: header.cwd, + parentSession: header.parentSession, }; } @@ -91,19 +90,28 @@ function parseEntryLine(line: string, filePath: string, lineNumber: number): Ses } catch (error) { throw invalidEntry(filePath, lineNumber, "is not valid JSON", toError(error)); } - if (!isRecord(parsed)) throw invalidEntry(filePath, lineNumber, "is not a valid session entry"); - if (typeof parsed.type !== "string") throw invalidEntry(filePath, lineNumber, "is missing entry type"); - if (typeof parsed.id !== "string" || !parsed.id) throw invalidEntry(filePath, lineNumber, "is missing entry id"); - if (parsed.parentId !== null && typeof parsed.parentId !== "string") { + if (typeof parsed !== "object" || parsed === null) { + throw invalidEntry(filePath, lineNumber, "is not a valid session entry"); + } + const entry = parsed as { + type?: unknown; + id?: unknown; + parentId?: unknown; + timestamp?: unknown; + targetId?: unknown; + }; + if (typeof entry.type !== "string") throw invalidEntry(filePath, lineNumber, "is missing entry type"); + if (typeof entry.id !== "string" || !entry.id) throw invalidEntry(filePath, lineNumber, "is missing entry id"); + if (entry.parentId !== null && typeof entry.parentId !== "string") { throw invalidEntry(filePath, lineNumber, "has invalid parentId"); } - if (typeof parsed.timestamp !== "string" || !parsed.timestamp) { + if (typeof entry.timestamp !== "string" || !entry.timestamp) { throw invalidEntry(filePath, lineNumber, "is missing timestamp"); } - if (parsed.type === "leaf" && parsed.targetId !== null && typeof parsed.targetId !== "string") { + if (entry.type === "leaf" && entry.targetId !== null && typeof entry.targetId !== "string") { throw invalidEntry(filePath, lineNumber, "has invalid targetId"); } - return parsed as unknown as SessionTreeEntry; + return entry as SessionTreeEntry; } function leafIdAfterEntry(entry: SessionTreeEntry): string | null { diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index 060898c74..c122894c3 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -16,18 +16,6 @@ interface JsonSchemaObject { oneOf?: JsonSchemaObject[]; } -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null; -} - -function isJsonSchemaObject(value: unknown): value is JsonSchemaObject { - return isRecord(value); -} - -function hasTypeBoxMetadata(schema: unknown): boolean { - return isRecord(schema) && Object.getOwnPropertySymbols(schema).includes(TYPEBOX_KIND); -} - function getSchemaTypes(schema: JsonSchemaObject): string[] { if (typeof schema.type === "string") { return [schema.type]; @@ -53,22 +41,15 @@ function matchesJsonType(value: unknown, type: string): boolean { case "array": return Array.isArray(value); case "object": - return isRecord(value) && !Array.isArray(value); + return typeof value === "object" && value !== null && !Array.isArray(value); default: return false; } } -function isValidatorSchema(value: unknown): value is Tool["parameters"] { - return isRecord(value); -} - function getSubSchemaValidator(schema: JsonSchemaObject): ReturnType | undefined { - if (!isValidatorSchema(schema)) { - return undefined; - } try { - return getValidator(schema); + return getValidator(schema as Tool["parameters"]); } catch { return undefined; } @@ -161,7 +142,7 @@ function applySchemaObjectCoercion(value: Record, schema: JsonS } } - if (schema.additionalProperties && isJsonSchemaObject(schema.additionalProperties)) { + if (schema.additionalProperties && typeof schema.additionalProperties === "object") { for (const [key, propertyValue] of Object.entries(value)) { if (definedKeys.has(key)) { continue; @@ -183,7 +164,7 @@ function applySchemaArrayCoercion(value: unknown[], schema: JsonSchemaObject): v return; } - if (isJsonSchemaObject(schema.items)) { + if (schema.items && typeof schema.items === "object") { for (let index = 0; index < value.length; index++) { value[index] = coerceWithJsonSchema(value[index], schema.items); } @@ -232,8 +213,13 @@ function coerceWithJsonSchema(value: unknown, schema: JsonSchemaObject): unknown } } - if (schemaTypes.includes("object") && isRecord(nextValue) && !Array.isArray(nextValue)) { - applySchemaObjectCoercion(nextValue, schema); + if ( + schemaTypes.includes("object") && + typeof nextValue === "object" && + nextValue !== null && + !Array.isArray(nextValue) + ) { + applySchemaObjectCoercion(nextValue as Record, schema); } if (schemaTypes.includes("array") && Array.isArray(nextValue)) { @@ -294,10 +280,10 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): any { Value.Convert(tool.parameters, args); const validator = getValidator(tool.parameters); - if (!hasTypeBoxMetadata(tool.parameters) && isJsonSchemaObject(tool.parameters)) { - const coerced = coerceWithJsonSchema(args, tool.parameters); + if (!Object.getOwnPropertySymbols(tool.parameters).includes(TYPEBOX_KIND)) { + const coerced = coerceWithJsonSchema(args, tool.parameters as JsonSchemaObject); if (coerced !== args) { - if (isRecord(args) && isRecord(coerced)) { + if (typeof args === "object" && args !== null && typeof coerced === "object" && coerced !== null) { for (const key of Object.keys(args)) { delete args[key]; } diff --git a/packages/ai/test/openai-responses-tool-result-images.test.ts b/packages/ai/test/openai-responses-tool-result-images.test.ts index da33ab861..f881118b6 100644 --- a/packages/ai/test/openai-responses-tool-result-images.test.ts +++ b/packages/ai/test/openai-responses-tool-result-images.test.ts @@ -24,27 +24,13 @@ const getImageTool: Tool = { parameters: getImageSchema, }; -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null; -} - -function isResponsePayload(value: unknown): value is { input: unknown[] } { - return isRecord(value) && Array.isArray(value.input); -} - -function isFunctionCallOutputItem( - value: unknown, -): value is { type: "function_call_output"; output: string | ResponseFunctionCallOutputItemList } { - return isRecord(value) && value.type === "function_call_output" && "output" in value; -} - -function isInputTextItem(value: unknown): value is { type: "input_text"; text: string } { - return isRecord(value) && value.type === "input_text" && typeof value.text === "string"; -} - -function isInputImageItem(value: unknown): value is { type: "input_image"; image_url: string } { - return isRecord(value) && value.type === "input_image" && typeof value.image_url === "string"; -} +type CapturedResponsePayload = { input?: unknown[] }; +type FunctionCallOutputItem = { + type: "function_call_output"; + output: string | ResponseFunctionCallOutputItemList; +}; +type InputTextItem = { type: "input_text"; text: string }; +type InputImageItem = { type: "input_image"; image_url: string }; async function verifyToolResultImagesStayInFunctionCallOutput( model: Model, @@ -105,15 +91,19 @@ async function verifyToolResultImagesStayInFunctionCallOutput( expect(secondResponse.stopReason, `Error: ${secondResponse.errorMessage}`).toBe("stop"); expect(secondResponse.errorMessage).toBeFalsy(); - expect(isResponsePayload(capturedPayload)).toBe(true); - if (!isResponsePayload(capturedPayload)) { + const responsePayload = capturedPayload as CapturedResponsePayload | undefined; + expect(Array.isArray(responsePayload?.input)).toBe(true); + if (!Array.isArray(responsePayload?.input)) { throw new Error("Expected payload with input array"); } + const responseInput = responsePayload.input; - const functionCallOutputIndex = capturedPayload.input.findIndex((item) => isFunctionCallOutputItem(item)); + const functionCallOutputIndex = responseInput.findIndex( + (item) => (item as { type?: unknown } | null)?.type === "function_call_output", + ); expect(functionCallOutputIndex).toBeGreaterThanOrEqual(0); - const functionCallOutput = capturedPayload.input[functionCallOutputIndex]; - if (!isFunctionCallOutputItem(functionCallOutput)) { + const functionCallOutput = responseInput[functionCallOutputIndex] as FunctionCallOutputItem | undefined; + if (!functionCallOutput) { throw new Error("Expected function_call_output item"); } @@ -123,8 +113,12 @@ async function verifyToolResultImagesStayInFunctionCallOutput( } const outputItems = functionCallOutput.output; - const textItem = outputItems.find((item) => isInputTextItem(item)); - const imageItem = outputItems.find((item) => isInputImageItem(item)); + const textItem = outputItems.find((item) => (item as { type?: unknown } | null)?.type === "input_text") as + | InputTextItem + | undefined; + const imageItem = outputItems.find((item) => (item as { type?: unknown } | null)?.type === "input_image") as + | InputImageItem + | undefined; expect(textItem).toBeTruthy(); expect(imageItem).toBeTruthy(); @@ -135,9 +129,9 @@ async function verifyToolResultImagesStayInFunctionCallOutput( expect(textItem.text).toContain(toolText); expect(imageItem.image_url.startsWith("data:image/png;base64,")).toBe(true); - const laterUserMessages = capturedPayload.input + const laterUserMessages = responseInput .slice(functionCallOutputIndex + 1) - .filter((item) => isRecord(item) && item.role === "user"); + .filter((item) => (item as { role?: unknown } | null)?.role === "user"); expect(laterUserMessages).toHaveLength(0); const responseText = secondResponse.content diff --git a/packages/coding-agent/src/core/keybindings.ts b/packages/coding-agent/src/core/keybindings.ts index 7c0de6431..8231d9ef7 100644 --- a/packages/coding-agent/src/core/keybindings.ts +++ b/packages/coding-agent/src/core/keybindings.ts @@ -263,17 +263,11 @@ const KEYBINDING_NAME_MIGRATIONS = { deleteSessionNoninvasive: "app.session.deleteNoninvasive", } as const satisfies Record; -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} - function isLegacyKeybindingName(key: string): key is keyof typeof KEYBINDING_NAME_MIGRATIONS { return key in KEYBINDING_NAME_MIGRATIONS; } -function toKeybindingsConfig(value: unknown): KeybindingsConfig { - if (!isRecord(value)) return {}; - +function toKeybindingsConfig(value: Record): KeybindingsConfig { const config: KeybindingsConfig = {}; for (const [key, binding] of Object.entries(value)) { if (typeof binding === "string") { @@ -331,7 +325,8 @@ function loadRawConfig(path: string): Record | undefined { if (!existsSync(path)) return undefined; try { const parsed = JSON.parse(readFileSync(path, "utf-8")) as unknown; - return isRecord(parsed) ? parsed : undefined; + if (typeof parsed !== "object" || parsed === null) return undefined; + return parsed as Record; } catch { return undefined; }