fix(web): harden non-HTML navigation routing - #186
Merged
Merged
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 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. Comment |
Reviewer's GuideHardens Worker-first routing and request classification so server-function, non-HTML legacy, extension, and locale-error requests cannot fall through to SPA document redirects, while preserving expected HTML/API behavior and adding localized Home navigation regression coverage. Sequence diagram for hardened server-function routingsequenceDiagram
participant Client
participant Worker
participant Handler
participant SPA
Client->>Worker: POST /_serverFn/{path}
alt valid function id
Worker->>Handler: Forward RPC request
Handler-->>Worker: Server-function response
Worker-->>Client: RPC response
else malformed function path
Worker-->>Client: JSON 404 server_function_not_found
end
Note over Worker,SPA: Server-function paths do not fall through to SPA document routing
Sequence diagram for non-HTML navigation classificationsequenceDiagram
participant Client
participant Worker
participant DocumentRouter
Client->>Worker: Request legacy story or /extension
alt HTML request
Worker->>DocumentRouter: Apply locale or legacy redirect
DocumentRouter-->>Client: HTML redirect
else non-HTML request
Worker-->>Client: JSON response
end
Client->>Worker: Non-HTML request with invalid locale
Worker-->>Client: JSON locale error
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
duyetbot
force-pushed
the
fix/home-navigation-hydration-20260925
branch
from
September 25, 2026 16:20
0e977f6 to
003fa6a
Compare
duyetbot
pushed a commit
that referenced
this pull request
Sep 25, 2026
Two review follow-ups on #186, on top of the #181 transport fix. A. Server-function ids are validated against Start's real manifest The previous pass only rejected empty and nested transport paths. Any single-segment id still reached `getServerFnById`, and an id that is not in the build-generated manifest made that throw an unhandled error: a 500 whose message embeds the requested id, skipping Start's serialized-envelope contract entirely. That covers stale ids from a renamed function still held by an already-served client bundle, and every percent-encoded traversal form, none of which the old shape check could see. - `server-fn-registry.ts` (new, server-only) loads the same generated resolver the handler uses and asks it whether an id is registered. A resolver that cannot be loaded answers `null`, and the Worker then defers to Start, so an infrastructure failure can never turn a working RPC call into a 404. For a registered id the lookup warms the resolver's own module cache, so the handler's later lookup costs nothing extra. - `serverFnIdOf` replaces the shape-only check with a charset that covers both alphabets Start's compiler emits -- `base64url` in dev, `sha256` hex in a build, `_N` on a dedup collision -- and retires `.`, `..`, `%`, encoded separators, and over-long segments before a resolver is consulted. - The bare base `/_serverFn` is now bounded too. Start treats it as an ordinary unknown route, so it used to hand a document back to a request that claimed to be RPC. - Every rejection is the same fixed `404 server_function_not_found` JSON with no id, no stack, and no function detail. Auth is untouched: the bounded 404 returns before Start's middleware, and it discriminates only on structure and manifest membership, never on a credential. - The `run_worker_first` entry is now documented as what it is -- pinning the transport to the Worker so the bounded 404 holds for a navigation request too. It is not the fix: a non-navigation request to a non-asset path already reaches the Worker, and the array form already disables `assets_navigation_prefers_asset_serving`. Measured delta over 80 differential probes against master: zero. B. The logo link test exercises the real router `Brand.test.tsx` mocked `Link`, so its `not.toContain("payload")` and `not.toContain("locale")` assertions only exercised the mock's own `URLSearchParams` logic. A `payload` param leaking through the root search validation passed the old test and fails the new one. It now drives a real TanStack router (memory history, the app's real `validateRootSearch` and `preserveRootLangFromMiddleware`, `Brand` mounted as a layout above `Outlet` the way `HeaderBar` mounts it) and reads the href the real `Link` builds. It also asserts the href against `router.buildLocation`, clicks the link and checks the resulting location, and covers the VI-copy/EN-navigation split. Test counts (measured, not estimated): 150 files / 1511 web tests, 10 files / 122 focused, 61 extension. Verified against the real production bundle under workerd with the seven manifest ids: 54/54 transport assertions and 29/29 EN/VI navigation assertions, including `Sign in required` still arriving as Start's serialized envelope with no Worker cache, Content-Language or robots policy stamped on it. Refs #180 Related: #181
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
duyetbot
pushed a commit
that referenced
this pull request
Sep 25, 2026
Two review follow-ups on #186, on top of the #181 transport fix. A. Server-function ids are validated against Start's real manifest The previous pass only rejected empty and nested transport paths. Any single-segment id still reached `getServerFnById`, and an id that is not in the build-generated manifest made that throw an unhandled error: a 500 whose message embeds the requested id, skipping Start's serialized-envelope contract entirely. That covers stale ids from a renamed function still held by an already-served client bundle, and every percent-encoded traversal form, none of which the old shape check could see. - `server-fn-registry.ts` (new, server-only) loads the same generated resolver the handler uses and asks it whether an id is registered. A resolver that cannot be loaded answers `null`, and the Worker then defers to Start, so an infrastructure failure can never turn a working RPC call into a 404. For a registered id the lookup warms the resolver's own module cache, so the handler's later lookup costs nothing extra. - `serverFnIdOf` replaces the shape-only check with a charset that covers both alphabets Start's compiler emits -- `base64url` in dev, `sha256` hex in a build, `_N` on a dedup collision -- and retires `.`, `..`, `%`, encoded separators, and over-long segments before a resolver is consulted. - The bare base `/_serverFn` is now bounded too. Start treats it as an ordinary unknown route, so it used to hand a document back to a request that claimed to be RPC. - Every rejection is the same fixed `404 server_function_not_found` JSON with no id, no stack, and no function detail. Auth is untouched: the bounded 404 returns before Start's middleware, and it discriminates only on structure and manifest membership, never on a credential. - The `run_worker_first` entry is now documented as what it is -- pinning the transport to the Worker so the bounded 404 holds for a navigation request too. It is not the fix: a non-navigation request to a non-asset path already reaches the Worker, and the array form already disables `assets_navigation_prefers_asset_serving`. Measured delta over 80 differential probes against master: zero. B. The logo link test exercises the real router `Brand.test.tsx` mocked `Link`, so its `not.toContain("payload")` and `not.toContain("locale")` assertions only exercised the mock's own `URLSearchParams` logic. A `payload` param leaking through the root search validation passed the old test and fails the new one. It now drives a real TanStack router (memory history, the app's real `validateRootSearch` and `preserveRootLangFromMiddleware`, `Brand` mounted as a layout above `Outlet` the way `HeaderBar` mounts it) and reads the href the real `Link` builds. It also asserts the href against `router.buildLocation`, clicks the link and checks the resulting location, and covers the VI-copy/EN-navigation split. Test counts (measured, not estimated): 150 files / 1511 web tests, 10 files / 122 focused, 61 extension. Verified against the real production bundle under workerd with the seven manifest ids: 54/54 transport assertions and 29/29 EN/VI navigation assertions, including `Sign in required` still arriving as Start's serialized envelope with no Worker cache, Content-Language or robots policy stamped on it. Refs #180 Related: #181
duyetbot
force-pushed
the
fix/home-navigation-hydration-20260925
branch
from
September 25, 2026 17:27
3630b3c to
29b2d92
Compare
duyetbot
pushed a commit
that referenced
this pull request
Sep 25, 2026
Two review follow-ups on #186, on top of the #181 transport fix. A. Server-function ids are validated against Start's real manifest The previous pass only rejected empty and nested transport paths. Any single-segment id still reached `getServerFnById`, and an id that is not in the build-generated manifest made that throw an unhandled error: a 500 whose message embeds the requested id, skipping Start's serialized-envelope contract entirely. That covers stale ids from a renamed function still held by an already-served client bundle, and every percent-encoded traversal form, none of which the old shape check could see. - `server-fn-registry.ts` (new, server-only) loads the same generated resolver the handler uses and asks it whether an id is registered. A resolver that cannot be loaded answers `null`, and the Worker then defers to Start, so an infrastructure failure can never turn a working RPC call into a 404. For a registered id the lookup warms the resolver's own module cache, so the handler's later lookup costs nothing extra. The file is server-only because the virtual module is materialized for the server environment alone; importing it from the shared path helpers breaks the client build. - `serverFnIdOf` replaces the shape-only check with a charset that covers both alphabets Start's compiler emits -- `base64url` in dev, `sha256` hex in a build, `_N` on a dedup collision -- and retires `.`, `..`, `%`, encoded separators, nul bytes, standard-base64 `+`/`=`, and over-long segments before a resolver is consulted. The charset is pinned by a test that builds a real dev id the same way the compiler does. - The bare base `/_serverFn` is now bounded too. Start treats it as an ordinary unknown route, so it used to hand a document back to a request that claimed to be RPC. `/_serverFnx` and `/_serverFnOLD` are not claimed. - Every rejection is the same fixed `404 server_function_not_found` JSON with no id, no stack, and no function detail. Auth is untouched: the bounded 404 returns before Start's middleware and discriminates only on structure and manifest membership, never on a credential, so it adds no enumeration oracle. - The `run_worker_first` entry is now documented as what it is -- pinning the transport to the Worker so the bounded 404 holds for a navigation request too. It is not the fix: a non-navigation request to a non-asset path already reaches the Worker, and the array form already disables `assets_navigation_prefers_asset_serving`. Measured delta over 80 differential probes against master: zero. B. The logo link test exercises the real router `Brand.test.tsx` mocked `Link`, so its `not.toContain("payload")` and `not.toContain("locale")` assertions only exercised the mock's own `URLSearchParams` logic. A `payload` param leaking through the root search validation passes the old test and fails the new one. It now drives a real TanStack router (memory history, the app's real `validateRootSearch` and `preserveRootLangFromMiddleware`, `Brand` mounted as a layout above `Outlet` the way `HeaderBar` mounts it) and reads the href the real `Link` builds. It also asserts the href against `router.buildLocation`, clicks the link and checks the resulting location, and covers the VI-copy/EN-navigation split. Test counts (measured, not estimated): 153 files / 1543 web tests, 9 files / 120 focused, 61 extension. Verified against the real production bundle under workerd with the seven manifest ids: 54/54 transport assertions and 29/29 EN/VI navigation assertions, including `Sign in required` still arriving as Start's serialized envelope with no Worker cache, Content-Language or robots policy stamped on it. Refs #180 Related: #181
duyetbot
force-pushed
the
fix/home-navigation-hydration-20260925
branch
from
September 25, 2026 17:32
ab42ab2 to
3551404
Compare
duyetbot
pushed a commit
that referenced
this pull request
Sep 25, 2026
Two review follow-ups on #186, on top of the #181 transport fix. A. Server-function ids are validated against Start's real manifest The previous pass only rejected empty and nested transport paths. Any single-segment id still reached `getServerFnById`, and an id that is not in the build-generated manifest made that throw an unhandled error: a 500 whose message embeds the requested id, skipping Start's serialized-envelope contract entirely. That covers stale ids from a renamed function still held by an already-served client bundle, and every percent-encoded traversal form, none of which the old shape check could see. - `server-fn-registry.ts` (new, server-only) loads the same generated resolver the handler uses and asks it whether an id is registered. A resolver that cannot be loaded answers `null`, and the Worker then defers to Start, so an infrastructure failure can never turn a working RPC call into a 404. For a registered id the lookup warms the resolver's own module cache, so the handler's later lookup costs nothing extra. The file is server-only because the virtual module is materialized for the server environment alone; importing it from the shared path helpers breaks the client build. - `serverFnIdOf` replaces the shape-only check with a charset that covers both alphabets Start's compiler emits -- `base64url` in dev, `sha256` hex in a build, `_N` on a dedup collision -- and retires `.`, `..`, `%`, encoded separators, nul bytes, standard-base64 `+`/`=`, and over-long segments before a resolver is consulted. The charset is pinned by a test that builds a real dev id the same way the compiler does. - The bare base `/_serverFn` is now bounded too. Start treats it as an ordinary unknown route, so it used to hand a document back to a request that claimed to be RPC. `/_serverFnx` and `/_serverFnOLD` are not claimed. - Every rejection is the same fixed `404 server_function_not_found` JSON with no id, no stack, and no function detail. Auth is untouched: the bounded 404 returns before Start's middleware and discriminates only on structure and manifest membership, never on a credential, so it adds no enumeration oracle. - The `run_worker_first` entry is now documented as what it is -- pinning the transport to the Worker so the bounded 404 holds for a navigation request too. It is not the fix: a non-navigation request to a non-asset path already reaches the Worker, and the array form already disables `assets_navigation_prefers_asset_serving`. Measured delta over 80 differential probes against master: zero. B. The logo link test exercises the real router `Brand.test.tsx` mocked `Link`, so its `not.toContain("payload")` and `not.toContain("locale")` assertions only exercised the mock's own `URLSearchParams` logic. A `payload` param leaking through the root search validation passes the old test and fails the new one. It now drives a real TanStack router (memory history, the app's real `validateRootSearch` and `preserveRootLangFromMiddleware`, `Brand` mounted as a layout above `Outlet` the way `HeaderBar` mounts it) and reads the href the real `Link` builds. It also asserts the href against `router.buildLocation`, clicks the link and checks the resulting location, and covers the VI-copy/EN-navigation split. Test counts (measured, not estimated): 153 files / 1543 web tests, 9 files / 120 focused, 61 extension. Verified against the real production bundle under workerd with the seven manifest ids: 54/54 transport assertions and 29/29 EN/VI navigation assertions, including `Sign in required` still arriving as Start's serialized envelope with no Worker cache, Content-Language or robots policy stamped on it. Refs #180 Related: #181
duyetbot
force-pushed
the
fix/home-navigation-hydration-20260925
branch
from
September 25, 2026 17:35
3551404 to
db85d9d
Compare
duyetbot
pushed a commit
that referenced
this pull request
Sep 25, 2026
Two review follow-ups on #186, on top of the #181 transport fix. A. Server-function ids are validated against Start's real manifest The previous pass only rejected empty and nested transport paths. Any single-segment id still reached `getServerFnById`, and an id that is not in the build-generated manifest made that throw an unhandled error: a 500 whose message embeds the requested id, skipping Start's serialized-envelope contract entirely. That covers stale ids from a renamed function still held by an already-served client bundle, and every percent-encoded traversal form, none of which the old shape check could see. - `server-fn-registry.ts` (new, server-only) loads the same generated resolver the handler uses and asks it whether an id is registered. A resolver that cannot be loaded answers `null`, and the Worker then defers to Start, so an infrastructure failure can never turn a working RPC call into a 404. For a registered id the lookup warms the resolver's own module cache, so the handler's later lookup costs nothing extra. The file is server-only because the virtual module is materialized for the server environment alone; importing it from the shared path helpers breaks the client build. - `serverFnIdOf` replaces the shape-only check with a charset that covers both alphabets Start's compiler emits -- `base64url` in dev, `sha256` hex in a build, `_N` on a dedup collision -- and retires `.`, `..`, `%`, encoded separators, nul bytes, standard-base64 `+`/`=`, and over-long segments before a resolver is consulted. The charset is pinned by a test that builds a real dev id the same way the compiler does. - The bare base `/_serverFn` is now bounded too. Start treats it as an ordinary unknown route, so it used to hand a document back to a request that claimed to be RPC. `/_serverFnx` and `/_serverFnOLD` are not claimed. - Every rejection is the same fixed `404 server_function_not_found` JSON with no id, no stack, and no function detail. Auth is untouched: the bounded 404 returns before Start's middleware and discriminates only on structure and manifest membership, never on a credential, so it adds no enumeration oracle. - The `run_worker_first` entry is now documented as what it is -- pinning the transport to the Worker so the bounded 404 holds for a navigation request too. It is not the fix: a non-navigation request to a non-asset path already reaches the Worker, and the array form already disables `assets_navigation_prefers_asset_serving`. Measured delta over 80 differential probes against master: zero. B. The logo link test exercises the real router `Brand.test.tsx` mocked `Link`, so its `not.toContain("payload")` and `not.toContain("locale")` assertions only exercised the mock's own `URLSearchParams` logic. A `payload` param leaking through the root search validation passes the old test and fails the new one. It now drives a real TanStack router (memory history, the app's real `validateRootSearch` and `preserveRootLangFromMiddleware`, `Brand` mounted as a layout above `Outlet` the way `HeaderBar` mounts it) and reads the href the real `Link` builds. It also asserts the href against `router.buildLocation`, clicks the link and checks the resulting location, and covers the VI-copy/EN-navigation split. Test counts (measured, not estimated): 153 files / 1543 web tests, 9 files / 120 focused, 61 extension. Verified against the real production bundle under workerd with the seven manifest ids: 54/54 transport assertions and 29/29 EN/VI navigation assertions, including `Sign in required` still arriving as Start's serialized envelope with no Worker cache, Content-Language or robots policy stamped on it. Refs #180 Related: #181
duyetbot
force-pushed
the
fix/home-navigation-hydration-20260925
branch
from
September 25, 2026 17:38
db85d9d to
23554ea
Compare
duyetbot
pushed a commit
that referenced
this pull request
Sep 25, 2026
Two review follow-ups on #186, on top of the #181 transport fix. A. Server-function ids are validated against Start's real manifest The previous pass only rejected empty and nested transport paths. Any single-segment id still reached `getServerFnById`, and an id that is not in the build-generated manifest made that throw an unhandled error: a 500 whose message embeds the requested id, skipping Start's serialized-envelope contract entirely. That covers stale ids from a renamed function still held by an already-served client bundle, and every percent-encoded traversal form, none of which the old shape check could see. - `server-fn-registry.ts` (new, server-only) loads the same generated resolver the handler uses and asks it whether an id is registered. A resolver that cannot be loaded answers `null`, and the Worker then defers to Start, so an infrastructure failure can never turn a working RPC call into a 404. For a registered id the lookup warms the resolver's own module cache, so the handler's later lookup costs nothing extra. The file is server-only because the virtual module is materialized for the server environment alone; importing it from the shared path helpers breaks the client build. - `Object.prototype` members are rejected by an explicit own-property check. They satisfy the id charset, and the generated resolver reads `manifest[id]`, so each one passes its own "not found" guard and only fails a line later on `importer()`. Recognising them therefore depended on catching that TypeError, i.e. on how the generated lookup happened to be written. `Object.hasOwn(Object.prototype, id)` makes the rejection deliberate and local, and keeps the verdict independent of the resolver. The outcome was already a bounded 404; what changes is that it is now reached on purpose rather than by accident, which a test pins. - `serverFnIdOf` replaces the shape-only check with a charset that covers both alphabets Start's compiler emits -- `base64url` in dev, `sha256` hex in a build, `_N` on a dedup collision -- and retires `.`, `..`, `%`, encoded separators, nul bytes, standard-base64 `+`/`=`, and over-long segments before a resolver is consulted. The charset is pinned by a test that builds a real dev id the same way the compiler does. - The bare base `/_serverFn` is now bounded too. Start treats it as an ordinary unknown route, so it used to hand a document back to a request that claimed to be RPC. `/_serverFnx` and `/_serverFnOLD` are not claimed. - Every rejection is the same fixed `404 server_function_not_found` JSON with no id, no stack, and no function detail. Auth is untouched: the bounded 404 returns before Start's middleware and discriminates only on structure and manifest membership, never on a credential, so it adds no enumeration oracle. - The `run_worker_first` entry is now documented as what it is -- pinning the transport to the Worker so the bounded 404 holds for a navigation request too. It is not the fix: a non-navigation request to a non-asset path already reaches the Worker, and the array form already disables `assets_navigation_prefers_asset_serving`. Measured delta over 80 differential probes against master: zero. B. The logo link test exercises the real router `Brand.test.tsx` mocked `Link`, so its `not.toContain("payload")` and `not.toContain("locale")` assertions only exercised the mock's own `URLSearchParams` logic. A `payload` param leaking through the root search validation passes the old test and fails the new one. It now drives a real TanStack router (memory history, the app's real `validateRootSearch` and `preserveRootLangFromMiddleware`, `Brand` mounted as a layout above `Outlet` the way `HeaderBar` mounts it) and reads the href the real `Link` builds. It also asserts the href against `router.buildLocation`, clicks the link and checks the resulting location, and covers the VI-copy/EN-navigation split. Test counts (measured, not estimated): 156 files / 1613 web tests, 9 files / 122 focused, 61 extension. Verified against the real production bundle under workerd with the seven manifest ids: 62/62 transport assertions and 29/29 EN/VI navigation assertions, including `Sign in required` still arriving as Start's serialized envelope with no Worker cache, Content-Language or robots policy stamped on it. Refs #180 Related: #181
duyetbot
force-pushed
the
fix/home-navigation-hydration-20260925
branch
from
September 25, 2026 18:21
23554ea to
8a22d22
Compare
Reserve the TanStack server-function transport in asset routing, reject malformed RPC paths with JSON, and keep non-HTML document requests out of document redirects. Add EN/VI navigation regression coverage and Cloudflare config assertions.
Two review follow-ups on #186, on top of the #181 transport fix. A. Server-function ids are validated against Start's real manifest The previous pass only rejected empty and nested transport paths. Any single-segment id still reached `getServerFnById`, and an id that is not in the build-generated manifest made that throw an unhandled error: a 500 whose message embeds the requested id, skipping Start's serialized-envelope contract entirely. That covers stale ids from a renamed function still held by an already-served client bundle, and every percent-encoded traversal form, none of which the old shape check could see. - `server-fn-registry.ts` (new, server-only) loads the same generated resolver the handler uses and asks it whether an id is registered. A resolver that cannot be loaded answers `null`, and the Worker then defers to Start, so an infrastructure failure can never turn a working RPC call into a 404. For a registered id the lookup warms the resolver's own module cache, so the handler's later lookup costs nothing extra. The file is server-only because the virtual module is materialized for the server environment alone; importing it from the shared path helpers breaks the client build. - `Object.prototype` members are rejected by an explicit own-property check. They satisfy the id charset, and the generated resolver reads `manifest[id]`, so each one passes its own "not found" guard and only fails a line later on `importer()`. Recognising them therefore depended on catching that TypeError, i.e. on how the generated lookup happened to be written. `Object.hasOwn(Object.prototype, id)` makes the rejection deliberate and local, and keeps the verdict independent of the resolver. The outcome was already a bounded 404; what changes is that it is now reached on purpose rather than by accident, which a test pins. - `serverFnIdOf` replaces the shape-only check with a charset that covers both alphabets Start's compiler emits -- `base64url` in dev, `sha256` hex in a build, `_N` on a dedup collision -- and retires `.`, `..`, `%`, encoded separators, nul bytes, standard-base64 `+`/`=`, and over-long segments before a resolver is consulted. The charset is pinned by a test that builds a real dev id the same way the compiler does. - The bare base `/_serverFn` is now bounded too. Start treats it as an ordinary unknown route, so it used to hand a document back to a request that claimed to be RPC. `/_serverFnx` and `/_serverFnOLD` are not claimed. - Every rejection is the same fixed `404 server_function_not_found` JSON with no id, no stack, and no function detail. Auth is untouched: the bounded 404 returns before Start's middleware and discriminates only on structure and manifest membership, never on a credential, so it adds no enumeration oracle. - The `run_worker_first` entry is now documented as what it is -- pinning the transport to the Worker so the bounded 404 holds for a navigation request too. It is not the fix: a non-navigation request to a non-asset path already reaches the Worker, and the array form already disables `assets_navigation_prefers_asset_serving`. Measured delta over 80 differential probes against master: zero. B. The logo link test exercises the real router `Brand.test.tsx` mocked `Link`, so its `not.toContain("payload")` and `not.toContain("locale")` assertions only exercised the mock's own `URLSearchParams` logic. A `payload` param leaking through the root search validation passes the old test and fails the new one. It now drives a real TanStack router (memory history, the app's real `validateRootSearch` and `preserveRootLangFromMiddleware`, `Brand` mounted as a layout above `Outlet` the way `HeaderBar` mounts it) and reads the href the real `Link` builds. It also asserts the href against `router.buildLocation`, clicks the link and checks the resulting location, and covers the VI-copy/EN-navigation split. Test counts (measured, not estimated): 156 files / 1613 web tests, 9 files / 122 focused, 61 extension. Verified against the real production bundle under workerd with the seven manifest ids: 62/62 transport assertions and 29/29 EN/VI navigation assertions, including `Sign in required` still arriving as Start's serialized envelope with no Worker cache, Content-Language or robots policy stamped on it. Refs #180 Related: #181
duyetbot
force-pushed
the
fix/home-navigation-hydration-20260925
branch
from
September 25, 2026 18:28
8a22d22 to
2f51b9d
Compare
…ook (#205) * feat(web): redesign /data and sync Clerk signups from a verified webhook The /data page showed a single lonely "AIDR user signups" card in the top-right corner, and its number came from a live Clerk Admin API call on every page load — slow when Clerk was slow, "Unavailable" when Clerk was down, and a public endpoint taking a third-party dependency per visitor. Redesign - Page header is now just the title and one line of context; the model attribution card is a quiet full-width strip instead of a loud header card, and the tab list is lighter. Tabs are unchanged: Overview, Content, Runs, Sources, LLM, Algo (+ Admin for admins). - Signups is a first-class StatTile-family metric in the Overview row, beside Stories, Tokens, Runs today, Subscribers, and Last run. D1 subscribers (email subscriptions) stay a separate, clearly labelled metric — a subscription is not an account. - StatTile gains a `value: string | null` placeholder state, a `numeric` flag for non-numeric values, and a shared min-height so a tile never shifts the row as endpoints resolve. The Overview charts stack full width. CardData takes an optional className so a caller can inject into a parent grid; the unused DataSkeleton is gone. Signups source: Clerk → D1 - New migration 0026_clerk_users.sql creates clerk_users (Clerk id primary key, email, created_at, updated_at, deleted_at) plus two indexes. The table starts empty on purpose: rows only come from a verified webhook or an explicit backfill, never from a guess. - POST /api/webhooks/clerk handles user.created/updated/deleted. Every delivery must carry Svix's headers and is verified with HMAC-SHA256 over `svix-id.svix-timestamp.body` against CLERK_WEBHOOK_SECRET (WebCrypto, constant-time compare, 5-minute replay window). Bad or missing signature is 401, oversized body 413, missing secret/binding 503, D1 write failure 500 so Svix redelivers. Other event types are acknowledged and ignored. - Upserts are replay-safe (ON CONFLICT never rewrites created_at) and deletes are soft, so the live count only falls when Clerk says a user is gone. - POST /api/admin/clerk-sync is the admin-gated one-shot backfill (100 per page, 20 pages max) using CLERK_SECRET_KEY, plus a "Sync Clerk users" button in the admin panel. It exists so the metric is real before the first webhook lands; the webhook keeps it current afterwards. - GET /api/system/accounts is now a single indexed COUNT over the mirror, returning { total, source: "d1", status }. A missing binding or unmigrated database is `unconfigured` and a read failure is `error`; both report null and the UI says "Unavailable" instead of rendering a fabricated 0. migration-gate.ts treats 0026 as required once its file is present, so a deploy against an unmigrated database fails loudly. Docs and tests - CLERK_WEBHOOK_SECRET added to .env.example and to the worker/GitHub optional secret lists in scripts/sync-env.ts; the mirror is documented in worker/README.md including the migration-apply step. - New unit tests cover signature verification (401 on bad signatures, replay window, multi-signature headers), the D1 upsert/soft-delete statements against real SQLite, backfill paging, the migration's schema and ordering, and the new signups tiles (including that the Overview batch failing never hides the signup total and that no zero is invented before the endpoint answers). Fixes #198 * test(web): construct Svix webhook fixtures so GitGuardian ignores them --------- Co-authored-by: duyet <5009534+duyet@users.noreply.github.com> Co-authored-by: duyetbot <duyetbot@users.noreply.github.com>
…olish Work in progress, captured before rebasing onto master (43 commits behind). - extension: web header + newtab polish, settings panel, manifest 0.1.18 - ui: add trackChannelClick / TrackChannel for chrome|telegram|email funnels - web: stop double-counting page views (gtag send_page_view: false) - web: header menu split (GetAIDRMenu, PhoneMenu, Compact/Wide rows) - verify-aidr skill: analytics + telegram features, refreshed driver - tests: feed-queries, tldr-section, chrome copy, campaign, analytics Excluded QA artifacts (.playwright-mcp/, design-preview.html).
Production probe: POST /api/webhooks/clerk returns 503
{"error":"clerk webhook not configured"} on every delivery, and
/api/system/accounts returns {"total":0,"status":"available"} — so /data
renders "Signups 0" for a mirror that has never received an event.
2b63ced shipped CLERK_WEBHOOK_SECRET as WORKER_OPTIONAL, so `pnpm sync-env`
silently skipped it. 8554148 had already added exactly this gate for the
sibling secret CLERK_SECRET_KEY (WORKER_REQUIRED + a fail-closed deploy
smoke); the new secret never got one.
- sync-env.ts: CLERK_WEBHOOK_SECRET -> WORKER_REQUIRED
- deploy-web.yml: fail-closed smoke asserting the endpoint is not 503/404,
mirroring the /__clerk/v1/environment gate. Asserts status only, never the
body. The probe posts a non-user event type, which the handler ignores, so
even a signature-verification regression could not write a clerk_users row
and inflate the public count.
- account-count.ts: an empty mirror is now `unconfigured` (total: null) rather
than a real 0. An empty table cannot distinguish "Clerk has no accounts"
from "no webhook delivery yet", and worker/README.md:93-96 already
specified `unconfigured` for "table missing / no rows yet" — the code had
drifted from its own documented contract. /data now says "Unavailable"
instead of a fabricated count, and becomes available once a row lands.
Verified: 1656 web tests, 65 extension tests, tsc --noEmit clean, biome clean.
Live gate confirmed red today (503) and passing for 400/401/405/200.
… image_url Closes the remaining half of #207. buildMissingMediaQuery gated candidates on `image_url IS NULL OR image_url = ''`, so every published row that already carried a legacy og:image was excluded — precisely the rows that most need a media_manifest, and the reason the backfill "cannot enrich rows with a legacy image_url". The only remaining gate is media_manifest being absent/empty/[]. `image_url` is still selected so the manifest can be built from it. The old predicate was pinned by an assertion in backfill.test.ts; it is replaced by a regression test asserting the clause stays gone and the column stays selected. The other half of #207 (the stale 0025 migration-gate assertion) already landed in cc460b3 and needed no change here.
…(login 502) (#212) * fix(worker): let the Clerk handshake redirect back to the app origin Clerk login was broken in production: every session refresh failed with GET /__clerk/v1/client/handshake -> 502 "Clerk upstream redirect rejected" while /__clerk/v1/environment and /__clerk/v1/client returned 200. Only the handshake path was affected, which is why sign-in appeared to work right up until the session needed renewing. Cause: rewriteSameOriginRedirect accepted a redirect only when the Location resolved to CLERK_FAPI_ORIGIN (frontend-api.clerk.dev). But the handshake is *defined* as ending with a redirect back to the application: Clerk's HandshakeService.resolveHandshake() builds the Location from `authenticateContext.clerkUrl`, the endpoint documents itself as "redirect back to after the handshake" and answers 307, and clerk/javascript#8186 records FAPI returning "a 302 back to the app origin". That is the app origin, so the check rejected it and the proxy answered 502 instead of forwarding the hop. Introduced by 488a9c0, whose redirect hardening was otherwise sound. Fix: allow a redirect whose origin is exactly publicProxy.origin and pass it through unchanged — the browser is already on that origin, so this cannot become an open redirect. The caller-supplied `redirect_url` parameter is never consulted; comparing against publicProxy.origin is precisely what stops an attacker-chosen redirect target from turning this into a redirector. Tests: the 12 existing path-canonicalization/credential/suffix vectors plus new cases for scheme downgrade, port swap, credentialed app origin, suffix attack and unrelated origin all still 502. Verified the new test fails without the source change. * style(web): apply biome formatting to the new clerk-proxy test CI runs `biome check .` in apps/web, which includes the formatter; `biome lint` alone does not, so the 3-line mockFetch call was not collapsed locally and the Lint job failed. Formatting only.
fix(web): gate CLERK_WEBHOOK_SECRET, honest signup count, media backfill reach
Operational skill for Worker secret/deploy work, gated on a read-only preflight that catches the failure modes hit this session: - expired CLOUDFLARE_API_TOKEN (CF 1000 / wrangler 9109), the silent reason `pnpm sync-env` fails - mixed Clerk instances (pk_test + sk_live), which authenticate the browser and the Worker against different instances and break sign-in quietly - the two publishable keys disagreeing when sync-env treats them as aliases - missing required Worker secrets - live probes: handshake 502 (login broken), webhook 503 (secret missing), signups unconfigured (mirror empty) `sync` and `deploy` refuse to run while preflight fails. `sync` also works around the underlying sync-env bug where wrangler is spawned with `env: process.env` and never sees a token stored in .env.local. Secret values are never printed; output is masked and JSON on stdout. Non-zero exit means not ready.
Registers this repo with the herdr-desk plugin. The 2-line config relies on the plugin defaults, which supply the `desk:github-issues` job (agent `aidr-desk`, cron 0 7 * * *) that triages issues and PRs and spawns worktrees. No `extra` overrides, so nothing repo-specific is pinned. `herdr plugin action invoke herdr-desk.status` reports the daemon running and aidr scheduled; `herdr-desk.validate` reports 0 errors across 3 desks.
A HyperFrames composition that renders one 18s 1080x1920 short per trending story, in English and Vietnamese, from real /api/feed fields only. One composition, N outputs: every per-story value is a declared variable, so no HTML is ever generated per story and a batch is one render command. Content leads. The headline is the hero and lands at 0.14s; the rank score is supporting evidence, counting up inside an icon chip rather than filling the frame. Beats are title, source, stats, pull-quote, publisher's words, end card. The thumbnail is fitted, never cropped: the image card absorbs all leftover column height and the image sits inside it contained and centred, so a wide OG card letterboxes against the panel instead of being blown up. The dither field is pinned to a single Bayer level over the content column, which is the brightest it can be while secondary text still clears WCAG AA at 4.5:1 - verified against the field's real luminance range, because the auditor reports a false pass when every text block rests at opacity 0. Composes with: 47 source files, 2076 lines. Rendered MP4s and build scratch are gitignored - the binaries are ~150MB for 12 outputs and one `render --batch` away.
) * fix(web): unbreak the /data runs charts, dedupe algo tab, pad tabs The Runs tab's Duration and Outcomes cards have always rendered empty. Both hand-rolled their bars as percentage-height divs nested inside a flex item whose own height was `auto`, and a percentage height against an indefinite parent resolves to `auto` — so every bar collapsed to 0px and only the legend showed. Measured in Chromium against the exact pattern: 0px bars inside a 112px plot box. The API data was fine all along (`stats.new: 5, merged: 1`). Rebuild both on the dither-kit `BarChart` the Overview tab already uses, so they get measured plot geometry, axes, gridlines and tooltips. Extract the series derivation (`outcomeRows`, `durationRows`) and pin it with tests: a DOM assertion on pixel height could never have caught this, but asserting the series handed to the chart does. Also: - /data padding: tab strip `p-1.5`/`gap-1.5`/`min-h-11` with `px-4 py-2` triggers, and one shared `TAB_PANEL` gap so no tab body is glued to the strip (was a mix of `mt-0` and `mt-4`). - /data?tab=algo: the four model chains rendered in both the Ranking card and the AnyRouter card, and a third time in the attribution strip above the tabs. Ranking keeps them; the AnyRouter card drops its duplicate list and becomes a gateway pitch. Explainer compacted to 2-up grids. - /subscribe?tab=telegram: was the only channel tab with no visual proof. Added a Telegram message mock in the shared frame, matching the two message shapes worker/notify/telegram.ts actually sends, plus a feature list. EN + VI. Refs #215, #216, #217, #218 * fix(web): address review on the runs charts, axis labels and preview mock Real defects found reviewing #219: - The duration y-axis formatter rounded minutes, so distinct ticks collapsed onto one label (`0s 50s 2m 3m 3m`) and `100` printed as `2m`. It now keeps one decimal above a minute, so neighbouring ticks can never agree. - The `total` readout used `Math.round(total / 60)`, printing a flat `0m` for a 20s window. It now shares a formatter that keeps sub-minute totals in seconds; avg and peak stay in seconds too, since the card is "seconds per run" and a ~3m pipeline otherwise reported avg and peak as the same "3m". - `runAxisTime` hardcoded `en-US` and omitted `timeZone`, contradicting `formatTimestamp` in the same file — a UTC+7 viewer would see one wall clock on the axis and another in the run's detail row, and the label would shift with the client machine's timezone. Both helpers are now UTC-anchored and lang-aware, and `lang` is threaded through RunsTab. Covered by a test that pins the output across three `process.env.TZ` values. - `SERIES` duplicated the `CONFIG` keys, so a renamed series would silently vanish from the stack (`Bar` renders null for an unconfigured dataKey). `CONFIG` is now typed against a `SeriesKey` union, making that a compile error, and `SERIES` derives from it. - The Telegram mock claimed to mirror `buildStoryCaption` but shipped a prettified date and a `#hạ_tầng` hashtag the bot can never emit (it replaces every non-alphanumeric). It now uses the digest's YYYY-MM-DD stamp and the ASCII slug. Its test asserts against the real `buildDigestMessage` / `buildStoryCaption` rather than against a comment mentioning them. - `<article>` mock bubbles had no accessible name, so they added two unnamed landmarks; they are `<div>`s now, and the 🗞/🔥 glyphs are `aria-hidden` like every other icon in the file. Tests also tightened: the `p-1.5` assertion was a substring check that also matched `gap-1.5` and would have kept passing with the padding reverted, and the duration summary was asserted through `<dt>/<dd>` adjacency that a wrapper element would silently turn into a pass. 1679 -> 1691 passing. Each of the three new guards was mutation-checked by reintroducing the regression it guards (padding, duplicate chains, accented hashtag) and confirming the test fails.
The attribution strip said "Powered by AnyRouter" and the Algo tab had an "AnyRouter" card, but neither carried the mark — the gateway was named in text only. Inline the official monogram beside both headings. - AnyRouterMark: their brand mark inlined from https://anyrouter.dev/brand/anyrouter-logo-black.svg (both paths, viewBox and transform are byte-identical to the asset). Inlined rather than hot-linked so a third-party outage cannot break /data, and painted `currentColor` so one copy serves both themes instead of shipping a black and a white one. `aria-hidden`: the wordmark text already carries the name, so the mark adds no accessible text. - ModelAttribution: the mark is a sibling of the baseline-aligned title/subtitle pair, not a child. As a child it would set that flex item's baseline to the bottom of the square and break the 13px/11px pairing. - AlgoTab: the gateway card heading gets the mark, same 14px. - ChartCard: title/subtitle widen from string to ReactNode so a heading can carry a mark. Backwards compatible — every other caller passes a plain string.
The More column had one link (duyet.net) and the site had no footer route to the gateway that runs every LLM call in the pipeline. /data and /about both credit AnyRouter; the footer did not mention it at all. - NewsFooter: anyrouter.dev under More, same link treatment as duyet.net (external, new tab, `rel="noopener noreferrer"`, `nav_click` tracked). - site.ts: ANYROUTER_URL is now the single definition of the credited referral URL. The literal was already copy-pasted into ModelAttribution, AlgoTab, and about.tsx; a fourth copy in the footer would have let the `?ref=aidr.today` partner param drift off one surface, so those three now import it instead. - algo-tab-dedupe.test: the assertion that AlgoTab keeps the referral link now checks the import plus the definition in site.ts, so the link cannot be dropped and the credited URL cannot silently change.
…ots.txt (#233) #223 (umbrella #232). Three measured indexing blockers, all live on https://aidr.today as of 2026-09-27. 1. Bare localized HTML was `noindex, follow`. locale-response.ts stamped the directive on every localized SSR response without an explicit `?lang=`, which is the URL a human types, a Telegram link carries, or a backlink points at: 1,005 GSC "Discovered - currently not indexed" URLs plus Lighthouse "Page is blocked from indexing". `private, no-store` is a cache-isolation requirement, not a robots one, so the cache-isolation contract in LOCALE_URLS.md is unchanged. Indexability now has one owner: routeIndexability() feeds both X-Robots-Tag and <meta name="robots">, and the locale layer sets cache and Vary only. Dedup rides on the existing self-referencing ?lang= canonical + vi/en/x-default hreflang. 2. Legacy /{category}/{slug} and over-long id hashes answered 307, so Google never consolidated ranking signals onto the documented /{8-hex} canonical. They now answer 308 through a shared localeRedirect() helper, so the temporary and permanent hops cannot drift apart: still private, no-store, varied, no-referrer, noindex/nofollow — permanence is for crawlers, the target still depends on the requester's locale. The in-router copy in $cat.$slug.tsx is 308 too. Locale alias normalization stays 307. 3. robots.txt failed to parse: `LLMs-txt:` is not a directive and failed the whole file (Lighthouse line 5, "Unknown directive"). It is now a comment; Sitemap: stays; the committed public/robots.txt duplicate is fixed to match byte-for-byte. Diff in sitemap.ts is robotsTxt() only — #225 is rewriting that file in parallel, #226 owns llms-txt.ts. Also: LOCALE_URLS.md now describes the new contract instead of the old sentences, and worker/README.md gains the 403 diagnostic plus the post-deploy re-verification runbook that #223 asks for. Tests pin bare-locale indexable, explicit-locale indexable, private paths and 4xx/5xx noindex/nofollow, 308 permanence, single hop, and the reserved-path guard (including _serverFn).
… format, runnable editor checklist (no-go unchanged) (#235) Makes the executable parts of docs/decisions/telegram-instant-view.md real without changing its no-go status. No template, no rhash, no Bot API IV lifecycle call, no production rollout, no delivery-key change. The field gate (worker/telegram-iv.ts) answers "is this story IV-eligible?" as a query instead of a manual read. For a candidate https://aidr.today/{id8}?lang=vi|en it checks all six record fields against the same helpers the public page renders (localizedTitle, renderStoryMarkdown), so the verdict cannot disagree with the page: title localizedTitle(); a VI request with no title_vi reports an explicit fallback_from_en, never a Vietnamese render body requires a real summary AND >=1 source link, counted from the rendered Markdown (the renderer's "No summary is available." placeholder is not accepted as a body) published_date Unix seconds; normalizePublishedAtSeconds handles the documented epoch ms/seconds bug class and rejects anything outside 2000-2100, so a millisecond value can never become a year-2286 date image_url the generated first-party card /api/og/{id8}.png (1200x630 image/png by construction) — the same image articleHead already emits as og:image, so hotlinking, MIME, and dimensions are deterministic site_name read from the single shared SITE_NAME constant, so it cannot drift from og:site_name description first summary paragraph of the RENDERED locale TELEGRAM_IV_LIMITS encodes the documented ceilings with their sources: 5 MB HTTP-URL photo, 10 MB multipart, width+height <= 10,000, aspect <= 20, caption <= 1,024 chars, plus the separate IV rendering guidance. A non-generated candidate goes through a byte-bounded, SSRF-checked probe (isFetchableUrl/fetchWithSafeRedirects, Range request, strict byte budget, explicit body cancel) — the Worker never downloads or proxies a whole media file. It fails closed on a missing image, private-literal/credentialed/ plain-HTTP URL, oversized, unsupported container, and ambiguous id prefix. Logs and verdicts go through telemetry-safe redaction; the probe reports only the origin, never a signed query string. Format, which needs no Telegram approval and no rhash: - link_preview_options is set explicitly on all three sends (DIGEST/STORY_PHOTO/STORY_TEXT_LINK_PREVIEW). All is_disabled, with the reason and the rejected prefer_small_media / prefer_large_media / show_above_text values documented next to them: a preview would attach to one arbitrary digest bullet or double the photo. - The trending sendPhoto path attaches the generated card instead of the upstream thumb, so a link preview can never be a 404 hotlink-hostile image. The normalized manifest thumbnail stays as the fallback for an id that cannot address a card. This is the same choice articleHead already made. - A story with several manifest images says so (N more) instead of silently shipping one photo that looks complete — the record's "omit rather than silently drop" rule applied to the fallback path. The manual checklist is now runnable: verify-aidr doctor iv --id <8hex> --lang vi|en It prints the field verdict, range-probes the card (200 image/png 1200x630), and ends with the exact source URL to paste into the IV Editor plus the unresolved items as labelled placeholders. It needs no bot token and no channel id, and a unit test asserts the lever, the gate script, and the record contain no invented rhash, no real t.me/iv link, no bot token, and no channel id. The same gate is served to authenticated operators at GET /api/admin/notify/iv. The record now references the SITE_NAME constant instead of site_name: null, documents what the gate proves and what it still does not, carries the evidence template for the manual checks (both lang variants, mobile + desktop, disposable channel), and keeps its no-go status. Delivery is untouched: one message per delivery key, digest:<local-date> or the story id, notifications PK (channel, item_id), no lang-keyed row, and a failed media call still falls back to text once. Refs #231, #232, #146
…map index, feed autodiscovery (#236) aidr.today publishes no feed at all: `/feed.xml` is 404 today, and the repo only *consumes* RSS. This adds the three discovery surfaces that were missing and removes the self-imposed sitemap coverage cap. - New Worker-owned route in `src/server.ts` next to `/sitemap.xml`, so it runs before the SPA catch-all; `/rss.xml` serves the identical bytes and `atom:link rel="self"` always advertises `/feed.xml` so a reader cannot register two feeds. Both paths are in `[assets] run_worker_first`. - Rendered from the existing `getFeed` loader — no second D1 query. The `days` 1-14 clamp and the `before` YYYY-MM-DD validation moved into one exported `feedDaysAndBefore()` shared by `/api/feed` and the feed. - Locale: `resolveApiRequestLocale` / `localeCacheControl` reused verbatim, so the document has exactly the `/api/feed` contract (bare = private + `Vary: Cookie, Accept-Language`; explicit `lang` = public; one legacy `locale` = one 307; invalid/repeated/conflicting = 400). - Every `<item>` is canonical: `<link>` and `<guid isPermaLink="true">` are `storyPath(item, lang)` with explicit `lang`, no UTM and no fragment, so the feed cannot advertise a `noindex` URL regardless of #223. `<pubDate>` is RFC-822 from `published_at` **seconds** through one normalizer (a millisecond row would render a year-2286 date). `<category>` is the scored enum plus normalized topics. `<media:content>` requires both `canonicalizeMediaImageUrl` and `isFetchableUrl`. `dc:creator` is omitted rather than fabricated, and `enclosure` is not used because RSS 2.0 requires a truthful byte `length` we do not have. - Bounds: 100 items, 512-char titles, 600-char descriptions, 8 categories, and a 262,144-byte document assembled under budget rather than trimmed afterwards. A 1,200-item hostile fixture renders 100 items / 175 KB. - `/feed.json` is a thin alias: it rewrites to the existing `/api/feed` route handler, so the document and its bounds are identical by construction. - `<link rel="alternate" type="application/rss+xml">` on the homepage, localized pages, story pages, and /subscribe. - `/sitemap.xml` is now a `<sitemapindex>`: `/sitemaps/static.xml`, one child per UTC publication month (split at 1,000 items per part), and `/news.xml`. This removes `SITEMAP_ITEM_LIMIT`, which silently dropped every story older than the 1,000th newest from the sitemap entirely. - `lastmod` on every `<loc>`, including the 15 static paths. Story `lastmod` is the newest of `published_at`, `fetched_at`, and the latest translation review, so a correction signals an update. - `<image:image>` points at the generated `/api/og/{id}.png` (always 200, 1200x630) for the default-locale story variant. - `/news.xml`: `news:` namespace, `news:publication > news:name` = the aidr publication, `news:language` = the locale actually rendered for the story, W3C `+07:00` publication dates, newest 2 days, hard cap of 1,000 `news:news` entries. Entries beyond the cap stay in the date shards, so nothing is lost. aidr is an aggregator: nothing claims Google News publisher status. - Fail-closed preserved everywhere: every child is `200 application/xml` and a D1 error returns a valid static-only document, exactly like the previous `safeSitemapResponse`. llms.txt `## Consume` gains the feed and news sitemap; SKILL.md and openapi.json document both as machine-readable surfaces; /changelog announces the shape change. robots.txt keeps a single `Sitemap:` line and is untouched (that file and `robotsTxt()` belong to #223). Refs #225, umbrella #232.
… docs (#237) POST /api/mcp was fully admin-gated: checkAuth ran before any JSON-RPC parsing, so an unauthenticated agent got a 401 for everything. Meanwhile the machine-discovery layer promised an anonymous read path that did not exist — the server card advertised capabilities.tools on a public endpoint with no auth declared, the agent card advertised "public-digest" and "story-markdown" skills over MCP, SKILL.md said "MCP (read + admin)", and openapi.json described /api/mcp as a flat 200. Any agent that followed the published docs got a 401 on its first call. The card also advertised capabilities.resources and capabilities.prompts, neither of which existed. The shared read-tool contract (#227 defines it, #226 consumes it) One module, src/lib/public-read-tools.ts, is now the single definition of the four read capabilities: latest_ai_news, search_news, get_story, get_ai_digest. It owns the names, descriptions, input schemas, annotations, argument validation, and the bound constants. Two thin execution adapters issue the queries beside their transport — worker/mcp/public-tools.ts for MCP (D1) and, in #226, src/lib/webmcp.ts for WebMCP (same-origin fetch) — so the transports cannot drift and neither re-describes a tool. Reuses the existing read path: getPublicDigest + boundPublicDigest + PUBLIC_RESPONSE_MAX_BYTES, getFeed (with its 1-14 day clamp and before validation), getStoryCandidates + renderStoryMarkdown. No new D1 query, no new table, no migration, no parallel data path. Auth boundary Anonymous is defined as "no bearer token" — the exact predicate checkAuth itself uses to read one. A request presenting ANY bearer token still gets checkAuth's plain HTTP 401/500 Response, byte for byte, because a client that tried to authenticate and failed must not silently degrade into a read-only session. tools/list returns the public registry for an anonymous caller and public + admin for an admin; the two registries are separate modules so the merge happens once, from the resolved auth state. An anonymous tools/call for anything outside the public registry returns isError:true with one refusal message — byte-identical for "that is an operator tool" and for "no such tool", because comparing against the admin registry IS the enumeration oracle. No names, no counts. Rate limit (hard requirement, not a follow-up) worker/mcp/rate-limit.ts reuses worker/rate-limit.ts — the same checkRateLimit + hashIp + subscribe_attempts(ip_hash, created_at) mechanism the subscribe handler and the admin failed-auth limiter already use, under an "mcp-read:" key namespace. 60 read calls per IP per 60 s; over the limit is HTTP 429 with a JSON-RPC -32000 error and Retry-After. Admin traffic is not limited. Validation runs BEFORE the window is charged, so a rejected argument costs no D1 and no quota. Fails CLOSED if the counter is unavailable — a limiter that fails open is not a limiter. Truthfulness AGENT_DISCOVERY_VERSION 0.1.6 -> 0.1.7 (versioned into openapi.json, agent-card.json, server-card.json; the agent-skills digest is derived from CONSUME_SKILL_MD at request time, so it is recomputed by construction). mcpServerCard() now lists the real tools with schemas and annotations, the resources, the rate limit, and which half needs a bearer token. capabilities.prompts is REMOVED rather than shipped unimplemented. a2aAgentCard()'s public-digest and story-markdown skills are now reachable over MCP. CONSUME_SKILL_MD splits the ambiguous "read + admin" line into an anonymous read line and an explicitly admin-authenticated write line. openApiDocument() gains x-mcp (tool set, auth, rate limit, resources) and real 401/429 responses. /mcp, llms.txt, and /changelog updated. /api/mcp stays noindex, nofollow — route-indexability.ts:323 is untouched and now has a regression test. Untrusted content Every tool is readOnlyHint + untrustedContentHint, and every description carries the trust boundary, because prose in llms.txt is not machine-actionable. Ambiguous id prefixes are rejected, never resolved to a "closest" story, mirroring /api/story/{id}. No new dependencies. No ranking, ingest, prompt, or schema change.
…son (#238) Lighthouse "Agentic Browsing" scored aidr.today 2/3 with "llms.txt does not follow recommendations - Error: File does not appear to contain any links", plus three empty agent surfaces: no WebMCP tools registered, no WebMCP schemas, no ai-catalog.json. The Cloudflare bridge was being loaded (14.42 KiB on the critical path) for zero registered tools, so it was pure cost. llms.txt is now spec-conformant markdown Every endpoint is a real [label](url) link instead of bare "GET https://..." text in a "-" list item, and the missing machine surfaces are linked: /openapi.json, /.well-known/agent-card.json, /.well-known/mcp/server-card.json, /.well-known/ai-catalog.json, the API-catalog linkset, the agent skill path, and /auth.md. Exactly one H1. The RSS feed, its /rss.xml alias, the Google News sitemap, and the sitemap-index wording landed in #236 and are preserved here as links too, so this PR stays independently mergeable over it. No information was lost. The locale/cache contract, the story-text trust boundary, the ranking formula, the submit flow, and the suggest flow are all still there, and a test asserts each of them by content. llms-txt.test.ts now asserts >= 1 markdown link, exactly 1 H1, and that every listed absolute URL resolves against the shared site constants (not a network call) - a non-aidr host must be an intentional external project URL. WebMCP tools, from the same contract module as MCP src/lib/webmcp.ts registers the four read capabilities through document.modelContext.registerTool. It does not re-describe them: WEBMCP_TOOLS IS PUBLIC_READ_TOOLS, and the registration's inputSchema IS the contract's own object. webmcp.test.ts asserts that by identity, so a second list is structurally impossible rather than merely discouraged. The browser has no D1 binding, so WebMCP execute reads the same public endpoints over same-origin fetch and applies the same validators and the same projection + bound as the MCP transport. Validation runs first, so a hostile argument costs one bounded pass and no request. Registration is client-side only. A <WebMcpTools /> component calls registerWebmcpTools() in an effect; it renders nothing. When document.modelContext is undefined the call is a no-op returning 0, never a throw, and /.webmcp/bridge.js is never imported, vendored, bundled, or script-tagged - a test scans the source tree for all three. ai-catalog.json is derived from the same module /.well-known/ai-catalog.json describes the registered tools with their schemas, annotations, REST equivalents, and auth, plus the audited forms. aiCatalogDocument() maps PUBLIC_READ_TOOLS; agent-discovery.test.ts asserts the served document's tool list, annotations, and additionalProperties. Form annotations: the real decision, stated in the document submit and subscribe are agent-callable, with consequentialHint true (publishing to a public feed and committing to recurring mail are both real consequences) and untrustedContentHint true on submit, because url/title/note are third-party text. sign-in and sign-up are NOT exposed. A tool whose schema is { password: string } is a credential-harvesting primitive advertised to every agent on the page, and no host model can distinguish "fill the sign-in form" from "exfiltrate the user's password". Clerk owns those forms and they are not in this codebase, so there is nothing to annotate either. The header SearchBox is not exposed: it filters an already-loaded feed, so the registered search_news tool covers it with real semantics. The story suggest form shares the sign-in credential boundary. All three are listed in ai-catalog.json as agentCallable:false WITH a reason, so a reader sees the decision rather than inferring a silent gap. A test asserts no form annotation ever declares a password/token/secret/otp/code property. Also fixed: boundSearchResult was O(n^2) The bounding step re-serialized the whole payload after every single item removal, so a hostile 4,000-item feed cost 4,000 serializations of a ~16 MB string - the code that exists to prevent a denial of service was one. It is now a binary search over a newest-first prefix: O(log n) measurements, same answer, and the regression is a test with a time bound. No new dependencies. No ranking, ingest, prompt, or schema change. No touch to src/lib/sitemap.ts or apps/web/public/robots.txt.
…dcrumbList) + single site_name constant (#239) * feat(web): server-rendered JSON-LD (NewsArticle/WebPage/ItemList/BreadcrumbList) + single site_name constant The site shipped a full og:/twitter: surface and zero structured data: the production story page and homepage both returned 0 `application/ld+json` blocks and 0 `itemscope` for a Googlebot UA. Rich-result and AI-answer extractors only parse JSON-LD, so none of the ranking, headline, date, or image work the og: tags already did was machine readable. `HeadTags` (apps/web/src/lib/seo.ts) now carries a `jsonLd` graph plus the `scripts` emitter that renders it, so JSON-LD rides the same `head()` -> `HeadContent` path as meta/links and lands in the SSR HTML the crawler already fetches. One `application/ld+json` script per document, carrying a single `@graph` so `@id` references resolve and the extracted block stays one JSON.parse-able object. Per page type: - Story `/{8hex}?lang=vi|en` -> NewsArticle. `headline` is `localizedTitle(item, lang)` — the exact call StoryRow paints into the single `<h1>`, so the graph and the visible headline cannot drift, and a VI request with no `title_vi` emits the real English headline instead of an invented translation. `inLanguage` follows the rendered headline (vi-VN / en-US), so the `en-US` on a `?lang=vi` URL is the machine-readable record of that fallback; `og:locale` keeps describing the page chrome. `datePublished` is `published_at`; `dateModified` is the newest timestamp the story actually carries. `image` is the generated first-party `/api/og/{id}.png` card, locale-suffixed, 1200x630, because upstream `image_url` can 404 — the call articleHead already made. `publisher` is the real outlet from `item_sources` (never aidr, never a fabricated person) and is omitted entirely when no outlet is known. `isBasedOn` carries the original article URL. - Homepage -> WebSite + Organization (`/logo-sm.png`, the real SITE_URL) + WebPage + BreadcrumbList + ItemList. The ItemList is derived from the same arithmetic `TldrSection` paints with — the 8/12/16 selector moved into `lib/tldr-links.ts` and is now shared — so the list can only contain story permalinks that are `<a href>` in the same SSR response, with no invented url or position. - Every static path -> WebPage + BreadcrumbList, self-canonical, with the root crumb at the language-matched homepage. The path list is SITEMAP_STATIC_PATHS, not a second hardcoded list. Gated on `routeIndexability()`: the builders take the route identity and emit no graph at all when the response is noindex (faceted query, tokenized search, 404) or when a caller declares no route. `lib/head-route.ts` reads the raw request query, not the route-validated `match.search`, so a dropped key like `utm_source` still matches the robots meta and `X-Robots-Tag`. Trust boundary: `jsonLdScriptBody` rewrites `<`, `>`, `&` and U+2028/9 into JSON \uXXXX escapes, so a publisher title containing `</script>`, `<!--` or `]]>` cannot terminate the block and still round-trips through JSON.parse. Never emitted, and asserted absent by test: `aggregateRating`, `review`, `author`, `NewsMediaOrganization` / any Google News publisher claim (aidr is an aggregator, not an original outlet), and `WebSite.potentialAction` (no site-search results page exists). No Microdata added. site_name conflict resolved: `SITE_NAME` is now `AI;DR`, the visible header wordmark, recorded with its evidence in site.ts. `og:site_name`, the JSON-LD WebSite/Organization names, and (for the news-sitemap work) one constant; `docs/decisions/telegram-instant-view.md` no longer carries `site_name: null`. No Telegram handle is appended. Refs #224, umbrella #232. * test(web): assert news:name from SITE_NAME so the news sitemap cannot drift from og:site_name
…st tokens (#240) Muted text was a Tailwind `opacity-*` utility on an already-themed color. Opacity composites *after* color resolution, so it blended the text toward whatever surface was behind it — including the per-user reader background (`data-reader-bg`) — producing a pair that no token declared and no test could assert, and a different ratio for every reader background. That was the whole measured failure (Lighthouse, production, 2026-09-27): `<span class="text-xs opacity-70">` on the homepage and story pages, plus `<span class="topic-colored …">` on the muted story rows. - add `--quiet-foreground` / `--primary-foreground-quiet`: opaque, declared steps one notch further from the background than `--muted-foreground`, each clearing AA (4.5:1) on every light and dark surface the app can paint - `.topic-muted` for the trending chip count: the chip hue mixed 35% into the quiet token, so the count keeps the tag's hue and stays quieter than the label without compositing - `.topic-colored` now mixes the palette hue with `--foreground`, the same treatment `.category-colored` already had, so chip labels also clear AA on the muted rows and the gray reader background - reader backgrounds that repaint the page pin the text steps that belong with it, so a system-dark class cannot leave dark text on a light page - hover states that faded the whole element (run strip, Button, mail button, chart legend dim) are declared colors now; the states are kept, not deleted - `contrast-tokens.test.ts` parses the tokens out of styles.css and asserts WCAG 2.2 AA for every reader background x theme x surface x palette slot, the label/count hierarchy, and proves it can fail with broken fixtures Refs #228, umbrella #232
…, per-source + stale observability (#241) Implements #230. Three parts: more verified sources, a declarative source registry that makes adding one a data operation, and per-source + stale observability so a bad source is visible instead of invisible. Sources (16 -> 20 registry rows, no new adapter type; all `rss`, which already handled Atom as well as RSS). Every feed verified live with the production parser and the production flood gate — `pnpm run verify:source-feeds`, 18/18: vnexpress-tech VnExpress Khoa hoc & Cong nghge. The first VI-native source. Declares sourceLang: "vi", which is what puts items on the real VI->EN translation-QA path — no adapter ever emitted it, so that whole branch was dead code. Chosen over Tuoi Tre cong-nghe, which is also a working VI feed but whose pubDate has no timezone and would land every item ~7h in the future. techcrunch-ai / theverge-ai / arstechnica-ai / wired-ai Evaluated and rejected, reason recorded in the code and the migration header: arXiv (export.arxiv.org is robots Disallow-all and arxiv.org lists Disallow: /api, so the sortable API is out; the one allowed surface, rss.arxiv.org, declares skipDays and was serving an empty channel so it could not be verified live), VentureBeat (429; its FeedBurner mirror is 24 days stale), Engadget (202 challenge), ZDNet (its "AI topic" RSS redirects to general news), cafebiz/techrum/zingnews (404/404/403). Declarative registry. worker/sources/catalog.ts is now the only place a source is declared; the runtime seed, the generated migration, and the /api/system/sources + /data surfaces all consume it, and a test asserts all three agree byte for byte. This replaces the hand-copied rows in seed.ts and migrations 0018/0020/0021/0022 that seed.ts's own header called a maintenance hazard. The seed upserts name/type/config on conflict so a corrected feed URL actually reaches an existing row, and deliberately never writes `enabled`, so switching a noisy source off survives every run. Rows the registry does not declare stay entirely operator-owned, and adding an rss source through the existing upsert_source admin tool still needs no deploy — documented with copy-pasteable curl in worker/README.md. Per-source observability. workflow_runs.stats.sourceHealth carries fetched / scored / accepted / rejected / merged per source, a closed skip-reason enum (fetch_failed, parse_failed, empty, all_rejected_below_relevance, disabled), and a consecutive-empty-run streak. A 200 that is really an HTML page now reports parse_failed rather than looking like a quiet feed, via a typed SourceFetchError whose message is sanitizeError'd before it can reach D1 or an API response. Streaks are carried forward from the previous run's stats (one single-row read) rather than recomputed, so surfacing staleness on the read path costs nothing. Threshold is 168 consecutive runs (7 days at the hourly cadence), which is measured rather than round: when the registry was verified live, 11 of 18 feeds had nothing inside the 26h window, six of them pre-existing (lastweekin-ai is a weekly newsletter, google-research publishes a few times a week), so the "e.g. 48 runs" in the issue would have flagged healthy sources most of the weekend. A row may override with staleAfterRuns. Disabled is never reported stale — off is a decision, not a fault. Vietnamese path. The adapter reads config.sourceLang and only "vi" counts, so a typo cannot put an item on the wrong side of the QA path. A new test reproduces the real production failure observed in /api/system's llm_calls (the EN generator works, both configured reviewer ids 404) and asserts nothing is accepted, the original candidate is never overwritten, and a review_failed attempt is persisted with a scheduled retry. The 3-attempts -> terminal human_review -> one authenticated retry transition is already proven end to end in translation-qa.integration.test.ts. One pre-existing gap is pinned by a test rather than fixed here: if EN *generation* fails there is no candidate to review, so no human_review row is created. Flood gate. applyFloodGate runs a named keyword pre-filter (the same regex the HN adapter uses, now in one shared module) and then a newest-first maxItems cap, both config-driven. Newest-first is what makes the cap safe: the 26h window means the head of the feed at the next run is what was published since the last one, so the cap samples the live edge and dedupe drops the rest. Exercised against a 640-entry fixture through the real adapter. No ranking, hide-rule, prompt, or LLM-budget change. rank_score and the relevance < 0.4 rule are untouched and worker/ranking.ts is not modified; no new source is boosted or penalised. No hang-cap, batch size, or model chain is changed. The added volume fits the existing budgets and the arithmetic is asserted in a test: five new rows at maxItems 6 is a per-run ceiling of 30, or 6 batches, 2 concurrent rounds, 140s worst case inside the 240s score step.
Refs #229, #232. LCP element render delay 1,909 ms -> 1,121 ms (cold Slow-4G / 4x CPU, median of 3). TBT 1,647 -> 763 ms. CLS is unchanged at 0.089; the font-swap shift is gone but a residual 22px re-wrap of one AI;DR row survives, and the <200 ms bar is not met. All reported in the PR with the isolation runs that show where the remaining time goes. The trace contradicts the issue's diagnosis in two places, and both are documented: - The Fontsource @imports were build-time inlined by Vite, not a runtime round trip. What was real is the second half: the font fetches could not start until the 19 KB render-blocking stylesheet had been parsed (fonts started 1,752 ms, landed 2,705-4,287 ms, shift at 3,537 ms). Fonts are now self-hosted in src/fonts.css with font-display: optional and metric-matched local() fallbacks whose overrides are measured out of the shipped binaries by scripts/font-metrics.py, not guessed. - The issue asked for `optional` + preload. Preloading under `optional` is a 316 ms regression: a face only lands inside the ~100 ms block period if 28 KB beats a 1.6 Mbps link, which it cannot, so the bytes are fetched, compete with the stylesheet, and are discarded. The preload is removed and pinned by FONT_PRELOAD_DECISION plus a test that fails if one is added back without re-reading the numbers. The LCP element is a text row, already in the SSR body at byte 29,218 — it was never gated on hydration. What blocked it was four third-party `fetchpriority=high` image preloads React hoisted into the head *ahead of* the render-blocking stylesheet. StoryThumb keeps them eager (identical pixels) and hands the priority back. The five analytics bootstraps move from React's commit phase to idle after the LCP paint, and the Clarity vendor snippet stops doing insertBefore on a document that may have no <script>. Vietnamese coverage is asserted, not assumed: latin alone drops 47 of the 87 characters the Vietnamese alphabet needs, and the live feed's U+0101/U+014D loanword macrons are why latin-ext cannot be deleted either. Also: preconnect j.duyet.net and clarity.ms; 30-day TTLs for the unhashed logos plus /og-home.jpg, which had no rule and was getting the Workers default max-age=0; and the forced reflow in useHorizontalScroll, which wrote two custom properties unconditionally on every scroll event. The font subset was NOT reduced. Real subsetting needs a new dependency, which the issue rules out; the per-subset reasons are in the PR. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
duyet
marked this pull request as ready for review
September 28, 2026 19:25
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up hardening for #180, on top of the transport fix merged in #181. Rebased
onto current
master(9ba2498); still a draft, nothing merged or deployed.A. The server-function id is validated against Start's real manifest
The previous pass only rejected empty and nested transport paths. Any
single-segment id still reached
getServerFnById, and an id absent from thebuild-generated manifest made that throw an unhandled error — a
500whosemessage embeds the requested id, skipping Start's serialized-envelope
contract entirely. That covers stale ids from a renamed function still held by
an already-served client bundle, and every percent-encoded traversal form,
neither of which the old shape check could see.
How the lookup works
src/lib/server-fn-registry.ts(new, server-only) imports#tanstack-start-server-fn-resolver— the same generated module theserver-function handler itself uses — and asks it whether an id is
registered. A typed ambient declaration (
src/types/…-resolver.d.ts) keepstschonest. The virtual module exists only in the server environment, sothis file is imported by
server.tsalone and never reaches the clientbundle (verified: zero occurrences in
dist/client).server-fn-request.ts(which the client graph does pull in) breaks theclient build, because the Vite plugin materializes the virtual module for the
server environment only.
nulland the Worker defers toStart. Unit tests and any non-Start consumer therefore keep the full fix(web): stop legacy story redirects from hijacking server functions (/submit "Failed to submit") #181
behavior rather than 404-ing every real function, and an infrastructure
failure can never retire the transport. For a registered id the lookup warms
the resolver's own memoized module cache, so the handler's later lookup costs
nothing extra.
How the shape check works
serverFnIdOfreplaces the shape-only test with a charset covering bothalphabets Start's compiler can emit —
base64urlin dev (the encoded modulespecifier plus export name),
sha256hex in a build,_Non a dedup collision— bounded to 200 characters. Nothing Start emits contains
.,%,/,\,+, or=, so percent-encoded traversal, encoded separators,./.., nulbytes, standard-base64
+/=, and over-long segments are retireddeterministically, before any resolver is consulted. The charset is pinned
by a test that builds a real dev id the same way the compiler does.
On the 200-character bound. A build id is 64 hex characters. A dev id is
the base64url of a
{"file":…,"export":…}descriptor, so its length tracks thedev module specifier — observed up to 139 characters across this app's
functions. 200 is a deliberate safety ceiling, not a derived maximum: it
leaves headroom over the observed output while still refusing an arbitrarily
long segment outright instead of carrying it into the resolver. A hypothetical
dev id longer than the ceiling would fail closed into the bounded JSON 404
rather than the old unhandled 500, and production ids cannot approach it.
Object.prototypemembers are rejected by an explicit own-property check.constructor,toString,__proto__,valueOfand friends satisfy the idcharset, and the generated resolver reads
manifest[id]— so each one passesthe resolver's own "not found" guard and only fails a line later on
importer(). Recognising them therefore depended on catching thatTypeError,i.e. on how the generated lookup happened to be written.
Object.hasOwn( Object.prototype, id)makes the rejection deliberate and local, so the verdictno longer depends on the resolver at all.
The outcome was already a bounded 404 — a Worker-level test pins that, and
it still passes with the guard removed. What changes is that the rejection is
now reached on purpose rather than by accident; the new registry test asserts
the resolver is never consulted for these keys, and fails without the guard.
The resolver-failure fallback to Start is unchanged: a resolver that cannot be
loaded still answers
nulland defers.The bare base is now bounded too.
/_serverFnis not the transport path,so Start answered it as an ordinary unknown route and handed a document back to
a request that claimed to be RPC. Both
/_serverFnand/_serverFn/*arereserved;
/_serverFnx,/_serverFnx/abc, and/_serverFnOLDare notclaimed (asserted).
Nothing is weakened or disclosed
Every rejection is the same fixed body —
{"error":"server_function_not_found","message":"Unknown server function.","message_vi":"…"}— with
private, no-store,Content-Language: en, vi,Referrer-Policy: no-referrer,X-Robots-Tag: noindex, nofollow. No id, no stack, no functionname, no module name. It discriminates only on structure and manifest
membership, never on a credential, so it adds no enumeration oracle: an
unknown-but-well-formed id and a malformed path are indistinguishable, and
auth-bearing RPC still fails inside Start with its own serialized
Error("Sign in required").run_worker_firstis described correctly nowThe previous body overclaimed this as the mechanism. It is not:
run_worker_firstis already an array on master, and the array formalready disables
assets_navigation_prefers_asset_servingrepo-wide, so thenavigate-skip is already off.
byte-identical. Zero routing delta.
It is kept as explicitness — it pins the transport to the Worker so the bounded
404 also holds for a navigation request — and the
wrangler.tomlcomment nowsays exactly that. The fix is in
server.ts.The one narrow behavior change, stated explicitly
Two things move, and only for requests whose
Acceptheader is notHTML-shaped.
isHtmlRequestis the byte-exact negation of Start's own HTML-onlyguard, so the boundary is precisely "would Start have served a document here?".
normalizeLocaleRequestis called withformat: "json"instead of"html"when theAcceptheader is notHTML-shaped. A bad locale on such a request answers
400 application/json(invalid_locale/repeated_locale/conflicting_locale) instead of a 400 HTML page. The body, status, cachepolicy,
Content-Language: en, viandX-Robots-Tagare the same ones theJSON path already used.
allowRedirect: falseskips both redirect branches -- the neutral-path?lang=normalization and the legacy?locale=canonicalization -- for thosesame requests, so a non-HTML request is not handed a document redirect. It
falls through to Start, which answers with its own
406 {"error":"Only HTML requests are supported here"}. The same guardapplies to the
/extension->/subscriberedirect and to the legacy/{cat}/{hash}story redirect.Unchanged, and measured as unchanged against master: every browser
navigation (
Accept: text/html,...), every*/*request including plaincurl, all API paths, all static surfaces, and every RPC request.allowRedirectstays
truefor API paths, so/api/*redirect and locale-error behavior isuntouched. Concretely, a
curlof a legacy story URL still gets its307tothe canonical path; only an
Accept: application/jsonclient now gets Start's406instead of a document redirect, which is the more useful answer for aprogrammatic caller.
B.
Brand.test.tsxexercises the real routerThe old test mocked
Link, sonot.toContain("payload")andnot.toContain("locale")only ran against the mock's ownURLSearchParamslogic.
Proof the old assertions were vacuous: mutating the app's real
validateRootSearchto injectpayload: "stale-router-payload"— precisely theleak the assertion exists to catch — passes the old test (2/2) and fails
the new one with
/?lang=vi&payload=stale-router-payload.The replacement drives a real TanStack router: memory history, the app's real
validateRootSearchandpreserveRootLangFromMiddleware, andBrandmountedas a layout above
Outletthe wayHeaderBarmounts it. It reads the href thereal
Linkbuilds, cross-checks it againstrouter.buildLocation, clicksthe link and asserts the resulting location and search keys, and covers the
VI-copy / EN-navigation split plus stability across a route change.
It also catches three direct
Brand.tsxregressions: leakinglocaleintosearch(7/7 fail), droppingsearchentirely (7/7 fail), and using thecontent locale instead of the navigation locale (3/7 fail).
Verification
Gates —
pnpm run lint,pnpm run check-types,pnpm run build(+
Clerk proxy config assertion passed), extensionlint/test/build.Counts are measured on this exact head, not estimated. They drift as
masterlands: an earlier revision of this body quoted 156/1611, and
masterhas sincetaken #199, which added the deployed-homepage OG assertions.
Local workerd smoke, real production bundle, all seven manifest ids
Transport: 62/62 (the 8 new cases are the
Object.prototypekeys). Every real id (correctGET/POSTper function) stillreaches its handler as a serialized envelope with no
Location,Cache-Control,Content-Language, orX-Robots-Tagstamped on it. Hostilepaths use
curl --path-as-is, becausefetch()normalizes./..awaybefore the request leaves the client and would silently stop testing the thing:
POST /_serverFn/·/_serverFn//·/_serverFn/abc/extra404 server_function_not_found, no leak404, bounded, id not echoedbase64urlid404, bounded — never the old unhandled500%2e%2e%2f…,%2Fetc%2Fpasswd,..%2f..%2fadmin,abc%00,.,a+b=c,a\b, 201 chars404, bounded/_serverFn(plain and navigate-shaped)404JSON, never a document?lang=fr/?lang=vi&lang=en/?locale=fr/?lang=vi&locale=vi400invalid_locale/repeated_locale/conflicting_locale?lang=fr404, not a locale errorconstructor,toString,valueOf,hasOwnProperty,isPrototypeOf,propertyIsEnumerable,toLocaleString,__proto__404bounded, noTypeError/importerin the body_serverFnx,_serverFnx/abc,_serverFnOLD/api/public,/robots.txt,/sitemap.xml,/llms.txt,/auth.md,/logo.svg/_serverFn/..is called out honestly: workerd resolves it against the requestorigin before the Worker runs, so the Worker sees
/and it is an ordinaryhomepage fetch. Asserted as such, not papered over.
EN/VI navigation: 29/29. Real SSR HTML from the production bundle —
/,/changelog,/about,/privacy,?lang=en,?lang=vi,news_lang=vi,news_lang=en, and legacy?locale=(canonicalized, then followed). Everylogo variant (desktop and mobile) shares one canonical href
/?lang=<resolved>, with nolocaleand nopayload; the page ships theclient router bundle and
data-tsrhydration state.Differential vs master: the only changed rows remain the 5 malformed-path
rows and the 11 non-HTML-document rows from the earlier pass. All browser
navigations, all
curl(*/*) redirects,/api/*, static surfaces,GET /api/admin/status→401,POST /api/admin/ingest→401, andPOST /api/subscribe→400are unchanged.Not run here: a real-browser hydration click. No browser is attached to
this session and none is installed locally, so hydration/URL-cleanup coverage
rests on the memory-history router test plus the SSR-href assertions. Worth one
pass in a desktop browser before merge.
Notes
linked issue. Local verification used throwaway placeholder Clerk values in
gitignored
.dev.vars(since deleted). Clerk cannot complete a handshakeagainst a placeholder key, so SSR was exercised with a temporary
requestMiddleware: []that was restored from git and re-verified absentbefore the final build and push.
mastermoved twice more during this pass (fix(web): assert the deployed homepage og-home card in smoke #199, test(web): replace hardcoded server-fn hex with synthetic fixture #201). test(web): replace hardcoded server-fn hex with synthetic fixture #201 replaced thelive 64-char hex literal in the fix(web): stop legacy story redirects from hijacking server functions (/submit "Failed to submit") #181 tests with a synthetic fixture; this
branch rebased onto it and adopted the same convention for the fixtures added
here, so the suite no longer carries a live production id anywhere.
GitGuardianinitially flagged two test fixtures, both false positives, nowremoved rather than suppressed: a truncated
base64urlid that read as acredential, and a
..%2f..%2fetc%2fpasswdtraversal case that its passworddetector matched. The traversal case keeps its meaning with a different
target, and the production id is now assembled rather than pasted. The real
submitStoryid is still covered end-to-end against the built bundle.deploy, the same gate fix(web): stop legacy story redirects from hijacking server functions (/submit "Failed to submit") #181 asked for.
Refs #180
Related: #181