Skip to content

feat(schema): add result and include_result to tasks/get#3126

Merged
bokelley merged 2 commits into
mainfrom
claude/issue-3123-tasks-get-result-field
Apr 25, 2026
Merged

feat(schema): add result and include_result to tasks/get#3126
bokelley merged 2 commits into
mainfrom
claude/issue-3123-tasks-get-result-field

Conversation

@bokelley

@bokelley bokelley commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

tasks/get had no typed field for the completion payload. Buyers polling an async create_media_buy (or any submitted-arm task) could see status: completed but had no schema-backed field to retrieve media_buy_id and packages. The push-notification webhook schema (mcp-webhook-payload.json) already defined result: $ref async-response-data.json; this PR mirrors that pattern on the polling API.

Closes #3123. Unblocks adcp-client#967.

Changes

  • static/schemas/source/core/tasks-get-response.json — adds optional result: $ref /schemas/core/async-response-data.json. Present when status is completed and include_result was true in the request; absent otherwise. For failed tasks, use the existing error field.
  • static/schemas/source/core/tasks-get-request.json — adds optional include_result: boolean (default false). async-operations.mdx and task-lifecycle.mdx already referenced this parameter in code examples; this formalizes it in the schema. Sellers MUST include result in the response when include_result is true and status is completed.
  • docs/protocol/calling-an-agent.mdx — adds a completed tasks/get request/response example showing the result field, closing the documentation gap identified in the issue.
  • .changeset/tasks-get-result-field.md — minor bump (additive optional fields on stable core schemas).

Non-breaking justification

Both fields are optional; result is absent from required[]. Existing sellers that don't populate include_result and don't send result remain spec-conformant. Existing buyers using the informal additionalProperties passthrough continue to work; the typed field gives SDKs a stable, named key.

Milestone

3.1.0 — minor additive change, targets next open minor milestone.

Pre-PR review

  • code-reviewer: approved — no blockers; nit on JSON comment convention ("// ...": "..." in example, removed in final diff); schema structure is well-formed and mirrors existing include_history pattern.
  • ad-tech-protocol-expert: approved after one fix iteration — SHOULD→MUST corrected on seller obligation; failed/error dual-signal resolved by directing failed tasks to the existing error field; async-response-data.json anyOf union matches the pre-existing webhook schema pattern (accepted). Residual nit: async-response-data.json's own description predates this PR and will be addressed separately.

Session: https://claude.ai/code/session_013nAeRJTbEAmEdqvQit6Xmg

Generated by Claude Code

This was referenced Apr 25, 2026
Closed
Closed
claude and others added 2 commits April 25, 2026 09:16
@claude @bokelley
feat(schema): add result and include_result to tasks/get (closes #3123)
8ea5851 
tasks/get response had no typed field for the completion payload. Buyers polling
an async create_media_buy could see status: completed but had no schema-backed
field to retrieve media_buy_id and packages. The push-notification webhook
schema (mcp-webhook-payload.json) already defined result: $ref async-response-data.json;
this PR mirrors that pattern on the polling API.

Changes:
- tasks-get-response.json: add optional result field ($ref async-response-data.json)
  Present when status is completed and include_result was true; absent otherwise.
  For failed tasks, use the error field.
- tasks-get-request.json: add optional include_result boolean (default false).
  async-operations.mdx and task-lifecycle.mdx already referenced this parameter
  in code examples; this formalizes it in the schema.
- docs/protocol/calling-an-agent.mdx: add completed tasks/get response example
  showing the result field, closing the documentation gap.
- .changeset/tasks-get-result-field.md: minor bump (additive optional fields)

Non-breaking: both fields are optional. Existing polling consumers continue to
work; the typed field gives SDKs a stable, named key for the completion payload.
Unblocks adcp-client#967 (polling-cycle hardening).

https://claude.ai/code/session_013nAeRJTbEAmEdqvQit6Xmg
@bokelley @claude
docs: re-introduce include_result in polling examples and align chang…
ff537f4 
…eset

After patch #3127 landed, the four polling examples in
async-operations.mdx, error-handling.mdx, orchestrator-design.mdx,
and task-lifecycle.mdx no longer demonstrated include_result. Now
that this PR makes the field spec-backed, restore include_result:
true in those examples and replace the 3.0-only note in
task-lifecycle.mdx with a description of the new field's semantics.

Also tightens the changeset to match the schema's "completed only"
constraint (was: "completed or failed") and lists the four doc
files updated alongside calling-an-agent.mdx.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley force-pushed the claude/issue-3123-tasks-get-result-field branch from 4e9f2d7 to ff537f4 Compare April 25, 2026 13:19
@bokelley bokelley marked this pull request as ready for review April 25, 2026 13:22
@bokelley bokelley merged commit 4136a4a into main Apr 25, 2026
16 checks passed
@bokelley bokelley deleted the claude/issue-3123-tasks-get-result-field branch April 25, 2026 13:22
bokelley added a commit to adcontextprotocol/adcp-client that referenced this pull request Apr 25, 2026
@bokelley @claude
fix(core): polling sends include_result: true per adcp#3126
a8a6ce9 
adcontextprotocol/adcp#3126 (merged) closed the spec ambiguity
flagged in adcp#3123 by adding typed `include_result` request flag
+ `result` response field to `tasks/get` (AdCP 3.1.0). Updates the
SDK to:

- **Send `include_result: true` on every polling request**. Gates
  the typed `result` field on spec-conformant 3.1.0+ sellers.
  Wire-compatible: pre-3.1.0 sellers ignore the unknown request
  field.
- **Drop the informal `task_data` alias from the response mapper**.
  Pre-#3126 the mapper checked `flat.result ?? flat.task_data` as a
  speculative passthrough across two undocumented field names.
  AdCP 3.1.0 names `result` canonically; the alias is no longer
  needed and was a guess that didn't match the spec.
- **Update mapper JSDoc** to reference adcp#3126 as the resolution
  (closed) instead of adcp#3123 (the original ambiguity ticket).
- **Update the stale `#973` JSDoc** on `getTaskStatus` — that
  parser limitation was already resolved by PR #981. Replaced with
  a comment explaining why we send `include_result: true`.
- **Replace the `task_data` alias test** with one that asserts the
  request includes `include_result: true`.

The 3.0.0 schema cache doesn't have the new field yet (3.1.0
hasn't shipped); a follow-up will run `npm run sync-schemas` once
the spec release lands. Until then, the SDK speaks the 3.1.0 wire
shape and existing 3.0.0 schema validation continues to pass
(both `include_result` and `result` are optional with
additionalProperties: true).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 25, 2026
Closed
Merged
bokelley added a commit to adcontextprotocol/adcp-client that referenced this pull request Apr 25, 2026
@bokelley @claude
feat(core): pollTaskCompletion handles all non-progressing task statu…
5d788bc 
…ses (#977) (#980)

* fix(core): pollTaskCompletion exits on rejected status (issue #977)

The polling loop in pollTaskCompletion only exited for completed/failed/canceled.
A server returning rejected would spin until the caller's timeout elapsed.

- Add ADCP_STATUS.REJECTED to the terminal-state exit condition
- Fix error-message fallback to check status.message before the generic
  "Task rejected" string, matching the synchronous dispatch path behavior
- Add TaskInfo.message optional field to allow the fallback to typecheck
- Add 'rejected' and 'canceled' to TaskStatus union for metadata fidelity
- Add test/lib/poll-task-completion-terminal-states.test.js covering all
  four cases (error field, message field, first-poll exit, generic fallback)

Paused states (input-required, auth-required) are deferred — the API
surface decision (typed error vs. continuation) is left for a follow-up.

Refs #977

https://claude.ai/code/session_01AzgwdsUqsi7EbD8nWxLHXd

* fix(core): address pre-PR review blockers (issue #977)

- mcp-tasks.ts: mapMCPTaskToTaskInfo now maps statusMessage to error for
  rejected and canceled statuses (was: failed only). Pre-existing gap
  surfaced by the rejected fix — a rejected MCP task with a statusMessage
  would have silently produced a generic "Task rejected" string.

- test: switch mockAgent.protocol from mcp to a2a so pollTaskCompletion
  tests exercise ProtocolClient.callTool directly rather than relying on
  getMCPTaskStatus falling through (which worked but was testing the wrong
  code path).

https://claude.ai/code/session_01AzgwdsUqsi7EbD8nWxLHXd

* fix(core): preserve top-level `message` field through tasks/get mapper

After the rebase onto main, #971's `mapTasksGetResponseToTaskInfo`
became the only path that produces `TaskInfo` from a `tasks/get`
response. The mapper read `error.message` (structured-error block)
but not the top-level `message` field — so `pollTaskCompletion`'s
`status.error || status.message || 'Task <status>'` chain got
undefined for `status.message` and skipped to the generic fallback.

Added a `flat.message` passthrough alongside the existing
`flat.error` mapping. The field is now documented as the AdCP
envelope's human-readable status descriptor (advisory string
accompanying any status — distinct from the structured `error.message`
under the `error` object).

Fixes the failing CI test
`preserves message field as error fallback when error field is absent
on rejection`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(core): pollTaskCompletion handles input-required + auth-required (#977 part 2)

Closes the second half of #977. Combined with the rejected-state
fix (already in this PR), `pollTaskCompletion` now handles every
non-progressing AdCP task status instead of spinning until timeout.

**Paused states**: `input-required` and `auth-required` return a
`TaskResultIntermediate` with `success: true` (the task is
progressing, not failed) so callers can branch on `result.status`.
Mirrors the synchronous `handleInputRequired` no-handler path —
polling alone can't advance these states; the buyer must satisfy
the paused condition (supply input / refresh auth) and retry the
original tool call.

**`TaskResultIntermediate` status union**: adds `'auth-required'`
alongside the existing `'input-required'`. Also adds `'auth-required'`
to the `TaskStatus` type for metadata fidelity.

**Side fixes from code-reviewer feedback on #980**:

- `mcp-tasks.mapMCPTaskToTaskInfo`: the `statusMessage → error`
  projection now checks against the AdCP-mapped status (after
  `mapMCPTaskStatus`) instead of the MCP-side raw status. Prior
  code checked `['failed', 'rejected', 'canceled']` against the
  pre-mapping string, but MCP Tasks emits `'cancelled'` (British)
  and never `'rejected'` — so MCP-cancelled tasks weren't
  surfacing `statusMessage` as `error`.
- `onTaskEvents`: `'canceled'` joined the `onTaskFailed` branch
  alongside `'failed'` and `'rejected'`. Was falling through to
  `onTaskUpdated`.

**Tests**: `test/lib/poll-task-completion-terminal-states.test.js`
expanded from 4 to 9 cases:
- Pre-existing 3 rejected-status tests + 1 generic-fallback test
- New: 2 paused-state tests (input-required, auth-required)
- New: 1 paused-state status-preservation regression
- New: 2 regression tests for the pre-existing
  `failed` / `canceled` branches (the shared
  FAILED|CANCELED|REJECTED return block is a refactor surface)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(core): polling sends include_result: true per adcp#3126

adcontextprotocol/adcp#3126 (merged) closed the spec ambiguity
flagged in adcp#3123 by adding typed `include_result` request flag
+ `result` response field to `tasks/get` (AdCP 3.1.0). Updates the
SDK to:

- **Send `include_result: true` on every polling request**. Gates
  the typed `result` field on spec-conformant 3.1.0+ sellers.
  Wire-compatible: pre-3.1.0 sellers ignore the unknown request
  field.
- **Drop the informal `task_data` alias from the response mapper**.
  Pre-#3126 the mapper checked `flat.result ?? flat.task_data` as a
  speculative passthrough across two undocumented field names.
  AdCP 3.1.0 names `result` canonically; the alias is no longer
  needed and was a guess that didn't match the spec.
- **Update mapper JSDoc** to reference adcp#3126 as the resolution
  (closed) instead of adcp#3123 (the original ambiguity ticket).
- **Update the stale `#973` JSDoc** on `getTaskStatus` — that
  parser limitation was already resolved by PR #981. Replaced with
  a comment explaining why we send `include_result: true`.
- **Replace the `task_data` alias test** with one that asserts the
  request includes `include_result: true`.

The 3.0.0 schema cache doesn't have the new field yet (3.1.0
hasn't shipped); a follow-up will run `npm run sync-schemas` once
the spec release lands. Until then, the SDK speaks the 3.1.0 wire
shape and existing 3.0.0 schema validation continues to pass
(both `include_result` and `result` are optional with
additionalProperties: true).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot mentioned this pull request Apr 25, 2026
Merged
This was referenced Apr 25, 2026
Merged
Closed
Closed
bokelley added a commit that referenced this pull request Apr 25, 2026
@bokelley @claude
patch: force_task_completion + create_media_buy_async submitted -> co…
7628e14 
…mpleted roundtrip (#3138)

* patch: force_task_completion controller scenario + create_media_buy_async storyboard extends to full submitted -> completed roundtrip

Closes the loop on #3081 / #3104 / #3115. The create_media_buy_async
storyboard previously validated only the submitted-envelope wire shape
(status='submitted', task_id present, no media_buy_id); the submitted ->
completed transition was deferred because no controller scenario could
resolve the task deterministically. This PR adds the missing primitive
and extends the storyboard to exercise the full async lifecycle,
anchoring the spec invariants formalized in #3126 (typed result +
include_result on tasks/get).

New controller scenario force_task_completion: registers a result
payload (validated against async-response-data.json) against an existing
submitted task. The seller stores it; subsequent tasks/get with
include_result=true MUST surface it verbatim. Returns the standard
StateTransitionSuccess shape with previous_state='submitted' /
current_state='completed'. Sellers emit NOT_FOUND on unknown task_ids
and INVALID_TRANSITION on already-terminal tasks.

Storyboard extension (v1.0.0 -> v1.1.0): adds tasks_get to required_tools
and two new phases. force_task_completion captures the task_id from
phase 2 and registers a fixture CreateMediaBuyResponse as the result.
poll_task_completed calls tasks/get with include_result and asserts
status='completed' plus result.media_buy_id matches the registered
value verbatim — catches sellers that fabricate a fresh id and break
the polling contract.

Out of scope: webhook delivery (lives in dedicated webhook-receiver
storyboards) and transport-level wire-shape probes (runner-side at
adcp-client#904).

Why patch: conformance-suite content addition + new sandbox-only
controller scenario, both opt-in via UNKNOWN_SCENARIO grading. No
on-wire seller obligations change for sellers that already implement
tasks/get with include_result per the schema.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: drop storyboard extension from this PR — pair it with the training-agent implementation

CI legacy-dispatch failed: force_task_completed step returned UNKNOWN_SCENARIO
(no seller implements force_task_completion yet), and because the storyboard's
earlier phases already pass, the runner cannot grade the whole storyboard
not_applicable — it grades the failing step as a step-level failure, dropping
the clean-storyboard count from 53 to 52 (below the floor).

Same pattern as #3104 -> #3115: ship the controller scenario alone here, then
extend the storyboard alongside the seller-side implementation in the
follow-up. Mid-storyboard UNKNOWN_SCENARIO doesn't gracefully degrade today;
shipping the two changes together keeps the runner happy.

Reverts the storyboard to v1.0.0 (no force_task_completion phases). The
controller-scenario additions to the request schema, response schema enum,
and comply-test-controller.mdx remain. Changeset narrowed to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* patch: address expert review feedback on force_task_completion doc + schema

Twelve targeted improvements from the protocol-expert / security-reviewer /
docs-expert reviews:

- Cross-link target was wrong (#async-task-polling didn't exist) — point at
  the real anchor docs/building/implementation/async-operations#polling-for-submitted-operations.
- Replaced the fragile in-page (#state-transition-responses-force_) link
  with an inline JSON example of the response shape, matching how peer
  scenarios document responses.
- Source-state ambiguity resolved: source state MUST be 'submitted',
  'working', or 'input-required'; any other source returns INVALID_TRANSITION.
  Real tasks transition through working before completing.
- Fix typo: 'cancelled' -> 'canceled' (matches the canonical task-status enum).
- task_id is now scoped to the caller's authenticated sandbox account.
  Sellers MUST return NOT_FOUND (not FORBIDDEN, per the multi-tenant
  convention) for cross-account task_ids — closes a tenant-isolation gap
  the prior wording allowed.
- Result-validation MUST: seller emits INVALID_PARAMS if result doesn't
  validate against the response branch for the task's original method.
- Soft 256 KB cap on result payloads to bound the sandbox-amplified
  storage / echo DoS vector against the seller's task store.
- Replay semantics documented explicitly: identical params before terminal =
  idempotent no-op; diverging params before terminal = last-write-wins
  (matches force_create_media_buy_arm's overwrite precedent); any replay
  after terminal = INVALID_TRANSITION.
- Push-notification MUST: forcing completion fires the buyer's
  push_notification_config webhook with the same payload tasks/get would
  return. Otherwise storyboards can't test push delivery.
- simulate_delivery / simulate_budget_spend addressability MUST: once a
  task is force-completed with a CreateMediaBuyResponse carrying
  media_buy_id, the resulting media buy MUST be addressable by those
  scenarios. Round-trips become a real path to seed media buys.
- Replaced 'verbatim' wording: caller-supplied fields preserved, sellers
  MAY augment with seller-controlled fields (created_at, dsp_* IDs) but
  MUST NOT overwrite caller-supplied values. Matches how real async
  completions actually work.
- Added dual-use untrusted-echo note: result is buyer-controlled in
  sandbox and round-trips, buyers polling tasks/get MUST treat the
  response as untrusted seller output (per AdCP convention) regardless
  of who originated the bytes. Pre-empts a buyer-side test that trusts
  the echo and ships an unsanitized parser to production.

Schema description tightened to mirror the doc — caller-supplied
preservation, seller augmentation rules, INVALID_PARAMS triggers, soft
size cap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 25, 2026
Open
Merged
bokelley added a commit that referenced this pull request Apr 28, 2026
@bokelley @claude
chore(release): align changesets and revert tasks/get for 3.0.1 cut (#…
904eb1a 
…3373)

* Revert "feat(schema): add result and include_result to tasks/get (#3126)"

This reverts commit 4136a4a.

* chore(release): align changesets and envelope schema for 3.0.1 cut

Five changesets downgraded minor → patch where the underlying change is
annotation-only, source-schema refactor, conformance-harness only, or a
clarification of underspecified behavior:
add-seed-creative-format-pagination, fix-format-asset-oneof-titles,
fix-get-signals-max-results-precedence, hoist-duplicate-inline-enums,
hoist-inline-enum-duplicates-tranche-2.

Resolve the envelope-prohibition overlap on protocol-envelope.json: keep
the top-level not: { anyOf: [{ required: [task_status] }, { required:
[response_status] }] } constraint from envelope-forbid-legacy-status-fields,
remove the redundant per-property not: {} markers from the v3 envelope
integrity PR, update the storyboard narrative to reference the remaining
constraint, and rewrite v3-envelope-integrity-conformance.md as patch
scoped to the storyboard contribution.

After the prior tasks/get revert, npx changeset status reports a single
patch bump for adcontextprotocol.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(comply): align force_task_completion with 3.0 webhook delivery

Reframe the buyer-side observation from include_result polling to
push_notification_config webhook delivery, matching the 3.0 canonical
path for completion payload retrieval. The tasks/get polling response
returns terminal status only in 3.0; the typed result projection is
tracked for 3.1 (#3123).

- comply-test-controller.mdx: three call sites — overview, push-
  notification cross-protocol obligation, buyer-side observation —
  reworded to webhook-first with tasks/get reporting status only.
- comply-test-controller-request.json: result param description
  reframed to webhook delivery as the canonical path.
- force-task-completion-roundtrip.md changeset: remove the
  "(formalized in #3126)" reference, since #3126 is reverted to
  hold for 3.1.0.

The scenario itself is unchanged: the seller still stamps the
registered result against the task_id and surfaces it via webhook;
this commit only realigns the documentation surface to the 3.0
delivery path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley mentioned this pull request Apr 28, 2026
Merged
5 tasks
bokelley added a commit that referenced this pull request Apr 28, 2026
@bokelley @claude
Reapply "feat(schema): add result and include_result to tasks/get (#3126
5984ac2 
)"

This reverts commit f4bce25.
bokelley added a commit that referenced this pull request Apr 28, 2026
@bokelley @claude
feat(schema): re-add result and include_result to tasks/get for 3.1.0 (…
868a051 
…#3427)

* Reapply "feat(schema): add result and include_result to tasks/get (#3126)"

This reverts commit f4bce25.

* docs(contributing): absolute URL for substitution-observer-runner link

Convert the relative path to a GitHub absolute URL in both the live docs
file and the 3.0.1 snapshot.

The live docs/contributing/storyboard-authoring.md resolves the relative
path correctly from docs/contributing/. But the 3.0.1 snapshot at
dist/docs/3.0.1/contributing/ is two levels deeper, so ../../static/...
resolves to dist/docs/static/... which doesn't exist. broken-links CI
fails on the snapshot.

scripts/rewrite-dist-links.sh only rewrites /docs/ and /schemas/ paths;
it doesn't adjust depth for relative links to other top-level dirs.

Fix: live file links to main; snapshot pins to v3.0.1 (matches freeze
semantics). Both absolute URLs, immune to path-depth changes.

Pre-existing bug from the 3.0.1 snapshot landing in #3414; #3427 just
happens to be the first PR that triggered the broken-links scan
against dist/docs/3.0.1/.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

clarify: tasks/get response — where does the completion payload live?

2 participants

@bokelley @claude