Specifying branch name as a path param instead of a query param

AGENTS.md

@@ -73,7 +73,7 @@ npx eslint "lib/**/*.js" "app/**/*.{js,jsx}" "components/**/*" "tests/**/*"
 ```
 app/                        # Next.js App Router (static export)
   layout.jsx                # CSP meta, header/footer, submission-target meta
-  not-found.jsx             # 404 page + branch deep-link redirect (§6)
+  not-found.jsx             # 404 page + in-place branch deep-link render (§6)
   error.jsx                 # global error boundary (+ per-route boundaries)
   page.jsx                  # landing (/)
   add-repo|about|api|.../page.jsx    # static content pages
@@ -139,8 +139,9 @@ coverage there when introducing new markup patterns.
 - **Static export constraints:** `dynamicParams = false` on dynamic routes;
   `generateStaticParams` enumerates `(username)`, `(username, repository)` and
   `(username, repository, main|master)` from `repositories.txt`; new repos render
-  client-side immediately thanks to the client-side listing refresh and the
-  `?branch=` deep-link redirect in `app/not-found.jsx`.
+  client-side immediately thanks to the client-side listing refresh, and
+  branch deep links (the branch is always a path parameter) are rendered
+  in place by the 404 shell (`app/not-found.jsx`) at their original URL.
 - **Widget loading:** CDN scripts load via `next/script` `lazyOnload` inside
   `components/report-view.jsx`, registered through `lib/widget-loader.js`);
   wasm/ESM bridges in `public/widgets/` run as native module scripts.

README.md

@@ -268,7 +268,7 @@ GitLab Pages deploys from the `pages` job and serves
 ### 4. GitLab-specific considerations
 
 - **Client-side routing**: GitLab Pages serves `404.html` for unknown paths; the Next.js export already produces
-  `out/404.html` (from `app/not-found.jsx`), which handles branch deep-link recovery — no extra copy step needed.
+  `out/404.html` (from `app/not-found.jsx`), which renders branch deep links in place at their path URLs — no extra copy step needed.
 - **Submission processing**: set `submission-target` in `app/layout.jsx` to your
   `<group>/<project>`, extend the CSP `connect-src` with your GitLab base (`https://gitlab.com` or your self-managed
   host), and create a pipeline schedule (**CI/CD → Schedules**, e.g. every 10 minutes) — GitLab has no issue-triggered

app/[username]/[repository]/[branch]/page.jsx

@@ -1,7 +1,7 @@
 // /{username}/{repository}/{branch} — report shell pinned to a branch.
-// Pre-generated for main/master; other branches reach this route via the
-// not-found client redirect which conveys the branch through the ?branch=
-// query parameter handled by ReportView.
+// Pre-generated for main/master; other branches have no exported HTML and
+// are rendered in place by the 404 shell (app/not-found.jsx) at the
+// original path-parameter URL.
 
 import { Suspense } from 'react';
 import { notFound } from 'next/navigation';

app/layout.jsx

@@ -54,11 +54,11 @@ export default function RootLayout({ children }) {
             vertically centered on one line. The row reuses the breadcrumbs'
             alignment formula (--width-content column, 0.8rem horizontal
             padding, border-box), so the first crumb's glyph sits at the menu
-            bar's left content edge and the toggle at its right content edge
-            — the two edges mirror each other (plans/css-only-dark-mode.md).
-            Suspense is required for useSearchParams (static export; the
-            fallback keeps prerendered HTML crumb-free and the toggle still
-            renders — an empty flex slot). */}
+             bar's left content edge and the toggle at its right content edge
+             — the two edges mirror each other (plans/css-only-dark-mode.md).
+             The Suspense boundary mirrors the fallback-free static-export
+             pattern (the prerendered HTML stays crumb-free and the toggle
+             still renders — an empty flex slot). */}
         <div className="theme-bar">
           <Suspense fallback={null}>
             <Breadcrumbs />

app/not-found.jsx

@@ -1,23 +1,25 @@
 'use client';
 
-import { useEffect } from 'react';
+import { Suspense, lazy, useEffect } from 'react';
 import Link from 'next/link';
 import { usePathname } from 'next/navigation';
 import { isValidGitHubName, STATIC_PAGES } from '../lib/routes.js';
 import { getBasePath } from '../lib/base-path';
 
+// Lazy-loaded so the report-rendering bundle (renderer, table enhancement)
+// stays out of the first-party budget of pages that never show a report —
+// the chunk is only fetched when a branch deep link actually renders one.
+const ReportView = lazy(() => import('../components/report-view'));
+
 // Exported for the 404.html produced by the static export. GitHub Pages (and
 // similar static hosts) serve this page for any path that has no exported
 // HTML file. Two recovery behaviors:
 //  - Branch deep links (/user/repo/<branch>) that are not pre-generated are
-//    redirected to the repo shell (/user/repo/) which loads the requested
-//    branch client-side (Technical Appendix §6).
+//    rendered in place by ReportView at the original URL — the branch is a
+//    path parameter, so no redirect is needed (Technical Appendix §6).
 //  - Repos added after the last deploy get the "recently added" note
 //    (Technical Appendix §5).
-//
-// `performRedirect` is injectable for tests (window.location is not
-// configurable under jsdom).
-export default function NotFound({ performRedirect }) {
+export default function NotFound() {
   const pathname = usePathname() || '';
   const basePath = getBasePath();
   const path = basePath && pathname.startsWith(basePath)
@@ -33,21 +35,32 @@ export default function NotFound({ performRedirect }) {
     isValidGitHubName(username) &&
     isValidGitHubName(repository);
 
-  // Branch deep links are redirected to the statically generated repo shell;
-  // ReportView picks the requested branch back up from the query parameter
-  // (Technical Appendix §6). Branch names may contain slashes, so any
-  // segments beyond user/repo belong to the branch name.
-  const redirectTarget =
-    segments.length >= 3 && isRepoLike
-      ? `/${username}/${repository}/?branch=${encodeURIComponent(segments.slice(2).join('/'))}`
-      : null;
+  // Branch names may contain slashes, so any segments beyond user/repo
+  // belong to the branch name. Each segment must pass the same validation
+  // as classifyRoute's branch guard: the branch is interpolated into raw
+  // content URLs, so path-injection values must fall through to the 404
+  // copy instead of being fetched.
+  const branchSegments = segments.slice(2);
+  const isBranchDeepLink =
+    segments.length >= 3 &&
+    isRepoLike &&
+    branchSegments.every(segment => isValidGitHubName(segment));
 
+  // The exported 404.html statically carries the site title; report shells
+  // pre-generate `<user>/<repo>@<branch>` titles, so match them here.
   useEffect(() => {
-    const redirect = performRedirect || (url => window.location.replace(url));
-    if (redirectTarget) {
-      redirect(`${basePath}${redirectTarget}`);
+    if (isBranchDeepLink) {
+      document.title = `${username}/${repository}@${branchSegments.join('/')} - RefactorFirst`;
     }
-  }, [redirectTarget, basePath, performRedirect]);
+  }, [isBranchDeepLink, username, repository, branchSegments]);
+
+  if (isBranchDeepLink) {
+    return (
+      <Suspense fallback={<p className="loading" role="status">Loading report&hellip;</p>}>
+        <ReportView username={username} repository={repository} branch={branchSegments.join('/')} />
+      </Suspense>
+    );
+  }
 
   return (
     <section className="error-page error-404" role="alert">

components/breadcrumbs.jsx

@@ -10,7 +10,7 @@
 // to the resolved branch so the trail identifies the displayed report.
 import { useEffect, useState } from 'react';
 import Link from 'next/link';
-import { usePathname, useSearchParams } from 'next/navigation';
+import { usePathname } from 'next/navigation';
 import {
   breadcrumbsFor,
   reportMatchesFor,
@@ -25,9 +25,7 @@ import {
  */
 export default function Breadcrumbs() {
   const pathname = usePathname();
-  const searchParams = useSearchParams();
-  const search = searchParams ? searchParams.toString() : '';
-  const routeKey = `${pathname}?${search}`;
+  const routeKey = pathname || '/';
   // The announced branch is keyed to the route it was announced on; a route
   // change resets it during render (the React-sanctioned alternative to a
   // setState call inside an effect).
@@ -55,7 +53,6 @@ export default function Breadcrumbs() {
 
   const crumbs = breadcrumbsFor(
     pathname || '/',
-    search,
     resolved.key === routeKey ? resolved.branch : null
   );
   if (crumbs.length === 0) return null;

components/report-view.jsx

@@ -9,7 +9,6 @@
 // lib/widget-loader.js.
 
 import { useCallback, useEffect, useRef, useState } from 'react';
-import { useSearchParams } from 'next/navigation';
 import Script from 'next/script';
 import { withBasePath } from '../lib/base-path';
 import { detectHostingEnvironment, getPlatformBaseUrl, readMetaTag } from '../lib/host';
@@ -66,7 +65,6 @@ export default function ReportView({
   enhanceTablesImpl = enhanceTables
 }) {
   const containerRef = useRef(null);
-  const searchParams = useSearchParams();
   const [attempt, setAttempt] = useState(0);
   const [payload, setPayload] = useState(null);
   const [tableStates, setTableStates] = useState({});
@@ -80,7 +78,9 @@ export default function ReportView({
     duration: toastDurationMs
   });
 
-  const branch = branchProp || searchParams.get('branch') || undefined;
+  // The branch comes exclusively from the route (pre-generated shells pass
+  // it as a prop; the 404 shell renders unlisted branches in place).
+  const branch = branchProp || undefined;
 
   const handleTableAction = useCallback((tableId, patch, meta) => {
     if (meta?.restoreFocus) {

lib/breadcrumbs.js

@@ -4,7 +4,7 @@
 // when the only sensible link target would be the page itself (crumbs never
 // self-link). Routes that are not a user listing or repository report get no
 // trail (empty array), so landing and static content pages stay crumb-free.
-import { classifyRoute, buildReportUrl, getQueryParam } from './routes.js';
+import { classifyRoute, buildReportUrl } from './routes.js';
 
 // CustomEvent dispatched by ReportView once the report fetch resolves the
 // branch actually loaded (a `main` 404 falls back to `master`). Breadcrumbs
@@ -79,13 +79,12 @@ export function reportMatchesFor(path, detail = {}) {
  * Builds the breadcrumb trail for a path.
  *
  * @param {string} path - Current pathname (basePath already stripped by Next).
- * @param {string} [search] - Query string conveying ?branch= on deep links.
  * @param {string|null} [resolvedBranch] - Branch the report actually loaded
  *   from (announced by ReportView after the fetch; null beforehand).
  * @returns {Array<{label: string, href?: string, current?: boolean}>} Crumbs
  *   in hierarchical order; the last crumb is the current page.
  */
-export function breadcrumbsFor(path, search = '', resolvedBranch = null) {
+export function breadcrumbsFor(path, resolvedBranch = null) {
   const route = classifyRoute(path);
 
   if (route.type === 'user') {
@@ -97,9 +96,7 @@ export function breadcrumbsFor(path, search = '', resolvedBranch = null) {
 
   if (route.type === 'report') {
     const hasBranchSegment = path.split('/').filter(Boolean).length > 2;
-    const explicitBranch = hasBranchSegment
-      ? route.branch
-      : getQueryParam(search, 'branch');
+    const explicitBranch = hasBranchSegment ? route.branch : null;
     // The resolved branch outranks any explicit/requested branch: when main
     // 404s and the fallback loads master, the label must say "master".
     const branch = resolvedBranch || explicitBranch || null;
@@ -117,9 +114,9 @@ export function breadcrumbsFor(path, search = '', resolvedBranch = null) {
       return crumbs;
     }
 
-    // With an explicit branch (path segment or ?branch=), the repository
-    // crumb navigates back up to the default-branch report. Without one the
-    // only target would be the current page, so the crumb stays unlinked.
+    // With an explicit branch path segment, the repository crumb navigates
+    // back up to the default-branch report. Without one the only target
+    // would be the current page, so the crumb stays unlinked.
     crumbs.push(
       explicitBranch
         ? { label: route.repository, href: buildReportUrl(route.username, route.repository) }

lib/routes.js

@@ -25,7 +25,9 @@ export function parseRoute(path) {
   return {
     username: parts[0] || null,
     repository: parts[1] || null,
-    branch: parts[2] || DEFAULT_BRANCH
+    // Join all branch segments, matching classifyRoute (paths such as
+    // /alice/repo/feature/login); default when none are present.
+    branch: parts.length > 2 ? parts.slice(2).join('/') : DEFAULT_BRANCH
   };
 }
 
@@ -44,7 +46,7 @@ export function isValidGitHubName(name) {
 
 // Classify a path into a route the application knows how to render.
 export function classifyRoute(path) {
-  const { username, repository, branch } = parseRoute(path);
+  const { username, repository } = parseRoute(path);
 
   if (!username) {
     return { type: 'landing' };
@@ -71,33 +73,35 @@ export function classifyRoute(path) {
   }
 
   const parts = path.split('/').filter(Boolean);
-  if (parts.length > 3) {
-    return { type: 'not-found' };
-  }
 
-  // Path-injection guard: the branch segment is interpolated into raw
-  // content URLs; reject values like ".." that browsers would normalize
-  // into a different path. (Multi-segment branches are not expressible in
-  // classifyRoute paths — those arrive via ?branch= on the shell.)
-  if (!isValidGitHubName(branch)) {
+  // Branch names may contain slashes, so every segment beyond user/repo
+  // belongs to the branch name. Path-injection guard: each segment is
+  // interpolated into raw content URLs, so reject values like ".." that
+  // browsers would normalize into a different path.
+  const branchParts = parts.slice(2);
+  if (branchParts.some(part => !isValidGitHubName(part))) {
     return { type: 'not-found' };
   }
 
-  return { type: 'report', username, repository, branch };
+  return {
+    type: 'report',
+    username,
+    repository,
+    branch: branchParts.length ? branchParts.join('/') : DEFAULT_BRANCH
+  };
 }
 
 export function buildReportUrl(username, repository, branch = DEFAULT_BRANCH) {
   const base = `/${encodeURIComponent(username)}/${encodeURIComponent(repository)}`;
-  return branch === DEFAULT_BRANCH ? base : `${base}/${encodeURIComponent(branch)}`;
+  if (branch === DEFAULT_BRANCH) return base;
+  // Encode each slash-delimited component while preserving the separators:
+  // a slashed branch like "feature/login" must stay real path segments so
+  // classifyRoute and the 404-shell deep-link render recognize the URL.
+  const branchPath = branch.split('/').map(encodeURIComponent).join('/');
+  return `${base}/${branchPath}`;
 }
 
 export function buildRepositoryListUrl(fullName) {
   const [username, repository] = String(fullName).split('/');
   return buildReportUrl(username, repository);
 }
-
-export function getQueryParam(search, name) {
-  const params = new URLSearchParams(search || '');
-  const value = params.get(name);
-  return value === null ? null : value;
-}

lib/static-params.js

@@ -12,7 +12,7 @@ export function reportStaticParams(repositories) {
 }
 
 // Branch pages are pre-generated for the common default branches; any other
-// branch deep link is handled client-side by the not-found redirect.
+// branch deep link is rendered in place client-side by the 404 shell.
 export function branchStaticParams(repositories) {
   return repositories.flatMap(({ username, repository }) =>
     ['main', 'master'].map(branch => ({ username, repository, branch }))

tests/e2e/breadcrumbs.spec.js

@@ -71,8 +71,10 @@ test('a main 404 falls back to master and the trail relabels to the displayed br
   await expect(nav.locator('[aria-current="page"]')).toHaveText('master');
 });
 
-test('branch deep link (?branch=) shows the deep-linked branch', async ({ page }) => {
-  await page.goto('/refactorfirst/refactorfirst/?branch=feature%2Flogin');
+test('branch deep link shows the deep-linked branch from the path', async ({ page }) => {
+  // Served by the 404 shell (feature/login is not pre-generated) — the
+  // branch path segments are joined into one crumb label.
+  await page.goto('/refactorfirst/refactorfirst/feature/login');
   const nav = page.getByRole('navigation', { name: 'Breadcrumb' });
   await expect(nav.locator('[aria-current="page"]')).toHaveText('feature/login');
 });

tests/e2e/static-export-parity.spec.js

@@ -3,8 +3,8 @@ import { readFileSync } from 'node:fs';
 
 // Static-export parity (Technical Appendix §9): every repositories.txt entry
 // must resolve to generated HTML; unknown paths serve the app 404 with a
-// real 404 status; branch deep links render the requested branch after the
-// client-side redirect.
+// real 404 status; branch deep links render the requested branch in place
+// on the 404 shell, keeping the path-parameter URL.
 
 const repositories = readFileSync('repositories.txt', 'utf8')
   .split('\n')
@@ -48,7 +48,7 @@ const SAMPLE_REPORT = {
   classCycles: { hasCycles: false, summary: [], largestCycle: { hasCycleMap: false } }
 };
 
-test('branch deep link redirects to the repo shell and renders that branch', async ({ page }) => {
+test('branch deep link renders the requested branch in place, keeping the URL', async ({ page }) => {
   const requested = [];
   await page.route('**/raw.githubusercontent.com/**', route => {
     const url = route.request().url();
@@ -61,10 +61,29 @@ test('branch deep link redirects to the repo shell and renders that branch', asy
   });
 
   await page.goto('/refactorfirst/refactorfirst/develop');
-  // Client redirect rewrites the URL to the repo shell carrying the branch
-  await page.waitForURL('**/refactorfirst/refactorfirst/**?branch=develop**');
+  // No redirect: the report renders on the 404 shell at the original URL
+  await expect(page).toHaveURL(/.*\/refactorfirst\/refactorfirst\/develop\/?$/);
   await expect(page.locator('h1').first()).toContainText('RefactorFirst Report for refactorfirst');
   expect(requested.some(u => u.includes('/develop/.refactorfirst/refactor-first.json'))).toBe(true);
+  // The 404 copy must not co-render with the report
+  await expect(page.locator('.error-page')).toHaveCount(0);
+  // Title matches the pre-generated branch shells
+  await expect(page).toHaveTitle('refactorfirst/refactorfirst@develop - RefactorFirst');
+});
+
+test('branch deep links with slashes render the full branch name', async ({ page }) => {
+  await page.route('**/raw.githubusercontent.com/**', route => {
+    const url = route.request().url();
+    if (url.includes('/refs/heads/feature/login/.refactorfirst/refactor-first.json')) {
+      route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(SAMPLE_REPORT) });
+    } else {
+      route.fulfill({ status: 404 });
+    }
+  });
+
+  await page.goto('/refactorfirst/refactorfirst/feature/login');
+  await expect(page).toHaveURL(/.*\/refactorfirst\/refactorfirst\/feature\/login\/?$/);
+  await expect(page.locator('h1').first()).toContainText('RefactorFirst Report for refactorfirst');
 });
 
 test('recently added repo note shows for unlisted /user/repo 404s', async ({ page }) => {