Skip to content

Conversation

@0ski
Copy link
Collaborator

@0ski 0ski commented Dec 10, 2025

✅ Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • I ran and tested the code works

Testing

Set up the local github application, and tested its connection with trigger.dev

Checked:

  • Change is backwards compatible
  • Actions/ux are uniform across the pages (empty Tasks, Deployments, project settings)
  • Connecting GH, Connecting Repo, disconnecting Repo, modifying settings

Changelog

  • Create new resource route for GitHub settings management with loader and actions
  • Add GitHubSettingsPresenter to fetch connected repos and installations
  • Implement GitHubSettingsPanel component for reusable GitHub configuration UI
  • Refactor project settings page to use shared GitHubSettingsPanel component
  • Integrate GitHub connection flow into empty state onboarding for Tasks and Deployments
  • Add support for GitHub repo connection, disconnection, and branch tracking settings
  • Include redirect URL support for seamless navigation after GitHub actions
  • Remove duplicate GitHub connection code from project settings route

Screenshots

GH-connections.mp4

🐐

samejr and others added 24 commits December 10, 2025 11:24
…ts pages

- Create new resource route for GitHub settings management with loader and actions
- Add GitHubSettingsPresenter to fetch connected repos and installations
- Implement GitHubSettingsPanel component for reusable GitHub configuration UI
- Refactor project settings page to use shared GitHubSettingsPanel component
- Integrate GitHub connection flow into empty state onboarding for Tasks and Deployments
- Add support for GitHub repo connection, disconnection, and branch tracking settings
- Include redirect URL support for seamless navigation after GitHub actions
- Remove duplicate GitHub connection code from project settings route
@0ski 0ski self-assigned this Dec 10, 2025
@changeset-bot
Copy link

changeset-bot bot commented Dec 10, 2025

⚠️ No Changeset found

Latest commit: 7ba22e8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@0ski 0ski changed the title Oo/feat GitHub onboarding feat(webapp): add GitHub onboarding flow to empty Tasks and Deployments pages Dec 10, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

Replaces duplicated deployment onboarding UIs with a shared DeploymentOnboardingSteps component and adds a tabbed onboarding flow using ClientTabs and GitHubSettingsPanel. Refactors ClientTabs and Tabs to support a Variants type (segmented, underline, pipe-divider) and context-driven behavior, adjusting component props and removing ClientTabsWithUnderline. Adds a GitHubSettingsPresenter to aggregate GitHub settings server-side and introduces a new GitHub resource route with loaders, actions, forms, and UI (ConnectGitHubRepoModal, ConnectedGitHubRepoForm, GitHubSettingsPanel). Settings and deployment routes and several storybook examples updated for the new components and styles.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Areas requiring extra attention:

  • ClientTabs.tsx refactor (context, controlled/uncontrolled behavior, layoutId motion).
  • Tabs.tsx variant branching and motion/spring indicator logic (segmented, pipe-divider, underline).
  • GitHubSettingsPresenter.server.ts async orchestration, schema validation, and error handling.
  • New GitHub resource route (loader/action schemas, discriminated GitHubAction handling, modal/forms wiring).
  • Settings route changes (removed GitHub actions/loader fields) and integration with GitHubSettingsPanel.
  • BlankStatePanels consolidation to DeploymentOnboardingSteps and environment-specific rendering.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.52% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically summarizes the main change: adding a GitHub onboarding flow to empty Tasks and Deployments pages, which aligns with the core objective of the changeset.
Description check ✅ Passed The PR description follows the template structure with completed checklist, testing details, comprehensive changelog, and screenshots. All required sections are present and adequately filled out.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch oo/feat-github-onboarding

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 990c6c8 and 7ba22e8.

📒 Files selected for processing (1)
  • apps/webapp/app/components/primitives/Tabs.tsx (3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

Files:

  • apps/webapp/app/components/primitives/Tabs.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

  • apps/webapp/app/components/primitives/Tabs.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

Files:

  • apps/webapp/app/components/primitives/Tabs.tsx
apps/webapp/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access all environment variables through the env export of env.server.ts instead of directly accessing process.env in the Trigger.dev webapp

Files:

  • apps/webapp/app/components/primitives/Tabs.tsx
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: When importing from @trigger.dev/core in the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Files:

  • apps/webapp/app/components/primitives/Tabs.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier

Files:

  • apps/webapp/app/components/primitives/Tabs.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/primitives/Tabs.tsx (3)
apps/webapp/app/components/primitives/ClientTabs.tsx (1)
  • TabsProps (200-209)
apps/webapp/app/utils/cn.ts (1)
  • cn (77-79)
apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
  • ShortcutKey (38-55)
⏰ 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). (23)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/webapp/app/components/primitives/Tabs.tsx (6)

8-8: LGTM!

Using a string union type for Variants aligns with coding guidelines (avoiding enums) and provides clear type safety for the variant options.


32-58: LGTM!

The variant-based rendering logic for TabContainer is clean and handles all three variants appropriately with consistent styling patterns.


108-130: Unused layoutId in pipe-divider variant.

The layoutId prop is received but not used in this variant, unlike segmented and underline which use it for motion.div layout animations. If this is intentional (no animation needed for this variant), consider documenting the design decision. Otherwise, add an animated active indicator for consistency.


71-106: LGTM!

The segmented variant implementation is well-structured with proper z-index layering (z-10 on content) to ensure text appears above the animated background indicator.


149-153: LGTM!

The spring animation parameters (stiffness: 500, damping: 30) provide a responsive feel for the underline indicator transition.


202-202: Text color is now always bright.

The text styling is now fixed to text-text-bright regardless of isActive state, with only the underline indicator differentiating active/inactive states. This is a behavioral change from typical tab patterns where inactive text is dimmed. Verify this is the intended UX.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/webapp/app/components/primitives/Tabs.tsx (1)

200-210: Text color conditional has no effect.

Both branches of the ternary apply the same class "text-text-bright", so the isActive state doesn't change the text appearance. This looks like an oversight.

           <span
             className={cn(
               "text-sm transition duration-200",
-              isActive ? "text-text-bright" : "text-text-bright"
+              isActive ? "text-text-bright" : "text-text-dimmed hover:text-text-bright"
             )}
           >
🧹 Nitpick comments (5)
apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts (2)

1-5: Remove unused import err.

The err function from neverthrow is imported but never used in this file.

-import { err, fromPromise, ok, ResultAsync } from "neverthrow";
+import { fromPromise, ok, ResultAsync } from "neverthrow";

13-23: Consider wrapping the early return in ResultAsync for type consistency.

The call() method returns Result<...> when GitHub is disabled (line 17) but ResultAsync<...> otherwise (line 126). While both can be awaited, wrapping the early return maintains consistent typing for consumers.

     if (!githubAppEnabled) {
-      return ok({
+      return ResultAsync.fromSafePromise(Promise.resolve({
         enabled: false,
         connectedRepository: undefined,
         installations: undefined,
         isPreviewEnvironmentEnabled: undefined,
-      });
+      }));
     }
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (2)

823-825: Add fetcher to the dependency array or use a stable reference.

The fetcher.load function should be stable, but ESLint's exhaustive-deps rule will flag this. Consider using fetcher or extracting the load call.

   useEffect(() => {
     fetcher.load(gitHubResourcePath(organizationSlug, projectSlug, environmentSlug));
-  }, [organizationSlug, projectSlug, environmentSlug]);
+  }, [organizationSlug, projectSlug, environmentSlug, fetcher]);

Alternatively, if fetcher causes re-fetching issues, you can disable the lint rule with a comment explaining the intentional omission.


869-873: Remove redundant condition check.

At this point in the code, data.connectedRepository is guaranteed to be falsy (we're in the else branch of line 845), so the condition on line 869 is always true.

   return (
     <div className="flex flex-col gap-2">
       <GitHubConnectionPrompt
         gitHubAppInstallations={data.installations ?? []}
         organizationSlug={organizationSlug}
         projectSlug={projectSlug}
         environmentSlug={environmentSlug}
         redirectUrl={effectiveRedirectUrl}
       />
-      {!data.connectedRepository && (
-        <Hint>
-          Connect your GitHub repository to automatically deploy your changes.
-        </Hint>
-      )}
+      <Hint>
+        Connect your GitHub repository to automatically deploy your changes.
+      </Hint>
     </div>
-    
   );
apps/webapp/app/components/primitives/ClientTabs.tsx (1)

63-81: Consider aligning default variant with Tabs component.

ClientTabsList defaults to "pipe-divider" (line 68), while Tabs and TabContainer default to "underline". If intentional, this is fine, but aligning defaults would reduce surprise when switching between the two tab implementations.

->(({ className, variant = "pipe-divider", ...props }, ref) => {
+>(({ className, variant = "underline", ...props }, ref) => {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b00198 and 990c6c8.

📒 Files selected for processing (9)
  • apps/webapp/app/components/BlankStatePanels.tsx (5 hunks)
  • apps/webapp/app/components/primitives/ClientTabs.tsx (2 hunks)
  • apps/webapp/app/components/primitives/Tabs.tsx (3 hunks)
  • apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts (1 hunks)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (1 hunks)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (5 hunks)
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (1 hunks)
  • apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx (1 hunks)
  • apps/webapp/app/routes/storybook.tabs/route.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

Files:

  • apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
  • apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts
  • apps/webapp/app/components/primitives/Tabs.tsx
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx
  • apps/webapp/app/routes/storybook.tabs/route.tsx
  • apps/webapp/app/components/BlankStatePanels.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
  • apps/webapp/app/components/primitives/ClientTabs.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

  • apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
  • apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts
  • apps/webapp/app/components/primitives/Tabs.tsx
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx
  • apps/webapp/app/routes/storybook.tabs/route.tsx
  • apps/webapp/app/components/BlankStatePanels.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
  • apps/webapp/app/components/primitives/ClientTabs.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

Files:

  • apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
  • apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts
  • apps/webapp/app/components/primitives/Tabs.tsx
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx
  • apps/webapp/app/routes/storybook.tabs/route.tsx
  • apps/webapp/app/components/BlankStatePanels.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
  • apps/webapp/app/components/primitives/ClientTabs.tsx
apps/webapp/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access all environment variables through the env export of env.server.ts instead of directly accessing process.env in the Trigger.dev webapp

Files:

  • apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
  • apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts
  • apps/webapp/app/components/primitives/Tabs.tsx
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx
  • apps/webapp/app/routes/storybook.tabs/route.tsx
  • apps/webapp/app/components/BlankStatePanels.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
  • apps/webapp/app/components/primitives/ClientTabs.tsx
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: When importing from @trigger.dev/core in the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Files:

  • apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
  • apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts
  • apps/webapp/app/components/primitives/Tabs.tsx
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx
  • apps/webapp/app/routes/storybook.tabs/route.tsx
  • apps/webapp/app/components/BlankStatePanels.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
  • apps/webapp/app/components/primitives/ClientTabs.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier

Files:

  • apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
  • apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts
  • apps/webapp/app/components/primitives/Tabs.tsx
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx
  • apps/webapp/app/routes/storybook.tabs/route.tsx
  • apps/webapp/app/components/BlankStatePanels.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
  • apps/webapp/app/components/primitives/ClientTabs.tsx
🧠 Learnings (5)
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx
📚 Learning: 2025-12-08T15:19:56.801Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.801Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes

Applied to files:

  • apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts
📚 Learning: 2025-09-02T11:27:36.336Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 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.tsx
📚 Learning: 2025-08-14T12:13:20.455Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2392
File: packages/cli-v3/src/utilities/gitMeta.ts:195-218
Timestamp: 2025-08-14T12:13:20.455Z
Learning: In the GitMeta schema (packages/core/src/v3/schemas/common.ts), all fields are intentionally optional to handle partial data from various deployment contexts (local, GitHub Actions, GitHub App). Functions like getGitHubAppMeta() are designed to work with missing environment variables rather than validate their presence.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
🧬 Code graph analysis (7)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (2)
apps/webapp/app/components/layout/AppLayout.tsx (1)
  • MainCenteredContainer (50-64)
apps/webapp/app/components/BlankStatePanels.tsx (1)
  • DeploymentsNoneDev (224-277)
apps/webapp/app/components/primitives/Tabs.tsx (2)
apps/webapp/app/components/primitives/ClientTabs.tsx (1)
  • TabsProps (200-209)
apps/webapp/app/utils/cn.ts (1)
  • cn (77-79)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (4)
apps/webapp/app/utils/pathBuilder.ts (3)
  • EnvironmentParamSchema (26-28)
  • githubAppInstallPath (148-152)
  • v3ProjectSettingsPath (427-433)
apps/webapp/app/models/runtimeEnvironment.server.ts (1)
  • findEnvironmentBySlug (116-145)
apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts (1)
  • GitHubSettingsPresenter (12-137)
apps/webapp/app/services/projectSettings.server.ts (1)
  • ProjectSettingsService (9-329)
apps/webapp/app/routes/storybook.tabs/route.tsx (4)
apps/webapp/app/components/primitives/Headers.tsx (1)
  • Header1 (32-50)
apps/webapp/app/components/primitives/Paragraph.tsx (1)
  • Paragraph (88-107)
apps/webapp/app/components/primitives/Tabs.tsx (1)
  • Tabs (20-30)
apps/webapp/app/components/primitives/ClientTabs.tsx (4)
  • ClientTabs (211-211)
  • ClientTabsList (211-211)
  • ClientTabsTrigger (211-211)
  • ClientTabsContent (211-211)
apps/webapp/app/components/BlankStatePanels.tsx (7)
apps/webapp/app/components/environments/EnvironmentLabel.tsx (2)
  • EnvironmentIcon (14-48)
  • environmentFullTitle (145-160)
apps/webapp/app/components/primitives/Buttons.tsx (1)
  • LinkButton (335-401)
apps/webapp/app/utils/pathBuilder.ts (2)
  • docsPath (510-512)
  • v3BillingPath (487-491)
apps/webapp/app/hooks/useEnvironment.tsx (1)
  • useEnvironment (19-23)
apps/webapp/app/components/SetupCommands.tsx (1)
  • PackageManagerProvider (21-29)
apps/webapp/app/components/primitives/TextLink.tsx (1)
  • TextLink (29-94)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (1)
  • GitHubSettingsPanel (807-877)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (2)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (2)
  • loader (117-142)
  • GitHubSettingsPanel (807-877)
apps/webapp/app/utils/pathBuilder.ts (1)
  • v3BillingPath (487-491)
apps/webapp/app/components/primitives/ClientTabs.tsx (2)
apps/webapp/app/components/primitives/Tabs.tsx (1)
  • Variants (8-8)
apps/webapp/app/utils/cn.ts (1)
  • cn (77-79)
⏰ 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). (23)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (20)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (1)

362-368: LGTM!

The width change from max-w-md to max-w-prose is appropriate for the onboarding content, which contains multiple paragraphs and step instructions. This provides better readability with approximately 65 characters per line.

apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts (1)

74-107: LGTM with a note on pagination.

The query limits (take: 200 for repositories, take: 20 for installations) are reasonable defaults. For organizations with many installations or repositories, consider adding a hasMore indicator or pagination support in a future iteration if users report truncated results.

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (4)

57-111: LGTM!

Types and schemas are well-structured. The use of type over interface and Zod for validation aligns with the coding guidelines. The discriminated union pattern for GitHubActionSchema enables type-safe action dispatch.


117-142: LGTM!

The loader follows Remix conventions, uses proper error handling with 404/500 responses, and correctly integrates with the GitHubSettingsPresenter.


164-309: LGTM!

The action handler is well-structured with:

  • Proper membership verification before mutations
  • Clear separation using ProjectSettingsService
  • User-friendly error messages for known error types
  • Exhaustive type checking with satisfies never

379-383: Verify: Modal auto-close logic may be unreachable.

The action handler always redirects on success (lines 210-216), so lastSubmission won't contain a success response—the page will navigate away. Additionally, the conform/zod parse result structure doesn't include a success property in this form.

This effect may be dead code. If auto-closing is needed for non-redirect scenarios, consider checking the actual submission result shape.

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (2)

46-46: LGTM!

Good refactoring to import the shared GitHubSettingsPanel component, consolidating GitHub settings UI into a single reusable module.


414-426: LGTM!

The integration of GitHubSettingsPanel is clean. The component handles its own data fetching via a fetcher, and the billingPath prop is correctly constructed using v3BillingPath. This removes duplicate GitHub-related code from the settings route.

apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx (1)

3-10: LGTM!

Minor styling update to the storybook tab content display. The centered layout with background is appropriate for demonstrating tab content.

apps/webapp/app/routes/storybook.tabs/route.tsx (1)

12-188: LGTM! Storybook demo showcases all tab variants effectively.

The two-column layout clearly demonstrates the difference between URL-based Tabs and state-based ClientTabs across all three variants. The multiple <Outlet /> components (lines 30, 43, 56) will all render the same child route content, which is appropriate for this demonstration context.

apps/webapp/app/components/BlankStatePanels.tsx (4)

55-61: LGTM! Clean imports for the new tabbed onboarding UI.

The ClientTabs primitives and GitHubSettingsPanel imports are correctly added to support the new deployment onboarding flow.


102-104: Good consolidation of deployment onboarding.

Delegating to DeploymentOnboardingSteps reduces duplication and centralizes the onboarding logic.


224-277: LGTM! DeploymentsNoneDev refactored with cleaner layout.

The new layout with EnvironmentSelector and doc/AI assistant links provides a streamlined experience for the development environment.


594-688: Well-structured tabbed deployment onboarding component.

The DeploymentOnboardingSteps component effectively consolidates the three deployment methods (GitHub, CLI, GitHub Actions) into a tabbed interface. The GitHubSettingsPanel integration at lines 652-657 correctly wires up the GitHub connection flow.

One minor observation: the billingPath prop uses { slug: organization.slug } rather than passing organization directly. This is fine since v3BillingPath only needs the slug, but for consistency with other usages, you could pass organization directly if OrgForPath type is satisfied.

apps/webapp/app/components/primitives/Tabs.tsx (3)

8-9: Good use of string union type for variants.

Using type Variants = "underline" | "pipe-divider" | "segmented" follows the coding guideline to prefer string unions over enums. As per coding guidelines.


32-58: LGTM! TabContainer variant rendering is clean.

The conditional rendering for each variant provides distinct styling while maintaining a consistent API.


71-161: LGTM! TabLink variant implementations with smooth animations.

The variant-specific rendering with motion animations provides a polished UI experience. The spring configurations are appropriate for each variant's visual style.

apps/webapp/app/components/primitives/ClientTabs.tsx (3)

9-17: LGTM! Context pattern for tracking active tab value.

The context approach enables ClientTabsTrigger to determine its active state without prop drilling, which is cleaner than relying solely on Radix's internal state.


22-60: Well-implemented controlled/uncontrolled pattern.

The component correctly handles both controlled (value prop) and uncontrolled (defaultValue prop) modes. The useEffect syncs external value changes, and handleValueChange properly calls the callback while managing internal state only when uncontrolled.


84-182: LGTM! Comprehensive variant rendering in ClientTabsTrigger.

The trigger correctly uses context to determine active state and provides appropriate rendering for all three variants. The layoutId handling gracefully falls back to static elements when not provided.

@matt-aitken matt-aitken merged commit d287078 into main Dec 10, 2025
31 checks passed
@matt-aitken matt-aitken deleted the oo/feat-github-onboarding branch December 10, 2025 17:03
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.

4 participants