Procházet zdrojové kódy

Merge pull request #625 from alvinunreal/codex/guard-opencode-message-transform

[codex] Guard OpenCode message transforms
Zaradacht Taifour (Zack) před 2 měsíci
rodič
revize
01bc5b845a

+ 18 - 0
src/hooks/filter-available-skills/index.test.ts

@@ -47,6 +47,24 @@ describe('filterAvailableSkillsText', () => {
 });
 
 describe('createFilterAvailableSkillsHook', () => {
+  test('ignores messages without OpenCode info or parts', async () => {
+    const hook = createFilterAvailableSkillsHook(mockCtx, {});
+    const output = {
+      messages: [
+        {},
+        { info: { role: 'assistant' } },
+        {
+          info: { role: 'system' },
+          parts: [{ type: 'text', text: availableSkillsBlock('skill1') }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, output as never);
+
+    expect(output.messages[2].parts[0].text).toContain('<name>skill1</name>');
+  });
+
   test('filters system prompt skill blocks for explicit agent skills', async () => {
     const config: PluginConfig = {
       agents: {

+ 10 - 4
src/hooks/filter-available-skills/index.ts

@@ -6,7 +6,11 @@
 import type { PluginInput } from '@opencode-ai/plugin';
 import { getSkillPermissionsForAgent } from '../../cli/skills';
 import { getAgentOverride, type PluginConfig } from '../../config';
-import type { MessageWithParts } from '../types';
+import {
+  isMessageWithParts,
+  isUserMessageWithParts,
+  type MessageWithParts,
+} from '../types';
 
 const AVAILABLE_SKILLS_BLOCK_REGEX =
   /<available_skills>\s*([\s\S]*?)\s*<\/available_skills>/g;
@@ -22,7 +26,7 @@ interface SkillEntry {
 function getCurrentAgent(messages: MessageWithParts[]): string {
   for (let index = messages.length - 1; index >= 0; index -= 1) {
     const message = messages[index];
-    if (message.info.role === 'user') {
+    if (isUserMessageWithParts(message)) {
       return message.info.agent ?? 'orchestrator';
     }
   }
@@ -113,9 +117,11 @@ export function createFilterAvailableSkillsHook(
   return {
     'experimental.chat.messages.transform': async (
       _input: Record<string, never>,
-      output: { messages: MessageWithParts[] },
+      output: { messages?: unknown },
     ): Promise<void> => {
-      const { messages } = output;
+      const messages = (Array.isArray(output.messages) ? output.messages : []).filter(
+        isMessageWithParts,
+      );
       if (messages.length === 0) {
         return;
       }

+ 45 - 1
src/hooks/foreground-fallback/index.test.ts

@@ -13,7 +13,7 @@ function createMockClient(overrides?: {
   promptAsyncImpl?: (args: unknown) => Promise<unknown>;
   abortImpl?: () => Promise<unknown>;
   includePromptAsync?: boolean;
-  messagesData?: Array<{ info: { role: string }; parts: unknown[] }>;
+  messagesData?: unknown[];
 }) {
   const promptAsync = mock(async (args: unknown) => {
     if (overrides?.promptAsyncImpl) return overrides.promptAsyncImpl(args);
@@ -174,6 +174,50 @@ describe('ForegroundFallbackManager session.error', () => {
     expect(call[0].body.model.modelID).toBe('gpt-4o');
   });
 
+  test('skips malformed messages without info when locating the last user message', async () => {
+    // OpenCode may return partial/streaming messages whose `info` is undefined;
+    // the fallback must ignore those rather than crash, and still re-submit the
+    // real last user message.
+    ({ client, mocks } = createMockClient({
+      messagesData: [
+        {},
+        { info: { role: 'assistant' }, parts: [] },
+        { parts: [{ type: 'text', text: 'no info' }] },
+        {
+          info: { role: 'user' },
+          parts: [{ type: 'text', text: 'real prompt' }],
+        },
+      ],
+    }));
+    mgr = new ForegroundFallbackManager(client, makeChains(), true);
+
+    await mgr.handleEvent({
+      type: 'message.updated',
+      properties: {
+        info: {
+          sessionID: 'sess-1',
+          providerID: 'anthropic',
+          modelID: 'claude-opus-4-5',
+          role: 'assistant',
+        },
+      },
+    });
+
+    await mgr.handleEvent({
+      type: 'session.error',
+      properties: {
+        sessionID: 'sess-1',
+        error: { message: 'Rate limit exceeded' },
+      },
+    });
+
+    expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
+    const call = mocks.promptAsync.mock.calls[0] as [
+      { body: { parts: Array<{ text?: string }> } },
+    ];
+    expect(call[0].body.parts[0]?.text).toBe('real prompt');
+  });
+
   test('does nothing when error is not a rate limit', async () => {
     await mgr.handleEvent({
       type: 'session.error',

+ 6 - 7
src/hooks/foreground-fallback/index.ts

@@ -21,6 +21,7 @@ import {
   abortSessionWithTimeout,
   parseModelReference,
 } from '../../utils/session';
+import { isUserMessageWithParts } from '../types';
 
 type OpencodeClient = PluginInput['client'];
 
@@ -284,13 +285,11 @@ export class ForegroundFallbackManager {
       const result = await this.client.session.messages({
         path: { id: sessionID },
       });
-      const messages = (result.data ?? []) as Array<{
-        info: { role: string };
-        parts: unknown[];
-      }>;
-      const lastUser = [...messages]
-        .reverse()
-        .find((m) => m.info.role === 'user');
+      // result.data may contain partial/streaming messages whose `info` is
+      // undefined at runtime (OpenCode violates its own declared type), so
+      // guard each entry instead of dereferencing `info` directly.
+      const messages = (result.data ?? []) as unknown[];
+      const lastUser = [...messages].reverse().find(isUserMessageWithParts);
       if (!lastUser) {
         log('[foreground-fallback] no user message found', { sessionID });
         return;

+ 2 - 2
src/hooks/image-hook.ts

@@ -9,7 +9,7 @@ import {
   writeFileSync,
 } from 'node:fs';
 import { basename, extname, join } from 'node:path';
-import type { MessageWithParts } from './types';
+import { isUserMessageWithParts, type MessageWithParts } from './types';
 
 // Debounce: only run cleanup every 10 minutes per directory
 const lastCleanupByDir = new Map<string, number>();
@@ -170,7 +170,7 @@ export function processImageAttachments(args: {
   }> = [];
 
   for (const msg of messages) {
-    if (msg.info.role !== 'user') continue;
+    if (!isUserMessageWithParts(msg)) continue;
     const imageParts = msg.parts.filter(isImagePart);
     if (imageParts.length > 0) {
       messagesWithImages.push({ msg, imageParts });

+ 34 - 0
src/hooks/phase-reminder/index.test.ts

@@ -121,6 +121,17 @@ describe('createPhaseReminderHook', () => {
     expect(output.messages).toEqual([]);
   });
 
+  test('handles missing or non-array messages', async () => {
+    const hook = createPhaseReminderHook();
+
+    await expect(
+      hook['experimental.chat.messages.transform']({}, {}),
+    ).resolves.toBeUndefined();
+    await expect(
+      hook['experimental.chat.messages.transform']({}, { messages: {} }),
+    ).resolves.toBeUndefined();
+  });
+
   test('handles no user messages', async () => {
     const hook = createPhaseReminderHook();
     const output = {
@@ -136,4 +147,27 @@ describe('createPhaseReminderHook', () => {
 
     expect(output.messages[0].parts[0].text).toBe('Hi');
   });
+
+  test('skips malformed messages while still appending to latest valid user message', async () => {
+    const hook = createPhaseReminderHook();
+    const output = {
+      messages: [
+        {},
+        { info: { role: 'assistant' } },
+        { parts: [{ type: 'text', text: 'missing info' }] },
+        {
+          info: { role: 'user', agent: 'orchestrator' },
+          parts: [{ type: 'text', text: 'hello' }],
+        },
+      ],
+    };
+
+    await expect(
+      hook['experimental.chat.messages.transform']({}, output as never),
+    ).resolves.toBeUndefined();
+
+    expect(output.messages[3].parts.length).toBe(2);
+    expect(output.messages[3].parts[0].text).toBe('hello');
+    expect(output.messages[3].parts[1].text).toBe(PHASE_REMINDER);
+  });
 });

+ 8 - 4
src/hooks/phase-reminder/index.ts

@@ -7,7 +7,7 @@
  */
 import { PHASE_REMINDER } from '../../config/constants';
 import { SLIM_INTERNAL_INITIATOR_MARKER } from '../../utils';
-import type { MessageWithParts } from '../types';
+import { isUserMessageWithParts, type MessageWithParts } from '../types';
 
 export { PHASE_REMINDER };
 
@@ -20,9 +20,9 @@ export function createPhaseReminderHook() {
   return {
     'experimental.chat.messages.transform': async (
       _input: Record<string, never>,
-      output: { messages: MessageWithParts[] },
+      output: { messages?: unknown },
     ): Promise<void> => {
-      const { messages } = output;
+      const messages = Array.isArray(output.messages) ? output.messages : [];
 
       if (messages.length === 0) {
         return;
@@ -30,7 +30,7 @@ export function createPhaseReminderHook() {
 
       let lastUserMessageIndex = -1;
       for (let i = messages.length - 1; i >= 0; i--) {
-        if (messages[i].info.role === 'user') {
+        if (isUserMessageWithParts(messages[i])) {
           lastUserMessageIndex = i;
           break;
         }
@@ -41,6 +41,10 @@ export function createPhaseReminderHook() {
       }
 
       const lastUserMessage = messages[lastUserMessageIndex];
+      if (!isUserMessageWithParts(lastUserMessage)) {
+        return;
+      }
+
       const agent = lastUserMessage.info.agent;
       if (agent && agent !== 'orchestrator') {
         return;

+ 36 - 0
src/hooks/task-session-manager/index.test.ts

@@ -43,6 +43,42 @@ function createMessages(sessionID: string, text = 'user message') {
 }
 
 describe('task-session-manager hook', () => {
+  test('ignores messages without OpenCode info or parts', async () => {
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map scheduler hooks',
+    });
+    const { hook } = createHook({ backgroundJobBoard: board });
+    const messages = {
+      messages: [
+        {},
+        { info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' } },
+        { parts: [{ type: 'text', text: 'missing info' }] },
+        {
+          info: { role: 'assistant' },
+          parts: [{ type: 'text', text: 'assistant response' }],
+        },
+        {
+          info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' },
+          parts: [{ type: 'text', text: 'valid user message' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, messages as never);
+
+    expect(messages.messages).toHaveLength(5);
+    expect(messages.messages[4].parts[0].text).toContain(
+      '### Background Job Board',
+    );
+    expect(messages.messages[4].parts[0].text).toContain(
+      'exp-1 / child-1 / explorer / running',
+    );
+  });
+
   test('stores background task launches in job board prompt context', async () => {
     const board = new BackgroundJobBoard();
     const { hook } = createHook({ backgroundJobBoard: board });

+ 13 - 7
src/hooks/task-session-manager/index.ts

@@ -13,7 +13,11 @@ import {
 import { isRecord as isObjectRecord } from '../../utils/guards';
 import { log } from '../../utils/logger';
 import { isRateLimitError } from '../foreground-fallback/index';
-import type { MessagePart, MessageWithParts } from '../types';
+import {
+  isUserMessageWithParts,
+  type MessagePart,
+  type MessageWithParts,
+} from '../types';
 
 interface TaskArgs {
   description?: unknown;
@@ -639,10 +643,12 @@ export function createTaskSessionManagerHook(
 
     'experimental.chat.messages.transform': async (
       _input: Record<string, never>,
-      output: { messages: MessageWithParts[] },
+      output: { messages?: unknown },
     ): Promise<void> => {
-      for (const [messageIndex, message] of output.messages.entries()) {
-        if (message.info.role !== 'user') continue;
+      const messages = Array.isArray(output.messages) ? output.messages : [];
+
+      for (const [messageIndex, message] of messages.entries()) {
+        if (!isUserMessageWithParts(message)) continue;
         if (message.info.agent && message.info.agent !== 'orchestrator') {
           continue;
         }
@@ -658,9 +664,9 @@ export function createTaskSessionManagerHook(
         }
       }
 
-      for (let i = output.messages.length - 1; i >= 0; i -= 1) {
-        const message = output.messages[i];
-        if (message.info.role !== 'user') continue;
+      for (let i = messages.length - 1; i >= 0; i -= 1) {
+        const message = messages[i];
+        if (!isUserMessageWithParts(message)) continue;
         if (message.info.agent && message.info.agent !== 'orchestrator') return;
         if (
           !message.info.sessionID ||

+ 20 - 0
src/hooks/types.ts

@@ -24,3 +24,23 @@ export type MessageWithParts = {
   info: MessageInfo;
   parts: MessagePart[];
 };
+
+export function isMessageWithParts(message: unknown): message is MessageWithParts {
+  if (!message || typeof message !== 'object') {
+    return false;
+  }
+
+  const candidate = message as Partial<MessageWithParts>;
+  return (
+    !!candidate.info &&
+    typeof candidate.info === 'object' &&
+    typeof candidate.info.role === 'string' &&
+    Array.isArray(candidate.parts)
+  );
+}
+
+export function isUserMessageWithParts(
+  message: unknown,
+): message is MessageWithParts {
+  return isMessageWithParts(message) && message.info.role === 'user';
+}

+ 15 - 9
src/index.ts

@@ -32,7 +32,11 @@ import {
   ForegroundFallbackManager,
 } from './hooks';
 import { processImageAttachments } from './hooks/image-hook';
-import type { MessageWithParts } from './hooks/types';
+import {
+  isMessageWithParts,
+  isUserMessageWithParts,
+  type MessageWithParts,
+} from './hooks/types';
 import { createInterviewManager } from './interview';
 import { createBuiltinMcps } from './mcp';
 import {
@@ -1031,12 +1035,14 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
     // API (doesn't show in UI)
     'experimental.chat.messages.transform': async (
       input: Record<string, never>,
-      output: { messages: unknown[] },
+      output: { messages?: unknown },
     ): Promise<void> => {
-      const typedOutput = output as { messages: MessageWithParts[] };
+      const messages = (Array.isArray(output.messages) ? output.messages : []).filter(
+        isMessageWithParts,
+      );
 
-      for (const message of typedOutput.messages) {
-        if (message.info.role !== 'user') {
+      for (const message of messages) {
+        if (!isUserMessageWithParts(message)) {
           continue;
         }
         for (const part of message.parts) {
@@ -1053,7 +1059,7 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       // image bytes with a text nudge so the orchestrator delegates to
       // @observer instead.
       processImageAttachments({
-        messages: typedOutput.messages,
+        messages,
         workDir: ctx.directory,
         disabledAgents,
         log,
@@ -1061,15 +1067,15 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
 
       await taskSessionManagerHook['experimental.chat.messages.transform'](
         input,
-        typedOutput,
+        { messages },
       );
       await phaseReminderHook['experimental.chat.messages.transform'](
         input,
-        typedOutput,
+        { messages },
       );
       await filterAvailableSkillsHook['experimental.chat.messages.transform'](
         input,
-        typedOutput,
+        { messages },
       );
     },