fix(daemon): stop killing AMR turns at 120s of first-token silence - #7342
fix(daemon): stop killing AMR turns at 120s of first-token silence#7342lefarcen wants to merge 1 commit into
Conversation
The first-output watchdog gave AMR a two-minute absolute budget: past it the daemon surfaced an error, burned a same-run retry, and SIGTERM'd the child. First-token latency tracks context size (p90 = 277s past 600k tokens), so healthy turns were being declared dead — across 14 days, 968 runs emitted their first output more than ten minutes in and then succeeded. Align the budget with the product's decision in 《Open Design 报错体验 设计方案》 §3: 「10 分钟（Cloud 30 分钟）没输出才报超时」. AMR (`amr_cloud`) is that document's Cloud runtime, so its budget becomes 30 minutes — matching the sliding inactivity watchdog and the ACP stage watchdog it sits beside. Every other runtime keeps its first-output watchdog disabled, which already satisfies 「不到超时不报错」. Also stop diagnosing the user's model. The timeout sentence asserted "The model or CLI likely hung while generating"; the daemon observed silence, not a hang, and the data says that guess is usually wrong. The daemon now reports only what it saw, and the card renders the design's copy off `failure_detail` — 「等了 N 分钟没有新的输出，先停下来了 —— 已做的部分都保留着。」 — in all 19 locales.
lefarcen
added
size/L
|
🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we’ll loop QA in once it’s merge-ready. |
Visual regression reviewHead: 0 changed · 53 unchanged · 0 new without baseline · 0 failed Unchanged cases
Visual diff is advisory only and does not block merging. |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
@lefarcen I reviewed the AMR watchdog change, daemon failure classification, duration readback, and all locale updates. The focused daemon and web Vitest suites, pnpm guard, and full pnpm typecheck all pass. I found one merge-safe duration-reporting edge case in the inline comment. Thanks for the careful end-to-end regression coverage and the clear failure copy.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| const match = /\bwithout emitting (?:a first|any new) output for (\d+)s\b/i.exec(text); | ||
| const seconds = match?.[1] ? Number(match[1]) : NaN; | ||
| if (!Number.isFinite(seconds) || seconds < 60) return null; | ||
| return Math.round(seconds / 60); |
There was a problem hiding this comment.
This rounds the configured wait to the nearest minute, but the daemon has already rounded the operator-configured timeout to whole seconds before writing the error. For a valid OD_CHAT_RUN_FIRST_OUTPUT_TIMEOUT_MS=90000 override, the message says 90s and this returns 2, so the card claims it waited 2 minutes after a 90-second budget; values just under 60s can likewise become 1 minute. That conflicts with this helper's whole-minutes contract and the PR's promise that operator overrides reflect the actual wait. Use one consistent floor/seconds policy (for example, floor the daemon's seconds and this minute conversion, with the sub-minute fallback) and add 59.5s and 90s regression cases.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
|
Heads-up: PR #7406 is also open against the same AMR first-output timeout path ( |
当前服务端 timeout 现状与 30 分钟对齐建议补充一下 AMR Cloud 当前端到端 timeout 链路，方便确认这个 PR 的 30 分钟到底由哪一层拥有。 当前生产配置来源：
因此 #7342 合入后，30 分钟首先是客户端 ceiling，还不是端到端可达的 30 分钟：
Retry 总预算当前 first-output budget 是 per attempt，且保留一次 same-run retry。如果未来把服务端也提高到 30 分钟： 这与产品文档“Cloud 30 分钟没输出才报超时”看起来更像整个 Run 总预算 30 分钟的表达有歧义，需要在服务端对齐前先确定。 推荐的未来目标建议明确由 Open Design 持有 30 分钟 total first-output deadline：
另一个已知边界现有分钟读取使用 总结：我赞同 #7342 把客户端短 120s 限制移除并采用 Cloud 30 分钟方向，但当前应把它描述为客户端 ceiling；真正的端到端 30 分钟需要后续 Vela + Apps timeout hierarchy 和 Run-level retry budget 一起对齐。 |
|
Thank you for the original diagnosis and implementation. We preserved commit |
3 participants
Why
My use case. Two users reported that their tasks "keep getting stuck" — one of them sat on a 171-minute wait. Chasing it down, the run was not stuck: the daemon was killing it.
The pain. AMR (
amr_cloud) is the only runtime that ships a first-output deadline, and it was two minutes (firstOutputTimeoutMs: 2 * 60 * 1000). Every other agent resolves to0, i.e. disabled. Past that budget the daemon surfaces an error, burns the same-run retry, and SIGTERMs the child — on a turn where the provider is very often still composing its first token.The data says two minutes is far too early:
The error copy made it worse by guessing:
The model or CLI likely hung while generating.Per the numbers above that diagnosis is usually wrong, and it points the user at the model instead of at the wait.This PR is not a new policy — it aligns the code with a decision the product has already made. Source of truth: 《Open Design 报错体验设计方案》, quoted verbatim:
AMR /
amr_cloudis the document's "Cloud", so its budget is 30 minutes.The three timeouts, and why only one moved
Reading them before touching anything, because they overlap in coverage but not in semantics:
inactivityTimeoutMs(resolveChatRunInactivityTimeoutMs)firstOutputTimeoutMs(resolveChatRunFirstOutputTimeoutMs)session/promptto the first substantive outputonPromptComplete, or an ACP errorresolveAcpStageTimeoutMs, derived frominactivityTimeoutMs)They are not redundant. The two sliding watchdogs are fed by vela's transport heartbeats, so on a heartbeat-only stall neither ever fires — which is exactly why the absolute first-output budget exists and why deleting it was not an option. Only its value was wrong. No fourth concept was introduced, and all three now land on the same 30-minute ceiling for AMR: one wait, one ceiling, whichever shape the silence takes.
What users will see
Deliberately out of scope
Called out so reviewers do not read these as missing:
0and already conform.Worth flagging for whoever picks up the 60s hint: until it lands, a slow AMR turn shows the ordinary running state for longer than it used to. That is still strictly better than today's behavior (kill + error + retry on a healthy turn), but the hint is what makes the long wait feel intentional rather than silent.
Surface area
OD_CHAT_RUN_FIRST_OUTPUT_TIMEOUT_MSalready existed as the operator override, and this PR is a behavior/copy fix rather than a new capability, so there is noodsubcommand to mirrorpackages/contractsgains one pure helper (readAgentStallWaitedMinutes) beside the existingreadModelWindowResetAt; no DTO shape changedchat.runError.inactivityTimeoutMessagereworded and now takes{minutes}; newchat.runError.inactivityTimeoutMessageNoTime. Both defined intypes.tsand in all 19 locale files, each written in its own orthography — no TODOs, no English placeholdersScreenshots
No new UI surface — the change is the timeout threshold plus the sentence rendered inside the run-error card that already exists. The final copy is pinned verbatim by
apps/web/tests/i18n/runErrors.test.ts.Bug fix verification
Red-first, per AGENTS.md → "Bug follow-up workflow". Four specs, all red before any source change:
Test paths
apps/daemon/tests/amr-first-output-budget.test.ts(new) — wired: realstartServer, real child process, real ACP bridge, fake vela that keeps emitting protocol heartbeats forever without ever producing a token. That stall shape is the one that matters: it deliberately feeds the sliding inactivity watchdog and the ACP stage watchdog, so the only watchdog that can end the run is the first-output budget under test.apps/daemon/tests/runtimes/chat-run-inactivity-timeout.test.ts— the shipped budget, plus a guard that AMR is still the only runtime with a first-output deadline at all.apps/web/tests/runtime/amr-guidance.test.ts— the failure→copy mapping and the duration read-back.apps/web/tests/i18n/runErrors.test.ts— the copy itself: names the wait, keeps the "work is saved" promise, blames nobody.Did they go red on
mainand green on this branch? Yes.Red on
main— daemon budget:Red on
main— wired, through the real server + real child:Red on
main— web copy + mapping:Note on the wired spec's clock. The budget is injected through the existing
OD_CHAT_RUN_FIRST_OUTPUT_TIMEOUT_MSoperator override (3s) rather than waiting out the shipped 30 minutes — no test sleeps for half an hour. The wired spec proves the wiring (silence before the budget is not a failure; the budget still bounds the wait; the sentence states facts; the classification the localized copy keys on survives the rewording); the shipped 30-minute value is proven in the resolver spec, where it costs nothing.Validation
pnpm guard— exit 0pnpm typecheck— exit 0 (all packages)cd apps/daemon && npx vitest run -c vitest.config.ts tests/amr-first-output-budget.test.ts tests/runtimes/chat-run-inactivity-timeout.test.ts— 33 passedcd apps/daemon && npx vitest run -c vitest.config.ts tests/run-failure-classification.test.ts— 108 passedcd apps/daemon && npx vitest run -c vitest.config.ts tests/acp.test.ts— 81 passedcd apps/daemon && npx vitest run -c vitest.config.ts tests/db-message-events.test.ts— 7 passedcd apps/daemon && npx vitest run -c vitest.config.ts tests/run-retry-runtime.test.ts— 11 passed (this suite's two heartbeat-stall cases flaked once at 20s under load; re-verified clean on this branch, and a stashed baseline run of the same suite onmainwas also 11/11, so the flake is the pre-existing timing sensitivitySTALL_WATCHDOG_TIMEOUT_MSdocuments, not a regression)cd apps/web && npx vitest run tests/runtime/amr-guidance.test.ts tests/i18n tests/components/RoutinesSection.test.tsx tests/components/AmrGuidance.test.tsx— 56 passed (includes the locale key/placeholder parity check across all 19 locales)