fix: code review bugs (LLM stream retry, SSE disconnect, publish retry variants, rate limit)#126
Merged
Merged
Conversation
…cate output The previous retry loop wrapped both connection establishment and stream consumption in one try/catch. When the stream failed mid-way (network drop after some tokens were already yielded), the catch branch would trigger a retry that re-yielded content from scratch, producing duplicated and out-of-order text in the user's output. Split the loop into two phases: phase one only retries fetch + status check and breaks once a valid response is obtained; phase two consumes the stream and never retries, surfacing any error directly.
When the client disconnects, controller.enqueue() throws a TypeError on the already-closed stream; the previous catch then tried to enqueue an error event (throwing again) and finally called controller.close() (throwing a third time). Wrap every enqueue and close in tolerant helpers, and skip the error event when the abort signal indicates the client cancelled. Stream consumption now ends cleanly instead of cascading exceptions.
Retry only passed title/content/platforms, dropping the per-platform variant content selected at publish time. A retried task would then publish the original body instead of the adapted variant. Persist variantIds on the SyncTask (optional, no schema migration), and rebuild platformDrafts from those variants on retry via the same resolveVariantsToDrafts helper now shared between publish and retry.
As a local single-user tool there is no reverse proxy, so x-forwarded-for and x-real-ip are client-controlled and trivially spoofable — rotating the header bypassed rate limiting entirely. Drop the IP extraction and bucket by a fixed local key, keeping the separate agent/default limits. localhostGuard is left as-is (its Host check is auxiliary; binding to 127.0.0.1 is the real boundary) with a clarifying comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Code review surfaced 4 confirmed bugs, split into 4 independent commits by theme.
Changes
1. LLM stream retry after mid-stream failure produced duplicate output (
6d402d9)The original retry loop wrapped both connection establishment and stream consumption in a single
try/catch. When the stream failed mid-way (network drop after some tokens were already yielded), thecatchbranch would trigger a retry that re-yielded content from scratch → duplicated, out-of-order text.Split into two phases: phase one only retries
fetch+ status check and breaks once a valid response is obtained; phase two consumes the stream and never retries, surfacing any error directly. Both providers (OpenAI / Anthropic) updated symmetrically.2. SSE enqueue/close cascaded errors on client disconnect (
9a18ac0)After the client disconnected,
controller.enqueue()threw aTypeError; thecatchthen tried to enqueue an error event (throwing again), andfinallycalledcontroller.close()(throwing a third time).Wrapped
send/closein tolerant helpers and skip the error event when the abort signal indicates the client cancelled.3. Retry publish dropped platform variant content (
36787bf)The retry path only passed
title/content/platforms, omittingplatformDrafts/variantIds, so a retried task published the original body instead of the selected per-platform variant.Added an optional
variantIdsfield onSyncTask(backward compatible, no schema migration), persisted at task creation; extracted thevariantIds → platformDraftsresolution intoresolveVariantsToDrafts, shared by both publish and retry.4. rateLimit trusted spoofable XFF header (
dc26853)As a local single-user tool with no reverse proxy,
x-forwarded-for/x-real-ipare client-controlled and trivially spoofable — rotating the header bypassed rate limiting entirely.Dropped IP extraction in favor of a fixed local key, keeping the separate agent/default limits.
localhostGuardlogic is unchanged (binding to 127.0.0.1 is the real boundary); added a clarifying comment on its role.Verification
pnpm --filter @publio/api exec tsc --noEmit✅pnpm --filter @publio/web exec tsc --noEmit✅pnpm lint✅