Skip to content

Conversation

myftija
Copy link
Member

@myftija myftija commented Oct 10, 2025

Docs page about the automatic deployments using the new Trigger.dev GitHub integration.

Copy link

changeset-bot bot commented Oct 10, 2025

⚠️ No Changeset found

Latest commit: 5370d71

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

Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

Walkthrough

  • apps/webapp/app/routes/_app.github.callback/route.tsx: Changed user-facing messages from "GitHub App" to "GitHub app" in error/redirect paths (invalid params, invalid session, unauthenticated org, installation/link errors, and default case). No control-flow or logic changes.
  • docs/docs.json: Replaced a single vercel-integration entry with a Deployment integrations group containing github-integration and vercel-integration.
  • docs/github-actions.mdx: Added a Tip block referencing the new GitHub integration.
  • docs/github-integration.mdx: Added a new documentation page covering GitHub integration setup, branch tracking, preview deployments, disconnect flow, repo access management, and build/environment configuration (includes private npm registry guidance).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description is only a single summary sentence and does not follow the repository’s required template because it omits the issue reference, checklist, testing steps, changelog entry, and screenshots sections. Please update the description to use the provided template by adding “Closes #”, completing the checklist, describing your testing steps, including a brief changelog, and attaching screenshots if applicable.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly summarizes the primary change of adding documentation for deploying with the GitHub integration and aligns with the main purpose of this pull request without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch gh-app-docs

📜 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 49a1eb0 and 5370d71.

📒 Files selected for processing (1)
  • docs/github-integration.mdx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/github-integration.mdx
⏰ 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 (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 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 (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 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 (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)

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: 1

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/routes/_app.github.callback/route.tsx (1)

40-119: Inconsistent capitalization of "GitHub app".

The changes update error messages to use "GitHub app" (lowercase 'a') on lines 40, 53, 75, 88, and 119, but success messages still use "GitHub App" (uppercase 'A') on lines 90, 104, and 114. This creates inconsistent terminology across user-facing messages.

For consistency, update all remaining instances:

-      return redirectWithSuccessMessage(redirectTo, request, "GitHub App installed successfully");
+      return redirectWithSuccessMessage(redirectTo, request, "GitHub app installed successfully");
-        return redirectWithErrorMessage(redirectTo, request, "Failed to update GitHub App");
+        return redirectWithErrorMessage(redirectTo, request, "Failed to update GitHub app");
-      return redirectWithSuccessMessage(redirectTo, request, "GitHub App updated successfully");
+      return redirectWithSuccessMessage(redirectTo, request, "GitHub app updated successfully");
-      return redirectWithSuccessMessage(redirectTo, request, "GitHub App installation requested");
+      return redirectWithSuccessMessage(redirectTo, request, "GitHub app installation requested");
🧹 Nitpick comments (1)
docs/github-integration.mdx (1)

52-62: Remove redundant phrasing.

The phrase "by default" appears twice - once at the end of line 52 and again in the Note at line 60, creating unnecessary redundancy.

Consider simplifying line 52:

-When you connect a repository, the default branch of your repository will be used as the production tracking branch, by default.
+When you connect a repository, the default branch of your repository will be used as the production tracking branch.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f8977a7 and a1f3b53.

⛔ Files ignored due to path filters (2)
  • docs/deployment/connect-repo.png is excluded by !**/*.png
  • docs/deployment/git-settings.png is excluded by !**/*.png
📒 Files selected for processing (4)
  • apps/webapp/app/routes/_app.github.callback/route.tsx (5 hunks)
  • docs/docs.json (1 hunks)
  • docs/github-actions.mdx (1 hunks)
  • docs/github-integration.mdx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/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/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/routes/_app.github.callback/route.tsx
🧠 Learnings (1)
📚 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.github.callback/route.tsx
🧬 Code graph analysis (1)
apps/webapp/app/routes/_app.github.callback/route.tsx (1)
apps/webapp/app/models/message.server.ts (1)
  • redirectWithErrorMessage (181-198)
⏰ 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 (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 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 (6, 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 (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
docs/docs.json (1)

113-115: LGTM!

The navigation structure correctly adds the new GitHub integration documentation alongside the existing Vercel integration.

docs/github-actions.mdx (1)

8-10: LGTM!

The Tip block effectively guides users to the new GitHub integration as an alternative to manual GitHub Actions workflows.

docs/github-integration.mdx (1)

1-132: Well-structured documentation!

The documentation is comprehensive and covers all key aspects of the GitHub integration: setup, branch tracking, repository management, and build configuration. The examples are clear and helpful.

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

🧹 Nitpick comments (1)
docs/github-integration.mdx (1)

52-61: Tighten duplicate “default branch” wording.

Lines 52-53 already explain that the default repo branch becomes the production tracking branch, and the note on Lines 60-61 repeats the same idea (with an extra “by default”). Consider consolidating into a single sentence to keep the section concise and avoid the echo.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1f3b53 and 49a1eb0.

📒 Files selected for processing (1)
  • docs/github-integration.mdx (1 hunks)
⏰ 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). (17)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 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 (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 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 (2, 8)
  • 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: typecheck / typecheck

@myftija myftija merged commit 0cabbdd into main Oct 13, 2025
28 of 32 checks passed
@myftija myftija deleted the gh-app-docs branch October 13, 2025 12:15
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.

2 participants