fix(agentic-kit): harden generate and tool ids

Author: yyyyaaa

packages/agentic-kit/__tests__/adapter.test.ts

@@ -1,5 +1,6 @@
 import {
   AgentKit,
+  type AssistantMessage,
   createAssistantMessageEventStream,
   getMessageText,
   type ModelDescriptor,
@@ -20,6 +21,29 @@ function createFakeModel(): ModelDescriptor {
   };
 }
 
+function createAssistantMessage(
+  overrides: Partial<AssistantMessage> = {}
+): AssistantMessage {
+  return {
+    role: 'assistant',
+    api: 'fake-api',
+    provider: 'fake',
+    model: 'demo',
+    usage: {
+      input: 0,
+      output: 0,
+      cacheRead: 0,
+      cacheWrite: 0,
+      totalTokens: 0,
+      cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
+    },
+    stopReason: 'stop',
+    timestamp: Date.now(),
+    content: [{ type: 'text', text: 'hello world' }],
+    ...overrides,
+  };
+}
+
 describe('agentic-kit core', () => {
   it('transforms cross-provider thinking and inserts orphaned tool results', () => {
     const sourceModel = createFakeModel();
@@ -150,6 +174,67 @@ describe('agentic-kit core', () => {
     });
   });
 
+  it('normalizes short mistral tool-call ids without hanging and keeps tool results aligned', () => {
+    const sourceModel = createFakeModel();
+    const targetModel: ModelDescriptor = {
+      ...sourceModel,
+      provider: 'mistral',
+      api: 'openai-compatible',
+      id: 'mistral-demo',
+      compat: {
+        toolCallIdFormat: 'mistral9',
+      },
+    };
+
+    const transformed = transformMessages(
+      [
+        {
+          ...createAssistantMessage({
+            api: sourceModel.api,
+            provider: sourceModel.provider,
+            model: sourceModel.id,
+            stopReason: 'toolUse',
+            content: [{ type: 'toolCall', id: '%', name: 'lookup', arguments: { city: 'Paris' } }],
+          }),
+        },
+        {
+          role: 'toolResult',
+          toolCallId: '%',
+          toolName: 'lookup',
+          content: [{ type: 'text', text: 'ok' }],
+          isError: false,
+          timestamp: Date.now(),
+        },
+      ],
+      targetModel
+    );
+
+    expect(transformed).toHaveLength(2);
+    expect(transformed[0]?.role).toBe('assistant');
+    expect(transformed[1]).toMatchObject({
+      role: 'toolResult',
+      toolName: 'lookup',
+      isError: false,
+    });
+
+    const assistant = transformed[0];
+    if (!assistant || assistant.role !== 'assistant') {
+      throw new Error('Expected assistant message');
+    }
+
+    const toolCall = assistant.content[0];
+    if (!toolCall || toolCall.type !== 'toolCall') {
+      throw new Error('Expected tool call content');
+    }
+
+    expect(toolCall.id).toMatch(/^[A-Za-z0-9]{9}$/);
+    expect(toolCall.id).not.toContain('_');
+    expect(transformed[1]).toMatchObject({
+      role: 'toolResult',
+      toolCallId: toolCall.id,
+    });
+  });
+
   it('keeps the legacy AgentKit generate API working through structured streams', async () => {
     const provider: ProviderAdapter & { name: string } = {
       api: 'fake-api',
@@ -158,23 +243,7 @@ describe('agentic-kit core', () => {
       createModel: () => createFakeModel(),
       stream: () => {
         const stream = createAssistantMessageEventStream();
-        const message = {
-          role: 'assistant' as const,
-          api: 'fake-api',
-          provider: 'fake',
-          model: 'demo',
-          usage: {
-            input: 0,
-            output: 0,
-            cacheRead: 0,
-            cacheWrite: 0,
-            totalTokens: 0,
-            cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
-          },
-          stopReason: 'stop' as const,
-          timestamp: Date.now(),
-          content: [{ type: 'text' as const, text: 'hello world' }],
-        };
+        const message = createAssistantMessage();
 
         queueMicrotask(() => {
           stream.push({ type: 'start', partial: { ...message, content: [{ type: 'text', text: '' }] } });
@@ -214,6 +283,108 @@ describe('agentic-kit core', () => {
     await expect(kit.generate({ model: 'demo', prompt: 'hi' })).resolves.toBe('hello world');
   });
 
+  it('rejects legacy generate when a provider returns a terminal error in non-stream mode', async () => {
+    const provider: ProviderAdapter & { name: string } = {
+      api: 'fake-api',
+      provider: 'fake',
+      name: 'fake',
+      createModel: () => createFakeModel(),
+      stream: () => {
+        const stream = createAssistantMessageEventStream();
+        const failure = createAssistantMessage({
+          stopReason: 'error',
+          errorMessage: 'provider failed',
+          content: [{ type: 'text', text: '' }],
+        });
+
+        queueMicrotask(() => {
+          stream.push({ type: 'error', reason: 'error', error: failure });
+          stream.end(failure);
+        });
+
+        return stream;
+      },
+    };
+
+    const kit = new AgentKit().addProvider(provider);
+    const onComplete = jest.fn();
+    const onError = jest.fn();
+    const onStateChange = jest.fn();
+
+    await expect(
+      kit.generate(
+        { model: 'demo', prompt: 'hi' },
+        { onComplete, onError, onStateChange }
+      )
+    ).rejects.toThrow('provider failed');
+
+    expect(onError).toHaveBeenCalledTimes(1);
+    expect(onComplete).not.toHaveBeenCalled();
+    expect(onStateChange).not.toHaveBeenCalledWith('complete');
+  });
+
+  it('rejects legacy generate when a provider returns a terminal error in stream mode', async () => {
+    const provider: ProviderAdapter & { name: string } = {
+      api: 'fake-api',
+      provider: 'fake',
+      name: 'fake',
+      createModel: () => createFakeModel(),
+      stream: () => {
+        const stream = createAssistantMessageEventStream();
+        const partial = createAssistantMessage({
+          content: [{ type: 'text', text: 'partial' }],
+        });
+        const failure = createAssistantMessage({
+          stopReason: 'error',
+          errorMessage: 'provider failed',
+          content: [{ type: 'text', text: 'partial' }],
+        });
+
+        queueMicrotask(() => {
+          stream.push({ type: 'start', partial: { ...partial, content: [{ type: 'text', text: '' }] } });
+          stream.push({
+            type: 'text_start',
+            contentIndex: 0,
+            partial: { ...partial, content: [{ type: 'text', text: '' }] },
+          });
+          stream.push({
+            type: 'text_delta',
+            contentIndex: 0,
+            delta: 'partial',
+            partial,
+          });
+          stream.push({ type: 'error', reason: 'error', error: failure });
+          stream.end(failure);
+        });
+
+        return stream;
+      },
+    };
+
+    const kit = new AgentKit().addProvider(provider);
+    const chunks: string[] = [];
+    const onComplete = jest.fn();
+    const onError = jest.fn();
+    const onStateChange = jest.fn();
+
+    await expect(
+      kit.generate(
+        { model: 'demo', prompt: 'hi', stream: true },
+        {
+          onChunk: (chunk) => chunks.push(chunk),
+          onComplete,
+          onError,
+          onStateChange,
+        }
+      )
+    ).rejects.toThrow('provider failed');
+
+    expect(chunks).toEqual(['partial']);
+    expect(onStateChange).toHaveBeenCalledWith('streaming');
+    expect(onComplete).not.toHaveBeenCalled();
+    expect(onError).toHaveBeenCalledTimes(1);
+  });
+
   it('extracts assistant text from mixed content blocks', () => {
     const text = getMessageText({
       role: 'assistant',

packages/agentic-kit/src/index.ts

@@ -141,6 +141,7 @@ export class AgentKit {
             options?.onChunk?.(event.delta);
           }
         }
+        assertLegacyGenerateSucceeded(await response.result());
         options?.onComplete?.();
       } catch (error) {
         options?.onError?.(error as Error);
@@ -150,10 +151,11 @@ export class AgentKit {
       return;
     }
 
-    options?.onStateChange?.('complete');
-
     try {
-      const message = await completeWithProvider(provider, model, context, streamOptions);
+      const message = assertLegacyGenerateSucceeded(
+        await completeWithProvider(provider, model, context, streamOptions)
+      );
+      options?.onStateChange?.('complete');
       const text = getMessageText(message);
       options?.onComplete?.();
       return text;
@@ -294,3 +296,15 @@ async function completeWithProvider(
 function getProviderName(provider: NamedProviderAdapter): string {
   return provider.name ?? provider.provider;
 }
+
+function assertLegacyGenerateSucceeded(message: AssistantMessage): AssistantMessage {
+  if (message.stopReason === 'error' || message.stopReason === 'aborted') {
+    throw new Error(
+      message.errorMessage ??
+        (message.stopReason === 'aborted' ? 'Generation aborted.' : 'Generation failed.'),
+      { cause: message }
+    );
+  }
+
+  return message;
+}

packages/agentic-kit/src/transform-messages.ts

@@ -141,7 +141,7 @@ function normalizeToolCallId(id: string, model: ModelDescriptor): string {
       return alphanumeric.slice(0, 9);
     }
 
-    return stableId(id, 9, /^[a-zA-Z0-9]+$/);
+    return stableAlphanumericId(id, 9);
   }
 
   const safe = id.replace(/[^a-zA-Z0-9_-]/g, '_').slice(0, 64);
@@ -177,3 +177,17 @@ function stableId(input: string, length: number, pattern: RegExp): string {
 
   return value.padEnd(length, '0');
 }
+
+function stableAlphanumericId(input: string, length: number): string {
+  let hash = 0;
+  for (let i = 0; i < input.length; i += 1) {
+    hash = (hash * 31 + input.charCodeAt(i)) >>> 0;
+  }
+
+  const value = `tc${hash.toString(36)}`.replace(/[^a-zA-Z0-9]/g, '');
+  if (value.length >= length) {
+    return value.slice(0, length);
+  }
+
+  return value.padEnd(length, '0');
+}