fix: address Alain's PR review findings

1. Single source of truth for runtimeArn — removed constructor param,
   strategy now reads exclusively from blueprintConfig.runtime_arn
2. Lazy singleton for BedrockAgentCoreClient — module-level shared
   client avoids creating new TLS sessions per invocation
3. ComputeType union type ('agentcore' | 'ecs') with exhaustive switch
   and never-pattern in resolveComputeStrategy
4. Differentiated error handling in stopSession — ResourceNotFoundException
   (info), ThrottlingException/AccessDeniedException (error), others (warn)
5. Added logger.info('Session started') after full invoke+transition+event
   sequence in orchestrate-task.ts
6. Added start-session-composition.test.ts with integration tests for
   happy path, error path (failTask), and partial failure (transitionTask throws)
7. pollSession now throws NotImplementedError instead of returning stale
   'running' status — clear signal for future developers

Author: MichaelWalker-git

cdk/src/handlers/orchestrate-task.ts

@@ -20,6 +20,7 @@
 import { withDurableExecution, type DurableExecutionHandler } from '@aws/durable-execution-sdk-js';
 import { TaskStatus, TERMINAL_STATUSES } from '../constructs/task-status';
 import { resolveComputeStrategy } from './shared/compute-strategy';
+import { logger } from './shared/logger';
 import {
   admissionControl,
   emitTaskEvent,
@@ -132,6 +133,12 @@ const durableHandler: DurableExecutionHandler<OrchestrateTaskEvent, void> = asyn
         strategy_type: handle.strategyType,
       });
 
+      logger.info('Session started', {
+        task_id: taskId,
+        session_id: handle.sessionId,
+        strategy_type: handle.strategyType,
+      });
+
       return handle.sessionId;
     } catch (err) {
       await failTask(taskId, TaskStatus.HYDRATING, `Session start failed: ${String(err)}`, task.user_id, true);

cdk/src/handlers/shared/compute-strategy.ts

@@ -17,7 +17,7 @@
  *  SOFTWARE.
  */
 
-import type { BlueprintConfig } from './repo-config';
+import type { BlueprintConfig, ComputeType } from './repo-config';
 import { AgentCoreComputeStrategy } from './strategies/agentcore-strategy';
 
 export interface SessionHandle {
@@ -43,11 +43,15 @@ export interface ComputeStrategy {
 }
 
 export function resolveComputeStrategy(blueprintConfig: BlueprintConfig): ComputeStrategy {
-  const computeType = blueprintConfig.compute_type;
+  const computeType: ComputeType = blueprintConfig.compute_type;
   switch (computeType) {
     case 'agentcore':
-      return new AgentCoreComputeStrategy({ runtimeArn: blueprintConfig.runtime_arn });
-    default:
-      throw new Error(`Unknown compute_type: '${computeType}'`);
+      return new AgentCoreComputeStrategy();
+    case 'ecs':
+      throw new Error("compute_type 'ecs' is not yet implemented");
+    default: {
+      const _exhaustive: never = computeType;
+      throw new Error(`Unknown compute_type: '${_exhaustive}'`);
+    }
   }
 }

cdk/src/handlers/shared/repo-config.ts

@@ -25,12 +25,14 @@ import { logger } from './logger';
  * Per-repository configuration written by the Blueprint CDK construct
  * and read at runtime by the task API gate and the orchestrator.
  */
+export type ComputeType = 'agentcore' | 'ecs';
+
 export interface RepoConfig {
   readonly repo: string;
   readonly status: 'active' | 'removed';
   readonly onboarded_at: string;
   readonly updated_at: string;
-  readonly compute_type?: string;
+  readonly compute_type?: ComputeType;
   readonly runtime_arn?: string;
   readonly model_id?: string;
   readonly max_turns?: number;
@@ -46,7 +48,7 @@ export interface RepoConfig {
  * settings with platform defaults.
  */
 export interface BlueprintConfig {
-  readonly compute_type: string;
+  readonly compute_type: ComputeType;
   readonly runtime_arn: string;
   readonly model_id?: string;
   readonly max_turns?: number;

cdk/src/handlers/shared/strategies/agentcore-strategy.ts

@@ -23,15 +23,16 @@ import type { ComputeStrategy, SessionHandle, SessionStatus } from '../compute-s
 import { logger } from '../logger';
 import type { BlueprintConfig } from '../repo-config';
 
+let sharedClient: BedrockAgentCoreClient | undefined;
+function getClient(): BedrockAgentCoreClient {
+  if (!sharedClient) {
+    sharedClient = new BedrockAgentCoreClient({});
+  }
+  return sharedClient;
+}
+
 export class AgentCoreComputeStrategy implements ComputeStrategy {
   readonly type = 'agentcore';
-  private readonly client: BedrockAgentCoreClient;
-  private readonly runtimeArn: string;
-
-  constructor(options: { runtimeArn: string }) {
-    this.runtimeArn = options.runtimeArn;
-    this.client = new BedrockAgentCoreClient({});
-  }
 
   async startSession(input: {
     taskId: string;
@@ -40,7 +41,7 @@ export class AgentCoreComputeStrategy implements ComputeStrategy {
   }): Promise<SessionHandle> {
     // AgentCore requires runtimeSessionId >= 33 chars; UUID v4 is 36 chars.
     const sessionId = randomUUID();
-    const runtimeArn = input.blueprintConfig.runtime_arn ?? this.runtimeArn;
+    const runtimeArn = input.blueprintConfig.runtime_arn;
 
     const command = new InvokeAgentRuntimeCommand({
       agentRuntimeArn: runtimeArn,
@@ -50,7 +51,7 @@ export class AgentCoreComputeStrategy implements ComputeStrategy {
       payload: new TextEncoder().encode(JSON.stringify({ input: input.payload })),
     });
 
-    await this.client.send(command);
+    await getClient().send(command);
 
     logger.info('AgentCore session invoked', { task_id: input.taskId, session_id: sessionId, runtime_arn: runtimeArn });
 
@@ -61,10 +62,12 @@ export class AgentCoreComputeStrategy implements ComputeStrategy {
     };
   }
 
+  /**
+   * Not implemented for AgentCore — polling is done at the orchestrator level via DDB reads.
+   * Future strategies (e.g. ECS/Fargate) will implement compute-level polling here.
+   */
   async pollSession(_handle: SessionHandle): Promise<SessionStatus> {
-    // Polling is currently done at the orchestrator level via DDB reads.
-    // This method exists for PR 2 where different strategies may poll differently.
-    return { status: 'running' };
+    throw new Error('pollSession is not implemented for AgentCore — use orchestrator-level DDB polling');
   }
 
   async stopSession(handle: SessionHandle): Promise<void> {
@@ -75,16 +78,27 @@ export class AgentCoreComputeStrategy implements ComputeStrategy {
     }
 
     try {
-      await this.client.send(new StopRuntimeSessionCommand({
+      await getClient().send(new StopRuntimeSessionCommand({
         agentRuntimeArn: runtimeArn,
         runtimeSessionId: handle.sessionId,
       }));
       logger.info('AgentCore session stopped', { session_id: handle.sessionId });
     } catch (err) {
-      logger.warn('Failed to stop AgentCore session (best-effort)', {
-        session_id: handle.sessionId,
-        error: err instanceof Error ? err.message : String(err),
-      });
+      const errName = err instanceof Error ? (err as Error & { name?: string }).name : undefined;
+      if (errName === 'ResourceNotFoundException') {
+        logger.info('AgentCore session already gone', { session_id: handle.sessionId });
+      } else if (errName === 'ThrottlingException' || errName === 'AccessDeniedException') {
+        logger.error('Failed to stop AgentCore session', {
+          session_id: handle.sessionId,
+          error_type: errName,
+          error: err instanceof Error ? err.message : String(err),
+        });
+      } else {
+        logger.warn('Failed to stop AgentCore session (best-effort)', {
+          session_id: handle.sessionId,
+          error: err instanceof Error ? err.message : String(err),
+        });
+      }
     }
   }
 }

cdk/test/handlers/shared/compute-strategy.test.ts

@@ -36,12 +36,12 @@ describe('resolveComputeStrategy', () => {
     expect(strategy.type).toBe('agentcore');
   });
 
-  test('throws for unknown compute_type', () => {
+  test("throws 'not yet implemented' for compute_type ecs", () => {
     expect(() =>
       resolveComputeStrategy({
-        compute_type: 'unknown',
+        compute_type: 'ecs',
         runtime_arn: 'arn:test',
       }),
-    ).toThrow("Unknown compute_type: 'unknown'");
+    ).toThrow("compute_type 'ecs' is not yet implemented");
   });
 });

cdk/test/handlers/shared/preflight.test.ts

@@ -40,7 +40,7 @@ const mockFetch = jest.fn();
 global.fetch = mockFetch as unknown as typeof fetch;
 
 const baseBlueprintConfig: BlueprintConfig = {
-  compute_type: 'AGENTCORE',
+  compute_type: 'agentcore',
   runtime_arn: 'arn:aws:bedrock:us-east-1:123456789012:agent-runtime/test',
   github_token_secret_arn: 'arn:aws:secretsmanager:us-east-1:123456789012:secret:github-token',
 };

cdk/test/handlers/shared/strategies/agentcore-strategy.test.ts

@@ -34,14 +34,14 @@ beforeEach(() => {
 
 describe('AgentCoreComputeStrategy', () => {
   test('type is agentcore', () => {
-    const strategy = new AgentCoreComputeStrategy({ runtimeArn: defaultRuntimeArn });
+    const strategy = new AgentCoreComputeStrategy();
     expect(strategy.type).toBe('agentcore');
   });
 
   describe('startSession', () => {
     test('invokes agent runtime and returns SessionHandle', async () => {
       mockSend.mockResolvedValueOnce({});
-      const strategy = new AgentCoreComputeStrategy({ runtimeArn: defaultRuntimeArn });
+      const strategy = new AgentCoreComputeStrategy();
 
       const handle = await strategy.startSession({
         taskId: 'TASK001',
@@ -55,39 +55,67 @@ describe('AgentCoreComputeStrategy', () => {
       expect(mockSend).toHaveBeenCalledTimes(1);
     });
 
-    test('uses blueprint runtime_arn override', async () => {
+    test('uses runtime_arn from blueprintConfig (single source of truth)', async () => {
       mockSend.mockResolvedValueOnce({});
-      const strategy = new AgentCoreComputeStrategy({ runtimeArn: defaultRuntimeArn });
-      const overrideArn = 'arn:aws:bedrock-agentcore:us-east-1:123:runtime/custom';
+      const strategy = new AgentCoreComputeStrategy();
+      const runtimeArn = 'arn:aws:bedrock-agentcore:us-east-1:123:runtime/custom';
 
       const handle = await strategy.startSession({
         taskId: 'TASK001',
         payload: { repo_url: 'org/repo', task_id: 'TASK001' },
-        blueprintConfig: { compute_type: 'agentcore', runtime_arn: overrideArn },
+        blueprintConfig: { compute_type: 'agentcore', runtime_arn: runtimeArn },
       });
 
-      expect(handle.metadata.runtimeArn).toBe(overrideArn);
+      expect(handle.metadata.runtimeArn).toBe(runtimeArn);
       const invokeCall = mockSend.mock.calls[0][0];
-      expect(invokeCall.input.agentRuntimeArn).toBe(overrideArn);
+      expect(invokeCall.input.agentRuntimeArn).toBe(runtimeArn);
+    });
+
+    test('reuses shared BedrockAgentCoreClient across instances', async () => {
+      const { BedrockAgentCoreClient } = require('@aws-sdk/client-bedrock-agentcore');
+      // The lazy singleton may already be initialized from prior tests.
+      // Record the current call count, then verify no additional constructor calls happen.
+      const callsBefore = BedrockAgentCoreClient.mock.calls.length;
+
+      mockSend.mockResolvedValue({});
+      const strategy1 = new AgentCoreComputeStrategy();
+      const strategy2 = new AgentCoreComputeStrategy();
+
+      await strategy1.startSession({
+        taskId: 'T1',
+        payload: {},
+        blueprintConfig: { compute_type: 'agentcore', runtime_arn: defaultRuntimeArn },
+      });
+      await strategy2.startSession({
+        taskId: 'T2',
+        payload: {},
+        blueprintConfig: { compute_type: 'agentcore', runtime_arn: defaultRuntimeArn },
+      });
+
+      // Lazy singleton: at most one constructor call total across all strategy instances
+      const callsAfter = BedrockAgentCoreClient.mock.calls.length;
+      expect(callsAfter - callsBefore).toBeLessThanOrEqual(1);
+      expect(mockSend).toHaveBeenCalledTimes(2);
     });
   });
 
   describe('pollSession', () => {
-    test('returns running status', async () => {
-      const strategy = new AgentCoreComputeStrategy({ runtimeArn: defaultRuntimeArn });
-      const result = await strategy.pollSession({
-        sessionId: 'test-session',
-        strategyType: 'agentcore',
-        metadata: { runtimeArn: defaultRuntimeArn },
-      });
-      expect(result.status).toBe('running');
+    test('throws because AgentCore polling is done at orchestrator level', async () => {
+      const strategy = new AgentCoreComputeStrategy();
+      await expect(
+        strategy.pollSession({
+          sessionId: 'test-session',
+          strategyType: 'agentcore',
+          metadata: { runtimeArn: defaultRuntimeArn },
+        }),
+      ).rejects.toThrow('pollSession is not implemented for AgentCore');
     });
   });
 
   describe('stopSession', () => {
     test('sends StopRuntimeSessionCommand', async () => {
       mockSend.mockResolvedValueOnce({});
-      const strategy = new AgentCoreComputeStrategy({ runtimeArn: defaultRuntimeArn });
+      const strategy = new AgentCoreComputeStrategy();
 
       await strategy.stopSession({
         sessionId: 'test-session',
@@ -101,9 +129,54 @@ describe('AgentCoreComputeStrategy', () => {
       expect(call.input.runtimeSessionId).toBe('test-session');
     });
 
-    test('does not throw when stop fails', async () => {
-      mockSend.mockRejectedValueOnce(new Error('Access denied'));
-      const strategy = new AgentCoreComputeStrategy({ runtimeArn: defaultRuntimeArn });
+    test('logs info for ResourceNotFoundException (session already gone)', async () => {
+      const err = new Error('Not found');
+      err.name = 'ResourceNotFoundException';
+      mockSend.mockRejectedValueOnce(err);
+      const strategy = new AgentCoreComputeStrategy();
+
+      await expect(
+        strategy.stopSession({
+          sessionId: 'test-session',
+          strategyType: 'agentcore',
+          metadata: { runtimeArn: defaultRuntimeArn },
+        }),
+      ).resolves.toBeUndefined();
+    });
+
+    test('logs error for ThrottlingException', async () => {
+      const err = new Error('Rate exceeded');
+      err.name = 'ThrottlingException';
+      mockSend.mockRejectedValueOnce(err);
+      const strategy = new AgentCoreComputeStrategy();
+
+      await expect(
+        strategy.stopSession({
+          sessionId: 'test-session',
+          strategyType: 'agentcore',
+          metadata: { runtimeArn: defaultRuntimeArn },
+        }),
+      ).resolves.toBeUndefined();
+    });
+
+    test('logs error for AccessDeniedException', async () => {
+      const err = new Error('Access denied');
+      err.name = 'AccessDeniedException';
+      mockSend.mockRejectedValueOnce(err);
+      const strategy = new AgentCoreComputeStrategy();
+
+      await expect(
+        strategy.stopSession({
+          sessionId: 'test-session',
+          strategyType: 'agentcore',
+          metadata: { runtimeArn: defaultRuntimeArn },
+        }),
+      ).resolves.toBeUndefined();
+    });
+
+    test('logs warn for unknown errors (best-effort)', async () => {
+      mockSend.mockRejectedValueOnce(new Error('Network error'));
+      const strategy = new AgentCoreComputeStrategy();
 
       await expect(
         strategy.stopSession({
@@ -115,7 +188,7 @@ describe('AgentCoreComputeStrategy', () => {
     });
 
     test('skips stop when no runtimeArn in metadata', async () => {
-      const strategy = new AgentCoreComputeStrategy({ runtimeArn: defaultRuntimeArn });
+      const strategy = new AgentCoreComputeStrategy();
 
       await strategy.stopSession({
         sessionId: 'test-session',