Add provider identity resolution API#686
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a GitHub provider identity resolution flow spanning the web and control-plane services. It introduces a canonical user ID validator, a control-plane route to upsert and resolve provider identities, router integration to serve the endpoint as SCM-agnostic, and a web API endpoint that invokes the control-plane to resolve the current authenticated user. ChangesProvider Identity Resolution Implementation
Sequence DiagramsequenceDiagram
participant Client
participant WebAPI as Web API
participant Auth as AuthSession
participant ControlPlane as Control-plane
participant UserStore as User Store
Client->>WebAPI: GET /api/me
WebAPI->>Auth: getServerSession()
Auth-->>WebAPI: session with GitHub user ID
WebAPI->>ControlPlane: PUT /provider-identities/github/:login (login,email,displayName,avatarUrl)
ControlPlane->>UserStore: resolveOrCreateUser(provider:"github", providerUserId, identity)
UserStore-->>ControlPlane: { userId: canonical_32_char_hex }
ControlPlane-->>WebAPI: { userId }
WebAPI-->>Client: 200 { userId }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
There was a problem hiding this comment.
Summary
PR #686, "Add provider identity resolution API" by @ColeMurray adds canonical user ID validation, an HMAC-authenticated control-plane provider identity upsert endpoint, and a web /api/me route that resolves the current NextAuth GitHub user to the canonical D1 user ID. Overall the design is straightforward and well covered by targeted tests; I left one non-blocking inline comment about returning 400 instead of 500 for a JSON null request body.
Files changed: 9, +500/-1.
Critical Issues
None found.
Suggestions
- Error Handling
packages/control-plane/src/routes/provider-identities.ts:41- Validate that the parsed JSON body is a non-null object before reading optional metadata fields so valid-but-wrong JSON likenullreturns 400 instead of surfacing as an internal error.
Nitpicks
None.
Positive Feedback
- The new route remains behind the existing internal HMAC auth path and is explicitly SCM-provider agnostic only for this endpoint.
- The web route derives identity from the server-side NextAuth session rather than trusting client-supplied user identity.
- Tests cover auth/router integration, unsupported providers, path decoding failures, invalid canonical IDs, and web error forwarding.
Questions
None.
Verdict
Comment: feedback provided, no blocking issues found.
Validation note: I attempted to run npm test -w @open-inspect/shared -- user-id from a temporary PR worktree, but that worktree did not have dependencies installed (vitest unavailable), so I did not complete local test execution.
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
* Allow Slack sessions to use OpenCode titles (ColeMurray#685) ## Summary - Stop pre-filling Slack-created session titles from the Slack message or repo fallback - Leave Slack session titles unset so OpenCode-generated title events can populate them - Add a regression assertion for the Slack session creation payload ## Validation - npm run build -w @open-inspect/shared - npm test -w @open-inspect/slack-bot - npm run typecheck -w @open-inspect/slack-bot - npm run lint -w @open-inspect/slack-bot <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Removed title field from session creation requests to the control plane. * **Tests** * Updated tests to verify title field is not included in session creation requests. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/685?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> * Add provider identity resolution API (ColeMurray#686) ## Summary - Add a shared canonical user ID validator for 32-character lowercase hex D1 user IDs. - Add an internal HMAC-authenticated `PUT /provider-identities/:provider/:providerUserId` control-plane route that upserts provider identity metadata through `UserStore.resolveOrCreateUser` and returns the canonical `userId`. - Add browser-facing `GET /api/me`, deriving GitHub identity from the server-side NextAuth session and returning only the canonical user ID. ## Validation - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/shared -- user-id` - `npm run typecheck -w @open-inspect/shared` - `npm run lint -w @open-inspect/shared -- --quiet` - `npm test -w @open-inspect/control-plane -- routes/provider-identities.test.ts router.provider-identities.test.ts` - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane -- --quiet` - `npm test -w @open-inspect/web -- app/api/me/route.test.ts` - `npm run typecheck -w @open-inspect/web` - `npm run lint -w @open-inspect/web -- --quiet` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * New API to return the authenticated user's GitHub-derived identity and canonical user ID. * New endpoints to upsert GitHub provider identities and return a canonical user ID. * Shared validator ensuring canonical 32-character lowercase hex user IDs. * **Bug Fixes** * Improved input validation and explicit error responses for invalid provider-user IDs and malformed requests. * Better error handling when identity resolution returns unexpected values. * **Tests** * Added integration and unit tests covering success, validation failures, and error paths. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/686?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> * Filter sessions by creator (ColeMurray#688) ## Summary - Add `createdBy` session-list filtering for canonical user IDs in the control plane and web BFF - Add a sidebar All/Mine filter that resolves the current canonical user via `/api/me` - Revalidate all session-list SWR cache variants and add a D1 index for creator-filtered recency queries ## Validation - `npm test -w @open-inspect/control-plane -- src/db/session-index.test.ts` - `npm run test:integration -w @open-inspect/control-plane -- test/integration/auth.test.ts` - `npm test -w @open-inspect/web -- src/lib/session-list.test.ts src/lib/control-plane-query.test.ts src/components/session-sidebar.test.tsx` - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane -- --quiet` - `npm run typecheck -w @open-inspect/web` - `npm run lint -w @open-inspect/web -- --quiet` - `npm run build -w @open-inspect/shared` - `npm run build -w @open-inspect/control-plane` - `npm run build -w @open-inspect/web` - `npx prettier --check ...changed TS/TSX files` - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Sidebar ownership filter (All vs Mine); session list supports filtering by creator (createdBy) and scope=mine resolves the current user. * **Performance** * New DB index to speed creator-filtered session queries. * **Improvements** * Unified session-list cache keys and more reliable cache invalidation for rename, archive/unarchive, and pagination. * **Bug Fixes** * Invalid createdBy or scope now return proper 400 errors. * **Tests** * Added coverage for createdBy/scope handling, cache-key behavior, and related flows. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/688?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> * fix(modal): support non-main Modal environments (ColeMurray#687) ## Summary - Takes over ColeMurray#629 on a new branch because the original PR head is company-owned. - Adds `modal_environment` through the Modal Terraform module so secrets and deploy commands run in the selected Modal environment. - Adds `modal_environment_web_suffix` / `MODAL_ENVIRONMENT_WEB_SUFFIX` for Modal endpoint URL hosts, keeping it separate from the CLI environment name. - Keeps `MODAL_WORKSPACE` as the raw workspace, passes `MODAL_ENVIRONMENT` for dashboard links, and uses the web suffix for Modal API endpoint slugs. - Documents the new Modal environment settings and wires them into Terraform GitHub Actions. ## Addresses Review Feedback - Owner thread on `terraform/environments/production/modal.tf`: Modal CLI now receives `MODAL_ENVIRONMENT` for both secret creation and deploys. - Owner thread on `packages/control-plane/src/sandbox/client.ts`: endpoint hosts now use explicit Modal web suffix instead of deriving from environment name. - CodeRabbit validation note on `modal_environment`: rejects empty values, colons, slashes, and backslashes for Modal deployments, including at the module boundary. - CodeRabbit deploy script note: validates all required env vars before first use under `set -u`. ## Validation - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/control-plane -- --run src/sandbox/client.test.ts` - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane` - `terraform fmt -recursive -check terraform` - `terraform -chdir=terraform/environments/production init -backend=false` - `terraform -chdir=terraform/environments/production validate` - `bash -n terraform/modules/modal-app/scripts/deploy.sh` - `bash -n terraform/modules/modal-app/scripts/create-secrets.sh` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Add selectable Modal deployment environments and pass environment to Modal clients and deploy tooling * Configurable environment-to-endpoint web-suffix for Modal URLs and health checks * **Documentation** * Updated setup, CI, and Terraform docs to document workspace, environment, and web-suffix and new secrets/vars * **Tests** * Added tests for workspace-slug generation and environment-specific client health endpoints <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/687?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Kyle Adams <kadams54@users.noreply.github.com> * fix(slack-bot): cap App Home repo-override list under Slack's block limit (ColeMurray#689) ## Problem The App Home tab renders **one block per repo-specific branch override** with no upper bound (`slack-bot/src/index.ts`). Slack's `views.publish` rejects any view over **100 blocks**, so once a user configures ~85+ repo overrides the entire Home tab fails to publish — including the UI needed to manage those overrides, leaving the user stuck. ## Fix - Cap rendered overrides at **50** (`MAX_RENDERED_REPO_OVERRIDES`), well under the 100-block ceiling (~15 base blocks). - Surface the remainder as a `…and N more` summary instead of dropping it silently. - Hidden overrides stay **fully manageable** via the existing "Search repository" selector → modal, which clears any repo's override on empty submit — so the cap strands nothing. ## Test Adds a regression test that drives the real `app_home_opened` → `views.publish` path with 60 overrides and asserts the published view stays ≤ 100 blocks, renders exactly 50 rows, and includes the "10 more overrides" note. Full slack-bot suite (87 tests) + typecheck pass. Found while syncing this change into a downstream fork. Happy to adjust the cap value or copy. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved handling of large numbers of repository overrides in the Slack App Home interface. The interface now displays up to 50 overrides with a summary note indicating additional hidden overrides, ensuring functionality within Slack's technical constraints. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/689?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> * Refactor Slack App Home module (ColeMurray#690) ## Summary - move Slack App Home publishing, interaction handling, view construction, modals, metadata, model lookup, and local Slack view types into focused `app-home/` modules - extract user preference persistence/resolution into `user-preferences.ts` - keep the `/interactions` route delegating App Home-specific payloads while leaving generic Slack interaction handling in `index.ts` - move App Home view unit coverage into `app-home.test.ts` ## Validation - `npm run typecheck -w @open-inspect/slack-bot` - `npm run lint -w @open-inspect/slack-bot` - `npx prettier --check packages/slack-bot/src/app-home packages/slack-bot/src/app-home.test.ts packages/slack-bot/src/user-preferences.ts` - `git diff --check` - `npm test -w @open-inspect/slack-bot` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * App Home settings: model selection, reasoning-effort choice, global and per-repo branch overrides with modals. * Repo search suggestions and truncated option labels to fit Slack limits; long override lists show a “more” summary. * **Refactor** * Centralized App Home interaction routing and unified preference resolution. * App Home publishing made asynchronous and robust. * **Tests** * Expanded coverage for App Home view, suggestions, truncation, and preference updates. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <colemurray.cs@gmail.com> Co-authored-by: Kyle Adams <kadams54@users.noreply.github.com> Co-authored-by: Merlin <40275364+merlin-mk@users.noreply.github.com>
Summary
PUT /provider-identities/:provider/:providerUserIdcontrol-plane route that upserts provider identity metadata throughUserStore.resolveOrCreateUserand returns the canonicaluserId.GET /api/me, deriving GitHub identity from the server-side NextAuth session and returning only the canonical user ID.Validation
npm run build -w @open-inspect/sharednpm test -w @open-inspect/shared -- user-idnpm run typecheck -w @open-inspect/sharednpm run lint -w @open-inspect/shared -- --quietnpm test -w @open-inspect/control-plane -- routes/provider-identities.test.ts router.provider-identities.test.tsnpm run typecheck -w @open-inspect/control-planenpm run lint -w @open-inspect/control-plane -- --quietnpm test -w @open-inspect/web -- app/api/me/route.test.tsnpm run typecheck -w @open-inspect/webnpm run lint -w @open-inspect/web -- --quietSummary by CodeRabbit
New Features
Bug Fixes
Tests