Repository navigation
fix(chat): keep the HTTP status when the read body fails - #6625
Conversation
The read boundary read the response body and only then built the error that carries httpStatus, so a failure after the headers lost the status that had already arrived. Discovery then classified on how the body read failed instead of on the status: a 503 whose body stalled past the request limit raised AbortError and never retried, and a 403 whose body dropped mid-read raised TypeError and retried a denied read. Keep the known status when the body read rejects, so the existing discovery decision stays authoritative: 5xx bodies still recover, denied and other 4xx bodies still stop. Cover both directions in the packaged browser scenario with fixture responses that send headers and then stall or cut the body short. Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; provider=OpenAI; runtime_reported
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
评审 6625@1ba07105daba6a4948a1d02fc7edf8d14e3acf7e。这是 #6592 合并后审计所提 P2 的修复:读取边界在正文读取失败时丢失了已经到达的 HTTP 状态,从而把重试判断交给了“正文是怎么失败的”,而不是状态本身。
动机
在 Goal 中打开指定 Agent 原工作记录、等待后续结果的本机所有者。
合并后的读恢复已经能处理网络中断、超时、完整 JSON/HTML/空体 5xx 与 4xx;但错误只在正文成功读完之后才被构造,于是响应头已经到达、正文却失败时状态会丢失。两个相反方向的误判随之出现:503 的正文超过请求期限时抛出 AbortError,状态没了又不算暂时中断,页面停住直到手动重连;403 的正文中途断流时抛出 TypeError,被当成暂时中断,于是对一次权限拒绝自动重试。
修复后,已知状态继续决定可重试性:5xx 正文停滞仍自动恢复,4xx 正文断流仍停住,不新增权限、不改变原 App 授权边界。
改动思路
状态原本就来自同一个 Response,属于派生诊断事实,不是新的会话或权限状态。因此修复留在保有 Response 的同一 request 边界:正文读取失败时抛出既有 ConversationRequestError 并带上已知状态,调用方发现逻辑(interruptedRead || httpStatus >= 500)保持不变、继续权威;403 等已知 4xx 的停止边界与 5xx 的恢复边界同时保留。由于构造顺序是唯一缺陷,不需要新状态框架、重连服务或共享协议。
回归放在既有打包浏览器场景:Playwright 无法流式给出部分正文,因此由既有 HTTP fixture 新增两个“先发响应头、再让正文停滞或按声明的 Content-Length 提前关闭”的故障端点,分别覆盖 503 停滞必须恢复、403 断流必须停住。
具体改动
共 4 个文件,+78/−6。
attached-agent-conversations.tsx:把await response.text()包进 try;失败时抛出带response.status的 ConversationRequestError。核心是让状态穿过正文失败,而不是按异常类型重新分类;发现逻辑一行未改。codex_attached_fixture.py:新增body_fault与/fixture/discovery-body-timeout、/fixture/discovery-body-drop两个布点端点;命中后先发 503/403 响应头与偏大的 Content-Length,写一段正文并 flush,timeout 分支保持连接超过客户端 6 秒读取上限,drop 分支提前关闭。故障只作用于不带agent_id的发现读取,消费一次即失效。codex-attached.mjs:新增两个故障的往返用例,断言可重试方向需自动恢复、不可重试方向在 1.3 秒内读取次数不自增,并保留“零原生输入”与目标 Agent 不变的断言。session-runtime-control-plane-adapter.md:记录响应头之后正文失败时仍由状态决定可重试性。
对主干的风险
行为变化仅限“响应头已到、正文读取失败”这一类失败的重试归类:5xx 从“停住”改为自动恢复,4xx 从“误重试”改为停住。纯传输失败(fetch 本身被拒)路径未改,interruptedRead 与既有完整响应体分类未改;409 未提交 operation、发送回执、离开页面取消定时器与零原生输入等边界均保持不变。可重试性仍由已知状态决定,不引入无差别重试 AbortError 或字符串匹配。
残余风险与限制:两个故障响应是合成 fixture,使用真实浏览器 fetch、真实 HTTP 处理器与真实打包产物,但不构成已安装 App 或真实 Desktop 宿主资格;timeout 分支使场景多等待约 6 秒;playwright 是流式响应能力限制,若未来支持流式 body,可把这两个故障迁回场景内注入。这些都不影响本修复的正确性判断。
我的整体评价
结论 APPROVE。修复点准确且最小:把已经存在的状态贯穿正文读取失败,让原有发现决定继续权威,同时保住了 4xx 停止边界。本地取证:打包场景 attached-host-follow-up 带修复通过;撤掉修复后同一场景分别以 body-timeout-503 left discovery unrecovered 与 body-drop-403 does not retry automatically 失败,说明两个方向都能被回归抓住;dashboard tsc --noEmit 干净;tests/presentation/test_codex_attached_conversation.py 25 项通过;ruff 与 git diff --check 干净。
English verdict: APPROVE - 1ba0710; the read boundary now carries the known HTTP status through a body-read failure, so 5xx bodies still recover and denied 4xx bodies still stop, with both directions covered by packaged-scenario regressions that fail without the fix.
What
Keeps the known HTTP status when the response body read fails, so the attached-conversation read boundary classifies retryability on the status instead of on how the body read happened to fail.
Why
Follow-up to the post-merge audit on #6592. That audit reproduced two opposite misclassifications at the same
requestboundary:AbortError, which lost the 503 and left discovery stalled until a manual reconnect;TypeError, which made a denied read retry automatically.Both happen after the headers arrived, so the status was already known and should have decided.
Changes
attached-agent-conversations.tsx: wrapresponse.text(); when it rejects, throw the sameConversationRequestErrorwith the status that already arrived. The existing discovery rule (interruptedRead || httpStatus >= 500) then stays authoritative: 5xx retries, denied/other 4xx stops.codex_attached_fixture.py: two armed fault endpoints that answer with headers and then either stall the body past the client limit or close short against the declaredContent-Length.codex-attached.mjs: cover both directions in the packaged scenario, keeping the zero-native-input assertion.session-runtime-control-plane-adapter.md: record that the status keeps deciding retryability after the headers.Validation
attached-host-follow-up: passes with the fix; with the fix reverted it fails on exactly the two new faults (body-timeout-503 left discovery unrecovered, andbody-drop-403 does not retry automaticallywhen run first), so both directions are discriminating.npx tsc --noEmiton the dashboard: clean.tests/presentation/test_codex_attached_conversation.py: 25 passed.ruff checkon the changed fixture: clean;git diff --checkclean.Coverage gap
The two fault responses are synthetic fixture output; they exercise the real browser fetch, the real HTTP handler and the real production bundle, but they do not qualify an installed App or a real Desktop host.