fix(pdf): time out idle remote PDF reads#84768
Conversation
|
Codex review: needs maintainer review before merge. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. at source/proof level. Current main omits PR rating Rank-up moves:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land the narrow PDF/web-media timeout fix after maintainer review accepts the fixed 120s remote-PDF idle limit, while leaving broader stuck-session recovery in the linked issue or a separate follow-up. Do we have a high-confidence way to reproduce the issue? Yes, at source/proof level. Current main omits Is this the best way to solve the issue? Yes for the narrow PDF hang. The PR reuses the existing Label changes:
Label justifications:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against e42726204490. |
|
ClawSweeper PR egg ✨ Hatched: 🌱 uncommon Tiny Crabkin Hatch commandComment Hatchability rules:
Rarity: 🌱 uncommon. What is this egg doing here?
|
d6f942d to
9fbbb80
Compare
|
Updated the PR body with controlled stalled remote PDF proof, rebased onto latest origin/main, and pushed the refreshed branch. @clawsweeper re-review |
|
Reformatted the proof section with the required behavior/environment/steps/evidence/observedResult/notTested fields while preserving the controlled stalled PDF evidence. @clawsweeper re-review |
|
Adjusted the real behavior proof to use the exact template field lines that the proof checker parses. @clawsweeper re-review |
16cce1d to
2b9c1db
Compare
Remote PDF reads could hang a session when a remote PDF server sent headers and then stopped sending body bytes; this threads the existing shared idle-read timeout through the PDF/web-media path so the tool returns instead of staying stuck.
Fixes #68649
Affected surface
src/agents/tools/pdf-tool.tscreatePdfToolsrc/media/web-media.tsloadWebMediaRaw/readRemoteMediaBufferoption threadingsrc/agents/tools/pdf-tool.test.tssrc/media/web-media.test.tsScope
readIdleTimeoutMs/readResponseWithLimitcontract./new,/restart, or broader stuck-session recovery policy.Real behavior proof
node --import tsx, local HTTP server bound to127.0.0.1, URL redacted as<local-stall-url>. The exercised OpenClaw path wasloadWebMediaRaw->readRemoteMediaBuffer-> sharedreadResponseWithLimitidle timeout contract. No secrets were used; SSRF private-network allowance was enabled only for this local controlled harness.origin/main(8284c035a0)./stall.pdf.content-type: application/pdf,content-length, and initial PDF bytes.loadWebMediaRaw(<local-stall-url>, { maxBytes, readIdleTimeoutMs: 300, ssrfPolicy: { allowPrivateNetwork: true } }).Controlled stalled remote PDF proof:
MediaFetchError: Media download stalled: no data received for 300ms. That shows the after-fix path does not stay stuck on an idle remote PDF body read./newor/restartstuck-session recovery behavior; that is outside this narrow PDF body-read timeout fix.readIdleTimeoutMson the remote PDF load path even though the lower-level body reader only enforces an idle timeout when that option is supplied.Additional checks:
Coverage note
Push-before-PR/update search was rerun after syncing
origin/main; no separate open PR appears to cover this issue/path. The onlyFixes #68649/#68649result is this PR (#84768). Other broad media PRs returned byreadResponseWithLimitsearch do not cover the PDFloadWebMediaRawcaller missingreadIdleTimeoutMs.Helper reuse direction: this is a caller-threading fix into the existing shared read-idle timeout contract. It does not alter
fetch.ts/readResponseWithLimitsemantics and does not introduce a PDF-only timeout policy.What was not tested
/newor/restartstuck-session recovery behavior; that is outside this narrow PDF body-read timeout fix.