Skip to content

ENG-1395 Official deployment version cannot access database#751

Merged
maparent merged 3 commits intomainfrom
eng-1395-official-deployment-version-cannot-access-database
Feb 6, 2026
Merged

ENG-1395 Official deployment version cannot access database#751
maparent merged 3 commits intomainfrom
eng-1395-official-deployment-version-cannot-access-database

Conversation

@maparent
Copy link
Collaborator

@maparent maparent commented Feb 6, 2026

@linear
Copy link

linear bot commented Feb 6, 2026

@supabase
Copy link

supabase bot commented Feb 6, 2026

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

@maparent
Copy link
Collaborator Author

maparent commented Feb 6, 2026

Devin spotted a route error, and complains I do not return in the throw clause. I think that's a mistake; anyone calling this from a dev environment should fall through. It also will work if the variables are set in the environment otherwise.

@maparent maparent requested a review from mdroidian February 6, 2026 02:03
Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

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

We need to throw if we don't have the vars. Also, some lint errors.

@maparent maparent requested a review from mdroidian February 6, 2026 14:10
@maparent maparent force-pushed the eng-1395-official-deployment-version-cannot-access-database branch from 939c53d to 37f7362 Compare February 6, 2026 14:15
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 8 additional findings in Devin Review.

Open in Devin Review

Comment on lines +12 to +16
return NextResponse.json(
// eslint-disable-next-line @typescript-eslint/naming-convention
{ SUPABASE_URL, SUPABASE_ANON_KEY },
{ status: 200 },
);

Choose a reason for hiding this comment

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

🟡 Missing CORS headers on GET response while OPTIONS handler has them

The new /api/supabase/env endpoint has inconsistent CORS handling. The OPTIONS handler uses defaultOptionsHandler which applies CORS headers, but the GET handler returns NextResponse.json() directly without applying CORS via the cors() function.

Root Cause

Other API endpoints in the codebase use createApiResponse() which internally calls cors(request, response) to add CORS headers (see apps/website/app/utils/supabase/apiUtils.ts:48). However, this new endpoint bypasses that pattern.

If this endpoint is ever called from a browser context (e.g., from the Roam extension at runtime), the browser would:

  1. Send OPTIONS preflight → receives CORS headers (allowed)
  2. Send actual GET request → receives response WITHOUT CORS headers
  3. Browser rejects the response due to missing Access-Control-Allow-Origin

Currently this doesn't affect the build process since curl is used (packages/database/scripts/createEnv.mts:113), but it makes the API inconsistent and would break any future browser-based usage.

Suggested change
return NextResponse.json(
// eslint-disable-next-line @typescript-eslint/naming-convention
{ SUPABASE_URL, SUPABASE_ANON_KEY },
{ status: 200 },
);
const response = NextResponse.json(
// eslint-disable-next-line @typescript-eslint/naming-convention
{ SUPABASE_URL, SUPABASE_ANON_KEY },
{ status: 200 },
);
return cors(request, response) as NextResponse;
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on lines +12 to +16
return NextResponse.json(
// eslint-disable-next-line @typescript-eslint/naming-convention
{ SUPABASE_URL, SUPABASE_ANON_KEY },
{ status: 200 },
);
Copy link
Contributor

Choose a reason for hiding this comment

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

@maparent maparent force-pushed the eng-1395-official-deployment-version-cannot-access-database branch from 37f7362 to a59b352 Compare February 6, 2026 23:00
@maparent maparent merged commit f26a877 into main Feb 6, 2026
5 checks passed
@maparent maparent deleted the eng-1395-official-deployment-version-cannot-access-database branch February 6, 2026 23:03
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 9 additional findings in Devin Review.

Open in Devin Review

Comment on lines +115 to +118
writeFileSync(
join(projectRoot, ".env"),
Object.entries(asJson).map(([k,v])=>`${k}=${v}`).join('\n')
);

Choose a reason for hiding this comment

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

🔴 Environment file written to wrong path - .env instead of .env.${variant} format

When ROAM_BUILD_SCRIPT=1 is set, createEnv.mts writes environment variables to packages/database/.env, but dbDotEnv.mjs:envFilePath() only reads files matching .env.${variant} pattern (like .env.local, .env.branch, .env.production).

Root Cause

The flow when ROAM_BUILD_SCRIPT=1:

  1. createEnv.mts:115-118 writes to join(projectRoot, ".env") - a plain .env file
  2. Later, compile.ts:153 calls envContents() from dbDotEnv.mjs
  3. dbDotEnv.mjs:64-68 in envFilePath() looks for .env.${variant} files only:
    const name = join(findRoot(), `.env.${variant}`);
    return existsSync(name) ? name : null;
  4. Since there's no .env.${variant} file and process.env doesn't have the variables, envContents() returns an empty object

Impact: The Roam build will not receive the SUPABASE_URL and SUPABASE_ANON_KEY values, causing the database connection to fail in the built extension. The fix should either write to .env.production (matching an existing variant) or add logic to read plain .env files.

Suggested change
writeFileSync(
join(projectRoot, ".env"),
Object.entries(asJson).map(([k,v])=>`${k}=${v}`).join('\n')
);
writeFileSync(
join(projectRoot, ".env.production"),
Object.entries(asJson).map(([k,v])=>`${k}=${v}`).join('\n')
);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the base dotenv reads the content of .env in all cases.

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.

2 participants