Skip to content

fix: Restore owner.organization_id on user-owned API keys - #1669

Open
KatBrandt wants to merge 2 commits into
mainfrom
devin/1785777114-api-key-user-owner-compat
Open

fix: Restore owner.organization_id on user-owned API keys#1669
KatBrandt wants to merge 2 commits into
mainfrom
devin/1785777114-api-key-user-owner-compat

Conversation

@KatBrandt

Copy link
Copy Markdown

Description

Customer report (Slack, Plain): API key validation started rejecting valid keys after upgrading to 10.8.0; downgrading to 10.7.0 fixed it.

#1650 made deserializeApiKey remap the user owner variant instead of passing owner through raw. That was a correct typing fix, but it silently changed the runtime shape in a minor release for user-owned keys:

 owner: {
   type: 'user',
   id: 'user_...',
-  organization_id: 'org_...',   // 10.7.0 (raw passthrough)
+  organizationId: 'org_...',    // 10.8.0
 }

Anyone parsing the deserialized object against a schema (the reporter runs zod safeParse on validation.apiKey) now fails on a valid key. Organization-owned keys are unaffected — I diffed 10.7.0 vs 10.8.0 against a stub API and their output is identical.

This emits both keys so the pre-10.8.0 shape keeps working, with organization_id marked @deprecated for removal in the next major.

Tradeoff worth a second opinion: consumers who already adapted to 10.8.0 and strict-parse the owner object will now see an unexpected extra key. The alternative is to leave 10.8.0 as-is and tell affected users to rename — that keeps the surface clean but leaves the minor-release break in place.

Not changed here: deserializeCreatedApiKey still passes owner through raw, so createOrganizationApiKey returns organization_id only and never had the organizationId field. Worth aligning separately.

Documentation

[ ] Yes

Link to Devin session: https://app.devin.ai/sessions/2f3d6911ede0413ca81245dfc637b4e0
Requested by: @KatBrandt

10.8.0 started remapping a user owner to `organizationId`, which silently
changed the runtime shape returned by `apiKeys.createValidation()` and the
list endpoints for user-owned keys. Consumers validating the deserialized
object against a schema began rejecting valid keys.

Emit both keys so the pre-10.8.0 shape keeps working; `organization_id` is
deprecated and can be dropped in the next major.
@KatBrandt
KatBrandt requested review from a team as code owners August 3, 2026 17:12
@KatBrandt
KatBrandt requested a review from stanleyphu August 3, 2026 17:12
@KatBrandt KatBrandt self-assigned this Aug 3, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor
Original prompt from kathleen.brandt

SYSTEM:
=== BEGIN THREAD HISTORY (in #dse-pre-triage) ===
Kat Brandt (U0A6AKYK3UJ) [ts=1785776214.954239]: @Devin Can you investigate whether this report is accurate?

[Slack unfurl — this is an automatic link preview, not a user message]
Quote of conversation (https://work-os.slack.com/archives/C0ADE32NA4R/p1785754076189549?thread_ts=1785754076.189549&cid=C0ADE32NA4R):
> From Nils Albroscheit
> Dear work OS team,

> We just discovered that in version 10.8.0 of the SDK (node package @`workos-inc/node`) API validation is broken for us. > Downgrading to @workos-inc/node version 10.7.0 fixed this issue. Is this something you are already aware of? Is this the right place to raise this issue, or should we raise it somewhere else?

> Best, Nils
> Posted on August 03, 2026 at 10:47 AM

<most_recent_message>
RoboHawk (U09V0J0LDEF) [ts=1785776216.575879]: <https://app.plain.com/workspace/w_01KEZ48Y1GZPN4PXQ41666E911/thread/th_01KZ3KSN4V0W4ZHT1WW1E3NVZ0/|Related Plain ticket>
</most_recent_message>
=== END THREAD HISTORY ===
Channel ID: C0BDCJL6BLL
Thread URL: https://work-os.slack.com/archives/C0BDCJL6BLL/p1785776214954239?thread_ts=1785776214.954239&amp;cid=C0BDCJL6BLL

The latest message is the one right above that tagged you. The <most_recent_message> is the message that you should use to guide your goals + task for this session, and you should use the rest of the slack thread as context.
A [ts=...] marker on a Slack message is that message's timestamp. To act on a specific message with the slack tool (e.g. adding an emoji reaction via the reaction command), pass that value as timestamp along with the Channel ID — no extra lookup call is needed.

@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@greptile-apps

greptile-apps Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Restores the pre-10.8.0 organization_id property on deserialized user-owned API keys while retaining the camelCase organizationId property.

  • Marks the legacy property as deprecated and optional so existing TypeScript object literals, fixtures, and mocks remain assignable.
  • Updates API-key validation expectations to cover both property names.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/api-keys/interfaces/api-key.interface.ts Adds the deprecated compatibility alias as optional, fully addressing the previously reported source-compatibility issue.
src/api-keys/serializers/api-key.serializer.ts Emits both camelCase and legacy snake_case organization identifiers for user-owned API keys.
src/api-keys/api-keys.spec.ts Updates the validation response expectation to verify the restored runtime property.

Reviews (2): Last reviewed commit: "fix: Make the deprecated organization_id..." | Re-trigger Greptile

Comment thread src/api-keys/interfaces/api-key.interface.ts Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified this independently before reviewing (context: customer report of apiKeys.createValidation() breaking in 10.8.0).

Regression confirmed empirically. I ran the same stub API response through 10.7.0, 10.8.0, and 10.9.0 from npm: for user-owned keys, owner went from {"type":"user","id":"user_123","organization_id":"org_123"} in 10.7.0 to {"type":"user","id":"user_123","organizationId":"org_123"} in 10.8.0/10.9.0, while org-owned owners are byte-identical across all three versions. That matches the user-branch remap #1650 added to deserializeApiKey.

This PR restores the 10.7.0 field correctly. On this branch, npx jest src/api-keys passes 12/12 and tsc --noEmit is clean. The updated spec asserts with toEqual, so it pins the dual-emit shape exactly, and every deserializeApiKey call site (listOrganizationApiKeys, validateApiKey, the event serializer) picks up the fix consistently. Dual-emit as a fix: patch is the right call over a pure revert: the only consumers who could notice are ones that already migrated to 10.8.0's shape and reject unknown keys, and 10.8.0 has only been out ~17 days.

Non-blocking notes:

  1. The @deprecated JSDoc on organization_id doesn't say when it will be removed — that lives only in the PR body. Suggest adding "will be removed in the next major" to the doc comment so the timeline survives in the code.
  2. PR body correction: org-owned keys have no organization_id field at all (owner is { type: 'organization', id } — see fixtures/validate-api-key.json), so there's no snake/camel gap on createOrganizationApiKey. The real remaining inconsistency is deserializeUserApiKey (src/user-management/serializers/user-api-key.serializer.ts), shipped in 10.9.0, which emits organizationId only for the same user-owner object. No regression there (that surface never had the snake_case key), but worth a deliberate follow-up on whether it should match.
  3. The files touched here are hand-written today (no oagen header, not in .oagen-manifest.json), but src/api-keys/api-keys.ts carries the oagen "do not edit" header — if this module is ever fully regenerated from the OpenAPI spec, this compat field would need to move into the spec/generator or it gets clobbered. A short comment next to the dual-emit line referencing the 10.8.0 regression would help it survive future cleanups.

Generated by Claude Code

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