Skip to content

fix(webapp): project integrations page — Staging gating, unreachable code, and follow-ups - #4784

Merged
0ski merged 7 commits into
mainfrom
oskar/fix-vercel-integrations-page
Aug 26, 2026
Merged

fix(webapp): project integrations page — Staging gating, unreachable code, and follow-ups#4784
0ski merged 7 commits into
mainfrom
oskar/fix-vercel-integrations-page

Conversation

@0ski

@0ski 0ski commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Three bugs on the project integrations page, one commit each for the two reported ones and four for the follow-ups found while fixing them.

chore: remove unreachable code on the integrations page (TRI-12645)

Two notification panels in VercelSettingsPanel could never render:

  1. The "Failed to load Vercel settings" panel was gated on a hasError state whose setter is never called anywhere, so it was permanently false.
  2. The "connection expired" banner inside the connectedProject branch was unreachable: VercelSettingsPresenter only populates connectedProject on its success exit, which hardcodes authInvalid: false, while both authInvalid: true exits return connectedProject: undefined.

Removing them makes the surrounding !showAuthInvalid guards vacuous, and the onboardingData?.authInvalid disjunct redundant — the loader already folds onboarding auth state into authInvalid before it reaches the component.

No behaviour change. An org with a connected project and an expired token still gets the banner, from the branch below (untouched).

fix: gate Staging settings on plans without a Staging environment (TRI-12646)

The ticket's premise was inverted, and I've corrected it there. In Git settings, Preview is the row that's correctly gated; Staging is the one with no gate at all:

  • Preview swaps its switch for an Upgrade button, and projectSettings.server.ts neutralises a forged previewDeploymentsEnabled=on.
  • Staging was a plain always-editable Input, and validateStagingBranch only checked the branch existed on GitHub. An org without a staging environment could type a tracking branch, hit Save, get a success toast, and have it silently do nothing.

Staging and Preview environments are created together for projects on a plan that includes them, so gating one and not the other was an oversight.

The Staging row now mirrors the Preview row. Server-side it ignores the submitted branch when there's no staging environment, but preserves the stored branch rather than clearing it — deliberately different from the Preview handling. Forcing a boolean off is harmless; forcing a string off would wipe a tracking branch the org had already configured the first time they saved after losing the environment.

The Vercel write path had the same gap: update-config / complete-onboarding / update-env-mapping never re-derived available env slugs server-side, so ["stg","preview"] could be persisted for a project with neither environment, and createDefaultVercelIntegrationData turned preview on unconditionally. Both now filter against the project's actual environments, via a pure restrictConfigToAvailableEnvSlugs helper that only touches keys present on the input.

fix: show build settings when the GitHub app is disabled (TRI-13488)

The page wrapped Git settings, the Vercel section and build settings in one githubAppEnabled guard, so with the GitHub app off it rendered an empty container.

The Vercel section genuinely depends on GitHub — it can't sync environment variables or link deployments without a connected repo — so it stays gated. Build settings don't: they also apply to CLI deploys run with --native-build-server, exactly as the section's own description states. They now render regardless.

fix: stop the Vercel onboarding modal spinning forever (TRI-13488)

computeInitialState starts in loading-projects whenever the org has a Vercel integration but no onboarding data yet, and the effect that escapes it waits for availableProjects !== undefined. When getOnboardingData returns null — it does that on any thrown error, and when the org integration row is missing — nothing ever arrives.

The empty-array case self-resolves ([] !== undefined), so this is specifically the null case. The route can tell "still loading" from "loaded nothing" because its fetcher always requests ?vercelOnboarding=true; it now passes that down and the modal explains the failure with a retry and a link to check the integration's access on Vercel.

fix: match staging and preview environments consistently (TRI-13488)

The four places that ask "does this project have a staging / preview environment?" disagreed. VercelSettingsPresenter matched on type with no parent filter, so any preview branch row satisfied it — branches are PREVIEW rows too. GitHubSettingsPresenter and ProjectSettingsService matched on slug instead.

Slug is the weaker key: it's derived at creation time and legacy rows can carry something else, which is why memberDevelopmentEnvironmentWhere deliberately avoids it. All four now match on type plus parentEnvironmentId: null, which excludes branches without depending on the slug being canonical.

fix: explain when no Vercel environment can be mapped to Staging (TRI-13488)

Reported while reviewing the branch. The Staging build settings show "Set a Vercel environment for Staging first." whenever the project has a staging environment and no mapping — but the control that sets the mapping only rendered when the Vercel project had at least one custom environment:

hint:     hasStagingEnvironment && !configValues.vercelStagingEnvironment
control:  hasStagingEnvironment && customEnvironments.length > 0

So a Vercel project with no custom environments, or one whose custom environments failed to fetch (the presenter swallows that error to []), got an instruction with nothing to act on. Both conditions predate this PR.

The mapping row now always renders alongside the hint and explains what to do when there's nothing to choose from, and the build-settings hint says the same thing.

chore: remove the remaining dead code (TRI-13488)

  • The "installing" OnboardingState is unproducible — no setState call yields it — so its redirect effect, switch arm, isLoadingState conjunct and the vercelAppInstallPath import it was the only user of are all dead.
  • (state as string) !== "completed" sits in a branch where TypeScript has already narrowed "completed" out; the cast is what let it compile.
  • hideSectionToggles was only ever passed alongside layout="settings" but only read inside layout="card" blocks, so it could never take effect. Removed the prop entirely.
  • Unused bindings and the helpers only they referenced: envSlugLabel, _formatSelectedEnvs, _CompleteOnboardingForm, _handleFinishOnboarding, and the rest.

No behaviour change in that commit.

Not included

The three overlapping modal-open effects in settings.integrations/route.tsx are left alone — they're defensive against a close-then-reopen race, and untangling them is a behavioural risk with no user-visible payoff.

Verification

pnpm run typecheck --filter webapp, pnpm run lint and pnpm run knip are clean. New apps/webapp/test/vercelIntegrationConfig.test.ts covers the slug restriction and the default-config seeding (both pure functions); 39 tests pass across it and the three existing Vercel/project-settings files.

The new projectId + slug query is served by the existing @@unique([projectId, slug, orgMemberId]) prefix — same access pattern as the preview check it mirrors.

refs TRI-12645, TRI-12646, TRI-13488

@0ski 0ski changed the title Two bugs @jamesritchie filed against the Vercel integration project while reworking the integrations page. One commit each. fix(webapp): gate Staging settings, and remove unreachable code on the integrations page Aug 26, 2026
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The change gates GitHub staging branch settings based on a top-level STAGING runtime environment. GitHub responses expose staging availability, and unavailable staging settings show an upgrade prompt while updates preserve the stored branch. Vercel integration defaults and persisted configuration filter unsupported environment slugs. Vercel settings rendering keeps staging configuration visible, renders build settings independently, and handles unavailable onboarding data.

Merge Risk: 🟡 Moderate · up to f2a69

The PR improves staging/preview gating, onboarding failure handling, and environment matching, but the current head can still retain, display, or submit Vercel settings for environments that are unavailable after project changes or deletion. Merge should wait for these bounded stale-configuration cases to be addressed or explicitly accepted.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 11 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: Staging gating, unreachable-code removal, and related fixes on the project integrations page.
Description check ✅ Passed The description is detailed and on-topic. It explains the changes, testing performed, changelog impact, and linked issue references. It does not reproduce the checklist, explicit Changelog heading, or…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is detailed and on-topic. It explains the changes, testing performed, changelog impact, and linked issue references. It does not reproduce the checklist, explicit Changelog heading, or screenshots section, but these omissions are non-critical because the required implementation and verification details are complete.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch oskar/fix-vercel-integrations-page

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.

coderabbitai[bot]

This comment was marked as resolved.

@JamesRitchie

Copy link
Copy Markdown

I think you have tagged the wrong James Ritchie here!

@0ski

0ski commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

I think you have tagged the wrong James Ritchie here!

Hi James! Yes, sorry :)

@changeset-bot

changeset-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: f2a69a7

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 force-pushed the oskar/fix-vercel-integrations-page branch from 8bd3d7a to fc0790d Compare August 26, 2026 12:28
@0ski 0ski changed the title fix(webapp): gate Staging settings, and remove unreachable code on the integrations page fix(webapp): project integrations page — Staging gating, unreachable code, and follow-ups Aug 26, 2026
coderabbitai[bot]

This comment was marked as resolved.

@0ski
0ski force-pushed the oskar/fix-vercel-integrations-page branch from fc0790d to 69453b4 Compare August 26, 2026 12:44
@0ski
0ski marked this pull request as ready for review August 26, 2026 12:47
coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

0ski added 7 commits August 26, 2026 14:56
The Vercel settings panel carried two notification panels that could never
render:

- The "Failed to load Vercel settings" panel was gated on a `hasError` state
  whose setter was never called anywhere, so it was permanently false.
- The "connection expired" banner inside the `connectedProject` branch was
  unreachable: VercelSettingsPresenter only populates `connectedProject` on its
  success exit, which hardcodes `authInvalid: false`. Both `authInvalid: true`
  exits return `connectedProject: undefined`. The banner users actually see is
  the one below that branch, which is untouched.

Dropping them makes the surrounding guards vacuous, so `!showAuthInvalid` and
the `onboardingData?.authInvalid` disjunct go too - the loader already folds
onboarding auth state into `authInvalid` before it reaches the component.

No behaviour change.

TRI-12645
…ment

On the integrations page the Preview row correctly swaps its switch for an
Upgrade button when the project has no preview environment, and the server
neutralises a forged `previewDeploymentsEnabled=on`. The Staging row had
neither: it was always an editable branch input, and `validateStagingBranch`
only checked the branch existed on GitHub. An org without a staging environment
could type a tracking branch, hit Save, get a success toast, and have it do
nothing.

Staging and Preview environments are created together for projects on a plan
that includes them, so gating one and not the other was an oversight, not
policy.

The Staging row now mirrors the Preview row, and the server ignores the
submitted branch when there is no staging environment. It keeps the stored
branch rather than clearing it, so losing the environment never destroys a
tracking branch the org had already configured.

The Vercel config actions had the same gap on the write path: nothing
re-derived the available env slugs server-side, so "stg" and "preview" could be
persisted for a project with neither environment, and the default config turned
preview on unconditionally. Both now filter against the project's actual
environments.

TRI-12646
The integrations page wrapped the Git section, the Vercel section and the build
settings in a single `githubAppEnabled` guard, so with the GitHub app off the
page rendered an empty container.

The Vercel section genuinely depends on GitHub - it cannot sync environment
variables or link deployments without a connected repo - so it stays inside the
guard. Build settings do not: they also apply to CLI deploys run with
--native-build-server, exactly as the section describes. They now render
regardless.

TRI-13488
`computeInitialState` starts in "loading-projects" whenever the org has a Vercel
integration but no onboarding data yet. The effect that escapes that state waits
for `availableProjects !== undefined`, so when `getOnboardingData` returns null -
it does that on any thrown error, and when the org integration row is missing -
nothing ever arrives and the modal spins indefinitely with no explanation.

The route knows the difference between "still loading" and "loaded nothing",
since its fetcher always requests the onboarding data. It now passes that down,
and the modal shows what went wrong plus a way to retry or check the
integration's access on Vercel.

TRI-13488
The four places that ask "does this project have a staging / preview
environment?" disagreed. VercelSettingsPresenter matched on type with no filter
on the parent, so any preview *branch* row satisfied it - branches are PREVIEW
rows too. GitHubSettingsPresenter and ProjectSettingsService matched on slug
instead.

Slug is the weaker key: it is derived at creation time and legacy rows can carry
something else, which is why memberDevelopmentEnvironmentWhere deliberately
avoids it. All four now match on type plus parentEnvironmentId: null, which
excludes branches and does not depend on the slug being canonical.

TRI-13488
…on UI

Follows the two unreachable panels removed in the parent branch. None of this is
reachable either:

- The `"installing"` OnboardingState is unproducible - no setState call ever
  yields it - so its redirect effect, switch arm, isLoadingState conjunct and the
  vercelAppInstallPath import it was the only user of are all dead.
- `(state as string) !== "completed"` is inside a branch where TypeScript has
  already narrowed "completed" out; the cast is what let it compile.
- `hideSectionToggles` was only ever passed alongside layout="settings" but only
  read inside layout="card" blocks, so it could never take effect.
- A handful of unused bindings and the helpers only they referenced:
  envSlugLabel, _formatSelectedEnvs, _CompleteOnboardingForm,
  _handleFinishOnboarding and friends.

No behaviour change.

TRI-13488
The Staging build settings show "Set a Vercel environment for Staging first."
whenever the project has a staging environment and no mapping, but the control
that sets the mapping only rendered when the Vercel project had at least one
custom environment. A project with none - or one whose custom environments could
not be fetched - got an instruction with nothing to act on.

The mapping row now always renders alongside that hint, and says what to do when
there is nothing to choose from. The build settings hint matches.

Also gates the build settings Save on write:github, which the action already
requires. The page admits write:vercel too, so without this a Vercel-only role
could fill the form in and only discover the denial on save.

TRI-13488
@0ski
0ski force-pushed the oskar/fix-vercel-integrations-page branch from 69453b4 to f2a69a7 Compare August 26, 2026 13:08
@pkg-pr-new

pkg-pr-new Bot commented Aug 26, 2026

Copy link
Copy Markdown

Open in StackBlitz

@trigger.dev/build

npm i https://pkg.pr.new/@trigger.dev/build@f2a69a7

trigger.dev

npm i https://pkg.pr.new/trigger.dev@f2a69a7

@trigger.dev/core

npm i https://pkg.pr.new/@trigger.dev/core@f2a69a7

@trigger.dev/python

npm i https://pkg.pr.new/@trigger.dev/python@f2a69a7

@trigger.dev/react-hooks

npm i https://pkg.pr.new/@trigger.dev/react-hooks@f2a69a7

@trigger.dev/redis-worker

npm i https://pkg.pr.new/@trigger.dev/redis-worker@f2a69a7

@trigger.dev/rsc

npm i https://pkg.pr.new/@trigger.dev/rsc@f2a69a7

@trigger.dev/schema-to-json

npm i https://pkg.pr.new/@trigger.dev/schema-to-json@f2a69a7

@trigger.dev/sdk

npm i https://pkg.pr.new/@trigger.dev/sdk@f2a69a7

commit: f2a69a7

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx (1)

62-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add crumb instrumentation for the new integration states.

Add // @Crumbs markers or `#region `@crumbs blocks for the new permission gate, unavailable onboarding state, and disabled save state. Keep the instrumentation on the branch until agentcrumbs strip runs.

As per coding guidelines, “Add crumbs as you write code.”

Also applies to: 228-229, 385-402, 417-417, 589-594

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f7232e3b-4c78-4b4b-aa34-c8587a17f45d

📥 Commits

Reviewing files that changed from the base of the PR and between 69453b4 and f2a69a7.

📒 Files selected for processing (1)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📜 Review details
⏰ Context from checks skipped due to timeout. (34)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
  • GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: runops-guard / runops-guard
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
  • GitHub Check: fk-cascade-guard / fk-cascade-guard
  • GitHub Check: report
  • GitHub Check: code-quality / code-quality
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Build and publish previews
🧰 Additional context used
📓 Path-based instructions (7)
For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.

📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)

Files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx
Add crumbs as you write code — not just when debugging. Mark lines with

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx
Use zod for validation in packages/core and apps/webapp

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

Files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`

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

Files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx
Use function declarations instead of default exports

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

Files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx
Use types over interfaces for TypeScript

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

Files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx

@0ski
0ski enabled auto-merge (squash) August 26, 2026 13:21
@0ski
0ski merged commit 4c16387 into main Aug 26, 2026
53 checks passed
@0ski
0ski deleted the oskar/fix-vercel-integrations-page branch August 26, 2026 13:26
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.

3 participants