Skip to content

feat: local dev without third-party credentials#4616

Open
Kitenite wants to merge 11 commits into
mainfrom
local-setup-no-env
Open

feat: local dev without third-party credentials#4616
Kitenite wants to merge 11 commits into
mainfrom
local-setup-no-env

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented May 16, 2026

Summary

A fresh clone can now boot the local dev stack against Docker Postgres + Electric without Neon, OAuth, Stripe, Resend, QStash, Upstash, or other third-party credentials.

git clone …
cd superset
bun install
bun setup:local
bun dev

bun setup:local copies local examples without overwriting existing files, starts Docker Postgres/Electric, runs caddy trust, and applies migrations. The copied .env.example sets SUPERSET_PROFILE=local, stable local 464x ports, local Docker database URLs, and SUPERSET_WORKSPACE_NAME=local-dev so desktop state lives in ~/.superset-local-dev instead of production / canary ~/.superset. If an existing .env is not the local contributor profile, the script stops before running migrations.

What changed

  • Replaced the experiment flag with explicit deployment profiles: cloud, local, ci, and internal.
  • Centralized env-validation behavior behind shouldSkipEnvValidation() and kept strict validation for cloud and internal.
  • Added API integration status helpers used by /api/health and the boot summary.
  • Made optional services lazy/guarded when credentials are blank: QStash, OAuth providers, auth QStash hooks, and Upstash rate limiting.
  • Kept cloud relay tunnels disabled for local / ci unless RELAY_URL is explicitly set.
  • Isolated contributor desktop state with SUPERSET_WORKSPACE_NAME=local-dev and skipped macOS Launch Services/protocol patching in the local profile.
  • Updated local docs/examples for macOS-only contributor setup and removed stale Linux/local-dev wording.
  • Added bun setup:local as the concise contributor setup command.

Production and canary safety

  • Default profile remains internal, which is strict.
  • Vercel markers (VERCEL=1 / VERCEL_ENV) force the cloud profile above any local/CI flags.
  • Local profile is opt-in via .env / SUPERSET_PROFILE=local.
  • Contributor desktop state uses ~/.superset-local-dev, not ~/.superset.
  • Local desktop predev skips Launch Services cleanup and protocol patching, so source builds do not rewrite shared production/canary registrations.

Manual test plan

These steps are safe to run while production/canary Superset is open. bun setup:local uses ~/.superset-local-dev for contributor desktop state and refuses to run migrations if an existing .env is not the local contributor profile.

1. Setup

git checkout local-setup-no-env
git pull
unset SUPERSET_HOME_DIR
bun install
bun setup:local

Expected:

  • .env exists and contains SUPERSET_PROFILE=local and SUPERSET_WORKSPACE_NAME=local-dev.
  • apps/electric-proxy/.dev.vars and Caddyfile exist.
  • Docker Postgres/Electric are running on 5433 / 4649.
  • DB migrations complete.
  • Existing files are not overwritten.

If you already have an internal .env, bun setup:local should stop before migrations. Move that .env aside only if you intentionally want the local contributor setup.

2. Run

bun dev

Expected services:

Service Expected
Web http://localhost:4640 ready
API http://localhost:4641 ready
Desktop renderer http://localhost:4645 ready
Caddy https://localhost:4650 ready
electric-proxy http://localhost:4652 ready
Desktop state files appear under ~/.superset-local-dev, not ~/.superset

Expected API boot summary includes:

[superset] profile=local (lenient)
[superset] disabled features

3. Checks

curl -fsS http://localhost:4641/api/health | jq
curl -fsS http://localhost:4641/api/auth/ok
curl -fsSI http://localhost:4640/sign-in | head

Expected:

  • /api/health returns "ok": true and "profile": "local".
  • Integrations without keys are reported as "missing", not boot failures.
  • /api/auth/ok returns {"ok":true}.
  • /sign-in returns 200.

Desktop checks:

  • Auto-signed-in as admin@local.test, or sign in with admin@local.test / supersetdev.
  • No production/canary workspaces are shown from ~/.superset.
  • Host service connects and terminals can be created.
  • ~/.superset-local-dev/auth-token.enc exists after sign-in.

4. Strict profile checks

Stop bun dev, then verify strict profiles still fail fast when required keys are blank:

cp .env.example .env
perl -0pi -e 's/SUPERSET_PROFILE=local
//g' .env
bun dev

Expected: strict internal profile fails env validation before serving API.

Cloud precedence check:

cp .env.example .env
VERCEL_ENV=preview bun dev

Expected: strict cloud profile fails env validation even though .env contains SUPERSET_PROFILE=local.

5. Cleanup

# Stop bun dev with Ctrl-C first.
docker compose -f docker-compose.dev.yml down
rm -rf ~/.superset-local-dev

Do not remove ~/.superset; that is production/canary state.

Verification

Run on May 17, 2026 from branch local-setup-no-env. The bun dev scenario checks below were run before the final desktop state-isolation cleanup; after that cleanup I intentionally did not relaunch Electron to avoid touching shared production/canary desktop state, and used static/unit/full-suite validation instead.

Scenario checks

  • bun dev with .env.example copied to .env (SUPERSET_PROFILE=local): web, API, desktop Vite, Caddy, electric-proxy, and Docker Electric booted on the documented ports. GET /api/health returned ok: true, profile: "local", and missing integrations as expected. GET /api/auth/ok returned {"ok":true}. HEAD /sign-in returned 200. Cloud relay tunnel logs did not appear after the explicit-relay guard.
  • bun dev with SUPERSET_PROFILE=ci / CI=true: stack booted and API logged profile=ci (lenient).
  • bun dev with no profile: failed fast during strict env validation, as expected for internal.
  • bun dev with VERCEL_ENV=preview: failed fast during strict env validation, as expected for cloud.

Static and automated checks

  • bun setup:local --help: passed.
  • bun setup:local --skip-docker --skip-migrate --skip-caddy-trust against this existing non-local .env: exited 1 before Docker/Caddy/migrations, confirming the safety guard.
  • bun run lint:fix: passed, no fixes remaining.
  • bun test packages/shared/src/deployment-profile.test.ts apps/api/src/lib/integration-status.test.ts apps/desktop/scripts/patch-dev-protocol.test.ts: 14 pass, 0 fail.
  • bun run lint: passed, 4499 files checked, 0 lint output.
  • bun run typecheck: passed, 28 successful / 28 total.
  • bun run test: passed, 12 successful / 12 total. Notable package results: @superset/desktop 1932 pass / 0 fail; @superset/host-service 633 pass / 8 todo / 0 fail.
  • git diff --check: passed.

Cleanup checks

  • Verified this worktree has no running next dev, wrangler dev, electron-vite dev, or Caddy process after testing.
  • Verified local app ports 4640, 4641, 4645, 4646, 4650, 4652, and 4653 are not listening after testing.
  • Docker still listens on 5433 and 4649 for the local Postgres/Electric dev containers.

Open in Stage

A fresh clone can now boot the full dev stack (web + api + desktop +
electric + electric-proxy + caddy) against a local Postgres in Docker,
with no Neon, OAuth, Stripe, Resend, or other cloud-service keys required.

Key changes:
- packages/db/src/client.ts: driver swap to drizzle-orm/node-postgres for
  non-Neon DATABASE_URLs (@neondatabase/serverless only speaks Neon's
  HTTP/WS protocol, not vanilla Postgres TCP).
- packages/auth/src/server.ts: emailAndPassword auth enabled + autoSignIn.
- apps/desktop/src/main/lib/dev-auto-sign-in.ts: during app.whenReady(),
  if SKIP_ENV_VALIDATION=1 and no token on disk, auto-signs-in as
  admin@local.test and persists the token via the existing saveToken().
  Renderer hydrates like a real OAuth user — no special renderer code.
- 17 renderer files: flip MOCK_ORG_ID priority — prefer real session's
  activeOrganizationId, fall back to MOCK_ORG_ID only without a session.
  Fixes the "Host service not available" toast that fired because the
  renderer was looking up host-service connections by the fake org id
  while host-service spawned for the real admin@local.test org.
- Lazy-init guards for Stripe (Proxy), Resend (Proxy), and PostHog (no-op
  surface) so they don't crash module load when keys are missing.
- apps/web: dev-only email/password form on sign-in/sign-up, gated on
  NODE_ENV !== "production".
- packages/db: bun db:seed:dev script to create admin@local.test;
  refuses prod + non-localhost DATABASE_URL.
- turbo.jsonc: SKIP_ENV_VALIDATION added to globalPassThroughEnv.
- docs/LOCAL_DEVELOPMENT.md: contributor-facing setup guide.
- plans/20260515-oss-local-setup.md: full record of what shipped + what's
  deferred (env vars .optional(), docker-compose.dev.yml, bun setup
  orchestrator, CI fresh-clone smoke test).

Verified end-to-end via Chrome DevTools Protocol (port 9333) — the
renderer's LocalHostServiceContext.activeHostUrl resolves to a live
host-service URL, so the import gate is open.
@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 16, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 16, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR enables a full fresh-clone local development flow: profile-driven env validation, lazy no-op guards for external services (PostHog/Resend/Stripe), Neon/local Postgres adapter switching + dev seeding, Electron dev auto-signin with CDP exposure for oss-dev, a web dev auth form, renderer org-id precedence fixes, boot-time health visibility, and updated docs/config.

Changes

OSS Local Development Setup

Layer / File(s) Summary
Deployment profile and exports
packages/shared/src/deployment-profile.ts, packages/shared/package.json
Adds ci profile, getDeploymentProfile()/isStrictProfile() behavior, and exports the deployment-profile helper.
Lazy-init guards for external services
apps/api/src/lib/analytics.ts, packages/trpc/src/lib/analytics.ts, packages/auth/src/lib/resend.ts, packages/auth/src/stripe.ts, packages/trpc/src/router/support/support.ts
PostHog, Resend, Stripe, and trpc resend switched to proxy-backed lazy initialization with safe no-op or clearer runtime errors when API keys are missing.
Database driver flexibility & dev seeding
packages/db/src/client.ts, packages/db/src/seed-dev.ts, packages/db/package.json, package.json
Detects Neon vs local Postgres to wire appropriate Drizzle adapters; adds seed:dev script and a dev seeding script that POSTs to api/auth/sign-up/email with safety checks; adds pg dependency types.
Profile-aware env validation wiring
apps/*/src/env.ts, packages/trpc/src/env.ts, turbo.jsonc
Apps and packages compute skipValidation from deployment profile (lenient for oss-dev/ci) while preserving SKIP_ENV_VALIDATION override; turbo.jsonc updated to include SKIP_ENV_VALIDATION in global env.
Electron CDP enablement & dev auto-sign-in
apps/desktop/src/main/index.ts, apps/desktop/src/main/lib/dev-auto-sign-in.ts
Adds CDP/remote-debugging switches for oss-dev; ensureDevAuthToken() implements dev-only token reuse/sign-in/sign-up and is triggered non-blocking during main startup.
Web-based dev authentication form
apps/web/src/app/(auth)/components/DevAuthForm/*, apps/web/src/app/(auth)/sign-in/..., apps/web/src/app/(auth)/sign-up/...
New DevAuthForm component for email/password sign-in/sign-up in non-production; sign-in/sign-up pages render it when not production.
Renderer activeOrganizationId precedence fixes
apps/desktop/src/renderer/routes/... (multiple files)
Multiple renderer components/providers now prefer session?.session?.activeOrganizationId and only use MOCK_ORG_ID when the session value is absent and SKIP_ENV_VALIDATION is enabled.
Boot summary and /api/health
apps/api/src/app/api/health/route.ts, apps/api/src/lib/boot-summary.ts, apps/api/src/instrumentation.ts
Adds logBootSummary() to report integration env var presence at boot and a /api/health endpoint returning the active profile and per-integration configured/missing statuses.
Documentation and configuration
README.md, docs/LOCAL_DEVELOPMENT.md, plans/20260515-oss-local-setup.md, docker-compose.dev.yml, apps/electric-proxy/.dev.vars.example, turbo.jsonc
Shortens README to link to detailed LOCAL_DEVELOPMENT.md; adds comprehensive local-dev doc and plan record; provides docker-compose for Postgres/Electric, example proxy vars, and turbo/packaging updates.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Poem

🐰 I hopped into code with a tiny cheer,
DB and dev seeds now run so near,
Keys may be missing — no crash, just a log,
Auto-signin hums and docs clear the fog,
Hooray — local dev, cozy and dear.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: enabling local development without third-party credentials.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed PR description is comprehensive with summary, detailed changes, production safety guarantees, and manual test plan with expected outcomes. All required template sections are well-completed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch local-setup-no-env

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 16, 2026

Greptile Summary

This PR enables a full local dev stack from a fresh clone, with no third-party credentials (Neon, OAuth, Stripe, Resend) required. It routes the DB client to node-postgres for non-Neon URLs, lazily guards all third-party singletons, adds a dev auto-sign-in flow in the Electron main process, and fixes 17 renderer files that were forcing MOCK_ORG_ID over a real session.

  • DB adapter split (packages/db/src/client.ts): detects Neon vs. standard Postgres via a URL regex and switches adapters accordingly; both db and dbWs fall back to drizzle-node-postgres with independent pg.Pool instances locally.
  • Auth surface expansion (packages/auth/src/server.ts): unconditionally enables emailAndPassword — needed for local dev but also activates email/password sign-up endpoints in production where previously only OAuth paths existed.
  • Lazy-throw guards: Stripe, Resend (with a console-log fallback for emails.send/batch.send), and PostHog are converted to proxy-based lazy-init so a missing key no longer crashes module load.

Confidence Score: 3/5

Merging adds email/password auth to production — a real expansion of the sign-up/sign-in surface that was not present before and is not gated to dev builds.

The unconditional emailAndPassword enabled flag in the shared auth server config activates open email/password registration on the live API with no email verification required. Everything else in the PR is well-guarded, but this one change alters the production auth surface in a way that deserves explicit sign-off before merging.

packages/auth/src/server.ts — the email/password enablement is unconditional; packages/db/src/client.ts — isNeon regex runs against the full URL string including credentials.

Security Review

  • Email/password auth enabled in production (packages/auth/src/server.ts): emailAndPassword: { enabled: true } is unconditional, activating /api/auth/sign-up/email on the production API. Without requireEmailVerification: true, anyone can register arbitrary accounts. The DevAuthForm UI is hidden in production builds, but the HTTP endpoint is fully reachable.
  • CDP exposed with remote-allow-origins: \"*\" (apps/desktop/src/main/index.ts): correctly gated to IS_DEV && SKIP_ENV_VALIDATION, so this is not a production risk.
  • Hardcoded dev credentials in source (dev-auto-sign-in.ts, seed-dev.ts): acceptable for dev-only paths, but credentials are also pre-filled in the DevAuthForm bundle for any non-production build.

Important Files Changed

Filename Overview
packages/auth/src/server.ts Adds emailAndPassword: { enabled: true, autoSignIn: true } globally — enables email/password auth in production, expanding the auth attack surface.
packages/db/src/client.ts Adds Neon vs standard-Postgres detection via regex on the full URL string; both db and dbWs fall back to drizzle-node-postgres with separate pools for local dev.
apps/desktop/src/main/lib/dev-auto-sign-in.ts New dev-only helper that auto-signs in as the seed admin on cold boot; token-freshness logic can overwrite a valid token that has no expiresAt, and the local TTL can drift from the server session lifetime.
packages/auth/src/lib/resend.ts Converts eager Resend init to a lazy proxy; stubs emails.send and batch.send to stdout when no API key is present.
packages/auth/src/stripe.ts Wraps Stripe client in a lazy-init proxy; throws with a clear message at first use if STRIPE_SECRET_KEY is absent.
apps/desktop/src/main/index.ts Wires ensureDevAuthToken() before host-service discovery and opens CDP on port 9333 — both correctly gated to dev-only.
packages/db/src/seed-dev.ts New dev seed script with adequate safety guards (refuses production env and non-localhost DB URLs).
apps/web/src/app/(auth)/components/DevAuthForm/DevAuthForm.tsx New dev-only email/password form pre-filled with seed credentials; rendered only when NODE_ENV !== "production".
apps/api/src/lib/analytics.ts Converts PostHog to a lazy-init proxy; returns a no-op stub when NEXT_PUBLIC_POSTHOG_KEY is missing.

Sequence Diagram

sequenceDiagram
    participant Main as Electron Main
    participant API as API Server
    participant Disk as Token Store (disk)
    participant Renderer as Renderer / AuthProvider

    Main->>Disk: loadToken()
    alt "token valid & not expired"
        Disk-->>Main: "{ token, expiresAt }"
        Main->>Renderer: hydrate (token on disk)
    else no token / expired
        Main->>API: POST /api/auth/sign-in/email
        alt INVALID_EMAIL_OR_PASSWORD
            Main->>API: POST /api/auth/sign-up/email
            API-->>Main: 200 OK
            Main->>API: POST /api/auth/sign-in/email (retry)
        end
        API-->>Main: "{ token }"
        Main->>Disk: "saveToken({ token, expiresAt: now+30d })"
        Main->>Renderer: hydrate (new token)
    end
    Renderer->>API: GET /api/auth/get-session
    API-->>Renderer: "{ user: admin@local.test, activeOrganizationId }"
Loading
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
packages/auth/src/server.ts:89-93
**Email/password auth enabled globally in production**

`emailAndPassword: { enabled: true }` is unconditionally added to the shared `betterAuth` config, meaning `/api/auth/sign-in/email` and `/api/auth/sign-up/email` are active in every deployed environment. The `DevAuthForm` UI is gated to non-production builds, but the HTTP endpoints themselves are reachable by anyone who sends a direct `POST`. Without email verification configured, any attacker can register arbitrary accounts on the production instance. If this must stay globally enabled, consider adding `requireEmailVerification: true` or restricting sign-up behind an invite check.

### Issue 2 of 4
packages/db/src/client.ts:13
**`isNeon` regex tests the full URL string including credentials**

`/\.neon\.tech|neon\.build/.test(env.DATABASE_URL)` runs against the entire connection string. A password or database name that happens to contain `neon.build` (or `.neon.tech`) would match and route the connection through Neon's WebSocket/HTTP adapters, causing an immediate and confusing connection failure for a standard Postgres setup. Using `new URL()` to check only the hostname is safer.

```suggestion
const _dbHost = (() => {
	try {
		return new URL(env.DATABASE_URL).hostname;
	} catch {
		return env.DATABASE_URL;
	}
})();
const isNeon = /\.neon\.tech$|\.neon\.build$/.test(_dbHost);
```

### Issue 3 of 4
apps/desktop/src/main/lib/dev-auto-sign-in.ts:51-55
**Token skip logic ignores tokens that have no `expiresAt`**

If `loadToken()` returns a `token` but a falsy `expiresAt` (e.g. a legacy entry written before this field existed), the `&&` short-circuits and the function falls through to sign-in, overwriting the existing valid token on every boot. Skipping re-auth when the token exists but has no expiry avoids the churn.

```suggestion
	const stored = await loadToken();
	if (stored.token) {
		if (!stored.expiresAt) return; // no expiry info — assume valid
		const isExpired = new Date(stored.expiresAt) < new Date();
		if (!isExpired) return;
	}
```

### Issue 4 of 4
apps/desktop/src/main/lib/dev-auto-sign-in.ts:9
**Client-side `TOKEN_TTL_MS` may diverge from server session lifetime**

`TOKEN_TTL_MS = 30 days` is written as the local `expiresAt` by `ensureDevAuthToken`, but the Better Auth server likely issues tokens with its own expiry. If the server session expires earlier (e.g. default 7 days), the renderer will present a stale token that passes the local freshness check but fails all API calls until the next cold boot triggers re-auth.

Reviews (1): Last reviewed commit: "feat: local dev without third-party cred..." | Re-trigger Greptile

Comment thread packages/auth/src/server.ts Outdated
Comment thread packages/db/src/client.ts Outdated
Comment on lines +51 to +55
const stored = await loadToken();
if (stored.token && stored.expiresAt) {
const isExpired = new Date(stored.expiresAt) < new Date();
if (!isExpired) return;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Token skip logic ignores tokens that have no expiresAt

If loadToken() returns a token but a falsy expiresAt (e.g. a legacy entry written before this field existed), the && short-circuits and the function falls through to sign-in, overwriting the existing valid token on every boot. Skipping re-auth when the token exists but has no expiry avoids the churn.

Suggested change
const stored = await loadToken();
if (stored.token && stored.expiresAt) {
const isExpired = new Date(stored.expiresAt) < new Date();
if (!isExpired) return;
}
const stored = await loadToken();
if (stored.token) {
if (!stored.expiresAt) return; // no expiry info — assume valid
const isExpired = new Date(stored.expiresAt) < new Date();
if (!isExpired) return;
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/dev-auto-sign-in.ts
Line: 51-55

Comment:
**Token skip logic ignores tokens that have no `expiresAt`**

If `loadToken()` returns a `token` but a falsy `expiresAt` (e.g. a legacy entry written before this field existed), the `&&` short-circuits and the function falls through to sign-in, overwriting the existing valid token on every boot. Skipping re-auth when the token exists but has no expiry avoids the churn.

```suggestion
	const stored = await loadToken();
	if (stored.token) {
		if (!stored.expiresAt) return; // no expiry info — assume valid
		const isExpired = new Date(stored.expiresAt) < new Date();
		if (!isExpired) return;
	}
```

How can I resolve this? If you propose a fix, please make it concise.


const DEV_EMAIL = "admin@local.test";
const DEV_PASSWORD = "supersetdev";
const DEV_NAME = "Local Admin";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Client-side TOKEN_TTL_MS may diverge from server session lifetime

TOKEN_TTL_MS = 30 days is written as the local expiresAt by ensureDevAuthToken, but the Better Auth server likely issues tokens with its own expiry. If the server session expires earlier (e.g. default 7 days), the renderer will present a stale token that passes the local freshness check but fails all API calls until the next cold boot triggers re-auth.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/dev-auto-sign-in.ts
Line: 9

Comment:
**Client-side `TOKEN_TTL_MS` may diverge from server session lifetime**

`TOKEN_TTL_MS = 30 days` is written as the local `expiresAt` by `ensureDevAuthToken`, but the Better Auth server likely issues tokens with its own expiry. If the server session expires earlier (e.g. default 7 days), the renderer will present a stale token that passes the local freshness check but fails all API calls until the next cold boot triggers re-auth.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/trpc/src/router/support/support.ts (1)

153-158: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve TRPCError instead of remapping all failures to 500.

The catch block at lines 153-158 catches errors from resend.emails.send(), which is accessed through a Proxy that calls getResend(). Since getResend() (line 16) throws a TRPCError with code SERVICE_UNAVAILABLE when RESEND_API_KEY is not configured, this error gets caught and remapped to INTERNAL_SERVER_ERROR, causing clients to lose the important SERVICE_UNAVAILABLE signal.

Proposed fix
 			} catch (error) {
+				if (error instanceof TRPCError) {
+					throw error;
+				}
 				console.error("[support/sendMigrationReport] failed", error);
 				throw new TRPCError({
 					code: "INTERNAL_SERVER_ERROR",
 					message: "Failed to send migration report",
 				});
 			}
🤖 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/trpc/src/router/support/support.ts` around lines 153 - 158, The
catch block currently catches all errors from resend.emails.send() (which is
proxied via getResend()) and replaces them with a generic TRPCError { code:
"INTERNAL_SERVER_ERROR" }; change the catch to detect if the caught error is
already a TRPCError (instanceof TRPCError or error.code present and equals
SERVICE_UNAVAILABLE) and rethrow it as-is so SERVICE_UNAVAILABLE from
getResend() is preserved, otherwise wrap non-TRPC errors in a new TRPCError with
code "INTERNAL_SERVER_ERROR" and the original error logged; update the catch
around resend.emails.send() in support.ts accordingly so getResend()-thrown
TRPCError is not remapped.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/sign-in/page.tsx (1)

21-24: 💤 Low value

Consider removing redundant dev bypass check.

Lines 21-24 redirect to /workspace when env.SKIP_ENV_VALIDATION && session?.user, but lines 36-38 already redirect when session?.user is truthy. Since both conditions evaluate session?.user and navigate to the same route, the dev bypass adds no functional behavior—when a session exists, the redirect happens either at line 23 or line 37.

The only distinction is the explanatory comment. If the intent is to document that the main process handles auto-sign-in in dev mode, consider preserving that context in a comment above the unified check at lines 36-38 instead.

♻️ Simplified alternative
- // Dev bypass: AuthProvider handles auto-sign-in; if session lands, redirect
- if (env.SKIP_ENV_VALIDATION && session?.user) {
-   return <Navigate to="/workspace" replace />;
- }
-
  // Show loading while session is being fetched
  if (isPending) {
    return (
      <div className="flex h-screen w-screen items-center justify-center bg-background">
        <Spinner className="size-8" />
      </div>
    );
  }

- // If already signed in, redirect to workspace
+ // If already signed in, redirect to workspace
+ // In dev mode (SKIP_ENV_VALIDATION), main process handles auto-sign-in before window loads
  if (session?.user) {
    return <Navigate to="/workspace" replace />;
  }
🤖 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/desktop/src/renderer/routes/sign-in/page.tsx` around lines 21 - 24,
Remove the redundant dev-bypass conditional that checks env.SKIP_ENV_VALIDATION
&& session?.user and redirects with <Navigate to="/workspace" replace />;
instead keep a single redirect when session?.user is truthy (the existing
redirect using Navigate and "/workspace") and move the explanatory comment about
Dev auto-sign-in/AuthProvider above that unified check so the intent is
preserved; update or delete the duplicate block referencing
env.SKIP_ENV_VALIDATION, session, and Navigate to avoid duplicate redirects.
🤖 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/api/src/lib/analytics.ts`:
- Around line 21-32: The proxy get trap for posthog returns methods from the
PostHog instance unbound, causing loss of this when callers extract method
references; update the get trap in the posthog Proxy (the get handler that uses
Reflect.get(client, prop)) to pass the proxy (receiver) into Reflect.get or
explicitly bind function results to the client instance (use Reflect.get(client,
prop, receiver) or, if the result is a function, return result.bind(client));
ensure the same approach is applied when returning from disabled via
Reflect.get(disabled, prop) so methods remain correctly bound to their
instances.

In `@apps/desktop/src/main/lib/dev-auto-sign-in.ts`:
- Around line 22-36: postAuth can hang because fetch has no timeout; wrap the
fetch in an AbortController with a configurable timeout (e.g., 10s) so the
request is aborted if it stalls. In postAuth, create an AbortController, pass
its signal to fetch, start a setTimeout that calls controller.abort() after the
timeout, and clear that timer after fetch completes. Catch the abort error (and
other fetch errors) and return a consistent failure shape (e.g., { ok: false,
status: 0, data: AuthErrorBody }) so callers of postAuth can proceed during
startup. Use the function name postAuth and the existing fetch call as the
insertion point.

In `@docs/LOCAL_DEVELOPMENT.md`:
- Around line 41-45: Replace the inline shell substitution in the
BETTER_AUTH_SECRET example with a concrete sample value and add a separate
instruction showing how to generate a secret; specifically, change the `.env`
example line using BETTER_AUTH_SECRET so it contains a fixed example token
(e.g., BETTER_AUTH_SECRET=dev_secret_<hex>) and add a short note or one-line
command (openssl rand -hex 24) below explaining "Run this to generate a secret
and paste it into BETTER_AUTH_SECRET" so contributors won't expect `$(...)`
substitution to be evaluated.

In `@packages/auth/src/lib/resend.ts`:
- Around line 44-53: The current Resend client factory silently returns
console-fallback senders (via logConsoleSend) when env.RESEND_API_KEY is
missing, which masks failures in production; update the logic in the factory
that branches on prop ("emails" / "batch") so that if RESEND_API_KEY is not set
and NODE_ENV !== "development" (or a similar explicit production check) it
throws an Error indicating Resend is not configured (include the accessed
property name), and only allow the logConsoleSend fallback when running in
development (or when an explicit DEV flag is present); adjust the error message
and references to prop, emails, batch, and logConsoleSend accordingly.

In `@packages/auth/src/server.ts`:
- Around line 92-95: Gate the email/password credential provider by introducing
and checking a non-production/local flag before setting emailAndPassword.enabled
to true; specifically, instead of always enabling the emailAndPassword block in
server.ts, wrap or condition its enabled value on a new flag (e.g.,
ENABLE_EMAIL_PASSWORD_AUTH or isLocalDev) so that emailAndPassword.enabled is
true only when the flag indicates local/dev (or NODE_ENV !== 'production'), and
ensure emailAndPassword.autoSignIn follows the same guard; update any config
reading (process.env or config module) and the code paths that construct the
auth config (the emailAndPassword object) so production builds cannot enable
credentials by default.

In `@packages/db/src/client.ts`:
- Around line 27-43: The non-Neon branch currently constructs two separate
pg.Pool instances for db and dbWs, causing excess DB connections; refactor by
creating a single local pool (use pg.Pool and env.DATABASE_URL) when isNeon is
false, then pass that same pool into drizzleNodePg for both exports; keep the
isNeon ternary behavior for the Neon branch (NeonPool/drizzleNeonWs) but ensure
db and dbWs reuse the single nodePgPool via drizzleNodePg so only one pg.Pool
instance is created.

In `@packages/db/src/seed-dev.ts`:
- Around line 33-35: The seed-dev script builds apiUrl and endpoint from
NEXT_PUBLIC_API_URL and may post dev credentials to a remote host; add the same
host validation you use for DATABASE_URL to guard NEXT_PUBLIC_API_URL before
sending credentials: parse/normalize process.env.NEXT_PUBLIC_API_URL (the apiUrl
variable) and ensure the hostname is localhost, 127.0.0.1, or a configured safe
dev host (or reject/non-post if not); if invalid, throw or abort with a clear
message instead of constructing endpoint and POSTing to
`${apiUrl}/api/auth/sign-up/email`. Update the logic around apiUrl/endpoint in
seed-dev.ts to perform this check prior to any network call.

In `@packages/trpc/src/lib/analytics.ts`:
- Around line 23-34: The Proxy get handler for posthog currently returns raw
values from Reflect.get(disabled, prop) or Reflect.get(client, prop), which can
return unbound functions and break methods that rely on this; modify the handler
in the posthog Proxy so after obtaining the value via Reflect.get(target, prop)
you check if it's a function and, if so, return the function bound to its
concrete object (bind to disabled when using the disabled target, or bind to
client when using the real PostHog instance), otherwise return the value
unchanged; update the get branch that initializes client and the branch that
returns from disabled to perform this binding.

In `@plans/20260515-oss-local-setup.md`:
- Around line 1-4: This document titled "OSS Local Setup — What Shipped + What's
Left" is a shipped/retrospective record and should be moved out of the active
plans area into the archived plans folder; relocate the file to plans/done/,
remove the original from plans/, and update any internal references or links
that point to this document so they use the new location.
- Around line 91-107: The fenced architecture block currently lacks a Markdown
language tag; update the opening triple-backtick that precedes "Contributor's
machine" to use a language identifier (use `text`) so the block becomes ```text
and the closing triple-backtick remains unchanged; this fixes the MD040 lint
warning for the block shown in the diff.

In `@README.md`:
- Around line 99-100: Update the README short setup flow to include the missing
environment variable DATABASE_URL_UNPOOLED alongside DATABASE_URL and
BETTER_AUTH_SECRET so fresh clones don’t fail; specifically, edit the short path
snippet (the lines that show copying .env.example and the subsequent note) to
mention setting DATABASE_URL_UNPOOLED (e.g., "cp .env.example .env   # then edit
DATABASE_URL, DATABASE_URL_UNPOOLED + BETTER_AUTH_SECRET") and ensure the
example .env.example lists DATABASE_URL_UNPOOLED as a required minimum variable.

---

Outside diff comments:
In `@packages/trpc/src/router/support/support.ts`:
- Around line 153-158: The catch block currently catches all errors from
resend.emails.send() (which is proxied via getResend()) and replaces them with a
generic TRPCError { code: "INTERNAL_SERVER_ERROR" }; change the catch to detect
if the caught error is already a TRPCError (instanceof TRPCError or error.code
present and equals SERVICE_UNAVAILABLE) and rethrow it as-is so
SERVICE_UNAVAILABLE from getResend() is preserved, otherwise wrap non-TRPC
errors in a new TRPCError with code "INTERNAL_SERVER_ERROR" and the original
error logged; update the catch around resend.emails.send() in support.ts
accordingly so getResend()-thrown TRPCError is not remapped.

---

Nitpick comments:
In `@apps/desktop/src/renderer/routes/sign-in/page.tsx`:
- Around line 21-24: Remove the redundant dev-bypass conditional that checks
env.SKIP_ENV_VALIDATION && session?.user and redirects with <Navigate
to="/workspace" replace />; instead keep a single redirect when session?.user is
truthy (the existing redirect using Navigate and "/workspace") and move the
explanatory comment about Dev auto-sign-in/AuthProvider above that unified check
so the intent is preserved; update or delete the duplicate block referencing
env.SKIP_ENV_VALIDATION, session, and Navigate to avoid duplicate redirects.
🪄 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: 6fd15ec1-eee8-43fa-80b6-a4bc5910e94f

📥 Commits

Reviewing files that changed from the base of the PR and between 2b86c05 and 04130c0.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (37)
  • README.md
  • apps/api/src/lib/analytics.ts
  • apps/desktop/src/main/index.ts
  • apps/desktop/src/main/lib/dev-auto-sign-in.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspaceV2/OpenInWorkspaceV2.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopoverV2/RunInWorkspacePopoverV2.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunIssuesInWorkspacePopover/RunIssuesInWorkspacePopover.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/hooks/useAccessibleV2Workspaces/useAccessibleV2Workspaces.ts
  • apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/DevicePicker/hooks/useWorkspaceHostOptions/useWorkspaceHostOptions.ts
  • apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceModalContent/DashboardNewWorkspaceModalContent.tsx
  • apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/V1ImportModal.tsx
  • apps/desktop/src/renderer/routes/_authenticated/layout.tsx
  • apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/CollectionsProvider.tsx
  • apps/desktop/src/renderer/routes/_authenticated/providers/LocalHostServiceProvider/LocalHostServiceProvider.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/HostsSettingsSidebar.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/projects/$projectId/page.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/projects/components/ProjectsSettingsSidebar/ProjectsSettingsSidebar.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx
  • apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx
  • apps/desktop/src/renderer/routes/sign-in/page.tsx
  • apps/web/src/app/(auth)/components/DevAuthForm/DevAuthForm.tsx
  • apps/web/src/app/(auth)/components/DevAuthForm/index.ts
  • apps/web/src/app/(auth)/sign-in/[[...sign-in]]/page.tsx
  • apps/web/src/app/(auth)/sign-up/[[...sign-up]]/page.tsx
  • docs/LOCAL_DEVELOPMENT.md
  • package.json
  • packages/auth/src/lib/resend.ts
  • packages/auth/src/server.ts
  • packages/auth/src/stripe.ts
  • packages/db/package.json
  • packages/db/src/client.ts
  • packages/db/src/seed-dev.ts
  • packages/trpc/src/lib/analytics.ts
  • packages/trpc/src/router/support/support.ts
  • plans/20260515-oss-local-setup.md
  • turbo.jsonc

Comment thread apps/api/src/lib/analytics.ts
Comment thread apps/desktop/src/main/lib/dev-auto-sign-in.ts
Comment thread docs/LOCAL_DEVELOPMENT.md Outdated
Comment thread packages/auth/src/lib/resend.ts
Comment thread packages/auth/src/server.ts
Comment thread packages/db/src/seed-dev.ts
Comment thread packages/trpc/src/lib/analytics.ts
Comment thread plans/20260515-oss-local-setup.md Outdated
Comment thread plans/20260515-oss-local-setup.md
Comment thread README.md Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 issues found across 38 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/db/src/seed-dev.ts">

<violation number="1" location="packages/db/src/seed-dev.ts:25">
P2: The localhost allowlist misses the bracketed IPv6 hostname (`[::1]`), so valid local IPv6 DATABASE_URL values are rejected.</violation>

<violation number="2" location="packages/db/src/seed-dev.ts:34">
P1: Add a localhost guard for `NEXT_PUBLIC_API_URL`; currently the seed request can be sent to a remote API host.</violation>
</file>

<file name="packages/auth/src/lib/resend.ts">

<violation number="1" location="packages/auth/src/lib/resend.ts:9">
P1: `emails.send`/`batch.send` fallback is active in all environments, so missing `RESEND_API_KEY` can silently fake successful delivery instead of failing loudly in production.</violation>
</file>

<file name="packages/auth/src/server.ts">

<violation number="1" location="packages/auth/src/server.ts:92">
P1: `emailAndPassword` is enabled globally without a production sign-up guard, which opens unauthenticated sign-up in prod and can grant org membership via domain auto-enrollment before any email verification.</violation>
</file>

<file name="apps/api/src/lib/analytics.ts">

<violation number="1" location="apps/api/src/lib/analytics.ts:31">
P2: Proxy getter returns unbound PostHog methods, so method calls use the proxy as `this` instead of the PostHog instance.</violation>
</file>

<file name="packages/db/src/client.ts">

<violation number="1" location="packages/db/src/client.ts:28">
P2: The non-Neon path creates two independent `pg.Pool` instances for the same database URL, which doubles connection-pool overhead and can hit Postgres connection limits unnecessarily.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread packages/db/src/seed-dev.ts
Comment thread packages/auth/src/lib/resend.ts
Comment thread packages/auth/src/server.ts
Comment thread packages/db/src/seed-dev.ts Outdated
Comment thread apps/api/src/lib/analytics.ts Outdated
Comment thread packages/db/src/client.ts Outdated
Three profiles reconciled in one schema-aware design — OSS contributors
boot without keys, internal devs and prod keep their fail-fast workflow.

## Profile resolution (`packages/shared/src/deployment-profile.ts`)

Ranked by discriminator trust:

  cloud         VERCEL=1                       (set by Vercel; untypable)
  internal-dev  SUPERSET_INTERNAL_DEV=1        (written by .superset/setup.sh)
  self-hosted   NODE_ENV=production            (Docker / bare metal)
  oss-dev       default                        (fresh clone)

Strict (cloud / internal-dev / self-hosted) hard-fail boot on any missing
integration key — same fail-fast internal devs and prod rely on today.
Lenient (oss-dev) skips validation so the app boots; lazy guards at
call-sites turn missing keys into clean throws when features are exercised.

The `SUPERSET_INTERNAL_DEV=1` discriminator is positive-presence and
written only by `.superset/setup.sh` `step_write_env` — never in
.env.example, never in docs. A contributor can't trip it accidentally.

## Wired into each env schema

apps/api, apps/web, apps/admin, apps/marketing, apps/docs, apps/relay,
apps/desktop/src/main/env.main, packages/trpc — all compute:

  const skipValidation =
    !isStrictProfile(getDeploymentProfile()) ||
    !!process.env.SKIP_ENV_VALIDATION;

SKIP_ENV_VALIDATION=1 remains a build-time escape hatch (Docker preview
builds, CI), not the primary discriminator.

## Visibility

- `apps/api/src/lib/boot-summary.ts` — one-time API startup log listing
  disabled features and the env var to set:

    [superset] profile=oss-dev (lenient)
    [superset] disabled features (set the listed env var to enable):
               - stripe                       STRIPE_SECRET_KEY
               - resend (email)               RESEND_API_KEY
               ...

- `apps/api/src/app/api/health/route.ts` — `GET /api/health` returns
  `{ profile, integrations: { stripe: "configured" | "missing", ... } }`
  for prod monitoring + contributor sanity checks.

## Other changes

- `apps/desktop/src/main/lib/dev-auto-sign-in.ts` + main/index.ts CDP
  gate now check `getDeploymentProfile() === "oss-dev"` instead of
  `SKIP_ENV_VALIDATION`. OSS contributors no longer need to set any flag.
- `.superset/lib/setup/steps.sh step_write_env` writes
  SUPERSET_INTERNAL_DEV=1 so internal workspaces auto-opt into strict.
- Docs (LOCAL_DEVELOPMENT.md, plans/20260515-oss-local-setup.md) updated
  to drop the SKIP_ENV_VALIDATION requirement and document the profile
  model + boot summary + health endpoint.

## Verified end-to-end

oss-dev: `bun dev` (no flag) → boots cleanly, prints disabled features,
`/api/health` returns `"profile": "oss-dev"`, dev auto-sign-in fires,
host-service spawns, desktop renderer's activeHostUrl populates.

internal-dev: `SUPERSET_INTERNAL_DEV=1 bun run dev` (in apps/api) →
env validator rejects missing keys with "Invalid environment variables"
exactly like before — fail-fast preserved.
@Kitenite
Copy link
Copy Markdown
Collaborator Author

Follow-up commit: three-profile env validation (oss-dev / internal-dev / self-hosted / cloud).

OSS contributors no longer need SKIP_ENV_VALIDATION=1. The flag becomes a build-time escape hatch only.

cloud         VERCEL=1                       (set by Vercel; untypable)
internal-dev  SUPERSET_INTERNAL_DEV=1        (written by .superset/setup.sh — never in .env.example)
self-hosted   NODE_ENV=production            (Docker / bare metal)
oss-dev       default                        (fresh clone)

Strict profiles (cloud / internal-dev / self-hosted) hard-fail boot on any missing integration key — same fail-fast internal devs and prod rely on today. Lenient (oss-dev) skips validation; lazy guards at call-sites turn missing keys into clean throws when features are exercised.

The discriminator SUPERSET_INTERNAL_DEV=1 is positive-presence and written only by .superset/setup.sh — never in .env.example, never in docs. A contributor can't trip it accidentally.

Plus visibility nobody else in the OSS ref class ships:

  • Boot-time summary log listing disabled features and the env var to set
  • GET /api/health returning { profile, integrations: { stripe: "configured" | "missing", ... } } for prod monitoring + contributor sanity checks

Verified both paths:

  • bun dev (no flag): boots, /api/health says oss-dev, host-service spawns, desktop signs in automatically
  • SUPERSET_INTERNAL_DEV=1: env validator rejects missing keys with "Invalid environment variables" exactly like before

Docs updated. See docs/LOCAL_DEVELOPMENT.md for the contributor-facing version and plans/20260515-oss-local-setup.md for the design record.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

♻️ Duplicate comments (1)
apps/desktop/src/main/lib/dev-auto-sign-in.ts (1)

23-37: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add timeout/abort handling to startup auth requests.

postAuth can block boot indefinitely on a stalled network call because fetch has no timeout and this path is awaited during startup.

Suggested fix
+const DEV_AUTH_TIMEOUT_MS = 10_000;
+
 async function postAuth<T>(
 	path: string,
 	body: Record<string, unknown>,
 ): Promise<{ ok: boolean; status: number; data: T | AuthErrorBody }> {
-	const res = await fetch(`${mainEnv.NEXT_PUBLIC_API_URL}${path}`, {
-		method: "POST",
-		headers: {
-			"Content-Type": "application/json",
-			Origin: mainEnv.NEXT_PUBLIC_API_URL,
-		},
-		body: JSON.stringify(body),
-	});
-	const data = (await res.json().catch(() => ({}))) as T | AuthErrorBody;
-	return { ok: res.ok, status: res.status, data };
+	const controller = new AbortController();
+	const timeout = setTimeout(() => controller.abort(), DEV_AUTH_TIMEOUT_MS);
+	try {
+		const res = await fetch(`${mainEnv.NEXT_PUBLIC_API_URL}${path}`, {
+			method: "POST",
+			headers: {
+				"Content-Type": "application/json",
+				Origin: mainEnv.NEXT_PUBLIC_API_URL,
+			},
+			body: JSON.stringify(body),
+			signal: controller.signal,
+		});
+		const data = (await res.json().catch(() => ({}))) as T | AuthErrorBody;
+		return { ok: res.ok, status: res.status, data };
+	} catch (error) {
+		const message =
+			error instanceof Error ? error.message : "network request failed";
+		return {
+			ok: false,
+			status: 0,
+			data: { code: "NETWORK_ERROR", message },
+		};
+	} finally {
+		clearTimeout(timeout);
+	}
 }
🤖 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/desktop/src/main/lib/dev-auto-sign-in.ts` around lines 23 - 37, postAuth
can hang indefinitely because fetch has no timeout; modify postAuth to use an
AbortController (create one inside postAuth), pass controller.signal to fetch,
and set a timer (e.g. 5–10s) that calls controller.abort() on timeout, clearing
the timer after fetch completes; also catch the abort error and return a
sensible AuthErrorBody or status indicating timeout so startup doesn't
block—update references inside postAuth (the fetch call and surrounding
try/catch) to implement this abort/timeout behavior.
🧹 Nitpick comments (2)
apps/api/src/lib/boot-summary.ts (1)

12-28: ⚡ Quick win

Centralize integration metadata used by observability endpoints.

This list duplicates the catalog in apps/api/src/app/api/health/route.ts. Keeping two sources invites drift and inconsistent status output.

🤖 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/api/src/lib/boot-summary.ts` around lines 12 - 28, The INTEGRATIONS
array in boot-summary.ts is duplicated in the health route, causing drift;
extract this metadata into a single shared export (e.g., create an integrations
constant in a new module like integrations.ts or shared/integrations) and export
the array (and any types) so both boot-summary.ts and the health route import
that single source; update references to use the imported INTEGRATIONS (preserve
the tuple shape and names) and remove the duplicate list from the route to
ensure a single canonical source of truth.
apps/desktop/src/main/lib/dev-auto-sign-in.ts (1)

3-6: ⚡ Quick win

Use desktop path aliases instead of deep relative imports.

Switch this import to the configured alias form for consistency and easier maintenance.

Suggested fix
 import { env as mainEnv } from "main/env.main";
 import {
 	loadToken,
 	saveToken,
-} from "../../lib/trpc/routers/auth/utils/auth-functions";
+} from "lib/trpc/routers/auth/utils/auth-functions";
As per coding guidelines: "Use aliases as defined in `tsconfig.json` when possible".
🤖 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/desktop/src/main/lib/dev-auto-sign-in.ts` around lines 3 - 6, Replace
the deep relative import in dev-auto-sign-in.ts that brings in loadToken and
saveToken with the configured desktop path alias; update the import of loadToken
and saveToken (currently from
"../../lib/trpc/routers/auth/utils/auth-functions") to use the tsconfig alias
form (e.g., import from "desktop/lib/trpc/routers/auth/utils/auth-functions" or
the project's configured alias) so the module resolution uses the alias instead
of deep relative paths.
🤖 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 @.superset/lib/setup/steps.sh:
- Around line 443-447: The script currently unconditionally calls write_env_var
"SUPERSET_INTERNAL_DEV" "1", forcing strict internal-dev for all workspaces;
change this to only write SUPERSET_INTERNAL_DEV=1 when the user explicitly opts
in (e.g., an env var or flag is set) and otherwise leave it unset or set to "0"
so OSS local-dev defaults to lenient oss-dev. Update the block around
write_env_var "SUPERSET_INTERNAL_DEV" "1" to check an opt-in condition (for
example test a passed flag or an existing environment variable like
SUPERSET_INTERNAL_DEV_APPROVE or SUPSERSET_INTERNAL_DEV already being "1") and
only call write_env_var when that condition is true; if you need a default,
explicitly write "0" instead of forcing "1".

In `@apps/api/src/app/api/health/route.ts`:
- Around line 27-37: The public GET() health handler currently returns
getDeploymentProfile() and a map built from INTEGRATIONS exposing which env vars
are present; restrict this by removing or gating sensitive details: change GET()
to avoid returning profile and per-integration status in public responses (e.g.,
return only { ok: true } or a non-sensitive status), or require an
authenticated/internal flag to include getDeploymentProfile() and the
integrations map; update the GET() implementation and any callers to use this
gated behavior so NextResponse.json no longer leaks getDeploymentProfile() or
the INTEGRATIONS-derived mapping to unauthenticated requests.

In `@apps/api/src/env.ts`:
- Around line 13-14: The skipValidation flag currently uses truthy coercion
(!!process.env.SKIP_ENV_VALIDATION) which treats any non-empty string as true;
change this to an explicit comparison against the documented value by replacing
that part with process.env.SKIP_ENV_VALIDATION === "1" so skipValidation becomes
!isStrictProfile(profile) || process.env.SKIP_ENV_VALIDATION === "1". Update the
same pattern wherever similar env checks occur to use explicit === "1"
comparisons (referencing skipValidation, isStrictProfile, and
process.env.SKIP_ENV_VALIDATION to locate occurrences).

In `@apps/desktop/src/main/lib/dev-auto-sign-in.ts`:
- Around line 45-47: ensureDevAuthToken currently runs for the "oss-dev" profile
regardless of API target; restrict it so it only proceeds when
NEXT_PUBLIC_API_URL points to a local loopback address (e.g., host is
"localhost", "127.0.0.1", or "::1") — parse process.env.NEXT_PUBLIC_API_URL with
the URL constructor, check the hostname against those loopback values, and
return early otherwise; apply the same hostname check before any dev auto
sign-up/sign-in logic referenced in the block around ensureDevAuthToken and the
code covering lines ~56-67 so no predictable dev credentials are issued to
non-local API endpoints.

In `@apps/relay/src/env.ts`:
- Around line 12-13: The skipValidation flag currently treats any non-empty
SKIP_ENV_VALIDATION as true; update the expression in the skipValidation
assignment (and other similar env files) to explicitly compare
process.env.SKIP_ENV_VALIDATION to the documented sentinel (e.g., === '1') so
only SKIP_ENV_VALIDATION='1' disables validation; locate the skipValidation
variable and the isStrictProfile(profile) usage in apps/relay/src/env.ts (and
replicate the same explicit check in apps/web/src/env.ts, apps/docs/src/env.ts,
apps/marketing/src/env.ts, apps/api/src/env.ts, apps/admin/src/env.ts, etc.).

In `@docs/LOCAL_DEVELOPMENT.md`:
- Around line 114-121: The fenced code block that begins with "[superset]
profile=oss-dev (lenient)" lacks a language identifier; update the opening fence
from ``` to ```text so the block is marked as plaintext (e.g., change ``` to
```text for the block containing the "[superset] ..." lines) to satisfy the
MD040 rule.

In `@plans/20260515-oss-local-setup.md`:
- Around line 95-102: The fenced code block that begins with ``` (containing the
superset profile snippet) is missing a language identifier which triggers MD040;
update the opening fence to ```text so the block is explicitly marked as plain
text (e.g., change the opening ``` to ```text for the block containing
"[superset] profile=oss-dev (lenient)" and its disabled features list).

---

Duplicate comments:
In `@apps/desktop/src/main/lib/dev-auto-sign-in.ts`:
- Around line 23-37: postAuth can hang indefinitely because fetch has no
timeout; modify postAuth to use an AbortController (create one inside postAuth),
pass controller.signal to fetch, and set a timer (e.g. 5–10s) that calls
controller.abort() on timeout, clearing the timer after fetch completes; also
catch the abort error and return a sensible AuthErrorBody or status indicating
timeout so startup doesn't block—update references inside postAuth (the fetch
call and surrounding try/catch) to implement this abort/timeout behavior.

---

Nitpick comments:
In `@apps/api/src/lib/boot-summary.ts`:
- Around line 12-28: The INTEGRATIONS array in boot-summary.ts is duplicated in
the health route, causing drift; extract this metadata into a single shared
export (e.g., create an integrations constant in a new module like
integrations.ts or shared/integrations) and export the array (and any types) so
both boot-summary.ts and the health route import that single source; update
references to use the imported INTEGRATIONS (preserve the tuple shape and names)
and remove the duplicate list from the route to ensure a single canonical source
of truth.

In `@apps/desktop/src/main/lib/dev-auto-sign-in.ts`:
- Around line 3-6: Replace the deep relative import in dev-auto-sign-in.ts that
brings in loadToken and saveToken with the configured desktop path alias; update
the import of loadToken and saveToken (currently from
"../../lib/trpc/routers/auth/utils/auth-functions") to use the tsconfig alias
form (e.g., import from "desktop/lib/trpc/routers/auth/utils/auth-functions" or
the project's configured alias) so the module resolution uses the alias instead
of deep relative paths.
🪄 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: 58d8c20d-1f17-4b4a-b930-14eaeddf0c28

📥 Commits

Reviewing files that changed from the base of the PR and between 04130c0 and 2b609b5.

📒 Files selected for processing (19)
  • .superset/lib/setup/steps.sh
  • README.md
  • apps/admin/src/env.ts
  • apps/api/src/app/api/health/route.ts
  • apps/api/src/env.ts
  • apps/api/src/instrumentation.ts
  • apps/api/src/lib/boot-summary.ts
  • apps/desktop/src/main/env.main.ts
  • apps/desktop/src/main/index.ts
  • apps/desktop/src/main/lib/dev-auto-sign-in.ts
  • apps/docs/src/env.ts
  • apps/marketing/src/env.ts
  • apps/relay/src/env.ts
  • apps/web/src/env.ts
  • docs/LOCAL_DEVELOPMENT.md
  • packages/shared/package.json
  • packages/shared/src/deployment-profile.ts
  • packages/trpc/src/env.ts
  • plans/20260515-oss-local-setup.md
✅ Files skipped from review due to trivial changes (1)
  • README.md

Comment thread .superset/lib/setup/steps.sh Outdated
Comment thread apps/api/src/app/api/health/route.ts
Comment thread apps/api/src/env.ts Outdated
Comment thread apps/desktop/src/main/lib/dev-auto-sign-in.ts Outdated
Comment thread apps/relay/src/env.ts Outdated
Comment thread docs/LOCAL_DEVELOPMENT.md Outdated
Comment thread plans/20260515-oss-local-setup.md Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 19 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/app/api/health/route.ts">

<violation number="1" location="apps/api/src/app/api/health/route.ts:35">
P2: Avoid returning deployment profile and per-integration configuration details in the production health response; this leaks environment posture to unauthenticated callers.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread apps/api/src/app/api/health/route.ts
…S=1 opts into lenient

Internal devs and self-hosters keep their current fail-fast workflow with
zero setup changes. OSS contributors set SUPERSET_OSS=1 once to opt into
the lenient profile.

## Why

Previous design wrote SUPERSET_INTERNAL_DEV=1 via .superset/setup.sh so
internal devs would land in strict mode. That added a flag to internal
devs' workflow (a regression) and made the default lenient (a silent-
failure footgun for self-hosters who forgot to source their .env).

Strict-by-default is the conservative direction:
  - Internal devs: no shell-config or setup-script changes; same fail-
    fast as today on missing keys.
  - Self-hosted: defaults to strict — operators get a loud error if
    they miss something, not a silently-degraded app.
  - OSS contributors: opt in with one flag (`SUPERSET_OSS=1`) and get
    explicit "you're in lenient mode" feedback from the boot summary.

## Profile model (3 not 4)

  cloud      VERCEL=1            (auto, set by Vercel)         strict
  oss-dev    SUPERSET_OSS=1                                     lenient
  internal   default (covers internal team + self-host)         strict

## Changes

- packages/shared/src/deployment-profile.ts: trigger flipped to look
  for SUPERSET_OSS (positive opt-in for lenient) instead of
  SUPERSET_INTERNAL_DEV (positive opt-in for strict).
- .superset/lib/setup/steps.sh: no longer writes SUPERSET_INTERNAL_DEV.
  Internal-dev workspaces now default to strict via the absence of any
  flag — same as production.
- turbo.jsonc: added SUPERSET_OSS to globalPassThroughEnv.
- README, docs/LOCAL_DEVELOPMENT.md, plans/20260515-oss-local-setup.md:
  updated to show SUPERSET_OSS=1 as the lenient opt-in.
- Comment blocks in each env.ts schema rewritten to reflect the new
  semantics.

## Verified

- `bun run dev` in apps/api (no flag): ❌ Invalid environment variables
  → strict validation fires exactly like today's internal-dev workflow.
- `SUPERSET_OSS=1 bun run dev`: ✓ Ready + boot summary
  "[superset] profile=oss-dev (lenient)".
@Kitenite
Copy link
Copy Markdown
Collaborator Author

Flipped the discriminator: strict by default, OSS opt-in.

cloud      VERCEL=1            (auto, set by Vercel)         strict
oss-dev    SUPERSET_OSS=1                                     lenient
internal   default                                            strict (internal team + self-host)
  • Internal devs: no shell or setup-script changes; same fail-fast workflow as today.
  • Self-hosters: default to strict — loud error on missing keys, not a silently-degraded app.
  • OSS contributors: one flag (SUPERSET_OSS=1) + clear boot summary feedback that they're in lenient mode.

Quickstart updated to SUPERSET_OSS=1 bun dev. Internal devs' .superset/setup.sh no longer writes any profile flag.

Verified:

  • bun run dev (apps/api, no flag) → ❌ Invalid environment variables
  • SUPERSET_OSS=1 bun run dev → ✓ Ready + [superset] profile=oss-dev (lenient)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (4)
plans/20260515-oss-local-setup.md (2)

142-158: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to this fenced block.

The code fence at line 142 lacks a language tag. Use text for the ASCII architecture diagram.

📝 Suggested fix
-```
+```text
 Contributor's machine
 ├─ Docker
🤖 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 `@plans/20260515-oss-local-setup.md` around lines 142 - 158, The fenced ASCII
diagram block (the triple-backtick block that starts with "Contributor's
machine" and the tree diagram) is missing a language tag; update the opening
fence to ```text so the block becomes a text-fenced code block (i.e., change the
opening ``` to ```text) to correctly mark the ASCII architecture diagram as
plain text.

98-105: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to this fenced block.

The code fence at line 98 lacks a language tag. Use text to satisfy MD040 linting.

📝 Suggested fix
-```
+```text
 [superset] profile=oss-dev (lenient)
 [superset] disabled features (set the listed env var to enable):
🤖 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 `@plans/20260515-oss-local-setup.md` around lines 98 - 105, The fenced code
block containing the Superset snippet starting with "[superset] profile=oss-dev
(lenient)" needs a language identifier to satisfy MD040; update that opening
fence to use "```text" instead of "```" so the block is explicitly marked as
plain text (ensure the block that includes the lines "[superset] profile=oss-dev
(lenient)" and "[superset] disabled features (set the listed env var to
enable):" is modified).
docs/LOCAL_DEVELOPMENT.md (2)

44-44: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid shell substitution syntax inside .env value examples.

Line 44 will be copied literally by contributors; .env parsing won't execute $(...). Use a concrete placeholder value and provide a separate command to generate the secret.

📝 Suggested fix
-BETTER_AUTH_SECRET=dev_secret_$(openssl rand -hex 24)
+BETTER_AUTH_SECRET=dev_secret_replace_me_with_generated_value

Then add after line 45:

Generate a secret and paste it in:
```bash
openssl rand -hex 24
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @docs/LOCAL_DEVELOPMENT.md at line 44, Replace the example env value that
uses shell substitution for BETTER_AUTH_SECRET with a concrete placeholder (e.g.
BETTER_AUTH_SECRET=your_dev_secret_here) so contributors don't copy literal
$(...) syntax, and add a following instruction line telling users to generate a
secret with the command openssl rand -hex 24 and paste the output into
BETTER_AUTH_SECRET; update the LOCAL_DEVELOPMENT.md around the existing example
to include both the placeholder example and the separate one-line command to
generate the secret.


</details>

---

`115-122`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_

**Add a language identifier to this fenced block.**

The code fence at line 115 lacks a language tag. Use `text` to satisfy MD040 linting.





<details>
<summary>📝 Suggested fix</summary>

````diff
-```
+```text
 [superset] profile=oss-dev (lenient)
 [superset] disabled features (set the listed env var to enable):
            - stripe                       STRIPE_SECRET_KEY
🤖 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 `@docs/LOCAL_DEVELOPMENT.md` around lines 115 - 122, The fenced code block that
begins with ``` in the docs/LOCAL_DEVELOPMENT.md sample is missing a language
tag which triggers MD040; update that fence to ```text (i.e. change the opening
``` to ```text) so the block is treated as plain text and linting passes—no
other edits to the block contents are required.
🤖 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 `@docs/LOCAL_DEVELOPMENT.md`:
- Line 182: The deployment profile names in the docs are inconsistent: update
the list in LOCAL_DEVELOPMENT.md to match the actual profiles defined in
packages/shared/src/deployment-profile.ts (use the same identifiers used in that
module, e.g., cloud | internal | oss-dev), or alternatively update the profiles
table to include internal-dev and self-hosted if those are the intended names;
ensure the text referencing isStrictProfile() and any examples use the exact
profile identifiers from deployment-profile.ts so the docs and code are
consistent.

In `@plans/20260515-oss-local-setup.md`:
- Line 12: Replace the contributor-facing environment flag SKIP_ENV_VALIDATION
with the primary OSS mode flag SUPERSET_OSS in the dev startup instruction:
change the step that currently shows "SKIP_ENV_VALIDATION=1 bun dev" to
"SUPERSET_OSS=1 bun dev" so it matches the deployment profiles' OSS trigger
(SUPERSET_OSS) and docs/LOCAL_DEVELOPMENT.md and avoids using the build-time
escape hatch variable; update the single-line command and any adjacent
explanatory text if it references SKIP_ENV_VALIDATION.
- Line 148: Replace the environment flag used in the local dev command: change
the command string "SKIP_ENV_VALIDATION=1 bun dev" to use the primary OSS flag
"SUPERSET_OSS=1 bun dev" so it matches the deployment profiles and the
architecture diagram; update any nearby text that references
"SKIP_ENV_VALIDATION" to "SUPERSET_OSS" for consistency.

---

Duplicate comments:
In `@docs/LOCAL_DEVELOPMENT.md`:
- Line 44: Replace the example env value that uses shell substitution for
BETTER_AUTH_SECRET with a concrete placeholder (e.g.
BETTER_AUTH_SECRET=your_dev_secret_here) so contributors don't copy literal
$(...) syntax, and add a following instruction line telling users to generate a
secret with the command `openssl rand -hex 24` and paste the output into
BETTER_AUTH_SECRET; update the LOCAL_DEVELOPMENT.md around the existing example
to include both the placeholder example and the separate one-line command to
generate the secret.
- Around line 115-122: The fenced code block that begins with ``` in the
docs/LOCAL_DEVELOPMENT.md sample is missing a language tag which triggers MD040;
update that fence to ```text (i.e. change the opening ``` to ```text) so the
block is treated as plain text and linting passes—no other edits to the block
contents are required.

In `@plans/20260515-oss-local-setup.md`:
- Around line 142-158: The fenced ASCII diagram block (the triple-backtick block
that starts with "Contributor's machine" and the tree diagram) is missing a
language tag; update the opening fence to ```text so the block becomes a
text-fenced code block (i.e., change the opening ``` to ```text) to correctly
mark the ASCII architecture diagram as plain text.
- Around line 98-105: The fenced code block containing the Superset snippet
starting with "[superset] profile=oss-dev (lenient)" needs a language identifier
to satisfy MD040; update that opening fence to use "```text" instead of "```" so
the block is explicitly marked as plain text (ensure the block that includes the
lines "[superset] profile=oss-dev (lenient)" and "[superset] disabled features
(set the listed env var to enable):" is modified).
🪄 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: b295467f-2b5b-4de5-aae7-eab908fe64a6

📥 Commits

Reviewing files that changed from the base of the PR and between 2b609b5 and f3c76b9.

📒 Files selected for processing (14)
  • README.md
  • apps/admin/src/env.ts
  • apps/api/src/env.ts
  • apps/desktop/src/main/env.main.ts
  • apps/desktop/src/main/index.ts
  • apps/docs/src/env.ts
  • apps/marketing/src/env.ts
  • apps/relay/src/env.ts
  • apps/web/src/env.ts
  • docs/LOCAL_DEVELOPMENT.md
  • packages/shared/src/deployment-profile.ts
  • packages/trpc/src/env.ts
  • plans/20260515-oss-local-setup.md
  • turbo.jsonc
✅ Files skipped from review due to trivial changes (2)
  • turbo.jsonc
  • README.md
🚧 Files skipped from review as they are similar to previous changes (8)
  • apps/marketing/src/env.ts
  • packages/trpc/src/env.ts
  • apps/web/src/env.ts
  • apps/docs/src/env.ts
  • apps/admin/src/env.ts
  • apps/relay/src/env.ts
  • apps/desktop/src/main/env.main.ts
  • apps/desktop/src/main/index.ts

Comment thread docs/LOCAL_DEVELOPMENT.md Outdated
Comment thread plans/20260515-oss-local-setup.md Outdated
Comment thread plans/20260515-oss-local-setup.md Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 15 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/LOCAL_DEVELOPMENT.md">

<violation number="1" location="docs/LOCAL_DEVELOPMENT.md:99">
P3: Deployment profile docs are internally inconsistent: this new section uses `internal` as the strict default, but the architecture notes still describe the old `internal-dev | self-hosted` profile names.</violation>
</file>

<file name="turbo.jsonc">

<violation number="1" location="turbo.jsonc:30">
P2: `SUPERSET_OSS` is passed through but not hashed, so Turbo cache can be reused across strict/oss modes even though this flag changes runtime/build validation behavior. Add it to `globalEnv` (or task `env`) so cache keys reflect mode changes.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic

Comment thread turbo.jsonc Outdated
Comment thread docs/LOCAL_DEVELOPMENT.md
…tion

CI builds (`bun run lint`, `typecheck`, `test`) run in GitHub Actions
without integration keys — they'd hit the new internal-default strict
validation and fail. Add `ci` profile triggered by `CI=true` (set
automatically by every major CI runner) so those jobs run lenient.

Final 4-profile model, ranked by discriminator trust:

  cloud      VERCEL=1            (auto at Vercel runtime)        strict
  oss-dev    SUPERSET_OSS=1                                       lenient
  ci         CI=true             (auto in GitHub Actions etc.)    lenient
  internal   default                                              strict (team dev + self-host)

Lenient at build time, strict at runtime: `vercel build` pulls env
from the Vercel project so the build itself has all keys; the actual
runtime strictness still fires once VERCEL=1 is set on the deployed
serverless function. CI auto-degrades for lint/typecheck/test where
the keys aren't present and aren't needed.

Verified all 6 resolution cases (fresh clone, OSS, CI, Vercel, and
overrides between them) produce the right profile + strictness.
@Kitenite
Copy link
Copy Markdown
Collaborator Author

Good catch — added a ci profile so GitHub Actions builds don't trip the new internal-default strict validation.

Final 4-profile model:

cloud      VERCEL=1            (auto at Vercel runtime)        strict
oss-dev    SUPERSET_OSS=1                                       lenient
ci         CI=true             (auto in GitHub Actions etc.)    lenient
internal   default                                              strict (team dev + self-host)

Lenient at build time, strict at runtime: vercel build pulls env from the Vercel project so the build itself has all keys; runtime strictness still fires once VERCEL=1 is set on the deployed serverless function. CI auto-degrades for lint/typecheck/test where the keys aren't present and aren't needed.

Verified all 6 resolution cases (fresh clone, OSS, CI, Vercel, and overrides between them) produce the right profile + strictness.

Five real bugs/gaps from review:

#1 OSS sign-up no longer crashes on Stripe.
   afterCreateOrganization in packages/auth/src/server.ts now gates Stripe
   customer creation on env.STRIPE_SECRET_KEY presence. seedDefaultStatuses
   still runs unconditionally so org creation completes for OSS users.

#2 Email/password disabled in production.
   packages/auth/src/server.ts: emailAndPassword.enabled flips to
   `process.env.NODE_ENV !== "production"` to match the UI form gating.
   Prevents direct credential sign-up/sign-in via API in prod builds.

#3 Local Electric is now part of the OSS path, not a separate step.
   - docker-compose.dev.yml ships Postgres 16 + Electric 1.4.13 with
     wal_level=logical and host-port mappings (5433, 4649).
   - apps/electric-proxy/.dev.vars.example updated to match the
     docker-compose ports (4649) and default `bun dev` ports (3001).
   - apps/desktop/src/renderer/env.renderer.ts: hardcoded production
     defaults for NEXT_PUBLIC_API_URL, NEXT_PUBLIC_WEB_URL,
     NEXT_PUBLIC_ELECTRIC_URL, RELAY_URL now switch to localhost in
     dev builds — fresh-clone contributors no longer silently sync
     against hosted production Electric/API.
   - README + docs/LOCAL_DEVELOPMENT.md: replace bare `docker run` with
     `docker compose -f docker-compose.dev.yml up -d` and add the
     electric-proxy .dev.vars copy step.

#4 Dev auto-sign-in survives the race against API startup.
   apps/desktop/src/main/lib/dev-auto-sign-in.ts: poll /api/auth/ok
   with 1s interval and 60s timeout before attempting sign-in. main
   process calls it fire-and-forget (`void ensureDevAuthToken()`) so
   the window opens immediately; AuthProvider's onTokenChanged
   subscription re-hydrates when the token lands.

#5 Profile flags now hash into the Turbo cache key.
   turbo.jsonc: moved SUPERSET_OSS, CI, VERCEL, SKIP_ENV_VALIDATION
   from globalPassThroughEnv to globalEnv. Strict and lenient cached
   build outputs can no longer cross-contaminate.
@Kitenite
Copy link
Copy Markdown
Collaborator Author

Pushed 513d198 addressing all 5 review findings.

#1 OSS sign-up no longer crashes on StripeafterCreateOrganization gates customer creation on env.STRIPE_SECRET_KEY; seedDefaultStatuses still runs unconditionally so org creation completes.

#2 Email/password disabled in productionemailAndPassword.enabled: process.env.NODE_ENV !== 'production' matches the UI form gating. No direct credential endpoints in prod builds.

#3 Electric is part of the OSS path now, not a separate step.

  • New docker-compose.dev.yml ships Postgres 16 + Electric 1.4.13 with wal_level=logical and host-port mappings (5433, 4649).
  • apps/electric-proxy/.dev.vars.example aligned with the compose ports.
  • apps/desktop/src/renderer/env.renderer.ts: hardcoded production defaults for NEXT_PUBLIC_{API,WEB,ELECTRIC}_URL + RELAY_URL switch to localhost in dev builds — fresh-clone contributors no longer silently sync against hosted production.
  • README + LOCAL_DEVELOPMENT.md updated.

#4 Auto sign-in survives the race against API startupensureDevAuthToken now polls /api/auth/ok with 1s interval × 60s timeout before attempting sign-in. Main calls it fire-and-forget (void ensureDevAuthToken()) so the window opens immediately; AuthProvider's onTokenChanged subscription re-hydrates when the token lands.

#5 Profile flags hash into the Turbo cache — moved SUPERSET_OSS, CI, VERCEL, SKIP_ENV_VALIDATION from globalPassThroughEnv to globalEnv.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
docs/LOCAL_DEVELOPMENT.md (3)

117-124: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language tag to the fenced block.

Use text on this code fence to satisfy markdown linting and keep formatting consistent.

Suggested doc fix
-```
+```text
 [superset] profile=oss-dev (lenient)
 [superset] disabled features (set the listed env var to enable):
            - stripe                       STRIPE_SECRET_KEY
            - resend (email)               RESEND_API_KEY
            - posthog (telemetry)          NEXT_PUBLIC_POSTHOG_KEY
            ...
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @docs/LOCAL_DEVELOPMENT.md around lines 117 - 124, Update the fenced code
block showing the superset profile and disabled features to include a language
tag; change the opening fence from totext so the block containing
"[superset] profile=oss-dev (lenient)" and the list of disabled features
(stripe, resend, posthog, etc.) is tagged as text for proper markdown linting
and consistent formatting.


</details>

---

`40-44`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_

**Avoid shell substitution inside `.env` example values.**

Most contributors will paste this literally; `.env` parsers won’t execute `$(...)`. Use a static placeholder in the example and give the generation command separately.

  

<details>
<summary>Suggested doc fix</summary>

```diff
 DATABASE_URL=postgres://superset:superset@localhost:5433/superset
 DATABASE_URL_UNPOOLED=postgres://superset:superset@localhost:5433/superset
-BETTER_AUTH_SECRET=dev_secret_$(openssl rand -hex 24)
+BETTER_AUTH_SECRET=dev_secret_replace_me
```

```bash
# Generate and paste into BETTER_AUTH_SECRET
openssl rand -hex 24
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/LOCAL_DEVELOPMENT.md` around lines 40 - 44, Replace the inline shell
substitution in the example .env: change BETTER_AUTH_SECRET=dev_secret_$(openssl
rand -hex 24) to a static placeholder (e.g.,
BETTER_AUTH_SECRET=dev_secret_PLACEHOLDER) and add a separate code block showing
how to generate and paste a value (e.g., openssl rand -hex 24) so contributors
copy a valid .env with DATABASE_URL, DATABASE_URL_UNPOOLED, and
BETTER_AUTH_SECRET shown as examples and generation instructions are provided
separately.
```

</details>

---

`184-184`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_

**Fix deployment profile names in architecture notes.**

This line uses outdated identifiers and omits `ci`; align it with the profile table above.

  

<details>
<summary>Suggested doc fix</summary>

```diff
-- **Deployment profiles** — `packages/shared/src/deployment-profile.ts` resolves `cloud | internal-dev | self-hosted | oss-dev` from env flags. Strict profiles fail boot on missing keys; lenient (`oss-dev`) lets the app boot. Use `isStrictProfile()` from this module when you need to gate dev-only behavior.
+- **Deployment profiles** — `packages/shared/src/deployment-profile.ts` resolves `cloud | oss-dev | ci | internal` from env flags. Strict profiles fail boot on missing keys; lenient (`oss-dev`, `ci`) lets the app boot. Use `isStrictProfile()` from this module when you need to gate dev-only behavior.
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/LOCAL_DEVELOPMENT.md` at line 184, Update the docs line to list the
current deployment profile identifiers exactly as used in the codebase: "cloud |
internal-dev | self-hosted | oss-dev | ci", and clarify that strict profiles
will fail boot on missing keys while lenient profiles (oss-dev and ci) allow the
app to boot; reference the isStrictProfile() helper to gate dev-only behavior
where appropriate so readers know to use that function when gating features.
```

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

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 @docker-compose.dev.yml:

  • Around line 38-41: The docker compose uses host.docker.internal in the
    DATABASE_URL which isn't reliable on Linux; update the DATABASE_URL value so
    Electric connects to the postgres service via Compose DNS (use host "postgres"
    and the container's internal port 5432) instead of host.docker.internal:5433 so
    both containers can talk over the same compose network; leave ELECTRIC_SECRET
    unchanged.

Duplicate comments:
In @docs/LOCAL_DEVELOPMENT.md:

  • Around line 117-124: Update the fenced code block showing the superset profile
    and disabled features to include a language tag; change the opening fence from
    totext so the block containing "[superset] profile=oss-dev (lenient)"
    and the list of disabled features (stripe, resend, posthog, etc.) is tagged as
    text for proper markdown linting and consistent formatting.
  • Around line 40-44: Replace the inline shell substitution in the example .env:
    change BETTER_AUTH_SECRET=dev_secret_$(openssl rand -hex 24) to a static
    placeholder (e.g., BETTER_AUTH_SECRET=dev_secret_PLACEHOLDER) and add a separate
    code block showing how to generate and paste a value (e.g., openssl rand -hex
  1. so contributors copy a valid .env with DATABASE_URL, DATABASE_URL_UNPOOLED,
    and BETTER_AUTH_SECRET shown as examples and generation instructions are
    provided separately.
  • Line 184: Update the docs line to list the current deployment profile
    identifiers exactly as used in the codebase: "cloud | internal-dev | self-hosted
    | oss-dev | ci", and clarify that strict profiles will fail boot on missing keys
    while lenient profiles (oss-dev and ci) allow the app to boot; reference the
    isStrictProfile() helper to gate dev-only behavior where appropriate so readers
    know to use that function when gating features.

</details>

<details>
<summary>🪄 Autofix (Beta)</summary>

Fix all unresolved CodeRabbit comments on this PR:

- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: defaults

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `33403934-29d7-49c8-b961-0f9823efe20b`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 101cd30036572a599307c123ddb38392be7fd375 and 513d1989f3d87a450afbe56d54ef8bcb91f4eb12.

</details>

<details>
<summary>📒 Files selected for processing (9)</summary>

* `README.md`
* `apps/desktop/src/main/index.ts`
* `apps/desktop/src/main/lib/dev-auto-sign-in.ts`
* `apps/desktop/src/renderer/env.renderer.ts`
* `apps/electric-proxy/.dev.vars.example`
* `docker-compose.dev.yml`
* `docs/LOCAL_DEVELOPMENT.md`
* `packages/auth/src/server.ts`
* `turbo.jsonc`

</details>

<details>
<summary>✅ Files skipped from review due to trivial changes (2)</summary>

* apps/electric-proxy/.dev.vars.example
* README.md

</details>

<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary>

* apps/desktop/src/main/index.ts
* apps/desktop/src/main/lib/dev-auto-sign-in.ts

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Comment thread docker-compose.dev.yml Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 9 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docker-compose.dev.yml">

<violation number="1" location="docker-compose.dev.yml:40">
P2: Electric should connect to Postgres via the Compose service name (`postgres:5432`) instead of `host.docker.internal:5433` to avoid host-DNS/port-mapping fragility in local dev.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic

Comment thread docker-compose.dev.yml
environment:
# host.docker.internal lets the container reach Postgres on the
# host's 5433 port (mapped from the postgres service above).
DATABASE_URL: postgres://superset:superset@host.docker.internal:5433/superset
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Electric should connect to Postgres via the Compose service name (postgres:5432) instead of host.docker.internal:5433 to avoid host-DNS/port-mapping fragility in local dev.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docker-compose.dev.yml, line 40:

<comment>Electric should connect to Postgres via the Compose service name (`postgres:5432`) instead of `host.docker.internal:5433` to avoid host-DNS/port-mapping fragility in local dev.</comment>

<file context>
@@ -0,0 +1,47 @@
+    environment:
+      # host.docker.internal lets the container reach Postgres on the
+      # host's 5433 port (mapped from the postgres service above).
+      DATABASE_URL: postgres://superset:superset@host.docker.internal:5433/superset
+      ELECTRIC_SECRET: local_electric_dev_secret
+    ports:
</file context>
Suggested change
DATABASE_URL: postgres://superset:superset@host.docker.internal:5433/superset
DATABASE_URL: postgres://superset:superset@postgres:5432/superset

Tip: Review your code locally with the cubic CLI to iterate faster.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 16, 2026

🚀 Preview Deployment

🔗 Preview Links

Service Status Link
Neon Database (Neon) View Branch
Vercel API (Vercel) Open Preview
Vercel Web (Vercel) Open Preview
Vercel Marketing (Vercel) Open Preview
Vercel Admin (Vercel) Open Preview
Vercel Docs (Vercel) Open Preview

Preview updates automatically with new commits

## CI build failure (desktop build job)

electron.vite.config.ts does `await import("./src/main/env.main")` at
config-load time, which Node's ESM loader handles directly (no Vite
transform). When env.main.ts imported `@superset/shared/deployment-profile`
(a `.ts` file), Node refused with `ERR_UNKNOWN_FILE_EXTENSION`.

Fix: inline the 4-line profile check in env.main.ts. Other consumers
keep using the shared module (Next.js apps handle .ts via SWC).

## Remaining hardcoded prod URL defaults

The previous renderer-env fix only covered renderer/env.renderer.ts.
Three other build/runtime spots still defaulted to hosted production
URLs when env vars were absent:

- apps/desktop/vite/helpers.ts (htmlEnvTransformPlugin)
- apps/desktop/electron.vite.config.ts (define blocks for main +
  renderer process.env replacements)
- apps/desktop/src/main/env.main.ts (Zod schema .default())

All four now use NODE_ENV-conditional defaults. Added a `devOrProdUrl()`
helper in vite/helpers.ts to keep the pattern consistent.

A fresh-clone OSS contributor's app no longer silently syncs against
hosted production Electric / API / streams / relay when their .env
omits the URLs.

## Verified

- `bun run lint` clean
- `bun run --cwd apps/desktop typecheck` clean
- `bun turbo run build --filter=@superset/desktop` succeeds locally (the
  CI failure that prompted this commit)
@Kitenite
Copy link
Copy Markdown
Collaborator Author

Done with the should-do-before-merge bucket.

#1 Smoke-tested end-to-end with the latest changes:

  • Wiped state, brought up docker-compose.dev.yml, ran db:migrate against fresh Postgres
  • SUPERSET_OSS=1 bun dev: all services up, profile=oss-dev, boot summary listed disabled features, /api/health returned the right shape
  • Dev auto-sign-in succeeded after the new API-readiness poll
  • Host-service spawned for the auto-created org — and DB confirms stripe_customer_id IS NULL proving the Stripe gate fix works (would have crashed before)

#2 CI was failingBuild Desktop errored with ERR_UNKNOWN_FILE_EXTENSION for @superset/shared/deployment-profile.ts. Root cause: electron.vite.config.ts does await import('./src/main/env.main') at config-load time, which Node's ESM loader handles directly (no Vite transform). My new import pulled a .ts file from a sibling workspace package — Node refused. Fix: inlined the 4-line profile check in env.main.ts. Other consumers (Next.js apps) keep the shared module — SWC handles .ts fine.

#3 Audited the other env defaults — found three more spots defaulting to hosted production when env vars are absent:

  • apps/desktop/vite/helpers.ts (htmlEnvTransformPlugin)
  • apps/desktop/electron.vite.config.ts (main + renderer define blocks)
  • apps/desktop/src/main/env.main.ts (Zod .default())

All four now use NODE_ENV-conditional defaults. Added a devOrProdUrl() helper to keep the pattern consistent. Fresh-clone OSS sessions no longer silently sync against hosted prod.

bun turbo run build --filter=@superset/desktop succeeds locally now.

Rewrites plans/20260515-oss-local-setup.md as the authoritative
'what shipped + why' document. Sections:

- Goal + three-audience requirements
- Final 4-profile design (cloud/oss-dev/ci/internal)
- What's wired into each part of the stack, with file paths
- Decisions made (with rejected alternatives + rationale)
- Lessons learned (8 surprises encountered during build)
- Commit-by-commit history of what was built
- Verification protocol (smoke test outputs, profile resolution
  unit cases, CDP probe, local CI reproduction)
- Honest TODO of what's still deferred
- Architecture diagrams (profile resolution flow, OSS session
  lifecycle, strictness x visibility matrix)
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 40 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/auth/src/lib/rate-limit.ts">

<violation number="1" location="packages/auth/src/lib/rate-limit.ts:6">
P2: This change silently disables invitation rate limiting when KV credentials are missing. In production, that should fail closed (or throw) instead of allowing unlimited invitations.</violation>
</file>

<file name="packages/trpc/src/lib/integrations/sync/tasks.ts">

<violation number="1" location="packages/trpc/src/lib/integrations/sync/tasks.ts:37">
P2: Throwing on missing `qstash` inside `Promise.allSettled` creates rejected entries that current fire-and-forget callers ignore, so sync enqueue failures can be silently dropped.

(Based on your team's feedback about handling async failures explicitly and avoiding swallowed rejections.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/api/src/lib/integration-status.ts">

<violation number="1" location="apps/api/src/lib/integration-status.ts:15">
P3: Remove the orphan `freestyle` integration entry; it is not backed by any env schema or runtime feature and causes misleading “missing integration” output.</violation>

<violation number="2" location="apps/api/src/lib/integration-status.ts:20">
P2: `upstash-kv` configuration status is incomplete: checking only `KV_REST_API_URL` can misreport the integration as configured when the required token is missing.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic

Comment thread packages/auth/src/lib/rate-limit.ts
Comment thread packages/trpc/src/lib/integrations/sync/tasks.ts Outdated
Comment thread apps/api/src/lib/integration-status.ts Outdated
Comment thread apps/api/src/lib/integration-status.ts Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/setup-local.ts">

<violation number="1" location="scripts/setup-local.ts:85">
P2: `isLocalDatabaseUrl` rejects valid IPv6 localhost URLs because `URL.hostname` is `[::1]`, not `::1`.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic

Comment thread scripts/setup-local.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant