Skip to content

feat: reset password #937

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

feat: reset password #937

wants to merge 4 commits into from
+375 −40

Conversation

Mkassabov
Copy link
Collaborator

No description provided.

@Mkassabov Mkassabov self-assigned this Sep 2, 2025
Mkassabov
Mkassabov commented Sep 2, 2025
View reviewed changes
alchemy/src/state/file-system-state-store.ts Outdated
Comment on lines 134 to 137
const rootStateDir = path.join(process.cwd(), ".alchemy");
const entries = await fs.promises.readdir(rootStateDir, {
withFileTypes: true,
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this feels inherintly flawed since we store other things there (logs, miniflare data, etc)

Copy link
Collaborator

Choose a reason for hiding this comment

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

That does look bad, yeah. State store is unfortunately legacy and really should be in its own folder under /.alchemy and not the root. Not sure how to achieve that for current users though.

@Mkassabov
Copy link
Collaborator Author

Honestly the more I think about it the more I think listStages is a mistake and this should only ever be allowed to run on 1 stage at a time.

@sam-goodwin
Copy link
Collaborator

Honestly the more I think about it the more I think listStages is a mistake and this should only ever be allowed to run on 1 stage at a time.

Oh yeah, why's that?

@Mkassabov
Copy link
Collaborator Author

Oh yeah, why's that?

  • our state stores don't really seem to be designed around that most of them seem kind of hacky to access the stage
  • there aren't a lot of usecases
    • lets say you have a project where every user gets their own stage; you can break somebody elses config
    • If you don't have a lot of stages (e.g. just production, staging, development) its not that hard to just run this 3 time
    • the only place I see this as being useful is per PR stages which don't last that long and aren't production so a key rotation there isn't really urgent (and can still be done with a script)
  • this is an inherently a dangerous task because once you rotate a key you can't do much without re configuring your env to use the new key; so it should be treated with some sensitivity

bulk potentially dangerous action that has limited usecases and isn't well supported by our underlying state stores just smells off

@Mkassabov Mkassabov force-pushed the mkassabov/reset-password branch 2 times, most recently from aafee5a to 892c4aa Compare September 4, 2025 07:52
@sam-goodwin sam-goodwin force-pushed the main branch 2 times, most recently from a3a4abd to 1eeb802 Compare September 29, 2025 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sam-goodwin sam-goodwin sam-goodwin left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@Mkassabov Mkassabov

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Mkassabov @sam-goodwin