Repository navigation
fix(pages): keep the named action key out of middleware redirects, rewrites and the page URL - #530
Conversation
…writes and the page URL Follow-ups to #522, which dropped the action key only from c.redirect() inside the action: - Middleware: pages with named actions now mark the request before routing rules, auth and user middleware, so an auth redirect on POST /books?delete&page=2 goes to /login?page=2. - Rewrites: a rewrite that keeps the query keeps the mark, so a handler reached from the action drops the key too. - 307 and 308 keep the key: they send the same request again, and the destination needs it to pick the action. - Page URL: when the action renders the page again, page.url leaves the key out, and the client writes that URL to history instead of /books?delete&page=2. - Docs: the actions guide says what happens to the query. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- plugin-class-a-wrapper: macOS can report server.mjs, written just before the dev server starts, as changed. The reload gave the second import a new Counter. Turn the watcher off. - plugin-vite-environment (#472): same watcher fix, a lighter schema file without drizzle's sqlite-core, and a 30s limit for the real dev server. - plugin-entry-imports: importing the whole plugin entry takes about 4s alone, so give it a 30s limit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2609ff5663
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The marker runs before routing rules, but a redirect rule merged the full request query into its location before c.redirect() saw it, so POST /books?delete&page=2 with a /books -> /login rule still went to /login?delete&page=2. Use the same filtered query as c.redirect(): drop the key for 301/302/303, keep it for 307/308. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31f61a5d22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…handling Platform dispatch applies redirect and rewrite rules before the Worker and knows nothing about pages, so on Void Cloud a rule redirect keeps the whole query. Filtering the key in native rules (31f61a5) made native and hosted deploys answer the same request differently. Revert that change and run the named-action marker after the routing rules again, so every target applies rules to the query as sent, as docs/guide/edge/redirects.md describes. Middleware, actions and c.rewrite() still drop the key. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7615acb309
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…key on repeated methods - The marker is now one global middleware. It looks at the routes Hono matched and marks the request only when a page with named actions is the handler that will answer. A catch-all page pattern no longer drops the first query param from an API route's redirect. - The key goes only when the browser follows with a GET: after a 303, or after a 301/302 to a POST. Fetch repeats PUT, PATCH and DELETE on a 301/302, and the repeated request needs the key to pick the action. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbdb8ee463
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Hono's matchedRoutes() reads the request through a Symbol that each copy of Hono defines for itself. In dev the runtime's bare 'hono/route' import was a different copy from the app's Hono (imported by resolved path), so the marker read undefined and every page request returned 500. The generated app now imports matchedRoutes from the resolved 'hono/route' path, next to Hono, and passes it to createNamedActionMarker(). The runtime no longer imports 'hono/route'. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A one-argument user middleware such as
defineMiddleware(c => c.redirect('/maintenance')) looked like the
answering route, so the request was left unmarked and the redirect
carried the action key. Void now tags the handlers it registers for
routes (API, page GET and page mutation handlers), and the marker skips
everything else, whatever its arity.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2ce1236c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…andlers With a catch-all page that has named actions, the marker skipped the untagged auth (/api/auth/*), internal (/__void/*) and WebSocket fallback handlers and marked the request for the page, so a global middleware redirect dropped the first query param of an auth callback. Every terminal route the generated app registers is now tagged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff46d9f70f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…he request (#533) Follow-up to #530 (Codex finding on that PR, which got merged before this fix landed). ## Problem A redirect that sends the same request again keeps the action key (307/308, or 301/302 after PUT/PATCH/DELETE). When the destination has its own query, the merge put the key **after** the destination's params: ``` PUT /books?delete&page=2 → c.redirect('/books-v2?tab=details', 302) → /books-v2?tab=details&delete=&page=2 repeated PUT picks "tab" ✗ ``` ## Fix (only in the `c.redirect` wrapper) | Destination | Result | |---|---| | `/books-v2?tab=details` (another page) | `/books-v2?delete&tab=details&page=2` (key first, as sent) | | `/books-v2?tab=1&delete=x` (sets the key too) | `/books-v2?delete=x&tab=1&page=2` (its value wins, moves first) | | `/books?archive` (same page, own first key) | `/books?archive=&page=2` (that key picks the action) | | `/books-v2?` (bare reset) | `/books-v2?delete` (rest dropped, key kept) | | `/books-v2` (no query) | `/books-v2?delete&page=2` (unchanged, byte for byte) | | `#fragment` | stays last | Unchanged: GET follow-ups (key dropped), unmarked requests, `mergeQueryIntoLocation`, redirect rules and platform dispatch. Known limit: a redirect to a **different** page whose first query key is one of that page's actions (`c.redirect('/other?archive', 307)`) still puts the request's key first. The redirect code can't tell an action key from a normal param on another page. ## Tests `routing-runtime.test.ts`: 10 new rows in the named-action table (307 and PUT 302 with a destination query, a valued key, a fragment, a bare `?`, the same key in the destination, a same-page action key). 9 fail on main. The POST 302 row passes on main too: it checks the GET follow-up stays unchanged. routing-runtime, pages-client, pages-compile and compile tests: 205/205. Lint, format and typecheck are clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 participant
Follow-ups to #522. That PR dropped the action key (
?delete) only fromc.redirect()inside the action. This covers the other places it leaked.What changes
POST /books?delete&page=2c.redirect('/login')/login?delete&page=2/login?page=2c.rewrite('/api/x'), thenc.redirect('/done')/done?delete&page=2/done?page=2c.redirect('/books-v2', 307)(or 308)/books-v2?page=2(since #522)/books-v2?delete&page=2PUT /books?update+c.redirect('/books')(302)/books(since #522)/books?updatePOST /api/x?return=/a+ redirect?returndropped (earlier commit here)?return=/akeptpage.url/books?delete&page=2/books?page=2useForm/action()/books?delete&page=2/books?page=2The key goes only when the browser follows with a GET (303, or 301/302 after a POST). Other redirects send the same method again, and the destination needs the key to pick the action.
Redirect and rewrite rules (
void.jsonrouting) are unchanged: they keep the query as sent. On Void Cloud, platform dispatch applies them before the Worker and knows nothing about pages, so every target leaves them alone.How
Docs: the actions guide now says what happens to the query. The rewrites reference mentions middleware and 307/308.
Flaky tests (second commit)
Each of these failed in a local full run under load:
plugin-class-a-wrapper… during buildserver.mjs, written just before startup, as changed (2 of 6 runs in a probe). The reload gives the second import a newCounter.watch: null(12 of 12 runs stable)plugin-vite-environment#472 testwatch: null, lighter schema file, 30 s limitplugin-entry-importsTests
routing-runtime.test.ts: middleware, 307, rewrite, rewrite with its own query, and page URL. 5 rows fail on main.pages-client.test.ts: history URL after a POST, 4 rows. 2 fail on main; the other 2 check that single-action URLs stay unchanged.pages-compile.test.ts: the marker is emitted afterroutingMiddlewareand before user middleware, only foractions.vp run test --unit: all pass, except onecf-version-runnertest that timed out under load and passes alone (not touched here). Lint, format and typecheck are clean.🤖 Generated with Claude Code