Skip to content

fix(webapp): keep branches list pagination when archiving a branch #4723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
isshaddad wants to merge 1 commit into from
+29 −6
Closed

fix(webapp): keep branches list pagination when archiving a branch #4723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .server-changes/preserve-branches-pagination-on-archive.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
area: webapp
type: fix
---

Archiving a preview branch now returns you to the same page of the branches list instead of resetting to the first page.
10 changes: 7 additions & 3 deletions apps/webapp/app/presenters/v3/BranchesPresenter.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ export class BranchesPresenter {
},
});

// Archiving shrinks the list, so a restored page number can point past the end.
const totalPages = Math.ceil(visibleCount / BRANCHES_PER_PAGE);
const currentPage = totalPages > 0 ? Math.min(page, totalPages) : 1;
Comment on lines +184 to +185

@devin-ai-integration devin-ai-integration Bot Aug 19, 2026

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.

🟡 Branches list shows an error page when the page number in the address bar is zero or negative

The requested page number is only capped at the top end (Math.min(page, totalPages) at apps/webapp/app/presenters/v3/BranchesPresenter.server.ts:185) and never given a lower bound, so a page number below one makes the list query ask the database to skip a negative number of rows and the page fails to load.
Impact: A user who edits the page number in the URL to 0 or a negative value sees a generic "Something went wrong" error instead of the branches list.

How a below-range page number reaches the database query

BranchesOptions (apps/webapp/app/utils/branches.ts) coerces page with Number(...) and only rejects NaN, so ?page=0 or ?page=-2 parses successfully and is passed straight to the presenter by both list loaders (apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx:108-114 and the dev-branches loader). With totalPages > 0, currentPage = Math.min(page, totalPages) keeps the non-positive value, giving skip: (currentPage - 1) * BRANCHES_PER_PAGE a negative value, which Prisma rejects; the loader's catch turns that into a 400 response. The new clamp is the natural place to also floor the value at 1 (the PR describes it as covering hand-edited URLs).

Suggested change
const totalPages = Math.ceil(visibleCount / BRANCHES_PER_PAGE);
const currentPage = totalPages > 0 ? Math.min(page, totalPages) : 1;
const totalPages = Math.ceil(visibleCount / BRANCHES_PER_PAGE);
const currentPage = totalPages > 0 ? Math.min(Math.max(Math.trunc(page), 1), totalPages) : 1;
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


Comment on lines +183 to +186

@coderabbitai coderabbitai Bot Aug 19, 2026

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Normalize the no-branchable-environment response.

The clamp at Lines 183-186 is bypassed when branchableEnvironment is absent. That earlier response still returns currentPage: page with totalPages: 0, so a request such as ?page=99 can return inconsistent pagination metadata.

Return currentPage: 1 in that early response, or compute one normalized pagination object before the early return.

Proposed fix 
-        currentPage: page,
+        currentPage: 1,

const limits = await checkBranchLimit({
prisma: this.#prismaClient,
organizationId: project.organizationId,
Expand Down Expand Up @@ -222,7 +226,7 @@ export class BranchesPresenter {
orderBy: {
branchName: "asc",
},
skip: (page - 1) * BRANCHES_PER_PAGE,
skip: (currentPage - 1) * BRANCHES_PER_PAGE,
take: BRANCHES_PER_PAGE,
});

Expand Down Expand Up @@ -252,8 +256,8 @@ export class BranchesPresenter {

return {
branchableEnvironment,
currentPage: page,
totalPages: Math.ceil(visibleCount / BRANCHES_PER_PAGE),
currentPage,
totalPages,
hasBranches: totalBranches > 0,
branches: branchesSorted,
hasFilters,
Expand Down
19 changes: 16 additions & 3 deletions apps/webapp/app/routes/resources.branches.archive.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ const schema = ArchiveBranchOptions.and(
})
);

// Only same-origin paths are safe to redirect to, since redirectPath comes from the form.
function internalRedirectPath(path: string): string | undefined {
return path.startsWith("/") && !path.startsWith("//") ? path : undefined;
}

Comment on lines +28 to +32

@coderabbitai coderabbitai Bot Aug 19, 2026

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.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

file="apps/webapp/app/routes/resources.branches.archive.tsx"
printf '%s\n' '--- relevant source ---'
cat -n "$file" | sed -n '1,100p'

printf '%s\n' '--- related tests/usages ---'
rg -n --glob '*.{ts,tsx,js,jsx}' \
  'internalRedirectPath|resources\.branches\.archive|redirectPath' \
  apps/webapp packages 2>/dev/null | head -200

printf '%s\n' '--- WHATWG URL behavior ---'
node - <<'JS'
const base = "https://internal.invalid";
for (const path of ["/\\evil.example", "/\\/evil.example", "//evil.example", "/safe", "/%5Cevil.example", "/\\`@evil.example`"]) {
  try {
    const url = new URL(path, base);
    console.log(JSON.stringify({path, href: url.href, origin: url.origin}));
  } catch (error) {
    console.log(JSON.stringify({path, error: String(error)}));
  }
}
JS

Repository: triggerdotdev/trigger.dev

Length of output: 15653

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- redirect helpers ---'
rg -n -A35 -B5 \
  'function redirectWith(Error|Success)Message|const redirectWith(Error|Success)Message|export .*redirectWith(Error|Success)Message' \
  apps/webapp/app/models/message.server.ts

printf '%s\n' '--- archive form construction ---'
cat -n apps/webapp/app/routes/resources.branches.archive.tsx | sed -n '100,135p'

Repository: triggerdotdev/trigger.dev

Length of output: 3614

Validate the resolved origin before redirecting.

internalRedirectPath accepts /\evil.example, which resolves to https://evil.example/ under the WHATWG URL parser. Parse path with a fixed same-origin base and compare url.origin with that base before passing it to either redirect helper. Add a regression test for a literal backslash.

export async function action({ request }: ActionFunctionArgs) {
const userId = await requireUserId(request);

Expand All @@ -45,16 +50,24 @@ export async function action({ request }: ActionFunctionArgs) {
);

if (result.success) {
return redirectWithSuccessMessage(
const listPath =
result.branch.type === "DEVELOPMENT"
? branchesDevPath(result.organization, result.project, result.branch)
: branchesPath(result.organization, result.project, result.branch),
: branchesPath(result.organization, result.project, result.branch);

// Return to the page the user archived from, so filters and pagination survive.
return redirectWithSuccessMessage(
internalRedirectPath(submission.value.redirectPath) ?? listPath,
request,
`Branch "${result.branch.branchName}" archived`
);
}

return redirectWithErrorMessage(submission.value.redirectPath, request, result.error);
return redirectWithErrorMessage(
internalRedirectPath(submission.value.redirectPath) ?? "/",
request,
result.error
);
}

export function ArchiveButton({
Expand Down