Skip to content

add seed to fix e2e test#5085

Merged
chitalian merged 2 commits intomainfrom
ci/justin/fix-e2e-test
Oct 21, 2025
Merged

add seed to fix e2e test#5085
chitalian merged 2 commits intomainfrom
ci/justin/fix-e2e-test

Conversation

@chitalian
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Oct 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
helicone Ready Ready Preview Comment Oct 21, 2025 3:08am
helicone-bifrost Ready Ready Preview Comment Oct 21, 2025 3:08am
helicone-eu Ready Ready Preview Comment Oct 21, 2025 3:08am

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

@claude
Copy link
Contributor

claude bot commented Oct 21, 2025

Claude finished @chitalian's task —— View job


PR Review Analysis

Todo List:

  • Read and analyze changed files
  • Review security implications of seed data changes
  • Check code quality and best practices
  • Provide detailed feedback with confidence score
  • Post comprehensive review

Score: 9/10 - Well-implemented seed data changes for e2e testing with proper security practices

Analysis Summary

This PR adds encrypted provider key seed data to support e2e tests that use the AI Gateway worker. The implementation follows security best practices for handling sensitive test data in database seeds.

Security Review ✅

Excellent security practices:

  • Proper encryption handling: Uses pgsodium encryption with pre-encrypted test data
  • Trigger management: Correctly disables/enables the provider_keys_encrypt_secret_trigger when inserting pre-encrypted data
  • Test isolation: Uses dedicated test organization UUID (a75d76e3-02e7-4d02-8a2b-c65ed27c69b2) that matches the workflow configuration
  • Non-production keys: The encrypted provider key appears to be a test key, not production credentials

Code Quality Assessment ✅

Strengths:

  1. Consistent patterns: Follows existing seed data patterns in the file
  2. Proper UUIDs: Uses valid UUIDs for all foreign key relationships
  3. Complete data setup: Includes all necessary components (pgsodium.key, provider_keys record)
  4. Conflict handling: Uses ON CONFLICT DO NOTHING for idempotent seeds

Workflow Changes:

  • .github/workflows/e2e-test-suite.yml:56: The hardcoded organization ID in the workflow now matches the seed data organization, ensuring proper test isolation

Technical Implementation ✅

Database seed structure:

  • Lines 46-47: Adds encryption key to pgsodium.key table with proper metadata
  • Lines 48-83: Inserts encrypted provider key with all required fields
  • Lines 45 & 83: Proper trigger disable/enable pattern for pre-encrypted data

Suggestions Summary:

Minor improvements (not critical):

  1. Documentation: Consider adding a comment explaining the encryption key ID relationship between pgsodium.key and provider_keys.vault_key_id (currently NULL but related to key_id)
  2. Key rotation: For long-term maintenance, consider documenting the test key rotation process

No Critical Issues Found:

  • ✅ No security vulnerabilities
  • ✅ No performance concerns
  • ✅ No breaking changes
  • ✅ Follows established patterns
  • ✅ Proper error handling with conflict resolution

This PR successfully addresses the e2e test requirements by providing the necessary encrypted provider key data while maintaining security best practices. The implementation is clean, follows existing patterns, and properly handles the encryption/decryption workflow.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Summary

Added provider key seed data for the Admin organization (a75d76e3-02e7-4d02-8a2b-c65ed27c69b2) to support e2e tests that use the AI Gateway worker.

Key changes:

  • Added pgsodium.key entry for encryption key management
  • Added encrypted OpenAI provider key for Admin organization
  • Used trigger disable/enable pattern to insert pre-encrypted data
  • Ensures AI Gateway worker can retrieve provider keys during e2e tests

Confidence Score: 5/5

  • This PR is safe to merge with no identified risks
  • The changes add test seed data only, following established patterns for database seeding. The PR disables/enables the encryption trigger appropriately when inserting pre-encrypted test credentials, which is the correct approach for seed data. No production code is affected.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
supabase/seeds/0_seed.sql 5/5 Added provider key seed data for e2e tests; includes encrypted test credentials for Admin org using pgsodium encryption

Sequence Diagram

sequenceDiagram
    participant Test as E2E Test Suite
    participant Supabase as Supabase (PostgreSQL)
    participant Trigger as provider_keys_encrypt_secret_trigger
    participant Gateway as AI Gateway Worker
    participant Provider as OpenAI API

    Note over Test,Supabase: Database Initialization
    Test->>Supabase: Run seed.sql
    Supabase->>Supabase: INSERT INTO pgsodium.key
    Note over Supabase: Disable encryption trigger
    Supabase->>Supabase: ALTER TABLE DISABLE TRIGGER
    Supabase->>Supabase: INSERT provider_keys (encrypted data)
    Note over Supabase: Re-enable encryption trigger
    Supabase->>Supabase: ALTER TABLE ENABLE TRIGGER

    Note over Test,Provider: E2E Test Execution
    Test->>Gateway: POST /v1/chat/completions
    Note over Test: Org: a75d76e3-02e7-4d02-8a2b-c65ed27c69b2
    Gateway->>Supabase: SELECT FROM decrypted_provider_keys_v2
    Note over Gateway: Lookup provider key for org
    Supabase-->>Gateway: Decrypted OpenAI key
    Gateway->>Provider: Forward request with provider key
    Provider-->>Gateway: OpenAI response
    Gateway-->>Test: Response (200 OK)
    Gateway->>Supabase: Log request/response

Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

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