fix: Restore owner.organization_id on user-owned API keys - #1669
fix: Restore owner.organization_id on user-owned API keys#1669KatBrandt wants to merge 2 commits into
Conversation
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.
Original prompt from kathleen.brandt
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Greptile SummaryRestores the pre-10.8.0
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "fix: Make the deprecated organization_id..." | Re-trigger Greptile |
There was a problem hiding this comment.
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:
- The
@deprecatedJSDoc onorganization_iddoesn'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. - PR body correction: org-owned keys have no
organization_idfield at all (owneris{ type: 'organization', id }— seefixtures/validate-api-key.json), so there's no snake/camel gap oncreateOrganizationApiKey. The real remaining inconsistency isdeserializeUserApiKey(src/user-management/serializers/user-api-key.serializer.ts), shipped in 10.9.0, which emitsorganizationIdonly 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. - The files touched here are hand-written today (no oagen header, not in
.oagen-manifest.json), butsrc/api-keys/api-keys.tscarries 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
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
deserializeApiKeyremap theuserowner variant instead of passingownerthrough 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
zodsafeParseonvalidation.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_idmarked@deprecatedfor 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:
deserializeCreatedApiKeystill passesownerthrough raw, socreateOrganizationApiKeyreturnsorganization_idonly and never had theorganizationIdfield. Worth aligning separately.Documentation
Link to Devin session: https://app.devin.ai/sessions/2f3d6911ede0413ca81245dfc637b4e0
Requested by: @KatBrandt