fix: close TTS rework fallback gaps#16
Hidden character warning
Conversation
Summary:
- add an OpenAI-compatible TTS adapter with request tests, /v1 path
fallback, and non-audio response detection
- add a legacy external TTS fallback provider and route all TTS generation
through a single provider entrypoint
- document the unattended design and implementation plan for TIA-51 and add
environment keys for external TTS config
Rationale:
- the existing route hardcoded one anonymous upstream and ignored the voice
parameter, so it could not meaningfully expand external TTS support
- the configured API endpoint in this environment exposes chat models but does
not provide a standard TTS audio endpoint, so runtime fallback is required
- keeping the response contract as { audio } preserves the editor/media
insertion flow while making provider behavior testable and replaceable
Tests:
- bun test apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.test.ts
- bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/legacy.ts apps/web/src/constants/tts-constants.ts packages/env/src/web.ts
- bunx tsc -p apps/web/tsconfig.json --noEmit
- NODE_ENV=development NEXT_PUBLIC_SITE_URL=http://localhost:4100 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy bun --eval 'import { NextRequest } from "next/server"; import { POST } from "./src/app/api/tts/generate/route.ts"; const request = new NextRequest("http://localhost/api/tts/generate", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ text: "你好,Cutia。", voice: "default" }) }); const response = await POST(request); const json = await response.json(); console.log(JSON.stringify({ status: response.status, audioLength: json.audio?.length ?? 0, audioHead: json.audio?.slice(0, 8) ?? null, error: json.error ?? null }));' (workdir: apps/web)
Co-authored-by: Codex <codex@openai.com>
Summary: - add timeout handling for external and legacy TTS fetches and expand OpenAI-compatible endpoint probing to cover both /v1 and root paths - harden legacy fallback with GET length limits, HTTPS/host validation, and audio content-type checks - rethrow missing external config instead of silently falling back, add regression tests, and fix the plan doc heading level Rationale: - review feedback identified real security and reliability gaps in the fallback provider and request orchestration - the legacy provider only supports GET, so length guards are the safe mitigation for query-string leakage and URL size limits - explicit tests are needed to lock the config-error path and timeout behavior before reopening review Tests: - bun test apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/constants/tts-constants.ts packages/env/src/web.ts docs/plans/2026-03-17-tts-external-provider.md docs/plans/2026-03-17-tts-external-provider-design.md - bunx tsc -p apps/web/tsconfig.json --noEmit - NODE_ENV=development NEXT_PUBLIC_SITE_URL=http://localhost:4100 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy bun --eval 'import { NextRequest } from "next/server"; import { POST } from "./src/app/api/tts/generate/route.ts"; const request = new NextRequest("http://localhost/api/tts/generate", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ text: "你好,Cutia。", voice: "default" }) }); const response = await POST(request); const json = await response.json(); console.log(JSON.stringify({ status: response.status, audioLength: json.audio?.length ?? 0, audioHead: json.audio?.slice(0, 8) ?? null, error: json.error ?? null }));' (workdir: apps/web) Co-authored-by: Codex <codex@openai.com>
Summary: - reject legacy and OpenAI-compatible audio responses when the content-type header is missing instead of silently accepting them - preserve raw upstream error bodies when JSON payloads do not match the expected error schema and add regression tests for both cases - normalize the external TTS plan task headings to a consistent level Rationale: - CodeRabbit found two real validation gaps that allowed untyped success payloads through and one bug where JSON error parsing consumed the body before the text fallback could read it - locking these paths with targeted tests keeps the rework focused on the new review findings instead of broad refactoring - fixing the plan heading mismatch removes a repeated doc-only review item Tests: - bun test apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/constants/tts-constants.ts packages/env/src/web.ts docs/plans/2026-03-17-tts-external-provider.md docs/plans/2026-03-17-tts-external-provider-design.md - bunx tsc -p apps/web/tsconfig.json --noEmit - NODE_ENV=development NEXT_PUBLIC_SITE_URL=http://localhost:4100 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy bun --eval 'import { NextRequest } from "next/server"; import { POST } from "./src/app/api/tts/generate/route.ts"; const request = new NextRequest("http://localhost/api/tts/generate", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ text: "hello", voice: "default" }) }); const response = await POST(request); const json = await response.json(); console.log(JSON.stringify({ status: response.status, audioLength: json.audio?.length ?? 0, audioHead: json.audio?.slice(0, 8) ?? null, error: json.error ?? null }));' (workdir: apps/web) Co-authored-by: Codex <codex@openai.com>
Summary: - compose caller cancellation with timeout handling in fetchWithTimeout and cover immediate and in-flight aborts with tests - normalize legacy and OpenAI-compatible MIME checks so valid audio types with casing or parameters are accepted - trim external TTS config values before validation and remove the assistant-specific directive from the plan doc Rationale: - CodeRabbit identified a real correctness issue where caller aborts were overwritten by timeout logic inside the fetch wrapper - MIME checks should validate the media type, not the raw header string, or valid responses can be rejected unnecessarily - whitespace-only config values should fail fast instead of surfacing opaque upstream errors later in the request path Tests: - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/constants/tts-constants.ts packages/env/src/web.ts docs/plans/2026-03-17-tts-external-provider.md docs/plans/2026-03-17-tts-external-provider-design.md - bunx tsc -p apps/web/tsconfig.json --noEmit - NODE_ENV=development NEXT_PUBLIC_SITE_URL=http://localhost:4100 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy bun --eval 'import { NextRequest } from "next/server"; import { POST } from "./src/app/api/tts/generate/route.ts"; const request = new NextRequest("http://localhost/api/tts/generate", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ text: "hello", voice: "default" }) }); const response = await POST(request); const json = await response.json(); console.log(JSON.stringify({ status: response.status, audioLength: json.audio?.length ?? 0, audioHead: json.audio?.slice(0, 8) ?? null, error: json.error ?? null }));' (workdir: apps/web) Co-authored-by: Codex <codex@openai.com>
Summary: - export FetchLike from fetch-with-timeout and reuse it across the legacy and OpenAI-compatible adapters - extract the duplicated caller-abort error construction into a shared helper inside fetch-with-timeout - keep the legacy voice argument for adapter parity while making its intent explicit in code Rationale: - the remaining CodeRabbit comments were all maintenance-only cleanup items with no behavior change required - sharing the helper type and abort-reason logic reduces repetition without widening the TTS error-handling surface - documenting the unused legacy voice parameter makes the interface parity intentional instead of accidental Tests: - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts - bunx @biomejs/biome check apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/openai-compatible.ts docs/plans/2026-03-17-tts-external-provider.md - bunx tsc -p apps/web/tsconfig.json --noEmit Co-authored-by: Codex <codex@openai.com>
Summary: - add shared TTS error codes and typed TtsError helpers - emit structured config and upstream failures from TTS providers - map route responses by error code and add regression tests Rationale: - avoid coupling API status mapping to fragile message text - keep provider fallback behavior explicit while preserving readable errors - close the remaining PR review item with direct route-level coverage Tests: - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/app/api/tts/generate/route.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/errors.ts - bunx tsc -p apps/web/tsconfig.json --noEmit - POST /api/tts/generate probe: status=200 audioHead=SUQzBAAA Co-authored-by: Codex <codex@openai.com>
Summary: - wrap external and legacy timeout failures in structured upstream TTS errors - add route success coverage and an exhaustive TTS error switch guard - tighten timeout regression tests to assert error codes instead of messages Rationale: - keep upstream timeouts on the 502 path instead of falling through to 500 - preserve readable timeout messages while restoring consistent route behavior - close the latest review round with direct regression coverage for the timeout path Tests: - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/app/api/tts/generate/route.test.ts apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/errors.ts apps/web/src/constants/tts-constants.ts packages/env/src/web.ts docs/plans/2026-03-17-tts-external-provider.md docs/plans/2026-03-17-tts-external-provider-design.md - bunx tsc -p apps/web/tsconfig.json --noEmit - NODE_ENV=development NEXT_PUBLIC_SITE_URL=http://localhost:4311 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy bun --eval 'import { NextRequest } from "next/server"; import { POST } from "./src/app/api/tts/generate/route.ts"; /* status=200 audioHead=SUQzBAAA */' Co-authored-by: Codex <codex@openai.com>
Summary: - limit legacy fallback to structured EXTERNAL_TTS_UPSTREAM failures only - switch route tests to real TtsError instances and add unexpected-error coverage - remove orchestration-only design doc text and mark background as pre-change context Rationale: - surface unexpected external provider bugs instead of silently masking them - keep test fixtures aligned with production error handling paths - make the design doc describe architecture rather than orchestration metadata Tests: - bun test apps/web/src/lib/tts/provider.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/app/api/tts/generate/route.test.ts apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/errors.ts apps/web/src/constants/tts-constants.ts packages/env/src/web.ts docs/plans/2026-03-17-tts-external-provider.md docs/plans/2026-03-17-tts-external-provider-design.md - bunx tsc -p apps/web/tsconfig.json --noEmit - NODE_ENV=development NEXT_PUBLIC_SITE_URL=http://localhost:4311 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy bun --eval 'import { NextRequest } from "next/server"; import { POST } from "./src/app/api/tts/generate/route.ts"; /* status=200 audioHead=SUQzBAAA */' Co-authored-by: Codex <codex@openai.com>
Summary: - harden legacy audio downloads against unsafe redirects and add regression tests for redirect validation - preserve retryability metadata for external upstream failures so provider fallback skips non-retryable contract errors - expand timeout, provider, and route coverage and refresh the design note for the current fallback behavior Rationale: - CodeRabbit rework flagged redirect bypass and silent fallback cases that could hide the real external provider failure - the live probe showed a 200 text/html response was still falling through to legacy, so the external error needed to remain terminal - the extra tests keep the rework fixes pinned to the actual failure modes seen in review and runtime validation Tests: - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/app/api/tts/generate/route.test.ts apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts docs/plans/2026-03-17-tts-external-provider-design.md - bunx tsc -p apps/web/tsconfig.json --noEmit - NODE_ENV=development NEXT_PUBLIC_SITE_URL=http://localhost:4311 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy bun --eval '...route probe...' Co-authored-by: Codex <codex@openai.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an OpenAI‑compatible external TTS adapter, a legacy TTS provider, a fallback orchestrator, structured TTS errors, a timeout-aware fetch helper, env schema updates, route delegation mapping TTS errors to HTTP statuses, tests across layers, and supporting docs and README/.env updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Route as API Route
participant Provider as Provider<br/>Orchestrator
participant ExtTTS as External TTS
participant LegacyTTS as Legacy TTS
Client->>Route: POST /api/tts/generate { text, voice }
Route->>Provider: synthesizeSpeechWithFallback({ env, text, voice })
Provider->>ExtTTS: synthesizeSpeechWithOpenAiCompatible({ config, text, voice })
alt External Success
ExtTTS-->>Provider: ArrayBuffer (audio)
Provider-->>Route: ArrayBuffer
Route-->>Client: 200 { audio: base64 }
else Retryable External Upstream Error
ExtTTS-->>Provider: TtsError(EXTERNAL_TTS_UPSTREAM, retryable: true)
Provider->>LegacyTTS: synthesizeSpeechWithLegacyProvider({ text, voice })
LegacyTTS-->>Provider: ArrayBuffer (audio)
Provider-->>Route: ArrayBuffer
Route-->>Client: 200 { audio: base64 }
else Non-Retryable / Auth Error
ExtTTS-->>Provider: TtsError(EXTERNAL_TTS_UPSTREAM, retryable: false) or TtsError(status:401)
Provider-->>Route: rethrow TtsError
Route-->>Client: 502 (or mapped status/message)
else Config Error
Provider-->>Route: TtsError(EXTERNAL_TTS_CONFIG)
Route-->>Client: 500 { error }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use your project's `biome` configuration to improve the quality of JS/TS/CSS/JSON code reviews.Add a configuration file to your project to customize how CodeRabbit runs |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/src/lib/tts/provider.test.ts (1)
96-105: Prefer using the TtsError constructor directly instead of Object.assign.The
TtsErrorconstructor already acceptsretryableandstatusparameters. UsingObject.assignafter construction is unnecessary and less clear.♻️ Cleaner TtsError construction
openAiSynthesize: async () => { - throw Object.assign( - new TtsError({ - code: "EXTERNAL_TTS_UPSTREAM", - message: "External TTS request failed: invalid api key", - }), - { - retryable: false, - status: 401, - }, - ); + throw new TtsError({ + code: "EXTERNAL_TTS_UPSTREAM", + message: "External TTS request failed: invalid api key", + retryable: false, + status: 401, + }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/tts/provider.test.ts` around lines 96 - 105, The test throws a TtsError via Object.assign which is unnecessary; update the throw to construct the error directly using the TtsError constructor and pass the retryable and status values (e.g., use new TtsError({...}, { retryable: false, status: 401 }) or the constructor signature your TtsError supports) instead of calling Object.assign on the new instance so the error is created clearly and idiomatically (modify the throw site that currently calls Object.assign with new TtsError).apps/web/src/app/api/tts/generate/route.test.ts (1)
33-41: Consider adding a test for non-TtsError exceptions.The route has a fallback path (lines 54-57 in route.ts) that returns a generic 500 for non-
TtsErrorexceptions. Adding a test case would ensure this path is covered.📝 Suggested test case
test("returns 500 for unexpected non-TtsError exceptions", async () => { synthesizeImpl = async () => { throw new Error("unexpected failure"); }; const response = await POST(createRequest({ text: "hello" })); expect(response.status).toBe(500); expect(await response.json()).toMatchObject({ error: "Internal server error", detail: "unexpected failure", }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/tts/generate/route.test.ts` around lines 33 - 41, Add a test in the existing describe("POST /api/tts/generate") block that simulates a non-TtsError exception by setting synthesizeImpl = async () => { throw new Error("unexpected failure"); }; then call POST(createRequest({ text: "hello" })) and assert the response has status 500 and JSON body matches { error: "Internal server error", detail: "unexpected failure" } to cover the fallback in route.ts for non-TtsError exceptions; place the test alongside the other POST tests and reuse the existing beforeEach/afterEach setup that restores console.error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/lib/tts/legacy.ts`:
- Around line 113-150: The code treats any non-ok audioResponse (including
validated 3xx redirects) as a hard failure; after validating the redirect target
inside the isRedirectStatus(audioResponse.status) block, allowlist-validated
redirects should not fall through to the final if (!audioResponse.ok) throw.
Modify the logic in legacy TTS download flow: after successfully parsing and
validating redirectUrl (using audioResponse, audioUrl and
LEGACY_TTS_ALLOWED_AUDIO_HOSTS), either return/continue the success path or skip
the final error check (e.g., wrap the final TtsError throw in an else branch
that only runs when not isRedirectStatus(audioResponse.status)), so validated
redirects are accepted and only truly error statuses trigger the TtsError.
In `@packages/env/src/web.ts`:
- Around line 24-26: Rename the generic env keys API_BASE_URL, API_MODEL and
API_KEY in the zod schema to namespaced keys like EXTERNAL_TTS_API_BASE_URL,
EXTERNAL_TTS_API_MODEL and EXTERNAL_TTS_API_KEY to avoid collisions; update any
code that references those symbols (e.g., the zod schema entries API_BASE_URL,
API_MODEL, API_KEY in packages/env/src/web.ts) to use the new names, and add
fallback logic that still reads the old names if the new ones are undefined so
migration is safe (keep the old keys as aliases for now). Ensure exported types
or env accessors that reference these keys are updated to the new identifiers or
resolve from new-or-legacy names.
---
Nitpick comments:
In `@apps/web/src/app/api/tts/generate/route.test.ts`:
- Around line 33-41: Add a test in the existing describe("POST
/api/tts/generate") block that simulates a non-TtsError exception by setting
synthesizeImpl = async () => { throw new Error("unexpected failure"); }; then
call POST(createRequest({ text: "hello" })) and assert the response has status
500 and JSON body matches { error: "Internal server error", detail: "unexpected
failure" } to cover the fallback in route.ts for non-TtsError exceptions; place
the test alongside the other POST tests and reuse the existing
beforeEach/afterEach setup that restores console.error.
In `@apps/web/src/lib/tts/provider.test.ts`:
- Around line 96-105: The test throws a TtsError via Object.assign which is
unnecessary; update the throw to construct the error directly using the TtsError
constructor and pass the retryable and status values (e.g., use new
TtsError({...}, { retryable: false, status: 401 }) or the constructor signature
your TtsError supports) instead of calling Object.assign on the new instance so
the error is created clearly and idiomatically (modify the throw site that
currently calls Object.assign with new TtsError).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efef0a9d-ddc6-496b-8240-8040b82a2144
📒 Files selected for processing (15)
apps/web/src/app/api/tts/generate/route.test.tsapps/web/src/app/api/tts/generate/route.tsapps/web/src/constants/tts-constants.tsapps/web/src/lib/tts/errors.tsapps/web/src/lib/tts/fetch-with-timeout.test.tsapps/web/src/lib/tts/fetch-with-timeout.tsapps/web/src/lib/tts/legacy.test.tsapps/web/src/lib/tts/legacy.tsapps/web/src/lib/tts/openai-compatible.test.tsapps/web/src/lib/tts/openai-compatible.tsapps/web/src/lib/tts/provider.test.tsapps/web/src/lib/tts/provider.tsdocs/plans/2026-03-17-tts-external-provider-design.mddocs/plans/2026-03-17-tts-external-provider.mdpackages/env/src/web.ts
Summary: - follow allowlisted legacy audio redirects instead of failing after validating the target URL - prefer EXTERNAL_TTS_API_* config with API_* aliases kept for backward-compatible migration - update provider integration tests and TTS docs to reflect the new redirect and config behavior Rationale: - the latest PR review caught a real regression where safe legacy redirects could never succeed under redirect: manual - namespaced TTS env keys reduce collisions with unrelated API config while preserving the existing unattended environment - keeping docs and tests aligned prevents this review feedback from recurring in later rework attempts Tests: - bun test apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.test.ts - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/app/api/tts/generate/route.test.ts apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts packages/env/src/web.ts docs/plans/2026-03-17-tts-external-provider.md docs/plans/2026-03-17-tts-external-provider-design.md - bunx tsc -p apps/web/tsconfig.json --noEmit Co-authored-by: Codex <codex@openai.com>
|
[codex] 已处理本轮两条可执行 review:
补充真实探针结论:
请基于最新提交 @coderabbitai review |
|
实时探针结论也已记录, 现在基于最新提交 ✅ Actions performedReview triggered.
|
Summary: - add a route test for unexpected non-TtsError failures so the generic 500 fallback path stays covered - simplify provider test fixtures by constructing TtsError directly instead of mutating the instance after creation Rationale: - the latest review only left test-layer gaps, but they still matter for future refactors around error handling - using the constructor directly keeps the fixture idiomatic and matches the runtime shape more clearly Tests: - bun test apps/web/src/lib/tts/provider.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx @biomejs/biome check apps/web/src/lib/tts/provider.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx tsc -p apps/web/tsconfig.json --noEmit Co-authored-by: Codex <codex@openai.com>
|
[codex] 已处理上一轮 review summary 里剩余的 2 个测试层 nitpick:
验证:
请基于最新提交 @coderabbitai review |
|
验证命令(全量 TTS 测试 + tsc 类型检查)均已执行,现在基于最新提交 ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/lib/tts/legacy.test.ts`:
- Around line 133-150: The follow-up download assertion only checks the URL but
not the fetch redirect mode; update the test in legacy.test.ts where
downloadCallCount and init are used so that for the second fetch you also assert
init?.redirect === "error" (in addition to the URL check), ensuring the fetch
call that follows the 302 uses redirect: "error" rather than the default
"follow". Locate the branch that returns a 200 response after the 302 (the block
that checks downloadCallCount and inspects init and input) and add the redirect
assertion against init to tighten the test.
In `@apps/web/src/lib/tts/openai-compatible.ts`:
- Around line 231-242: The call to response.arrayBuffer() can throw outside the
fetchWithTimeout try-catch, so wrap the arrayBuffer() call in the same error
handling and convert any thrown Error into a TtsError with code
"EXTERNAL_TTS_UPSTREAM", retryable: false and status: response.status (include
the original error message/details). Locate the block using fetchWithTimeout and
response.arrayBuffer(), and either move the arrayBuffer() into that try-catch or
add a specific try-catch around response.arrayBuffer() that rethrows a TtsError
to preserve the provider's retry/fallback logic.
- Around line 84-91: The current validation only parses apiBaseUrl with new
URL(apiBaseUrl) but doesn't enforce protocol, allowing non-HTTP schemes to slip
through; after successfully constructing the URL object from apiBaseUrl, check
url.protocol is "http:" or "https:" and if not throw the same TtsError({ code:
"EXTERNAL_TTS_CONFIG", message: "External TTS is not configured" }) so
misconfigured values (e.g., mailto:, non-HTTP) are classified as config errors
rather than becoming EXTERNAL_TTS_UPSTREAM retryable failures; update the
validation logic around apiBaseUrl in openai-compatible.ts to perform this
protocol check using the URL instance and the existing TtsError.
In `@docs/plans/2026-03-17-tts-external-provider.md`:
- Around line 126-129: Replace the placeholder "<补一段调用适配层的脚本>" in Step 2's Run
line with a concrete bun --eval probe that invokes the TTS adapter entrypoint
(the call that triggers the adapter export/function used by the external TTS
integration) and outputs only the audio byte length; specifically, call the
adapter function used in your codebase (the module/function name exported by the
adapter layer) with sample text and print the returned audio byte length while
ensuring no secrets or API keys are logged; if you cannot construct that probe,
remove the step entirely so the doc is reproducible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0204f431-e763-440b-8560-b449a4c4e3f7
📒 Files selected for processing (9)
apps/web/src/lib/tts/legacy.test.tsapps/web/src/lib/tts/legacy.tsapps/web/src/lib/tts/openai-compatible.test.tsapps/web/src/lib/tts/openai-compatible.tsapps/web/src/lib/tts/provider.test.tsapps/web/src/lib/tts/provider.tsdocs/plans/2026-03-17-tts-external-provider-design.mddocs/plans/2026-03-17-tts-external-provider.mdpackages/env/src/web.ts
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/lib/tts/openai-compatible.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/env/src/web.ts
- apps/web/src/lib/tts/provider.test.ts
- apps/web/src/lib/tts/legacy.ts
- apps/web/src/lib/tts/provider.ts
Summary: - reject non-http external base URLs and wrap audio body read failures as structured non-retryable TtsError values - tighten the legacy redirect success test to assert the follow-up fetch uses redirect: error - replace the implementation-plan placeholder with a concrete bun probe for the external adapter entrypoint Rationale: - the latest review found two real gaps where malformed config or body stream failures could escape the expected TTS error semantics - the stricter redirect assertion pins the security-sensitive fetch mode after the validated redirect handoff - the doc probe now matches the shipped adapter and is reproducible Tests: - bun test apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts docs/plans/2026-03-17-tts-external-provider.md - bunx tsc -p apps/web/tsconfig.json --noEmit Co-authored-by: Codex <codex@openai.com>
Summary: - add service-level tests for base64 decoding from `/api/tts/generate` - verify generated speech is uploaded into the media library and inserted onto an available audio track - verify overlapping inserts allocate a new audio track when needed Rationale: - the remaining acceptance surface was the editor-side integration from generated audio into Cutia's media and timeline workflow - these tests give deterministic evidence for the app integration even while the shared external provider still lacks real audio output Tests: - bun test apps/web/src/lib/tts/service.test.ts - bunx @biomejs/biome check apps/web/src/lib/tts/service.test.ts apps/web/src/lib/tts/service.ts - bunx tsc -p apps/web/tsconfig.json --noEmit Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/web/src/lib/tts/legacy.test.ts (2)
178-228: Use a shared, slightly higher timeout to reduce test flake risk.
timeoutMs: 10is pretty tight for CI scheduler jitter. A shared constant with a bit more headroom keeps the behavior test intact while making failures less timing-sensitive.Proposed refactor
describe("synthesizeSpeechWithLegacyProvider", () => { + const TEST_TIMEOUT_MS = 50; + test("rejects audio urls outside the expected https host allowlist", async () => { const calls: string[] = []; @@ test("aborts the metadata request when the upstream hangs", async () => { await expect( synthesizeSpeechWithLegacyProvider({ text: "hello", - timeoutMs: 10, + timeoutMs: TEST_TIMEOUT_MS, fetchImpl: async (_input, init) => @@ test("aborts the audio download when the legacy audio fetch hangs", async () => { let callCount = 0; @@ synthesizeSpeechWithLegacyProvider({ text: "hello", - timeoutMs: 10, + timeoutMs: TEST_TIMEOUT_MS, fetchImpl: async (_input, init) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/tts/legacy.test.ts` around lines 178 - 228, The two tests use a very small hardcoded timeout (timeoutMs: 10) which causes flakiness; define a shared constant (e.g., const LEGACY_TTS_TIMEOUT_MS = 50) at the top of the test file and replace both timeoutMs: 10 occurrences with that constant when calling synthesizeSpeechWithLegacyProvider; keep the rest of each test (the fetchImpl behavior and expected rejection objects) unchanged so the tests still assert abort behavior but with more CI-friendly timing.
24-84: Metadata fixture setup is repeated; extract a small helper.The repeated
Response.json({ code: 200, url: ... })setup makes the suite noisier than needed. A tiny helper would improve readability and reduce maintenance overhead.Proposed refactor
describe("synthesizeSpeechWithLegacyProvider", () => { + const LEGACY_AUDIO_URL = "https://api.milorapart.top/voice/test.mp3"; + const legacyMetadataOk = () => + Response.json({ + code: 200, + url: LEGACY_AUDIO_URL, + }); + test("rejects non-audio content returned by the legacy audio download", async () => { await expect( synthesizeSpeechWithLegacyProvider({ text: "hello", fetchImpl: async (input) => { if (String(input).includes("/apis/mbAIsc?")) { - return Response.json({ - code: 200, - url: "https://api.milorapart.top/voice/test.mp3", - }); + return legacyMetadataOk(); } @@Also applies to: 90-98, 124-129, 208-213, 167-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/tts/legacy.test.ts` around lines 24 - 84, Several tests in legacy.test.ts repeat the same metadata response (Response.json({ code: 200, url: ... })) inside fetchImpl for synthesizeSpeechWithLegacyProvider; extract a small helper function (e.g., makeMbAIscResponse or mbAiscResponse) that accepts the URL and returns Response.json({ code: 200, url }) and replace the duplicated inline Response.json(...) calls in each test (including the occurrences referenced around synthesizeSpeechWithLegacyProvider in the three shown tests and the other noted ranges) to keep the fetchImpl branches concise and maintainable.apps/web/src/lib/tts/openai-compatible.ts (1)
267-274: Minor: Defensive fallback forlastErrorResponsemay mask programming errors.At line 270, if
lastErrorResponseis null, a synthetic 500 response is created. However, reaching this code path withlastErrorResponse === nullshould be impossible ifendpointUrlsis non-empty (which is guaranteed bygetSpeechEndpointUrls). The defensive fallback is harmless but could mask a logic bug if the URL generation changes. Consider adding an assertion or removing the fallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/tts/openai-compatible.ts` around lines 267 - 274, The fallback that constructs a synthetic Response when lastErrorResponse is null can mask logic bugs; instead assert that lastErrorResponse is present and fail loudly: in the block that currently throws TtsError, replace the synthetic Response fallback with an assertion (or throw a clear internal invariant error) if lastErrorResponse is null, then pass lastErrorResponse directly to getUpstreamErrorMessage; reference the symbols TtsError, lastErrorResponse, getUpstreamErrorMessage, isRetryableStatus and the endpoint generator getSpeechEndpointUrls when making this change so the code fails fast if the supposedly-impossible null state occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/lib/tts/legacy.test.ts`:
- Around line 178-228: The two tests use a very small hardcoded timeout
(timeoutMs: 10) which causes flakiness; define a shared constant (e.g., const
LEGACY_TTS_TIMEOUT_MS = 50) at the top of the test file and replace both
timeoutMs: 10 occurrences with that constant when calling
synthesizeSpeechWithLegacyProvider; keep the rest of each test (the fetchImpl
behavior and expected rejection objects) unchanged so the tests still assert
abort behavior but with more CI-friendly timing.
- Around line 24-84: Several tests in legacy.test.ts repeat the same metadata
response (Response.json({ code: 200, url: ... })) inside fetchImpl for
synthesizeSpeechWithLegacyProvider; extract a small helper function (e.g.,
makeMbAIscResponse or mbAiscResponse) that accepts the URL and returns
Response.json({ code: 200, url }) and replace the duplicated inline
Response.json(...) calls in each test (including the occurrences referenced
around synthesizeSpeechWithLegacyProvider in the three shown tests and the other
noted ranges) to keep the fetchImpl branches concise and maintainable.
In `@apps/web/src/lib/tts/openai-compatible.ts`:
- Around line 267-274: The fallback that constructs a synthetic Response when
lastErrorResponse is null can mask logic bugs; instead assert that
lastErrorResponse is present and fail loudly: in the block that currently throws
TtsError, replace the synthetic Response fallback with an assertion (or throw a
clear internal invariant error) if lastErrorResponse is null, then pass
lastErrorResponse directly to getUpstreamErrorMessage; reference the symbols
TtsError, lastErrorResponse, getUpstreamErrorMessage, isRetryableStatus and the
endpoint generator getSpeechEndpointUrls when making this change so the code
fails fast if the supposedly-impossible null state occurs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4cf4cac0-cfde-40ef-adf6-a54285562f70
📒 Files selected for processing (7)
apps/web/src/app/api/tts/generate/route.test.tsapps/web/src/lib/tts/legacy.test.tsapps/web/src/lib/tts/openai-compatible.test.tsapps/web/src/lib/tts/openai-compatible.tsapps/web/src/lib/tts/provider.test.tsapps/web/src/lib/tts/service.test.tsdocs/plans/2026-03-17-tts-external-provider.md
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/app/api/tts/generate/route.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/lib/tts/provider.test.ts
Summary: - extract shared legacy metadata fixtures and raise timeout test values to reduce scheduler-sensitive flakes - replace the impossible synthetic external fallback response with an explicit invariant error before final upstream error mapping Rationale: - the latest review only pointed out maintainability and test stability issues, but these are cheap to fix and remove avoidable noise - failing loudly on an impossible null upstream response makes future adapter regressions easier to diagnose than masking them as 500s Tests: - bun test apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.test.ts - bunx @biomejs/biome check apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.ts - bunx tsc -p apps/web/tsconfig.json --noEmit Co-authored-by: Codex <codex@openai.com>
|
[codex] 已处理上一轮 review summary 里的 3 个低风险收尾项:
验证:
|
Summary: - add external TTS env examples to apps/web/.env.example - document preferred EXTERNAL_TTS_API_* settings in the README Rationale: - keep repo-local setup docs aligned with the shipped TTS adapter - reduce configuration drift while origin push and live provider remain blocked Tests: - rg -n "EXTERNAL_TTS_API_BASE_URL|EXTERNAL_TTS_API_MODEL|EXTERNAL_TTS_API_KEY|compatibility aliases|Optional TTS env values" apps/web/.env.example README.md - git diff --check Co-authored-by: Codex <codex@openai.com>
Summary: - document an adapter-level external TTS probe in the README - document an end-to-end route probe in the README Rationale: - make future provider validation runnable without digging through Linear - reduce manual handoff friction while external blockers remain Tests: - rg -n "Cutia TTS probe|Cutia route probe|verify that the configured provider can actually return audio|verify the route end-to-end" README.md - git diff --check Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/src/lib/tts/legacy.test.ts (1)
36-37: Reduce repeated metadata-route string matching in mocks.
String(input).includes("/apis/mbAIsc?")is repeated across multiple tests; extracting a small helper/constant would reduce brittleness and future edit overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/tts/legacy.test.ts` around lines 36 - 37, Extract the repeated literal check String(input).includes("/apis/mbAIsc?") into a shared constant or small helper function (e.g., METADATA_ROUTE or isMetadataRequest) and replace the inline checks in the test mocks with that symbol; update the mock branch that currently calls legacyMetadataOk() to use the new constant/helper so all tests reference the same single source of truth and future edits only change the constant/helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/.env.example`:
- Around line 19-23: The example env exposes placeholder
EXTERNAL_TTS_API_BASE_URL/EXTERNAL_TTS_API_MODEL/EXTERNAL_TTS_API_KEY which
makes getExternalTtsConfig() treat external TTS as enabled; comment out or
remove those three EXTERNAL_TTS_* lines in the example so they are disabled by
default (or prefix them with # and include a short note), ensuring a fresh copy
of the .env won't point at the placeholder host and trigger external TTS logic
in getExternalTtsConfig().
In `@apps/web/src/lib/tts/openai-compatible.ts`:
- Around line 223-232: The branches that throw a TtsError after inspecting
mimeType leave the fetch Response body unconsumed; before throwing, explicitly
cancel or consume the body to free the underlying connection (e.g. await
response.arrayBuffer() or await response.body?.cancel()) so the stream is
cleaned up. Update the two places that currently throw the TtsError (the
branches that check mimeType and the other branch at the later check) to first
drain/cancel response (using the existing response variable) and then throw the
TtsError, keeping the rest of the error fields intact.
---
Nitpick comments:
In `@apps/web/src/lib/tts/legacy.test.ts`:
- Around line 36-37: Extract the repeated literal check
String(input).includes("/apis/mbAIsc?") into a shared constant or small helper
function (e.g., METADATA_ROUTE or isMetadataRequest) and replace the inline
checks in the test mocks with that symbol; update the mock branch that currently
calls legacyMetadataOk() to use the new constant/helper so all tests reference
the same single source of truth and future edits only change the
constant/helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52f59345-fefe-4bf1-b4de-304f8ca96df6
📒 Files selected for processing (4)
README.mdapps/web/.env.exampleapps/web/src/lib/tts/legacy.test.tsapps/web/src/lib/tts/openai-compatible.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
Summary:
- add a /responses WebSocket fallback to the external TTS adapter
when /audio/speech is unavailable
- assemble audio chunks from response audio delta events and map
websocket close reasons into structured upstream errors
- extend TTS adapter and provider tests to cover the new fallback
path and updated error behavior
Rationale:
- the active shared gateway exposes TTS generation through
/responses over WebSocket rather than only /audio/speech
- supporting both compatibility shapes narrows the remaining live
blocker to provider account availability instead of protocol gaps
Tests:
- bun test ./src/lib/tts/service.test.ts ./src/lib/tts/fetch-with-timeout.test.ts ./src/lib/tts/provider.test.ts ./src/lib/tts/openai-compatible.test.ts ./src/lib/tts/legacy.test.ts ./src/app/api/tts/generate/route.test.ts
- bun --eval 'import { getExternalTtsConfig, synthesizeSpeechWithOpenAiCompatible } from "./src/lib/tts/openai-compatible.ts"; const config = getExternalTtsConfig({ env: process.env }); const audio = await synthesizeSpeechWithOpenAiCompatible({ config, text: "Cutia TTS probe", voice: "default" }); console.log(audio.byteLength);' (fails: External TTS websocket request failed: no available account)
- NODE_ENV=development NEXT_PUBLIC_SITE_URL=http://localhost:3000 UPSTASH_REDIS_REST_URL=http://localhost:8079 UPSTASH_REDIS_REST_TOKEN=cutia_redis_token bun --eval 'import { NextRequest } from "next/server"; import { POST } from "./src/app/api/tts/generate/route.ts"; const request = new NextRequest("http://localhost/api/tts/generate", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ text: "Cutia route probe", voice: "default" }) }); const response = await POST(request); console.log(response.status); console.log(await response.text());' (returns 502 with no available account)
Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/lib/tts/openai-compatible.ts (1)
601-646: Cancel unconsumed response bodies to release connections promptly.When the HTTP flow branches to the WebSocket fallback (lines 604-606) or continues iterating after a 404 (lines 642-646), the response body is left unconsumed. In server-side fetch runtimes, this can tie up connections until garbage collection.
♻️ Proposed fix
if (response.ok) { const contentType = response.headers.get("content-type") ?? ""; if (!isAudioContentType({ contentType })) { if (shouldTryResponsesWebSocket({ response })) { lastErrorResponse = response; + try { + await response.body?.cancel(); + } catch { + // Best-effort cleanup only. + } break; } throw new TtsError({lastErrorResponse = response; if (response.status !== 404) { break; } + + try { + await response.body?.cancel(); + } catch { + // Best-effort cleanup only. + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/tts/openai-compatible.ts` around lines 601 - 646, When branching to the WebSocket fallback (where shouldTryResponsesWebSocket({ response }) is true) or when continuing after a non-OK 404 (before setting lastErrorResponse and breaking/continuing), ensure you cancel the unconsumed response body to free connections: detect response.body and call its cancel method (or getReader().cancel() if cancel is not exposed), wrapping the call in a safe try/catch so cancellation failures don’t shadow the existing logic that sets lastErrorResponse and triggers the fallback or loop continuation; apply this in the block handling response.ok where you break for shouldTryResponsesWebSocket and in the path just before breaking when response.status !== 404 so the connection is released promptly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/lib/tts/openai-compatible.ts`:
- Around line 601-646: When branching to the WebSocket fallback (where
shouldTryResponsesWebSocket({ response }) is true) or when continuing after a
non-OK 404 (before setting lastErrorResponse and breaking/continuing), ensure
you cancel the unconsumed response body to free connections: detect
response.body and call its cancel method (or getReader().cancel() if cancel is
not exposed), wrapping the call in a safe try/catch so cancellation failures
don’t shadow the existing logic that sets lastErrorResponse and triggers the
fallback or loop continuation; apply this in the block handling response.ok
where you break for shouldTryResponsesWebSocket and in the path just before
breaking when response.status !== 404 so the connection is released promptly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c10c42a-784f-4add-b6ee-eb10c0034db1
📒 Files selected for processing (4)
README.mdapps/web/src/lib/tts/openai-compatible.test.tsapps/web/src/lib/tts/openai-compatible.tsapps/web/src/lib/tts/provider.test.ts
✅ Files skipped from review due to trivial changes (2)
- README.md
- apps/web/src/lib/tts/provider.test.ts
Summary: - fix the FakeWebSocket helper typings in openai-compatible tests so tsc accepts eventful listeners without collapsing them to the open handler - update the websocket test call sites to use a no-arg open event helper - apply biome formatting to the touched TTS files Rationale: - the repo-level web typecheck was still failing even though the test suite passed, which left the ticket with an unverified validation gap - keeping the fix scoped to the test helper preserves runtime behavior while restoring the full validation surface for the TTS work Tests: - bun test apps/web/src/lib/tts/openai-compatible.test.ts - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/service.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx tsc -p apps/web/tsconfig.json --noEmit - bunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/app/api/tts/generate/route.test.ts apps/web/src/lib/tts/errors.ts apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/service.ts apps/web/src/lib/tts/service.test.ts apps/web/src/constants/tts-constants.ts packages/env/src/web.ts README.md apps/web/.env.example docs/plans/2026-03-17-tts-external-provider-design.md docs/plans/2026-03-17-tts-external-provider.md Co-authored-by: Codex <codex@openai.com>
Summary: - treat websocket `no available account` closes as retryable external upstream errors - add regression coverage proving provider fallback recovers from account exhaustion while keeping non-retryable websocket errors terminal - update the design note so retryable external failures include websocket capacity exhaustion Rationale: - the shared external gateway currently reports account exhaustion even though the legacy provider is healthy, so treating it as terminal left real route requests failing with 502 unnecessarily - classifying account exhaustion as retryable preserves the explicit error semantics inside the adapter while restoring user-facing audio output via the existing legacy fallback path Tests: - bun test apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.test.ts - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/service.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx tsc -p apps/web/tsconfig.json --noEmit - bunx @biomejs/biome check apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.test.ts docs/plans/2026-03-17-tts-external-provider-design.md - cd apps/web && NEXT_PUBLIC_SITE_URL=http://localhost:4100 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy NODE_ENV=production bun run build - NODE_ENV=development NEXT_PUBLIC_SITE_URL=http://localhost:4311 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy bun --eval 'import { NextRequest } from "next/server"; import { POST } from "./apps/web/src/app/api/tts/generate/route.ts"; /* status=200 audioLength=34364 */' Co-authored-by: Codex <codex@openai.com>
Summary: - cancel unconsumed external TTS responses before retry and websocket fallback branches - add regression tests that assert response cleanup on 404, html fallback, and non-audio success responses - keep example external TTS env vars disabled by default and extract a shared metadata-route helper in legacy tests Rationale: - releasing abandoned response bodies avoids tying up fetch connections while probing multiple upstream endpoints - disabling placeholder external env vars prevents fresh setups from accidentally enabling the external path against a fake host - absorbing the remaining test helper nit removes repeated brittle string matching from the legacy suite Tests: - bun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/service.test.ts apps/web/src/app/api/tts/generate/route.test.ts - bunx tsc -p apps/web/tsconfig.json --noEmit - bunx @biomejs/biome check apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts - cd apps/web && NEXT_PUBLIC_SITE_URL=http://localhost:4100 UPSTASH_REDIS_REST_URL=https://example.com UPSTASH_REDIS_REST_TOKEN=dummy NODE_ENV=production bun run build - git diff --check Co-authored-by: Codex <codex@openai.com>
Summary: - recommend a concrete TTS-capable model in README and env example - document that shared API_MODEL aliases may point at non-TTS models Rationale: - fresh live probes showed the current alias model is not a clear TTS model - clearer config guidance reduces false negatives during runtime validation Tests: - git diff --check Co-authored-by: Codex <codex@openai.com>
Summary: - document the provider-side prerequisites for successful TTS probes - explain how /models and audio endpoints affect runtime validation Rationale: - recent investigation showed live probe failures were caused by provider capabilities and upstream availability, not local code regressions - keeping that guidance in repo docs reduces repeated misdiagnosis Tests: - git diff --check Co-authored-by: Codex <codex@openai.com>
|
[codex] 当前代码已停在提交 已验证三条路径都因 upstream 写权限不足失败:
如果需要继续推进到 |
关联 Issue
背景
TIA-51在Rework阶段的新一轮返工 PR,目标是收口 review 指出的 redirect 绕过、external non-retryable error 误回退,以及测试/文档遗漏。本次变更
legacy.ts的音频下载保持redirect: "manual"首跳校验,并在 allowlist 校验通过后继续拉取被允许的重定向目标,避免安全收紧后把合法 302 一并打死。openai-compatible.ts现在优先读取EXTERNAL_TTS_API_*,并兼容回退到旧的API_*别名;同时保留 external upstream error 的retryable/status语义,避免把不可重试的 external 契约错误静默回退到 legacy。openai-compatible.ts现在会拒绝非http/httpsbase URL,并把response.arrayBuffer()读取失败包装成 non-retryableTtsError。service.ts已补集成测试,验证/api/tts/generate返回的音频会被解码、加入媒体库并插入音轨,且在音频重叠时自动新建音轨。验证
bun test apps/web/src/lib/tts/service.test.tsbun test apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/app/api/tts/generate/route.test.tsbunx @biomejs/biome check apps/web/src/app/api/tts/generate/route.ts apps/web/src/app/api/tts/generate/route.test.ts apps/web/src/lib/tts/fetch-with-timeout.ts apps/web/src/lib/tts/fetch-with-timeout.test.ts apps/web/src/lib/tts/openai-compatible.ts apps/web/src/lib/tts/openai-compatible.test.ts apps/web/src/lib/tts/legacy.ts apps/web/src/lib/tts/legacy.test.ts apps/web/src/lib/tts/provider.ts apps/web/src/lib/tts/provider.test.ts apps/web/src/lib/tts/service.ts apps/web/src/lib/tts/service.test.ts packages/env/src/web.ts docs/plans/2026-03-17-tts-external-provider.md docs/plans/2026-03-17-tts-external-provider-design.mdbunx tsc -p apps/web/tsconfig.json --noEmit502且保留原始 external 错误,不再被 legacy fallback 假阳性掩盖。当前阻塞
/v1/audio/speech,或在支持 audio output 的场景返回音频字段;但当前共享网关表现为:/v1/audio/speech返回404responses的 audio 模式返回502 upstream_error或503 api_errorchat/completions的普通和流式 audio 请求都只返回文本内容,没有任何音频字段或音频 chunkAPI_BASE_URL、API_MODEL、API_KEY,没有额外的专用 TTS / audio 配置可切换。Summary by CodeRabbit
New Features
Improvements
Tests
Documentation