Skip to content

Conversation

@IMax153
Copy link
Collaborator

@IMax153 IMax153 commented Nov 21, 2025

  • Moves some models out of Auth.ts and into Model.ts
  • Uses Redacted for sensitive tokens
  • Removes need for admin URL interpolation
  • Uses the optional service pattern to handle token generation in the Admin service
    • Removes a lot of the indirection between Auth <-> Admin, so you can just provide Auth to Admin when you want / need authenticated requests

@IMax153 IMax153 requested review from cmwhited and fubhy November 21, 2025 16:58
export const layer = (url: string) =>
Layer.effect(Admin)(
Effect.gen(function*() {
const auth = yield* Effect.serviceOption(Auth.Auth)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cmwhited - FYI this is what I was referring to by the "optional service pattern"

@fubhy
Copy link
Contributor

fubhy commented Nov 21, 2025

Does this PR re-introduce the issue with the Node.js dependencies? Just making sure we don';t break browser usage again.

@fubhy
Copy link
Contributor

fubhy commented Nov 21, 2025

LocalCache.ts has a dependency on node:os

@CLAassistant
Copy link

CLAassistant commented Nov 21, 2025

CLA assistant check
All committers have signed the CLA.

@IMax153
Copy link
Collaborator Author

IMax153 commented Nov 22, 2025

I was able to run the studio without issue, but perhaps I can sync up with @cmwhited when there's time (maybe after you all are back) to understand if there's something I missed / a more thorough way I can test to ensure all works properly.

@leoyvens
Copy link
Collaborator

@IMax153 are you blocked here? What do you want to do with this PR?

@IMax153
Copy link
Collaborator Author

IMax153 commented Dec 18, 2025

@IMax153 are you blocked here? What do you want to do with this PR?

Hey @leoyvens - @cmwhited is going to do a secondary validation that this PR does not affect the functionality of Amp Studio that @fubhy mentioned above either today or tomorrow.

As long as everything is working properly, we should be good to merge this after that.

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.

6 participants