Skip to content

fix(claude): bound picker uploads before upstream response - #7015

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:codex/picker-pre-response-deadlines-bf57900-20261012
Closed

luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:codex/picker-pre-response-deadlines-bf57900-20261012

Conversation

@luvs01

@luvs01 luvs01 commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Bound an accepted unfinished Claude picker upload before its first upstream response with a 60-second absolute deadline and a 15-second idle deadline. Idle enforcement respects upstream backpressure, while the absolute deadline still releases admission.
  • Clear both deadlines on upload completion or the first upstream response. Preserve the existing client FIN handling and post-response TCP linger behavior, with regression coverage and synchronized guides in eight languages.

Verification

  • Fixed source HEAD: 5977aab4c36fbc9af512d05664fc12ccfbbff2fb.
  • Current upstream dev: 3bb7a5612c35c8079f5683a6d408a6907cf98bbe. A clean temporary integration at 09daa6a4e1a209b35f21cf688cc204efe5282497 retains the identical source delta and passed 168 tests across 11 files with pinned Bun 1.4.3. Coverage includes upload deadlines, linger and listener lifecycle, picker models, native context, GPT-6.1 Sol rows, Meta Muse Wonder, test layout, file-size ratchet and the core/Lab import boundary.
  • bun run typecheck, bun run privacy:scan, bun run structure:check and git diff --check passed on that integration. The temporary integration is validation evidence; the posted source remains the fixed HEAD above.
  • The fixed source's previous focused validation passed 139 tests. Its docs build passed for eight languages, 569 pages and 79,369 links. No changed guide overlaps the four newer upstream commits.
  • Full-suite and test:changed runs remain outstanding under the repository's resource exception. Native macOS/Windows application and GUI validation are unavailable in this Linux environment. Exact posted-head upstream CI and independent review remain required; this PR stays in draft.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Add 60-second absolute and 15-second idle-progress limits for accepted unfinished uploads. Release aggregate admission on expiry and stop both clocks on body completion or the first upstream reply. Keep upstream backpressure distinct from client idleness.

Preserve the merged graceful FIN and 30-second TCP linger policies from upstream dev bf57900. Adapt upload regressions from fork PR #724 at a4195a9, with new HTTP/1 admission and early-reply tests.
@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 13, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Thank you @luvs01 for the careful work and thorough tests here!

After review I'm closing this one. To clarify the overlap question first: it does not duplicate #6984 / #7010 (43232a4). That change bounds the post-response discard of an abandoned upload, and this PR bounds the pre-response upload phase.

The reasons for closing:

  • The picker listener relays every claude.ai request from Claude Desktop, including file-attachment uploads. A 60-second absolute deadline from request start would cut off legitimate large attachments on slower uplinks (for example, a 30 MB upload at ~3 Mbps takes ~80 s), which is a user-visible regression.
  • A stalled pre-response upload is already bounded in practice: claude.ai's edge times out an idle request body and then responds or closes, and the existing onUpstreamClose / response paths release the slot.

If you've seen a real stall that the upstream timeout doesn't cover, an idle-only bound (no absolute cap) with a reproducer would be very welcome as a fresh PR. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants