Repository navigation
fix(claude): expire idle picker uploads without a lifetime cap #736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -378,3 +378,78 @@ for (const protocol of ["h1", "h2"] as const) { | |
| } finally { await wait(f.close()); } | ||
| }); | ||
| } | ||
|
|
||
|
|
||
| for (const protocol of ["h1", "h2"] as const) { | ||
| bounded(`${protocol} idle upload releases admission without any upstream timeout`, async wait => { | ||
| const entered = Promise.withResolvers<void>(); | ||
| const upstreamClosed = Promise.withResolvers<void>(); | ||
| const f = await fixture((req, res) => { | ||
| req.on("error", () => res.destroy()); req.resume(); | ||
| if (req.url === "/idle") { | ||
| req.once("data", () => entered.resolve()); | ||
| req.once("close", () => upstreamClosed.resolve()); | ||
| // No upstream response, socket timeout, or forced close: only the relay can release it. | ||
| } else res.end("ordinary"); | ||
| }, { uploadIdleMs: 120, maxActiveUpstreams: 1 }); | ||
| try { | ||
| const session = f.h2(); | ||
| const upload = protocol === "h2" | ||
| ? session.request({ ":method": "POST", ":path": "/idle", ":authority": "claude.ai" }) | ||
| : f.h1("/idle", "POST", { "Content-Length": "1000" }); | ||
| const closed = protocol === "h2" ? observeClose(upload) : observeSocketClose(upload as ClientRequest); | ||
| upload.write("x"); await wait(entered.promise); | ||
| expect((await wait(normalH2(session))).status).toBe(503); | ||
| await wait(closed); await wait(upstreamClosed.promise); | ||
| expect(await wait(normalH2(session))).toEqual({ status: 200, text: "ordinary" }); | ||
| expect(f.logs.filter(line => line === "picker upload idle timeout")).toHaveLength(1); | ||
| } finally { await wait(f.close()); } | ||
| }); | ||
|
|
||
| bounded(`${protocol} progressing upload outlives the idle interval and preserves every byte`, async wait => { | ||
| let received = ""; | ||
| const entered = Promise.withResolvers<void>(); | ||
| const f = await fixture((req, res) => { | ||
| req.on("error", () => res.destroy()); | ||
| req.on("data", chunk => { received += chunk.toString(); entered.resolve(); }); | ||
| req.once("end", () => res.end(received)); | ||
| }, { uploadIdleMs: 160 }); | ||
| try { | ||
| const upload = protocol === "h2" | ||
| ? f.h2().request({ ":method": "POST", ":path": "/progress", ":authority": "claude.ai" }) | ||
| : f.h1("/progress", "POST", { "Content-Length": "13" }); | ||
| const result = protocol === "h2" ? h2Result(upload as ClientHttp2Stream) : h1Result(upload as ClientRequest); | ||
| upload.write("x"); await wait(entered.promise); | ||
| for (let i = 0; i < 8; i++) { await wait(Bun.sleep(40)); upload.write("x"); } | ||
| upload.end("tail"); | ||
| expect(await wait(result)).toEqual({ status: 200, text: "xxxxxxxxxtail" }); | ||
| expect(f.logs).not.toContain("picker upload idle timeout"); | ||
| } finally { await wait(f.close()); } | ||
|
Comment on lines
+409
to
+427
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: rg -n 'backpressure|pause|resume|uploadIdleMs|idle timeout' tests/claude-integration/claude-picker-upload.test.tsRepository: luvs01/opencodex Length of output: 1834 🏁 Script executed: set -eu
printf '%s\n' '--- diff files ---'
git diff --stat f5f73ff3970b3b238c4e2fade29ba3f64914f637 3d9a7274da1405431ba81b0315504064edd78447 -- tests/claude-integration/claude-picker-upload.test.ts src/claude/intercept/picker-listener.ts
printf '%s\n' '--- listener references ---'
rg -n -F -- 'uploadIdleMs' 'src/claude/intercept/picker-listener.ts' 'tests/claude-integration/claude-picker-upload.test.ts'
rg -n -F -e 'pause' -e 'resume' -e 'req.pipe' -e 'request' 'src/claude/intercept/picker-listener.ts' 'tests/claude-integration/claude-picker-upload.test.ts'
printf '%s\n' '--- listener source ---'
nl -ba src/claude/intercept/picker-listener.ts
printf '%s\n' '--- upload test source ---'
nl -ba tests/claude-integration/claude-picker-upload.test.ts | sed -n '1,480p'
printf '%s\n' '--- fixture definitions ---'
rg -n -F -e 'function fixture' -e 'const fixture' -e 'async function fixture' -e 'interface' tests/claude-integration | head -80Repository: luvs01/opencodex Length of output: 42900 🏁 Script executed: set -eu
printf '%s\n' '--- listener source ---'
nl -ba src/claude/intercept/picker-listener.ts
printf '%s\n' '--- upload tests ---'
nl -ba tests/claude-integration/claude-picker-upload.test.ts | sed -n '330,480p'
printf '%s\n' '--- fixture and pipe wiring ---'
rg -n -F -e 'function fixture' -e 'const fixture' -e 'req.pipe' -e 'uploadIdleMs' tests/claude-integration src/claudeRepository: luvs01/opencodex Length of output: 41474 Add an upstream-backpressure pause-and-resume regression case. The current upload tests do not create upstream-induced request backpressure. The progressing case consumes Add HTTP/1.1 and HTTP/2 coverage that stops the upstream from reading, sends enough data to pause the relay request, waits beyond 🤖 Prompt for AI Agents |
||
| }); | ||
|
|
||
| for (const completed of [true, false]) { | ||
| bounded(`${protocol} ${completed ? "completed upload" : "early response"} leaves long-lived responses outside the idle guard`, async wait => { | ||
| const answered = Promise.withResolvers<void>(); | ||
| let response: ServerResponse | undefined; | ||
| const f = await fixture((req, res) => { | ||
| req.on("error", () => res.destroy()); req.resume(); | ||
| const begin = () => { | ||
| response = res; res.writeHead(200, { "Content-Type": "text/event-stream" }); | ||
| res.write("data: first\n\n"); answered.resolve(); | ||
| }; | ||
| if (completed) req.once("end", begin); else req.once("data", begin); | ||
| }, { uploadIdleMs: 80 }); | ||
| try { | ||
| const upload = protocol === "h2" | ||
| ? f.h2().request({ ":method": "POST", ":path": "/events", ":authority": "claude.ai" }) | ||
| : f.h1("/events", "POST", { "Content-Length": completed ? "1" : "1000" }); | ||
| const result = protocol === "h2" ? h2Result(upload as ClientHttp2Stream) : h1Result(upload as ClientRequest); | ||
| if (completed) upload.end("x"); else upload.write("x"); | ||
| await wait(answered.promise); await wait(Bun.sleep(240)); | ||
| response!.end("data: last\n\n"); | ||
| expect(await wait(result)).toEqual({ status: 200, text: "data: first\n\ndata: last\n\n" }); | ||
| expect(f.logs).not.toContain("picker upload idle timeout"); | ||
| } finally { await wait(f.close()); } | ||
| }); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Move the Picker timeout rule into each locale’s Picker section.
The new paragraphs describe the local Picker relay, but their headings place them in gateway-profile or remote-hub guidance. That placement can misstate which mode uses the timeout.
docs-site/src/content/docs/tr/guides/claude-code.md#L282-L282: move the paragraph out of the gateway-only profile section and into Picker mode.docs-site/src/content/docs/ru/guides/claude-code.md#L208-L208: move the paragraph out of the remote-hub section and into Picker mode.docs-site/src/content/docs/zh-cn/guides/claude-code.md#L182-L182: move the paragraph out of the remote-hub section and into Picker mode.docs-site/src/content/docs/zh-tw/guides/claude-code.md#L267-L267: move the paragraph out of the remote-hub section and into Picker mode.As per path instructions, “Check that user-facing docs stay in sync with actual CLI/API behavior and that translated locale pages (ja, ko, ru, zh-cn) are not left contradicting the English source.”
📍 Affects 4 files
docs-site/src/content/docs/tr/guides/claude-code.md#L282-L282(this comment)docs-site/src/content/docs/ru/guides/claude-code.md#L208-L208docs-site/src/content/docs/zh-cn/guides/claude-code.md#L182-L182docs-site/src/content/docs/zh-tw/guides/claude-code.md#L267-L267🤖 Prompt for AI Agents
Source: Path instructions