chore: add local full-stack dev setup#573
Open
sidpalas wants to merge 10 commits intoColeMurray:mainfrom
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds environment templates, validation and serialization utilities, CLI scripts to generate per-service env files and provision Modal secrets, docs for running the web app and control plane locally with real GitHub OAuth and remote sandboxes, npm scripts, and D1 migration script improvements for local/remote modes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Developer
participant LocalScripts as "Local Scripts\n(env-utils, local-env, modal-secrets)"
participant Web as "Web App\nlocalhost:3000"
participant ControlPlane as "Control Plane\nlocalhost:8787"
participant Modal as "Modal Sandbox / Secrets"
participant GitHub as "GitHub App OAuth"
participant D1 as "D1 Database"
Developer->>LocalScripts: run `npm run dev:env`
LocalScripts->>LocalScripts: read & normalize root `.env.local`
LocalScripts->>Web: write `packages/web/.env.local`
LocalScripts->>ControlPlane: write `packages/control-plane/.dev.vars`
Developer->>LocalScripts: run `npm run dev:modal-secrets`
LocalScripts->>Modal: create/update secrets (llm-api-keys, github-app, internal-api)
Developer->>GitHub: configure App callback to CONTROL_PLANE_URL
Developer->>ControlPlane: start control plane (wrangler dev)
Developer->>Web: start web dev server
Developer->>D1: run `scripts/d1-migrate.sh` (local/remote)
D1-->>Developer: migration results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 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 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
docs/SETUP_GUIDE.md (1)
45-57:⚠️ Potential issue | 🟡 MinorSeparate required vs optional quick-check commands.
Putting optional tools in the same block as required checks can make Path A/B users think setup failed when those binaries are intentionally absent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/SETUP_GUIDE.md` around lines 45 - 57, Split the single "Quick check" code block into two labeled blocks (Required checks and Optional checks) in docs/SETUP_GUIDE.md: create one fenced block containing the truly required commands (e.g., node -v, npm -v, git --version, python --version) and a second fenced block titled "Optional checks" containing tools like uv --version, modal --version, npx wrangler --version, jq --version, ngrok --version; ensure headings or short notes clarify that absence of optional tools is acceptable so Path A/B users won't treat missing optional binaries as failures.
🧹 Nitpick comments (6)
scripts/local-env.mjs (1)
23-24: Consider making the post-step hint provider-aware.The script always prompts Modal secret sync; this can be confusing when
SANDBOX_PROVIDERis set to a non-Modal path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/local-env.mjs` around lines 23 - 24, The two console.log hints that tell users to "Next, sync Modal secrets..." should be conditional on the sandbox provider; check process.env.SANDBOX_PROVIDER and only print those console.log messages when the provider indicates Modal (e.g., equals or contains "modal"); otherwise skip or print a provider-appropriate hint. Update the code around the existing console.log calls so the messages are gated by SANDBOX_PROVIDER and keep the original messages unchanged when shown..env.example (1)
5-58: Optional: reorder keys if dotenv-linter warnings are CI-gated.Current grouping is readable, but it triggers multiple
UnorderedKeywarnings. If dotenv-linter is enforced, reordering will keep CI clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 5 - 58, Dotenv-linter is flagging UnorderedKey warnings; reorder the keys in .env.example so they follow the linter's expected order (typically alphabetical) to remove those warnings. Specifically, sort the environment variable entries (for example GITHUB_CLIENT_ID, GITHUB_CLIENT_SECRET, GITHUB_APP_ID, GITHUB_APP_PRIVATE_KEY, GITHUB_APP_INSTALLATION_ID, SANDBOX_PROVIDER, MODAL_TOKEN_ID, MODAL_TOKEN_SECRET, MODAL_WORKSPACE, MODAL_API_SECRET, DAYTONA_API_KEY, DAYTONA_API_URL, DAYTONA_BASE_SNAPSHOT, ANTHROPIC_API_KEY, TOKEN_ENCRYPTION_KEY, REPO_SECRETS_ENCRYPTION_KEY, INTERNAL_CALLBACK_SECRET, NEXTAUTH_SECRET, NEXTAUTH_URL, CONTROL_PLANE_URL, NEXT_PUBLIC_WS_URL, WEB_APP_URL, WORKER_URL, DEPLOYMENT_NAME, ALLOWED_USERS, ALLOWED_EMAIL_DOMAINS, UNSAFE_ALLOW_ALL_USERS, SCM_PROVIDER, LOG_LEVEL) while preserving comment blocks and example values; run dotenv-linter locally or in CI to confirm warnings are resolved.scripts/env-utils.mjs (1)
122-127: Silent error swallowing hides root cause.The empty catch block discards the actual filesystem error (e.g., permission denied vs. file not found). Logging the error before the user-friendly message would help debugging.
🔧 Proposed improvement
try { source = readFileSync(sourcePath, "utf8"); - } catch { + } catch (err) { + if (process.env.DEBUG) console.error(err); console.error("Missing .env.local. Copy .env.example to .env.local and fill in real values."); process.exit(1); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/env-utils.mjs` around lines 122 - 127, The catch currently swallows the actual filesystem error when calling readFileSync(sourcePath, "utf8") and only prints a generic message; change the catch to capture the thrown error (e.g., catch (err)) and include that error in the log before calling process.exit(1) so you see details like ENOENT or EACCES; update the console.error call(s) around readFileSync/source to print both a user-friendly instruction and the captured error (or use console.error("...", err)) to preserve root-cause information.scripts/modal-secrets.mjs (2)
5-17:shellWordsedge case: empty string returns[""]instead of[].When
commandis an empty string, the regex matches nothing, so the nullish coalescing returns[""]. This could causespawnSyncto fail cryptically ifMODAL_CLI=""is set.🛡️ Proposed fix to handle empty command
function shellWords(command) { + if (!command?.trim()) { + return ["modal"]; + } return ( command.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g)?.map((part) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/modal-secrets.mjs` around lines 5 - 17, The shellWords function returns [""] for an empty command because the regex match is null and the fallback uses [command]; update shellWords to treat empty or all-whitespace command as no words: if command.trim() === "" return [] (or change the nullish fallback to []). Locate the shellWords function and replace the current nullish-coalescing fallback ([command]) with a guard that returns an empty array for empty input so callers like spawnSync receive [] instead of [""]. Ensure the function still strips surrounding quotes for non-empty inputs.
31-34: Consider logging the command on failure for debugging.When
spawnSyncfails, the script exits but doesn't indicate which secret creation failed if the Modal CLI itself doesn't print useful output. Theconsole.logon line 30 helps, but capturing and logging stderr on failure could aid troubleshooting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/modal-secrets.mjs` around lines 31 - 34, When handling the spawnSync call that assigns to result, enhance the failure logging so you print which command failed and the captured stderr/stdout; specifically, after const result = spawnSync(command, args, {...}) check result.status or result.error and log a clear message including the command and args (or reconstructed command string) and result.stderr (and result.stdout) before exiting. Reference the existing symbols spawnSync, result, command, args, root and the current console.log so the new log appears when the Modal CLI fails.docs/LOCAL_WEB_CONTROL_PLANE.md (1)
31-46: Document ngrok alternatives or free-tier limitations.ngrok's free tier cycles URLs on restart. Consider mentioning this and alternatives (e.g.,
cloudflared tunnel,localhost.run) for developers who hit rate limits or need stable URLs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LOCAL_WEB_CONTROL_PLANE.md` around lines 31 - 46, Update the section that instructs to run "ngrok http 8787" to note ngrok's free-tier behavior (ephemeral URLs on restart and potential rate limits) and add brief alternatives such as "cloudflared tunnel" and "localhost.run" with a short line about using a stable tunnel for production/local development; also update the example env variables (CONTROL_PLANE_URL, NEXT_PUBLIC_WS_URL, WORKER_URL) to mention replacing them with your tunnel URL and call out that stable paid ngrok plans or cloudflared provide persistent hostnames if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/d1-migrate.sh`:
- Line 53: The script sets VERSION from FILENAME using VERSION=$(printf "%s"
"$FILENAME" | grep -oE '^[0-9]+') but does not validate that VERSION is
non-empty; add a check right after that assignment to ensure VERSION contains
only digits (e.g., test -n "$VERSION" && [[ "$VERSION" =~ ^[0-9]+$ ]]) and if it
fails print an error referencing the offending FILENAME and exit non‑zero so the
SQL execution/tracking steps are not run with an empty version; ensure error
messages mention the variables VERSION and FILENAME so it's clear what failed.
- Around line 44-46: The APPLIED assignment masks errors by appending
"2>/dev/null || true" to the WRANGLER/jq pipeline; remove the stderr redirection
and the "|| true" so failures in the command that queries _schema_migrations
propagate (fail the script under set -euo pipefail). Locate the APPLIED
assignment (the line using WRANGLER d1 execute with DATABASE_NAME and D1_OPTIONS
and jq -r '.[0].results[].version // empty') and update it to let errors surface
instead of silencing them.
In `@scripts/env-utils.mjs`:
- Around line 5-6: The current root and sourcePath definitions use resolve(new
URL("..", import.meta.url).pathname) which yields POSIX-style paths on Windows;
replace that URL->pathname usage with Node's fileURLToPath to get a
platform-correct path. Import fileURLToPath from "url" and use fileURLToPath(new
URL("..", import.meta.url)) (or fileURLToPath(import.meta.url) combined with
dirname) when computing root and then resolve(root, ".env.local") for
sourcePath; update the symbols root and sourcePath accordingly so resolve
receives a proper OS path.
In `@scripts/modal-secrets.mjs`:
- Around line 55-56: The code constructs controlPlaneUrl with new
URL(env.CONTROL_PLANE_URL) which will throw a TypeError for malformed URLs;
update the initialization of controlPlaneUrl/allowedHosts to validate and handle
bad input: wrap the new URL(...) in a try/catch or explicitly validate
env.CONTROL_PLANE_URL before constructing controlPlaneUrl, and on failure throw
or log a clear error (including the invalid value) and exit, or fall back to a
safe default; reference controlPlaneUrl, allowedHosts, and env.CONTROL_PLANE_URL
when making this change so the script no longer crashes with an unhandled
exception for malformed CONTROL_PLANE_URL.
---
Outside diff comments:
In `@docs/SETUP_GUIDE.md`:
- Around line 45-57: Split the single "Quick check" code block into two labeled
blocks (Required checks and Optional checks) in docs/SETUP_GUIDE.md: create one
fenced block containing the truly required commands (e.g., node -v, npm -v, git
--version, python --version) and a second fenced block titled "Optional checks"
containing tools like uv --version, modal --version, npx wrangler --version, jq
--version, ngrok --version; ensure headings or short notes clarify that absence
of optional tools is acceptable so Path A/B users won't treat missing optional
binaries as failures.
---
Nitpick comments:
In @.env.example:
- Around line 5-58: Dotenv-linter is flagging UnorderedKey warnings; reorder the
keys in .env.example so they follow the linter's expected order (typically
alphabetical) to remove those warnings. Specifically, sort the environment
variable entries (for example GITHUB_CLIENT_ID, GITHUB_CLIENT_SECRET,
GITHUB_APP_ID, GITHUB_APP_PRIVATE_KEY, GITHUB_APP_INSTALLATION_ID,
SANDBOX_PROVIDER, MODAL_TOKEN_ID, MODAL_TOKEN_SECRET, MODAL_WORKSPACE,
MODAL_API_SECRET, DAYTONA_API_KEY, DAYTONA_API_URL, DAYTONA_BASE_SNAPSHOT,
ANTHROPIC_API_KEY, TOKEN_ENCRYPTION_KEY, REPO_SECRETS_ENCRYPTION_KEY,
INTERNAL_CALLBACK_SECRET, NEXTAUTH_SECRET, NEXTAUTH_URL, CONTROL_PLANE_URL,
NEXT_PUBLIC_WS_URL, WEB_APP_URL, WORKER_URL, DEPLOYMENT_NAME, ALLOWED_USERS,
ALLOWED_EMAIL_DOMAINS, UNSAFE_ALLOW_ALL_USERS, SCM_PROVIDER, LOG_LEVEL) while
preserving comment blocks and example values; run dotenv-linter locally or in CI
to confirm warnings are resolved.
In `@docs/LOCAL_WEB_CONTROL_PLANE.md`:
- Around line 31-46: Update the section that instructs to run "ngrok http 8787"
to note ngrok's free-tier behavior (ephemeral URLs on restart and potential rate
limits) and add brief alternatives such as "cloudflared tunnel" and
"localhost.run" with a short line about using a stable tunnel for
production/local development; also update the example env variables
(CONTROL_PLANE_URL, NEXT_PUBLIC_WS_URL, WORKER_URL) to mention replacing them
with your tunnel URL and call out that stable paid ngrok plans or cloudflared
provide persistent hostnames if needed.
In `@scripts/env-utils.mjs`:
- Around line 122-127: The catch currently swallows the actual filesystem error
when calling readFileSync(sourcePath, "utf8") and only prints a generic message;
change the catch to capture the thrown error (e.g., catch (err)) and include
that error in the log before calling process.exit(1) so you see details like
ENOENT or EACCES; update the console.error call(s) around readFileSync/source to
print both a user-friendly instruction and the captured error (or use
console.error("...", err)) to preserve root-cause information.
In `@scripts/local-env.mjs`:
- Around line 23-24: The two console.log hints that tell users to "Next, sync
Modal secrets..." should be conditional on the sandbox provider; check
process.env.SANDBOX_PROVIDER and only print those console.log messages when the
provider indicates Modal (e.g., equals or contains "modal"); otherwise skip or
print a provider-appropriate hint. Update the code around the existing
console.log calls so the messages are gated by SANDBOX_PROVIDER and keep the
original messages unchanged when shown.
In `@scripts/modal-secrets.mjs`:
- Around line 5-17: The shellWords function returns [""] for an empty command
because the regex match is null and the fallback uses [command]; update
shellWords to treat empty or all-whitespace command as no words: if
command.trim() === "" return [] (or change the nullish fallback to []). Locate
the shellWords function and replace the current nullish-coalescing fallback
([command]) with a guard that returns an empty array for empty input so callers
like spawnSync receive [] instead of [""]. Ensure the function still strips
surrounding quotes for non-empty inputs.
- Around line 31-34: When handling the spawnSync call that assigns to result,
enhance the failure logging so you print which command failed and the captured
stderr/stdout; specifically, after const result = spawnSync(command, args,
{...}) check result.status or result.error and log a clear message including the
command and args (or reconstructed command string) and result.stderr (and
result.stdout) before exiting. Reference the existing symbols spawnSync, result,
command, args, root and the current console.log so the new log appears when the
Modal CLI fails.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8ee263a-a68b-4a1b-99d6-71c091868630
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.env.exampledocs/LOCAL_WEB_CONTROL_PLANE.mddocs/SETUP_GUIDE.mdeslint.config.jspackage.jsonpackages/control-plane/package.jsonscripts/d1-migrate.shscripts/env-utils.mjsscripts/local-env.mjsscripts/modal-secrets.mjsterraform/environments/production/d1.tf
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.
Addresses #486
Summary
.env.local, with generated service-specific env files for web and control-plane.@ColeMurray -- what do you think of this env management approach: using root
.env.localas the single editable source of truth, then generating service-specific env files for web and control-plane? Open to suggestions if you have a preferred approach!Future Work
Summary by CodeRabbit
Documentation
New Features
Style
Chores