Skip to content

chore: replace sso_connection_id with workos_id in queries and app code#2246

Merged
qstearns merged 1 commit intomainfrom
drop-sso-connection-id/behavior
Apr 17, 2026
Merged

chore: replace sso_connection_id with workos_id in queries and app code#2246
qstearns merged 1 commit intomainfrom
drop-sso-connection-id/behavior

Conversation

@qstearns
Copy link
Copy Markdown
Contributor

@qstearns qstearns commented Apr 16, 2026

Remove all dependencies on sso_connection_id, there are no more dependencies on it and it seems to confuse humans and Claude alike. This PR changes behavior to target workos_id as a column instead of sso_connection_id. After we merge it, we should run the following data migration:

UPDATE organization_metadata
SET workos_id = sso_connection_id,
    updated_at = clock_timestamp()
WHERE sso_connection_id IS NOT NULL
  AND workos_id IS NULL;

There is a follow-up PR to drop the column. We first merge the behavior change and let it run for a couple days to avoid risks of data loss.

@qstearns qstearns requested a review from a team as a code owner April 16, 2026 04:59
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 16, 2026

🦋 Changeset detected

Latest commit: 2c5516b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
server Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gram-docs-redirect Ready Ready Preview, Comment Apr 17, 2026 2:33am

Request Review

@qstearns qstearns force-pushed the drop-sso-connection-id/behavior branch from 6003dcd to c08599c Compare April 16, 2026 05:09
@qstearns qstearns changed the title fix: replace sso_connection_id with workos_id in queries and app code chore: replace sso_connection_id with workos_id in queries and app code Apr 16, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@qstearns qstearns force-pushed the drop-sso-connection-id/behavior branch from bc914ea to aee305f Compare April 16, 2026 22:18
@qstearns qstearns requested a review from a team as a code owner April 16, 2026 22:18
@github-actions github-actions bot added the preview Spawn a preview environment label Apr 16, 2026
@speakeasybot
Copy link
Copy Markdown
Collaborator

speakeasybot commented Apr 16, 2026

🚀 Preview Environment (PR #2246)

Preview URL: https://pr-2246.dev.getgram.ai

Component Status Details Updated (UTC)
✅ Database Ready Existing database reused 2026-04-17 02:38:59.
✅ Images Available Container images ready 2026-04-17 02:38:42.

Gram Preview Bot

@blacksmith-sh

This comment has been minimized.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 0 new potential issues.

View 3 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 0 new potential issues.

View 5 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 new potential issues.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment on lines +43 to +49
type Organization struct {
ID string
Name string
Slug string
WorkosID *string
UserWorkspaceSlugs []string
}
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot Apr 17, 2026

Choose a reason for hiding this comment

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

📝 Info: Cache serialization compatibility with new Organization type

The CachedUserInfo.Organizations field changed from []auth.OrganizationEntry to []sessions.Organization. The Redis cache uses msgpack serialization (via go-redis/cache). Old cached entries have fields like SsoConnectionID and Projects that don't exist in the new type, while the new type has WorkosID which doesn't exist in old data. This is safe because msgpack (like JSON) ignores unknown fields and zero-initializes missing ones. WorkosID will be nil for old cached entries, which is acceptable since it's *string. The 15-minute cache TTL (server/internal/auth/sessions/types.go:12) means old entries will naturally expire quickly after deployment.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread server/internal/auth/impl.go Outdated
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 3 new potential issues.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread server/internal/auth/impl.go
Comment on lines 351 to 360
})
}

// write through organization metadata when not from cache to keep entries updated
// TODO: there may be a better place to do this
if !fromCache {
if _, err := s.orgRepo.UpsertOrganizationMetadata(ctx, orgRepo.UpsertOrganizationMetadataParams{
ID: org.ID,
Name: org.Name,
Slug: org.Slug,
SsoConnectionID: conv.PtrToPGText(org.SsoConnectionID),
Whitelisted: pgtype.Bool{Bool: false, Valid: false},
}); err != nil {
return nil, oops.E(oops.CodeUnexpected, err, "error upserting organization metadata").Log(ctx, s.logger)
}
}

organizations = append(organizations, &gen.OrganizationEntry{
ID: org.ID,
Name: org.Name,
Slug: org.Slug,
SsoConnectionID: org.SsoConnectionID,
UserWorkspaceSlugs: org.UserWorkspaceSlugs,
Projects: orgProjects,
})
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot Apr 17, 2026

Choose a reason for hiding this comment

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

📝 Info: Info method no longer upserts org metadata on cache miss — equivalent behavior

The Info method removed the !fromCache upsert loop that previously wrote org metadata on every cache miss. This is equivalent because GetUserInfo on cache miss calls GetUserInfoFromSpeakeasysyncOrgsAndWorkOSIDs, which upserts all orgs. The old code's fromCache variable (now _ at line 295) was used to conditionally run the upsert. The new code achieves the same effect implicitly — org metadata is always upserted when fetching fresh data from the IDP.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +1 to +5
---
"server": patch
---

Remove the legacy column sso_connection_id
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.

🚩 sso_connection_id column not dropped from schema despite changeset title

The changeset at .changeset/petite-cooks-like.md says 'Remove the legacy column sso_connection_id', but the database schema (server/database/schema.sql:54) still has the sso_connection_id column, and no migration drops it. The PR only removes code that WRITES to the column. The generated sqlc code (e.g., server/internal/organizations/repo/queries.sql.go:359) still scans sso_connection_id from SELECT * queries. This is likely intentional (gradual deprecation), but the changeset title is misleading. A follow-up migration will be needed to actually drop the column.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration[bot]

This comment was marked as resolved.

…ication code

Introduce sessions.Organization as an internal type to decouple the
cached user info from the Goa API surface. The Speakeasy IDP's workos_id
field now flows through to organization_metadata.workos_id via the upsert.

Removes sso_connection_id from the Goa OrganizationEntry design and
cleans up the frontend reference.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 0 new potential issues.

View 8 additional findings in Devin Review.

Open in Devin Review

@qstearns qstearns merged commit 7b925e4 into main Apr 17, 2026
28 checks passed
@qstearns qstearns deleted the drop-sso-connection-id/behavior branch April 17, 2026 02:40
@github-actions github-actions bot locked and limited conversation to collaborators Apr 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

preview Spawn a preview environment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants