Skip to content

experiment: cdk oauth refactor with Cursor #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dbgold17
Copy link
Contributor

@dbgold17 dbgold17 commented Apr 10, 2025

[WIP] - putting up for a quick sanity check on direction / value of this change before I spend any more time on it.

This is an experiment to use Cursor to refactor the CDK OAuth module. On top of hopefully providing value in terms of code and test quality, this is a good learning experience for me working with Cursor.

Upon diving into cdk OAuth work I noticed lots of complexity with the implementation that made it difficult to make even a small change. Have to update the same thing in multiple places, hard to conceptually understand the branching logic flows, hard to get confidence that changes will not break something (even assuming unit tests are passing).

I think this warrants improvement for the following reasons:

  1. OAuth flows are well specified (there is a clear "right" way to do it and clear things that are / are not supported). Therefore, this code should be relatively stable going forward and worth solidifying the implementation.
  2. Bugs in this code can be very impactful. Not only is auth on the critical path for nearly all data syncs but also it handles sensitive data (like refresh tokens) so bad bugs can lead to security vulnerabilities

new implementation:

old implementation:

@dbgold17 dbgold17 changed the title use cursor to make a new version of the classes experiment: cdk oauth refactor with Cursor Apr 10, 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.

1 participant