Repository navigation
fix(frontend): make /invite reach discord and stop asking for admin - #1889
Merged
Merged
Conversation
Two defects in the invite funnel, both found while investigating the Top.gg rejection (#1885). /invite had no route. App.tsx defines `/`, `/login` and a catch-all that navigates to `/`, and vercel.json rewrites everything else to index.html, so https://lucky.lucassantana.tech/invite served the SPA and bounced to the landing page. That URL is what the Top.gg listing body, the README CTA and every tracked campaign URL point at, so no invite link in the funnel actually reached Discord and no utm_source ever recorded an install. Adds a vercel redirect. Docs.tsx hardcoded `permissions=8`, which is Administrator, while Landing.tsx deliberately used the minimal 3165184 set. The public listings state "No admin permission", so we were shipping a link that contradicted our own copy, and a reviewer following the docs invite would have seen an Administrator consent screen. Both pages now build the URL from one helper in lib/discord.ts. The drift was only possible because each page rolled its own. Test asserts the URL never requests Administrator, keeps the applications.commands scope (its absence is the shape of #1885), and targets the OAuth endpoint. Verified it fails when the permission set is put back to 8. Frontend suite green: 86 files, 1019 tests. Closes #1888.
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Failed to generate code suggestions for PR |
|
Size Change: -40 B (-0.01%) Total Size: 497 kB 📦 View Changed
ℹ️ View Unchanged
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
DEFAULT_DISCORD_CLIENT_ID is a non-empty literal, so the `||` fallback always yields a client id and `if (!clientId) return ''` could never run. Raised by cubic on #1889. The helper's real contract is that it always returns a URL, so say that rather than keeping a guard that reads like a configuration path but is not one. Frontend suite green: 86 files, 1019 tests.
Owner
Author
|
Both threads addressed. cubic P3 — unreachable disabled-invite path. Correct, fixed in Trivy — "Secret Discord client ID" (MEDIUM). False positive. A Discord client id is the application id, and it is public by design: it is embedded in every OAuth invite URL, it is visible in the Top.gg listing, and it is already published in The value that must never ship is // Public Discord Application ID (a.k.a. client_id). Safe to ship: it appears in
// every OAuth invite link and is not a secret. Used as the default so the CTA
// works out of the box; override via VITE_DISCORD_CLIENT_ID for a fork.Worth noting the rule is finding this only because the constant was centralised by this PR. The same literal was already hardcoded in Frontend suite green: 86 files, 1019 tests. |
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Auto-approved: Fixes two bugs: makes /invite redirect to Discord OAuth and removes Administrator permission from docs invite, consolidating URL generation. Changes are bounded, well-tested, and reduce risk.
Re-trigger cubic
|
Merged
LucasSantana-Dev
added a commit
that referenced
this pull request
Jul 26, 2026
🤖 I have created a release *beep* *boop* --- <details><summary>2.37.1</summary> ## [2.37.1](v2.37.0...v2.37.1) (2026-07-26) ### Bug Fixes * **bot:** register slash commands globally, not per cached guild ([#1887](#1887)) ([f7d38d5](f7d38d5)) * **ci:** make the security gate passable and clear production advisories ([#1876](#1876)) ([ca410d2](ca410d2)) * **frontend:** make /invite reach discord and stop asking for admin ([#1889](#1889)) ([c322834](c322834)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved bot slash command registration. * Updated the CI and security gate configuration. * Improved the frontend invite flow so it reaches Discord without requesting administrator permissions. * **Chores** * Released version 2.37.1. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
LucasSantana-Dev
added a commit
that referenced
this pull request
Jul 27, 2026
cubic was right on #1893: redirecting at the edge fixes the click but drops the attribution. backend/src/routes/invite.ts already existed and logs the utm_* parameters before redirecting, but nginx only proxies /api, so /invite never reached it and fell through to the SPA catch-all. nginx now proxies the exact path to the backend instead of returning a redirect itself, so tracked links keep emitting `[invite] click`. Chasing that turned up a third permission value. The codebase shipped: backend invite.ts 36970496 Manage Messages, Connect, Speak frontend discord.ts 3165184 View Channels, Send Messages, Embed Links, Connect, Speak docs page (fixed #1889) 8 Administrator The backend one is the worst of the three: it asks for Manage Messages, which is message deletion, while omitting View Channels and Send Messages, so it is over-scoped and non-functional at the same time. It is also the value that every tracked link resolved to. Single source of truth is now BOT_INVITE_PERMISSIONS in shared, consumed by the backend redirect and the frontend helper. Test pins the bitfield: no Administrator, no Manage Messages, exactly the five needed bits, and the applications.commands scope whose absence caused #1885. vercel.json points at the backend too, so preview deploys log the same. nginx -t clean against nginxinc/nginx-unprivileged:1.31-alpine. Suites green: shared 1398, frontend 1019, backend 1350.
LucasSantana-Dev
added a commit
that referenced
this pull request
Jul 27, 2026
…ons (#1893) nginx only proxied /api, so the canonical /invite URL never reached backend/src/routes/invite.ts and fell through to the SPA catch-all, which bounces unknown paths to the landing page. Every invite click from the Top.gg listing, the README CTA and every tracked campaign link was swallowed, and no utm_source ever recorded an install. nginx now proxies the exact path to the backend rather than redirecting at the edge, so the existing [invite] click attribution keeps working. Raised by cubic on the first version of this PR, which redirected at the edge and would have dropped it. That surfaced a third permission value: the backend redirect shipped 36970496 (Manage Messages, Connect, Speak), over-scoped on message deletion and missing View Channels / Send Messages, while the frontend used 3165184 and the docs page had shipped 8 (Administrator) until #1889. Single source of truth is now BOT_INVITE_PERMISSIONS in shared, with a test pinning the bitfield. Closes the production half of #1888.
LucasSantana-Dev
added a commit
that referenced
this pull request
Jul 27, 2026
I fixed this twice in the wrong layer. lucky.lucassantana.tech is served by Cloudflare Pages (project lucky-webapp, deploy-frontend-cf.yml), not by Vercel and not by the homelab nginx behind the tunnel. So neither the vercel.json redirect (#1889) nor the nginx location block (#1893) applied to the public site, and /invite kept returning 200 with the SPA. What gave it away: the live response carries a CSP allowing static.cloudflareinsights.com, which appears only in packages/frontend/public/_headers. The nginx config serves a different CSP, so the request was never reaching it. Meanwhile lucky-api.lucassantana.tech DOES go through the tunnel, and already returns the correct 302 with permissions=3165184, so the backend handler and the nginx work from #1893 are both fine and stay. _redirects rules are evaluated top to bottom, so /invite is placed above the SPA catch-all, which is what was swallowing it. Points at the backend rather than Discord directly so the utm_* attribution logging still runs. Verified the file lands in packages/frontend/dist after a build, which is the directory `wrangler pages deploy` uploads.
LucasSantana-Dev
added a commit
that referenced
this pull request
Jul 27, 2026
lucky.lucassantana.tech is served by Cloudflare Pages (project lucky-webapp), not by Vercel and not by the homelab nginx behind the tunnel. So neither the vercel.json redirect (#1889) nor the nginx location block (#1893) applied to the public site, and /invite kept returning 200 with the SPA, whose catch-all bounces to the landing page. _redirects rules are evaluated top to bottom, so /invite now sits above the SPA catch-all that was swallowing it. It points at the backend rather than Discord directly, so the utm_* attribution logging still runs. The nginx block and shared BOT_INVITE_PERMISSIONS from #1893 stay: they are what make lucky-api.lucassantana.tech/invite return the correct 302. Closes #1888.
This was referenced Jul 29, 2026
Merged
This was referenced Sep 6, 2026
Merged
LucasSantana-Dev
added a commit
that referenced
this pull request
Sep 8, 2026
## Summary Nothing in the repo said which of the three independent hosting layers (Cloudflare Pages, Cloudflare Tunnel + homelab nginx, legacy Vercel preview) serves which host, or which config file governs each. Fixing the /invite redirect took three PRs (#1889, #1893, #1895) before landing in the right file. - Added a "Deployment & hosting" section to docs/ARCHITECTURE.md: the host/layer/config table, the CSP trick for identifying which layer answered a request, and the nginx.conf/_redirects gotchas that cost time before. - Added a one-line pointer to that section at the top of nginx.conf and _redirects. - vercel.json and _headers intentionally left untouched: vercel.json is strict JSON with no safe comment syntax, and _headers' Cloudflare Pages comment support isn't proven in this repo the way _redirects' is (its existing comment block already works in production - _headers has none to point to as precedent). Not worth guessing on a live config file for a one-line pointer when docs/ARCHITECTURE.md already names both files. ## Test plan - [x] Read-only doc/comment change, no code paths touched - [x] nginx.conf comment uses `#`, the format nginx already uses throughout this file - [x] _redirects comment extends the file's own pre-existing `#` comment block, proven safe since it's already live in production Closes #1924 <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Documents which of the three hosting layers (Cloudflare Pages, Cloudflare Tunnel + homelab nginx, legacy Vercel preview) serves which host and which config file governs each, so fixes like `/invite` land in the right file instead of taking three PRs. - Adds a Deployment & hosting section to `docs/ARCHITECTURE.md` with the host/layer/config table, the CSP trick for identifying which layer answered, the `nginx.conf` `/api`-proxying rule plus its `/invite`, `/webhook/`, `/webhooks/` exceptions, and the `_redirects` top-to-bottom gotcha. - Adds pointers to that section at the top of `nginx.conf` and `_redirects`. - Read-only change; leaves `vercel.json` and `_headers` untouched. Closes #1924. <sup>Written for commit 4b401a0. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/2253?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img](https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a){kind=link}
This was referenced Oct 1, 2026
Merged
Merged
Merged
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
2 participants
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.
Closes #1888. Found while investigating the Top.gg rejection (#1885); neither of these caused it, but both are live and both are risks for the resubmission.
1.
/invitenever reached Discordhttps://lucky.lucassantana.tech/inviteis the canonical invite URL, used by the Top.gg listing body, the README CTA and every tracked campaign URL in.agents/tracking-urls.md.There was no
/inviteroute.App.tsxdefines/,/loginand a catch-all<Navigate to='/' replace />, andvercel.jsonrewrites/:path*toindex.html. So the link served the SPA and bounced to the landing page.Verified before the fix:
Every invite CTA in the funnel was a no-op, and the
utm_sourceattribution the tracking doc is built on never recorded an install. Fixed with aredirectsentry invercel.json(Vercel evaluates redirects before rewrites, so the SPA catch-all is unaffected).2. The docs invite requested Administrator
Docs.tsxhardcodedpermissions=8. That is Administrator.Landing.tsxdeliberately used3165184(View Channels, Send Messages, Embed Links, Connect, Speak) with a comment explaining that Administrator is not wanted.The Top.gg listing states:
So we shipped a link contradicting our own public copy, and a reviewer following the docs invite would have hit an Administrator consent screen. Independently, it is a poor default for a bot that sells itself on minimal permissions.
Change
One helper in
lib/discord.ts, used by both pages. The drift was only possible because each page rolled its own URL.Tests
lib/discord.test.tsasserts the URL never requests Administrator, keeps theapplications.commandsscope (its absence is the shape of #1885), and targets the OAuth endpoint. Verified the guard fails when the permission set is put back to8:Frontend suite green: 86 files, 1019 tests. Typecheck clean.
Summary by cubic
Fixes the invite funnel so
/invitereaches Discord OAuth and all invite links request minimal permissions (no Administrator). Consolidates the invite URL in one helper and adds tests to prevent regressions.Bug Fixes
vercel.jsonredirect so/invitegoes to Discord OAuth (bypasses SPA rewrite and keeps UTM).permissions=3165184withapplications.commands; removedpermissions=8from docs.Refactors
getBotInviteUrlinpackages/frontend/src/lib/discord.ts, used by Landing and Docs; always returns a URL. Addedpackages/frontend/src/lib/discord.test.tsto assert endpoint, scope, and non-admin permissions.Written for commit 6d8b3ff. Summary will update on new commits.