Repository navigation
feat: integrate Nova web and enhance authentication flow: - #322
Conversation
- Updated the dashboard and Nova web to run on port 3003, improving accessibility. - Implemented a safe redirect mechanism for Nova login sessions, ensuring only allowed origins are accepted. - Enhanced the authentication client to support one-time tokens for secure session management. - Added new API routes for Composio interactions, including session management and tool execution. - Updated environment configurations to support Nova web integration and CLI authentication. - Improved error handling and response structures for Composio-related requests.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedToo many files! This PR contains 119 files, which is 19 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (119)
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }, | ||
| plugins: [ | ||
| oneTimeToken({ | ||
| expiresIn: 3, |
There was a problem hiding this comment.
| return { | ||
| url: session.mcp.url, | ||
| headers: session.mcp.headers, | ||
| sessionId: session.sessionId, |
There was a problem hiding this comment.
session_id, but this code reads sessionId. If the SDK returns session_id, the CLI response omits the session ID and Nova passes an undefined ID to the Connections UI. The new tests mock sessionId, so they do not catch that mismatch.
| function terminalDatabaseUrl(): string { | ||
| const url = process.env.DATABASE_URL_TERMINAL?.trim() | ||
| if (!url) { | ||
| throw new Error( | ||
| "DATABASE_URL_TERMINAL is required for the terminal/harness database. " | ||
| + "It must not fall back to the dashboard DATABASE_URL.", | ||
| ) | ||
| } |
There was a problem hiding this comment.
DATABASE_URL, as the web environment example still permits, importing this client now throws instead of using the previous fallback. The waitlist and several Nova routes import it statically, so their handlers cannot run in that configuration.
| const posted = await postSessionMessage({ | ||
| userId: user.id, | ||
| sessionId, | ||
| content: parsed.data.content, | ||
| clientMessageId: parsed.data.clientMessageId, | ||
| surface: "desktop", | ||
| }) | ||
| if (!posted) return NextResponse.json({ error: "Session not found" }, { status: 404 }) | ||
| return NextResponse.json(posted, { status: 201 }) | ||
| } catch (error) { |
There was a problem hiding this comment.
postSessionMessage creates a queued run and marks it active, but this route returns without starting the turn or dispatching the run. The client receives an acknowledgement while Nova never answers and the session remains marked as working. The new web message endpoint follows the same pattern.
| const loadSession = useCallback(async (sessionId: string, reset = false) => { | ||
| const [detailResponse, sync] = await Promise.all([ | ||
| getSession(sessionId), | ||
| syncSession(sessionId, reset ? 0 : cursorRef.current, 500), | ||
| ]) | ||
| setSession(detailResponse.session) | ||
| if (reset) { | ||
| setTimeline( | ||
| mergeTimeline( | ||
| [], | ||
| sync.messages.map((entry) => ({ ...entry, kind: "message" as const })), | ||
| sync.activities.map((entry) => ({ ...entry, kind: "activity" as const })), | ||
| ), | ||
| ) | ||
| setSyncCursor(sync.nextSequence) |
There was a problem hiding this comment.
hasMore. Later polling starts after that page, so the remaining history is omitted from the view until it is reloaded.
| createdAt: existing.createdAt.toISOString(), | ||
| references: referencesFromMetadata(existing.metadata), | ||
| }, | ||
| runId: existing.agentSession.activeRunId ?? "", |
There was a problem hiding this comment.
clientMessageId returns the session’s current active run rather than the run for that message. After the original run completes, activeRunId is cleared, so a retry receives runId: "". The turn then tries to save an activity against a nonexistent run and fails instead of returning or resuming the original result.
| <Composer | ||
| value={draft} | ||
| onChange={onDraftChange} | ||
| references={references} | ||
| onReferencesChange={onReferencesChange} | ||
| onSubmit={onSubmit} | ||
| streaming={streaming} | ||
| disabled={loading} |
There was a problem hiding this comment.
onStop handler and the streaming request has no abort signal. Clicking Stop does nothing; the harness can keep running and executing connected tools.
| type GeneralPrefs = { | ||
| openLinksInDesktop: boolean | ||
| enterBehavior: "interrupt" | "queue" | "newline" | ||
| cmdEnterBehavior: "queue" | "interrupt" | "steer" | ||
| altEnterBehavior: "steer" | "queue" | "interrupt" | ||
| } | ||
|
|
||
| const DEFAULT_PREFS: GeneralPrefs = { | ||
| openLinksInDesktop: false, | ||
| enterBehavior: "interrupt", | ||
| cmdEnterBehavior: "queue", | ||
| altEnterBehavior: "steer", |
There was a problem hiding this comment.
🤖 Supercode AI ReviewSummarySummaryThis PR shifts the dashboard to serve “Dashboard / Nova Web” on port 3003, and adds a Nova-specific auth + redirect flow using one-time tokens and HMAC-signed OAuth state. It also introduces a new CLI-server Composio integration layer (including tool listing/execution) and a set of Nova Web API routes for sessions, approvals, connectors, and Composio connection management. Walkthrough
Changes table
Risk assessmentHigh — Auth and redirect flows across multiple apps (CLI, dashboard, Nova web) plus state signing/callback handling affect login/session and OAuth completion. Bugs here can break onboarding or cause confusing redirect behavior. Test plan
Suggested PR descriptionWhat
Why
How tested
Findings
Sequence diagramSequence DiagramsequenceDiagram
participant U as User
participant NovaLogin as Nova Web (/login)
participant CLI as CLI Login Form
participant AuthServer as CLI Auth (better-auth)
participant CLIPlugin as oneTimeToken plugin
participant CliCallback as Nova Web /api/auth/cli/callback
participant Prisma as DB (prisma.user)
participant NovaApp as Nova Web (/app /connections)
U->>NovaLogin: Open Nova host login (nova.localhost:3003)
NovaLogin->>CLI: Redirect to CLI /sign-in?redirect=<nova /app>
U->>CLI: Complete GitHub sign-in
CLI->>AuthServer: Create/obtain CLI session
AuthServer-->>CLI: CLI session exists (better-auth)
CLI->>CLIPlugin: authClient.oneTimeToken.generate()
CLIPlugin-->>CLI: one-time token
CLI->>CliCallback: GET/redirect to /api/auth/cli/callback?token=...&redirect=/app
CliCallback->>AuthServer: POST /api/auth/one-time-token/verify { token }
AuthServer-->>CliCallback: Session + token validity payload
CliCallback->>Prisma: find/update/create user
CliCallback-->>NovaApp: 302 redirect to redirect URL + set session cookie (and harness token cookie if present)
Automated review by Supercode · leave a 👍/👎 reaction to rate this review |
yashdev9274
left a comment
yashdev9274
left a comment
There was a problem hiding this comment.
Supercode found actionable issues during its complete PR analysis.
| const value = error as { message?: string; statusCode?: number } | ||
| res.status(value.statusCode ?? 500).json({ error: value.message || fallback }) | ||
| function stateSecret(): string { | ||
| const secret = process.env.BETTER_AUTH_SECRET |
There was a problem hiding this comment.
P1 · CRITICAL · verifyState secret failure treated as server error instead of invalid state
verifyState() calls stateSecret() which throws when BETTER_AUTH_SECRET is missing, causing the OAuth callback to hit the general error handler instead of returning a consistent 'invalid/expired state' response. This can turn a client-originated invalid state into a server misconfig failure. Evidence: stateSecret() throws 503; verifyState() does not catch it, so request handler falls into catch and sendError(). Suggested fix: have verifyState() return null if the secret is missing (or explicitly handle missing secret in the callback route) so callbacks fail closed as invalid state rather than noisy 5xx.
| return errorResponse("The authentication service is unavailable", 502) | ||
| } | ||
|
|
||
| if (!cliResponse.ok) { |
There was a problem hiding this comment.
P1 · HIGH · crypto.randomUUID used without import/guaranteed global
apps/web/app/api/auth/cli/callback/route.ts calls crypto.randomUUID() but the diff shows no import for crypto (no import { randomUUID } from 'node:crypto'). Depending on runtime (node vs edge) this can break at runtime when creating a new user. Suggested fix: import randomUUID from node:crypto or use globalThis.crypto with explicit handling.
| @@ -9,6 +9,23 @@ import { Github, Code2, Sparkles, ArrowRight } from 'lucide-react' | |||
| import { ParticleBackground } from './particle-background' | |||
There was a problem hiding this comment.
P2 · MEDIUM · Duplicated Nova origin/host allowlists across apps can drift
Nova redirect/origin logic is implemented separately in CLI (ALLOWED_REDIRECT_ORIGINS) and web (NOVA_HOSTS). If ports/hosts/staging domains change, one side can drift and break redirects or weaken validation. Suggested fix: centralize these values via shared config/env and reuse the same source of truth in both places.
- Introduced new API routes for managing local projects, including creation, listing, updating, and binding to sessions. - Implemented local attachments handling in session messages, allowing users to attach files directly from their local environment. - Enhanced session management to include local project details and ensure proper authorization checks. - Updated schemas and validation for local project inputs and attachments, improving data integrity. - Added tests for local attachments and project management functionalities to ensure reliability.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| onLocalFilesChange={onLocalFilesChange} | ||
| onLocalProjectChange={onLocalProjectChange} | ||
| onSubmit={onSubmit} | ||
| streaming={streaming} |
There was a problem hiding this comment.
The "Stop" button shown while a Nova turn is streaming is a no-op because onStop is never wired through ThreadView → Composer, leaving users unable to cancel a running turn.
This branch was successfully deployed
1 participant
Description
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
bun testpassesbun run typecheckpassesbun run lintpasses (if applicable)Checklist:
Summary by Supercode Review
New Features
Infrastructure
Tests