Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds publishableClientKey support to the local emulator API and wiring, introduces three emulator credential constants, enables runtime emulator credential overrides across client/server/admin interfaces, and adds emulator detection + async credential fetching into app initialization flows. Changes
Sequence DiagramsequenceDiagram
participant AppInit as App Initialization
participant Detect as Emulator Detection
participant FS as Config File
participant EmulatorAPI as Local Emulator API
participant Interface as SDK Interface
participant Request as API Request
AppInit->>Detect: check getLocalEmulatorConfigFilePath(...)
Detect->>FS: resolve config path
alt config found (emulator)
Detect->>EmulatorAPI: fetchEmulatorProjectCredentials(configPath)
EmulatorAPI-->>Detect: return { projectId, publishableClientKey, secretServerKey, superSecretAdminKey }
Detect->>Interface: iface._updateEmulatorCredentials({...})
Interface-->>Interface: store override values
else no config (normal)
Detect-->>AppInit: proceed with defaults
end
AppInit->>Request: prepareRequest()
Request->>Interface: request credentials (projectId, keys)
Interface-->>Request: return overrides if present, else defaults
Request->>EmulatorAPI: perform API call with headers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
Greptile SummaryThis PR extends the local emulator to support the full SDK hierarchy (client, server, and admin) by adding Key changes and findings:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant App as SDK App (Client/Server/Admin)
participant CI as StackClientInterface
participant SI as StackServerInterface
participant BE as Local Emulator Backend
participant DB as Prisma DB
App->>CI: new StackClientInterface({ prepareRequest })
App->>App: _emulatorInitPromise = fetchEmulatorProjectCredentials()
App->>BE: POST /api/v1/internal/local-emulator/project
BE->>DB: findFirst(apiKeySet where secretServerKey!=null AND superSecretAdminKey!=null)
alt Key set found (may lack publishableClientKey — bug)
DB-->>BE: existing keySet
BE->>BE: assert publishableClientKey != null (throws if null)
else No key set found
DB-->>BE: null
BE->>DB: create apiKeySet (all 3 keys)
DB-->>BE: new keySet
end
BE-->>App: { project_id, publishable_client_key, secret_server_key, super_secret_admin_key }
App->>CI: _updateEmulatorCredentials(credentials)
App->>SI: _updateEmulatorCredentials(credentials)
Note over App,SI: Client requests: prepareRequest awaits _emulatorInitPromise ✓
Note over App,SI: Server/Admin requests: NO prepareRequest set — may race ✗
App->>CI: sendClientRequest (awaits prepareRequest → init promise)
App->>SI: sendServerRequest (NO prepareRequest → may use placeholder keys)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
Show resolved
Hide resolved
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx (1)
113-130:⚠️ Potential issue | 🟡 MinorPotential issue: Prisma query doesn't filter for
publishableClientKeyexistence.The query at lines 113-130 filters for
secretServerKey: { not: null }andsuperSecretAdminKey: { not: null }but notpublishableClientKey: { not: null }. If there are existing key sets in the database without apublishableClientKey, this could return them, and the assertion at line 144 would throw.Consider adding
publishableClientKey: { not: null }to the query filter for consistency:Proposed fix
const existingKeySet = await globalPrismaClient.apiKeySet.findFirst({ where: { projectId, manuallyRevokedAt: null, expiresAt: { gt: new Date(), }, + publishableClientKey: { + not: null, + }, secretServerKey: { not: null, }, superSecretAdminKey: { not: null, }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx` around lines 113 - 130, The Prisma query in globalPrismaClient.apiKeySet.findFirst (assigned to existingKeySet) filters for secretServerKey and superSecretAdminKey but not publishableClientKey, so it may return rows missing publishableClientKey and later cause the assertion at line 144 to fail; update the findFirst where clause to include publishableClientKey: { not: null } so only key sets that contain all three keys (secretServerKey, superSecretAdminKey, publishableClientKey) are returned by existingKeySet.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stack-shared/src/interface/client-interface.ts`:
- Around line 53-70: The emulator override only updates
_projectIdOverride/_publishableClientKeyOverride via _updateEmulatorCredentials
but fetchNewAccessToken, getOAuthUrl, and callOAuthCallback still read
this.options.publishableClientKey (and possibly this.options.projectId), causing
inconsistent behavior; update those methods to read the effective accessors
(this.publishableClientKey and this.projectId) instead of directly using
this.options.* so the OAuth/token refresh flow honors the emulator overrides
from _updateEmulatorCredentials.
In `@packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts`:
- Around line 154-164: AdminAppImpl (and similarly ServerAppImpl) sets
this._emulatorInitPromise but never wires a prepareRequest callback on the
created interface, allowing requests to race before emulator credentials load;
add a prepareRequest async callback when constructing/assigning this._interface
that awaits this._emulatorInitPromise if present (i.e., implement
prepareRequest: async () => { if (this._emulatorInitPromise) await
this._emulatorInitPromise; }) so any request via the interface waits for
_emulatorInitPromise to resolve before proceeding.
In `@packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts`:
- Around line 503-538: The emulator credentials are applied asynchronously after
the StackClientInterface is created, so synchronous methods that derive cookie
names (notably _getSession and _getOrCreateTokenStore) can use the pre-override
projectId and miss emulator sessions; fix by ensuring emulator credentials are
resolved before any code can synchronously read projectId or cookie names:
either await fetchEmulatorProjectCredentials(emulatorConfigFilePath) and apply
the returned project_id/publishable_client_key to projectId/publishableClientKey
before constructing StackClientInterface, or modify _getSession and
_getOrCreateTokenStore to await this._emulatorInitPromise (if present) before
deriving cookie names (and ensure any hook/path that calls them will suspend
while _emulatorInitPromise resolves); update references to prepareRequest only
for network protection and keep a single source of truth by applying the
emulator override prior to exposing the app interface.
In `@packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts`:
- Around line 420-438: The server interface is created immediately so initial
requests can run with pre-override keys; to fix, mirror the client-side behavior
by ensuring emulator credential fetch completes before the server request path
uses the interface: when isEmulator and no extraOptions.interface, set
this._emulatorInitPromise = fetchEmulatorProjectCredentials(...), then either
await that promise before constructing/assigning the StackServerInterface or
ensure this._interface’s request-building methods internally await
this._emulatorInitPromise (i.e., update server-app-impl.ts so the code around
StackServerInterface, this._interface and _emulatorInitPromise ensures the
credentials are applied before any inherited request code runs).
---
Outside diff comments:
In `@apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx`:
- Around line 113-130: The Prisma query in
globalPrismaClient.apiKeySet.findFirst (assigned to existingKeySet) filters for
secretServerKey and superSecretAdminKey but not publishableClientKey, so it may
return rows missing publishableClientKey and later cause the assertion at line
144 to fail; update the findFirst where clause to include publishableClientKey:
{ not: null } so only key sets that contain all three keys (secretServerKey,
superSecretAdminKey, publishableClientKey) are returned by existingKeySet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f1b977d5-c56f-409f-a728-6a208f15751d
📒 Files selected for processing (11)
apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsxapps/backend/src/lib/local-emulator.tsdocker/server/entrypoint.shpackages/stack-shared/src/interface/admin-interface.tspackages/stack-shared/src/interface/client-interface.tspackages/stack-shared/src/interface/server-interface.tspackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/apps/implementations/client-app-impl.tspackages/template/src/lib/stack-app/apps/implementations/common.tspackages/template/src/lib/stack-app/apps/implementations/server-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/client-app.ts
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
Show resolved
Hide resolved
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
Show resolved
Hide resolved
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
Show resolved
Hide resolved
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
Show resolved
Hide resolved
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
Show resolved
Hide resolved
Add prepareRequest callbacks to StackServerInterface and StackAdminInterface that await _emulatorInitPromise, matching the existing pattern in the client app. Prevents race conditions where requests use placeholder credentials before the emulator credentials are fetched.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (1)
435-435: Consider replacing non-null assertion with explicit error.While the non-null assertion is logically safe here (the
isEmulatorguard on line 433 ensuresemulatorConfigFilePathis truthy), the coding guidelines prefer explicit error messages that state the assumption.💡 Suggested improvement
- this._emulatorInitPromise = fetchEmulatorProjectCredentials(emulatorConfigFilePath!).then((data) => { + this._emulatorInitPromise = fetchEmulatorProjectCredentials(emulatorConfigFilePath ?? throwErr("emulatorConfigFilePath must be defined when isEmulator is true")).then((data) => {As per coding guidelines: "Prefer
?? throwErr(...)over non-null assertions with good error messages explicitly stating the assumption."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts` at line 435, Replace the non-null assertion on emulatorConfigFilePath when initializing this._emulatorInitPromise with an explicit null-check that throws a clear error; specifically, change the call to fetchEmulatorProjectCredentials(emulatorConfigFilePath!) in the _emulatorInitPromise assignment to use emulatorConfigFilePath ?? throwErr("expected emulatorConfigFilePath to be set when isEmulator is true") (or your project's throw helper), keeping the surrounding isEmulator guard and the fetchEmulatorProjectCredentials call intact so the code fails with a descriptive message instead of relying on a non-null assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts`:
- Line 435: Replace the non-null assertion on emulatorConfigFilePath when
initializing this._emulatorInitPromise with an explicit null-check that throws a
clear error; specifically, change the call to
fetchEmulatorProjectCredentials(emulatorConfigFilePath!) in the
_emulatorInitPromise assignment to use emulatorConfigFilePath ??
throwErr("expected emulatorConfigFilePath to be set when isEmulator is true")
(or your project's throw helper), keeping the surrounding isEmulator guard and
the fetchEmulatorProjectCredentials call intact so the code fails with a
descriptive message instead of relying on a non-null assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1420a7b-0c62-4608-9dff-9f228ac2008c
📒 Files selected for processing (2)
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
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.
Summary by CodeRabbit
New Features
Chores