Skip to content

fix(daemon): give AMR the Cloud first-output budget - #7495

Open
alchemistklk wants to merge 6 commits into
mainfrom
fix/opend-2133-client-first-output-timeout
Open

fix(daemon): give AMR the Cloud first-output budget#7495
alchemistklk wants to merge 6 commits into
mainfrom
fix/opend-2133-client-first-output-timeout

Conversation

@alchemistklk

@alchemistklk alchemistklk commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Why

We are taking over the abandoned draft #7342 on a clean latest-main branch. Its original commit and author are preserved; the two current-main conflicts are resolved without dropping the newer membership-concurrency guidance, and the outstanding duration-rounding review comment is fixed.

AMR's first-output watchdog is now a 15-minute absolute deadline. Production evidence from the Vela stream-establishment monitor shows why the former 120-second threshold terminated healthy long-tail requests across providers, not only Claude/Bedrock:

  • Aiping DeepSeek V4 Pro/Flash produced 146 request_body_written + caller_cancelled failures in the observed window.
  • 145/146 (99.3%) clustered at 122–125 seconds.
  • Those watchdog-shaped cancellations were about 4.2% of the two models' monitored stream attempts.

The approved policy gives AMR (amr_cloud) a 15-minute absolute first-output deadline while retaining its 30-minute sliding inactivity watchdog. The closed #7406 moved the limit only to 150 seconds; it was correctly closed as a duplicate.

Related:

What users will see

  • A quietly waiting AMR turn is no longer killed after two minutes. Its absolute first-output budget is 15 minutes.
  • The sliding inactivity watchdog remains 30 minutes. ACP owns setup only; after prompt dispatch the first-output deadline is the sole pre-output watchdog.
  • A first-output deadline expiry is terminal and does not start a same-run retry.
  • Other agents remain unchanged and keep their first-output deadline disabled.
  • A real timeout reports observed silence rather than guessing that the model or CLI hung.
  • The existing error card says how many whole minutes elapsed and promises completed work is kept. A one-minute operator override uses grammatical singular copy; a sub-minute value uses the no-duration fallback.

The separate 60-second waiting hint and Continue Running button remain out of scope.

Surface area

  • UI — no new surface; only copy in the existing run-error card changes
  • Keyboard shortcut
  • CLI / env varOD_CHAT_RUN_FIRST_OUTPUT_TIMEOUT_MS already exists and retains override/0 disable semantics
  • API / contract — no endpoint, SSE shape, or DTO change; contracts only owns the pure shared diagnostic formatter/parser
  • Extension point
  • i18n keys — plural, singular one-minute, and no-duration timeout copy across all 19 locales
  • New top-level dependency
  • Default behavior change — AMR first-output budget changes from 120 seconds to 15 minutes; its deadline expiry does not retry the same run
  • None

Screenshots

No new UI entry point or component. This changes the threshold and localized sentence inside the existing run-error card; English and Chinese copy are pinned by apps/web/tests/i18n/runErrors.test.ts.

Bug fix verification

  • apps/daemon/tests/amr-first-output-budget.test.ts — real server + child + ACP heartbeat-only stall; old behavior/copy was red on main in fix(daemon): stop killing AMR turns at 120s of first-token silence #7342 and is green here.
  • apps/daemon/tests/runtimes/chat-run-inactivity-timeout.test.ts — shipped 15-minute first-output budget, 30-minute inactivity budget, and non-AMR guard.
  • apps/web/tests/runtime/amr-guidance.test.ts — duration readback, 59.5-second floor, 90-second singular, unreadable fallback, and current-main failure mappings.
  • packages/contracts/tests/runtime/agent-stall-timeout.test.ts — shared daemon formatter/parser ownership so prose cannot drift silently.
  • apps/web/tests/i18n/runErrors.test.ts — timeout copy and no-causal-guess contract.

Validation

  • pnpm guard — passed
  • pnpm typecheck using repository-pinned pnpm@10.33.2 — passed
  • daemon wired first-output + shipped policy suites — 35 passed
  • complete same-run retry suite — 11 passed
  • focused generic/caller-owned ACP timer ownership — 2 passed (82 unrelated cases skipped)
  • daemon classification/retry/ACP validation from takeover baseline — passed
  • web timeout/i18n suites — 53 passed
  • contracts formatter/parser suite — 3 passed
  • independent standards/spec review — prior singular/prose-ownership findings fixed; final review recorded before publication

lefarcen and others added 2 commits August 27, 2026 15:59
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.
@alchemistklk
alchemistklk requested a review from nettee August 27, 2026 08:30
@lefarcen lefarcen added size/XL PR changes 700-1500 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix needs-validation Runtime change detected; needs human or /explore agent validation. labels Aug 27, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

🧪 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0dd2870c7c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/daemon/src/runtimes/defs/amr.ts Outdated
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Visual regression review

Head: 98303cb · Base: f2e1bec

0 changed · 53 unchanged · 0 new without baseline · 0 failed

Unchanged cases
Case Main PR Diff
visual-avatar-local-agent-list
0 px (0.00%)
main pr diff
visual-avatar-local-agent-list-panel
0 px (0.00%)
main pr diff
visual-avatar-menu
0 px (0.00%)
main pr diff
visual-avatar-menu-panel
0 px (0.00%)
main pr diff
visual-avatar-open-design-model-picker
0 px (0.00%)
main pr diff
visual-critical-settings
0 px (0.00%)
main pr diff
visual-critical-workspace
0 px (0.00%)
main pr diff
visual-critical-workspace-preview
0 px (0.00%)
main pr diff
visual-deepseek-unpaid-campaign-600
0 px (0.00%)
main pr diff
visual-deepseek-unpaid-campaign-short-height
0 px (0.00%)
main pr diff
visual-design-system-detail
0 px (0.00%)
main pr diff
visual-design-systems
4 px (0.00%)
main pr diff
visual-home
0 px (0.00%)
main pr diff
visual-home-catalog
0 px (0.00%)
main pr diff
visual-home-context-picker
0 px (0.00%)
main pr diff
visual-home-context-picker-popover
0 px (0.00%)
main pr diff
visual-home-plugin-filter
0 px (0.00%)
main pr diff
visual-home-plugin-use-staged
0 px (0.00%)
main pr diff
visual-home-plugin-use-with-query
0 px (0.00%)
main pr diff
visual-home-staged-attachment
0 px (0.00%)
main pr diff

Visual diff is advisory only and does not block merging.

@nettee nettee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 30-minute AMR value addresses the original short deadline, but the default ACP path still lets its prompt-stage watchdog win the same deadline on a silent bridge. That bypasses the new first-output classification and the localized duration/work-preserved card; the inline comment details the timer ordering and the regression coverage needed.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Comment thread apps/daemon/src/runtimes/defs/amr.ts Outdated
@alchemistklk
alchemistklk requested a review from nettee August 27, 2026 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-validation Runtime change detected; needs human or /explore agent validation. risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/XL PR changes 700-1500 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants