fix(pages): keep the named action key first when a redirect repeats the request

packages/void/src/pages/action-key.ts

@@ -11,6 +11,16 @@ export function hasActionKey(request: Request): boolean {
   return actionKeyRequests.has(request);
 }
 
+/** The first non-empty segment of `search` as sent: `delete` in `?delete&page=2`. */
+export function actionKeySegment(search: string): string {
+  let start = 1;
+  while (search[start] === '&') {
+    start++;
+  }
+  const end = search.indexOf('&', start);
+  return search.slice(start, end === -1 ? search.length : end);
+}
+
 /** `search` without its first non-empty segment; the rest is kept as sent. */
 export function searchWithoutActionKey(search: string): string {
   let start = 1;

packages/void/src/runtime/routing.ts

@@ -8,7 +8,12 @@ import {
 } from '@void/edge';
 import type { Context, Hono, MiddlewareHandler } from 'hono';
 import type { RedirectStatusCode } from 'hono/utils/http-status';
-import { hasActionKey, markActionKey, searchWithoutActionKey } from '../pages/action-key.ts';
+import {
+  actionKeySegment,
+  hasActionKey,
+  markActionKey,
+  searchWithoutActionKey,
+} from '../pages/action-key.ts';
 import { VoidAssetRewriteError } from './handler.ts';
 import { convertReturnValue } from './response.ts';
 import { ASSET_EXTENSIONS } from './routing-assets.ts';
@@ -179,10 +184,7 @@ export function createRoutingRuntime(
     const redirect = c.redirect.bind(c);
     c.redirect = (location, status) => {
       if (typeof location === 'string' && location.startsWith('/') && !location.startsWith('//')) {
-        return redirect(
-          mergeQueryIntoLocation(location, redirectSearch(c.req.raw, status)),
-          status,
-        );
+        return redirect(redirectLocation(c.req.raw, location, status), status);
       }
       return redirect(location, status);
     };
@@ -252,15 +254,66 @@ export function createRoutingRuntime(
   };
 }
 
-function redirectSearch(request: Request, status: RedirectStatusCode | undefined): string {
-  const search = new URL(request.url).search;
+function redirectLocation(
+  request: Request,
+  location: string,
+  status: RedirectStatusCode | undefined,
+): string {
+  const url = new URL(request.url);
+  const search = url.search;
+  if (!hasActionKey(request)) {
+    return mergeQueryIntoLocation(location, search);
+  }
   // The key goes only when the browser follows with a GET: after a 303, or after
   // a 301/302 (the default) to a POST. Other redirects send the same method and
   // body again, and the destination needs the key to pick the action.
   const followsWithGet =
     status === 303 ||
     ((status === undefined || status === 301 || status === 302) && request.method === 'POST');
-  return hasActionKey(request) && followsWithGet ? searchWithoutActionKey(search) : search;
+  if (followsWithGet) {
+    return mergeQueryIntoLocation(location, searchWithoutActionKey(search));
+  }
+  const hash = location.indexOf('#');
+  const destinationQuery = location.indexOf('?');
+  if (destinationQuery === -1 || (hash !== -1 && hash < destinationQuery)) {
+    return mergeQueryIntoLocation(location, search);
+  }
+  const merged = mergeQueryIntoLocation(location, searchWithoutActionKey(search));
+  // On the same page, a key the destination starts with picks the action itself.
+  const destinationKey = actionKeySegment(
+    location.slice(destinationQuery, hash === -1 ? location.length : hash),
+  );
+  if (destinationKey && new URL(location, url).pathname === url.pathname) {
+    return merged;
+  }
+  // Elsewhere the key goes first again, ahead of the destination parameters; a
+  // value the destination gives the same key wins. A bare "?" still drops the
+  // rest of the request query.
+  const mergedHash = merged.indexOf('#');
+  const base = mergedHash === -1 ? merged : merged.slice(0, mergedHash);
+  const fragment = mergedHash === -1 ? '' : merged.slice(mergedHash);
+  const q = base.indexOf('?');
+  const segments = q === -1 ? [] : base.slice(q + 1).split('&');
+  const key = actionKeySegment(search);
+  const name = queryKey(key);
+  const hasName = (segment: string) => segment !== '' && queryKey(segment) === name;
+  // Only the destination's own value replaces the key. The merge then left out
+  // the request's values for that name and put the destination's first.
+  const own = location
+    .slice(destinationQuery + 1, hash === -1 ? location.length : hash)
+    .split('&')
+    .some(hasName)
+    ? segments.findIndex(hasName)
+    : -1;
+  const first = own === -1 ? key : segments.splice(own, 1)[0];
+  const rest = segments.join('&');
+  return (
+    base.slice(0, q === -1 ? base.length : q) + '?' + first + (rest ? '&' + rest : '') + fragment
+  );
+}
+
+function queryKey(segment: string): string | undefined {
+  return new URLSearchParams(segment).keys().next().value;
 }
 
 /** A rewrite that keeps the query keeps the action key in it too. */

packages/void/test/unit/routing-runtime.test.ts

@@ -511,6 +511,14 @@ describe('typed Pages and SSR execution', () => {
               delete: (c: Context) => c.redirect('/named'),
               update: (c: Context) => c.redirect('/named', 303),
               move: (c: Context) => c.redirect('/named-v2', 307),
+              moveTab: (c: Context) => c.redirect('/named-v2?tab=details', 307),
+              moveHash: (c: Context) => c.redirect('/named-v2?tab=details#top', 307),
+              moveReset: (c: Context) => c.redirect('/named-v2?', 307),
+              replace: (c: Context) => c.redirect('/named-v2?tab=details', 302),
+              moveKey: (c: Context) => c.redirect('/named-v2?tab=1&moveKey=x', 307),
+              moveSame: (c: Context) => c.redirect('/named-v2?moveSame', 307),
+              toArchive: (c: Context) => c.redirect('/named?archive', 307),
+              toArchiveHere: (c: Context) => c.redirect('/named?', 307),
               go: (c: Context) => c.rewrite('/api/x'),
               goWithQuery: (c: Context) => c.rewrite('/api/x?q=1'),
               save: () => undefined,
@@ -525,6 +533,32 @@ describe('typed Pages and SSR execution', () => {
     it.each([
       ['POST', '/named?delete&page=2', 'middleware', '/login?page=2'],
       ['POST', '/named?move&page=2', 'a 307 redirect', '/named-v2?move&page=2'],
+      ['POST', '/named?moveTab&page=2', 'a 307 redirect', '/named-v2?moveTab&tab=details&page=2'],
+      [
+        'POST',
+        '/named?moveTab&moveTab=secondary&page=2',
+        'a 307 redirect',
+        '/named-v2?moveTab&tab=details&moveTab=secondary&page=2',
+      ],
+      ['PUT', '/named?replace&page=2', 'a 302 redirect', '/named-v2?replace&tab=details&page=2'],
+      [
+        'PUT',
+        '/named?&replace=1&page=2',
+        'a 302 redirect',
+        '/named-v2?replace=1&tab=details&page=2',
+      ],
+      ['POST', '/named?replace&page=2', 'a 302 redirect', '/named-v2?tab=details&page=2'],
+      [
+        'POST',
+        '/named?moveHash&page=2',
+        'a 307 redirect',
+        '/named-v2?moveHash&tab=details&page=2#top',
+      ],
+      ['POST', '/named?moveReset&page=2', 'a 307 redirect', '/named-v2?moveReset'],
+      ['POST', '/named?moveKey&page=2', 'a 307 redirect', '/named-v2?moveKey=x&tab=1&page=2'],
+      ['POST', '/named?moveSame', 'a 307 redirect', '/named-v2?moveSame'],
+      ['PUT', '/named?toArchive&page=2', 'a 307 redirect', '/named?archive=&page=2'],
+      ['PUT', '/named?toArchiveHere&page=2', 'a 307 redirect', '/named?toArchiveHere'],
       ['PUT', '/named?delete&page=2', 'a 302 redirect', '/named?delete&page=2'],
       ['PUT', '/named?update&page=2', 'a 303 redirect', '/named?page=2'],
       ['POST', '/named?go&page=2', 'a rewrite', '/done?page=2'],

platform/packages/api/test/integration/email-admission.test.ts

@@ -345,6 +345,8 @@ describe('authoritative email admission', () => {
               payloadDigest: 'f'.repeat(64),
               route: receiptRoute(),
             });
+      // It can fail before the assertion below listens; that is not an unhandled rejection.
+      receive.catch(() => {});
       try {
         await waitForPostgresLockWaiter();
         releaseDisable();
@@ -1185,6 +1187,8 @@ describe('authoritative email admission', () => {
               payloadDigest: 'd'.repeat(64),
               route: receiptRoute(),
             });
+      // It can fail before the assertion below listens; that is not an unhandled rejection.
+      start.catch(() => {});
       try {
         await waitForPostgresLockWaiter();
         releaseSuspension();