Skip to content

fix(db): guard postgres init against invalid DATABASE_URL to prevent import-time crash#491

Merged
MightyPrytanis merged 2 commits intomainfrom
copilot/fix-ci-failures-job-66355029822
Mar 10, 2026
Merged

fix(db): guard postgres init against invalid DATABASE_URL to prevent import-time crash#491
MightyPrytanis merged 2 commits intomainfrom
copilot/fix-ci-failures-job-66355029822

Conversation

Copy link
Contributor

Copilot AI commented Mar 9, 2026

Cyrano/src/db.ts called postgres(dbUrl) whenever DATABASE_URL was truthy, with no URL validation. In CI, the secret value is malformed or whitespace-padded, causing postgres.js to throw TypeError: Invalid URL at import time — propagating through every module that imports db and crashing 15+ test files before any assertions run, dropping the quality gate pass rate below 85%.

Changes

  • Cyrano/src/db.ts
    • Added isValidPostgresUrl() — validates via new URL(), accepts only postgres: / postgresql: protocols before touching postgres()
    • .trim() on DATABASE_URL to handle whitespace from copy-paste or CI secret injection
    • Wrapped postgres() init in try/catch; falls back to null and logs error.message (not the URL)
    • Structured as explicit branches: missing → warn+null, invalid → warn+null, valid → guarded init
} else if (!isValidPostgresUrl(dbUrl)) {
  console.warn('[DB] WARNING: DATABASE_URL is not a valid postgres URL. Database operations will fail.');
} else {
  try {
    client = postgres(dbUrl, { ... });
  } catch (error) {
    console.warn('[DB] WARNING: Failed to initialize postgres client.', error instanceof Error ? error.message : 'Unknown error');
    client = null;
  }
}

Modules that don't use the DB will import and run cleanly. DB-dependent paths will fail at query time with a clear runtime error rather than a silent import-time crash.

Original prompt

Create a PR to fix CI failures in failing job 66355029822 for PR #472 in MightyPrytanis/codebase.

Context

  • CI job logs show many Vitest test files failing with TypeError: Invalid URL originating from postgres.js parsing process.env.DATABASE_URL.
  • Stack traces point to import-time crash chain:
    • postgres parseUrl -> Cyrano/src/db.ts:20:24
    • triggered by modules importing db:
      • Cyrano/src/services/hipaa-compliance.ts (imports db)
      • Cyrano/src/tools/arkiver-mcp-tools.ts (imports db)
      • Cyrano/src/routes/auth.ts (imports db)
      • and others.
  • This causes 15 test files to fail and then the Quality Gates job fails (pass rate 73.21% < 85%).
  • PR Fix CI/CD pipeline: Playwright version mismatch, browser install, and onboarding test failures #472 added DATABASE_URL from GitHub secrets into test steps in .github/workflows/ci.yml, making this import-time crash occur in CI.

Goal

  • Ensure that importing the DB module never throws, even if DATABASE_URL is missing or malformed or postgres client initialization fails.
  • Keep behavior consistent with comments in Cyrano/src/db.ts about not failing on import.

Required changes

  1. Update Cyrano/src/db.ts:

    • Normalize env var: const dbUrl = process.env.DATABASE_URL?.trim();
    • Validate via new URL(dbUrl) ensuring protocol is postgres: or postgresql:.
    • Wrap postgres client initialization postgres(dbUrl, options) in try/catch.
    • If invalid/missing/fails to init, set client = null and export db as null-ish (null as any is acceptable for now) so imports succeed.
    • Add warnings (no secret contents) when DB URL is missing/invalid/init fails.
  2. Optionally (only if needed for type safety): export a helper like isDbAvailable() or getDbOrThrow() and update any obvious call sites that assume db exists at runtime; but do not refactor broadly.

  3. Run/update tests as needed so CI no longer fails due to invalid URL import-time crash.

Non-goals

  • Do not require editing GitHub Actions workflow YAML.
  • Do not change secrets handling.
  • Do not introduce new infrastructure for DB in CI.

Acceptance criteria

  • Vitest tests no longer crash with TypeError: Invalid URL on module import.
  • CI jobs that previously failed due to the import-time crash should proceed; quality gate pass rate should improve correspondingly.
  • Code does not log secrets.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Please find a solution for failing job 66355029822. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.

Assistant: The failing job 66355029822 is ultimately failing because multiple test files crash at import-time with:

  • TypeError: Invalid URL
  • originating from postgres’s parseUrl(...)
  • called from Cyrano/src/db.ts:20
  • and triggered by modules that import db (e.g. Cyrano/src/services/hipaa-compliance.ts:7, Cyrano/src/tools/arkiver-mcp-tools.ts:17, Cyrano/src/routes/auth.ts:16).

That import-time crash causes 15 test files to fail, which then trips the Quality Gates step (pass rate 73.21% < 85%).

Root cause

In Cyrano/src/db.ts, the code calls postgres(dbUrl, ...) whenever process.env.DATABASE_URL is “truthy”:

const dbUrl = process.env.DATABASE_URL;
const client = dbUrl ? postgres(dbUrl, { ... }) : null as any;

In CI, DATABASE_URL is being provided (.github/workflows/ci.yml sets it from secrets.DATABASE_URL), but the value being passed into the runtime is not a valid URL for postgres (the logs show ERR_INVALID_URL with redacted input). As soon as any module imports db, postgres(dbUrl, ...) parses it and throws.

Fix (recommended): make DB initialization safe in test/CI when URL is invalid

Update Cyrano/src/db.ts to validate the URL (and/or catch initialization errors) and fall back to null instead of throwing during import. This preserves the “don’t fail on import” intent mentioned in comments.

Patch suggestion (minimal, robust)

// Cyrano/src/db.ts
import { drizzle } from 'drizzle-orm/postgres-js';
import postgres from 'postgres';
import * as schema from './schema.js';

function isValidDbUrl(value: string): boolean {
  try {
    // Accept common postgres schemes
    const u = new URL(value);
    return ['postgres:', 'postgresql:'].includes(u.protocol);
  } catch {
    return false;
  }
}

const dbUrl = process.env.DATABASE_URL;

let client: ReturnType<typeof postgres> | null = null;

if (!dbUrl) {
  console.warn('[DB] WARNING: DATABASE_URL not set. Database operations will fail.');
} else if (!isValidDbUrl(dbUrl)) {
  console.warn('[DB] WARNING: DATABASE_URL is not a valid postgres URL. Database operations will fail.');
} else {
  try {
    client = postgres(dbUrl, {
      max: 10,
      idle_timeout: 20,
      connect_timeout: 10,
      onnotice: () => {},
    });
  } catch (e) {
    console.warn('[DB] WARNING: Failed to initialize pos...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.

…r malformed

Co-authored-by: MightyPrytanis <219587333+MightyPrytanis@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix CI failures in job 66355029822 for PR #472 fix(db): guard postgres init against invalid DATABASE_URL to prevent import-time crash Mar 9, 2026
@MightyPrytanis MightyPrytanis marked this pull request as ready for review March 10, 2026 08:55
@MightyPrytanis MightyPrytanis self-requested a review as a code owner March 10, 2026 08:55
Copilot AI review requested due to automatic review settings March 10, 2026 08:55
@MightyPrytanis MightyPrytanis merged commit a3d282a into main Mar 10, 2026
14 of 15 checks passed
@MightyPrytanis MightyPrytanis deleted the copilot/fix-ci-failures-job-66355029822 branch March 10, 2026 08:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an import-time crash caused by postgres.js throwing TypeError: Invalid URL when DATABASE_URL is malformed or whitespace-padded in CI. The module now validates and sanitizes the URL before attempting to create a postgres client, falling back to null gracefully.

Changes:

  • Added isValidPostgresUrl() helper to validate URL protocol before passing it to postgres()
  • Trimmed DATABASE_URL to handle whitespace from CI secret injection
  • Wrapped postgres() initialization in try/catch with structured fallback branches

Comment on lines 27 to +29
console.warn('[DB] WARNING: DATABASE_URL not set. Database operations will fail.');
} else if (!isValidPostgresUrl(dbUrl)) {
console.warn('[DB] WARNING: DATABASE_URL is not a valid postgres URL. Database operations will fail.');
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

In the original code, the if (!dbUrl) block existed as part of the old ternary flow. Now with the new branching structure, this if block only logs a warning but does not explicitly assign client = null. While client is initialized as null on line 24 so the behavior is correct, the asymmetry is worth noting: the catch block on line 42 explicitly sets client = null, but neither the !dbUrl branch nor the !isValidPostgresUrl branch do. For clarity and defensive consistency, consider adding client = null; in both the !dbUrl and !isValidPostgresUrl branches.

Suggested change
console.warn('[DB] WARNING: DATABASE_URL not set. Database operations will fail.');
} else if (!isValidPostgresUrl(dbUrl)) {
console.warn('[DB] WARNING: DATABASE_URL is not a valid postgres URL. Database operations will fail.');
console.warn('[DB] WARNING: DATABASE_URL not set. Database operations will fail.');
client = null;
} else if (!isValidPostgresUrl(dbUrl)) {
console.warn('[DB] WARNING: DATABASE_URL is not a valid postgres URL. Database operations will fail.');
client = null;

Copilot uses AI. Check for mistakes.
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.

3 participants