growth(day-30): @byok-relay/hono — Hono middleware + route factory for Cloudflare Workers, Deno, Bun#66
growth(day-30): @byok-relay/hono — Hono middleware + route factory for Cloudflare Workers, Deno, Bun#66alokit-bot wants to merge 7 commits into
Conversation
packages/solid/src/index.js: four stores with SolidJS signal contract ([getter,setter]): - createByokRelayStore: token registration + key CRUD + logout (localStorage persistence, SolidStart SSR-safe via typeof window guard) - createChatStore: stateful message list, non-streaming, openai/anthropic/groq/mistral/openrouter, systemPrompt + extraParams - createStreamingChatStore: SSE streaming with AbortController, stopStreaming(), streamingContent() live signal, partial-commit on abort - createRelayHealthStore: polls /health on configurable interval, refetch(), destroy() for onCleanup() lifecycle management Signal shim: all stores work without solid-js installed (signals degrade to plain getter/setter pairs for testing and non-Solid environments). 20 smoke tests passing (node test/stores.test.js). packages/solid/README.md: full API docs, SolidJS quick-start, SolidStart SSR guide, all four stores documented, provider table, self-hosting section, related packages table. llms.txt: SolidJS Signals section + npm link. README.md: SolidJS stores section before 'For AI coding agents' with install + streaming example + links to other framework packages. package.json: workspaces added. metrics/daily.jsonl: 2026-07-03 snapshot (stars=52 forks=0 watchers=1 clones=110 views=20). Next for Avi: cd packages/solid && npm publish --access public
… BYOK AI packages/angular/src/index.js: four Angular injectable services: - ByokRelayService (token registration + key CRUD + logout, localStorage persistence, Analog SSR-safe in-memory fallback) - ChatService (stateful message list, non-streaming, openai/anthropic/groq/mistral/openrouter, systemPrompt + extraParams, rollback on error) - StreamingChatService (SSE streaming with AbortController, stopStreaming(), streamingContent() live signal, partial-commit on abort) - RelayHealthService (polls /health, check(deep=true) for upstream ping, destroy() for ngOnDestroy cleanup) Angular signal shim: services use Angular 16+ signal() when @angular/core is detected; plain getter/setter shim otherwise — same reactive API in both cases. provideByokRelay(config): makeEnvironmentProviders() with all four services wired together via deps when @angular/core is installed; plain createByokRelayBundle() otherwise (for testing and standalone use). createByokRelayBundle(config): factory function for standalone / non-Angular usage (Analog islands, unit tests, vanilla JS). 29 smoke tests passing (node test/services.test.js, no DOM, no @angular/core required): - ByokRelayService: 12 tests (register, getOrRegister, storeKey, listKeys, deleteKey, rotateKey, logout, error signals) - ChatService: 5 tests (send, rollback, clear) - StreamingChatService: 4 tests (stream, stopStreaming partial-commit) - RelayHealthService: 5 tests (check, deep check, error, polling lifecycle) - createByokRelayBundle: 2 tests (service wiring) packages/angular/README.md: full API docs, Angular 14+ quick-start, Angular 16+ signals note, Analog SSR guide, provider table, self-hosting section, related packages table. llms.txt: Angular Injectable Services section + npm link. README.md: Angular services section after SolidJS stores. package.json: workspaces already includes packages/*. metrics/daily.jsonl: 2026-07-04 snapshot (stars=52 forks=0 watchers=1 clones=112 views=22). Next for Avi: cd packages/angular && npm publish --access public
… adapter Implements LanguageModelV1 spec so byok-relay can be used as a first-class provider with generateText, streamText, generateObject, and all AI SDK tools. packages/vercel-ai/src/index.js: - createByokRelayProviderSync(config): synchronous factory, lazy token registration; module-scope safe for Next.js / SvelteKit / Nuxt initialisation - createByokRelayProvider(config): async factory, eager token registration - createLanguageModel(): LanguageModelV1 implementation — specificationVersion=v1, doGenerate() (non-streaming, full OpenAI response parsing, tool_calls, finish_reason), doStream() (SSE ReadableStream, text-delta / tool-call-delta / finish / error parts) - parseModelId(): 'provider/model' or bare model (default: openai) - promptToMessages(): AI SDK LanguageModelV1Prompt → OpenAI messages array, handles system/user/assistant/tool roles, multi-part content, image_url conversion - provider.storeKey(provider, apiKey): store user's API key in relay - provider.health(deep?): relay liveness/readiness check - provider.stats(appId?): per-user usage stats - provider.deleteAccount(): GDPR erasure packages/vercel-ai/test/smoke.test.js: 48 smoke tests, all passing packages/vercel-ai/README.md: full API docs, Next.js/SvelteKit/Nuxt examples, all supported providers, AI SDK feature matrix, related packages table llms.txt: Vercel AI SDK Custom Provider section + npm link README.md: Vercel AI SDK section after Angular (before 'For AI coding agents') metrics/daily.jsonl: 2026-07-04 snapshot (stars=52 forks=0 watchers=1 clones=112 views=22) Next for Avi: cd packages/vercel-ai && npm publish --access public
packages/preact/src/index.js: four hooks with runtime hook resolution (preact/hooks → react → inline shim): useByokRelay (token registration + key CRUD + logout, SSR-safe localStorage), useChat (stateful non-streaming, openai/anthropic/groq/ mistral/openrouter, systemPrompt + extraParams), useStreamingChat (SSE streaming with AbortController, stopStreaming(), streamingContent live string, partial-commit on abort), useRelayHealth (polls /health, check(deep?), intervalMs control). Hook shim ensures the package works in test/SSR environments with no preact installed. 24 smoke tests passing. packages/preact/README.md: full API docs, Astro component island quick-start, all four hooks documented, provider table, self-hosting section, related packages table. llms.txt: Preact Hooks section + npm link. README.md: Preact hooks section after Angular (before 'Vercel AI SDK') with Astro island code example. metrics/daily.jsonl: 2026-07-05 snapshot (stars=52 forks=0 watchers=1 clones=112 views=22). **Next for Avi:** cd packages/preact && npm publish --access public.
…okRelayClient (Growth Day 28) - packages/astro/src/index.js: createByokRelayMiddleware (onRequest handler, proxies /api/relay/* server-side, keepss RELAY_URL private), createRelayApiRoute (catch-all Astro API route factory, all HTTP methods, optional app_id whitelist), ByokRelayClient (plain-JS class for <script> blocks / View Transitions: register, ensureToken, logout, storeKey, listKeys, deleteKey, rotateKey, chat, streamChat with AbortController, health, stats, getModels, deleteAccount). SSR-safe window.localStorage fallback for server render context. - packages/astro/README.md: full API docs, SSR quick-start (API route + script block), middleware quick-start, static/client-only mode, streaming example (View Transitions), ByokRelayClient API reference table, provider table, related packages table. - llms.txt: Astro Integration section + npm link (before Links). - README.md: @byok-relay/astro section before Preact hooks with API route + script block example. - metrics/daily.jsonl: 2026-07-06 snapshot (stars=52 forks=0 watchers=1). - 33 smoke tests passing (node test/integration.test.js). Next for Avi: cd packages/astro && npm publish --access public
…+action factories, React hooks, ByokRelayClient packages/remix/src/index.js: createRelayLoader (Remix LoaderFunction catch-all; maps params['*'] to relay sub-path; hop-by-hop header filtering; optional app_id allowlist; 403 on disallowed app), createRelayAction (ActionFunction; forwards POST/PUT/PATCH/DELETE with original body; optional app_id allowlist), useByokRelay (token registration + key CRUD + logout, localStorage persistence, auto-register on mount), useChat (stateful non-streaming, openai/anthropic/groq/mistral/openrouter, systemPrompt + extraParams), useStreamingChat (SSE streaming with AbortController, stopStreaming(), streamingContent live string, partial-commit on abort), useRelayHealth (polls /health, refetch(), check(deep?) readiness probe, intervalMs control), ByokRelayClient plain-JS class (register, ensureToken, logout, storeKey, listKeys, deleteKey, rotateKey, chat, streamChat with onChunk/onDone callbacks, health/deep, stats, getModels, deleteAccount — works in both loaders (server) and browser scripts). 32 smoke tests passing (node test/remix.test.js). packages/remix/README.md: full API docs, Remix v2 quick-start (createRelayLoader + createRelayAction catch-all route + window.ENV pattern), React Router v7 framework mode guide, all hooks documented with props/return tables, ByokRelayClient method table, provider table, self-hosting note, related packages table. llms.txt: Remix / React Router v7 Integration section + npm link. README.md: @byok-relay/remix section before Astro SSR integration with loader+action + useStreamingChat examples. metrics/daily.jsonl: 2026-07-06 snapshot (stars=52 forks=0 watchers=1 clones=105 views=24). Key differentiator vs @byok-relay/react: server-side loader/action pattern keeps RELAY_URL in process.env only; browser never sees the upstream relay URL — meaningful security improvement for production Remix deployments. Next for Avi: cd packages/remix && npm publish --access public.
…r Cloudflare Workers, Deno, Bun packages/hono/src/index.js: createByokRelayMiddleware (Hono MiddlewareHandler; intercepts pathPrefix; reads RELAY_URL from c.env binding so it stays server-side; hop-by-hop header filtering; optional app_id allowlist); createRelayRoute (Hono Handler for explicit catch-all route registration; all HTTP methods; optional allowlist); ByokRelayClient plain-JS class (register, ensureToken, logout, storeKey, listKeys, deleteKey, rotateKey, relayRequest, chat, streamChat async generator with AbortSignal support, health, stats, getModels, deleteAccount; in-memory storage on edge runtimes with optional custom adapter for Workers KV / Deno KV). 24 smoke tests passing (node test/middleware.test.js). packages/hono/README.md: full API docs, Cloudflare Workers quick-start (c.env.RELAY_URL binding, never in bundle), Bun/Node.js explicit route quick-start, Deno Deploy quick-start, middleware vs route factory API tables, streaming chat in Workers TransformStream example, all provider table, self-hosting note, related packages table. llms.txt: Hono Middleware section (createByokRelayMiddleware + createRelayRoute + ByokRelayClient, Workers + Bun snippets, npm link). README.md: Hono middleware section after Vercel AI SDK (before 'For AI coding agents') with Workers binding example + Bun/Node catch-all example. metrics/daily.jsonl: 2026-07-07 snapshot (stars=52 forks=0 watchers=1 clones=105 views=24). Key differentiator vs astro/remix: targets stateless edge runtimes (Cloudflare Workers, Deno Deploy) where localStorage is unavailable; c.env binding keeps RELAY_URL in Workers Secrets, never in the bundle; custom storage adapter enables Workers KV persistence. Next for Avi: cd packages/hono && npm publish --access public.
📝 WalkthroughWalkthroughThis PR adds seven new monorepo packages under packages/* (angular, astro, hono, preact, remix, solid, vercel-ai), each providing framework-specific BYOK relay integrations (services/hooks/stores/middleware/provider adapters), with documentation, manifests, and smoke tests. Root README.md, llms.txt, package.json (workspaces), and a metrics data file are also updated. ChangesNew framework integration packages
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant FrameworkLayer as Framework Hook/Store/Service
participant RelayServer
Client->>FrameworkLayer: register / storeKey / chat / streamChat
FrameworkLayer->>RelayServer: proxied fetch with token/app_id
RelayServer-->>FrameworkLayer: JSON response or SSE stream
FrameworkLayer-->>Client: state update (messages, token, health)
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (22)
metrics/daily.jsonl-1-4 (1)
1-4: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNormalize the metrics JSONL schema and dedupe the date.
These rows mix two record shapes (
clones_14d/views_14dvsclones/views) and repeat2026-07-06, which makes the series ambiguous for downstream parsers. Emit one consistent schema per day, or add an explicitkind/windowfield if both snapshots are intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@metrics/daily.jsonl` around lines 1 - 4, The daily metrics JSONL currently mixes two schemas and repeats the same date, so normalize the records in the daily stream using a single shape in the metrics generation path and ensure each date appears only once. Update the producer that writes daily.jsonl so it consistently emits either the 14d-window fields or the short-form fields, and if both snapshot types are required, add an explicit discriminator such as kind or window in the record builder to distinguish them. Refer to the daily metrics writer/serializer that produces the dated rows and dedupes by date before writing.README.md-28-43 (1)
28-43: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winImport the Solid helpers and avoid
windowin the Remix example.
The Solid snippet uses<For>and<Show>without importing them, and the Remix snippet readswindow.ENV.RELAY_URLduring render, which breaks SSR. Pass the relay URL from the loader or another server-safe source.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 28 - 43, The Solid and Remix examples need SSR-safe imports and data access fixes: in the Solid snippet, import the Solid helpers used by the JSX, namely For and Show, alongside the store functions; in the Remix example, remove the direct window.ENV.RELAY_URL read from render and instead pass the relay URL through the loader or another server-provided prop into the component, using the existing App and loader setup to keep it server-safe.packages/angular/src/index.js-29-36 (1)
29-36: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGoogle provider path is never populated —
{model}placeholder is not substituted.
PROVIDER_PATHS.googleis'models/{model}:generateContent', but bothChatService.sendMessage(Line 288) andStreamingChatService.streamMessage(Line 397) build the request path withPROVIDER_PATHS[provider] || 'chat/completions'and never replace{model}with the actualmodelvalue. Any request withprovider: 'google'will hit a URL literally containing the string{model}, breaking the Google integration that's advertised in the README's provider table (Line 211).🐛 Proposed fix (apply in both ChatService.sendMessage and StreamingChatService.streamMessage)
- const path = PROVIDER_PATHS[provider] || 'chat/completions'; + const path = (PROVIDER_PATHS[provider] || 'chat/completions').replace('{model}', model);Also applies to: 287-296, 396-406
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/angular/src/index.js` around lines 29 - 36, The Google provider path is being used as a static string, so `ChatService.sendMessage` and `StreamingChatService.streamMessage` must substitute the actual model into `PROVIDER_PATHS.google` before building the request URL. Update the path construction in both methods to detect `provider === 'google'` and replace the `{model}` placeholder with the current `model` value, while leaving the other provider paths unchanged.packages/astro/src/index.js-90-96 (1)
90-96: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrefix match lacks segment boundary check.
url.pathname.startsWith(pathPrefix)matches any path that merely begins with the prefix string, not just paths under it as a segment. E.g. withpathPrefix: '/api/relay', a request to/api/relaygroundalso matches, andsubPathbecomes'ground', silently hijacking an unrelated route into the relay proxy instead of callingnext().🐛 Proposed fix
- if (!url.pathname.startsWith(pathPrefix)) { + const isPrefixMatch = url.pathname === pathPrefix || url.pathname.startsWith(pathPrefix + '/'); + if (!isPrefixMatch) { return next(); }Also applies to: 90-96
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/index.js` around lines 90 - 96, The path prefix check in the relay proxy logic is too permissive because `url.pathname.startsWith(pathPrefix)` also matches longer unrelated paths; update the matching in the `index.js` relay handler so only true segment-boundary matches are accepted before computing `subPath` and `upstreamUrl`. Use the existing relay route logic around `pathPrefix`, `subPath`, and `next()` to require either an exact match or a following slash, and keep non-matching paths flowing to `next()` instead of proxying them.packages/astro/src/index.js-81-241 (1)
81-241: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSignificant duplication between
createByokRelayMiddlewareandcreateRelayApiRoute.Header filtering, upstream URL construction, the
allowedAppsguard, fetch/error handling, and hop-by-hop stripping are near-identical across both factories (Lines 94-151 vs 182-237). Extracting a sharedproxyRequest({ relayUrl, allowedApps, request, subPath })helper would remove ~60 lines of duplication and keep future fixes (e.g. the timeout/content-encoding issues above) in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/index.js` around lines 81 - 241, `createByokRelayMiddleware` and `createRelayApiRoute` duplicate the same proxy logic for URL construction, `allowedApps` validation, hop-by-hop header filtering, fetch setup, and error handling. Extract that shared behavior into a reusable helper such as `proxyRequest` (taking `relayUrl`, `allowedApps`, `request`, and `subPath`) and have both factories call it, keeping only the middleware-specific path handling in each wrapper.packages/astro/src/index.js-132-132 (1)
132-132: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo timeout on upstream relay fetch.
fetch(upstreamUrl, init)has noAbortController/timeout. On the Node.js target (this package explicitly notes "Server-side helpers use the Nodefetchglobal"), a slow or hanging relay will hold the request open indefinitely, tying up the request thread/handler.🛡️ Proposed fix (apply to both handlers)
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), opts.timeoutMs || 30000); try { - const upstreamResp = await fetch(upstreamUrl, init); + const upstreamResp = await fetch(upstreamUrl, { ...init, signal: controller.signal }); + clearTimeout(timeoutId); ... } catch (err) { + clearTimeout(timeoutId); ... }Also applies to: 220-220
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/index.js` at line 132, The upstream relay fetch currently has no timeout, so a slow or hanging upstream can block the handler indefinitely. Update both relay handlers that call fetch(upstreamUrl, init) to use an AbortController with a timeout, pass its signal through init, and ensure the controller is cleaned up after the request completes or aborts. Keep the timeout handling consistent across both call sites so the Node fetch global cannot hold the request open forever.packages/astro/src/index.js-99-107 (1)
99-107: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
allowedAppsguard is trivially bypassable — not a real authorization control.The allowlist check trusts a client-supplied
x-app-idheader orapp_idquery param (Lines 100-101, 188-189). Any caller can simply set that header/param to a value from the allowlist to bypass the restriction; it provides no actual authentication of the calling app. Docs (README lines 105, 183, 201) present this as a security allowlist ("whitelist"/"guard"), which overstates the protection offered.Consider documenting this clearly as a routing/bucketing convenience rather than a security boundary, or requiring a signed/verified token instead of a self-reported header for real access control.
Also applies to: 187-195
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/index.js` around lines 99 - 107, The allowedApps check in the request handling logic is only a client-supplied routing filter, not an authorization boundary, because it trusts x-app-id and app_id values that callers can spoof. Update the docs and any related comments around the allowlist handling in the request path and the shared validation logic so they clearly describe it as a convenience/bucketing guard, or replace it with a verified token-based check if real access control is intended. Refer to the request guard in the main handler and the corresponding allowlist check in the later request path so both are consistent.packages/astro/src/index.js-134-145 (1)
134-145: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStrip
content-encodingandcontent-lengthfrom proxied responsesNode fetch in this package can decompress the upstream body while leaving those headers intact, so forwarding them makes the payload metadata wrong and can break clients that re-decode or trust
content-length. Remove those two headers before building theResponsein both forwarding paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/index.js` around lines 134 - 145, The proxied response header copying in the Response निर्माण path is preserving upstream content metadata that can become stale after Node fetch decompresses the body. Update the header filtering logic in the response-building code that iterates over upstreamResp.headers.entries() so it also removes content-encoding and content-length before constructing the new Response, and apply the same fix in both forwarding paths that return proxied responses.packages/solid/src/index.js-113-121 (1)
113-121: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSSE parser doesn't buffer partial lines across chunk reads.
parseSSEsplits each independently-decodedchunkon\nand tries toJSON.parseeachdata:line immediately. If areader.read()boundary falls in the middle of an SSE line (common with chunked transfer/slow networks), the split line will be incomplete on both sides and silently failJSON.parse(swallowed by thecatch), dropping content instead of corrupting the visible output but still losing data.🩹 Proposed fix — buffer partial lines across reads
const reader = res.body.getReader(); const decoder = new TextDecoder(); let accumulated = ''; + let buffer = ''; while (true) { const { done, value } = await reader.read(); if (done) break; const chunk = decoder.decode(value, { stream: true }); - for (const event of parseSSE(chunk)) { + buffer += chunk; + const lines = buffer.split('\n'); + buffer = lines.pop(); // keep incomplete trailing line for next read + for (const event of parseSSE(lines.join('\n'))) { const delta = extractDelta(event, provider); if (delta) { accumulated += delta; setStreamingContent(accumulated); } } }Also applies to: 459-471
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/solid/src/index.js` around lines 113 - 121, The SSE parsing in parseSSE currently treats each chunk as complete, so partial lines split across reader.read() boundaries are lost when JSON.parse fails. Update the streaming logic around parseSSE to carry a buffer between chunks, prepend any leftover partial line to the next decoded chunk, and only parse complete newline-delimited data lines once they are fully assembled. Preserve handling of data: and [DONE] while ensuring malformed JSON is only skipped for truly invalid messages, not incomplete chunk fragments.packages/solid/src/index.js-42-88 (1)
42-88: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winImport Solid directly or document the non-reactive fallback.
createSignal/createStoreonly switch toglobalThis.__solidCreate*, but nothing in this package sets those globals, so Solid consumers get the plain closure/proxy shim and JSX won’t track updates.createStoreis also unused here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/solid/src/index.js` around lines 42 - 88, The fallback implementations in createSignal and createStore are currently non-reactive shims, and nothing in this package initializes the globalThis.__solidCreateSignal/globalThis.__solidCreateStore hooks, so consumers won’t get real Solid tracking. Update the exports in packages/solid/src/index.js to either import and re-export the actual SolidJS APIs directly or clearly document and wire up the intended fallback behavior; also remove or reconcile the unused createStore shim if it is not meant to be part of the public API.packages/preact/src/index.js-40-52 (1)
40-52: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInline effect shim discards the cleanup function, risking timer leaks.
_useEffect = (fn) => { fn(); }runs the effect once but never invokes (or stores) the returned cleanup.useRelayHealth(lines 422-427) relies on this cleanup toclearInterval; in the shim/fallback path (used whenever neitherpreact/hooksnorreactis resolvable — a legitimate path sincepreactis an optional peer dependency), callinguseRelayHealthwith the defaultintervalMs=30000starts asetIntervalthat is never cleared.🔧 Proposed fix
+ let _pendingCleanup; _useEffect = (fn) => { - fn(); + if (typeof _pendingCleanup === 'function') { try { _pendingCleanup(); } catch (_) {} } + _pendingCleanup = fn(); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/preact/src/index.js` around lines 40 - 52, The inline effect shim in the Preact fallback currently ignores the cleanup returned by the effect, which can leave intervals running. Update `_useEffect` in `index.js` so it captures the return value from `fn()` and, if it is a function, stores or invokes that cleanup appropriately in the shim path. Make sure this covers `useRelayHealth`, since it relies on the cleanup to clear the interval when the optional `preact/hooks` or `react` imports are unavailable.packages/preact/src/index.js-93-130 (1)
93-130: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
_streamSSEsilently treats mid-stream read failures as successful completion, and drops a trailing unterminated line.At line 113,
catch (_) { break; }exits the read loop on a genuine stream error, then falls through toonDone()at line 129 — the caller (useStreamingChat.sendMessage) commits whatever was accumulated so far as if the response completed normally, with noerrorsurfaced to the app. Separately, any content left inbufafter the finalread()(a line not yet terminated by\n) is never parsed/flushed beforeonDone()is called.🔧 Proposed fix
while (true) { let chunk; - try { chunk = await reader.read(); } catch (_) { break; } + try { chunk = await reader.read(); } catch (err) { onError(err.message || 'Stream read error'); return; } if (chunk.done) break; buf += decoder.decode(chunk.value, { stream: true }); const lines = buf.split('\n'); buf = lines.pop(); for (const line of lines) { const parsed = _parseSSEData(line.trim()); if (!parsed) continue; if (parsed.done) { onDone(); return; } const delta = parsed.data?.choices?.[0]?.delta?.content || parsed.data?.delta?.text || ''; if (delta) onChunk(delta); } } + if (buf.trim()) { + const parsed = _parseSSEData(buf.trim()); + if (parsed && !parsed.done) { + const delta = parsed.data?.choices?.[0]?.delta?.content || parsed.data?.delta?.text || ''; + if (delta) onChunk(delta); + } + } onDone();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/preact/src/index.js` around lines 93 - 130, The _streamSSE helper is treating real stream read failures as normal completion and also drops any final buffered SSE line. Update _streamSSE to distinguish a reader.read() error from an orderly end-of-stream: surface the error through onError instead of falling through to onDone, so useStreamingChat.sendMessage can handle failures correctly. Also flush and parse any remaining buf after the read loop before calling onDone, so a trailing unterminated event is not lost.packages/preact/src/index.js-156-232 (1)
156-232: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winConcurrent calls can trigger duplicate
/usersregistrations.
getTokenre-reads_safeGet(tokenKey)and, if empty, issues a freshPOST /userswith no in-flight-request guard. If two calls race before either resolves (e.g. app callsstoreKey(...)andlistKeys()back-to-back before a token exists, oruseChat/useStreamingChat'ssendMessageat lines 267/335 races with a directuseByokRelaycall), each sees no cached token and independently registers, producing two distinct relay tokens/user records — data written under one may not be visible via the other.🔧 Proposed fix — memoize the in-flight registration
+ const inflightRef = useRef(null); const getToken = useCallback(async () => { const stored = _safeGet(tokenKey); if (stored) { setToken(stored); return stored; } + if (inflightRef.current) return inflightRef.current; setLoading(true); setError(null); - try { + const p = (async () => { + try { const res = await fetch(`${relayUrl}/users`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ app_id: appId }), }); const data = await res.json(); if (!res.ok) throw new Error(data.error || `HTTP ${res.status}`); const t = data.token; _safeSet(tokenKey, t); setToken(t); return t; - } catch (err) { + } catch (err) { setError(err.message); return null; - } finally { + } finally { setLoading(false); - } + inflightRef.current = null; + } + })(); + inflightRef.current = p; + return p; }, [relayUrl, appId, tokenKey]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/preact/src/index.js` around lines 156 - 232, The `useByokRelay` hook can create duplicate relay users when `getToken` is called concurrently because each call independently posts to `/users` after `_safeGet(tokenKey)` returns empty. Add an in-flight request guard inside `getToken` so concurrent callers share the same pending registration promise and all resolve to the same token, clearing the guard when it settles. Use the existing `tokenKey`, `getToken`, and `storeKey`/`listKeys` flow to keep behavior unchanged except for deduplicating registration.packages/hono/src/index.js-108-176 (1)
108-176: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo timeout on upstream
fetch()calls in the hot request path; also significant duplication between the two factories.Neither
createByokRelayMiddlewarenorcreateRelayRouteapply a timeout/AbortSignalto theirfetch()calls (lines 158-163 and 233-238). A slow or hanging upstream relay will block the request indefinitely on that worker/isolate — particularly costly on Cloudflare Workers' CPU-time limits and on Node/Bun where it ties up an event-loop task with no bound. The same applies toByokRelayClient's methods, which never pass asignaleither (onlystreamChataccepts an optional one).Additionally, the two functions duplicate essentially the entire proxy body (header filtering, body read, fetch, error handling, response construction), which means any future fix (e.g., the header-stripping and timeout issues above) has to be applied twice and can drift.
♻️ Suggested consolidation with a timeout
+async function _proxyToRelay (c, upstreamUrl, { allowedAppIds } = {}) { + if (allowedAppIds && allowedAppIds.length > 0) { + const appId = c.req.header('x-app-id') || ''; + if (!appId || !allowedAppIds.includes(appId)) { + return c.json({ error: 'app_id not allowed' }, 403); + } + } + + const forwardHeaders = _filterHeaders(c.req.raw.headers); + const method = c.req.method.toUpperCase(); + const body = ['GET', 'HEAD', 'OPTIONS'].includes(method) + ? undefined + : await c.req.raw.arrayBuffer(); + + let upstreamRes; + try { + upstreamRes = await fetch(upstreamUrl, { + method, + headers: forwardHeaders, + body, + redirect: 'follow', + signal: AbortSignal.timeout(30_000), + }); + } catch (err) { + return c.json({ error: 'Failed to reach upstream relay', detail: String(err) }, 502); + } + + return new Response(upstreamRes.body, { + status: upstreamRes.status, + headers: _filterHeaders(upstreamRes.headers), + }); +}Both
createByokRelayMiddlewareandcreateRelayRoutecan then computeupstreamUrland delegate to_proxyToRelay.Also applies to: 201-250
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hono/src/index.js` around lines 108 - 176, Add a timeout/AbortSignal to the upstream relay fetch path and remove the duplicated proxy logic. Update both createByokRelayMiddleware and createRelayRoute to delegate the shared request/response handling to a single helper such as _proxyToRelay, and have that helper create an AbortController with a bounded timeout that is passed into fetch(). Also thread an optional signal through ByokRelayClient methods (not just streamChat) so all relay calls can be canceled consistently.packages/hono/README.md-154-191 (1)
154-191: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDocumented pattern breaks encapsulation by mutating a private field directly.
relay._token = token;(line 166) directly writes to an underscore-prefixed field thatByokRelayClienttreats as private/internal. Documenting this pattern publicly means any future rename/refactor of_tokensilently breaks every consumer copying this example. Consider exposing a publicsetToken(token)method onByokRelayClient(see companion comment inpackages/hono/src/index.js) and updating this example to use it.📝 Suggested doc update (once a public setter exists)
const relay = new ByokRelayClient({ relayUrl: c.env.RELAY_URL }); - relay._token = token; // inject user's relay token directly + relay.setToken(token); // inject user's relay token directly🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hono/README.md` around lines 154 - 191, The README example is mutating the internal ByokRelayClient token field directly, which exposes a private implementation detail. Update the example to use a public token setter on ByokRelayClient instead of assigning to _token, and ensure the docs reference the public API only. Use the ByokRelayClient example in the Cloudflare Workers section and align it with the setToken token-handling method once available.packages/remix/src/index.js-99-133 (1)
99-133: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
createRelayLoaderdrops the original query string.
targetUrlis built fromsubPathalone (Line 116); the incoming request's query string (?deep=1,?app_id=..., pagination params, etc.) is never appended. This silently breaks documented flows likerelay.health(true)(GET /health?deep=1) or any query-parameterized relay endpoint proxied through this loader.🐛 Proposed fix
function createRelayLoader ({ relayUrl = DEFAULT_RELAY_URL, allowedApps } = {}) { return async function relayLoader ({ request, params }) { const subPath = params['*'] || ''; + const { search } = new URL(request.url); // app_id allowlist check if (allowedApps && allowedApps.length > 0) { const url = new URL(request.url); const appId = url.searchParams.get('app_id') || request.headers.get('x-app-id') || ''; if (!allowedApps.includes(appId)) { return new Response(JSON.stringify({ error: 'app_id not allowed' }), { status: 403, headers: { 'Content-Type': 'application/json' }, }); } } - const targetUrl = `${relayUrl.replace(/\/$/, '')}/${subPath}`; + const targetUrl = `${relayUrl.replace(/\/$/, '')}/${subPath}${search}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/remix/src/index.js` around lines 99 - 133, `createRelayLoader` is rebuilding the relay URL without preserving the incoming request’s query string, so proxied endpoints lose parameters like `deep`, `app_id`, or pagination values. Update `relayLoader` to read the original request URL and append its search string when constructing `targetUrl`, alongside `subPath`, so the fetch in `createRelayLoader` forwards queries unchanged.packages/remix/src/index.js-411-480 (1)
411-480: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStale closure breaks "partial response committed on stop" for streaming chat.
accumulated(443-445) is scoped inside thetryblock, so it isn't visible fromcatch. Thecatchbranch (469-475) instead readsstreamingContent, the React state value captured when this particularsendclosure instance was created — not the live text accumulated during this in-flight stream. As a result, callingstopStreaming()will typically commit stale/empty content instead of the actual partial response, contradicting the documented behavior ("partial response is committed tomessages", README Line 206).🐛 Proposed fix — hoist `accumulated` above the try block
const ctrl = new AbortController(); abortRef.current = ctrl; const body = { model, messages: systemPrompt ? [{ role: 'system', content: systemPrompt }, ...newMessages] : newMessages, stream: true, ...extraParams, }; + let accumulated = ''; try { const r = await fetch(`${relayUrl}/relay/${provider}/chat/completions`, { method: 'POST', headers: { 'Content-Type': 'application/json', 'x-relay-token': token }, body: JSON.stringify(body), signal: ctrl.signal, }); if (!r.ok) { const errData = await r.json().catch(() => ({})); throw new Error(errData.error || `HTTP ${r.status}`); } const reader = r.body.getReader(); const decoder = new TextDecoder(); - let accumulated = ''; while (true) { ... } setMessages(m => [...m, { role: 'assistant', content: accumulated }]); setStreamingContent(''); } catch (e) { if (e.name !== 'AbortError') setError(e.message); - else if (streamingContent) { + else if (accumulated) { // partial commit on abort - setMessages(m => [...m, { role: 'assistant', content: streamingContent }]); + setMessages(m => [...m, { role: 'assistant', content: accumulated }]); setStreamingContent(''); } } finally {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/remix/src/index.js` around lines 411 - 480, The partial-response commit on abort is using stale React state instead of the live streamed text. In send, hoist the accumulated buffer out of the try block and use that same local value in the catch branch for AbortError handling, rather than reading streamingContent from the closure. Keep the logic in send and the stopStreaming/abortRef flow unchanged, but ensure the committed assistant message comes from the in-flight accumulation.packages/vercel-ai/src/index.js-74-81 (1)
74-81: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winLocal token removed even when server-side deletion fails.
storage.removeItemruns unconditionally, regardless ofres.ok. A failed delete leaves the account undeleted server-side but wipes the only credential that could retry the deletion.🐛 Proposed fix
async function deleteAccount({ relayUrl, token, storage, appId }) { const res = await fetch(`${relayUrl}/users`, { method: 'DELETE', headers: { Authorization: `Bearer ${token}` }, }); - storage.removeItem(`byok_relay_token_${appId}`); - return res.ok; + if (res.ok) { + storage.removeItem(`byok_relay_token_${appId}`); + } + return res.ok; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vercel-ai/src/index.js` around lines 74 - 81, The deleteAccount flow removes the cached relay token even when the DELETE request fails, which prevents retrying the server-side cleanup. Update deleteAccount to only call storage.removeItem for byok_relay_token_${appId} after a successful res.ok response, and preserve the token when the request fails so the caller can retry. Keep the fix localized to deleteAccount and its fetch/res.ok handling.packages/vercel-ai/src/index.js-217-233 (1)
217-233: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo timeout on relay fetch calls.
relay()(and the otherfetchcalls throughout this file:registerToken,storeProviderKey,deleteAccount,health,stats) has no timeout/AbortSignal. A slow or hung relay backend will blockdoGenerate/doStreamindefinitely.🛡️ Proposed fix for `relay()`
async function relay(body, stream) { const token = await getToken(); const url = `${relayUrl}/relay/${relayProvider}/chat/completions`; + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30_000); const res = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}`, }, body: JSON.stringify({ ...body, stream }), + signal: controller.signal, }); + clearTimeout(timeoutId); if (!res.ok) { const errText = await res.text(); throw new Error(`byok-relay [${res.status}]: ${errText}`); } return res; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vercel-ai/src/index.js` around lines 217 - 233, The relay-related fetches in this module can hang indefinitely because they do not use a timeout or AbortSignal. Update relay() and the other fetch helpers in index.js such as registerToken, storeProviderKey, deleteAccount, health, and stats to accept or create a shared timeout signal, pass it into fetch, and handle abort/timeout errors consistently. Use the existing helper functions around getToken, relay, and the other request wrappers to keep the timeout behavior centralized and apply it to both doGenerate and doStream paths.packages/vercel-ai/src/index.js-393-406 (1)
393-406: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
createByokRelayProviderdoesn't actually await registration, and lacks the sync factory's concurrency guard.Per
README.md(lines 107-113): "Same as above but awaits token registration before returning. Use when you need the token immediately." The implementation never callsgetToken()before returning, so it behaves identically tocreateByokRelayProviderSync— registration only happens lazily on first model use. Additionally, unlikecreateByokRelayProviderSync(which caches an in-flight promise via_pending), thisgetToken()has no dedup guard, so concurrent calls before_tokenis set will trigger multiple/usersregistrations.🐛 Proposed fix
async function createByokRelayProvider({ relayUrl, appId = 'vercel-ai', storage, settings = {} } = {}) { if (!relayUrl) throw new Error('byok-relay: relayUrl is required'); const store = storage ?? defaultStorage(); const storageKey = `byok_relay_token_${appId}`; let _token = store.getItem(storageKey) ?? null; + let _pending = null; async function getToken() { - if (!_token) { - _token = await registerToken({ relayUrl, appId, storage: store }); - } - return _token; + if (_token) return _token; + if (!_pending) { + _pending = registerToken({ relayUrl, appId, storage: store }).then((t) => { + _token = t; + _pending = null; + return t; + }); + } + return _pending; } + await getToken(); + return {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vercel-ai/src/index.js` around lines 393 - 406, `createByokRelayProvider` is not honoring the “await registration before returning” behavior and it also lacks the in-flight deduping used by `createByokRelayProviderSync`. Update `createByokRelayProvider` to eagerly call `getToken()` before returning, and add a `_pending`-style guard inside `getToken()` so concurrent calls share the same registration promise instead of issuing multiple `/users` requests. Keep the fix localized to `createByokRelayProvider` and its `getToken` helper in `index.js`.packages/vercel-ai/src/index.js-454-456 (1)
454-456: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
chat()alias breaks when the returned provider object is destructured.
this.languageModel(...)relies on method-call binding. Destructuringchatoff the provider (e.g.const { chat } = provider) and invoking it directly throwsTypeError: Cannot read properties of undefinedunder'use strict'. Same pattern repeats increateByokRelayProviderSyncat Lines 519-521.🐛 Proposed fix (define `languageModel` as a closure function, alias `chat` to it directly)
+ function languageModel(modelId, overrides = {}) { + return createLanguageModel({ + relayUrl, + getToken, + modelId, + settings: { ...settings, ...overrides }, + }); + } + return { ... - languageModel(modelId, overrides = {}) { - return createLanguageModel({ - relayUrl, - getToken, - modelId, - settings: { ...settings, ...overrides }, - }); - }, + languageModel, ... - chat(modelId, overrides = {}) { - return this.languageModel(modelId, overrides); - }, + chat: languageModel,Apply the equivalent change in
createByokRelayProviderSync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vercel-ai/src/index.js` around lines 454 - 456, The chat alias currently depends on method-call binding, so destructuring it from the provider breaks when chat calls this.languageModel. Update the provider returned by languageModel to make languageModel a closure that does not rely on this, and have chat directly reference that same function instead of calling through this. Apply the same fix in createByokRelayProviderSync so both provider factories behave correctly when chat is destructured.packages/vercel-ai/src/index.js-143-149 (1)
143-149: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTool messages need to preserve every result.
toolmessages can contain multipleToolResultParts, but this branch only serializesmsg.content[0], so later parallel tool-call results are dropped. Map each part to its owntoolmessage and switch the outer mapper toflatMap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vercel-ai/src/index.js` around lines 143 - 149, The tool-message handling in the message mapper only serializes the first ToolResultPart from msg.content, so parallel tool results are lost. Update the mapping logic around the tool role branch to emit one tool message per part, using each part’s toolCallId and result, and change the surrounding mapper in index.js to use flatMap so all tool results are preserved.
🟡 Minor comments (6)
README.md-101-108 (1)
101-108: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse loader data instead of
windowin the Remix component example. The current snippet readswindow.ENV.RELAY_URLduring render and will break on SSR; passrelayUrlfromuseLoaderDatahere, matching the loader setup above.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 101 - 108, The AiChat Remix example is reading relayUrl from window.ENV during render, which is not SSR-safe. Update the AiChat component to take relayUrl from useLoaderData, matching the loader setup shown above, and pass that loader-derived value into both useByokRelay and useStreamingChat instead of accessing window directly.packages/astro/src/index.js-493-529 (1)
493-529: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winStream reader not released on error path.
If
onChunk(or any code inside theforloop) throws, the exception propagates out ofstreamChatwithout releasingreaderor canceling the underlying stream, since there's notry/finally. Thetry/catchat Lines 511-523 only guardsJSON.parse.🔧 Proposed fix
const reader = resp.body.getReader(); const decoder = new TextDecoder(); let accumulated = ''; let buffer = ''; - while (true) { - const { value, done } = await reader.read(); - if (done) break; - ... - } + try { + while (true) { + const { value, done } = await reader.read(); + if (done) break; + ... + } + } finally { + reader.releaseLock(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/index.js` around lines 493 - 529, The stream handling in streamChat does not release the reader if onChunk or other logic inside the read loop throws, because only JSON.parse is wrapped in a catch. Add a try/finally around the full reader loop so the reader is always released or the stream is canceled on exit, and keep the existing onDone(accumulated) behavior only for successful completion.packages/preact/test/hooks.test.js-295-299 (1)
295-299: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTest never actually calls
stopStreaming().
assert(() => { h.stopStreaming(); return true; }, 'no throw')passes a function reference toassert, which only checks truthiness — the arrow function is never invoked. The test always passes without exercisingstopStreaming().🔧 Proposed fix
await test('stopStreaming does not throw when no stream active', () => { const h = useStreamingChat({ relayUrl: 'http://relay', appId: 'test' }); - assert(() => { h.stopStreaming(); return true; }, 'no throw'); + h.stopStreaming(); afterEach(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/preact/test/hooks.test.js` around lines 295 - 299, The test for useStreamingChat.stopStreaming never actually invokes the method because assert is being given a function reference instead of executing it. Update the test so stopStreaming() is called directly within the assertion block, and keep the check focused on ensuring h.stopStreaming does not throw when no stream is active.packages/hono/src/index.js-322-334 (1)
322-334: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCached-token branch returns a shape inconsistent with the fresh-registration branch.
When a token is already cached,
register()returns{ token: this._token }— omittingexpires_at, which the fresh-registration path (and the documented return typePromise<{token:string,expires_at:string}>) includes. Callers relying onexpires_atto check expiry will getundefinedon the cached path.🔧 Suggested fix
async register (appId, force = false) { - if (!force && this._token) return { token: this._token }; + if (!force && this._token) return { token: this._token, expires_at: this._expiresAt }; const res = await fetch(`${this._relayUrl}/users`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ app_id: appId || this._appId }), }); if (!res.ok) throw new Error(`Registration failed: ${res.status} ${await res.text()}`); const data = await res.json(); this._token = data.token; + this._expiresAt = data.expires_at; if (this._token) this._storage.set(this._storageKey, this._token); return data; }(also cache
expires_aton the fresh path so it's available on the cached return.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hono/src/index.js` around lines 322 - 334, The register() method in the cached-token branch returns a different shape than the fresh-registration path, so update it to return the same token and expires_at fields as the response from the relay. In the Hono client’s register() flow, cache expires_at alongside the token when a fresh registration succeeds, and have the early return from the cached-token path include that stored expires_at value instead of only token.packages/remix/src/index.js-234-248 (1)
234-248: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStorage key uses un-normalized
relayUrl, unlike the normalized value stored on the instance.In
ByokRelayClient(Line 568) anduseByokRelay(Line 238), the token storage key is built from the rawrelayUrlargument, whilethis.relayUrl(Line 566) strips a trailing slash. Configuring the same relay with/without a trailing slash (e.g.https://relay.example.comvshttps://relay.example.com/) produces two different storage keys, silently losing a previously stored token.🔧 Proposed fix
constructor ({ relayUrl = DEFAULT_RELAY_URL, appId = '', storage } = {}) { - this.relayUrl = relayUrl.replace(/\/$/, ''); + this.relayUrl = relayUrl.replace(/\/$/, ''); this.appId = appId; - this._storageKey = `byok_token_${relayUrl}_${appId}`; + this._storageKey = `byok_token_${this.relayUrl}_${appId}`;Also applies to: 565-575
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/remix/src/index.js` around lines 234 - 248, The token storage key is being built from the raw relayUrl in both ByokRelayClient and useByokRelay, which can create different keys for the same relay when a trailing slash is present. Update the storage key construction to use the normalized relayUrl value already stored on the instance (and the same normalization in the hook) so ByokRelayClient and useByokRelay always derive a consistent key regardless of trailing slash formatting.packages/vercel-ai/src/index.js-100-103 (1)
100-103: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSpread-based base64 fallback can overflow the call stack on large images.
🐛 Proposed fix
function bufferToBase64(buf) { if (typeof Buffer !== 'undefined') return Buffer.from(buf).toString('base64'); - return btoa(String.fromCharCode(...new Uint8Array(buf))); + const bytes = new Uint8Array(buf); + let binary = ''; + for (let i = 0; i < bytes.length; i++) binary += String.fromCharCode(bytes[i]); + return btoa(binary); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vercel-ai/src/index.js` around lines 100 - 103, The bufferToBase64 fallback uses a spread over Uint8Array, which can overflow the call stack for large inputs. Update bufferToBase64 to avoid String.fromCharCode(...new Uint8Array(buf)) in the non-Buffer path, and replace it with a chunked conversion approach or another safe browser-side base64 encoder so large images are handled reliably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4e8c524-97b2-4f7e-b1ff-9a025a679c58
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
README.mdllms.txtmetrics/daily.jsonlpackage.jsonpackages/angular/README.mdpackages/angular/package.jsonpackages/angular/src/index.jspackages/angular/test/services.test.jspackages/astro/README.mdpackages/astro/package.jsonpackages/astro/src/index.jspackages/astro/test/integration.test.jspackages/hono/README.mdpackages/hono/package.jsonpackages/hono/src/index.jspackages/hono/test/middleware.test.jspackages/preact/README.mdpackages/preact/package.jsonpackages/preact/src/index.jspackages/preact/test/hooks.test.jspackages/remix/README.mdpackages/remix/package.jsonpackages/remix/src/index.jspackages/remix/test/remix.test.jspackages/solid/README.mdpackages/solid/package.jsonpackages/solid/src/index.jspackages/solid/test/stores.test.jspackages/vercel-ai/README.mdpackages/vercel-ai/package.jsonpackages/vercel-ai/src/index.jspackages/vercel-ai/test/smoke.test.js
| /** Strip hop-by-hop headers before forwarding between server ↔ relay. */ | ||
| const HOP_BY_HOP = new Set([ | ||
| 'connection', 'keep-alive', 'proxy-authenticate', 'proxy-authorization', | ||
| 'te', 'trailer', 'transfer-encoding', 'upgrade', | ||
| 'content-length', // let fetch set it | ||
| ]); | ||
|
|
||
| function _filterHeaders (headers) { | ||
| const out = {}; | ||
| for (const [k, v] of Object.entries(headers)) { | ||
| if (!HOP_BY_HOP.has(k.toLowerCase())) out[k] = v; | ||
| } | ||
| return out; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does Node.js undici fetch allow setting the Host header on outgoing requests unlike browsers?
💡 Result:
No, Node.js undici fetch does not allow you to manually set the Host header on outgoing requests [1][2][3]. Similar to browsers and the Fetch Standard, Node.js treats Host as a "forbidden header" in its fetch implementation [2][4]. Any attempt to set the Host header manually via the headers option in fetch will be ignored or overwritten by the system, which automatically generates the Host header based on the request URL [1][3]. If you need to control the Host header, the recommended workarounds include: 1. Use undici.Client.request instead of fetch: The undici client's request method provides low-level control and allows you to set the Host header manually [3][5]. 2. Use Node.js built-in http or https modules: These modules offer full control over headers and connection details, which is often necessary when implementing reverse proxies or testing scenarios that require specific Host header manipulation [3][5]. 3. Use a custom dispatcher/connector: For more advanced scenarios, such as modifying the connection target while maintaining a specific Host header, you can configure an undici Agent with a custom connector [6]. While there has been significant community discussion regarding the utility of overriding the Host header in server-side environments—unlike the security-driven restrictions in browsers—the current standard implementation within Node.js fetch maintains the restriction [1][7][2].
Citations:
- 1: Fetch failed when setting host: header correctly nodejs/undici#2318
- 2: Node 18.18.2 : unable to override the Host header in fetch nodejs/node#50305
- 3: https://github.com/sibelius/undici-reproduction-host-header-noport
- 4: docs: forbidden headers not documented nodejs/undici#1470
- 5: Does not set TLS
servernameto the value of theHostheader nodejs/undici#3401 - 6: Cannot change `host` using Agent in fetch. nodejs/undici#4169
- 7: disallow setting host header in fetch nodejs/undici#2322
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file ranges and call sites.
sed -n '1,220p' packages/remix/src/index.js | cat -n
# Locate all uses of the header filter and relay URL defaults.
rg -n "_filterHeaders|DEFAULT_RELAY_URL|createRelayLoader|createRelayAction|fetch\\(" packages/remix/src/index.jsRepository: avikalpg/byok-relay
Length of output: 12151
Forward only an explicit allowlist of headers to the relay
Both proxy calls forward incoming request headers almost verbatim after stripping only hop-by-hop headers, so cookie, authorization, and other app-specific secrets will be sent to the default third-party relay. Switch this to an allowlist of the few headers the relay actually needs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/remix/src/index.js` around lines 60 - 73, The current _filterHeaders
helper only removes hop-by-hop headers, so sensitive request headers still get
forwarded to the relay. Update the header forwarding logic in
packages/remix/src/index.js to use an explicit allowlist of only the
relay-required headers instead of copying nearly everything. Keep the change
localized around _filterHeaders and the two proxy call sites that rely on it,
and ensure cookie, authorization, and other app-specific headers are excluded.
| const PROVIDER_PATHS = { | ||
| openai: 'chat/completions', | ||
| anthropic: 'messages', | ||
| google: 'models/{model}:generateContent', | ||
| groq: 'chat/completions', | ||
| mistral: 'chat/completions', | ||
| openrouter: 'chat/completions', | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare relay URL construction across sibling packages to confirm the expected contract.
rg -n "relay/\\\$\{provider\}" packages -A2 -B2
rg -n "PROVIDER_PATHS" packages -A5Repository: avikalpg/byok-relay
Length of output: 5621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Solid implementation around the provider paths, body shaping, and fetch URLs.
sed -n '1,180p' packages/solid/src/index.js
printf '\n---\n'
sed -n '280,480p' packages/solid/src/index.js
printf '\n=== README provider mentions ===\n'
rg -n "google|anthropic|openai|groq|mistral|openrouter|relay/" README.md packages/solid packages/remix packages/angular -A2 -B2Repository: avikalpg/byok-relay
Length of output: 50377
Relay requests need the provider path suffix.
createChatStore and createStreamingChatStore POST to ${relayUrl}/relay/${provider}, so the unused PROVIDER_PATHS map never affects the request. The repo documents relay calls as /relay/:provider/:path (for example /relay/openai/chat/completions), and Solid still has no Google-specific body or delta handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/solid/src/index.js` around lines 24 - 31, The relay request URL is
missing the provider-specific path suffix, so the unused PROVIDER_PATHS map is
not being applied in createChatStore and createStreamingChatStore. Update the
request construction to append the mapped provider path to
`${relayUrl}/relay/${provider}` so relay calls match the documented
`/relay/:provider/:path` format, and make sure the Google entry still routes
through its special path handling in the Solid store code.
Summary
Growth Day 30 — adds
@byok-relay/hono: Hono middleware and route factory for byok-relay, targeting Cloudflare Workers, Deno Deploy, Bun, and Node.js.What's in this PR
packages/hono/src/index.jscreateByokRelayMiddleware(opts)— HonoMiddlewareHandler; intercepts requests matchingpathPrefix(default/relay); readsRELAY_URLfromc.env(Workers binding) so it stays server-side only; hop-by-hop header filtering; optionalallowedAppIdslistcreateRelayRoute(opts)— HonoHandlerfor explicit catch-all route registrationByokRelayClient— plain-JS class for server-side use; in-memory storage by default; accepts customstorageadapter for Workers KV / Deno KV; full API: register, ensureToken, logout, storeKey, listKeys, deleteKey, rotateKey, chat, streamChat (async generator with AbortSignal), health, stats, getModels, deleteAccountTest: 24 smoke tests, all passing
Key differentiator: targets stateless edge runtimes where localStorage is unavailable;
c.env.RELAY_URLbinding keeps RELAY_URL in Workers Secrets, never in the bundle.Next for Avi
Summary by CodeRabbit
New Features
Documentation
Chores
Tests