Skip to content

github-devloop implementation for #2742 - #2746

Open
potter-sun wants to merge 2 commits into
devfrom
devloop/issue/aevatarAI/aevatar/2742/ready-consensus-github-devloop-issue-aevatarAI-aevatar-2742-2026-07-14T09-56-17Z-0252755960
Open

github-devloop implementation for #2742#2746
potter-sun wants to merge 2 commits into
devfrom
devloop/issue/aevatarAI/aevatar/2742/ready-consensus-github-devloop-issue-aevatarAI-aevatar-2742-2026-07-14T09-56-17Z-0252755960

Conversation

@potter-sun

Copy link
Copy Markdown
Contributor

github-devloop implementation PR for issue #2742

@potter-sun
potter-sun requested a review from AbigailDeng as a code owner July 14, 2026 10:44
@potter-sun

Copy link
Copy Markdown
Contributor Author

github-devloop PR child open

@potter-sun

Copy link
Copy Markdown
Contributor Author

github-devloop PR is ready for review

@potter-sun potter-sun added the fkst-dev:reviewing pr-review-consensus-in-progress label Jul 14, 2026
@potter-sun

Copy link
Copy Markdown
Contributor Author

github-devloop PR is ready for review

@potter-sun

Copy link
Copy Markdown
Contributor Author

github-devloop PR review decision: reject
Three-angle verdicts: teleology=reject parsimony=comment fidelity=comment
Blocking gap: skipped-purpose: critical zh-CN consent/review copy is untranslated English despite the stated i18n consent requirement, evidenced at diff.patch lines 103-117, 125-158, and 174-186.

teleology:
The implementation is structurally close, but merge-ready consent UI requires clear localized consent text; adding English consent copy into zh-CN.ts makes the core Agent Key authorization step formally present but not faithfully informed for that locale.

parsimony:
The PR can advance with comments: it preserves the server-authoritative credential model, blocks creation until explicit Agent Key consent, avoids generic /api/schedules, and verifies no secret markers in payload/cache/DOM. The parsimony cleanup is to localize the zh-CN additions and remove or test-scope unused fixture constants, not to block the consent/mock contract.

fidelity:
The proposal is merge-directionally sound for the stated scope: it avoids generic /api/schedules, adds explicit review/consent gating, keeps create tied to review digest/version, and tests non-leakage. I would not reject it, but before calling it fully polished, tighten the fidelity edges: localize the new zh-CN consent strings and render/assert returned credential-plan facts instead of relying on static safety copy.

⟦AI:FKST⟧

@potter-sun potter-sun added fkst-dev:fixing review-rejected-and-fix-in-progress and removed fkst-dev:reviewing pr-review-consensus-in-progress labels Jul 14, 2026
@potter-sun

Copy link
Copy Markdown
Contributor Author

github-devloop merge gate failed: skipped-purpose: critical zh-CN consent/review copy is untranslated English despite the stated i18n consent requirement, evidenced at diff.patch lines 103-117, 125-158, and 174-186.
Reproduce locally with pnpm --dir apps/aevatar-console-web tsc && pnpm --dir apps/aevatar-console-web test:ui && pnpm --dir apps/aevatar-console-web build from the repository root.

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.45%. Comparing base (b1c2534) to head (8aedd85).

@@            Coverage Diff             @@
##              dev    #2746      +/-   ##
==========================================
- Coverage   83.45%   83.45%   -0.01%     
==========================================
  Files        1376     1376              
  Lines      112732   112732              
  Branches    14366    14366              
==========================================
- Hits        94084    94081       -3     
- Misses      12096    12098       +2     
- Partials     6552     6553       +1     
Flag Coverage Δ
ci 83.45% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 3 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.

@eanz17

eanz17 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🤖 测试审查拒绝:3 条新增权限控制路径缺少行为覆盖

TL;DR

  • 这是什么:从测试覆盖与测试质量角度审查 PR 2746 的 Team Automation 创建流程。
  • 结论是什么:现有成功流测试质量合格,但 plan-changed、preflight 失败恢复和真实 consent 拒绝逻辑均未覆盖,因此 verdict 为 reject。
  • 需要 maintainer 做什么:补齐这 3 类行为测试并验证通过后再进入合并判断。

必须补齐的测试

teamAutomationApi 是 Team Automation 创建前权限预检与提交的前端接口模块。页面测试在 detail.test.tsx:745 整体 mock 了该模块,因此无法执行 teamAutomationApi.ts:121 中“未同意 consent 必须拒绝”的真实分支。

同时,TeamAutomationsTab.tsx:1194 的预检失败状态和 TeamAutomationsTab.tsx:1215plan-changed 状态都是新增、可见且影响授权控制流的业务分支,但当前测试没有让预检请求 reject,也没有返回 status: "plan-changed"

需要增加以下行为验证:

  • plan-changed 时禁止创建,刷新 review 后才允许重新 consent 并提交。
  • preflight 失败时展示错误、保留草稿,并可重试恢复。
  • 直接测试真实 teamAutomationApiaccepted: false 必须拒绝,同时覆盖权限 review 的 trim/fallback 结果。

已确认新增页面测试对已覆盖路径有实际业务断言,包括按钮 consent 门禁、身份隔离、secret 不进入 payload/cache/DOM、关闭后保留草稿以及 create 失败反馈;未发现新增 skip/manual、弱化断言或测试侧 sleep/delay。

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

pr: 2746
role: tests
head_sha: af8549e
verdict: reject

Verdict

Reject because net-new permission-review control-flow branches and the real consent-enforcement API logic are not exercised by tests.

Evidence

  • apps/aevatar-console-web/src/pages/teams/tabs/TeamAutomationsTab.tsx:1194 and :1215 add distinct preflight-error and plan-changed outcomes, including error rendering/retry and blocking creation until a refreshed review, but apps/aevatar-console-web/src/pages/teams/detail.test.tsx has no test that makes teamAutomationApi.preflightCreate reject or return status: "plan-changed". These are user-visible, security-sensitive branches in the new creation flow.
  • apps/aevatar-console-web/src/shared/api/teamAutomationApi.ts:76 and :121 add executable production logic, including fallback/normalization behavior and a mandatory rejection when consent.accepted is false. The page suite mocks the entire module at apps/aevatar-console-web/src/pages/teams/detail.test.tsx:745, and there is no matching API unit test, so the real implementation and its consent guard receive zero coverage.
  • The added page tests do assert business outcomes for the covered paths: consent gates the create button, the submitted identities remain distinct, secrets stay out of payload/cache/DOM, closing preserves the draft, and create failures remain in-flow. No new skip/manual marker, assertion weakening, or test-side sleep/delay was found.

What would change your verdict

Add behavior tests that (1) return plan-changed, assert creation remains blocked, refresh the review, and only then permit consent/create; (2) reject preflight, assert the error state and preserved draft, then verify retry recovery; and (3) exercise the real teamAutomationApi implementation, especially rejection of accepted: false plus permission-review normalization/fallback behavior.

⟦AI:AUTO-LOOP⟧
REVIEW_DONE:2746:tests:reject

⟦AI:AUTO-LOOP⟧

@eanz17

eanz17 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🤖 质量审查结论:reject,需先收敛可读性与无效 surface

TL;DR

  • 这是什么:PR github-devloop implementation for #2742 #2746 的可读性、命名、简洁性与 dead code 审查。
  • 结论是什么:consent 流程范围正确,但发现 4 项合并前必须修复的质量问题。
  • controller 下一步:进入 fix 轮,完成中文文案、删除无效 surface,并拆分 136 行提交回调后复审。

必须修复的问题

  1. zh-CN.ts:755 起新增的多数 consent 文案仍是英文,且中文 locale 测试直接断言这些英文文本。这会让中文界面出现中英混排,并把临时文案固化成契约。
  2. teamAutomationApi.ts:64 的 mock fixture ID 集合中,memberIdworkflowIdcredentialFixtureId 没有任何调用者。mock fixture ID 是测试用的固定身份值;无调用者的导出字段属于 dead public surface。
  3. TeamAutomationsTab.tsx:1188 的 permission-review mutation(负责准备权限审阅的异步操作)接收 member,但执行函数完全不读取它;这是无效参数。
  4. TeamAutomationsTab.tsx:1499saveAutomation 已达 136 行,同时承担校验、旧编辑 DTO 构造、创建 draft、preflight、consent 与 create 状态转换。应抽取校验/draft 构造,并将 edit 与 create 分支拆开,让主回调只负责分派。

变更本身仍局限于 issue #2742 的五个前端/mock/test 文件;HOST_REFACTOR_COMMENT_POLICY=none,也未新增 Refactor/Old/New/iteration 源码注释。

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

pr: 2746
role: quality
head_sha: af8549e
verdict: reject

Verdict

Reject: the consent flow is in scope, but it introduces untranslated Chinese UI, dead mock surface, an unused mutation parameter, and an oversized save callback that should be simplified before merge.

Evidence

  • apps/aevatar-console-web/src/locales/zh-CN.ts:755: most of the 39 new zh-CN messages are English (agentKeyConsent, close, review labels, grant labels, errors, and success text), and apps/aevatar-console-web/src/pages/teams/detail.test.tsx:2811 onward asserts those English strings while rendering the Chinese locale. This makes the Chinese consent flow internally inconsistent and cements the accidental fallback as expected behavior.
  • apps/aevatar-console-web/src/shared/api/teamAutomationApi.ts:64: teamAutomationMockFixtureIds.memberId, .workflowId, and .credentialFixtureId have no production or test caller; only .publishedServiceId and .scheduleId are read. Exporting unused identity fixtures adds dead public surface to the temporary contract.
  • apps/aevatar-console-web/src/pages/teams/tabs/TeamAutomationsTab.tsx:1188: permissionReviewMutation accepts a member variable but its mutationFn reads only draft; the extra parameter is passed at line 1599 and has no observable purpose.
  • apps/aevatar-console-web/src/pages/teams/tabs/TeamAutomationsTab.tsx:1499: saveAutomation now spans 136 lines and combines form validation, legacy schedule-update DTO construction, Team Automation draft construction, preflight transitions, consent enforcement, and creation. The new state-machine work pushed this callback well beyond the 80-line readability guideline and makes the edit/create paths harder to reason about independently.
  • The five-file diff remains within issue Frontend: Team Automations AgentKey consent UI 与 mock contract(可先做) #2742's declared frontend/mock/test scope, and the effective HOST_REFACTOR_COMMENT_POLICY is none; no new Refactor/Old/New/iteration source comments were introduced.

What would change your verdict (only if comment or reject)

Translate the new zh-CN values and update locale-sensitive assertions; delete the unused fixture IDs and the unused member mutation variable; extract validation/draft construction and separate the edit submission from the create-state transition so saveAutomation becomes a short dispatcher.

⟦AI:AUTO-LOOP⟧
REVIEW_DONE:2746:quality:reject

⟦AI:AUTO-LOOP⟧

@eanz17

eanz17 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🤖 架构审查结论:拒绝,mock 创建路径伪造了已接受回执

TL;DR

  • 这是什么:PR 2746 的独立架构合规审查。
  • 结论是什么:阻塞合并;生产前端未发送命令却返回固定 accepted 回执,并把固定 schedule 写入读取界面。
  • controller 下一步:将 fixture 限定为测试/开发注入;真实后端未接通时,生产流程只能停在明确的非写入预览态。

阻塞问题

ACK 是系统对命令已到达某个阶段的回执。当前 teamAutomationApi.create 没有调用任何后端,却返回固定的 scheduleIdcommandIdcorrelationIdackStage: "accepted";随后界面把该 fixture 当作待同步 schedule 加入列表。这意味着浏览器进程内的 mock 状态冒充了后端已接受事实。

这违反两条 CLAUDE.md 强制约束:

  • apps/aevatar-console-web/src/shared/api/teamAutomationApi.ts:121:违反“ACK 诚实:同步返回只承诺已达到阶段(默认 accepted + stable command id);committed/read-model observed 等强保证须通过独立契约或异步观察获取。”
  • apps/aevatar-console-web/src/pages/teams/tabs/TeamAutomationsTab.tsx:1260:违反“事实源唯一:跨请求/跨节点一致性事实必须有唯一权威来源(Actor 持久态或分布式状态),不依赖进程内偶然状态。”这里固定 sch-alpha 不可能被权威后端观察并收敛。

修复要求:保留 typed draft/review/receipt contract,但将 mock 实现放到显式测试/开发注入或默认关闭的 feature boundary。真实 Team Automation command endpoint 可用前,生产 UI 不得显示“已接受”、合成 pending schedule 或触发 schedule read-model 刷新;consent 流程可以停在清楚标注的非写入预览态。

补充核查:PR 未新增 Old/New 历史注释、schema/protocol、actor 技术拆分、外部仓库依赖或 .refactor-loop 生产事实;fast-gatesfkst-host-policyconsole-web CI 均通过,但这些结果不覆盖上述语义违规。


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

pr: 2746
role: architect
head_sha: af8549e
verdict: reject

Verdict

Reject: the production Team Automation client fabricates an accepted command receipt without dispatching a command, then projects that fixture into the schedule read surface as if backend acceptance had occurred.

Evidence

  • apps/aevatar-console-web/src/shared/api/teamAutomationApi.ts:121: createTeamAutomation is the production implementation exported by teamAutomationApi, but it performs no transport call and unconditionally returns fixed scheduleId, commandId, correlationId, ackedAt, and ackStage: "accepted" values. This introduces an ACK that has not reached the stage it claims, violating CLAUDE.md: “ACK 诚实:同步返回只承诺已达到阶段(默认 accepted + stable command id);committed/read-model observed 等强保证须通过独立契约或异步观察获取。”
  • apps/aevatar-console-web/src/pages/teams/tabs/TeamAutomationsTab.tsx:1260: the mock receipt is treated as a successful accepted request, passed to showCreatedScheduleFeedback, and inserted into pendingCreatedSchedules; no authoritative backend can ever publish the fixed sch-alpha fixture. This makes process-local UI state a competing schedule fact source and violates CLAUDE.md: “事实源唯一:跨请求/跨节点一致性事实必须有唯一权威来源(Actor 持久态或分布式状态),不依赖进程内偶然状态。”

What would change your verdict (only if comment or reject)

Keep the typed draft/review/receipt contracts, but move the fixture implementation behind explicit test/development injection or a disabled-by-default feature boundary. In production, do not report accepted, synthesize a pending schedule, or refresh the schedule read model unless a real host-owned Team Automation command endpoint returned that receipt; while the backend is unavailable, the consent flow may stop at a clearly labeled non-mutating preview.

⟦AI:AUTO-LOOP⟧
REVIEW_DONE:2746:architect:reject

⟦AI:AUTO-LOOP⟧

@potter-sun

potter-sun commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop timeout redrive attempt: fixing 2

⟦AI:FKST⟧

@potter-sun

Copy link
Copy Markdown
Contributor Author

github-devloop timeout reconcile action: drop

Reason:
state-output-obligation-timeout-after-3-attempts

Structured WHY:
reason_class=state-output-obligation-timeout
from_state=fixing
from_version=ready/consensus-github-devloop/issue/aevatarAI/aevatar/2742/2026-07-14T09-56-17Z/review-loop/1/fix/1
age_minutes=250
budget_minutes=120
attempt=3
attempt_limit=3
driving_queue=devloop_fixing
source_ref.kind=external
source_ref.ref=aevatarAI/aevatar#pr/2746

⟦AI:FKST⟧

@potter-sun potter-sun added fkst-dev:blocked devloop-blocked-terminal and removed fkst-dev:fixing review-rejected-and-fix-in-progress labels Jul 14, 2026
@potter-sun

potter-sun commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop timeout redrive attempt: blocked 2

⟦AI:FKST⟧

@potter-sun

Copy link
Copy Markdown
Contributor Author

github-devloop decompose output obligation exhausted

Structured WHY:
reason_class=decompose-output-obligation-timeout
from_state=blocked
from_version=ready/consensus-github-devloop/issue/aevatarAI/aevatar/2742/2026-07-14T09-56-17Z/review-loop/1/fix/1/timeout-reconcile/fixing/3
attempt=3

⟦AI:FKST⟧

…nsensus-github-devloop-issue-aevatarAI-aevatar-2742-2026-07-14T09-56-17Z-0252755960
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fkst-dev:blocked devloop-blocked-terminal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants