Add dashboard workspace artifact reads#34
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
WalkthroughThis PR implements WorkOS web callback idempotency and cursor/limit pagination for web artifacts. It updates contracts/OpenAPI and route errors, extends WorkOS verification to expose session/token identifiers and remote JWKS caching, adds parsePagination and idempotency-key logic in API handlers, implements cursor pagination and idempotent resolveWebMember in Local and Postgres repositories (with DB query helpers), expands tests across API/DB/WorkOS, and updates docs to record the completed dashboard-read endpoints. Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant WorkOS
participant DB
Client->>API: POST /v1/auth/web/callback (JWT)
API->>WorkOS: verifyWebToken(token)
WorkOS-->>API: WebCallbackIdentity (token_id or session_id)
API->>DB: resolveWebMember({ workosUserId, email, idempotencyKey })
DB-->>API: webAuthResponse
Client->>API: GET /v1/web/artifacts?limit=2&cursor=abc
API->>API: parsePagination(limit,cursor)
API->>DB: listWebArtifacts(actor, {cursor, limit})
DB-->>API: { items, page_info }
API-->>Client: 200 { items, page_info }
Possibly related PRs
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-34.isaac-a46.workers.dev |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/db/src/local-repository.ts`:
- Around line 731-742: The decodeWebArtifactCursor function currently accepts
any string for raw.created_at; change it to validate and normalize that field:
after confirming raw.created_at is a string, parse it with new
Date(raw.created_at), throw an error if the result is an invalid date
(isNaN(date.getTime())), and if valid return created_at normalized via
date.toISOString() (instead of the original string) along with id; ensure the
function (decodeWebArtifactCursor) still throws on invalid cursor formats or
invalid dates so downstream string-based cursor comparisons match Postgres
canonical ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0aa4e649-5558-4848-98d4-295c5143153f
📒 Files selected for processing (11)
apps/api/src/index.test.tsapps/api/src/index.tsdocs/ops/project-status.mddocs/ops/web-app-todo.mdpackages/contracts/openapi/api.jsonpackages/contracts/src/openapi/api.tspackages/contracts/src/routes.tspackages/db/src/index.test.tspackages/db/src/local-repository.tspackages/db/src/postgres/repository.tspackages/db/src/queries/artifacts.ts
32786ab to
9bc4d38
Compare
fcd5d91 to
f819aa5
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-34.isaac-a46.workers.dev |
|
@codereview review |
f819aa5 to
cf09800
Compare
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-34.isaac-a46.workers.dev |
cf09800 to
002b3a4
Compare
002b3a4 to
8cd3c15
Compare
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-34.isaac-a46.workers.dev |
* feat(api): add WorkOS web auth callback * fix(api): address WorkOS callback review * fix(api): address WorkOS callback review follow-ups * fix(api): address WorkOS callback review follow-ups
8cd3c15 to
eda9d27
Compare
eda9d27 to
b850a8c
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-34.isaac-a46.workers.dev |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/workos.ts (1)
31-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t require callback-only claims in the shared WorkOS authenticator.
resolveWorkOsIdentity()is now used as a gate for every/v1/web/*request, but it returnsnullunlesssidorjtiis present. Those fields are still optional onWorkOsIdentity, so a valid WorkOS access token without them now fails general web auth instead of only failing the callback idempotency path. Keep this helper returning a plainWorkOsIdentity, and enforcetoken_id/session_idonly insidewebAuthCallback().🔧 Suggested direction
-export async function resolveWorkOsIdentity( +export async function resolveWorkOsIdentity( bearerValue: string, options: WorkOsVerificationOptions, -): Promise<WebCallbackIdentity | null> { +): Promise<WorkOsIdentity | null> { @@ - const identity = { + return { workos_user_id: user.id, email: user.email, ...(verified.sessionId ? { session_id: verified.sessionId } : {}), - }; - if (verified.tokenId) { - return { ...identity, token_id: verified.tokenId }; - } - if (verified.sessionId) { - return { workos_user_id: user.id, email: user.email, session_id: verified.sessionId }; - } - return null; + ...(verified.tokenId ? { token_id: verified.tokenId } : {}), + }; }Then keep the existing
hasWebCallbackId()check inwebAuthCallback()as the callback-specific guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/workos.ts` around lines 31 - 61, resolveWorkOsIdentity currently forces presence of sessionId (sid) or tokenId (jti) and returns null unless one is present; change it to always return a WorkOsIdentity (with optional token_id and session_id when present) for any valid user/token combo: parseBearerToken, verifyWorkOsAccessToken, fetchWorkOsUser, build and return an identity object containing workos_user_id and email and include token_id and/or session_id only if verified.tokenId/verified.sessionId exist, but do not return null solely for missing callback-only claims. Keep the callback-specific guard in webAuthCallback by using hasWebCallbackId() there to enforce token_id/session_id for the idempotency path.packages/db/src/local-repository.ts (1)
218-263:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake local web-member provisioning atomic.
Lines 225 and 236 persist the workspace/member before the awaited
generateApiKey(...)on Line 238. If that call rejects, retries will hit the existing-member path and never create the default key, leaving local state inconsistent with Postgres.Suggested fix
const workspace: Workspace = { id: crypto.randomUUID(), name: `${input.email.split("@")[0] ?? "user"}'s Workspace`, contact_email: input.email, created_at: now, updated_at: now, }; - this.workspaces.set(workspace.id, workspace); const member: WorkspaceMember = { id: createId("mem"), workspace_id: workspace.id, workos_user_id: input.workosUserId, @@ created_at: now, last_seen_at: now, }; - this.workspaceMembers.set(member.id, member); const generated = await generateApiKey(this.options.apiKeyEnv ?? "preview", this.options.apiKeyPepper); const apiKey: ApiKey = { id: createId("key"), workspace_id: workspace.id, @@ last_used_at: null, created_at: now, }; + this.workspaces.set(workspace.id, workspace); + this.workspaceMembers.set(member.id, member); this.apiKeys.set(apiKey.id, apiKey);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/local-repository.ts` around lines 218 - 263, The workspace/member creation is persisted before awaiting generateApiKey, causing partial state if generateApiKey rejects; change the flow in the function that performs web-member provisioning to generate the API key first (call generateApiKey(this.options.apiKeyEnv ?? "preview", this.options.apiKeyPepper)) and only after it resolves, create and set the Workspace (this.workspaces.set) and WorkspaceMember (this.workspaceMembers.set), then create and set the ApiKey (this.apiKeys.set) and emit events (addEvent), finally return webAuthResponse; alternatively, if you must persist early, wrap persistence in a rollback path that removes the workspace and member on generateApiKey failure to keep local state consistent with Postgres.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/contracts/openapi/api.json`:
- Around line 4298-4306: The cursor query parameter schema currently allows any
string length; update the "cursor" parameter definition (the object with "name":
"cursor", "in": "query") to include "minLength": 1 and "maxLength": 500 so it
matches the existing cursor contract and rejects empty/oversized values while
keeping "required": false and the type "string".
In `@packages/contracts/src/openapi/api.ts`:
- Around line 107-108: The OpenAPI param description for limit currently only
mentions the max; update the queryIntegerParam call for "limit" to include the
default page size (50) so docs/SDKs reflect runtime behavior used by
parsePagination(); e.g., change the description passed to
queryIntegerParam("limit", ...) to state "Maximum number of artifacts to return,
up to 100. Defaults to 50 when omitted." and keep the existing
cursor/queryStringParam unchanged.
In `@packages/db/src/postgres/repository.ts`:
- Around line 360-375: In listWebArtifacts, validate the incoming
pagination.limit before using it: if pagination.limit is defined and is not a
positive integer (<= 0 or NaN), throw a clear RangeError (or similar) to fail
fast instead of passing invalid values to SQL; then compute const limit =
pagination.limit ?? 50 and continue using that limit for the query and
pagination logic (references: listWebArtifacts, pagination.limit,
decodeWebArtifactCursor, encodeWebArtifactCursor). Ensure the check happens at
the top of the function so negative or zero values are rejected immediately.
---
Outside diff comments:
In `@apps/api/src/workos.ts`:
- Around line 31-61: resolveWorkOsIdentity currently forces presence of
sessionId (sid) or tokenId (jti) and returns null unless one is present; change
it to always return a WorkOsIdentity (with optional token_id and session_id when
present) for any valid user/token combo: parseBearerToken,
verifyWorkOsAccessToken, fetchWorkOsUser, build and return an identity object
containing workos_user_id and email and include token_id and/or session_id only
if verified.tokenId/verified.sessionId exist, but do not return null solely for
missing callback-only claims. Keep the callback-specific guard in
webAuthCallback by using hasWebCallbackId() there to enforce token_id/session_id
for the idempotency path.
In `@packages/db/src/local-repository.ts`:
- Around line 218-263: The workspace/member creation is persisted before
awaiting generateApiKey, causing partial state if generateApiKey rejects; change
the flow in the function that performs web-member provisioning to generate the
API key first (call generateApiKey(this.options.apiKeyEnv ?? "preview",
this.options.apiKeyPepper)) and only after it resolves, create and set the
Workspace (this.workspaces.set) and WorkspaceMember (this.workspaceMembers.set),
then create and set the ApiKey (this.apiKeys.set) and emit events (addEvent),
finally return webAuthResponse; alternatively, if you must persist early, wrap
persistence in a rollback path that removes the workspace and member on
generateApiKey failure to keep local state consistent with Postgres.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d49fd4d1-48ba-4740-90d0-815f178c9978
📒 Files selected for processing (13)
apps/api/src/index.test.tsapps/api/src/index.tsapps/api/src/workos.test.tsapps/api/src/workos.tsdocs/ops/project-status.mddocs/ops/web-app-todo.mdpackages/contracts/openapi/api.jsonpackages/contracts/src/openapi/api.tspackages/contracts/src/routes.tspackages/db/src/index.test.tspackages/db/src/local-repository.tspackages/db/src/postgres/repository.tspackages/db/src/queries/artifacts.ts
💤 Files with no reviewable changes (1)
- docs/ops/project-status.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/db/src/local-repository.ts`:
- Around line 293-315: In listWebArtifacts, add the same validation as
PostgresRepository for pagination.limit: if pagination.limit is provided and is
not a positive integer (>0) throw a clear error (fail-fast) before using it to
compute limit; keep using decodeWebArtifactCursor, compareArtifactsForWeb and
encodeWebArtifactCursor as-is and only proceed when limit is valid (or absent,
default to 50).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e9364bcc-d42a-4a8f-bce9-738a27ecf45d
📒 Files selected for processing (13)
apps/api/src/index.test.tsapps/api/src/index.tsapps/api/src/workos.test.tsapps/api/src/workos.tsdocs/ops/project-status.mddocs/ops/web-app-todo.mdpackages/contracts/openapi/api.jsonpackages/contracts/src/openapi/api.tspackages/contracts/src/routes.tspackages/db/src/index.test.tspackages/db/src/local-repository.tspackages/db/src/postgres/repository.tspackages/db/src/queries/artifacts.ts
b850a8c to
86433e0
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-34.isaac-a46.workers.dev |
86433e0 to
2e8e5a8
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-34.isaac-a46.workers.dev |
|
@coderabbitai resolve |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/contracts/src/openapi/api.ts`:
- Around line 14-19: The cursor length constraints are duplicated between the
queryCursorParam helper and applyWebArtifactCursorParameterBounds; either remove
the redundant minLength/maxLength from one place or add a clarifying comment
explaining why both are required (e.g., note that .optional() strips openapi
schema propagation so post-processing is needed). Update either queryCursorParam
(remove minLength/maxLength in Cursor.openapi) or
applyWebArtifactCursorParameterBounds (remove the re-application of bounds), and
if you keep both, add an inline comment referencing queryCursorParam and the
.optional() call to justify the duplication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c0eab229-4224-4bfe-b544-fb77038643c2
📒 Files selected for processing (13)
apps/api/src/index.test.tsapps/api/src/index.tsapps/api/src/workos.test.tsapps/api/src/workos.tsdocs/ops/project-status.mddocs/ops/web-app-todo.mdpackages/contracts/openapi/api.jsonpackages/contracts/src/openapi/api.tspackages/contracts/src/routes.tspackages/db/src/index.test.tspackages/db/src/local-repository.tspackages/db/src/postgres/repository.tspackages/db/src/queries/artifacts.ts
2e8e5a8 to
a439136
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
✅ Actions performedComments resolved and changes approved. |
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-34.isaac-a46.workers.dev |
|
agent-paste PR preview resources were cleaned up. The pr-preview-${context.issue.number} environment is left in place; remove it from the GitHub UI if desired. |
Summary
Implements the first dashboard read tranche after the WorkOS callback so authenticated Workspace Members can load workspace context and artifact data through the API. This intentionally leaves keys, audit, settings, and admin lockdown endpoints for later PRs.
Changes
GET /v1/web/workspace,GET /v1/web/artifacts, andGET /v1/web/artifacts/{artifactId}behavior inapps/api.not_foundbehavior in contracts/OpenAPI.Risk: MEDIUM
apps/api,packages/dbrepository/query reads,packages/contracts, status docs.not_found.created_at, id; no new writes or migrations.artifact_not_foundto genericnot_found.Test plan
pnpm --filter @agent-paste/api testpnpm --filter @agent-paste/db testpnpm --filter @agent-paste/contracts testpnpm --filter @agent-paste/contracts openapi:checkpnpm --filter @agent-paste/api typecheckpnpm --filter @agent-paste/contracts typecheckpnpm --filter @agent-paste/db typecheckpnpm --filter @agent-paste/db db:checkpnpm verifyCodeRabbit notes
coderabbit review --agent --base agents/workos-callback-apicould not run: CodeRabbit returned a usage-credit rate limit (waitTime: 9 minutes and 17 seconds). No CodeRabbit findings were available to address.Summary by CodeRabbit
New Features
Improvements
Tests
Documentation