fix: address all Gemini Code Assist review comments on PR #450

jmbish04 · claude · jmbish04 · commit 88b27735d673 · 2026-03-31T14:15:34.000-07:00
1. .agent/rules/000-bootstrap.md: Replace PostgreSQL-specific
   generatedAlwaysAsIdentity() with D1/SQLite primaryKey({ autoIncrement: true })
2. container/lib/logger.ts: Replace singleton globalSocket with per-session
   socket map to prevent multi-session log routing issues
3. container/package.json: Wrap backgrounded server.js so container exits
   if either process crashes
4. container/lib/HookHandler.ts: Fix event name from image_created to
   image_artifact to match ServerToClientEvents interface, and align
   payload with ImageArtifactPayload type
5. container/lib/HookHandler.ts: Replace all raw console.log/error/warn
   with structured this.log.* calls for proper JSON logging
6. docs seed_project_tasks.py: Replace npx wrangler with pnpm dlx
   wrangler@latest per infrastructure standards
7. container/lib/types.ts: Add container_log to ServerToClientEvents

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/container/lib/HookHandler.ts b/container/lib/HookHandler.ts
@@ -227,31 +227,31 @@ export class HookHandler {
       const inputObj = input as Record<string, unknown> | null;
       const toolName = inputObj?.tool_name || 'unknown';
 
-      console.log(`[HOOK ENTRY] ${eventName} for ${toolName}, socket=${socket?.id || 'null'}, connected=${socket?.connected}`);
+      this.log.debug(`[HOOK ENTRY] ${eventName} for ${toolName}, socket=${socket?.id || 'null'}, connected=${socket?.connected}`, undefined, socket?.id);
 
       // Capture SDK session ID from hook event data
       this.captureSessionId(inputObj, eventName, socket);
 
       // Check if session is aborted
       if (this.context.abortSignal.aborted) {
-        console.log(`[HOOK ABORT] ${eventName} - session aborted`);
+        this.log.warn(`[HOOK ABORT] ${eventName} - session aborted`, this.context.sessionId);
         return {};
       }
 
       // Handle disconnected socket
       if (!socket || !socket.connected) {
-        console.warn(`[HOOK SKIP] ${eventName} - socket disconnected or null`);
+        this.log.warn(`[HOOK SKIP] ${eventName} - socket disconnected or null`, this.context.sessionId);
         this.log.warn(`Skipping hook ${eventName} (socket disconnected)`, this.context.sessionId);
         return {};
       }
 
       // Use strategy based on hook type
       if (AUTO_CONTINUE_HOOKS.has(eventName)) {
-        console.log(`[HOOK AUTO] ${eventName} - using auto-continue strategy`);
+        this.log.debug(`[HOOK AUTO] ${eventName} - using auto-continue strategy`, undefined, socket.id);
         return this.handleAutoContinueHook(eventName, input, socket);
       }
 
-      console.log(`[HOOK REQUEST] ${eventName} - using request-response strategy`);
+      this.log.debug(`[HOOK REQUEST] ${eventName} - using request-response strategy`, undefined, socket.id);
       return this.handleRequestResponseHook(eventName, input, socket, timeoutMs);
     };
   }
@@ -303,7 +303,7 @@ export class HookHandler {
     }
 
     const sdkSessionId = input.session_id as string;
-    console.log(`[SDK SESSION] Captured SDK session_id from ${eventName} hook: ${sdkSessionId}`);
+    this.log.info(`[SDK SESSION] Captured SDK session_id from ${eventName} hook: ${sdkSessionId}`, socket?.id);
 
     this.log.outgoing(socket.id, 'message[system/init]', { sdk_session_id: sdkSessionId });
 
@@ -359,17 +359,19 @@ export class HookHandler {
         return;
       }
 
-      console.log(`[HookHandler] Detected ${imagePaths.length} image(s):`, imagePaths);
+      this.log.info(`Detected ${imagePaths.length} image artifact(s): ${imagePaths.join(', ')}`, socket.id);
 
       // Emit each image path - the worker will handle upload to R2
       for (const filePath of imagePaths) {
-        socket.emit("image_created", {
+        socket.emit("image_artifact", {
+          type: 'image',
+          url: '', // Worker populates after R2 upload
           sandboxPath: filePath,
-          sessionId: this.context.sessionId,
+          mimeType: filePath.endsWith('.png') ? 'image/png' : filePath.endsWith('.svg') ? 'image/svg+xml' : 'image/jpeg',
         });
       }
     } catch (error) {
-      console.error('[HookHandler] Error detecting image paths:', error);
+      this.log.error('Error detecting image paths', error, socket.id);
     }
   }
 
@@ -386,8 +388,8 @@ export class HookHandler {
     const toolName = inputObj?.tool_name || 'unknown';
     const hookId = `${eventName}-${Date.now()}`;
 
-    console.log(`[HOOK ${hookId}] START: ${eventName} for tool=${toolName}`);
-    console.log(`[HOOK ${hookId}] Socket state: connected=${socket.connected}, id=${socket.id}`);
+    this.log.debug(`[HOOK ${hookId}] START: ${eventName} for tool=${toolName}`, undefined, socket.id);
+    this.log.debug(`[HOOK ${hookId}] Socket state: connected=${socket.connected}, id=${socket.id}`, undefined, socket.id);
 
     this.log.debug(
       `Hook triggered: ${eventName}`,
@@ -405,13 +407,13 @@ export class HookHandler {
           progressCount++;
           const elapsed = Date.now() - startTime;
           const socketNow = this.context.getSocket();
-          console.warn(`[HOOK ${hookId}] WAITING ${progressCount * 5}s (${elapsed}ms elapsed), socket=${socketNow?.id || 'null'}, connected=${socketNow?.connected}`);
+          this.log.warn(`[HOOK ${hookId}] WAITING ${progressCount * 5}s (${elapsed}ms elapsed), socket=${socketNow?.id || 'null'}, connected=${socketNow?.connected}`, this.context.sessionId);
         }, 5000);
 
         // Timeout handler
         const timeout = setTimeout(() => {
           const elapsed = Date.now() - startTime;
-          console.error(`[HOOK ${hookId}] TIMEOUT after ${elapsed}ms (limit=${timeoutMs}ms)`);
+          this.log.error(`[HOOK ${hookId}] TIMEOUT after ${elapsed}ms (limit=${timeoutMs}ms)`, undefined, this.context.sessionId);
           this.log.warn(`Hook ${eventName} timed out after ${timeoutMs}ms`, this.context.sessionId);
           cleanup();
           resolve({});
@@ -420,7 +422,7 @@ export class HookHandler {
         // Disconnect handler
         const onDisconnect = () => {
           const elapsed = Date.now() - startTime;
-          console.error(`[HOOK ${hookId}] DISCONNECT after ${elapsed}ms`);
+          this.log.error(`[HOOK ${hookId}] DISCONNECT after ${elapsed}ms`, undefined, this.context.sessionId);
           this.log.warn(`Client disconnected while waiting for hook ${eventName}`, this.context.sessionId);
           cleanup();
           resolve({});
@@ -435,7 +437,7 @@ export class HookHandler {
         };
 
         // Emit hook request with callback
-        console.log(`[HOOK ${hookId}] EMITTING hook_request to client...`);
+        this.log.debug(`[HOOK ${hookId}] EMITTING hook_request to client...`, undefined, socket.id);
         this.log.outgoing(
           socket.id,
           'hook_request',
@@ -447,22 +449,22 @@ export class HookHandler {
           { event: eventName, data: input },
           (clientResponse: HookResponse) => {
             const elapsed = Date.now() - startTime;
-            console.log(`[HOOK ${hookId}] CALLBACK received after ${elapsed}ms:`, JSON.stringify(clientResponse));
+            this.log.debug(`[HOOK ${hookId}] CALLBACK received after ${elapsed}ms`, clientResponse, socket.id);
             cleanup();
             this.log.incoming(socket.id, `hook_response[${eventName}]`, clientResponse);
             resolve(clientResponse || {});
           }
         );
 
-        console.log(`[HOOK ${hookId}] hook_request emitted, waiting for callback...`);
+        this.log.debug(`[HOOK ${hookId}] hook_request emitted, waiting for callback...`, undefined, socket.id);
       });
 
       const totalTime = Date.now() - startTime;
-      console.log(`[HOOK ${hookId}] COMPLETE in ${totalTime}ms, response:`, JSON.stringify(response));
+      this.log.debug(`[HOOK ${hookId}] COMPLETE in ${totalTime}ms`, response, socket.id);
       return response;
     } catch (error) {
       const elapsed = Date.now() - startTime;
-      console.error(`[HOOK ${hookId}] ERROR after ${elapsed}ms:`, error);
+      this.log.error(`[HOOK ${hookId}] ERROR after ${elapsed}ms`, error, this.context.sessionId);
       this.log.error(`Error in hook ${eventName}`, error, this.context.sessionId);
       return {};
     }
diff --git a/container/lib/SocketHandlers.ts b/container/lib/SocketHandlers.ts
@@ -12,7 +12,7 @@ import type { SDKUserMessage } from "@anthropic-ai/claude-agent-sdk";
 import type { ExtendedOptions, Logger } from './types.js';
 import { SessionManager } from './SessionManager.js';
 import { QueryOrchestrator } from './QueryOrchestrator.js';
-import { log as defaultLog, setLogSocket } from './logger.js';
+import { log as defaultLog, setLogSocket, removeLogSocket } from './logger.js';
 
 // ============================================================================
 // TYPES
@@ -385,9 +385,14 @@ export function registerSocketHandlers(socket: Socket, deps: SocketHandlerDeps):
   const log = deps.logger || defaultLog;
   const { sessionManager } = deps;
 
-  // Set global socket for log forwarding to worker
+  // Register socket for log forwarding to worker (keyed by socket ID)
   setLogSocket(socket);
 
+  // Clean up socket from log map on disconnect
+  socket.on('disconnect', () => {
+    removeLogSocket(socket.id);
+  });
+
   // Extract session ID from connection
   const sessionId = socket.handshake.query.sessionId as string;
   log.info(`New client connected`, socket.id);
diff --git a/container/lib/index.ts b/container/lib/index.ts
@@ -10,7 +10,7 @@
 export * from './types.js';
 
 // Logger
-export { log, createLogger, getTimestamp, truncateStrings, setLogSocket, getLogSocket } from './logger.js';
+export { log, createLogger, getTimestamp, truncateStrings, setLogSocket, removeLogSocket, getLogSocket } from './logger.js';
 export type { LoggerOptions } from './logger.js';
 
 // MessageStream
diff --git a/container/lib/logger.ts b/container/lib/logger.ts
@@ -27,24 +27,38 @@ import type { LogLevel, LogDirection, Logger } from './types.js';
 const MAX_STRING_LENGTH = 200;
 
 /**
- * Global socket reference for emitting logs to connected clients.
- * This allows container logs to be forwarded through Socket.IO to the worker.
+ * Per-session socket map for emitting logs to the correct connected client.
+ * Keyed by socket ID to avoid the singleton problem where only the most
+ * recently connected client receives logs.
  */
-let globalSocket: Socket | null = null;
+const socketMap = new Map<string, Socket>();
 
 /**
- * Sets the global socket for log forwarding.
+ * Registers a socket for log forwarding, keyed by its ID.
  * Call this when a socket connects to enable live log streaming.
+ * Pass null to remove the socket (on disconnect).
  */
 export function setLogSocket(socket: Socket | null): void {
-  globalSocket = socket;
+  if (socket) {
+    socketMap.set(socket.id, socket);
+  }
+}
+
+/**
+ * Removes a socket from the log forwarding map.
+ * Call this on socket disconnect to prevent stale references.
+ */
+export function removeLogSocket(socketId: string): void {
+  socketMap.delete(socketId);
 }
 
 /**
- * Gets the current log socket.
+ * Gets all currently connected log sockets.
  */
 export function getLogSocket(): Socket | null {
-  return globalSocket;
+  // Return the most recently added socket for backward compatibility
+  const entries = Array.from(socketMap.values());
+  return entries.length > 0 ? entries[entries.length - 1] : null;
 }
 
 // ============================================================================
@@ -118,21 +132,31 @@ function writeLog(entry: LogEntry): void {
   console.log(logStr);
 
   // Forward important logs via Socket.IO for live streaming to worker/frontend
-  // Forward: errors, warnings, hook events, SDK messages, and key connection events
-  if (globalSocket?.connected) {
-    const event = entry.event || '';
-    const message = entry.message || '';
-
-    const shouldForward = entry.level === 'ERROR' ||
-                          entry.level === 'WARN' ||
-                          event.includes('hook') ||
-                          event.startsWith('SDK_MSG') ||
-                          message.includes('connected') ||
-                          message.includes('Query') ||
-                          message.includes('session');
-
-    if (shouldForward) {
-      globalSocket.emit('container_log', entry);
+  // Route to all connected sockets based on their socketId
+  const event = entry.event || '';
+  const message = entry.message || '';
+
+  const shouldForward = entry.level === 'ERROR' ||
+                        entry.level === 'WARN' ||
+                        event.includes('hook') ||
+                        event.startsWith('SDK_MSG') ||
+                        message.includes('connected') ||
+                        message.includes('Query') ||
+                        message.includes('session');
+
+  if (shouldForward) {
+    // If log has a socketId, route to that specific socket; otherwise broadcast to all
+    if (entry.socketId) {
+      const targetSocket = socketMap.get(entry.socketId);
+      if (targetSocket?.connected) {
+        targetSocket.emit('container_log', entry);
+      }
+    } else {
+      for (const socket of socketMap.values()) {
+        if (socket.connected) {
+          socket.emit('container_log', entry);
+        }
+      }
     }
   }
 }
diff --git a/container/lib/types.ts b/container/lib/types.ts
@@ -441,6 +441,8 @@ export interface ServerToClientEvents {
   image_artifact: (payload: ImageArtifactPayload) => void;
   /** Emitted when the agent creates, modifies, or deletes files */
   file_changed: (payload: FileChangedPayload) => void;
+  /** Forwarded structured log entries from the container for live streaming */
+  container_log: (payload: Record<string, unknown>) => void;
 }
 
 /**
diff --git a/container/package.json b/container/package.json
@@ -4,7 +4,7 @@
     "scripts": {
         "build": "tsc",
         "watch": "tsc --watch",
-        "start": "node dist/src/server.js & node dist/agent-sdk.js"
+        "start": "node dist/src/server.js & SERVER_PID=$! && node dist/agent-sdk.js; EXIT_CODE=$?; kill $SERVER_PID 2>/dev/null; exit $EXIT_CODE"
     },
     "dependencies": {
         "@anthropic-ai/claude-agent-sdk": "^0.2.85",
diff --git a/docs/20260329/continuous_improvement/seed_project_tasks.py b/docs/20260329/continuous_improvement/seed_project_tasks.py
@@ -217,10 +217,10 @@ def main():
     print("── Apply to D1 ─────────────────────────────────────────────────────")
     print()
     print("  # Local (wrangler dev):")
-    print(f"  npx wrangler d1 execute {D1_DATABASE_NAME} --local --file={OUTPUT_SQL}")
+    print(f"  pnpm dlx wrangler@latest d1 execute {D1_DATABASE_NAME} --local --file={OUTPUT_SQL}")
     print()
     print("  # Remote (production):")
-    print(f"  npx wrangler d1 execute {D1_DATABASE_NAME} --remote --file={OUTPUT_SQL}")
+    print(f"  pnpm dlx wrangler@latest d1 execute {D1_DATABASE_NAME} --remote --file={OUTPUT_SQL}")
     print()
     print(f"── Summary ───────────────────────────────────────────────────────────")
     print(f"  epics   : {len(epics)}")

Original file line number	Diff line number	Diff line change
`@@ -441,6 +441,8 @@ export interface ServerToClientEvents {`
`441`	`441`	`image_artifact: (payload: ImageArtifactPayload) => void;`
`442`	`442`	`/** Emitted when the agent creates, modifies, or deletes files */`
`443`	`443`	`file_changed: (payload: FileChangedPayload) => void;`
	`444`	`+ /** Forwarded structured log entries from the container for live streaming */`
	`445`	`+ container_log: (payload: Record<string, unknown>) => void;`
`444`	`446`	`}`
`445`	`447`
`446`	`448`	`/**`