Skip to content

Conversation

@jumski
Copy link
Contributor

@jumski jumski commented Nov 12, 2025

Fix Supabase connectivity issues and improve test reliability

TL;DR

This PR addresses Supabase connectivity issues in CI by configuring Docker ulimits, improves test reliability with channel stabilization delays, and enhances article worker tests with proper mocking.

What changed?

  • Added Docker daemon configuration in GitHub Actions to allow containers to set high ulimits (prevents "ulimit: open files" errors in Supabase containers)
  • Improved client tests by adding stabilization delay for realtime channels to fully establish routing
  • Refactored article flow worker tests to use proper mocking instead of real HTTP requests
  • Updated Claude bash command patterns in settings.json to use proper syntax
  • Simplified client project.json by removing redundant db:ensure target and integrating database setup directly into test commands
  • Added test-stability.sh script to verify channel stabilization reliability
  • Added separate vitest config for type checking that doesn't need global setup

How to test?

  1. Run client tests with nx test client to verify they pass consistently
  2. Test article worker with cd apps/demo/supabase/functions/article_flow_worker && deno test
  3. Run the stability test script: cd pkgs/client && ./test-stability.sh 10
  4. Verify CI workflow completes successfully

Why make this change?

The changes address several reliability issues:

  1. Supabase containers were failing in CI due to ulimit restrictions
  2. Client tests were flaky due to race conditions in realtime channel subscriptions
  3. Article worker tests were making real HTTP requests, making them brittle and slow
  4. The test setup was unnecessarily complex with separate database preparation steps

These improvements make the tests more reliable, faster, and less dependent on external services, resulting in more consistent CI runs and developer experience.

@changeset-bot
Copy link

changeset-bot bot commented Nov 12, 2025

⚠️ No Changeset found

Latest commit: 96bf56f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor Author

jumski commented Nov 12, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge:queue - adds this PR to the back of the merge queue
  • hotfix:queue - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@jumski jumski marked this pull request as ready for review November 12, 2025 23:14
@jumski jumski force-pushed the 11-11-chore_failfast_client_tests_if_connectivity_issues branch 3 times, most recently from 44cd342 to 45cddc4 Compare November 13, 2025 23:42
@nx-cloud
Copy link

nx-cloud bot commented Nov 13, 2025

View your CI Pipeline Execution ↗ for commit 96bf56f

Command Status Duration Result
nx affected -t verify-exports --base=0adc276a6b... ✅ Succeeded 3s View ↗
nx affected -t build --configuration=production... ✅ Succeeded 30s View ↗
nx affected -t lint typecheck test --parallel -... ✅ Succeeded 7m View ↗
nx affected -t test:e2e --parallel --base=0adc2... ✅ Succeeded 4m 24s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-15 12:07:35 UTC

Comment on lines 45 to 124
} finally {
await supabase.removeChannel(channel);
await sql.end();
Copy link
Contributor

Choose a reason for hiding this comment

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

Resource leak: If supabase.removeChannel(channel) throws an error, sql.end() will never be called, leaving the PostgreSQL connection open.

This will cause connection pool exhaustion over time. The fix is to handle cleanup independently:

finally {
  await Promise.allSettled([
    supabase.removeChannel(channel),
    sql.end()
  ]);
}

This ensures both cleanup operations are attempted regardless of whether either fails.

Suggested change
} finally {
await supabase.removeChannel(channel);
await sql.end();
} finally {
await Promise.allSettled([
supabase.removeChannel(channel),
sql.end()
]);

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@jumski jumski force-pushed the 11-11-chore_failfast_client_tests_if_connectivity_issues branch 2 times, most recently from 0bb3550 to 7283c3a Compare November 14, 2025 00:12
@jumski jumski force-pushed the 11-11-chore_failfast_client_tests_if_connectivity_issues branch 3 times, most recently from 0a0445c to b23f159 Compare November 14, 2025 11:24
@jumski jumski force-pushed the 11-11-chore_failfast_client_tests_if_connectivity_issues branch 5 times, most recently from 03c2baf to 044c088 Compare November 14, 2025 14:40
@jumski jumski changed the title chore: failfast client tests if connectivity issues chore: sort out lot of flaky CI issues Nov 14, 2025
@jumski jumski force-pushed the 11-11-chore_failfast_client_tests_if_connectivity_issues branch from 044c088 to a920aa6 Compare November 14, 2025 15:42
@jumski jumski force-pushed the 11-11-chore_failfast_client_tests_if_connectivity_issues branch from a920aa6 to 96bf56f Compare November 14, 2025 15:57
@github-actions
Copy link
Contributor

🔍 Preview Deployment: Website

Deployment successful!

🔗 Preview URL: https://pr-366.pgflow.pages.dev

📝 Details:

  • Branch: 11-11-chore_failfast_client_tests_if_connectivity_issues
  • Commit: fdca0c46ebe7a5b93a91357fd06d8a6532fdf492
  • View Logs

_Last updated: _

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 16, 2025

Merge activity

  • Nov 16, 9:23 PM UTC: jumski added this pull request to the Graphite merge queue.
  • Nov 16, 9:30 PM UTC: CI is running for this pull request on a draft pull request (#370) due to your merge queue CI optimization settings.
  • Nov 16, 9:30 PM UTC: Merged by the Graphite merge queue via draft PR: #370.

@graphite-app graphite-app bot changed the base branch from 11-11-feat_fix_client_subscription_ci_issues to graphite-base/366 November 16, 2025 21:27
graphite-app bot pushed a commit that referenced this pull request Nov 16, 2025
# Fix Supabase connectivity issues and improve test reliability

### TL;DR

This PR addresses Supabase connectivity issues in CI by configuring Docker ulimits, improves test reliability with channel stabilization delays, and enhances article worker tests with proper mocking.

### What changed?

- Added Docker daemon configuration in GitHub Actions to allow containers to set high ulimits (prevents "ulimit: open files" errors in Supabase containers)
- Improved client tests by adding stabilization delay for realtime channels to fully establish routing
- Refactored article flow worker tests to use proper mocking instead of real HTTP requests
- Updated Claude bash command patterns in settings.json to use proper syntax
- Simplified client project.json by removing redundant db:ensure target and integrating database setup directly into test commands
- Added test-stability.sh script to verify channel stabilization reliability
- Added separate vitest config for type checking that doesn't need global setup

### How to test?

1. Run client tests with `nx test client` to verify they pass consistently
2. Test article worker with `cd apps/demo/supabase/functions/article_flow_worker && deno test`
3. Run the stability test script: `cd pkgs/client && ./test-stability.sh 10`
4. Verify CI workflow completes successfully

### Why make this change?

The changes address several reliability issues:
1. Supabase containers were failing in CI due to ulimit restrictions
2. Client tests were flaky due to race conditions in realtime channel subscriptions
3. Article worker tests were making real HTTP requests, making them brittle and slow
4. The test setup was unnecessarily complex with separate database preparation steps

These improvements make the tests more reliable, faster, and less dependent on external services, resulting in more consistent CI runs and developer experience.
@graphite-app graphite-app bot closed this Nov 16, 2025
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