patch: training-agent implements force_task_completion + bumps @adcp/client to 5.18.0

.changeset/training-agent-force-task-completion.md

@@ -0,0 +1,21 @@
+---
+---
+
+patch: training-agent implements force_task_completion + bumps @adcp/client to 5.18.0
+
+Wires the training-agent (this repo's reference seller) to handle the `force_task_completion` controller scenario from #3138 — the spec primitive that resolves a previously-submitted task to `completed` with a buyer-supplied result payload. Companion to #3115 (which implemented `force_create_media_buy_arm`).
+
+**Controller scenario.** `handleComplyTestController` pre-dispatches `force_task_completion` before delegating to the SDK's `handleTestControllerRequest` (the SDK's enum is closed; new spec scenarios live in the wrapper until adopted upstream). The handler validates `task_id` (required, non-empty, ≤128 chars), `result` (required, plain object), and the soft 256 KB result-payload cap. It records `(task_id, result, ownerKey)` in a process-global Map so cross-account replays return `NOT_FOUND` (per the spec MUST), same-params replays are idempotent no-ops, and diverging-params replays against a terminal task return `INVALID_TRANSITION`. `list_scenarios` is augmented to advertise the new scenario.
+
+**Why a local Map and not the SDK task store.** `@adcp/client` 5.18.0 (adcp-client#996, this PR's bump) added `PostgresTaskStore.createTask({ taskId })` for caller-supplied IDs — meaningful progress. But two adjacent gaps still block end-to-end roundtripping through the SDK task store:
+
+1. `InMemoryTaskStore` (re-exported from `@modelcontextprotocol/sdk`) doesn't yet honor caller-supplied IDs. The training-agent falls back to InMemory in test/CI without `DATABASE_URL`, so an SDK-backed implementation would silently get random IDs in CI.
+2. The SDK auto-registers `tasks/get` with the MCP `Task` shape (`taskId`, `status` ∈ `working|completed|failed|input_required|cancelled`); the AdCP `tasks-get-response.json` schema requires the AdCP shape (`task_id`, `status` ∈ `submitted|working|input-required|completed|canceled|...`, plus `task_type`, `protocol`, `result`, etc.). A storyboard polling phase asserting against the AdCP schema fails against the SDK's auto-registered handler.
+
+Both gaps are tracked in adcp-client#994. The local-Map controller-side primitive ships the spec contract sellers must honor (cross-account NOT_FOUND, replay idempotency, terminal INVALID_TRANSITION) without depending on infrastructure that isn't there yet. When upstream lands, swapping the storage layer is mechanical.
+
+**Storyboard extension still deferred.** The `create_media_buy_async` storyboard remains v1.0.0 (submitted-arm only). The polling phase needs Gap 2 above to land and the storyboard runner needs to thread caller-supplied IDs through tool input. Both are tracked in adcp-client#994.
+
+**SDK upgrade fallout.** `@adcp/client` 5.18.0 broadened the `step.hints` type to a union (`ContextValueRejectedHint | ShapeDriftHint | MissingRequiredFieldHint | FormatMismatchHint | MonotonicViolationHint`). `renderAllHintFixPlans` now accepts the broader `StoryboardStepHint[]` and filters to `context_value_rejected` internally — other hint kinds will get their own fix-plan templates as authors demand them.
+
+**Tests.** New `server/tests/unit/training-agent-force-task-completion.test.ts` (9 tests): directive registration with valid params, INVALID_PARAMS for missing/oversized fields, replay idempotency, diverging-replay INVALID_TRANSITION, cross-account isolation NOT_FOUND, list_scenarios advertisement. Existing `comply-test-controller.test.ts` length assertion bumped 8→9 to cover the new local scenario.

package.json

@@ -69,7 +69,7 @@
     "check:images": "bash scripts/check-image-quality.sh"
   },
   "dependencies": {
-    "@adcp/client": "5.17.0",
+    "@adcp/client": "5.18.0",
     "@anthropic-ai/sdk": "^0.90.0",
     "@asteasolutions/zod-to-openapi": "^8.5.0",
     "@contentauth/c2pa-node": "^0.5.4",

server/src/addie/services/storyboard-fix-plan.ts

@@ -14,7 +14,7 @@
  * hints to format.
  */
 
-import type { ContextValueRejectedHint } from '@adcp/client/testing';
+import type { ContextValueRejectedHint, StoryboardStepHint } from '@adcp/client/testing';
 
 export type { ContextValueRejectedHint };
 
@@ -208,9 +208,15 @@ function formatAcceptedList(values: unknown[]): string {
  * Convenience: render every hint on a step result as fix plans, joined
  * by horizontal rules. Returns `null` when there are no actionable
  * hints (lets callers omit the section entirely).
+ *
+ * Accepts the broader `StoryboardStepHint[]` union the SDK now emits and
+ * filters to `context_value_rejected` internally — other hint kinds
+ * (shape_drift, missing_required_field, format_mismatch, monotonic_violation)
+ * will get their own fix-plan templates as they're added; until then they're
+ * silently dropped here.
  */
 export function renderAllHintFixPlans(
-  hints: ContextValueRejectedHint[] | undefined,
+  hints: StoryboardStepHint[] | undefined,
   ctx: { current_step_id: string; current_task: string; surface: 'step' | 'full' }
 ): string | null {
   if (!hints || !hints.length) return null;
@@ -222,10 +228,12 @@ export function renderAllHintFixPlans(
   const seen = new Set<string>();
   const blocks: string[] = [];
   for (const h of hints) {
-    const key = `${h.source_step_id}::${h.context_key}::${stableStringify(h.rejected_value)}`;
+    if (h.kind !== 'context_value_rejected') continue;
+    const cvr = h as ContextValueRejectedHint;
+    const key = `${cvr.source_step_id}::${cvr.context_key}::${stableStringify(cvr.rejected_value)}`;
     if (seen.has(key)) continue;
     seen.add(key);
-    blocks.push(renderHintFixPlan({ hint: h, ...ctx }));
+    blocks.push(renderHintFixPlan({ hint: cvr, ...ctx }));
   }
   return blocks.length ? blocks.join('\n\n---\n\n') : null;
 }

server/src/training-agent/comply-test-controller.ts

@@ -465,7 +465,7 @@ function createStore(session: SessionState): TestControllerStore {
  * adcontextprotocol/adcp-client — the dedup below means it is safe to leave this
  * entry in place during the transition; remove once a release has landed and the
  * cross-impl tests no longer rely on it). */
-const LOCAL_SCENARIOS = ['force_create_media_buy_arm', 'seed_creative_format'] as const;
+const LOCAL_SCENARIOS = ['force_create_media_buy_arm', 'force_task_completion', 'seed_creative_format'] as const;
 
 // ── Tool definition ───────────────────────────────────────────────
 
@@ -540,7 +540,8 @@ export async function handleComplyTestController(args: ToolArgs, ctx: TrainingCo
     };
   }
 
-  const session = await getSession(sessionKeyFromArgs(args, ctx.mode, ctx.userId, ctx.moduleId));
+  const sessionKey = sessionKeyFromArgs(args, ctx.mode, ctx.userId, ctx.moduleId);
+  const session = await getSession(sessionKey);
 
   // Pre-dispatch local scenarios the SDK doesn't know about yet. The SDK's
   // dispatcher would return UNKNOWN_SCENARIO for these, so handle them before
@@ -549,6 +550,9 @@ export async function handleComplyTestController(args: ToolArgs, ctx: TrainingCo
   if (scenario === 'force_create_media_buy_arm') {
     return handleForceCreateMediaBuyArm(session, rawArgs);
   }
+  if (scenario === 'force_task_completion') {
+    return handleForceTaskCompletion(sessionKey, rawArgs);
+  }
   // seed_creative_format is a training-agent extension not in the SDK's
   // CONTROLLER_SCENARIOS. Handle it before the SDK dispatcher so the SDK
   // doesn't return UNKNOWN_SCENARIO. Idempotency (same ID + same fixture
@@ -697,6 +701,137 @@ function handleForceCreateMediaBuyArm(session: SessionState, rawArgs: Record<str
   };
 }
 
+/**
+ * Resolve a previously-registered task to `completed` with a buyer-supplied
+ * result payload. Spec: `force_task_completion` in
+ * `comply-test-controller-request.json` and the matching mdx section.
+ *
+ * The training-agent records completions in a process-global Map keyed by
+ * (caller-supplied task_id) → ({ result, ownerKey }). Cross-account calls return
+ * NOT_FOUND (per the spec MUST). Tasks already at `completed` with the same
+ * result are idempotent no-ops; tasks at any other terminal state return
+ * INVALID_TRANSITION.
+ *
+ * Buyer-side observability via tasks/get is intentionally **deferred** to a
+ * follow-up. The MCP SDK's TaskStore generates task_ids server-side and exposes
+ * no API for caller-supplied IDs, and the SDK's auto-registered tasks/get
+ * returns the MCP Task shape rather than the AdCP `tasks-get-response.json`
+ * shape — both gaps need fixing before a storyboard polling phase against the
+ * training-agent can pass. This commit ships the controller-side primitive
+ * (the directive write) so other reference sellers and the upstream SDK have a
+ * concrete behavior to mirror; the storyboard extension lands once the
+ * polling integration exists. See PR description for the deferred tracking.
+ */
+const FORCED_TASK_COMPLETIONS = new Map<string, { result: Record<string, unknown>; ownerKey: string; completedAt: string }>();
+const MAX_FORCED_TASK_COMPLETIONS = 1000;
+
+/** Test-only: clear the forced-completion pool. */
+export function clearForcedTaskCompletions(): void {
+  FORCED_TASK_COMPLETIONS.clear();
+}
+
+/** Test-only: read the forced-completion pool. */
+export function getForcedTaskCompletions(): ReadonlyMap<string, { result: Record<string, unknown>; ownerKey: string; completedAt: string }> {
+  return FORCED_TASK_COMPLETIONS;
+}
+
+function handleForceTaskCompletion(sessionKey: string, rawArgs: Record<string, unknown>): object {
+  const params = rawArgs.params as Record<string, unknown> | undefined;
+  if (!params || typeof params !== 'object') {
+    return {
+      success: false,
+      error: 'INVALID_PARAMS',
+      error_detail: 'force_task_completion requires params',
+    };
+  }
+
+  const taskId = params.task_id;
+  if (typeof taskId !== 'string' || taskId.length === 0) {
+    return {
+      success: false,
+      error: 'INVALID_PARAMS',
+      error_detail: 'task_id is required',
+    };
+  }
+  if (taskId.length > 128) {
+    return {
+      success: false,
+      error: 'INVALID_PARAMS',
+      error_detail: 'task_id exceeds maxLength 128',
+    };
+  }
+
+  const result = params.result;
+  if (!result || typeof result !== 'object' || Array.isArray(result)) {
+    return {
+      success: false,
+      error: 'INVALID_PARAMS',
+      error_detail: 'result is required and must be an object (validates against async-response-data.json)',
+    };
+  }
+
+  // Soft 256 KB cap on result payloads, per the spec's recommendation. Bounds
+  // sandbox-amplified storage/echo DoS against the seller's task store.
+  const resultBytes = JSON.stringify(result).length;
+  if (resultBytes > 256 * 1024) {
+    return {
+      success: false,
+      error: 'INVALID_PARAMS',
+      error_detail: `result payload exceeds 256 KB (${resultBytes} bytes)`,
+    };
+  }
+
+  const existing = FORCED_TASK_COMPLETIONS.get(taskId);
+  if (existing) {
+    // Cross-account check (spec MUST): NOT_FOUND for task_ids belonging to other
+    // accounts, conventional "not yours" → "doesn't exist" treatment.
+    if (existing.ownerKey !== sessionKey) {
+      return {
+        success: false,
+        error: 'NOT_FOUND',
+        error_detail: `Task "${taskId}" was not registered for this sandbox account`,
+      };
+    }
+    // Idempotent replay: same params → no-op success.
+    if (JSON.stringify(existing.result) === JSON.stringify(result)) {
+      return {
+        success: true,
+        previous_state: 'completed',
+        current_state: 'completed',
+        message: `Task ${taskId} already completed with the same result`,
+      };
+    }
+    // Diverging replay against a terminal task: INVALID_TRANSITION.
+    return {
+      success: false,
+      error: 'INVALID_TRANSITION',
+      error_detail: `Task "${taskId}" is already terminal (completed); cannot re-complete with diverging result`,
+      current_state: 'completed',
+    };
+  }
+
+  if (FORCED_TASK_COMPLETIONS.size >= MAX_FORCED_TASK_COMPLETIONS) {
+    return {
+      success: false,
+      error: 'INVALID_STATE',
+      error_detail: `Forced-completion cap reached (${MAX_FORCED_TASK_COMPLETIONS})`,
+    };
+  }
+
+  FORCED_TASK_COMPLETIONS.set(taskId, {
+    result: result as Record<string, unknown>,
+    ownerKey: sessionKey,
+    completedAt: new Date().toISOString(),
+  });
+
+  return {
+    success: true,
+    previous_state: 'submitted',
+    current_state: 'completed',
+    message: `Task ${taskId} transitioned from submitted to completed`,
+  };
+}
+
 // Module-level seed-fixture cache enforces the spec's same-ID-different-
 // fixture rejection rule across all seed calls in the process. Scoping per-
 // process keeps it aligned with the CONTROLLER_SCENARIOS list being static.

server/tests/unit/comply-test-controller.test.ts

@@ -176,11 +176,12 @@ describe('comply_test_controller', () => {
         // Local scenarios — see LOCAL_SCENARIOS in
         // server/src/training-agent/comply-test-controller.ts.
         'force_create_media_buy_arm',
+        'force_task_completion',
         'seed_creative_format',
       ]));
       // Catch silent drift in either direction (entries removed, or new ones
       // not yet documented in this assertion).
-      expect(scenarios.length).toBe(8);
+      expect(scenarios.length).toBe(9);
       // Dedup invariant — see SCENARIO_ENUM dedup in the wrapper.
       expect(new Set(scenarios).size).toBe(scenarios.length);
     });