Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/training-agent-force-task-completion.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
---

patch: training-agent implements force_task_completion

Wires this repo's reference seller to 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 (`force_create_media_buy_arm`); rebased onto #3191 (the `@adcp/client` 5.18.0 bump that ships `PostgresTaskStore.createTask({ taskId })`).

**Controller scenario.** `handleComplyTestController` pre-dispatches `force_task_completion` before delegating to the SDK (the SDK's `CONTROLLER_SCENARIOS` enum is closed; new spec scenarios live in the wrapper until adopted upstream). Validates `task_id` (non-empty, ≤128 chars), `result` (plain object), and a soft 256 KB cap on the payload. Records `(task_id, result, ownerKey)` in a process-global Map so cross-account replays return `NOT_FOUND` (per the spec MUST), identical-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.** 5.18.0 ships `PostgresTaskStore.createTask({ taskId })` — meaningful progress, but two adjacent gaps still block SDK-backed roundtripping:

1. `InMemoryTaskStore` (re-exported from `@modelcontextprotocol/sdk`) doesn't yet honor caller-supplied IDs. Training-agent CI without `DATABASE_URL` falls back to InMemory, so an SDK-backed implementation would silently get random IDs there.
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`). A storyboard polling phase asserting AdCP fails against the SDK's auto-registered handler.

Both tracked in adcp-client#994. The local-Map 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. Swapping the storage layer is mechanical when upstream lands.

**Storyboard extension still deferred.** `create_media_buy_async.yaml` remains v1.0.0 (submitted-arm only). The polling phase needs Gap 2 to land plus storyboard-runner wiring to thread caller-supplied IDs through tool input.

**Tests.** New `server/tests/unit/training-agent-force-task-completion.test.ts` (9 tests): directive registration, INVALID_PARAMS for missing/oversized fields, replay idempotency, diverging-replay INVALID_TRANSITION, cross-account NOT_FOUND, list_scenarios advertisement. `comply-test-controller.test.ts` length assertion bumped 8→9.
139 changes: 137 additions & 2 deletions server/src/training-agent/comply-test-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ───────────────────────────────────────────────

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion server/tests/unit/comply-test-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
Loading
Loading