Skip to content

fix(spec): expose event.data on Attempt when include=event.data#917

Merged
leggetter merged 3 commits into
fix/spec-sdk-fixesfrom
test/sdk-include-event-data-bug
May 28, 2026
Merged

fix(spec): expose event.data on Attempt when include=event.data#917
leggetter merged 3 commits into
fix/spec-sdk-fixesfrom
test/sdk-include-event-data-bug

Conversation

@alexluong
Copy link
Copy Markdown
Collaborator

@alexluong alexluong commented May 28, 2026

Stacked on #916.

The bug

listAttempts({ include: ['event.data', 'response_data'] }) was returning attempt.event without the data payload, even though the server returns it on the wire.

Speakeasy's TS generator emits zod unions in declaration order, and z.union matches the first member that validates — it does not pick the best fit. The OpenAPI spec listed EventSummary before EventFull, and EventSummary is a plain (non-strict) z.object that silently accepts and strips any unknown keys — including data. So the union matched EventSummary first and dropped data before EventFull ever got a turn.

sdks/outpost-typescript/src/models/components/attempt.ts (before):

export const EventUnion$inboundSchema = z.union([
  z.lazy(() => EventSummary$inboundSchema),  // ← matches first, drops `data`
  z.lazy(() => EventFull$inboundSchema),
]);

export const EventSummary$inboundSchema = z.object({
  id: z.string().optional(),
  // …
  // no `data` field, no `.strict()` — extras silently dropped
});

This is a Speakeasy TS-generator pattern (not an OpenAPI spec problem, not a server problem). Other SDKs are unaffected:

  • Python — Pydantic v2 smart-union picks EventFull when data is present ✅
  • Go — tries EventSummary with disallowUnknownFields=true, falls through to EventFull
  • TS — Zod has no equivalent in z.union, so spec order matters ❌

Fix

Swap the oneOf order so EventFull is declared first:

 event:
   nullable: true
   oneOf:
-    - $ref: "#/components/schemas/EventSummary"
     - $ref: "#/components/schemas/EventFull"
+    - $ref: "#/components/schemas/EventSummary"

After regen, EventUnion$inboundSchema lists EventFull first and matches when data is present.

Test

TDD: failing test added that asserts attempt.event.data round-trips through listAttempts({ include: ['event.data', …] }).

Verified locally

  • Regenerated TS SDK (./spec-sdk-tests/scripts/regenerate-sdk.sh TS) — EventUnion$inboundSchema now lists EventFull first
  • New test passes
  • Full spec-sdk-tests suite: 152 passing, 1 unrelated pre-existing SQS failure

SDK regen will follow via the standard Speakeasy bot flow once this merges. Considering an upstream Speakeasy request later so order-independence isn't required.

alexluong and others added 2 commits May 28, 2026 20:33
Speakeasy CLI 1.741.7 → 1.753.0 (commit 3f311e0, 2026-03-13) changed
the generated TS SDK return type for list operations from the flat
paginated body to a PageIterator wrapper: ListEventsResponse now wraps
the body in `result`, so callers access `response.result.models` instead
of `response.models`. The shape change was a side-effect of the
Speakeasy version bump and wasn't surfaced in the SDK regen PR changelog
(which only flagged the unrelated `request` query-param restyle).

Tests in spec-sdk-tests/tests/{events,tenants}.test.ts still used the
pre-bump accessor and failed to compile against the current SDK.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Speakeasy's TS generator emits zod unions in declaration order. The
non-strict EventSummary matched first and silently stripped event.data
from the parsed attempt — losing the payload added by include=event.data.
Declaring EventFull first lets it match when data is present, with
EventSummary as the fallback.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alexluong alexluong changed the title test(spec-sdk): assert event.data is exposed when include=event.data test+fix: include=event.data on listAttempts (TS SDK union ordering) May 28, 2026
@alexluong alexluong changed the title test+fix: include=event.data on listAttempts (TS SDK union ordering) fix(spec): expose event.data on Attempt when include=event.data May 28, 2026
@leggetter leggetter changed the base branch from fix/spec-sdk-tests-paginator-shape to feat/spec-sdk-fixes May 28, 2026 16:35
@leggetter leggetter merged commit ea0ff25 into fix/spec-sdk-fixes May 28, 2026
1 of 2 checks passed
@leggetter leggetter deleted the test/sdk-include-event-data-bug branch May 28, 2026 17:07
alexluong added a commit that referenced this pull request May 29, 2026
…event.data oneOf (#924)

* fix(openapi): add type discriminator to DestinationUpdate union (#920)

Trigger: the DestinationUpdate union lost its discriminator in 8be278c
(May 2025), when `type` wasn't on PATCH. Server has since added `type`
acceptance and RFC 7396 merge-patch (bd7701b), making the missing
discriminator an active bug — Hookdeck's permissive `config: {}` variant
greedily matched any partial-config payload, the SDK sent it through
with no field remap (camelCase reached the server), and merge-patch
silently no-op'd. All non-Webhook partial config updates affected.

Spec: `type` becomes a required, enum-locked discriminator on every
DestinationUpdate* variant; 17 new `*ConfigUpdate`/`*CredentialsUpdate`
companions model the now-explicit partial PATCH shape; Hookdeck's
`config: {}` removed.

Impact:
  * Breaking for typed SDK consumers: `update()` must include `type`
    after regen. Server is more lenient (still accepts PATCH without
    `type`), so raw HTTP callers keep working but are spec-noncompliant.
  * SDK regen left to the bot — separate PR. CI compile-check on this
    PR will fail until that lands; tests were validated locally
    against a 1.4.1 regen.

spec-sdk-tests (knock-on): `type` added to 46 update sites; paginator
shape fixed (supersedes fix/spec-sdk-tests-paginator-shape); SDK retries
enabled; cleanup parallelized; cleanup-error logging hardened.

Test status (managed Outpost, local regen'd SDK): 133→152 passing,
14→0 failing. spec-sdk-tests have no CI today; follow-up PR will add one.

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

* fix(spec): expose event.data on Attempt when include=event.data (#917)

* test(spec-sdk): fix paginator response shape (response.result.models)

Speakeasy CLI 1.741.7 → 1.753.0 (commit 3f311e0, 2026-03-13) changed
the generated TS SDK return type for list operations from the flat
paginated body to a PageIterator wrapper: ListEventsResponse now wraps
the body in `result`, so callers access `response.result.models` instead
of `response.models`. The shape change was a side-effect of the
Speakeasy version bump and wasn't surfaced in the SDK regen PR changelog
(which only flagged the unrelated `request` query-param restyle).

Tests in spec-sdk-tests/tests/{events,tenants}.test.ts still used the
pre-bump accessor and failed to compile against the current SDK.

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

* test(spec-sdk): assert event.data is exposed when include=event.data

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

* fix(spec): order EventFull before EventSummary in Attempt.event oneOf

Speakeasy's TS generator emits zod unions in declaration order. The
non-strict EventSummary matched first and silently stripped event.data
from the parsed attempt — losing the payload added by include=event.data.
Declaring EventFull first lets it match when data is present, with
EventSummary as the fallback.

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

---------

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

* chore(sdks): drop x-speakeasy-pagination overlay from all SDK sources (#919)

The shared pagination-config-overlay added x-speakeasy-pagination to list
endpoints, which Speakeasy then converted into PageIterator wrappers (TS),
res.Next() helpers (Go), and res.next() chaining (Python).

In TS the wrapper changed the response shape to response.result.models,
which is a noisy regression for the common single-page case. In Go and
Python the helper saves one line at the cost of hiding which request
params get carried across the cursor call.

Drop the overlay from all three sources for symmetric, explicit DX:
callers paginate manually using res.pagination.next on the flat response.

SDK regeneration is intentionally left out of this commit so the workflow
change can be reviewed in isolation.

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

* test(spec-sdk): revert paginator wrapper accessors after pagination overlay drop

Trigger: #919 (merged into umbrella) drops the x-speakeasy-pagination
overlay, so the TS SDK no longer wraps list responses in PageIterator.
events.list()/tenants.list() return EventPaginatedResult/TenantPaginatedResult
directly — accessor is `response.models`, not `response.result.models`.

Tests in #920 had been adapted to the wrapper shape; this realigns them
back to the flat shape post-regen. Local run after fresh TS SDK regen:
153 passing / 0 failing / 15 pending.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Alex Luong <alex.luong@hookdeck.com>
leggetter added a commit that referenced this pull request May 29, 2026
… a local Outpost (#925)

* fix(openapi): add type discriminator to DestinationUpdate union (#920)

Trigger: the DestinationUpdate union lost its discriminator in 8be278c
(May 2025), when `type` wasn't on PATCH. Server has since added `type`
acceptance and RFC 7396 merge-patch (bd7701b), making the missing
discriminator an active bug — Hookdeck's permissive `config: {}` variant
greedily matched any partial-config payload, the SDK sent it through
with no field remap (camelCase reached the server), and merge-patch
silently no-op'd. All non-Webhook partial config updates affected.

Spec: `type` becomes a required, enum-locked discriminator on every
DestinationUpdate* variant; 17 new `*ConfigUpdate`/`*CredentialsUpdate`
companions model the now-explicit partial PATCH shape; Hookdeck's
`config: {}` removed.

Impact:
  * Breaking for typed SDK consumers: `update()` must include `type`
    after regen. Server is more lenient (still accepts PATCH without
    `type`), so raw HTTP callers keep working but are spec-noncompliant.
  * SDK regen left to the bot — separate PR. CI compile-check on this
    PR will fail until that lands; tests were validated locally
    against a 1.4.1 regen.

spec-sdk-tests (knock-on): `type` added to 46 update sites; paginator
shape fixed (supersedes fix/spec-sdk-tests-paginator-shape); SDK retries
enabled; cleanup parallelized; cleanup-error logging hardened.

Test status (managed Outpost, local regen'd SDK): 133→152 passing,
14→0 failing. spec-sdk-tests have no CI today; follow-up PR will add one.

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

* fix(spec): expose event.data on Attempt when include=event.data (#917)

* test(spec-sdk): fix paginator response shape (response.result.models)

Speakeasy CLI 1.741.7 → 1.753.0 (commit 3f311e0, 2026-03-13) changed
the generated TS SDK return type for list operations from the flat
paginated body to a PageIterator wrapper: ListEventsResponse now wraps
the body in `result`, so callers access `response.result.models` instead
of `response.models`. The shape change was a side-effect of the
Speakeasy version bump and wasn't surfaced in the SDK regen PR changelog
(which only flagged the unrelated `request` query-param restyle).

Tests in spec-sdk-tests/tests/{events,tenants}.test.ts still used the
pre-bump accessor and failed to compile against the current SDK.

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

* test(spec-sdk): assert event.data is exposed when include=event.data

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

* fix(spec): order EventFull before EventSummary in Attempt.event oneOf

Speakeasy's TS generator emits zod unions in declaration order. The
non-strict EventSummary matched first and silently stripped event.data
from the parsed attempt — losing the payload added by include=event.data.
Declaring EventFull first lets it match when data is present, with
EventSummary as the fallback.

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

---------

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

* chore(sdks): drop x-speakeasy-pagination overlay from all SDK sources (#919)

The shared pagination-config-overlay added x-speakeasy-pagination to list
endpoints, which Speakeasy then converted into PageIterator wrappers (TS),
res.Next() helpers (Go), and res.next() chaining (Python).

In TS the wrapper changed the response shape to response.result.models,
which is a noisy regression for the common single-page case. In Go and
Python the helper saves one line at the cost of hiding which request
params get carried across the cursor call.

Drop the overlay from all three sources for symmetric, explicit DX:
callers paginate manually using res.pagination.next on the flat response.

SDK regeneration is intentionally left out of this commit so the workflow
change can be reviewed in isolation.

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

* test(spec-sdk): revert paginator wrapper accessors after pagination overlay drop

Trigger: #919 (merged into umbrella) drops the x-speakeasy-pagination
overlay, so the TS SDK no longer wraps list responses in PageIterator.
events.list()/tenants.list() return EventPaginatedResult/TenantPaginatedResult
directly — accessor is `response.models`, not `response.result.models`.

Tests in #920 had been adapted to the wrapper shape; this realigns them
back to the flat shape post-regen. Local run after fresh TS SDK regen:
153 passing / 0 failing / 15 pending.

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

* ci(spec-sdk-tests): add workflow that runs the contract suite against a local Outpost

Closes #921.

Trigger: PRs touching the spec, TS SDK, Speakeasy config, handlers,
destination providers, models, or server entry. Plus workflow_dispatch
and a weekly cron as drift safety net.

Job: spin up Postgres/Redis/RabbitMQ as service containers, build the
outpost binary from source, run migrations, start api + delivery in
background, wait for /healthz, build the TS SDK from this PR's spec
state, then run `npm test` in spec-sdk-tests pointed at localhost.

Notes:
* Uses RabbitMQ for the message queue (default in .outpost.yaml.dev).
* spec-sdk-tests/.env is written inline so we never need to commit a
  CI-specific .env to the repo.
* Server logs are dumped on failure for debuggability.

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

* ci(spec-sdk-tests): fix YAML — use block scalar for migrations run step

The quoted-string form 'run: "$OUTPOST_BIN" migrate apply --yes' confuses
the YAML parser (treats the leading quote as opening a scalar). Block-scalar
form matches the other run: steps in the file.

* ci(spec-sdk-tests): use outpost-server in singular mode, not the CLI wrapper

The 'outpost' binary is just a CLI wrapper that delegates 'serve' to
a separate 'outpost-server' binary. Running it without a subcommand
prints help and exits — which is why /healthz never came up.

Switch to building both binaries: 'outpost' for migrations, then
'outpost-server' for the actual server. Drop SERVICE=api / SERVICE=delivery
in favour of singular mode (empty SERVICE → api + log + delivery in one
process), which is enough for the contract tests and halves the moving
parts.

* ci(spec-sdk-tests): use npm install for spec-sdk-tests deps

spec-sdk-tests/.gitignore excludes package-lock.json (intentional —
the test suite isn't published, and treating SDK contract tests as
production-locked dependencies adds noise to PRs without value).
npm ci needs a lock file; switch to npm install.

* ci(spec-sdk-tests): regen TS SDK from the PR's spec before running tests

The whole point of this workflow is to validate that spec ↔ SDK ↔
server agree. If we test against the checked-in SDK src/ (which can
lag the spec arbitrarily, since regen is bot-driven), we're testing
the wrong thing — drift hides instead of surfacing.

Add a Speakeasy regen step before the existing build. Now any PR that
changes docs/apis/openapi.yaml gets its tests run against an SDK
regen'd from that PR's spec, exercising the actual contract this CI
exists to validate.

Uses the SPEAKEASY_API_KEY secret already configured for the publish
workflows.

* ci(spec-sdk-tests): use redis-stack-server for RediSearch

tenants.list requires RediSearch; vanilla redis:7-alpine returns
501 'list tenant feature is not enabled'. Swap to the official
redis/redis-stack-server image which bundles RediSearch. Same port
(6379), same redis-cli ping for health check.

* ci(spec-sdk-tests): expand header with explicit scope and rationale

Makes it clear at the top of the file what this workflow validates
(PR's server vs PR's spec), what it does not (managed drift, SDK
version compat, post-deploy smoke), and why we picked this shape
over alternatives. Spares future maintainers from re-litigating
the design decision.

* ci(spec-sdk-tests): drop the weekly cron

The cron was copied from the docs-eval pattern out of habit. In this
workflow it adds essentially no signal — between PRs, main's state
hasn't changed, so a scheduled run re-tests what we last tested. The
"deploy drift" question a cron would help with is out of scope for
this workflow anyway (it's about PR's spec vs PR's server, not
managed vs main).

* ci(spec-sdk-tests): add cmd/outpost-server/** to trigger paths

Workflow builds and runs ./cmd/outpost-server but the path filter only
watched cmd/outpost/** (the CLI wrapper). Changes to the actual server
entrypoint could land without triggering the contract suite.

Spotted by Copilot review on #925.

---------

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants