fix(webapp): toast message issue after gh app installation #2546
Conversation
Fixes an issue with displaying toasts messages in the project settings page. The github callback cookie was interfering with the flash cookie used for toast messages.
|
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (2)
688-698: Use replace when clearing the query param to avoid “Back” re‑opening the modalWithout replace, deleting the param pushes a new history entry. Using replace prevents the user pressing Back and re-triggering the modal.
Apply:
- if (params.get("openGithubRepoModal") === "1") { + if (params.get("openGithubRepoModal") === "1") { setIsModalOpen(true); params.delete("openGithubRepoModal"); - setSearchParams(params); + setSearchParams(params, { replace: true }); }
658-659: Remove unused prop from ConnectGitHubRepoModal
open?: booleanisn’t used; trim to avoid confusion.environmentSlug: string; - open?: boolean;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/routes/_app.github.callback/route.tsx(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(7 hunks)apps/webapp/app/services/projectSettings.server.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/services/projectSettings.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/routes/_app.github.callback/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/services/projectSettings.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/routes/_app.github.callback/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/services/projectSettings.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/routes/_app.github.callback/route.tsx
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Files:
apps/webapp/app/services/projectSettings.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/services/projectSettings.server.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/routes/_app.github.callback/route.tsx:33-44
Timestamp: 2025-09-02T11:27:36.336Z
Learning: In the GitHub App installation callback flow in apps/webapp/app/routes/_app.github.callback/route.tsx, the install session cookie is not cleared after use due to interface limitations with redirectWithSuccessMessage/redirectWithErrorMessage not supporting custom headers. The maintainer accepts this design as the 1-hour cookie expiration provides sufficient protection against replay attacks.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/services/gitHubSession.server.ts:31-36
Timestamp: 2025-09-02T11:18:06.602Z
Learning: In the GitHub App installation flow in apps/webapp/app/services/gitHubSession.server.ts, the redirectTo parameter stored in httpOnly session cookies is considered acceptable without additional validation by the maintainer, as the httpOnly cookie provides sufficient security for this use case.
📚 Learning: 2025-09-05T15:32:41.553Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2464
File: apps/webapp/app/services/projectSettings.server.ts:210-0
Timestamp: 2025-09-05T15:32:41.553Z
Learning: In the ProjectSettingsService.updateGitSettings method in apps/webapp/app/services/projectSettings.server.ts, when productionBranch or stagingBranch parameters are undefined, the method intentionally writes empty objects to clear existing branch configurations. This is the desired behavior, not a bug.
Applied to files:
apps/webapp/app/services/projectSettings.server.ts
📚 Learning: 2025-09-02T11:27:36.336Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/routes/_app.github.callback/route.tsx:33-44
Timestamp: 2025-09-02T11:27:36.336Z
Learning: In the GitHub App installation callback flow in apps/webapp/app/routes/_app.github.callback/route.tsx, the install session cookie is not cleared after use due to interface limitations with redirectWithSuccessMessage/redirectWithErrorMessage not supporting custom headers. The maintainer accepts this design as the 1-hour cookie expiration provides sufficient protection against replay attacks.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/routes/_app.github.callback/route.tsx
📚 Learning: 2025-09-02T11:18:06.602Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/services/gitHubSession.server.ts:31-36
Timestamp: 2025-09-02T11:18:06.602Z
Learning: In the GitHub App installation flow in apps/webapp/app/services/gitHubSession.server.ts, the redirectTo parameter stored in httpOnly session cookies is considered acceptable without additional validation by the maintainer, as the httpOnly cookie provides sufficient security for this use case.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/routes/_app.github.callback/route.tsx
🧬 Code graph analysis (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (3)
apps/webapp/app/routes/_app.github.callback/route.tsx (1)
loader(29-121)apps/webapp/app/hooks/useSearchParam.ts (1)
useSearchParams(7-70)apps/webapp/app/utils/pathBuilder.ts (1)
v3ProjectSettingsPath(418-424)
apps/webapp/app/routes/_app.github.callback/route.tsx (1)
apps/webapp/app/models/message.server.ts (1)
redirectWithSuccessMessage(162-179)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (5)
apps/webapp/app/services/projectSettings.server.ts (1)
92-95: Staging tracking intentionally empty on connect — LGTMInitializing
staging: {}(while keepingprodon the default branch) aligns with the documented behavior to use empty objects for “no tracking.” This avoids accidental staging deploys and matches the prior learning for clearing configs.apps/webapp/app/routes/_app.github.callback/route.tsx (1)
6-6: Unified success redirects fix the toast-cookie conflict — LGTMSwitching to
redirectWithSuccessMessageacross success paths resolves the flash message interference and keeps the flow consistent. This also preserves the accepted trade‑off where the GitHub install cookie isn’t explicitly cleared post‑callback.Also applies to: 91-91, 104-104, 114-114
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (3)
125-130: Loader now returns only core data (no session flags) — LGTMTyped response and removal of modal/session flags is cleaner and aligns with the new URL‑driven flow.
755-760: Passing the openGithubRepoModal flag in redirect — LGTMAppending
?openGithubRepoModal=1ensures the modal opens post‑install without relying on loader/session state.
865-870: URL‑driven modal open for initial app install — LGTMSame pattern as above; consistent and stateless.
Fixes an issue with displaying toasts messages in the project settings
page. The github callback cookie was interfering with the flash cookie used
for toast messages.