Skip to content

#1424 first slice: WorkflowChatSource.Direct() address-free#1433

Merged
loning merged 2 commits into
auto-refact-devfrom
refactor/issue1424-first
May 31, 2026
Merged

#1424 first slice: WorkflowChatSource.Direct() address-free#1433
loning merged 2 commits into
auto-refact-devfrom
refactor/issue1424-first

Conversation

@loning
Copy link
Copy Markdown
Contributor

@loning loning commented May 30, 2026

摘要

closes #1424(由 Phase 9 split first-slice 产生)

详见 GitHub issue body 完整方案 + 关联 audit + judge artifact。

Stacked-PR

base = auto-refact-dev

🤖 Auto-loop / codex-refactor-loop

⟦AI:AUTO-LOOP⟧

由 Phase 9 split first-slice 产生。详见 GitHub issue #1424 body。

⟦AI:AUTO-LOOP⟧
@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 30, 2026

📊 当前状态 — Phase 8 r1 reviewer 派出(❌ 不需要人介入)

维度
阶段 Phase 8 r1 reviewer
codex 任务 architect / tests / quality(各 60min)
是否需要人介入 ❌ 否

下一步自动会做:3 reviewer 全 approve + CI 绿 → auto-merge;reject → fix r2。

🤖 controller status banner

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 30, 2026

🤖 quality review: comment

TL;DR

  • 这是什么: PR 1433 的 readability / naming / simplicity / dead-code 角度 review。
  • 结论是什么: 代码改动聚焦且没有发现死代码、过度抽象或明显复杂度回归;仅 refactor self-doc 前缀没有按 cluster 约定格式写。
  • 需要 maintainer 做什么: 建议作者把 3 处 phase9/cluster-349 改成规定的 iterN/cluster-349 形式后即可通过 quality 角度。

详细说明

WorkflowChatRunModels.cs:111WorkflowRunActorResolver.cs:226ChatRunRequestNormalizer.cs:144Old pattern / New principle 内容本身清楚,能说明 Direct source address-free 的意图;问题是 checklist 明确要求 self-doc 使用 // Refactor (iterN/cluster-XXX): 形态。这里写成 phase9/cluster-349,和同文件已有 iter112/cluster-3iter165/cluster-007 风格不一致,会让后续审计脚本或人工扫描难以按统一格式识别。

其他 quality 项未发现问题:WorkflowChatSource.Direct() 收窄后调用点可读;新增 ThrowingWorkflowActorBindingReader 被测试直接使用;没有新增单实现抽象、三处以上重复片段或 unrelated cleanup。


📎 完整 codex 原始输出(存档备查)

pr: 1433
role: quality
verdict: comment

Verdict

Code is focused and readable, but the new refactor self-doc prefixes do not match the mandated iterN/cluster-XXX format.

Evidence

  • src/workflow/Aevatar.Workflow.Application.Abstractions/Runs/WorkflowChatRunModels.cs:111, src/workflow/Aevatar.Workflow.Application/Runs/WorkflowRunActorResolver.cs:226, and src/workflow/Aevatar.Workflow.Infrastructure/CapabilityApi/ChatRunRequestNormalizer.cs:144: the new comments have clear Old/New explanations, but use // Refactor (phase9/cluster-349): rather than the checklist-required // Refactor (iterN/cluster-XXX): shape. This is a small but objective readability / auditability inconsistency with nearby refactor comments.

What would change your verdict

Update those three comment prefixes to the exact mandated shape, for example // Refactor (iter349/cluster-349): if 349 is the correct iteration number.

REVIEW_DONE:1433:quality:comment

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 30, 2026

🤖 Architect review: approve, no blocking architecture regression

TL;DR


详细说明

我按 auto-refact-dev...refactor/issue1424-first 的三点 diff 复核了 5 个变更文件,并对指定反模式做了 diff grep。生产路径没有出现 actor.HandleEventAsync(SubscribeAsync<EventEnvelope>、内部 JSON 持久化、Task.Delay(GetAwaiter().GetResult()TypeUrl.Contains(...)、raw HttpClient[Skip]、外部 NyxID/chrono 依赖或 proto 字段变更。

架构上,这个 first slice 保持在 issue #1424 声明范围内:WorkflowChatSource.Direct() 不再携带 actorId 这个 actor 地址标识;actor-targeted workflow 仍走现有 DefinitionActor 路径。Dictionary<,> 的唯一 diff 命中是测试内 recording fake,不是中间层跨请求事实状态。


📎 完整 codex 原始输出(存档备查)
---
pr: 1433
role: architect
verdict: approve
---

## Verdict
approve: no architectural concerns; the PR removes the Direct-source actor-address carrier without adding a parallel path or new CLAUDE/AGENTS violation.

## Evidence
- `src/workflow/Aevatar.Workflow.Application.Abstractions/Runs/WorkflowChatRunModels.cs:75`: `WorkflowChatSource.ActorId` no longer returns an actor id for `Direct`, aligning with CLAUDE.md: "API 字段单一语义:一个字段只表达一个含义,禁止双重语义(如\"名称查找 + inline 内容\")。"
- `src/workflow/Aevatar.Workflow.Application.Abstractions/Runs/WorkflowChatRunModels.cs:111`: the changed `Direct()` factory has the required Old/New refactor comment and makes `Direct` address-free; this supports CLAUDE.md: "`actorId` 对调用方不透明:不得解析前缀/类型名/实现来源,不得把字面模式当业务判断条件。"
- `src/workflow/Aevatar.Workflow.Application/Runs/WorkflowRunActorResolver.cs:224`: `ResolveSourceActorId` only binds `DefinitionActor` and `InlineYamlBundle`, so `Direct` no longer performs actor binding lookup; this keeps query/command identity semantics separated under CLAUDE.md: "追踪标识与目标身份分离:`commandId/correlationId` 追踪请求,`actorId` 标识实体;禁止混用或假设一一对应。"
- `src/workflow/Aevatar.Workflow.Infrastructure/CapabilityApi/ChatRunRequestNormalizer.cs:170`: typed `source.kind=direct + actorId` is rejected instead of silently aliasing to actor-targeted execution, matching the issue #1424 first-slice scope and CLAUDE.md: "删除优先:空转发、重复抽象、无业务价值代码直接删除,不保留兼容空壳。"
- Scope honesty: the diff touches only the declared first-slice areas from issue #1424: workflow chat source model, run actor resolver, normalizer, and corresponding tests. No `.proto` or `docs/canon/*.md` changes are present.
- Anti-pattern scan: no production diff hits for `actor.HandleEventAsync(`, `SubscribeAsync<EventEnvelope>`, JSON state/payload serialization, `Task.Delay(`, `GetAwaiter().GetResult()`, `TypeUrl.Contains(...)`, raw `new HttpClient(`, disabled tests, `*WriteActor`/`*ReadActor`/`*Store` splits, or NyxID/chrono references. The only `Dictionary<,>` diff hit is a test-local fake in `WorkflowRunActorResolverTests`, not middle-layer cross-request fact state.

REVIEW_DONE:1433:architect:approve

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 30, 2026

🤖 tests reviewer:PR 1433 测试覆盖通过

TL;DR

  • 这是什么:我按 tests 角色检查了 PR 1433 的测试覆盖与测试质量。
  • 结论是什么:verdict 是 approve;新增 direct source address-free 行为有对应行为测试,未发现新增轮询等待、Skip 或弱断言。
  • 需要 maintainer 做什么:无需因测试质量阻塞;controller 可继续收集其他 reviewer 结论。

详细说明

本轮只看测试角度。WorkflowRunActorResolverTests.cs:97 覆盖了 Direct() 不再读取 source actor binding 的行为;WorkflowCapabilityEndpointsCoverageTests.cs:500 覆盖 typed direct source 携带 ActorId 时被拒绝;WorkflowCapabilityEndpointsCoverageTests.cs:472 同时把 direct alias 的成功路径改成 address-free 断言。两个相关测试过滤执行均通过:resolver 24 个测试通过,capability endpoint coverage 37 个测试通过。

我也检查了 touched tests 中的 Task.Delay / Thread.Sleep / WaitUntilAsync / [Skip] / manual trait / bump-only assertion 模式,未发现新增问题。IWorkflowActorBindingReader 是解析 source actor 绑定的读取接口,本 PR 用 throwing fake 确认 direct source 不再触发这条读取路径。


📎 完整 codex 原始输出(存档备查)
---
pr: 1433
role: tests
verdict: approve
---

## Verdict
Test coverage and quality are adequate for the direct-source address-free change.

## Evidence
- `test/Aevatar.Workflow.Application.Tests/WorkflowRunActorResolverTests.cs:97` adds `ResolveOrCreateAsync_ShouldNotReadSourceActorBinding_WhenSourceIsDirect`, which asserts the resolver still creates the default direct run and that the binding reader is never called.
- `test/Aevatar.Workflow.Host.Api.Tests/WorkflowCapabilityEndpointsCoverageTests.cs:500` adds `ChatRunRequestNormalizer_ShouldRejectDirectSourceActorId`, covering the new typed-direct branch that rejects an actor id instead of smuggling it into `Direct`.
- `test/Aevatar.Workflow.Host.Api.Tests/WorkflowCapabilityEndpointsCoverageTests.cs:472` updates the direct alias case to assert an address-free `Direct` source (`ActorId == null`) rather than preserving an actor id.
- Checked touched tests for `Task.Delay`, `Thread.Sleep`, `WaitUntilAsync`, `[Skip]`, manual traits, `Assert.True(true)`, and `NotThrow`; no new violations found.
- Verification: `dotnet test test/Aevatar.Workflow.Application.Tests/Aevatar.Workflow.Application.Tests.csproj --nologo --filter FullyQualifiedName~WorkflowRunActorResolverTests` passed: 24 passed, 0 failed, 0 skipped.
- Verification: `dotnet test test/Aevatar.Workflow.Host.Api.Tests/Aevatar.Workflow.Host.Api.Tests.csproj --nologo --filter FullyQualifiedName~WorkflowCapabilityEndpointsCoverageTests` passed: 37 passed, 0 failed, 0 skipped.

REVIEW_DONE:1433:tests:approve

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 30, 2026

✅ Phase 8 r1 共识达成

维度
PR #1433 #1424 first slice
Round r1
Reviewer 投票 architect: approve · tests: approve · quality: comment(2 approve + 1 comment)
共识规则 2 approve + 1 comment → auto-merge with surfaced comment
是否需要人介入 ❌ 否

下一步自动会做:CI 全绿 → auto-merge + close #1424

🤖 controller status banner

⟦AI:AUTO-LOOP⟧

@codecov
Copy link
Copy Markdown

codecov Bot commented May 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.71%. Comparing base (21a3951) to head (e5b06d9).
⚠️ Report is 1 commits behind head on auto-refact-dev.

@@               Coverage Diff                @@
##           auto-refact-dev    #1433   +/-   ##
================================================
  Coverage            83.70%   83.71%           
================================================
  Files                 1012     1012           
  Lines                68261    68257    -4     
  Branches              8832     8830    -2     
================================================
+ Hits                 57135    57138    +3     
+ Misses                7232     7228    -4     
+ Partials              3894     3891    -3     
Flag Coverage Δ
ci 83.71% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ication.Abstractions/Runs/WorkflowChatRunModels.cs 93.42% <100.00%> (-0.41%) ⬇️
...kflow.Application/Runs/WorkflowRunActorResolver.cs 91.42% <ø> (+0.37%) ⬆️
...tructure/CapabilityApi/ChatRunRequestNormalizer.cs 91.86% <100.00%> (+0.06%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 31, 2026

🤖 conflict-resolve 报告 — #1433

  • 已执行 git fetch origingit merge origin/auto-refact-dev
  • 冲突文件:src/workflow/Aevatar.Workflow.Infrastructure/CapabilityApi/ChatRunRequestNormalizer.cs
  • 解决方式:保留 #1424 first slice: WorkflowChatSource.Direct() address-free #1433 的 address-free WorkflowChatSource.Direct() 语义,采用 trunk 侧新增的 NormalizeActorId 命名/实现承接 actor id 归一化;direct source 携带 actor id 仍按无地址约束拒绝。
  • 自动合并文件:test/Aevatar.Workflow.Host.Api.Tests/WorkflowCapabilityEndpointsCoverageTests.cs,保留 trunk 同期覆盖用例。

验证:

  • dotnet build aevatar.slnx --nologo 2>&1 | tail -3:通过,0 个错误。
  • dotnet test aevatar.slnx --nologo --no-build 2>&1 | tail -30:通过,失败 0。

CONFLICT_RESOLVED:1433:resolved:1:tests-pass
⟦AI:AUTO-LOOP⟧

…ssue1424-first

# Conflicts:
#	src/workflow/Aevatar.Workflow.Infrastructure/CapabilityApi/ChatRunRequestNormalizer.cs
@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 31, 2026

🤖 Tests review: approve

TL;DR

  • 这是什么: PR 1433 的测试质量评审,只看覆盖、断言强度和测试稳定性。
  • 结论: approve,新增行为有对应测试,未发现 polling wait、skip/manual trait 或断言弱化。
  • 需要 maintainer 做什么: 无需因测试角度阻塞,可以继续合并前的其他角色检查。

详细说明

新增测试覆盖了本次核心行为:WorkflowRunActorResolverTests.ResolveOrCreateAsync_ShouldNotReadSourceActorBinding_WhenSourceIsDirect 验证 Direct source 不再读取 source actor binding,并断言实际创建的是默认 direct workflow;WorkflowCapabilityEndpointsCoverageTests.ChatRunRequestNormalizer_ShouldRejectDirectSourceActorId 验证 typed API 中 direct + ActorId 被拒绝,避免继续把地址塞进 direct source。

我同时检查了触达测试 diff,没有新增 Task.Delay / Thread.Sleep / WaitUntilAsync、没有新增 [Skip][Trait("Category","Manual")],也没有看到把强断言改松的情况。git diff --check origin/auto-refact-dev...origin/refactor/issue1424-first 通过。


📎 完整 codex 原始输出(存档备查)

pr: 1433
role: tests
verdict: approve

Verdict

Test coverage and quality are adequate for the direct-source address-free change.

Evidence

  • test/Aevatar.Workflow.Application.Tests/WorkflowRunActorResolverTests.cs:97 adds ResolveOrCreateAsync_ShouldNotReadSourceActorBinding_WhenSourceIsDirect, which exercises resolver behavior and asserts no source binding lookup plus successful default workflow run creation.
  • test/Aevatar.Workflow.Host.Api.Tests/WorkflowCapabilityEndpointsCoverageTests.cs:560 adds ChatRunRequestNormalizer_ShouldRejectDirectSourceActorId, which covers the new typed normalizer rejection for direct sources carrying an actor id.
  • Diff scan found no added Task.Delay, Thread.Sleep, WaitUntilAsync, [Skip], or manual-category traits in the touched test files; no assertion weakening was observed.

REVIEW_DONE:1433:tests:approve

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 31, 2026

🤖 architect review: comment,refactor 注释前缀需统一

TL;DR

  • 这是什么: PR 1433 conflict-resolve 后的 architecture compliance 复审。
  • 结论是什么: comment;没有发现新的 CLAUDE/AGENTS 架构违规,但 3 处新 refactor self-doc 没按 checklist 要求使用 iterN/cluster-XXX 前缀。
  • 需要 maintainer 做什么: 建议作者把 phase9/cluster-349 改成 iterN/cluster-349;controller 可按 comment 处理,不是架构阻塞。

详细说明

我按最新 origin/auto-refact-dev...origin/refactor/issue1424-first 三点 diff 复查了 5 个变更文件。生产代码没有新增 actor.HandleEventAsync(SubscribeAsync<EventEnvelope>、内部 JSON 持久化、Task.Delay(GetAwaiter().GetResult()TypeUrl.Contains(...)、raw HttpClient[Skip]、NyxID/chrono 外部依赖、proto 字段变更,唯一 Dictionary<,> 命中在测试 fake 内。

需要 surfacing 的点是 reviewer checklist 明确要求每个 refactored type/method 使用 // Refactor (iterN/cluster-XXX): Old pattern: … New principle: …。当前 3 处内容本身清楚,但前缀是 phase9/cluster-349,会降低后续审计按统一格式扫描的确定性。


📎 完整 codex 原始输出(存档备查)
---
pr: 1433
role: architect
verdict: comment
---

## Verdict
comment-only: no blocking architecture regression, but the new refactor self-doc comments do not use the checklist-required `iterN/cluster-XXX` prefix.

## Evidence
- `src/workflow/Aevatar.Workflow.Application.Abstractions/Runs/WorkflowChatRunModels.cs:111`, `src/workflow/Aevatar.Workflow.Application/Runs/WorkflowRunActorResolver.cs:226`, and `src/workflow/Aevatar.Workflow.Infrastructure/CapabilityApi/ChatRunRequestNormalizer.cs:134`: each new Old/New refactor comment is meaningful, but the prefix is `// Refactor (phase9/cluster-349):` instead of the role checklist's required `// Refactor (iterN/cluster-XXX): Old pattern: …  New principle: …`. This is a review checklist miss, not a CLAUDE/AGENTS architecture violation.
- `src/workflow/Aevatar.Workflow.Application.Abstractions/Runs/WorkflowChatRunModels.cs:75` and `src/workflow/Aevatar.Workflow.Infrastructure/CapabilityApi/ChatRunRequestNormalizer.cs:160`: the net behavior makes `Direct` address-free and rejects typed direct actor ids, which aligns with CLAUDE.md: "API 字段单一语义:一个字段只表达一个含义,禁止双重语义(如"名称查找 + inline 内容")。"
- Scope honesty: current diff is limited to workflow chat source normalization/resolution plus corresponding tests; no `.proto` or `docs/canon/*.md` changes are present, and no NyxID / chrono-* references are introduced.
- Anti-pattern scan: no production diff hits for `actor.HandleEventAsync(`, `SubscribeAsync<EventEnvelope>`, JSON state/payload serialization, `Task.Delay(`, `GetAwaiter().GetResult()`, `TypeUrl.Contains(...)`, raw `new HttpClient(`, disabled tests, `*WriteActor`/`*ReadActor`/`*Store` splits, or NyxID/chrono references. The only `Dictionary<,>` diff hit is a test-local fake in `WorkflowRunActorResolverTests`, not middle-layer cross-request fact state.

## What would change your verdict
Update the three new refactor comment prefixes from `phase9/cluster-349` to the mandated `iterN/cluster-349` form, preserving their existing Old pattern / New principle content.

REVIEW_DONE:1433:architect:comment
⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 31, 2026

🤖 quality review:approve

TL;DR

  • 这是什么: PR 1433 的 code quality 视角复核,重点看命名、死代码、复杂度、注释和 scope。
  • 结论: approve,改动集中在让 WorkflowChatSource.Direct() 不再携带 actor 地址,读起来更单一。
  • 需要 maintainer 做什么: 不需要额外处理;可继续后续 review/merge 流程。

详细说明

本轮按三点检查:先确认 PR base/head 是 auto-refact-dev...refactor/issue1424-first,再打开 5 个 touched file 的完整内容,最后做 repo-wide grep 确认没有残留 WorkflowChatSource.Direct(actorId) 调用形态。

质量结论是 approve:新增 self-doc comment 都有 Old pattern / New principle,并且能直接说明为什么 Direct 应该 address-free;新增测试 helper ThrowingWorkflowActorBindingReader 只服务一个明确断言,没有扩散成新的抽象;diff 只有 66 行新增、10 行删除,没有看到 drive-by cleanup 或复杂度上升。


📎 完整 codex 原始输出(存档备查)
---
pr: 1433
role: quality
verdict: approve
---

## Verdict
Approve: the change is focused, names keep the source semantics clear, no dead code or over-engineering was introduced, and the required refactor self-docs are readable.

## Evidence
- src/workflow/Aevatar.Workflow.Application.Abstractions/Runs/WorkflowChatRunModels.cs:111 documents the old address-smuggling pattern and replaces it with an address-free `Direct()` factory, which makes the public source variant single-purpose.
- src/workflow/Aevatar.Workflow.Application/Runs/WorkflowRunActorResolver.cs:224 keeps actor-id lookup restricted to actor-addressed variants; the method stays small and branch count remains straightforward.
- src/workflow/Aevatar.Workflow.Infrastructure/CapabilityApi/ChatRunRequestNormalizer.cs:132 rejects typed `direct` sources carrying an actor id instead of silently reusing another source shape, avoiding ambiguous field semantics.
- test/Aevatar.Workflow.Application.Tests/WorkflowRunActorResolverTests.cs:97 adds a focused regression test with a reachable private test helper at line 718; no unused helper or production-only abstraction is added.
- test/Aevatar.Workflow.Host.Api.Tests/WorkflowCapabilityEndpointsCoverageTests.cs:526 keeps the alias matrix consistent and adds a narrow direct+actor rejection case at line 558.

REVIEW_DONE:1433:quality:approve

⟦AI:AUTO-LOOP⟧

@loning loning merged commit b10efb0 into auto-refact-dev May 31, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant