Skip to content

refactor(adk): improve turn loop structure#1134

Merged
shentongmartin merged 4 commits into
alpha/10from
refactor/tl
Jul 7, 2026
Merged

refactor(adk): improve turn loop structure#1134
shentongmartin merged 4 commits into
alpha/10from
refactor/tl

Conversation

@shentongmartin

@shentongmartin shentongmartin commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

TurnLoop Teardown and Structure

Problem

TurnLoop had three quality risks in the same lifecycle area:

  1. A proxy goroutine could still be forwarding framework events after OnAgentEvents returned. That made captured framework state such as CancelError and interrupt contexts vulnerable to unsafe concurrent access during Stop or preempt teardown.
  2. Cleanup could drain TurnBuffer before preventing later writes. A concurrent Push that had already passed the stopped check could write to the buffer after TakeAll, making an accepted item disappear from both UnhandledItems and TakeLateItems.
  3. The implementation had grown into one very large file, which made the lifecycle invariants harder to audit: preempt routing, stop handling, checkpoint persistence, turn planning, and event forwarding were all colocated.

Solution

The teardown path now treats the proxy goroutine as part of the turn lifecycle. The loop cancels the agent, drains the proxy iterator, and waits for proxyDone before reading framework-captured state.

Cleanup now closes TurnBuffer before the final TakeAll. This makes the exit boundary explicit: pushes that arrive before the close are drained as unhandled items, while pushes that race with or arrive after the close fail TrySend and are recoverable as late items.

Per-turn mutable state now lives in turnExecution, so the TurnLoop object no longer carries transient cancel, interrupt, and runner checkpoint fields across turns.

The large implementation file is split along existing lifecycle boundaries:

  • turn_loop_preempt.go: preempt options, Push preempt routing, and preemptController.
  • turn_loop_stop.go: Stop options, Stop method behavior, and stopController.
  • turn_loop_checkpoint.go: checkpoint persistence, resume state, bridge store setup, and cleanup checkpoint decisions.
  • turn_loop_run.go: item collection, turn planning, event forwarding, proxy teardown, and framework error handling.
  • turn_loop.go: public TurnLoop types, configuration, constructor, Run, and Wait.

Key Insight

For TurnLoop, done only means the user callback has returned. It does not imply the framework proxy has finished observing the underlying agent iterator. Correct teardown needs a second boundary: callback completion first, proxy completion second, then exit-state/checkpoint decisions.

For cleanup, stopped is only an admission check. The durable recovery boundary is the buffer close: after cleanup starts, every pushed item must be classified exactly once as either unhandled or late.

Summary

Problem Solution
Framework CancelError could be captured after callback return Join proxy teardown before reading captured framework state
Accepted Push items could be written after cleanup's final drain Close TurnBuffer before TakeAll, routing post-close pushes to late items
Per-turn state lived on the long-lived loop object Move transient state into turnExecution
TurnLoop lifecycle logic was difficult to audit in one file Split around preempt, stop, checkpoint, and run lifecycle boundaries

TurnLoop 退出流程与结构整理

问题

TurnLoop 在同一生命周期区域存在三个质量风险:

  1. OnAgentEvents 返回后,代理 goroutine 仍可能继续转发 framework event。这会让 CancelError、interrupt contexts 等 framework 捕获状态在 Stop 或 preempt teardown 期间存在并发访问风险。
  2. cleanup 可能先 drain TurnBuffer,之后才阻止后续写入。并发 Push 如果已经通过 stopped 检查,就可能在 TakeAll 之后写入 buffer,导致一个已被接受的 item 既不在 UnhandledItems 中,也不在 TakeLateItems 中。
  3. 实现集中在一个过大的文件中,preempt 路由、stop 处理、checkpoint 持久化、turn planning、event forwarding 混在一起,生命周期不变量不容易审查。

方案

teardown 路径现在把代理 goroutine 视为 turn 生命周期的一部分。循环会先 cancel agent,再 drain proxy iterator,并等待 proxyDone,之后才读取 framework 捕获状态。

cleanup 现在会在最终 TakeAll 前关闭 TurnBuffer。这让退出边界更明确:close 前到达的 push 会作为 unhandled item 被 drain;与 close 竞争或 close 后到达的 push 会因为 TrySend 失败而作为 late item 可恢复。

每个 turn 的临时状态现在保存在 turnExecution 中,TurnLoop 本身不再跨 turn 持有 transient cancel、interrupt、runner checkpoint 字段。

大文件按已有生命周期边界拆分:

  • turn_loop_preempt.go:preempt options、Push preempt routing 和 preemptController
  • turn_loop_stop.go:Stop options、Stop method behavior 和 stopController
  • turn_loop_checkpoint.go:checkpoint persistence、resume state、bridge store setup 和 cleanup checkpoint decisions。
  • turn_loop_run.go:item collection、turn planning、event forwarding、proxy teardown 和 framework error handling。
  • turn_loop.go:public TurnLoop types、configuration、constructor、Run 和 Wait。

关键认知

TurnLoop 来说,done 只表示用户回调已经返回,不表示 framework proxy 已经完成对底层 agent iterator 的观察。正确的退出流程需要第二个边界:先等待 callback 完成,再等待 proxy 完成,之后才能做 exit-state 和 checkpoint 决策。

对 cleanup 来说,stopped 只是 admission check。真正的可恢复边界是 buffer close:cleanup 开始后,每个 pushed item 都必须且只能被归类为 unhandled 或 late。

总结

问题 方案
framework CancelError 可能在 callback 返回后才被捕获 读取捕获状态前先 join proxy teardown
已接受的 Push item 可能在 cleanup 最终 drain 后写入 TakeAll 前关闭 TurnBuffer,将 close 后 push 路由到 late items
每个 turn 的临时状态保存在长期存在的 loop 对象上 将 transient state 移入 turnExecution
TurnLoop 生命周期逻辑集中在单个大文件中,难以审查 围绕 preempt、stop、checkpoint、run 生命周期边界拆分

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.62076% with 104 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (alpha/10@43d03da). Learn more about missing BASE report.

Files with missing lines Patch % Lines
adk/turn_loop_run.go 83.14% 38 Missing and 21 partials ⚠️
adk/turn_loop_checkpoint.go 82.68% 21 Missing and 10 partials ⚠️
adk/turn_loop_stop.go 95.03% 4 Missing and 4 partials ⚠️
adk/turn_loop_preempt.go 97.87% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             alpha/10    #1134   +/-   ##
===========================================
  Coverage            ?   81.82%           
===========================================
  Files               ?      193           
  Lines               ?    30599           
  Branches            ?        0           
===========================================
  Hits                ?    25038           
  Misses              ?     3764           
  Partials            ?     1797           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shentongmartin shentongmartin force-pushed the refactor/tl branch 2 times, most recently from 663a748 to b96ffa6 Compare July 6, 2026 08:20
Comment thread adk/interrupt.go
Comment thread adk/interrupt.go
Change-Id: I49b6c3194e0cb6de1dbffa8e3a00cef61f5ceab2
Change-Id: I4196fdde757e9f954978f3eb563a16a0aeae8634
Change-Id: I28981523402d5962d8500c0df0d749afdc810482
Change-Id: Icaca8726d10128529b496b3565292e5acfc192c6
@shentongmartin shentongmartin merged commit b074fff into alpha/10 Jul 7, 2026
16 checks passed
@shentongmartin shentongmartin deleted the refactor/tl branch July 7, 2026 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants