Make Studio team command ACK responses honest#765
Conversation
Return accepted receipts for team update/archive so write responses no longer imply readmodel freshness, and document the new contract for clients. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Clarify accepted-command UI messaging and reject unexpected ACK stages so the frontend does not imply read-model freshness beyond the backend contract. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…o-team-command-ack
louis4li
requested review from
AbigailDeng,
eanzhao,
jason-aelf and
potter-sun
as code owners
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## dev #765 +/- ##
=======================================
Coverage 82.47% 82.48%
=======================================
Files 941 941
Lines 60101 60114 +13
Branches 7872 7872
=======================================
+ Hits 49571 49586 +15
Misses 7132 7132
+ Partials 3398 3396 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
🤖 Phase 10 主动 advisory review 派出(非 auto-merge gate)
性质说明:本仓 auto-loop 自动扫开 PR 池主动派 advisory review,不会 auto-merge 本 PR,verdict 仅供 maintainer 参考。可忽略 / 部分采纳 / 全采纳。 🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
🤖 Phase 10 advisory review — qualityTL;DR
详细说明我按要求用 阻塞点是本 review checklist 明确要求 refactor self-doc: 这是 auto-loop 主动 advisory review(非 auto-merge gate)。verdict 仅参考;PR author / maintainer 可忽略 / 部分采纳 / 全采纳 — controller 不会 auto-merge 本 PR,maintainer 控制 merge 决策。 📎 完整 codex 原始输出(存档备查)---
pr: 765
role: quality
verdict: reject
---
## Verdict
Reject from the quality checklist because the major refactor has no mandated `// Refactor (iterN/cluster-XXX):` Old/New self-doc block in the touched PR-head files.
## Evidence
- `src/Aevatar.Studio.Application/Studio/Contracts/TeamContracts.cs:30` introduces the new `StudioTeamCommandAcceptedResponse` public contract, but the touched file has no `// Refactor (iterN/cluster-XXX):` marker or Old/New block explaining the response-shape shift.
- `src/Aevatar.Studio.Application/Studio/Services/StudioTeamService.cs:59` and `src/Aevatar.Studio.Application/Studio/Services/StudioTeamService.cs:94` change update/archive from "dispatch then reread readmodel" to "return accepted receipt", but the PR-head file has no mandated self-doc block at the behavior change site.
- `src/Aevatar.Studio.Hosting/Endpoints/StudioTeamEndpoints.cs:191` and `src/Aevatar.Studio.Hosting/Endpoints/StudioTeamEndpoints.cs:220` change PATCH/archive success responses to `202 Accepted`, but the PR-head file has no `Refactor` Old/New block.
- Exact check run: `git grep -n "Refactor (iter\\|Old:\\|New:" origin/design/issue-547-studio-team-command-ack -- <touched files>` returned no matches. I did not find dead code, harmful single-implementer abstraction, large new methods, or unrelated drive-by cleanup in the implementation pass.
## What would change your verdict (only if comment or reject)
Add a clear self-doc block near the main contract/behavior change, for example `// Refactor (iter96/cluster-547):` with short `Old:` and `New:` lines explaining that update/archive used to dispatch then reread an eventually consistent readmodel, and now return an honest accepted command receipt without post-state. One block in the central service plus endpoint/contract notes if desired would satisfy the quality checklist.
REVIEW_DONE:765:quality:reject
⟦AI:AUTO-LOOP⟧REVIEW_DONE:765:quality-advisory:reject ⟦AI:AUTO-LOOP⟧ |
🤖 Phase 10 advisory review — architectTL;DR
详细说明我按 阻塞点只有文档治理：PR 新增 📎 完整 codex 原始输出(存档备查)---
pr: 765
role: architect
verdict: reject
---
## Verdict
Reject: the implementation aligns with command ACK/readmodel separation, but the PR introduces a new `docs/history/` document that does not explicitly mark itself non-authoritative.
## Evidence
- `docs/history/2026-05/2026-05-19-issue-547-studio-team-command-ack-design.md:7`: the body starts directly with the design content and never states that the archived document is non-authoritative. This violates `CLAUDE.md` engineering docs rule: "`docs/history/` 仅放归档快照，正文必须明确非权威，不得被实现或测试当作规范来源。"
## What would change your verdict
Add an explicit non-authoritative archive notice to the body of `docs/history/2026-05/2026-05-19-issue-547-studio-team-command-ack-design.md`, near the top after the H1/frontmatter, so readers and future agents cannot treat it as the active specification. Keep ADR-0024 as the authoritative decision record for the ACK semantics.
REVIEW_DONE:765:architect-advisory:reject
⟦AI:AUTO-LOOP⟧这是 auto-loop 主动 advisory review(非 auto-merge gate)。verdict 仅参考;PR author / maintainer 可忽略 / 部分采纳 / 全采纳 — controller 不会 auto-merge 本 PR,maintainer 控制 merge 决策。 REVIEW_DONE:765:architect-advisory:reject |
📊 codex 进展 review-pr765-architect-advisory (⏳ 进行中; 已跑 12 min)
|
📊 codex 进展 review-pr765-quality-advisory (⏳ 进行中; 已跑 11 min)
|
🤖 Phase 10 advisory review — testsTL;DR
详细说明我按三点 diff 检查了本次 touched 的生产文件和对应测试文件。核心语义是 Studio Team 的 update/archive 只返回 轮询稳定性也检查过：diff 没有新增 📎 完整 codex 原始输出(存档备查)---
pr: 765
role: tests
verdict: approve
---
## Verdict
Test coverage and quality are adequate for the ACK semantics diff.
## Evidence
- `test/Aevatar.Studio.Tests/StudioTeamServiceTests.cs:111` and `:125` assert update/archive return the command-port receipt while a throwing query port would fail any post-dispatch readmodel reread.
- `test/Aevatar.Studio.Tests/ActorDispatchStudioTeamCommandServiceTests.cs:67`, `:107`, and `:151` cover update/archive dispatch receipts, envelope command IDs/timestamps, and the no-op update branch with no dispatch and null `CommandId`.
- `test/Aevatar.Studio.Tests/StudioTeamEndpointTests.cs:95`, `:244`, `:263`, and `:287` cover `202 Accepted`, `Location`, no-op ACK, escaped receipt location, and receipt bodies that do not contain post-state fields.
- `apps/aevatar-console-web/src/shared/studio/api.test.ts:1219` and `:1362` assert update/archive decode accepted receipts and reject unknown ACK stages; `apps/aevatar-console-web/src/pages/teams/detail.test.tsx:1170` and `:1231` assert accepted user messages and refetch behavior.
- No added `Task.Delay`, `Thread.Sleep`, `[Skip]`, manual trait, or bump-only assertion was found in the diff. `bash tools/ci/test_stability_guards.sh` passed. `dotnet test test/Aevatar.Studio.Tests/Aevatar.Studio.Tests.csproj --nologo --filter "FullyQualifiedName~StudioTeam"` passed with 122 passed, 0 failed, 0 skipped.
- Frontend targeted tests were not run locally because dependencies are unavailable in this checkout: bare `pnpm` was missing, and `corepack pnpm --dir apps/aevatar-console-web test -- --runInBand src/shared/studio/api.test.ts src/pages/teams/detail.test.tsx` failed with `jest: command not found`.
## What would change your verdict (only if comment or reject)
N/A.
REVIEW_DONE:765:tests:approve这是 auto-loop 主动 advisory review(非 auto-merge gate)。verdict 仅参考;PR author / maintainer 可忽略 / 部分采纳 / 全采纳 — controller 不会 auto-merge 本 PR,maintainer 控制 merge 决策。 REVIEW_DONE:765:tests-advisory:approve |
🤖 Phase 10 advisory review 完成(汇总)
整体:2/3 reject,1 approve(tests)。 性质:这是 auto-loop 主动 advisory review(非 auto-merge gate)。controller 不会 派 fix codex 改本 PR,不会 auto-merge,不 escalate。@louis4li 可:
advisory review 给的 verdict 不进入 Phase 8 fix-merge loop;maintainer 完全控制 merge 决策。 📢 cc @louis4li(PR author) 🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
2 participants
Summary
202 Acceptedcommand receipts instead of post-state snapshots.Test plan
npm --prefix /Users/liyingpei/Desktop/Code/aevatar/apps/aevatar-console-web run tscnpm --prefix /Users/liyingpei/Desktop/Code/aevatar/apps/aevatar-console-web run test -- src/shared/studio/api.test.ts src/pages/teams/detail.test.tsx --runInBandCloses #547
🤖 Generated with Claude Code