Skip to content

feat(subagent): 完成 #278 子代理取消/重试/恢复与标准事件流#346

Merged
pionxe merged 4 commits into
1024XEngineer:mainfrom
Cai-Tang-www:feat(subagent)
Apr 17, 2026
Merged

feat(subagent): 完成 #278 子代理取消/重试/恢复与标准事件流#346
pionxe merged 4 commits into
1024XEngineer:mainfrom
Cai-Tang-www:feat(subagent)

Conversation

@Cai-Tang-www

Copy link
Copy Markdown
Collaborator

背景

本 PR 对齐并完成 issue #278:为 subagent 补齐可用级执行可靠性(取消/重试/恢复)与可观测事件流。

Closes #278

本次改动

1) 调度可靠性(Scheduler)

  • 新增失败策略:continue_on_error / fail_fast
  • 新增恢复策略:retry / fail
  • 启动恢复:调度开始前自动处理遗留 in_progress 任务,恢复到可重试或失败终态
  • 默认重试退避改为指数退避(可配置 base/max)
  • 增加回写幂等保护:忽略过期 attempt 的结果回写,避免重复写入
  • 失败/取消/重试回写时清理 owner_type / owner_id,避免假锁定状态残留

2) 超时与取消语义

  • 区分 context.Canceledcontext.DeadlineExceeded
  • deadline 统一映射为 timeout 语义(StopReasonTimeout
  • 取消保留 canceled 语义

3) 事件可观测性

  • 补齐标准事件类型:subagent_retried
  • 在调度关键路径补发标准事件:
    • subagent_started
    • subagent_progress
    • subagent_retried
    • subagent_completed
    • subagent_failed
    • subagent_canceled

测试

  • 新增/增强测试覆盖:
    • 恢复场景(retry/fail 两种模式)
    • fail-fast 与 continue-on-error 策略分支
    • 过期 attempt 幂等防护
    • 标准事件顺序断言
    • timeout/cancel 语义分支
  • 本地验证:
    • go test ./internal/subagent/... ./internal/runtime/...

影响范围

  • internal/subagent/*
  • internal/runtime/subagent_run.go
  • internal/runtime/events_subagent.go

风险与说明

  • go test ./... 当前仍受仓库既有用例 internal/configTestLoadCustomProvidersReadDirAndStatErrors/providers_dir_read_error 影响(与本 PR 无关)。

@Cai-Tang-www

Copy link
Copy Markdown
Collaborator Author

@xgopilot 先分析当前PR中可能存在哪些风险点,然后给我一个让AI能够进行review的提示词

@fennoai fennoai Bot 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.

Reviewed in four passes (code quality, performance, security, docs). One noteworthy issue remains.

Comment thread internal/subagent/scheduler.go Outdated
delete(state.running, outcome.id)
running, ok := state.running[outcome.id]
if ok {
delete(state.running, outcome.id)

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.

state.running is deleted before verifying outcome.attempt matches the currently running attempt. For stale outcomes (older attempt arrives after a retry has already started), this removes the active run from tracking, and the real in-flight attempt can later be ignored (!ok path). Consider checking outcome.attempt == running.attempt first, and only deleting from state.running when it matches.

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

风险点分析

  1. 高风险:stale attempt 会错误清除当前运行态,导致在跑任务“失踪”
    位置:internal/subagent/scheduler.go:480-490
    问题:handleOneOutcomedelete(state.running, outcome.id),再校验 outcome.attempt 是否匹配当前 running attempt。若旧 attempt 的结果晚到,会把新 attempt 的 running 记录误删,然后直接 return nil。后果是:

    • 真正仍在执行的 attempt 不再被调度器跟踪;
    • 后续真实结果可能走 !ok 分支被忽略;
    • 统计、取消、终态回写可能不一致。
      建议:先比对 attempt,一致时才删除 state.running
  2. 中风险:恢复阶段直接失败的任务未计入 ScheduleResult.Failed,结果汇总可能失真
    位置:internal/subagent/scheduler.go:63-80 + 137-210
    问题:recoverInterruptedTodos() 会把遗留 in_progress 标记为 failed(某些模式下),但 Run() 只把它们写入 result.Recovered,不会进入 result.Failed。如果上层依赖 Failed 做失败统计/告警,可能漏报。
    建议:恢复时若落入 failed 终态,同步写入 result.Failed(或在文档明确 Failed 不含 recovery-fail)。

  3. 中风险:subagent_progress 在 blocked/backoff 路径高频发射,存在事件风暴风险
    位置:internal/subagent/scheduler.go:261-279289-314
    问题:轮询期间,依赖未满足或 backoff 未到期的任务每轮都发 subagent_progress。任务量大、PollInterval 小时会显著放大事件流,影响 UI/日志吞吐。
    建议:对相同 task_id + reason + attempt 做节流或状态去重(仅状态变化时发)。

可直接给 AI 的 Review 提示词

你是 Go 代码审查助手,请严格按“只找问题不表扬”的方式审查 PR #346(分支对比:origin/main...HEAD)。

仓库背景:NeoCode,核心边界是 TUI -> Runtime -> Provider/Tools;本次 PR 主要改动在 subagent scheduler 的取消/重试/恢复与标准事件流。

请重点检查以下高风险维度,并按严重级别输出(High/Medium/Low):
1) 并发与幂等:
   - stale attempt 结果回写是否会污染当前 running 状态
   - state.running / outcomes / revision conflict 处理是否可能导致任务“失踪”或重复结算
2) 失败与恢复语义:
   - continue_on_error / fail_fast 分支是否符合定义
   - recovery(retry/fail) 是否与 retryLimit、RetryCount、终态统计一致
3) 取消与超时语义:
   - context.Canceled 与 context.DeadlineExceeded 是否被正确区分并映射到 stop_reason
4) 事件流一致性:
   - subagent_started/progress/retried/completed/failed/canceled 的触发条件、顺序、去重
   - 是否存在高频噪声事件或漏事件
5) 汇总正确性:
   - ScheduleResult 的 Succeeded/Failed/Retried/Recovered/BlockedLeft 是否与真实状态一致
6) 测试充分性:
   - 是否覆盖 stale attempt 并发场景、fail-fast 中在跑任务的取消、recovery 后统计一致性、事件顺序稳定性

输出格式要求:
- 先给 Findings(按严重级别排序)
- 每条包含:标题 / 文件:行号 / 触发条件 / 实际影响 / 修复建议
- 如果无问题,明确写“未发现阻断性问题”,并给出剩余测试风险点

补充:本次仅做分析与审查建议,未修改代码。

View job run

@codecov

codecov Bot commented Apr 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.71074% with 37 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/subagent/scheduler.go 85.63% 16 Missing and 9 partials ⚠️
internal/subagent/scheduler_types.go 72.97% 5 Missing and 5 partials ⚠️
internal/subagent/scheduler_executor.go 84.61% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@Cai-Tang-www

Copy link
Copy Markdown
Collaborator Author

@xgopilot 你是 Go 代码审查助手,请严格按“只找问题不表扬”的方式审查 PR #346(分支对比:origin/main...HEAD)。

仓库背景:NeoCode,核心边界是 TUI -> Runtime -> Provider/Tools;本次 PR 主要改动在 subagent scheduler 的取消/重试/恢复与标准事件流。

请重点检查以下高风险维度,并按严重级别输出(High/Medium/Low):

  1. 并发与幂等:
    • stale attempt 结果回写是否会污染当前 running 状态
    • state.running / outcomes / revision conflict 处理是否可能导致任务“失踪”或重复结算
  2. 失败与恢复语义:
    • continue_on_error / fail_fast 分支是否符合定义
    • recovery(retry/fail) 是否与 retryLimit、RetryCount、终态统计一致
  3. 取消与超时语义:
    • context.Canceled 与 context.DeadlineExceeded 是否被正确区分并映射到 stop_reason
  4. 事件流一致性:
    • subagent_started/progress/retried/completed/failed/canceled 的触发条件、顺序、去重
    • 是否存在高频噪声事件或漏事件
  5. 汇总正确性:
    • ScheduleResult 的 Succeeded/Failed/Retried/Recovered/BlockedLeft 是否与真实状态一致
  6. 测试充分性:
    • 是否覆盖 stale attempt 并发场景、fail-fast 中在跑任务的取消、recovery 后统计一致性、事件顺序稳定性

输出格式要求:

  • 先给 Findings(按严重级别排序)
  • 每条包含:标题 / 文件:行号 / 触发条件 / 实际影响 / 修复建议
  • 如果无问题,明确写“未发现阻断性问题”,并给出剩余测试风险点

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Findings

High

  1. stale attempt 回写会错误移除当前 running,导致任务“失踪”
    • 文件: internal/subagent/scheduler.go:480-489
    • 触发条件: 任务进入重试后,旧 attempt 的 outcome 晚到(outcome.attempt != running.attempt)。
    • 实际影响: 代码在 attempt 校验前先 delete(state.running, outcome.id),会把新 attempt 的 running 记录删掉;后续真实 outcome 可能走 !ok 被忽略,造成任务不再被跟踪、终态/统计不一致。
    • 修复建议: 先校验 attempt 一致性,再删除 state.running;仅对匹配的当前 attempt 执行回写和移除。

Medium

  1. Recovery=fail 的失败任务未计入 ScheduleResult.Failed,汇总失真
    • 文件: internal/subagent/scheduler.go:63-80, internal/subagent/scheduler.go:137-210, internal/subagent/scheduler.go:612-633
    • 触发条件: 启动恢复阶段把遗留 in_progress 直接落到 failedRecoveryMode=fail 或恢复时超出重试上限)。
    • 实际影响: Todo 真实状态为 failed,但结果仅进入 RecoveredFailed 不包含该任务;上层若依赖 Failed 做失败统计/告警会漏报。
    • 修复建议: 恢复阶段返回结构化结果(如 recoveredFailed),在 Run() 中合并到 result.Failed;或明确文档约定 Failed 不含 recovery-fail(但当前 PR 目标更偏向“统计与真实状态一致”)。

Low

  1. subagent_progress 在 blocked/backoff 轮询路径无去重,易产生事件噪声

    • 文件: internal/subagent/scheduler.go:261-278, internal/subagent/scheduler.go:299-313
    • 触发条件: 大量任务处于依赖未满足或重试退避窗口,且 PollInterval 较小时。
    • 实际影响: 每轮都会重复发相同 reason 的 progress 事件,UI/日志端会承受高频冗余流量,掩盖关键终态事件。
    • 修复建议: 按 task_id + attempt + reason 做状态变更去重或最小间隔节流,仅在状态变化时发事件。
  2. stale attempt 并发场景测试断言不充分,未覆盖 running 保活与后续结算

    • 文件: internal/subagent/scheduler_test.go:935-954
    • 触发条件: 当前 TestSchedulerHandleOneOutcomeIgnoresStaleAttempt 仅断言 Todo 状态未变。
    • 实际影响: 无法捕获“stale outcome 导致 state.running 被误删”的核心并发缺陷,回归风险高。
    • 修复建议: 在该用例补充断言:state.running[task] 仍存在且 attempt 不变,并追加一次新 attempt outcome 验证可正常结算。

测试风险点

  • 尚未看到针对“fail-fast 触发后,其他 in-flight 任务 outcome 晚到”的确定性顺序测试(当前主要校验终态,不校验晚到 outcome 幂等行为)。
  • 事件顺序测试目前只覆盖 started/retried/completed 子集,未约束 progress/failed/canceled 的全链路顺序稳定性与去重语义。

View job run

@Cai-Tang-www

Copy link
Copy Markdown
Collaborator Author

@xgopilot 帮我解决上述的review问题

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

已按你提到的 review 点完成修复并推到当前 PR 分支(feat(subagent),commit: a80164d)。

修复内容

  • 修复 stale attempt 回写顺序问题:仅在 outcome.attempt 与当前 running attempt 匹配时才删除 state.running,避免在旧结果晚到时把新 attempt“删丢”。
  • 修复恢复失败统计失真:recoverInterruptedTodos 新增 failed 聚合,Run() 初始化时并入 ScheduleResult.Failed,保证 recovery 直接失败任务被统计。
  • 降低 progress 事件噪声:新增 progress 去重状态(按 task_id + attempt + reason),在 dependency/backoff/running 进度路径仅状态变化时发 subagent_progress

新增/增强测试

  • TestSchedulerRunRecoveryModeFail:补充断言 recovery-fail 会进入 result.Failed
  • TestSchedulerRunRecoveryExceedRetryLimitMarkedFailed:覆盖 recovery=retry 且超重试上限时进入 failed 汇总。
  • TestSchedulerRunProgressEventDeduplicatedForRetryBackoff:覆盖 retry_backoff 轮询下 progress 去重。
  • TestSchedulerHandleOneOutcomeIgnoresStaleAttempt:补充 running 保活断言,并验证后续最新 attempt 可正常结算完成。

本地验证

  • go test ./internal/subagent/... ./internal/runtime/... 通过
  • go test ./... 通过

同步信息:push 后已更新 fork 侧 PR 链接(https://github.com/Cai-Tang-www/neo-code/pull/13)。

View job run

- keep running state on stale outcomes
- include recovery-failed tasks in schedule summary
- dedupe repeated subagent progress events
- strengthen regression tests for stale/recovery/backoff

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
@pionxe pionxe merged commit 1009edc into 1024XEngineer:main Apr 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(subagent): 增加取消/重试/恢复机制与事件可观测性

3 participants