From 799a990ade18cf596ef9c58e684f042ea2e85765 Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 27 Jan 2026 23:21:38 +0100 Subject: [PATCH] refactor(coding-agent): extracted type guard functions for improved type safety and reusability - Extracted type guard functions for improved type safety and code reusability across RPC and migration modules. - Centralized AgentEvent type validation into a dedicated Set constant and type guard function in rpc-client. - Replaced inline type assertions with type guard functions in RPC response and event handling logic. - Refactored test utilities to use parseSessionEntries helper and type guard functions for cleaner test code. - Added reasoning content configuration properties to OpenAI compatibility settings. --- ...nai-completions-tool-result-images.test.ts | 3 ++ packages/coding-agent/src/migrations.ts | 18 +++++-- .../coding-agent/src/modes/rpc/rpc-client.ts | 54 ++++++++++++++++--- packages/coding-agent/test/rpc.test.ts | 49 ++++++++++++----- 4 files changed, 101 insertions(+), 23 deletions(-) diff --git a/packages/ai/test/openai-completions-tool-result-images.test.ts b/packages/ai/test/openai-completions-tool-result-images.test.ts index c903a3f19..85bed2deb 100644 --- a/packages/ai/test/openai-completions-tool-result-images.test.ts +++ b/packages/ai/test/openai-completions-tool-result-images.test.ts @@ -23,6 +23,9 @@ const compat: Required = { requiresThinkingAsText: false, requiresMistralToolIds: false, thinkingFormat: "openai", + reasoningContentField: "reasoning_content", + requiresReasoningContentForToolCalls: false, + requiresAssistantContentForToolCalls: false, openRouterRouting: {}, }; diff --git a/packages/coding-agent/src/migrations.ts b/packages/coding-agent/src/migrations.ts index 4340a6f03..5376557d0 100644 --- a/packages/coding-agent/src/migrations.ts +++ b/packages/coding-agent/src/migrations.ts @@ -9,6 +9,17 @@ import { getAgentDbPath, getAgentDir, getBinDir } from "./config"; import { AgentStorage } from "./session/agent-storage"; import type { AuthCredential } from "./session/auth-storage"; +type SessionHeader = { + type: "session"; + cwd: string; +}; + +function isSessionHeader(value: unknown): value is SessionHeader { + if (!value || typeof value !== "object") return false; + const record = value as Record; + return record.type === "session" && typeof record.cwd === "string" && record.cwd.length > 0; +} + /** * Migrate legacy oauth.json and settings.json apiKeys to agent.db. * @@ -107,12 +118,11 @@ export async function migrateSessionsFromAgentRoot(): Promise { if (isEnoent(err)) continue; throw err; } - const entries = Bun.JSONL.parse(content); + const entries: unknown[] = Bun.JSONL.parse(content); const header = entries[0]; - if (!header) continue; - if (header.type !== "session" || !header.cwd) continue; + if (!isSessionHeader(header)) continue; - const cwd: string = header.cwd; + const cwd = header.cwd; // Compute the correct session directory (same encoding as session-manager.ts) const safePath = `--${cwd.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-")}--`; diff --git a/packages/coding-agent/src/modes/rpc/rpc-client.ts b/packages/coding-agent/src/modes/rpc/rpc-client.ts index c69c047e7..71089fcc7 100644 --- a/packages/coding-agent/src/modes/rpc/rpc-client.ts +++ b/packages/coding-agent/src/modes/rpc/rpc-client.ts @@ -41,6 +41,42 @@ export interface ModelInfo { export type RpcEventListener = (event: AgentEvent) => void; +const agentEventTypes = new Set([ + "agent_start", + "agent_end", + "turn_start", + "turn_end", + "message_start", + "message_update", + "message_end", + "tool_execution_start", + "tool_execution_update", + "tool_execution_end", +]); + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function isRpcResponse(value: unknown): value is RpcResponse { + if (!isRecord(value)) return false; + if (value.type !== "response") return false; + if (typeof value.command !== "string") return false; + if (typeof value.success !== "boolean") return false; + if (value.id !== undefined && typeof value.id !== "string") return false; + if (value.success === false) { + return typeof value.error === "string"; + } + return true; +} + +function isAgentEvent(value: unknown): value is AgentEvent { + if (!isRecord(value)) return false; + const type = value.type; + if (typeof type !== "string") return false; + return agentEventTypes.has(type as AgentEvent["type"]); +} + // ============================================================================ // RPC Client // ============================================================================ @@ -434,16 +470,22 @@ export class RpcClient { for (const data of result.values) { // Check if it's a response to a pending request - if (data.type === "response" && data.id && this.pendingRequests.has(data.id)) { - const pending = this.pendingRequests.get(data.id)!; - this.pendingRequests.delete(data.id); - pending.resolve(data as RpcResponse); - return; + if (isRpcResponse(data)) { + const id = data.id; + if (id && this.pendingRequests.has(id)) { + const pending = this.pendingRequests.get(id)!; + this.pendingRequests.delete(id); + pending.resolve(data); + return; + } + continue; } + if (!isAgentEvent(data)) continue; + // Otherwise it's an event for (const listener of this.eventListeners) { - listener(data as AgentEvent); + listener(data); } } } diff --git a/packages/coding-agent/test/rpc.test.ts b/packages/coding-agent/test/rpc.test.ts index 2f52e2029..1475228bf 100644 --- a/packages/coding-agent/test/rpc.test.ts +++ b/packages/coding-agent/test/rpc.test.ts @@ -2,10 +2,28 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; -import type { AgentEvent } from "@oh-my-pi/pi-agent-core"; +import type { AgentEvent, AgentMessage } from "@oh-my-pi/pi-agent-core"; +import type { AssistantMessage, TextContent } from "@oh-my-pi/pi-ai"; +import { + type CompactionEntry, + type FileEntry, + parseSessionEntries, + type SessionMessageEntry, +} from "@oh-my-pi/pi-coding-agent"; import { RpcClient } from "@oh-my-pi/pi-coding-agent/modes/rpc/rpc-client"; +import type { BashExecutionMessage } from "@oh-my-pi/pi-coding-agent/session/messages"; import { nanoid } from "nanoid"; +type MessageEndEvent = Extract; + +const isMessageEndEvent = (event: AgentEvent): event is MessageEndEvent => event.type === "message_end"; + +const isAssistantMessage = (message: AgentMessage): message is AssistantMessage => message.role === "assistant"; + +const isSessionMessageEntry = (entry: FileEntry): entry is SessionMessageEntry => entry.type === "message"; + +const isCompactionEntry = (entry: FileEntry): entry is CompactionEntry => entry.type === "compaction"; + /** * RPC mode tests. */ @@ -67,16 +85,16 @@ describe.skipIf(!process.env.ANTHROPIC_API_KEY && !process.env.ANTHROPIC_OAUTH_T expect(sessionFiles.length).toBe(1); const sessionContent = await Bun.file(path.join(cwdSessionDir, sessionFiles[0])).text(); - const entries = Bun.JSONL.parse(sessionContent); + const entries = parseSessionEntries(sessionContent); // First entry should be session header expect(entries[0].type).toBe("session"); // Should have user and assistant messages - const messages = entries.filter((e: { type: string }) => e.type === "message"); + const messages = entries.filter(isSessionMessageEntry); expect(messages.length).toBeGreaterThanOrEqual(2); - const roles = messages.map((m: { message: { role: string } }) => m.message.role); + const roles = messages.map(message => message.message.role); expect(roles).toContain("user"); expect(roles).toContain("assistant"); }, 90000); @@ -101,9 +119,9 @@ describe.skipIf(!process.env.ANTHROPIC_API_KEY && !process.env.ANTHROPIC_OAUTH_T const cwdSessionDir = path.join(sessionsPath, sessionDirs[0]); const sessionFiles = fs.readdirSync(cwdSessionDir).filter(f => f.endsWith(".jsonl")); const sessionContent = await Bun.file(path.join(cwdSessionDir, sessionFiles[0])).text(); - const entries = Bun.JSONL.parse(sessionContent); + const entries = parseSessionEntries(sessionContent); - const compactionEntries = entries.filter((e: { type: string }) => e.type === "compaction"); + const compactionEntries = entries.filter(isCompactionEntry); expect(compactionEntries.length).toBe(1); expect(compactionEntries[0].summary).toBeDefined(); }, 120000); @@ -136,11 +154,11 @@ describe.skipIf(!process.env.ANTHROPIC_API_KEY && !process.env.ANTHROPIC_OAUTH_T const cwdSessionDir = path.join(sessionsPath, sessionDirs[0]); const sessionFiles = fs.readdirSync(cwdSessionDir).filter(f => f.endsWith(".jsonl")); const sessionContent = await Bun.file(path.join(cwdSessionDir, sessionFiles[0])).text(); - const entries = Bun.JSONL.parse(sessionContent); + const entries = parseSessionEntries(sessionContent); const bashMessages = entries.filter( - (e: { type: string; message?: { role: string } }) => - e.type === "message" && e.message?.role === "bashExecution", + (entry): entry is SessionMessageEntry & { message: BashExecutionMessage } => + isSessionMessageEntry(entry) && entry.message.role === "bashExecution", ); expect(bashMessages.length).toBe(1); expect(bashMessages[0].message.output).toContain(uniqueValue); @@ -159,14 +177,19 @@ describe.skipIf(!process.env.ANTHROPIC_API_KEY && !process.env.ANTHROPIC_OAUTH_T ); // Find assistant's response - const messageEndEvents = events.filter(e => e.type === "message_end") as AgentEvent[]; + const messageEndEvents = events.filter(isMessageEndEvent); const assistantMessage = messageEndEvents.find( - e => e.type === "message_end" && e.message?.role === "assistant", - ) as any; + (event): event is MessageEndEvent & { message: AssistantMessage } => isAssistantMessage(event.message), + ); expect(assistantMessage).toBeDefined(); + if (!assistantMessage) { + throw new Error("Expected assistant message_end event"); + } - const textContent = assistantMessage.message.content.find((c: any) => c.type === "text"); + const textContent = assistantMessage.message.content.find( + (content): content is TextContent => content.type === "text", + ); expect(textContent?.text).toContain(uniqueValue); }, 90000);