Skip to content

docs(events): note that GET /events silently ignores the order parameter#1611

Open
tsushanth wants to merge 3 commits into
workos:mainfrom
tsushanth:docs-events-order-no-op
Open

docs(events): note that GET /events silently ignores the order parameter#1611
tsushanth wants to merge 3 commits into
workos:mainfrom
tsushanth:docs-events-order-no-op

Conversation

@tsushanth

Copy link
Copy Markdown

Refs #1610.

ListEventOptions.order was added in #1524 to match the published API reference, but the live GET /events endpoint silently ignores the parameter — every value (asc, desc, normal, omitted) returns byte-identical, oldest-first pages, verified with raw curl against a sandbox key on 2026-06-11. So this is a server-side bug, not an SDK serialization one. The consequence for SDK consumers is real: anyone who upgraded for #1524 to fetch the newest event with order=desc&limit=1 silently gets the oldest event instead, with no error to root-cause from.

This PR adds a JSDoc annotation to the field pointing at #1610, explaining the current ascending behaviour and the cursor-pagination workaround. The wire format is unchanged — callers automatically pick up the server-side fix the moment it ships.

I'd rather not remove the field (that's a breaking change for downstream code that already passes the parameter through), and surfacing it at the type-system level seems like the lowest-friction way to make sure the next person reading intellisense for listEvents doesn't waste an afternoon debugging why their desc request comes back ascending. Happy to swap to a @deprecated tag instead if that's the maintainer preference.

…meter

Refs workos#1610.

`ListEventOptions.order` was added in workos#1524 to match the API reference,
but the live `GET /events` endpoint silently ignores it on every value
(`asc`, `desc`, `normal`, omitted) and always returns events oldest-first.
That's been verified with raw curl against a sandbox key on 2026-06-11,
so it's not an SDK serialization bug. A consumer who upgraded for workos#1524
to seed cursor pagination from the newest event with
`order=desc&limit=1` is silently getting the oldest event instead, with
no error to root-cause from.

Annotate the field's JSDoc with a pointer to the tracking issue so
intellisense surfaces the gotcha at the point of confusion, and explain
the current ascending behaviour and the cursor-pagination workaround.
Leaves the wire format unchanged so callers automatically pick up the
server-side fix the moment it ships.
@tsushanth tsushanth requested review from a team as code owners June 11, 2026 16:13
@tsushanth tsushanth requested a review from awolfden June 11, 2026 16:13
@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds JSDoc warning comments to ListEventOptions.order and SerializedListEventOptions.order documenting that the GET /events endpoint currently ignores the order parameter and always returns events oldest-first regardless of the value sent, with a link to the tracking issue (#1610).

  • Both the public-facing ListEventOptions interface and the wire-layer SerializedListEventOptions interface are annotated, so developers reading either surface see the caveat.
  • No type, runtime, or serialization behavior is changed — the SDK continues to forward the value on the wire so consumers automatically benefit when the server-side fix ships.

Confidence Score: 5/5

Documentation-only change; no runtime code, types, or serialization logic is modified.

The diff is two JSDoc block comments added to existing optional interface fields. Nothing is compiled, serialized, or executed differently. The previous reviewer concern about annotating only the public interface has been addressed by also annotating SerializedListEventOptions.order.

No files require special attention.

Important Files Changed

Filename Overview
src/events/interfaces/list-events-options.interface.ts Adds JSDoc to both order fields documenting the server-side bug and workaround; no runtime or type changes.

Sequence Diagram

sequenceDiagram
    participant SDK as SDK Consumer
    participant Client as WorkOS Node SDK
    participant API as GET /events

    Note over SDK,API: Current behaviour (server-side bug)
    SDK->>Client: "listEvents({ order: 'desc', limit: 1 })"
    Client->>API: "GET /events?order=desc&limit=1"
    Note over API: Server silently ignores `order`
    API-->>Client: oldest event (ascending-first page)
    Client-->>SDK: oldest event (not newest)

    Note over SDK,API: After server-side fix ships
    SDK->>Client: "listEvents({ order: 'desc', limit: 1 })"
    Client->>API: "GET /events?order=desc&limit=1"
    API-->>Client: newest event (descending-first page)
    Client-->>SDK: newest event
Loading

Reviews (3): Last reviewed commit: "docs: drop 'normal' from order doc — not..." | Re-trigger Greptile

Comment thread src/events/interfaces/list-events-options.interface.ts Outdated
…ions

Greptile pointed out (workos#1611 review) that the wire-layer interface
SerializedListEventOptions.order had no JSDoc, so a maintainer working
purely at the serialization layer would miss the caveat that lives on
ListEventOptions.order. Add a short note pointing at the canonical
JSDoc and at issue workos#1610 so both surfaces stay in sync.
@tsushanth

Copy link
Copy Markdown
Author

Thanks @greptile-apps — good catch. Pushed 23ec8f7 mirroring the JSDoc onto SerializedListEventOptions.order so the serialization-layer view also surfaces the caveat, with a back-reference to the canonical block and to #1610.

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Looks great — the mirror on SerializedListEventOptions.order is exactly right. The back-reference to ListEventOptions.order keeps the two surfaces in sync without duplicating the full prose, and the direct link to #1610 means anyone who lands here via "go to definition" on the serialized type gets the same signal. LGTM from my side.

@tsushanth

Copy link
Copy Markdown
Author

Greptile gave this 5/5 on the latest revision (see review above). Tagging @nicknisi @awolfden — based on recent merged events/SDK PRs you'd be good folks to take a look when there's a window. It's a docs-only change to the order field of ListEventOptions / SerializedListEventOptions calling out the known server-side no-op tracked in #1610. Happy to address any feedback. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant