fix(web): name the Google and GitHub login flags for what they do - #1556
Open
rajarshidattapy wants to merge 3 commits into
Open
fix(web): name the Google and GitHub login flags for what they do#1556rajarshidattapy wants to merge 3 commits into
rajarshidattapy wants to merge 3 commits into
Conversation
NEXT_PUBLIC_GOOGLE_AUTH_ENABLED and NEXT_PUBLIC_GITHUB_AUTH_ENABLED were read negated, so setting either one to "true" hid the button it appeared to enable. To show the buttons you had to leave the *_ENABLED variable unset. Neither was in .env.example, so nothing contradicted the name. Rename both to *_AUTH_DISABLED. The negation stays, so the gate now reads as it behaves -- "show on cloud, or when not disabled" -- and no deployment changes behaviour: Google and GitHub stay on by default when self-hosting, exactly as before. Document all three social flags in .env.example, including that AgentID is opt-in while these two are opt-out.
| ) : null} | ||
| {process.env.NEXT_PUBLIC_HOST_ID === "supermemory" || | ||
| !process.env.NEXT_PUBLIC_GITHUB_AUTH_ENABLED ? ( | ||
| !process.env.NEXT_PUBLIC_GITHUB_AUTH_DISABLED ? ( |
There was a problem hiding this comment.
Environment variable boolean handling bug. Setting NEXT_PUBLIC_GITHUB_AUTH_DISABLED=false will hide the button instead of showing it, because environment variables are strings and !"false" evaluates to false.
Fix by checking for explicit string values:
process.env.NEXT_PUBLIC_HOST_ID === "supermemory" ||
process.env.NEXT_PUBLIC_GITHUB_AUTH_DISABLED !== "true"Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1543.
Why?
NEXT_PUBLIC_GOOGLE_AUTH_ENABLEDandNEXT_PUBLIC_GITHUB_AUTH_ENABLEDwere read negated:So a self-hoster who read the variable name and set
NEXT_PUBLIC_GOOGLE_AUTH_ENABLED=truegot the opposite of what they asked for: the Google button disappeared. To show it you had to leave the*_ENABLEDvariable unset.Neither variable appeared in
.env.exampleor anywhere else in the repo, so nothing contradicted the name. The AgentID flag added in #1467 reads correctly (opt-in), which made the same*_AUTH_ENABLEDsuffix mean opt-in for one provider and opt-out for two others on the same screen.What?
Rename both to
*_AUTH_DISABLED. The negation stays, so the gate now reads as it behaves — show on cloud, or when not disabled:Plus all three social flags documented in
.env.example, including the fact that AgentID is opt-in while these two are opt-out.No deployment changes behaviour. Google and GitHub stay on by default when self-hosting, exactly as today; cloud is unaffected either way.
On the alternative fix
The issue offered two resolutions, and I deliberately took the conservative one.
Flipping the logic instead (
!X→X) would also make the name truthful, but it changes the self-hosted default from shown to hidden — every existing self-hoster with Google OAuth configured and the variable unset (the only way to have it working today) would silently lose the button on upgrade, for a variable they've never heard of because it isn't documented.If you'd rather have opt-in semantics for consistency with AgentID, that's a one-line change on top of this and I'm happy to push it — but it's a product decision about your self-hosting contract, and it wants a release note rather than a quiet rename.
Testing
Behaviour is unchanged by construction, so there is nothing new to assert — the truth table is identical, only the identifier differs:
HOST_IDsupermemoryNo remaining references to the old names (
rg 'GOOGLE_AUTH_ENABLED|GITHUB_AUTH_ENABLED'is empty).tscis unaffected —process.env.Xisstring | undefinedunder either name.