Skip to content

Conversation

fomalhautb
Copy link
Contributor

@fomalhautb fomalhautb commented Jul 22, 2025


Important

Fix webhook handling in tests by updating URLs, adding retry logic, and ensuring correct event handling.

  • Behavior:
    • Update webhook URL to http://localhost:12345/webhook in backend-helpers.ts and webhooks.test.ts.
    • Add retry logic in listWebhookAttempts() in backend-helpers.ts to handle slow responses.
    • Modify findWebhookAttempt() in backend-helpers.ts to use updated listWebhookAttempts().
  • Tests:
    • Update webhooks.test.ts to use local webhook URL for testing.
    • Add retry logic in team-memberships.test.ts to ensure webhook events are captured.
    • Ensure team_permission.created events are correctly handled in team-memberships.test.ts.

This description was created by Ellipsis for cfbff54. You can customize this summary. It will automatically update as commits are pushed.

Copy link

vercel bot commented Jul 22, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stack-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2025 8:47pm
stack-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2025 8:47pm
stack-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2025 8:47pm
stack-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2025 8:47pm

@fomalhautb fomalhautb changed the title Update webhook URLs in backend tests to use local server for testing Fix GitHub action webhook errors Jul 22, 2025
Copy link

recurseml bot commented Jul 22, 2025

😱 Found 2 issues. Time to roll up your sleeves! 😱

🗒️ View all ignored comments in this repo
  • The constraint 'TokenStoreType extends string' is too restrictive. It should likely be 'TokenStoreType extends string | object' to match the condition check in line 113 where TokenStoreType is checked against {}
  • Return type mismatch - the interface declares useUsers() returning ServerUser[] but the Team interface that this extends declares useUsers() returning TeamUser[]
  • There is a syntax error in the super constructor call due to the ellipsis operator used incorrectly. Objects aren't being merged correctly. This syntax usage can lead to runtime errors when trying to pass the merged object to 'super()'. Verify that the intended alterations to the object occur before or outside of the super() call if needed.
  • Throwing an error when no active span is found is too aggressive. The log function should gracefully fallback to console.log or another logging mechanism when there's no active span, since not all execution contexts will have an active span. This makes the code less resilient and could break functionality in non-traced environments.

📚 Relevant Docs

  • Function sets backendContext with a new configuration but doesn't pass 'defaultProjectKeys'. Since defaultProjectKeys is required in the type definition and cannot be updated (throws error if tried to set), this will cause a type error.
  • The schema is using array syntax for pick() which is incorrect for Yup schemas. The pick() method in Yup expects individual arguments, not an array. Should be changed to: emailConfigSchema.pick('type', 'host', 'port', 'username', 'sender_name', 'sender_email')

📚 Relevant Docs

  • Creating a refresh token with current timestamp as expiration means it expires immediately. Should set a future date for token expiration.
  • The 'tools' object is initialized as an empty object, even though 'tools' is presumably expected to contain tool definitions. This could cause the server capabilities to lack necessary tool configurations, thus potentially impacting functionalities that depend on certain tool setups.

📚 Relevant Docs

  • 'STACK_SECRET_SERVER_KEY' is potentially being included in every request header without checking its existence again here. Although it's checked during initialization, this could lead to security issues as it's exposed in all communications where the header is logged or captured.

📚 Relevant Docs

  • When adding 'use client' directive at the beginning, it doesn't check if file.text already contains the 'use client' directive. This could lead to duplicate 'use client' directives if the file already has one.

📚 Relevant Docs

Need help? Join our Discord for support!
https://discord.gg/qEjHQk64Z9

@fomalhautb fomalhautb marked this pull request as ready for review July 22, 2025 20:13
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 Summary

This PR refactors webhook testing infrastructure to improve CI/CD reliability by addressing GitHub Actions webhook errors. The changes focus on two key areas:

URL Endpoint Standardization: The webhook test URL has been updated from https://example.com to http://localhost:12345/webhook, aligning with the existing pattern used throughout the test suite. This change ensures webhook tests work consistently in CI environments where external domain access may be restricted.

Retry Logic Restructuring: The retry mechanism has been moved from findWebhookAttempt to listWebhookAttempts, creating a cleaner separation of concerns. The listWebhookAttempts function now handles the polling logic with up to 20 attempts and 500ms delays, while findWebhookAttempt focuses solely on filtering results. This architectural improvement makes the webhook testing more predictable and maintainable.

The changes integrate well with the existing e2e testing framework, particularly the niceBackendFetch utility and webhook testing helpers that are used across multiple test files in the /integrations/neon/ directory.

Confidence score: 4/5

• This PR is quite safe to merge with minimal risk of production issues
• The changes are well-architected improvements to testing infrastructure that don't affect production code
• The apps/e2e/tests/backend/backend-helpers.ts file should be reviewed to ensure the retry logic handles edge cases properly

1 file reviewed, no comments

Edit Code Review Bot Settings | Greptile

@fomalhautb fomalhautb merged commit 5f7d238 into dev Jul 22, 2025
15 of 17 checks passed
@fomalhautb fomalhautb deleted the fix-webhook branch July 22, 2025 20:58
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