Skip to content

Conversation

@jumski
Copy link
Contributor

@jumski jumski commented Oct 11, 2025

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Oct 11, 2025

🦋 Changeset detected

Latest commit: ef187d5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
pgflow Minor
@pgflow/edge-worker Minor
@pgflow/client Minor
@pgflow/core Minor
@pgflow/dsl Minor
@pgflow/example-flows Minor

Not sure what this means? Click here to learn what changesets are.

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

Copy link
Contributor Author

jumski commented Oct 11, 2025

@nx-cloud
Copy link

nx-cloud bot commented Oct 11, 2025

View your CI Pipeline Execution ↗ for commit ef187d5

Command Status Duration Result
nx affected -t verify-exports --base=origin/mai... ✅ Succeeded 3s View ↗
nx affected -t build --configuration=production... ✅ Succeeded 28s View ↗
nx affected -t lint typecheck test --parallel -... ✅ Succeeded 3m 41s View ↗
nx run edge-worker:e2e ✅ Succeeded 2m 55s View ↗
nx run cli:e2e ✅ Succeeded 6s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-27 09:38:01 UTC

@github-actions
Copy link
Contributor

🔍 Preview Deployment: Playground

Deployment successful!

🔗 Preview URL: https://pr-253--pgflow-demo.netlify.app

📝 Details:

  • Branch: feat-auto-compilation
  • Commit: b110c092ab1d460059993a00f02f0c5c66288c7e
  • View Logs

_Last updated: _

Comment on lines 108 to 38
const errorData = await response.json();
throw new Error(
`Flow '${flowSlug}' not found.\n\n` +
`${errorData.message || 'Did you add it to flows.ts?'}\n\n` +
`Fix:\n` +
`1. Add your flow to supabase/functions/pgflow/flows.ts\n` +
`2. Restart edge functions: supabase functions serve`
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The 404 error handling assumes all 404 responses mean the flow wasn't registered, but the ControlPlane server regex /^/flows/([a-zA-Z0-9_]+)$/ only accepts alphanumeric characters and underscores. If a user passes a slug with hyphens like my-flow, the route won't match and returns a generic 404 with "Route GET /flows/my-flow not found". This code will misleadingly tell users to add the flow to flows.ts when the real issue is invalid characters in the slug.

if (response.status === 404) {
  const errorData = await response.json();
  // Check if it's a route mismatch vs missing flow
  if (errorData.error === 'Not Found' && errorData.message.includes('Route')) {
    throw new Error(
      `Invalid flow slug: '${flowSlug}'\n\n` +
        `Flow slugs can only contain letters, numbers, and underscores.\n` +
        `Example: my_flow (not my-flow)`
    );
  }
  throw new Error(
    `Flow '${flowSlug}' not found.\n\n` +
      `${errorData.message || 'Did you add it to flows.ts?'}\n\n` +
      `Fix:\n` +
      `1. Add your flow to supabase/functions/pgflow/flows.ts\n` +
      `2. Restart edge functions: supabase functions serve`
  );
}
Suggested change
const errorData = await response.json();
throw new Error(
`Flow '${flowSlug}' not found.\n\n` +
`${errorData.message || 'Did you add it to flows.ts?'}\n\n` +
`Fix:\n` +
`1. Add your flow to supabase/functions/pgflow/flows.ts\n` +
`2. Restart edge functions: supabase functions serve`
);
}
const errorData = await response.json();
// Check if it's a route mismatch vs missing flow
if (errorData.error === 'Not Found' && errorData.message.includes('Route')) {
throw new Error(
`Invalid flow slug: '${flowSlug}'\n\n` +
`Flow slugs can only contain letters, numbers, and underscores.\n` +
`Example: my_flow (not my-flow)`
);
}
throw new Error(
`Flow '${flowSlug}' not found.\n\n` +
`${errorData.message || 'Did you add it to flows.ts?'}\n\n` +
`Fix:\n` +
`1. Add your flow to supabase/functions/pgflow/flows.ts\n` +
`2. Restart edge functions: supabase functions serve`
);
}

Spotted by Graphite Agent

Fix in Graphite


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

@jumski jumski force-pushed the feat-auto-compilation branch from dcd753f to 0d80511 Compare November 21, 2025 09:16
Comment on lines +45 to +46
find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.ts" -type f -exec sed -i 's/\.js"/\.ts"/g' {} +
find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.ts" -type f -exec sed -i "s/\.js'/\.ts'/g" {} +
Copy link
Contributor

Choose a reason for hiding this comment

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

The sed -i flag behaves differently on macOS vs Linux. On macOS, it requires a backup extension argument (e.g., sed -i ''), while on Linux it doesn't. This will cause the script to fail for developers on macOS.

# Fix for cross-platform compatibility:
find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.ts" -type f -exec sed -i.bak 's/\.js"/\.ts"/g' {} + && find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.bak" -delete
find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.ts" -type f -exec sed -i.bak "s/\.js'/\.ts'/g" {} + && find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.bak" -delete
Suggested change
find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.ts" -type f -exec sed -i 's/\.js"/\.ts"/g' {} +
find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.ts" -type f -exec sed -i "s/\.js'/\.ts'/g" {} +
find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.ts" -type f -exec sed -i.bak 's/\.js"/\.ts"/g' {} + && find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.bak" -delete
find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.ts" -type f -exec sed -i.bak "s/\.js'/\.ts'/g" {} + && find "$VENDOR_DIR/@pgflow/edge-worker" -name "*.bak" -delete

Spotted by Graphite Agent

Fix in Graphite


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

@jumski jumski force-pushed the feat-auto-compilation branch 6 times, most recently from d55d7e7 to e74ef8e Compare November 25, 2025 10:18
Comment on lines +80 to +88
'--control-plane-url <url>',
'Control plane URL',
'http://127.0.0.1:54321/functions/v1/pgflow'
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Port mismatch with CLI package configuration. The default control plane URL uses port 54321, but pkgs/cli/supabase/config.toml configures the API on port 54421 (line 10). This will cause compilation to fail when users run pgflow compile without the --control-plane-url flag in the CLI package's test environment.

.option(
  '--control-plane-url <url>',
  'Control plane URL',
  'http://127.0.0.1:54421/functions/v1/pgflow'  // Fix: use 54421
)

The E2E test at pkgs/cli/__tests__/e2e/compile.test.ts:139 already works around this by explicitly passing the correct URL, but the default will fail for users.

Suggested change
'--control-plane-url <url>',
'Control plane URL',
'http://127.0.0.1:54321/functions/v1/pgflow'
)
'--control-plane-url <url>',
'Control plane URL',
'http://127.0.0.1:54421/functions/v1/pgflow'
)

Spotted by Graphite Agent

Fix in Graphite


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

@jumski jumski force-pushed the feat-auto-compilation branch from e74ef8e to e8886cd Compare November 25, 2025 10:31
@jumski jumski force-pushed the feat-auto-compilation branch 5 times, most recently from f95d77b to 1ea4670 Compare November 27, 2025 05:24
Comment on lines +49 to +51
console.error(`[DEBUG] Fetch failed for URL: ${url}`);
console.error(`[DEBUG] Error message: ${error.message}`);
console.error(`[DEBUG] Error cause:`, (error as any).cause);
Copy link
Contributor

Choose a reason for hiding this comment

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

Debug logging is unconditionally enabled in production. These console.error statements will always print to stderr when fetch fails, creating noise in production logs.

// Remove debug logging or make it conditional:
if (error.message.includes('ECONNREFUSED') || error.message.includes('fetch failed')) {
  throw new Error(
    'Could not connect to ControlPlane.\n\n' +
    // ... rest of error message
  );
}
Suggested change
console.error(`[DEBUG] Fetch failed for URL: ${url}`);
console.error(`[DEBUG] Error message: ${error.message}`);
console.error(`[DEBUG] Error cause:`, (error as any).cause);
if (process.env.DEBUG) {
console.error(`[DEBUG] Fetch failed for URL: ${url}`);
console.error(`[DEBUG] Error message: ${error.message}`);
console.error(`[DEBUG] Error cause:`, (error as any).cause);
}

Spotted by Graphite Agent

Fix in Graphite


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

}

if (filesToCreate.some((f) => f.path === denoJsonPath)) {
fs.writeFileSync(denoJsonPath, DENO_JSON_TEMPLATE(getVersion()));
Copy link
Contributor

Choose a reason for hiding this comment

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

If getVersion() returns 'unknown', invalid package specifiers like npm:@pgflow/core@unknown will be written to deno.json, causing import failures when the edge function starts.

if (filesToCreate.some((f) => f.path === denoJsonPath)) {
  const version = getVersion();
  if (version === 'unknown') {
    throw new Error('Could not determine package version. Check package.json exists.');
  }
  fs.writeFileSync(denoJsonPath, DENO_JSON_TEMPLATE(version));
}
Suggested change
fs.writeFileSync(denoJsonPath, DENO_JSON_TEMPLATE(getVersion()));
const version = getVersion();
if (version === 'unknown') {
throw new Error('Could not determine package version. Check package.json exists.');
}
fs.writeFileSync(denoJsonPath, DENO_JSON_TEMPLATE(version));

Spotted by Graphite Agent

Fix in Graphite


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

@jumski jumski force-pushed the feat-auto-compilation branch 2 times, most recently from 77bca6f to c4e9c95 Compare November 27, 2025 07:12
@jumski jumski force-pushed the feat-auto-compilation branch 2 times, most recently from 251ac47 to b02f41d Compare November 27, 2025 08:34
@github-actions
Copy link
Contributor

🔍 Preview Deployment: Website

Deployment successful!

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

📝 Details:

  • Branch: feat-auto-compilation
  • Commit: c66aa1f2f75a53464c285dd730a97960942afc2d
  • View Logs

_Last updated: _

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 28, 2025

Merge activity

  • Nov 28, 11:44 PM UTC: jumski added this pull request to the Graphite merge queue.
  • Nov 28, 11:45 PM UTC: CI is running for this pull request on a draft pull request (#449) due to your merge queue CI optimization settings.
  • Nov 28, 11:53 PM UTC: Merged by the Graphite merge queue via draft PR: #449.

graphite-app bot pushed a commit that referenced this pull request Nov 28, 2025
@graphite-app graphite-app bot closed this Nov 28, 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