MAESTRO: fix MCP type coercion for batch endpoints, add defensive observation error handling

Add string-to-array coercion for ids and memorySessionIds in DataRoutes.ts
batch endpoints so MCP clients sending "[1,2,3]" or "1,2,3" instead of
native arrays no longer get 400 errors. Wrap observation storage path in
SessionRoutes.ts with try/catch returning 200 on recoverable errors instead
of 500, preventing hook breakage.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

src/services/worker/http/routes/DataRoutes.ts

@@ -114,7 +114,12 @@ export class DataRoutes extends BaseRouteHandler {
    * Body: { ids: number[], orderBy?: 'date_desc' | 'date_asc', limit?: number, project?: string }
    */
   private handleGetObservationsByIds = this.wrapHandler((req: Request, res: Response): void => {
-    const { ids, orderBy, limit, project } = req.body;
+    let { ids, orderBy, limit, project } = req.body;
+
+    // Coerce string-encoded arrays from MCP clients (e.g. "[1,2,3]" or "1,2,3")
+    if (typeof ids === 'string') {
+      try { ids = JSON.parse(ids); } catch { ids = ids.split(',').map(Number); }
+    }
 
     if (!ids || !Array.isArray(ids)) {
       this.badRequest(res, 'ids must be an array of numbers');
@@ -163,7 +168,12 @@ export class DataRoutes extends BaseRouteHandler {
    * Body: { memorySessionIds: string[] }
    */
   private handleGetSdkSessionsByIds = this.wrapHandler((req: Request, res: Response): void => {
-    const { memorySessionIds } = req.body;
+    let { memorySessionIds } = req.body;
+
+    // Coerce string-encoded arrays from MCP clients (e.g. '["a","b"]' or "a,b")
+    if (typeof memorySessionIds === 'string') {
+      try { memorySessionIds = JSON.parse(memorySessionIds); } catch { memorySessionIds = memorySessionIds.split(',').map((s: string) => s.trim()); }
+    }
 
     if (!Array.isArray(memorySessionIds)) {
       this.badRequest(res, 'memorySessionIds must be an array');

src/services/worker/http/routes/SessionRoutes.ts

@@ -504,57 +504,63 @@ export class SessionRoutes extends BaseRouteHandler {
       }
     }
 
-    const store = this.dbManager.getSessionStore();
+    try {
+      const store = this.dbManager.getSessionStore();
 
-    // Get or create session
-    const sessionDbId = store.createSDKSession(contentSessionId, '', '');
-    const promptNumber = store.getPromptNumberFromUserPrompts(contentSessionId);
+      // Get or create session
+      const sessionDbId = store.createSDKSession(contentSessionId, '', '');
+      const promptNumber = store.getPromptNumberFromUserPrompts(contentSessionId);
 
-    // Privacy check: skip if user prompt was entirely private
-    const userPrompt = PrivacyCheckValidator.checkUserPromptPrivacy(
-      store,
-      contentSessionId,
-      promptNumber,
-      'observation',
-      sessionDbId,
-      { tool_name }
-    );
-    if (!userPrompt) {
-      res.json({ status: 'skipped', reason: 'private' });
-      return;
+      // Privacy check: skip if user prompt was entirely private
+      const userPrompt = PrivacyCheckValidator.checkUserPromptPrivacy(
+        store,
+        contentSessionId,
+        promptNumber,
+        'observation',
+        sessionDbId,
+        { tool_name }
+      );
+      if (!userPrompt) {
+        res.json({ status: 'skipped', reason: 'private' });
+        return;
+      }
+
+      // Strip memory tags from tool_input and tool_response
+      const cleanedToolInput = tool_input !== undefined
+        ? stripMemoryTagsFromJson(JSON.stringify(tool_input))
+        : '{}';
+
+      const cleanedToolResponse = tool_response !== undefined
+        ? stripMemoryTagsFromJson(JSON.stringify(tool_response))
+        : '{}';
+
+      // Queue observation
+      this.sessionManager.queueObservation(sessionDbId, {
+        tool_name,
+        tool_input: cleanedToolInput,
+        tool_response: cleanedToolResponse,
+        prompt_number: promptNumber,
+        cwd: cwd || (() => {
+          logger.error('SESSION', 'Missing cwd when queueing observation in SessionRoutes', {
+            sessionId: sessionDbId,
+            tool_name
+          });
+          return '';
+        })()
+      });
+
+      // Ensure SDK agent is running
+      this.ensureGeneratorRunning(sessionDbId, 'observation');
+
+      // Broadcast observation queued event
+      this.eventBroadcaster.broadcastObservationQueued(sessionDbId);
+
+      res.json({ status: 'queued' });
+    } catch (error) {
+      // Return 200 on recoverable errors so the hook doesn't break
+      logger.error('SESSION', 'Observation storage failed', { contentSessionId, tool_name }, error as Error);
+      res.json({ stored: false, reason: (error as Error).message });
     }
-
-    // Strip memory tags from tool_input and tool_response
-    const cleanedToolInput = tool_input !== undefined
-      ? stripMemoryTagsFromJson(JSON.stringify(tool_input))
-      : '{}';
-
-    const cleanedToolResponse = tool_response !== undefined
-      ? stripMemoryTagsFromJson(JSON.stringify(tool_response))
-      : '{}';
-
-    // Queue observation
-    this.sessionManager.queueObservation(sessionDbId, {
-      tool_name,
-      tool_input: cleanedToolInput,
-      tool_response: cleanedToolResponse,
-      prompt_number: promptNumber,
-      cwd: cwd || (() => {
-        logger.error('SESSION', 'Missing cwd when queueing observation in SessionRoutes', {
-          sessionId: sessionDbId,
-          tool_name
-        });
-        return '';
-      })()
-    });
-
-    // Ensure SDK agent is running
-    this.ensureGeneratorRunning(sessionDbId, 'observation');
-
-    // Broadcast observation queued event
-    this.eventBroadcaster.broadcastObservationQueued(sessionDbId);
-
-    res.json({ status: 'queued' });
   });
 
   /**

tests/worker/http/routes/data-routes-coercion.test.ts

@@ -0,0 +1,195 @@
+/**
+ * DataRoutes Type Coercion Tests
+ *
+ * Tests that MCP clients sending string-encoded arrays for `ids` and
+ * `memorySessionIds` are properly coerced before validation.
+ *
+ * Mock Justification:
+ * - Express req/res mocks: Required because route handlers expect Express objects
+ * - DatabaseManager/SessionStore: Avoids database setup; we test coercion logic, not queries
+ * - Logger spies: Suppress console output during tests
+ */
+
+import { describe, it, expect, mock, beforeEach, afterEach, spyOn } from 'bun:test';
+import type { Request, Response } from 'express';
+import { logger } from '../../../../src/utils/logger.js';
+
+// Mock dependencies before importing DataRoutes
+mock.module('../../../../src/shared/paths.js', () => ({
+  getPackageRoot: () => '/tmp/test',
+}));
+mock.module('../../../../src/shared/worker-utils.js', () => ({
+  getWorkerPort: () => 37777,
+}));
+
+import { DataRoutes } from '../../../../src/services/worker/http/routes/DataRoutes.js';
+
+let loggerSpies: ReturnType<typeof spyOn>[] = [];
+
+// Helper to create mock req/res
+function createMockReqRes(body: any): { req: Partial<Request>; res: Partial<Response>; jsonSpy: ReturnType<typeof mock>; statusSpy: ReturnType<typeof mock> } {
+  const jsonSpy = mock(() => {});
+  const statusSpy = mock(() => ({ json: jsonSpy }));
+  return {
+    req: { body, path: '/test', query: {} } as Partial<Request>,
+    res: { json: jsonSpy, status: statusSpy } as unknown as Partial<Response>,
+    jsonSpy,
+    statusSpy,
+  };
+}
+
+describe('DataRoutes Type Coercion', () => {
+  let routes: DataRoutes;
+  let mockGetObservationsByIds: ReturnType<typeof mock>;
+  let mockGetSdkSessionsBySessionIds: ReturnType<typeof mock>;
+
+  beforeEach(() => {
+    loggerSpies = [
+      spyOn(logger, 'info').mockImplementation(() => {}),
+      spyOn(logger, 'debug').mockImplementation(() => {}),
+      spyOn(logger, 'warn').mockImplementation(() => {}),
+      spyOn(logger, 'error').mockImplementation(() => {}),
+      spyOn(logger, 'failure').mockImplementation(() => {}),
+    ];
+
+    mockGetObservationsByIds = mock(() => [{ id: 1 }, { id: 2 }]);
+    mockGetSdkSessionsBySessionIds = mock(() => [{ id: 'abc' }]);
+
+    const mockDbManager = {
+      getSessionStore: () => ({
+        getObservationsByIds: mockGetObservationsByIds,
+        getSdkSessionsBySessionIds: mockGetSdkSessionsBySessionIds,
+      }),
+    };
+
+    routes = new DataRoutes(
+      {} as any, // paginationHelper
+      mockDbManager as any,
+      {} as any, // sessionManager
+      {} as any, // sseBroadcaster
+      {} as any, // workerService
+      Date.now()
+    );
+  });
+
+  afterEach(() => {
+    loggerSpies.forEach(spy => spy.mockRestore());
+    mock.restore();
+  });
+
+  describe('handleGetObservationsByIds — ids coercion', () => {
+    // Access the handler via setupRoutes
+    let handler: (req: Request, res: Response) => void;
+
+    beforeEach(() => {
+      const mockApp = {
+        get: mock(() => {}),
+        post: mock((path: string, fn: any) => {
+          if (path === '/api/observations/batch') handler = fn;
+        }),
+        delete: mock(() => {}),
+      };
+      routes.setupRoutes(mockApp as any);
+    });
+
+    it('should accept a native array of numbers', () => {
+      const { req, res, jsonSpy } = createMockReqRes({ ids: [1, 2, 3] });
+      handler(req as Request, res as Response);
+
+      expect(mockGetObservationsByIds).toHaveBeenCalledWith([1, 2, 3], expect.anything());
+      expect(jsonSpy).toHaveBeenCalled();
+    });
+
+    it('should coerce a JSON-encoded string array "[1,2,3]" to native array', () => {
+      const { req, res, jsonSpy } = createMockReqRes({ ids: '[1,2,3]' });
+      handler(req as Request, res as Response);
+
+      expect(mockGetObservationsByIds).toHaveBeenCalledWith([1, 2, 3], expect.anything());
+      expect(jsonSpy).toHaveBeenCalled();
+    });
+
+    it('should coerce a comma-separated string "1,2,3" to native array', () => {
+      const { req, res, jsonSpy } = createMockReqRes({ ids: '1,2,3' });
+      handler(req as Request, res as Response);
+
+      expect(mockGetObservationsByIds).toHaveBeenCalledWith([1, 2, 3], expect.anything());
+      expect(jsonSpy).toHaveBeenCalled();
+    });
+
+    it('should reject non-integer values after coercion', () => {
+      const { req, res, statusSpy } = createMockReqRes({ ids: 'foo,bar' });
+      handler(req as Request, res as Response);
+
+      // NaN values should fail the Number.isInteger check
+      expect(statusSpy).toHaveBeenCalledWith(400);
+    });
+
+    it('should reject missing ids', () => {
+      const { req, res, statusSpy } = createMockReqRes({});
+      handler(req as Request, res as Response);
+
+      expect(statusSpy).toHaveBeenCalledWith(400);
+    });
+
+    it('should return empty array for empty ids array', () => {
+      const { req, res, jsonSpy } = createMockReqRes({ ids: [] });
+      handler(req as Request, res as Response);
+
+      expect(jsonSpy).toHaveBeenCalledWith([]);
+    });
+  });
+
+  describe('handleGetSdkSessionsByIds — memorySessionIds coercion', () => {
+    let handler: (req: Request, res: Response) => void;
+
+    beforeEach(() => {
+      const mockApp = {
+        get: mock(() => {}),
+        post: mock((path: string, fn: any) => {
+          if (path === '/api/sdk-sessions/batch') handler = fn;
+        }),
+        delete: mock(() => {}),
+      };
+      routes.setupRoutes(mockApp as any);
+    });
+
+    it('should accept a native array of strings', () => {
+      const { req, res, jsonSpy } = createMockReqRes({ memorySessionIds: ['abc', 'def'] });
+      handler(req as Request, res as Response);
+
+      expect(mockGetSdkSessionsBySessionIds).toHaveBeenCalledWith(['abc', 'def']);
+      expect(jsonSpy).toHaveBeenCalled();
+    });
+
+    it('should coerce a JSON-encoded string array to native array', () => {
+      const { req, res, jsonSpy } = createMockReqRes({ memorySessionIds: '["abc","def"]' });
+      handler(req as Request, res as Response);
+
+      expect(mockGetSdkSessionsBySessionIds).toHaveBeenCalledWith(['abc', 'def']);
+      expect(jsonSpy).toHaveBeenCalled();
+    });
+
+    it('should coerce a comma-separated string to native array', () => {
+      const { req, res, jsonSpy } = createMockReqRes({ memorySessionIds: 'abc,def' });
+      handler(req as Request, res as Response);
+
+      expect(mockGetSdkSessionsBySessionIds).toHaveBeenCalledWith(['abc', 'def']);
+      expect(jsonSpy).toHaveBeenCalled();
+    });
+
+    it('should trim whitespace from comma-separated values', () => {
+      const { req, res, jsonSpy } = createMockReqRes({ memorySessionIds: 'abc, def , ghi' });
+      handler(req as Request, res as Response);
+
+      expect(mockGetSdkSessionsBySessionIds).toHaveBeenCalledWith(['abc', 'def', 'ghi']);
+      expect(jsonSpy).toHaveBeenCalled();
+    });
+
+    it('should reject non-array, non-string values', () => {
+      const { req, res, statusSpy } = createMockReqRes({ memorySessionIds: 42 });
+      handler(req as Request, res as Response);
+
+      expect(statusSpy).toHaveBeenCalledWith(400);
+    });
+  });
+});