Serve realtime and MCP from the web app on Vercel - #87
Merged
Conversation
The server owned both the Bun socket handling and every piece of realtime state, so nothing could drive it from another transport. Move the connections map, presence, authentication, Redis fan-out and heartbeat into a hub that accepts an abstract socket, and leave Bun.serve as a thin adapter over it. A second adapter can now attach a different socket implementation to the same hub.
The web app needs the same hub to serve sockets from a route handler, and cross-app code belongs in packages rather than being imported out of another app. Move authentication, the connection, presence, the socket abstraction and the hub into @orbit/realtime-server, leaving the app with its Bun.serve adapter and entrypoint.
Vercel functions can upgrade a request through @vercel/functions, so the web app can host the socket the client already expects instead of a separate service. Attach the shared hub to the upgraded socket in a route handler, and default the client to the same origin so no realtime host has to be configured. NEXT_PUBLIC_REALTIME_URL still wins when it is set, which keeps the local Bun server and any external host working. A health route reports whether Redis is configured and what the hub is holding, because publishing silently does nothing without REDIS_URL.
The Vercel route hands the hub a ws socket rather than a Bun one, and nothing covered that adapter. Run authentication, scope authorisation and redis delta delivery over a real ws server so the transport the web app uses is exercised the same way the Bun one already is.
The MCP server ran as its own deployable, which the move to a single Vercel project leaves nowhere to run, and /mcp has been returning 404 since the migration. Its transport was tied to node http, so swap it for the SDK's web standard transport and expose a fetch handler that a route can call directly. Tools and the handler move into @orbit/mcp-server so they keep their own suite instead of joining the web app's DOM tests, and the tests now drive the handler through the client transport rather than a listening socket.
Vercel only upgrades websockets on its node runtime, and the bun runtime was chosen because the data layer imports Bun.SQL. Move the driver to postgres.js behind the same pool cache and transaction pooler detection. Its unsafe path does not serialise a Date, which drizzle uses for raw execute, so the cycle queries now pass timestamps as ISO strings.
Bun.RedisClient only exists on the bun runtime, and the hub has to run inside a Vercel function on node. Move the publisher and the subscriber to ioredis, which keeps working under bun for the local socket server.
Bun.S3Client cannot run on the node runtime the websocket upgrade needs. Move to the aws sdk and its presigner, which take explicit credentials without ever reaching for an ambient session token. That removes the probe which refused to start when one leaked in, and with it the need to unset AWS_SESSION_TOKEN before running the suite.
Vercel only injects the websocket upgrade bridge on its node runtime. The bun runtime was selected because the server code reached for Bun built-ins, and with those gone the pin can go too. Passwords move to @node-rs/argon2 and sortable ids to uuid v7 behind a shared helper. Both argon2 implementations read the same PHC digest, so existing passwords keep verifying, which a test now covers.
The health route only described a hub that some other request had already built, so it answered ok while the socket route was failing to reach redis. Open the hub and surface its connection state or the error.
Awaiting the hub first meant the handler did redis work before the upgrade, and the connection never reached a 101. Upgrade immediately, then attach the hub, holding any frames that arrive in between.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
pulkitxm has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change moves deployed services toward Node-compatible libraries, adds shared realtime and MCP packages, hosts WebSocket and MCP routes in the web app, replaces container and Kubernetes deployment with Vercel guidance, and updates storage, authentication, database, CI, and infrastructure configuration. ChangesRuntime and platform migration
Shared realtime service
MCP package and route
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant WebSocketRoute
participant RealtimeHub
participant Redis
Browser->>WebSocketRoute: GET /api/ws upgrade
WebSocketRoute->>RealtimeHub: Create or reuse hub
WebSocketRoute->>RealtimeHub: Accept socket and buffer early messages
RealtimeHub->>Redis: Authenticate and subscribe
Redis-->>RealtimeHub: Delta or presence message
RealtimeHub-->>Browser: Deliver authorized realtime update
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Nothing is containerised now that the app ships as one Vercel project, and the images job still built an mcp image whose app no longer exists, so it could not pass. Remove the job, the Dockerfiles and the manifests.
|
Too many files changed for review. ( Bypass the limit by tagging |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (10)
apps/web/src/lib/realtime/provider.tsx (1)
43-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the SSR-aware URL resolution into one shared hook.
The same
typeof window === 'undefined'resolution also exists inapps/web/src/features/inbox/inbox-realtime.tsxandapps/web/src/features/pulls/pulls-realtime.tsx. Three copies will drift. Move the branch into ause-realtime-url.tshook next tourl.tsand call it from all three components.♻️ Proposed refactor
Add
apps/web/src/lib/realtime/use-realtime-url.ts:'use client'; import { resolveRealtimeUrl } from './url.ts'; export function useRealtimeUrl(configured: string): string { if (typeof window === 'undefined') return configured; return resolveRealtimeUrl(configured, window.location.origin); }Then in
provider.tsx:- const socketUrl = - typeof window === 'undefined' ? url : resolveRealtimeUrl(url, window.location.origin); + const socketUrl = useRealtimeUrl(url);🤖 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/web/src/lib/realtime/provider.tsx` around lines 43 - 44, Extract the SSR-aware URL selection into a shared useRealtimeUrl hook beside url.ts, preserving the configured URL during SSR and using resolveRealtimeUrl with window.location.origin in the browser. Replace the duplicated typeof window resolution in the provider, inbox realtime, and pulls realtime components with this hook.packages/realtime-server/src/hub.ts (1)
104-108: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider failing fast when
REDIS_URLis not set.The hub throws when
BETTER_AUTH_SECRETis missing, but it silently falls back toredis://localhost:6380whenREDIS_URLis missing. The web app now creates this hub inside a serverless function, where localhost has no Redis. In that case clients connect and authenticate, and the hub then drops every delta while only logging Redis connection errors. Apply the same explicit failure, or keep the localhost default for local development only.♻️ Proposed change
- const redisUrl = options.redisUrl ?? process.env['REDIS_URL'] ?? 'redis://localhost:6380'; + const configuredRedisUrl = options.redisUrl ?? process.env['REDIS_URL']; + if (configuredRedisUrl === undefined && process.env['NODE_ENV'] === 'production') { + throw new Error('REDIS_URL is required to run the realtime hub.'); + } + const redisUrl = configuredRedisUrl ?? 'redis://localhost:6380';🤖 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/realtime-server/src/hub.ts` around lines 104 - 108, Update the Redis configuration in the hub initialization to avoid silently using redis://localhost:6380 when REDIS_URL is absent in serverless deployments. Apply the same required-configuration validation used for ticketSecret to redisUrl, or conditionally retain the localhost fallback only for explicit local-development environments, while preserving configured REDIS_URL behavior.apps/web/src/features/inbox/inbox-realtime.tsx (1)
28-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the browser realtime URL resolution into one helper. Both components repeat the same
typeof window === 'undefined'branch aroundresolveRealtimeUrl. A third copy exists inapps/web/src/lib/realtime/provider.tsx. Add oneuse-realtime-url.tshook, or export abrowserRealtimeUrl(configured)helper fromapps/web/src/lib/realtime/url.ts, and call it from every site.
apps/web/src/features/inbox/inbox-realtime.tsx#L28-L33: replace the inline branch with the shared helper and keep passing the result toRealtimeProvider.apps/web/src/features/pulls/pulls-realtime.tsx#L19-L24: replace the identical inline branch with the same shared helper.🤖 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/web/src/features/inbox/inbox-realtime.tsx` around lines 28 - 33, Extract the repeated browser/SSR realtime URL selection into one shared helper, such as browserRealtimeUrl in the realtime URL module, and update the RealtimeProvider setup in apps/web/src/features/inbox/inbox-realtime.tsx lines 28-33 and apps/web/src/features/pulls/pulls-realtime.tsx lines 19-24 to call it. Also replace the existing equivalent branch in apps/web/src/lib/realtime/provider.tsx with the same helper, preserving configured URLs during SSR and resolveRealtimeUrl behavior in the browser.packages/realtime-server/tsconfig.json (1)
3-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBan accidental
Bunglobals in non-test source.
src/socket.tsrequires the explicitServerWebSockettype import forfromBunSocket, but the package source must not add Bun runtime dependencies. Add a lint rule forBunreferences in non-test files, or isolate the Bun adapter from the source compiled for web imports.🤖 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/realtime-server/tsconfig.json` around lines 3 - 6, Update the realtime-server TypeScript/lint configuration around the compilerOptions and include entries to prevent Bun globals from being used in non-test source files, while preserving the explicit ServerWebSocket type import required by fromBunSocket. Add a rule scoped to non-test source, or isolate the Bun adapter from code compiled for web imports without introducing Bun runtime dependencies.Source: Coding guidelines
packages/mcp-server/src/logger.ts (1)
26-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude the stack trace in
errorFields.
errorFieldskeeps onlyerror.message. Adderror.stackwhen available. This helps diagnose crashes from the structured logs inserver.tsandtools/support.tswithout needing to reproduce the failure locally.♻️ Proposed refactor
export function errorFields(error: unknown): LogFields { - return { error: error instanceof Error ? error.message : String(error) }; + return { + error: error instanceof Error ? error.message : String(error), + ...(error instanceof Error && error.stack ? { stack: error.stack } : {}), + }; }🤖 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/mcp-server/src/logger.ts` around lines 26 - 28, The errorFields function currently omits available stack traces from structured error logs. Update errorFields to include error.stack when the input is an Error with a stack, while preserving the existing message and String(error) handling for all other cases.packages/mcp-server/src/resolve.ts (1)
31-36: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider archived teams in
resolveTeam.
resolveTeamcallslistTeams(principal)with default options, so archived teams are excluded.resolveProjectat Line 77 passesincludeArchived: true. Thelist_teamstool also exposes archived teams. A caller that reads an archived team fromlist_teamscannot then pass that team tolist_statesorsearch_issues. Align the two resolvers, or document the exclusion in the tool descriptions.🤖 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/mcp-server/src/resolve.ts` around lines 31 - 36, Update resolveTeam to call listTeams with includeArchived: true, matching resolveProject and the list_teams tool so archived team references resolve for downstream operations.packages/mcp-server/src/views.ts (1)
74-84: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLoad workflow states for all teams in parallel.
The loop awaits
listWorkflowStatesonce per team, in sequence.search_issuesreturns up to 200 issues, which can span many teams for an admin principal. Each round trip adds latency to the tool response.♻️ Proposed change
const stateNames = new Map<string, string>(); - for (const teamId of new Set(rows.map((row) => row.teamId))) { - for (const state of await listWorkflowStates(principal, teamId)) { - stateNames.set(state.id, state.name); - } - } + const teamIds = [...new Set(rows.map((row) => row.teamId))]; + const stateLists = await Promise.all( + teamIds.map((teamId) => listWorkflowStates(principal, teamId)), + ); + for (const states of stateLists) { + for (const state of states) stateNames.set(state.id, state.name); + }🤖 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/mcp-server/src/views.ts` around lines 74 - 84, Update the workflow-state loading loop around listWorkflowStates to start requests for all unique team IDs concurrently and await them together, then populate stateNames from the aggregated results while preserving the existing state ID-to-name mapping.packages/mcp-server/src/tools/planning.ts (1)
88-89: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winResolve team references without one query per reference.
The loop awaits
resolveTeamonce per element, andresolveTeamruns a fulllistTeamsquery on every call. Theteamsarray accepts up to 50 entries, so onecreate_projectcall can issue 50 sequential team queries. The same pattern exists inpackages/mcp-server/src/tools/admin.tsat Lines 51-52 forinvite_member.Add a batch resolver in
packages/mcp-server/src/resolve.tsthat loads the team list once and resolves every reference against it, then use it in both tools.♻️ Proposed batch resolver
// packages/mcp-server/src/resolve.ts export async function resolveTeamIds( principal: Principal, refs: readonly string[], ): Promise<string[]> { if (refs.length === 0) return []; const teams = await listTeams(principal); return refs.map((ref) => { const found = pick(teams, ref, (team) => [team.id, team.key, team.name]); if (found === undefined) throw notFound(`No team matches "${ref}".`); return found.id; }); }- const teamIds: string[] = []; - for (const ref of args.teams ?? []) teamIds.push((await resolveTeam(principal, ref)).id); + const teamIds = await resolveTeamIds(principal, args.teams ?? []);🤖 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/mcp-server/src/tools/planning.ts` around lines 88 - 89, Issue: Team references are resolved with one full team-list query per reference. Add a resolveTeamIds function in resolve.ts that returns early for empty refs, loads listTeams once, matches each reference against team id, key, or name using the existing pick/notFound behavior, and returns the matched IDs; replace the per-reference resolveTeam loops in create_project planning.ts and invite_member admin.ts with this batch resolver.packages/mcp-server/src/tools/admin.ts (1)
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
z.email()for the email schema. Zod 4.4.3 deprecatesz.string().email()in favor of the top-level format schema.🤖 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/mcp-server/src/tools/admin.ts` at line 45, Update the email field schema in the admin invite tool to use Zod’s top-level z.email() format schema instead of z.string().email(), while preserving its existing description and validation behavior.packages/mcp-server/src/tools/support.ts (1)
59-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant double assertion.
z.object(config.inputSchema)is assignable toz.ZodObject<Shape>with Zod 4.4.3. Pass it directly. In SDK 1.29.0,registerToolorders generics as<OutputArgs, InputArgs>; the firstz.ZodRawShapeis the output generic.🤖 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/mcp-server/src/tools/support.ts` around lines 59 - 60, Remove the redundant double assertion from the inputSchema initialization and pass z.object(config.inputSchema) directly as the z.ZodObject<Shape> value. Preserve the registerTool generic ordering, where z.ZodRawShape is the output type and z.ZodObject<Shape> is the input type.
🤖 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 `@apps/web/src/app/`(app)/layout.tsx:
- Around line 15-16: Update realtimeUrl() to validate the configured realtime
URL with a Zod schema, accepting only non-empty ws: or wss: URLs while
preserving '' for the same-origin fallback. Ensure malformed or unsupported
values are rejected before they reach new WebSocket(options.url), using the
existing configuredRealtimeUrl() source.
In `@apps/web/src/app/api/realtime/health/route.ts`:
- Around line 35-38: Update the catch block in the health route to stop
returning describe(error) in the public response; use a generic hub error value
instead, and log the original error server-side while preserving the 503
response and other fields.
In `@apps/web/src/app/api/ws/route.ts`:
- Around line 11-42: Bound the pre-accept message queue in attach by enforcing a
maximum buffered-frame count before pushing new messages. When the cap is
exceeded, close the socket using the established overload/limit close behavior
and stop buffering or processing further frames; preserve normal buffering and
replay once realtimeHub resolves.
In `@apps/web/src/app/mcp/route.ts`:
- Around line 1-9: Update the handle function’s handleMcpRequest options to use
the centralized publicAppUrl() helper instead of reading NEXT_PUBLIC_APP_URL
through serverEnv(), preserving centralized URL normalization and the existing
request handling flow.
In `@apps/web/src/lib/auth/password.ts`:
- Around line 16-22: Update verifyPassword so it explicitly validates the
digest’s PHC format and returns false only for malformed input or an invalid
password result. Remove the catch-all error suppression around verify, allowing
unrelated Argon2/runtime failures to propagate; use the existing verifyPassword
symbol and the Argon2 API’s expected digest structure for validation.
In `@apps/web/src/lib/realtime/hub.ts`:
- Around line 5-11: Update realtimeHub so a rejection from the cached
createRealtimeHub promise removes globalForHub.orbitRealtimeHub before
propagating the error, allowing subsequent calls to retry creation while
preserving the existing promise reuse on success.
In `@apps/web/src/lib/realtime/url.ts`:
- Around line 3-5: Update configuredRealtimeUrl to use static
process.env.NEXT_PUBLIC_REALTIME_URL access, then validate the configured value
with Zod and return it only when it parses as a WebSocket URL with a ws: or wss:
origin; otherwise preserve the existing empty-string fallback before the value
reaches RealtimeProvider.
In `@CLAUDE.md`:
- Around line 57-60: Align the deployment and runtime documentation across
CLAUDE.md lines 57-60, CLAUDE.md lines 27-28, and README.md line 27: remove or
rewrite standalone Bun container/realtime/MCP service instructions to describe
the single Node-based Vercel app, replace Bun.password guidance with
`@node-rs/argon2`, and update the end-to-end runtime description to
Node-compatible integrations. Also ensure shipped web server code does not
import or call Bun built-ins.
In `@packages/mcp-server/src/comments.ts`:
- Around line 35-47: In the comment-creation transaction around the insert in
add_comment, validate any non-empty parsed.parentId by querying the parent
comment and requiring matching principal.organizationId and issue.id; reject a
missing parent with notFound and reject a parent belonging to another
organization or issue. Only insert after this validation, preserving null/empty
parent behavior.
In `@packages/mcp-server/src/tools/identity.ts`:
- Around line 82-104: The member-directory permission is missing, allowing
unauthorized email access through listMembers consumers. Define the intended
directory-read permission in the shared policy, enforce it within listMembers
before returning member data, and ensure list_users, list_members, and web
member endpoints use that protected path while preserving existing role
permissions.
In `@packages/mcp-server/src/tools/support.ts`:
- Around line 29-40: Update failed to stop spreading errorFields(error) into
logger.warn; log only the tool name, domain.code, and domain.status. Preserve
the existing response body and error handling behavior while ensuring
caller-supplied messages and identifiers are not written to logs.
In `@packages/mcp-server/src/views.ts`:
- Around line 89-93: Update describeIssue to throw the typed internal domain
error from `@orbit/shared/errors` instead of a bare Error when describeIssues
returns no view. Follow the existing internal(...) usage in comments.ts and
preserve the current message and control flow.
---
Nitpick comments:
In `@apps/web/src/features/inbox/inbox-realtime.tsx`:
- Around line 28-33: Extract the repeated browser/SSR realtime URL selection
into one shared helper, such as browserRealtimeUrl in the realtime URL module,
and update the RealtimeProvider setup in
apps/web/src/features/inbox/inbox-realtime.tsx lines 28-33 and
apps/web/src/features/pulls/pulls-realtime.tsx lines 19-24 to call it. Also
replace the existing equivalent branch in apps/web/src/lib/realtime/provider.tsx
with the same helper, preserving configured URLs during SSR and
resolveRealtimeUrl behavior in the browser.
In `@apps/web/src/lib/realtime/provider.tsx`:
- Around line 43-44: Extract the SSR-aware URL selection into a shared
useRealtimeUrl hook beside url.ts, preserving the configured URL during SSR and
using resolveRealtimeUrl with window.location.origin in the browser. Replace the
duplicated typeof window resolution in the provider, inbox realtime, and pulls
realtime components with this hook.
In `@packages/mcp-server/src/logger.ts`:
- Around line 26-28: The errorFields function currently omits available stack
traces from structured error logs. Update errorFields to include error.stack
when the input is an Error with a stack, while preserving the existing message
and String(error) handling for all other cases.
In `@packages/mcp-server/src/resolve.ts`:
- Around line 31-36: Update resolveTeam to call listTeams with includeArchived:
true, matching resolveProject and the list_teams tool so archived team
references resolve for downstream operations.
In `@packages/mcp-server/src/tools/admin.ts`:
- Line 45: Update the email field schema in the admin invite tool to use Zod’s
top-level z.email() format schema instead of z.string().email(), while
preserving its existing description and validation behavior.
In `@packages/mcp-server/src/tools/planning.ts`:
- Around line 88-89: Issue: Team references are resolved with one full team-list
query per reference. Add a resolveTeamIds function in resolve.ts that returns
early for empty refs, loads listTeams once, matches each reference against team
id, key, or name using the existing pick/notFound behavior, and returns the
matched IDs; replace the per-reference resolveTeam loops in create_project
planning.ts and invite_member admin.ts with this batch resolver.
In `@packages/mcp-server/src/tools/support.ts`:
- Around line 59-60: Remove the redundant double assertion from the inputSchema
initialization and pass z.object(config.inputSchema) directly as the
z.ZodObject<Shape> value. Preserve the registerTool generic ordering, where
z.ZodRawShape is the output type and z.ZodObject<Shape> is the input type.
In `@packages/mcp-server/src/views.ts`:
- Around line 74-84: Update the workflow-state loading loop around
listWorkflowStates to start requests for all unique team IDs concurrently and
await them together, then populate stateNames from the aggregated results while
preserving the existing state ID-to-name mapping.
In `@packages/realtime-server/src/hub.ts`:
- Around line 104-108: Update the Redis configuration in the hub initialization
to avoid silently using redis://localhost:6380 when REDIS_URL is absent in
serverless deployments. Apply the same required-configuration validation used
for ticketSecret to redisUrl, or conditionally retain the localhost fallback
only for explicit local-development environments, while preserving configured
REDIS_URL behavior.
In `@packages/realtime-server/tsconfig.json`:
- Around line 3-6: Update the realtime-server TypeScript/lint configuration
around the compilerOptions and include entries to prevent Bun globals from being
used in non-test source files, while preserving the explicit ServerWebSocket
type import required by fromBunSocket. Add a rule scoped to non-test source, or
isolate the Bun adapter from code compiled for web imports without introducing
Bun runtime dependencies.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b53e228-e1d7-4ebf-be4b-829cddc0c7c0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (96)
.env.example.github/workflows/ci.ymlCLAUDE.mdREADME.mdapps/mcp/Dockerfileapps/mcp/README.mdapps/mcp/src/env.tsapps/mcp/src/index.tsapps/mcp/src/server.tsapps/realtime/package.jsonapps/realtime/src/active-organization.test.tsapps/realtime/src/client-reconnect.test.tsapps/realtime/src/index.tsapps/realtime/src/membership-revocation.test.tsapps/realtime/src/node-socket.test.tsapps/realtime/src/server.test.tsapps/realtime/src/server.tsapps/realtime/src/session-revocation.test.tsapps/realtime/src/test-helpers.tsapps/web/next.config.tsapps/web/package.jsonapps/web/src/app/(app)/inbox/page.tsxapps/web/src/app/(app)/layout.tsxapps/web/src/app/(app)/pulls/page.tsxapps/web/src/app/api/integrations/github/route.tsapps/web/src/app/api/realtime/health/route.tsapps/web/src/app/api/webhooks/github/route.tsapps/web/src/app/api/ws/route.tsapps/web/src/app/mcp/route.tsapps/web/src/features/inbox/inbox-realtime.tsxapps/web/src/features/pulls/pulls-realtime.tsxapps/web/src/lib/auth/password.test.tsapps/web/src/lib/auth/password.tsapps/web/src/lib/auth/server.tsapps/web/src/lib/integrations/oauth-state.tsapps/web/src/lib/realtime/hub.tsapps/web/src/lib/realtime/provider.tsxapps/web/src/lib/realtime/url.test.tsapps/web/src/lib/realtime/url.tsapps/web/vercel.jsonpackages/core/package.jsonpackages/core/src/analytics/burndown.tspackages/core/src/realtime/publisher.tspackages/db/package.jsonpackages/db/src/client.tspackages/db/src/ensure-extensions.tspackages/mcp-server/bunfig.tomlpackages/mcp-server/package.jsonpackages/mcp-server/src/auth.test.tspackages/mcp-server/src/comments.tspackages/mcp-server/src/index.tspackages/mcp-server/src/logger.tspackages/mcp-server/src/resolve.tspackages/mcp-server/src/server.tspackages/mcp-server/src/test-helpers.tspackages/mcp-server/src/tools.test.tspackages/mcp-server/src/tools/admin.tspackages/mcp-server/src/tools/identity.tspackages/mcp-server/src/tools/index.tspackages/mcp-server/src/tools/issues.tspackages/mcp-server/src/tools/planning.tspackages/mcp-server/src/tools/support.tspackages/mcp-server/src/views.tspackages/mcp-server/tests-preload.tspackages/mcp-server/tsconfig.jsonpackages/realtime-server/bunfig.tomlpackages/realtime-server/package.jsonpackages/realtime-server/src/auth.test.tspackages/realtime-server/src/auth.tspackages/realtime-server/src/connection.test.tspackages/realtime-server/src/connection.tspackages/realtime-server/src/hub.tspackages/realtime-server/src/index.tspackages/realtime-server/src/logger.tspackages/realtime-server/src/presence.tspackages/realtime-server/src/socket.tspackages/realtime-server/tests-preload.tspackages/realtime-server/tsconfig.jsonpackages/services/package.jsonpackages/services/src/email/email.test.tspackages/services/src/email/index.tspackages/services/src/github/apply.test.tspackages/services/src/github/apply.tspackages/services/src/github/install.test.tspackages/services/src/github/install.tspackages/services/src/notifications/index.tspackages/services/src/notifications/notifications.test.tspackages/services/src/slack/dispatch.test.tspackages/services/src/slack/dispatch.tspackages/services/src/storage/credentials.test.tspackages/services/src/storage/credentials.tspackages/services/src/storage/parent.test.tspackages/services/src/storage/s3.tspackages/services/src/storage/validate.tspackages/shared/package.jsonpackages/shared/src/utils/index.ts
💤 Files with no reviewable changes (6)
- apps/mcp/src/env.ts
- .env.example
- apps/mcp/README.md
- apps/mcp/src/server.ts
- apps/mcp/Dockerfile
- apps/mcp/src/index.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 12
🧹 Nitpick comments (10)
apps/web/src/lib/realtime/provider.tsx (1)
43-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the SSR-aware URL resolution into one shared hook.
The same
typeof window === 'undefined'resolution also exists inapps/web/src/features/inbox/inbox-realtime.tsxandapps/web/src/features/pulls/pulls-realtime.tsx. Three copies will drift. Move the branch into ause-realtime-url.tshook next tourl.tsand call it from all three components.♻️ Proposed refactor
Add
apps/web/src/lib/realtime/use-realtime-url.ts:'use client'; import { resolveRealtimeUrl } from './url.ts'; export function useRealtimeUrl(configured: string): string { if (typeof window === 'undefined') return configured; return resolveRealtimeUrl(configured, window.location.origin); }Then in
provider.tsx:- const socketUrl = - typeof window === 'undefined' ? url : resolveRealtimeUrl(url, window.location.origin); + const socketUrl = useRealtimeUrl(url);🤖 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/web/src/lib/realtime/provider.tsx` around lines 43 - 44, Extract the SSR-aware URL selection into a shared useRealtimeUrl hook beside url.ts, preserving the configured URL during SSR and using resolveRealtimeUrl with window.location.origin in the browser. Replace the duplicated typeof window resolution in the provider, inbox realtime, and pulls realtime components with this hook.packages/realtime-server/src/hub.ts (1)
104-108: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider failing fast when
REDIS_URLis not set.The hub throws when
BETTER_AUTH_SECRETis missing, but it silently falls back toredis://localhost:6380whenREDIS_URLis missing. The web app now creates this hub inside a serverless function, where localhost has no Redis. In that case clients connect and authenticate, and the hub then drops every delta while only logging Redis connection errors. Apply the same explicit failure, or keep the localhost default for local development only.♻️ Proposed change
- const redisUrl = options.redisUrl ?? process.env['REDIS_URL'] ?? 'redis://localhost:6380'; + const configuredRedisUrl = options.redisUrl ?? process.env['REDIS_URL']; + if (configuredRedisUrl === undefined && process.env['NODE_ENV'] === 'production') { + throw new Error('REDIS_URL is required to run the realtime hub.'); + } + const redisUrl = configuredRedisUrl ?? 'redis://localhost:6380';🤖 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/realtime-server/src/hub.ts` around lines 104 - 108, Update the Redis configuration in the hub initialization to avoid silently using redis://localhost:6380 when REDIS_URL is absent in serverless deployments. Apply the same required-configuration validation used for ticketSecret to redisUrl, or conditionally retain the localhost fallback only for explicit local-development environments, while preserving configured REDIS_URL behavior.apps/web/src/features/inbox/inbox-realtime.tsx (1)
28-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the browser realtime URL resolution into one helper. Both components repeat the same
typeof window === 'undefined'branch aroundresolveRealtimeUrl. A third copy exists inapps/web/src/lib/realtime/provider.tsx. Add oneuse-realtime-url.tshook, or export abrowserRealtimeUrl(configured)helper fromapps/web/src/lib/realtime/url.ts, and call it from every site.
apps/web/src/features/inbox/inbox-realtime.tsx#L28-L33: replace the inline branch with the shared helper and keep passing the result toRealtimeProvider.apps/web/src/features/pulls/pulls-realtime.tsx#L19-L24: replace the identical inline branch with the same shared helper.🤖 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/web/src/features/inbox/inbox-realtime.tsx` around lines 28 - 33, Extract the repeated browser/SSR realtime URL selection into one shared helper, such as browserRealtimeUrl in the realtime URL module, and update the RealtimeProvider setup in apps/web/src/features/inbox/inbox-realtime.tsx lines 28-33 and apps/web/src/features/pulls/pulls-realtime.tsx lines 19-24 to call it. Also replace the existing equivalent branch in apps/web/src/lib/realtime/provider.tsx with the same helper, preserving configured URLs during SSR and resolveRealtimeUrl behavior in the browser.packages/realtime-server/tsconfig.json (1)
3-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBan accidental
Bunglobals in non-test source.
src/socket.tsrequires the explicitServerWebSockettype import forfromBunSocket, but the package source must not add Bun runtime dependencies. Add a lint rule forBunreferences in non-test files, or isolate the Bun adapter from the source compiled for web imports.🤖 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/realtime-server/tsconfig.json` around lines 3 - 6, Update the realtime-server TypeScript/lint configuration around the compilerOptions and include entries to prevent Bun globals from being used in non-test source files, while preserving the explicit ServerWebSocket type import required by fromBunSocket. Add a rule scoped to non-test source, or isolate the Bun adapter from code compiled for web imports without introducing Bun runtime dependencies.Source: Coding guidelines
packages/mcp-server/src/logger.ts (1)
26-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude the stack trace in
errorFields.
errorFieldskeeps onlyerror.message. Adderror.stackwhen available. This helps diagnose crashes from the structured logs inserver.tsandtools/support.tswithout needing to reproduce the failure locally.♻️ Proposed refactor
export function errorFields(error: unknown): LogFields { - return { error: error instanceof Error ? error.message : String(error) }; + return { + error: error instanceof Error ? error.message : String(error), + ...(error instanceof Error && error.stack ? { stack: error.stack } : {}), + }; }🤖 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/mcp-server/src/logger.ts` around lines 26 - 28, The errorFields function currently omits available stack traces from structured error logs. Update errorFields to include error.stack when the input is an Error with a stack, while preserving the existing message and String(error) handling for all other cases.packages/mcp-server/src/resolve.ts (1)
31-36: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider archived teams in
resolveTeam.
resolveTeamcallslistTeams(principal)with default options, so archived teams are excluded.resolveProjectat Line 77 passesincludeArchived: true. Thelist_teamstool also exposes archived teams. A caller that reads an archived team fromlist_teamscannot then pass that team tolist_statesorsearch_issues. Align the two resolvers, or document the exclusion in the tool descriptions.🤖 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/mcp-server/src/resolve.ts` around lines 31 - 36, Update resolveTeam to call listTeams with includeArchived: true, matching resolveProject and the list_teams tool so archived team references resolve for downstream operations.packages/mcp-server/src/views.ts (1)
74-84: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLoad workflow states for all teams in parallel.
The loop awaits
listWorkflowStatesonce per team, in sequence.search_issuesreturns up to 200 issues, which can span many teams for an admin principal. Each round trip adds latency to the tool response.♻️ Proposed change
const stateNames = new Map<string, string>(); - for (const teamId of new Set(rows.map((row) => row.teamId))) { - for (const state of await listWorkflowStates(principal, teamId)) { - stateNames.set(state.id, state.name); - } - } + const teamIds = [...new Set(rows.map((row) => row.teamId))]; + const stateLists = await Promise.all( + teamIds.map((teamId) => listWorkflowStates(principal, teamId)), + ); + for (const states of stateLists) { + for (const state of states) stateNames.set(state.id, state.name); + }🤖 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/mcp-server/src/views.ts` around lines 74 - 84, Update the workflow-state loading loop around listWorkflowStates to start requests for all unique team IDs concurrently and await them together, then populate stateNames from the aggregated results while preserving the existing state ID-to-name mapping.packages/mcp-server/src/tools/planning.ts (1)
88-89: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winResolve team references without one query per reference.
The loop awaits
resolveTeamonce per element, andresolveTeamruns a fulllistTeamsquery on every call. Theteamsarray accepts up to 50 entries, so onecreate_projectcall can issue 50 sequential team queries. The same pattern exists inpackages/mcp-server/src/tools/admin.tsat Lines 51-52 forinvite_member.Add a batch resolver in
packages/mcp-server/src/resolve.tsthat loads the team list once and resolves every reference against it, then use it in both tools.♻️ Proposed batch resolver
// packages/mcp-server/src/resolve.ts export async function resolveTeamIds( principal: Principal, refs: readonly string[], ): Promise<string[]> { if (refs.length === 0) return []; const teams = await listTeams(principal); return refs.map((ref) => { const found = pick(teams, ref, (team) => [team.id, team.key, team.name]); if (found === undefined) throw notFound(`No team matches "${ref}".`); return found.id; }); }- const teamIds: string[] = []; - for (const ref of args.teams ?? []) teamIds.push((await resolveTeam(principal, ref)).id); + const teamIds = await resolveTeamIds(principal, args.teams ?? []);🤖 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/mcp-server/src/tools/planning.ts` around lines 88 - 89, Issue: Team references are resolved with one full team-list query per reference. Add a resolveTeamIds function in resolve.ts that returns early for empty refs, loads listTeams once, matches each reference against team id, key, or name using the existing pick/notFound behavior, and returns the matched IDs; replace the per-reference resolveTeam loops in create_project planning.ts and invite_member admin.ts with this batch resolver.packages/mcp-server/src/tools/admin.ts (1)
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
z.email()for the email schema. Zod 4.4.3 deprecatesz.string().email()in favor of the top-level format schema.🤖 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/mcp-server/src/tools/admin.ts` at line 45, Update the email field schema in the admin invite tool to use Zod’s top-level z.email() format schema instead of z.string().email(), while preserving its existing description and validation behavior.packages/mcp-server/src/tools/support.ts (1)
59-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant double assertion.
z.object(config.inputSchema)is assignable toz.ZodObject<Shape>with Zod 4.4.3. Pass it directly. In SDK 1.29.0,registerToolorders generics as<OutputArgs, InputArgs>; the firstz.ZodRawShapeis the output generic.🤖 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/mcp-server/src/tools/support.ts` around lines 59 - 60, Remove the redundant double assertion from the inputSchema initialization and pass z.object(config.inputSchema) directly as the z.ZodObject<Shape> value. Preserve the registerTool generic ordering, where z.ZodRawShape is the output type and z.ZodObject<Shape> is the input type.
🤖 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 `@apps/web/src/app/`(app)/layout.tsx:
- Around line 15-16: Update realtimeUrl() to validate the configured realtime
URL with a Zod schema, accepting only non-empty ws: or wss: URLs while
preserving '' for the same-origin fallback. Ensure malformed or unsupported
values are rejected before they reach new WebSocket(options.url), using the
existing configuredRealtimeUrl() source.
In `@apps/web/src/app/api/realtime/health/route.ts`:
- Around line 35-38: Update the catch block in the health route to stop
returning describe(error) in the public response; use a generic hub error value
instead, and log the original error server-side while preserving the 503
response and other fields.
In `@apps/web/src/app/api/ws/route.ts`:
- Around line 11-42: Bound the pre-accept message queue in attach by enforcing a
maximum buffered-frame count before pushing new messages. When the cap is
exceeded, close the socket using the established overload/limit close behavior
and stop buffering or processing further frames; preserve normal buffering and
replay once realtimeHub resolves.
In `@apps/web/src/app/mcp/route.ts`:
- Around line 1-9: Update the handle function’s handleMcpRequest options to use
the centralized publicAppUrl() helper instead of reading NEXT_PUBLIC_APP_URL
through serverEnv(), preserving centralized URL normalization and the existing
request handling flow.
In `@apps/web/src/lib/auth/password.ts`:
- Around line 16-22: Update verifyPassword so it explicitly validates the
digest’s PHC format and returns false only for malformed input or an invalid
password result. Remove the catch-all error suppression around verify, allowing
unrelated Argon2/runtime failures to propagate; use the existing verifyPassword
symbol and the Argon2 API’s expected digest structure for validation.
In `@apps/web/src/lib/realtime/hub.ts`:
- Around line 5-11: Update realtimeHub so a rejection from the cached
createRealtimeHub promise removes globalForHub.orbitRealtimeHub before
propagating the error, allowing subsequent calls to retry creation while
preserving the existing promise reuse on success.
In `@apps/web/src/lib/realtime/url.ts`:
- Around line 3-5: Update configuredRealtimeUrl to use static
process.env.NEXT_PUBLIC_REALTIME_URL access, then validate the configured value
with Zod and return it only when it parses as a WebSocket URL with a ws: or wss:
origin; otherwise preserve the existing empty-string fallback before the value
reaches RealtimeProvider.
In `@CLAUDE.md`:
- Around line 57-60: Align the deployment and runtime documentation across
CLAUDE.md lines 57-60, CLAUDE.md lines 27-28, and README.md line 27: remove or
rewrite standalone Bun container/realtime/MCP service instructions to describe
the single Node-based Vercel app, replace Bun.password guidance with
`@node-rs/argon2`, and update the end-to-end runtime description to
Node-compatible integrations. Also ensure shipped web server code does not
import or call Bun built-ins.
In `@packages/mcp-server/src/comments.ts`:
- Around line 35-47: In the comment-creation transaction around the insert in
add_comment, validate any non-empty parsed.parentId by querying the parent
comment and requiring matching principal.organizationId and issue.id; reject a
missing parent with notFound and reject a parent belonging to another
organization or issue. Only insert after this validation, preserving null/empty
parent behavior.
In `@packages/mcp-server/src/tools/identity.ts`:
- Around line 82-104: The member-directory permission is missing, allowing
unauthorized email access through listMembers consumers. Define the intended
directory-read permission in the shared policy, enforce it within listMembers
before returning member data, and ensure list_users, list_members, and web
member endpoints use that protected path while preserving existing role
permissions.
In `@packages/mcp-server/src/tools/support.ts`:
- Around line 29-40: Update failed to stop spreading errorFields(error) into
logger.warn; log only the tool name, domain.code, and domain.status. Preserve
the existing response body and error handling behavior while ensuring
caller-supplied messages and identifiers are not written to logs.
In `@packages/mcp-server/src/views.ts`:
- Around line 89-93: Update describeIssue to throw the typed internal domain
error from `@orbit/shared/errors` instead of a bare Error when describeIssues
returns no view. Follow the existing internal(...) usage in comments.ts and
preserve the current message and control flow.
---
Nitpick comments:
In `@apps/web/src/features/inbox/inbox-realtime.tsx`:
- Around line 28-33: Extract the repeated browser/SSR realtime URL selection
into one shared helper, such as browserRealtimeUrl in the realtime URL module,
and update the RealtimeProvider setup in
apps/web/src/features/inbox/inbox-realtime.tsx lines 28-33 and
apps/web/src/features/pulls/pulls-realtime.tsx lines 19-24 to call it. Also
replace the existing equivalent branch in apps/web/src/lib/realtime/provider.tsx
with the same helper, preserving configured URLs during SSR and
resolveRealtimeUrl behavior in the browser.
In `@apps/web/src/lib/realtime/provider.tsx`:
- Around line 43-44: Extract the SSR-aware URL selection into a shared
useRealtimeUrl hook beside url.ts, preserving the configured URL during SSR and
using resolveRealtimeUrl with window.location.origin in the browser. Replace the
duplicated typeof window resolution in the provider, inbox realtime, and pulls
realtime components with this hook.
In `@packages/mcp-server/src/logger.ts`:
- Around line 26-28: The errorFields function currently omits available stack
traces from structured error logs. Update errorFields to include error.stack
when the input is an Error with a stack, while preserving the existing message
and String(error) handling for all other cases.
In `@packages/mcp-server/src/resolve.ts`:
- Around line 31-36: Update resolveTeam to call listTeams with includeArchived:
true, matching resolveProject and the list_teams tool so archived team
references resolve for downstream operations.
In `@packages/mcp-server/src/tools/admin.ts`:
- Line 45: Update the email field schema in the admin invite tool to use Zod’s
top-level z.email() format schema instead of z.string().email(), while
preserving its existing description and validation behavior.
In `@packages/mcp-server/src/tools/planning.ts`:
- Around line 88-89: Issue: Team references are resolved with one full team-list
query per reference. Add a resolveTeamIds function in resolve.ts that returns
early for empty refs, loads listTeams once, matches each reference against team
id, key, or name using the existing pick/notFound behavior, and returns the
matched IDs; replace the per-reference resolveTeam loops in create_project
planning.ts and invite_member admin.ts with this batch resolver.
In `@packages/mcp-server/src/tools/support.ts`:
- Around line 59-60: Remove the redundant double assertion from the inputSchema
initialization and pass z.object(config.inputSchema) directly as the
z.ZodObject<Shape> value. Preserve the registerTool generic ordering, where
z.ZodRawShape is the output type and z.ZodObject<Shape> is the input type.
In `@packages/mcp-server/src/views.ts`:
- Around line 74-84: Update the workflow-state loading loop around
listWorkflowStates to start requests for all unique team IDs concurrently and
await them together, then populate stateNames from the aggregated results while
preserving the existing state ID-to-name mapping.
In `@packages/realtime-server/src/hub.ts`:
- Around line 104-108: Update the Redis configuration in the hub initialization
to avoid silently using redis://localhost:6380 when REDIS_URL is absent in
serverless deployments. Apply the same required-configuration validation used
for ticketSecret to redisUrl, or conditionally retain the localhost fallback
only for explicit local-development environments, while preserving configured
REDIS_URL behavior.
In `@packages/realtime-server/tsconfig.json`:
- Around line 3-6: Update the realtime-server TypeScript/lint configuration
around the compilerOptions and include entries to prevent Bun globals from being
used in non-test source files, while preserving the explicit ServerWebSocket
type import required by fromBunSocket. Add a rule scoped to non-test source, or
isolate the Bun adapter from code compiled for web imports without introducing
Bun runtime dependencies.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b53e228-e1d7-4ebf-be4b-829cddc0c7c0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (96)
.env.example.github/workflows/ci.ymlCLAUDE.mdREADME.mdapps/mcp/Dockerfileapps/mcp/README.mdapps/mcp/src/env.tsapps/mcp/src/index.tsapps/mcp/src/server.tsapps/realtime/package.jsonapps/realtime/src/active-organization.test.tsapps/realtime/src/client-reconnect.test.tsapps/realtime/src/index.tsapps/realtime/src/membership-revocation.test.tsapps/realtime/src/node-socket.test.tsapps/realtime/src/server.test.tsapps/realtime/src/server.tsapps/realtime/src/session-revocation.test.tsapps/realtime/src/test-helpers.tsapps/web/next.config.tsapps/web/package.jsonapps/web/src/app/(app)/inbox/page.tsxapps/web/src/app/(app)/layout.tsxapps/web/src/app/(app)/pulls/page.tsxapps/web/src/app/api/integrations/github/route.tsapps/web/src/app/api/realtime/health/route.tsapps/web/src/app/api/webhooks/github/route.tsapps/web/src/app/api/ws/route.tsapps/web/src/app/mcp/route.tsapps/web/src/features/inbox/inbox-realtime.tsxapps/web/src/features/pulls/pulls-realtime.tsxapps/web/src/lib/auth/password.test.tsapps/web/src/lib/auth/password.tsapps/web/src/lib/auth/server.tsapps/web/src/lib/integrations/oauth-state.tsapps/web/src/lib/realtime/hub.tsapps/web/src/lib/realtime/provider.tsxapps/web/src/lib/realtime/url.test.tsapps/web/src/lib/realtime/url.tsapps/web/vercel.jsonpackages/core/package.jsonpackages/core/src/analytics/burndown.tspackages/core/src/realtime/publisher.tspackages/db/package.jsonpackages/db/src/client.tspackages/db/src/ensure-extensions.tspackages/mcp-server/bunfig.tomlpackages/mcp-server/package.jsonpackages/mcp-server/src/auth.test.tspackages/mcp-server/src/comments.tspackages/mcp-server/src/index.tspackages/mcp-server/src/logger.tspackages/mcp-server/src/resolve.tspackages/mcp-server/src/server.tspackages/mcp-server/src/test-helpers.tspackages/mcp-server/src/tools.test.tspackages/mcp-server/src/tools/admin.tspackages/mcp-server/src/tools/identity.tspackages/mcp-server/src/tools/index.tspackages/mcp-server/src/tools/issues.tspackages/mcp-server/src/tools/planning.tspackages/mcp-server/src/tools/support.tspackages/mcp-server/src/views.tspackages/mcp-server/tests-preload.tspackages/mcp-server/tsconfig.jsonpackages/realtime-server/bunfig.tomlpackages/realtime-server/package.jsonpackages/realtime-server/src/auth.test.tspackages/realtime-server/src/auth.tspackages/realtime-server/src/connection.test.tspackages/realtime-server/src/connection.tspackages/realtime-server/src/hub.tspackages/realtime-server/src/index.tspackages/realtime-server/src/logger.tspackages/realtime-server/src/presence.tspackages/realtime-server/src/socket.tspackages/realtime-server/tests-preload.tspackages/realtime-server/tsconfig.jsonpackages/services/package.jsonpackages/services/src/email/email.test.tspackages/services/src/email/index.tspackages/services/src/github/apply.test.tspackages/services/src/github/apply.tspackages/services/src/github/install.test.tspackages/services/src/github/install.tspackages/services/src/notifications/index.tspackages/services/src/notifications/notifications.test.tspackages/services/src/slack/dispatch.test.tspackages/services/src/slack/dispatch.tspackages/services/src/storage/credentials.test.tspackages/services/src/storage/credentials.tspackages/services/src/storage/parent.test.tspackages/services/src/storage/s3.tspackages/services/src/storage/validate.tspackages/shared/package.jsonpackages/shared/src/utils/index.ts
💤 Files with no reviewable changes (6)
- apps/mcp/src/env.ts
- .env.example
- apps/mcp/README.md
- apps/mcp/src/server.ts
- apps/mcp/Dockerfile
- apps/mcp/src/index.ts
🛑 Comments failed to post (4)
packages/mcp-server/src/comments.ts (1)
35-47: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Inspect the comment table definition and existing parentId validation. set -euo pipefail fd -t f -e ts . packages/db/src --exec rg -n -C 10 'comment\s*=\s*pgTable|parentId' {} \; echo '--- commentCreateSchema ---' rg -n -C 10 'commentCreateSchema' packages/shared/src echo '--- other comment creation paths ---' rg -n -C 5 'parentId' packages/core/src apps/web/src --type=tsRepository: Noveum/orbit
Length of output: 18733
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- candidate files ---' fd -t f -i 'comments|comment' packages/mcp-server packages/db packages/shared | head -80 echo '--- comments.ts structure ---' ast-grep outline packages/mcp-server/src/comments.ts --view summary echo '--- comments.ts relevant code ---' rg -n -C 12 'createComment|add_comment|replyTo|parentId|transaction' packages/mcp-server/src/comments.ts echo '--- query and error exports ---' rg -n -C 5 'export.*(and|eq)|function notFound|const notFound|notFound' packages/db packages/shared --glob '*.ts'Repository: Noveum/orbit
Length of output: 433
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- comments.ts structure ---' ast-grep outline packages/mcp-server/src/comments.ts --view signatures echo '--- comments.ts relevant code ---' rg -n -C 14 'createComment|add_comment|replyTo|parentId|transaction' packages/mcp-server/src/comments.ts echo '--- comment validator ---' cat -n packages/shared/src/validators/comment.ts echo '--- query and error exports ---' rg -n -C 5 'export.*(and|eq)|function notFound|const notFound|notFound' packages/db packages/shared --glob '*.ts'Repository: Noveum/orbit
Length of output: 12594
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- comments.ts ---' cat -n packages/mcp-server/src/comments.ts echo '--- add_comment and createComment call sites ---' rg -n -C 16 'add_comment|createComment|replyTo' packages/mcp-server --glob '*.ts' echo '--- all comment parent reads and writes ---' rg -n -C 10 'schema\.comment|parentId|replyTo' packages/mcp-server packages/core apps packages/db --glob '*.{ts,tsx}' echo '--- db barrel exports ---' fd -t f -e ts . packages/db/src | sort | head -80 rg -n -C 5 'drizzle-orm|export.*and|export.*eq' packages/db/src --glob '*.ts'Repository: Noveum/orbit
Length of output: 50369
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- MCP source files ---' git ls-files packages/mcp-server -- '*.ts' echo '--- comments.ts ---' cat -n packages/mcp-server/src/comments.ts echo '--- MCP comment tool references ---' rg -n -C 10 --glob '*.ts' 'add_comment|replyTo|createComment|commentCreateSchema' packages/mcp-server echo '--- MCP comment reads ---' rg -n -C 10 --glob '*.ts' 'schema\.comment|commentQuerySchema|parentId' packages/mcp-serverRepository: Noveum/orbit
Length of output: 41873
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path import re schema = Path("packages/db/src/schema/content.ts").read_text() service = Path("packages/mcp-server/src/comments.ts").read_text() tool = Path("packages/mcp-server/src/tools/issues.ts").read_text() validator = Path("packages/shared/src/validators/comment.ts").read_text() comment_block = re.search( r"export const comment = pgTable\(.*?\n\);", schema, re.S, ) assert comment_block, "comment table not found" comment = comment_block.group(0) assert re.search(r"parentId: text\('parent_id'\)", comment) assert not re.search(r"parentId:.*references\(", comment) assert "parentId: parsed.parentId" in service assert "replyTo: z.string().min(1).optional()" in tool assert "parentId: args.replyTo ?? null" in tool assert "parentId: idSchema.nullable().default(null)" in validator print("comment.parentId: plain text, no foreign key") print("createComment: inserts parsed.parentId without a parent query") print("add_comment: accepts non-empty replyTo and passes it unchanged") print("validator: checks identifier shape only") PYRepository: Noveum/orbit
Length of output: 360
Validate
parsed.parentIdagainst the target issue and organization.
comment.parentIdhas no foreign key, andadd_commentpasses any non-emptyreplyTounchanged. Query the parent inside the transaction and reject it unless itsorganizationIdmatchesprincipal.organizationIdand itsissueIdmatchesissue.id. Reject nonexistent parents withnotFound.🤖 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/mcp-server/src/comments.ts` around lines 35 - 47, In the comment-creation transaction around the insert in add_comment, validate any non-empty parsed.parentId by querying the parent comment and requiring matching principal.organizationId and issue.id; reject a missing parent with notFound and reject a parent belonging to another organization or issue. Only insert after this validation, preserving null/empty parent behavior.packages/mcp-server/src/tools/identity.ts (1)
82-104: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Check permissions related to member listing and how web routes gate it. set -euo pipefail echo '--- permissions and roles ---' rg -n -C 3 'member:|PERMISSIONS_BY_ROLE' packages/shared/src/policy echo '--- listMembers definition and callers ---' rg -n -C 5 '\blistMembers\s*\(' packages/core/src apps/web/src packages/mcp-server/src --type=tsRepository: Noveum/orbit
Length of output: 9675
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- policy definitions ---' cat -n packages/shared/src/policy/index.ts | sed -n '1,125p' echo '--- member service ---' cat -n packages/core/src/org/member-service.ts | sed -n '1,115p' echo '--- MCP identity and admin tools ---' cat -n packages/mcp-server/src/tools/identity.ts | sed -n '1,135p' cat -n packages/mcp-server/src/tools/admin.ts | sed -n '1,75p' echo '--- principal and authorization usage ---' rg -n -C 4 'assertCan|can\(|member:invite|member:manage' packages/core/src packages/mcp-server/src apps/web/src packages/shared/src --type=tsRepository: Noveum/orbit
Length of output: 50369
Restrict member directory access.
listMembersreturns every member’s email without authorization. The shared policy has no directory-read permission;member:manageis limited toadmin, whilemember:inviteis available tomemberandadmin. Define the intended directory permission and enforce it inlistMembersbeforelist_users,list_members, and web member endpoints return email addresses.🤖 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/mcp-server/src/tools/identity.ts` around lines 82 - 104, The member-directory permission is missing, allowing unauthorized email access through listMembers consumers. Define the intended directory-read permission in the shared policy, enforce it within listMembers before returning member data, and ensure list_users, list_members, and web member endpoints use that protected path while preserving existing role permissions.packages/mcp-server/src/tools/support.ts (1)
29-40: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid writing caller-supplied identifiers into logs.
errorFields(error)returns the error message, andlogger.warnwrites it. Several resolver errors embed the raw reference in the message. For exampleresolveUserIdinpackages/mcp-server/src/resolve.tsat Line 47 throwsNo user matches "<ref>", and<ref>is often an email address. The same applies to team, label, project, and cycle references. Emails and other user identifiers then persist in the log store.Log the tool name, the error code, and the status. Keep the free-text message out of the log line, or redact it for client errors.
🛡️ Proposed change
export function failed(name: string, error: unknown): CallToolResult { const domain = asDomainError(error); - logger.warn('tool failed', { tool: name, code: domain.code, ...errorFields(error) }); + logger.warn('tool failed', { + tool: name, + code: domain.code, + status: domain.status, + ...(domain.status >= 500 ? errorFields(error) : {}), + });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export function failed(name: string, error: unknown): CallToolResult { const domain = asDomainError(error); logger.warn('tool failed', { tool: name, code: domain.code, status: domain.status, ...(domain.status >= 500 ? errorFields(error) : {}), }); const body = domain.status >= 500 ? { error: { code: domain.code, message: 'Something went wrong on our side.' } } : domain.toJSON(); return { isError: true, content: [{ type: 'text', text: JSON.stringify(body) }], }; }🤖 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/mcp-server/src/tools/support.ts` around lines 29 - 40, Update failed to stop spreading errorFields(error) into logger.warn; log only the tool name, domain.code, and domain.status. Preserve the existing response body and error handling behavior while ensuring caller-supplied messages and identifiers are not written to logs.packages/mcp-server/src/views.ts (1)
89-93: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Throw a typed domain error instead of
Error.Line 91 throws a bare
Error.support.tsmaps unknown errors throughtoDomainError, which converts them tointernal, so the behavior is close. A typed error keeps the code explicit and matches the rest of the package, for exampleinternal(...)inpackages/mcp-server/src/comments.tsat Line 47.The coding guidelines require typed domain errors from
@orbit/shared/errors.♻️ Proposed change
+import { internal } from '`@orbit/shared/errors`'; + export async function describeIssue(principal: Principal, row: IssueRow): Promise<IssueView> { const [view] = await describeIssues(principal, [row]); - if (view === undefined) throw new Error('The issue could not be described.'); + if (view === undefined) throw internal('The issue could not be described.'); return view; }As per coding guidelines: "Throw typed domain errors from
@orbit/shared/errors, and have route handlers map them to responses; never silently swallow errors."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { internal } from '`@orbit/shared/errors`'; export async function describeIssue(principal: Principal, row: IssueRow): Promise<IssueView> { const [view] = await describeIssues(principal, [row]); if (view === undefined) throw internal('The issue could not be described.'); return view; }🤖 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/mcp-server/src/views.ts` around lines 89 - 93, Update describeIssue to throw the typed internal domain error from `@orbit/shared/errors` instead of a bare Error when describeIssues returns no view. Follow the existing internal(...) usage in comments.ts and preserve the current message and control flow.Source: Coding guidelines
There was a problem hiding this comment.
🧹 Nitpick comments (2)
CLAUDE.md (2)
134-136: 🗄️ Data Integrity & Integration | 🔵 TrivialAdd a production migration release gate
CI updates only ephemeral test databases. Add an enforced release step that applies migrations and verifies the target schema before Vercel promotion. Otherwise, production code can start against an older schema.
🤖 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 `@CLAUDE.md` around lines 134 - 136, Add an enforced production release gate that applies migrations to the target database and verifies the resulting schema before Vercel promotion. Update the deployment workflow or release configuration referenced by the migration guidance, ensuring promotion cannot proceed when migration application or schema verification fails.
120-132: 🩺 Stability & Availability | 🔵 TrivialAdd a preview WebSocket smoke test.
Authenticate against the preview, mint a realtime ticket, connect to
/api/ws, and assert101 Switching Protocolsbefore sending the ticket frame. Remove thebunVersionassertion because Vercel supportsexperimental_upgradeWebSocketwith the Bun runtime.🤖 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 `@CLAUDE.md` around lines 120 - 132, Add a preview WebSocket smoke test that authenticates against the preview, mints a realtime ticket, connects to /api/ws, and verifies a 101 Switching Protocols response before sending the ticket frame. Remove the documentation’s bunVersion assertion, including the claim that the node runtime is required for experimental_upgradeWebSocket, while preserving the remaining route handshake guidance.Source: MCP tools
🤖 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.
Nitpick comments:
In `@CLAUDE.md`:
- Around line 134-136: Add an enforced production release gate that applies
migrations to the target database and verifies the resulting schema before
Vercel promotion. Update the deployment workflow or release configuration
referenced by the migration guidance, ensuring promotion cannot proceed when
migration application or schema verification fails.
- Around line 120-132: Add a preview WebSocket smoke test that authenticates
against the preview, mints a realtime ticket, connects to /api/ws, and verifies
a 101 Switching Protocols response before sending the ticket frame. Remove the
documentation’s bunVersion assertion, including the claim that the node runtime
is required for experimental_upgradeWebSocket, while preserving the remaining
route handshake guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 81e306b8-ff6d-49cb-a03d-d88bd4e40da6
📒 Files selected for processing (15)
.dockerignore.github/workflows/ci.ymlCLAUDE.mdDockerfile.depsapps/realtime/Dockerfileapps/web/Dockerfilek8s/00-namespace.yamlk8s/01-redis.yamlk8s/02-web.yamlk8s/03-realtime.yamlk8s/04-mcp.yamlk8s/05-ingress.yamlk8s/README.mdk8s/apply.shk8s/s3-cors.json
💤 Files with no reviewable changes (14)
- k8s/s3-cors.json
- apps/realtime/Dockerfile
- .dockerignore
- k8s/README.md
- k8s/apply.sh
- k8s/01-redis.yaml
- k8s/05-ingress.yaml
- Dockerfile.deps
- k8s/00-namespace.yaml
- apps/web/Dockerfile
- .github/workflows/ci.yml
- k8s/04-mcp.yaml
- k8s/02-web.yaml
- k8s/03-realtime.yaml
The uploads bucket still needs a CORS rule because the browser PUTs straight to storage, but the manifest directory that carried it is gone. Move it to infra with the command that applies it.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@infra/README.md`:
- Around line 12-13: Update the aws s3api put-bucket-cors command to use the
existing S3_BUCKET environment variable instead of ORBIT_S3_BUCKET, keeping the
CORS configuration generation unchanged.
In `@packages/services/src/storage/round-trip.test.ts`:
- Around line 90-93: The test “documents how it reaches the bucket” should
assert the complete documented sed command and complete aws s3api
put-bucket-cors command from infra/README.md, including the expected
k8s/s3-cors.json source, temporary file, bucket variable, and command arguments,
rather than checking only partial substrings.
- Line 51: Update both test file reads in the round-trip tests to import and use
readFile from node:fs/promises with the utf8 encoding, replacing each
Bun.file(...).text() call while preserving the existing file paths and test
behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c3b6a386-71a3-4927-a9ea-0470e0d2e8a2
📒 Files selected for processing (3)
infra/README.mdinfra/s3-cors.jsonpackages/services/src/storage/round-trip.test.ts
Only an end-to-end test touched this path, so a driver change could break it without any suite noticing. Assert the row lands, the sync action carries the scopes other tabs subscribe to, and toggling again removes it.
A failing spec only showed what the browser saw, so a server side error during the run left no trace to read.
Sanitizing chose Bun's HTMLRewriter and otherwise fell back to a browser document, so on the node runtime every render threw 'document is not defined' and each comment POST answered 500. Parse with linkedom when there is no document. The template element it returns has no content fragment, so the cleaner walked nothing and handed back the input untouched, which would have let a script tag through. Hold the markup in a real element instead and compare node types rather than instances, and cover the path the server now takes.
The browser branch handed back the template content fragment, which has no innerHTML, so sanitizing returned undefined wherever a document exists. Keep the element for serialising and walk the fragment's nodes.
Review findings from the pull request. A rejected hub promise stayed cached for the life of the instance, so one unreachable redis left every later connection closing with 1011. Drop the entry when it rejects. Frames arriving before the hub resolves were buffered without a limit and before any rate limit applied, so cap the buffer and close past it. The health route returned the raw error, the MCP route reached past publicAppUrl, verify turned every argon2 failure into a wrong password, and a malformed realtime url reached the WebSocket constructor.
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 `@apps/web/src/lib/auth/password.ts`:
- Line 4: Update the PHC_ARGON2 pattern used by password verification to accept
only the $argon2id$ prefix, excluding Argon2i and Argon2d hashes. Preserve the
existing verification flow while ensuring legacy algorithms are rejected unless
an explicit controlled migration path already exists.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7caf3c5e-b287-4a31-a924-c53dee3a1e8b
📒 Files selected for processing (11)
CLAUDE.mdREADME.mdapps/web/src/app/api/realtime/health/route.tsapps/web/src/app/api/ws/route.tsapps/web/src/app/mcp/route.tsapps/web/src/lib/auth/password.tsapps/web/src/lib/realtime/hub.tsapps/web/src/lib/realtime/url.test.tsapps/web/src/lib/realtime/url.tsinfra/README.mdpackages/services/src/storage/round-trip.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/web/src/lib/realtime/url.ts
- infra/README.md
- packages/services/src/storage/round-trip.test.ts
- apps/web/src/app/mcp/route.ts
- apps/web/src/app/api/realtime/health/route.ts
- CLAUDE.md
| import { hash, verify } from '@node-rs/argon2'; | ||
|
|
||
| const ARGON2ID = 2; | ||
| const PHC_ARGON2 = /^\$argon2(?:id|i|d)\$/; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
cat -n apps/web/src/lib/auth/password.ts
printf '%s\n' '--- related symbols and callers ---'
rg -n --glob '!node_modules' 'PHC_ARGON2|hashPassword|verifyPassword|argon2' apps packages
printf '%s\n' '--- relevant package configuration ---'
rg -n --glob 'package.json' --glob 'bun.lockb' --glob 'bun.lock' '`@node-rs/argon2`|argon2|password' .Repository: Noveum/orbit
Length of output: 7529
🏁 Script executed:
#!/bin/bash
set -eu
cat -n apps/web/src/lib/auth/password.ts
rg -n --glob '!node_modules' 'PHC_ARGON2|hashPassword|verifyPassword|argon2' apps packages
rg -n --glob 'package.json' --glob 'bun.lockb' --glob 'bun.lock' '`@node-rs/argon2`|argon2|password' .Repository: Noveum/orbit
Length of output: 7434
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- password tests ---'
cat -n apps/web/src/lib/auth/password.test.ts
printf '%s\n' '--- authentication path ---'
sed -n '1,140p' apps/web/src/lib/auth/server.ts
printf '%s\n' '--- standalone prefix behavior ---'
python3 - <<'PY'
import re
pattern = re.compile(r'^\$argon2(?:id|i|d)\$')
for digest in ('$argon2id$v=19$m=1,t=1,p=1$x$y',
'$argon2i$v=19$m=1,t=1,p=1$x$y',
'$argon2d$v=19$m=1,t=1,p=1$x$y',
'$bcrypt$anything'):
print(f'{digest.split("$")[1]}: {bool(pattern.match(digest))}')
PYRepository: Noveum/orbit
Length of output: 6266
🌐 Web query:
@node-rs/argon2 verify Argon2id Argon2i Argon2d PHC encoded password support
💡 Result:
The @node-rs/argon2 package fully supports Argon2id, Argon2i, and Argon2d algorithms [1][2] and is designed to handle PHC-encoded password strings for verification [3]. The library's verify function expects a valid PHC-encoded hash string (e.g., $argon2id$v=19$m=65536,t=3,p=4$salt$hash) as the first argument [3]. When you pass this string along with the plaintext password, the library automatically parses the algorithm, version, and parameters embedded within the PHC string to perform the verification process [4][3]. This ensures that the parameters used during hashing (such as memory cost, time iterations, and parallelism) are correctly applied during verification [4]. It is recommended to wrap the verify function in a try...catch block, as passing an invalid or malformed PHC string will throw an error [3]. The package supports Argon2id (the default and recommended algorithm), Argon2i, and Argon2d variants [1][2].
Citations:
- 1: https://github.com/napi-rs/node-rs/tree/main/packages/argon2
- 2: https://www.npmjs.com/package/@node-rs/argon2
- 3: https://ssojet.com/hashing/argon2-in-typescript
- 4: https://docs.rs/argon2/latest/argon2/index.html
🏁 Script executed:
#!/bin/bash
set -eu
cat -n apps/web/src/lib/auth/password.test.ts
sed -n '1,140p' apps/web/src/lib/auth/server.ts
python3 - <<'PY'
import re
pattern = re.compile(r'^\$argon2(?:id|i|d)\$')
for digest in ('$argon2id$v=19$m=1,t=1,p=1$x$y',
'$argon2i$v=19$m=1,t=1,p=1$x$y',
'$argon2d$v=19$m=1,t=1,p=1$x$y',
'$bcrypt$anything'):
print(f'{digest.split("$")[1]}: {bool(pattern.match(digest))}')
PYRepository: Noveum/orbit
Length of output: 6180
Restrict password verification to Argon2id.
PHC_ARGON2 accepts $argon2i$ and $argon2d$, and @node-rs/argon2.verify processes those algorithms. Change the prefix check to /^\$argon2id\$/, or implement a controlled migration for legacy hashes.
🤖 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/web/src/lib/auth/password.ts` at line 4, Update the PHC_ARGON2 pattern
used by password verification to accept only the $argon2id$ prefix, excluding
Argon2i and Argon2d hashes. Preserve the existing verification flow while
ensuring legacy algorithms are rejected unless an explicit controlled migration
path already exists.
Source: Coding guidelines
Member
Author
|
production after the deploy, the socket that was failing in the console now upgrades: 4001 is the hub rejecting a deliberately invalid ticket, so auth is running too. /mcp was a 404 before this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move the websocket hub and the MCP server into the Next.js app so Orbit deploys as one Vercel project, and off Bun built-ins so the node runtime can upgrade the socket.