Fix build and lint failures on dev#1445
Conversation
- Fix CLI typecheck error: resolveSessionAuth() takes no arguments
- Add dashboard .env.production.local setup to CI workflows that run
build:packages (check-prisma-migrations, setup-tests, setup-tests-with-custom-base-port)
The dashboard build is now triggered by build:packages because
@stackframe/stack-cli#build depends on @stackframe/dashboard#build:rde-standalone.
Without the env file, new URL('') throws TypeError: Invalid URL in layout.tsx.
Co-Authored-By: Konstantin Wohlwend <n2d4xc@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 PR converts the dashboard server export to a lazy, memoized accessor, updates callers to use getStackServerApp(), centralizes dashboard metadata to derive from NEXT_PUBLIC_STACK_API_URL, and simplifies whoami's auth resolution call. ChangesServer app lazy singleton pattern
CLI and dashboard metadata updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes two unrelated CI failures on
Confidence Score: 5/5Both fixes are targeted and correct; the workflow env file copy and the removed argument are straightforward corrections with no side effects. The No files require special attention; the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CI Workflow triggered] --> B{build:packages}
B --> C[stack-cli#build]
C --> D[dashboard#build:rde-standalone\nvia turbo.json dependency]
D --> E{.env.production.local\nexists?}
E -- No --> F["TypeError: Invalid URL\n(NEXT_PUBLIC_STACK_API_URL empty)"]
E -- Yes --> G[Build succeeds]
subgraph "Fix applied to 3 workflows"
H["cp .env.development\n→ .env.production.local"]
end
H --> E
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
.github/workflows/check-prisma-migrations.yaml:45-49
The `.env.production.local` copy step is placed before `pnpm install` here, while the equivalent step in `setup-tests.yaml` and `setup-tests-with-custom-base-port.yaml` is placed after install. Functionally this makes no difference (the source file is a committed repo file), but moving it after `pnpm install` would keep all three workflows consistent with each other and with `lint-and-build.yaml`.
```suggestion
- name: Install dependencies
run: pnpm install --frozen-lockfile
- name: Create .env.production.local file for apps/dashboard
run: cp apps/dashboard/.env.development apps/dashboard/.env.production.local
```
Reviews (1): Last reviewed commit: "Fix build and lint failures on dev" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - name: Create .env.production.local file for apps/dashboard | ||
| run: cp apps/dashboard/.env.development apps/dashboard/.env.production.local | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile |
There was a problem hiding this comment.
.env.production.local copy step is placed before pnpm install here, while the equivalent step in setup-tests.yaml and setup-tests-with-custom-base-port.yaml is placed after install. Functionally this makes no difference (the source file is a committed repo file), but moving it after pnpm install would keep all three workflows consistent with each other and with lint-and-build.yaml.
| - name: Create .env.production.local file for apps/dashboard | |
| run: cp apps/dashboard/.env.development apps/dashboard/.env.production.local | |
| - name: Install dependencies | |
| run: pnpm install --frozen-lockfile | |
| - name: Install dependencies | |
| run: pnpm install --frozen-lockfile | |
| - name: Create .env.production.local file for apps/dashboard | |
| run: cp apps/dashboard/.env.development apps/dashboard/.env.production.local |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/check-prisma-migrations.yaml
Line: 45-49
Comment:
The `.env.production.local` copy step is placed before `pnpm install` here, while the equivalent step in `setup-tests.yaml` and `setup-tests-with-custom-base-port.yaml` is placed after install. Functionally this makes no difference (the source file is a committed repo file), but moving it after `pnpm install` would keep all three workflows consistent with each other and with `lint-and-build.yaml`.
```suggestion
- name: Install dependencies
run: pnpm install --frozen-lockfile
- name: Create .env.production.local file for apps/dashboard
run: cp apps/dashboard/.env.development apps/dashboard/.env.production.local
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Pull request overview
Fixes CI failures on dev by addressing a Stack CLI typecheck break and ensuring the dashboard can be built as an indirect dependency of build:packages in workflows that previously didn’t prepare dashboard production env files.
Changes:
- Update
stack-cli’swhoamicommand to callresolveSessionAuth()with no arguments (matching the refactor). - Add a GitHub Actions step to create
apps/dashboard/.env.production.local(copied from.env.development) before runningbuild:packagesin additional workflows.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/stack-cli/src/commands/whoami.ts | Removes the now-invalid flags argument when calling resolveSessionAuth(), fixing TS2554. |
| .github/workflows/setup-tests.yaml | Creates apps/dashboard/.env.production.local before build:packages so dashboard builds don’t fail due to missing public env vars. |
| .github/workflows/setup-tests-with-custom-base-port.yaml | Same env-file preparation as above for the custom base port workflow. |
| .github/workflows/check-prisma-migrations.yaml | Ensures dashboard env file exists before build:packages runs as part of the migrations check workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…_URL gracefully Instead of requiring .env.production.local in CI workflows, make layout.tsx handle the case where NEXT_PUBLIC_STACK_API_URL is unset by conditionally setting metadataBase and OG images. This reverts the workflow changes from the previous commit. Co-Authored-By: Konstantin Wohlwend <n2d4xc@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/setup-tests.yaml (1)
38-39: 💤 Low valueConsider standardizing step placement across workflows.
In
check-prisma-migrations.yaml, this step runs beforepnpm install, while here and insetup-tests-with-custom-base-port.yaml, it runs after. Both work since the file is only needed for the build step, but consistent placement would improve maintainability.🤖 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 @.github/workflows/setup-tests.yaml around lines 38 - 39, The "Create .env.production.local file for apps/dashboard" step is placed after pnpm install in this workflow but before pnpm install in check-prisma-migrations and after in setup-tests-with-custom-base-port, causing inconsistency; pick one standard placement (e.g., move the "Create .env.production.local file for apps/dashboard" step to run before the pnpm install step across all workflows, or consistently keep it after) and update this workflow to match that convention so all workflows (check-prisma-migrations, setup-tests-with-custom-base-port, and this file) have the same ordering.
🤖 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 @.github/workflows/setup-tests.yaml:
- Around line 38-39: The "Create .env.production.local file for apps/dashboard"
step is placed after pnpm install in this workflow but before pnpm install in
check-prisma-migrations and after in setup-tests-with-custom-base-port, causing
inconsistency; pick one standard placement (e.g., move the "Create
.env.production.local file for apps/dashboard" step to run before the pnpm
install step across all workflows, or consistently keep it after) and update
this workflow to match that convention so all workflows
(check-prisma-migrations, setup-tests-with-custom-base-port, and this file) have
the same ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad425c96-d148-4114-977e-236288f7a0ba
📒 Files selected for processing (4)
.github/workflows/check-prisma-migrations.yaml.github/workflows/setup-tests-with-custom-base-port.yaml.github/workflows/setup-tests.yamlpackages/stack-cli/src/commands/whoami.ts
The dashboard .env file is loaded by Next.js during build. Without a truthy STACK_SECRET_SERVER_KEY, StackServerApp constructor throws at module evaluation time when Next.js collects page metadata for pages that import from @/stack/server. This is the server-side equivalent of the sentinel mechanism for NEXT_PUBLIC_* vars — a build-time placeholder that gets overridden by .env.development in dev and actual env vars in production. Co-Authored-By: Konstantin Wohlwend <n2d4xc@gmail.com>
…ild time The StackServerApp constructor requires STACK_SECRET_SERVER_KEY, but this env var isn't available during next build in CI workflows that don't provide .env.production.local. Since Next.js evaluates modules at build time to collect page metadata, importing server.tsx would crash. Fix: change stackServerApp from a module-level constant to a lazily- initialized getter function getStackServerApp(). The StackServerApp is only constructed on first runtime use, not during build. Also reverts the .env placeholder hack from the previous commit. Co-Authored-By: Konstantin Wohlwend <n2d4xc@gmail.com>
…ser type The getStackServerApp() was typed as plain StackServerApp (defaults to <boolean, string>), losing the 'internal' ProjectId type parameter. This caused getUser() to return CurrentServerUser instead of CurrentInternalServerUser, which doesn't have listOwnedProjects. Co-Authored-By: Konstantin Wohlwend <n2d4xc@gmail.com>
Fixes two issues causing CI failures on
dev:1. CLI typecheck error
resolveSessionAuth()was refactored to take no arguments, butwhoami.tswas still passingflagsto it — causingerror TS2554: Expected 0 arguments, but got 1.2. Dashboard build failure in workflows using
build:packagesSince
@stackframe/stack-cli#builddepends on@stackframe/dashboard#build:rde-standalone(via turbo.json), runningbuild:packagesnow also builds the dashboard. Without.env.production.local,new URL(getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL') || '')inlayout.tsxthrowsTypeError: Invalid URL.Added the
.env.production.localcopy step (already present inlint-and-build.yaml) to:check-prisma-migrations.yamlsetup-tests.yamlsetup-tests-with-custom-base-port.yamlLink to Devin session: https://app.devin.ai/sessions/5ac9f5f6d4f0405e946b3993c6b08cd8
Requested by: @N2D4
Note
Medium Risk
Touches server-side initialization paths in the dashboard by replacing a global
stackServerAppexport with a lazygetStackServerApp()accessor; mistakes here could surface as runtime failures in server actions/pages. Other changes are straightforward build/typecheck fixes (URL guarding and CLI arg removal).Overview
Fixes dashboard build/runtime edge cases by replacing the exported
stackServerAppsingleton with a lazy, memoizedgetStackServerApp()(including the remote-dev-environment guard) and updating server actions/pages to call the accessor.Hardens
apps/dashboardmetadata generation to avoid constructingnew URL('')whenNEXT_PUBLIC_STACK_API_URLis missing, and updates the CLIwhoamicommand to match the refactoredresolveSessionAuth()signature (no args).Reviewed by Cursor Bugbot for commit e321ee5. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Refactor
Improvements