Skip to content

iter43 issue-865 streaming-proxy-room-chat-host-orchestration: reuse StreamingProxyGAgent + 删 coordinator/side-store#874

Closed
loning wants to merge 4 commits into
auto-refact-devfrom
refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration
Closed

iter43 issue-865 streaming-proxy-room-chat-host-orchestration: reuse StreamingProxyGAgent + 删 coordinator/side-store#874
loning wants to merge 4 commits into
auto-refact-devfrom
refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration

Conversation

@loning
Copy link
Copy Markdown
Contributor

@loning loning commented May 23, 2026

摘要

issue #865 cluster-043-streaming-proxy-room-chat-host-orchestration(high,多 CLAUDE 条款:top-layering / read-write-split / command-skeleton / runtime-shape-not-fact / authoritative-state / actor-lifecycle / ai-streaming)。

  • Old:StreamingProxy chat endpoint 和 participant coordinator 拿 runtime actor 对象、跑 Nyx 多轮 discussion loop、改 participant side-store state、从 Host 派 room event,Host 端编排业务。
  • New:StreamingProxyGAgent(复用现有 actor)拥有 participant admission / reply rounds / leave-failure decision / terminal-state publication;Host 只 typed command submission + projection/SSE 观察。Coordinator 大幅 shrink 仅作 Nyx 外部 adapter call。Participant store 转 projection readmodel。

Phase 9 共识来源

r1 三 solver(minimal 复用 / structural 新 actor / delete 删多轮)→ converge:round-2:"new session-scoped actor vs reuse with self-continuation"。r2 三 solver 都收敛到 reuse StreamingProxyGAgent + delete multi-round coordinator + remove side-store write authority(避开 hardcoded trigger #2 new actor)。

META_JUDGE_DONE:consensus:reuse-existing-room-actor:Reuse StreamingProxyGAgent as the authoritative room/session fact owner; move retained chat progression into typed actor-owned sub-state and self-continuation only if multi-step behavior remains; delete or shrink StreamingProxyNyxParticipantCoordinator; remove participant side-store write authority; keep Host/API as typed command submission plus projection/SSE observation only.

改动范围

16 files (+828/-1169,净 -341 LOC 删 coordinator + side-store write):

  • StreamingProxyGAgent.cs:扩展拥有 participant facts / rounds / failures / terminal-state
  • StreamingProxyEndpoints.cs:删 IActorRuntime + typed command/interaction service only
  • StreamingProxyNyxParticipantCoordinator.cs:大幅 shrink — 仅 Nyx external catalog/provider adapter
  • StreamingProxyRoomParticipantsProjector.cs(新):从 committed events 物化 participant readmodel
  • StreamingProxyRoomParticipantsQueryPort.cs(新):read-only query port
  • StreamingProxyRoomParticipantsSnapshot.Partial.cs + MetadataProvider.cs(新):typed readmodel
  • streaming_proxy_messages.proto:typed events 扩展
  • ActorBackedStreamingProxyParticipantStore.cs:转 projection readmodel(read-only)
  • StreamingProxyRoomInteraction.cs / ServiceCollectionExtensions.cs:适配
  • 测试:StreamingProxyCoverageTests / EndpointsCoverageTests / NyxParticipantCoordinatorTests 重构覆盖 actor-owned + delete coordinator behaviors

本地验证:dotnet build aevatar.slnx --nologo ✅ · architecture_guards / test_stability_guards / query_projection_priming_guard / projection_state_version_guard ✅ · StreamingProxy 测试 ✅。

详见 implement summarymeta-judge r2

Closes #865

📢 cc 原作者: @loning @eanzhao

🤖 Auto-loop / codex-refactor-loop iter43

⟦AI:AUTO-LOOP⟧

…StreamingProxyGAgent + 删 coordinator/side-store

按 issue #865 Phase 9 r2 consensus(meta-judge: reuse-existing-room-actor):

1. StreamingProxyGAgent 扩展拥有 participant facts / rounds / failures / terminal-state(典型 actor-owned events)
2. Endpoint: 删 IActorRuntime 注入,改 typed command/interaction service only
3. Coordinator: StreamingProxyNyxParticipantCoordinator 大幅 shrink — 仅作 Nyx external catalog/provider adapter call(无 business decision)
4. Participant side-store: ActorBackedStreamingProxyParticipantStore 转 projection readmodel(read-only),通过 StreamingProxyRoomParticipantsProjector + ReadModel snapshot
5. 新增 StreamingProxyRoomParticipantsQueryPort + Snapshot + Projector + MetadataProvider
6. streaming_proxy_messages.proto: 扩展 typed events
7. Host/API: typed command submission + projection/SSE observation only(无 actor 实例 / 无 envelope 构造)

不引入新 actor type(复用 StreamingProxyGAgent)。

Closes #865

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

loning commented May 23, 2026

📊 状态卡片 — PR #874 opened,r1 reviewer × 3 派出

维度
Issue #865 cluster-043-streaming-proxy-room-chat-host-orchestration
PR #874
改动 16 files +828/-1169(净 -341 LOC)
是否需要 maintainer 介入 ❌ 否

🤖 controller status banner

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

🤖 Phase 8 Reviewer — quality (round r1)

REVIEW_DONE:874:quality:comment

Verdict

Comment.

The PR direction matches the Phase 9 r2 consensus: no new session actor, StreamingProxyGAgent becomes the room/session fact owner, the endpoint now submits a typed StreamingProxyRoomChatCommand, the Nyx coordinator is reduced to Nyx catalog/reply adapter work, and the large negative LOC looks like real deletion of endpoint/coordinator orchestration rather than replacement over-engineering.

I found no remaining chat-path references to coordinator-side actor dispatch, participant-store writes, HandleEventAsync, Any.Pack in the endpoint, or process-local active/failed participant sets. Typed names are mostly clear: StreamingProxyRoomChatRequested, StreamingProxyRoomParticipantsSnapshot, StreamingProxyRoomChatAcceptedReceipt, and the RoomChat* interaction types describe their responsibilities.

Comments

  1. src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedStreamingProxyParticipantStore.cs:16 still reads the old singleton streaming-proxy-participants readmodel and unpacks StreamingProxyParticipantGAgentState at lines 31-37. The new endpoint participant query uses IStreamingProxyRoomParticipantsQueryPort, so this looks like a stale side-store surface kept alive by DI at src/Aevatar.Studio.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs:47 and tests. If IStreamingProxyParticipantStore has no production consumer, delete the interface/adapter/registration/tests. If it must remain as a compatibility query port, redirect it to the new room actor participants readmodel so there is only one participant fact source.

  2. docs/2026-04-02-streaming-proxy-flow.md is now materially stale. It still says IStreamingProxyParticipantStore is used for participant query, automatic join, and failed removal at line 21; the topology still has API -> participant store, API -> runtime, API -> subscription, and Nyx -> LLM ChatAsync -> actor at lines 30-46. That conflicts with this PR's command/projection path and the repo rule that architecture changes update docs.

Raw

  • Diff reviewed: origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration
  • HEAD verified: 6e756c0ba495ebb8ec7b4fdc87cd2c900bf5de8a
  • Missing requested input: .refactor-loop/runs/implement-issue865-streaming-proxy-room-chat-host-orchestration.md was not present in this worktree.
  • Meta-judge input read: .refactor-loop/runs/meta-judge-issue865-r2.md
  • Static checks performed:
    • git diff --stat
    • git diff --check
    • git grep for StreamingProxyNyxParticipantCoordinator, IStreamingProxyParticipantStore, actor dispatch, inline event handling, process-local participant sets, and new typed room chat/participants names.

POSTED:pending

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

🤖 Phase 8 Reviewer — tests (round r1)

REVIEW_DONE:874:tests:comment

raw
# Phase 8 Reviewer — tests r1

Review target: PR #874, HEAD `6e756c0ba495ebb8ec7b4fdc87cd2c900bf5de8a`

Verdict: `comment`

## Findings

1. Missing direct tests for the new room participant readmodel projector.

   The PR adds `StreamingProxyRoomParticipantsProjector`, but the tests only cover the endpoint consuming a stubbed `IStreamingProxyRoomParticipantsQueryPort`. That proves the endpoint maps an already-present snapshot, but not that committed `StreamingProxyGAgentState.Participants` is actually materialized into `StreamingProxyRoomParticipantsSnapshot` with actor-owned `StateVersion`, `RootActorId`, `LastEventId`, and join/leave removal semantics.

   Suggested coverage: add tests analogous to the terminal projector tests:
   - committed `GroupChatParticipantJoinedEvent` with room state participants writes a snapshot
   - committed `GroupChatParticipantLeftEvent` writes the post-leave participant list
   - non participant-affecting event is ignored

2. Actor-owned terminal failure/edge paths are under-covered.

   There is a good happy-path actor test for typed chat command orchestration: topic -> participant join -> message -> completed terminal. However, the moved behavior in `StreamingProxyGAgent.HandleRoomChatRequested` also owns important terminal branches: missing token, missing coordinator, no Nyx participants, failed/unavailable reply, and duplicate terminal idempotency. Those branches determine whether the actor commits `Completed` vs `Failed` and whether it emits participant-left facts after a failed participant reply.

   Suggested coverage: add focused actor tests for at least failed/unavailable reply and duplicate terminal session. These would protect the coordinator shrink from regressing back into host-side or adapter-side terminal compensation.

## Coverage Notes

- Actor-owned events: partially covered. Existing tests cover topic, participant join, participant message, and completed terminal for a successful typed room chat. They also cover regular join/leave/message lifecycle and terminal projector materialization.
- Readmodel query path: endpoint query path is covered through a stub query port; direct query-port/document-reader behavior and projector write behavior are not directly covered for the new participants readmodel.
- Coordinator shrink behavior: covered at adapter level for resolving participants, preserving node identity, classifying unavailable responses, stream-content fallback, and legacy `/api/v1/llm/status` fallback. The tests assert no actor/store collaborator remains in the coordinator fixture.
- Adapter Nyx call behavior: covered for catalog endpoint fallback and LLM request route metadata in the happy-path actor test.
- `[Skip]` / `Task.Delay`: no new violation found in this PR scope. `tools/ci/test_stability_guards.sh` passed.

## Verification

- `dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy"`: passed, 65 tests, 0 skipped.
- `dotnet test test/Aevatar.Tools.Cli.Tests/Aevatar.Tools.Cli.Tests.csproj --nologo --filter "FullyQualifiedName~ActorBackedStoreAdapterTests"`: passed, 68 tests, 0 skipped.
- `bash tools/ci/test_stability_guards.sh`: passed.

REVIEW_DONE:874:tests:comment

POSTED:#874 (comment)

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

🤖 Phase 8 Reviewer — architect (round r1)

PR: #874
HEAD: 6e756c0ba495ebb8ec7b4fdc87cd2c900bf5de8a
Base diff reviewed: origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration

Verdict: comment

Scope checked

  • StreamingProxyGAgent owns participant/session facts without adding a new actor type.
  • Endpoint has no direct IActorRuntime and uses typed chat interaction service for :chat.
  • StreamingProxyNyxParticipantCoordinator no longer dispatches actor events or writes participant side-store state.
  • Participant list endpoint reads from room current-state readmodel.
  • Host endpoint code no longer constructs EventEnvelope.

Findings

Comment: chat command carries bearer token as actor protobuf payload

StreamingProxyRoomChatRequested adds access_token = 4, HandleChatAsync copies the HTTP bearer token into StreamingProxyRoomChatCommand, and StreamingProxyRoomChatCommandEnvelopeFactory packs it into an EventEnvelope delivered to the room actor. The actor then reads request.AccessToken and passes it to the Nyx adapter.

Evidence:

  • agents/Aevatar.GAgents.StreamingProxy/streaming_proxy_messages.proto: StreamingProxyRoomChatRequested.access_token
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyEndpoints.cs: ExtractBearerToken(http) is passed into StreamingProxyRoomChatCommand
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyRoomInteraction.cs: AccessToken = command.AccessToken ?? string.Empty
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyGAgent.cs: var accessToken = request.AccessToken?.Trim()

This is not a Host orchestration regression, but it misses the r2 acceptance wording around bounded credential input/reference semantics and "no persisted bearer token." Actor inbox EventEnvelope traffic is not a safe credential carrier unless the runtime contract explicitly guarantees the command envelope is never persisted, projected, logged, replayed, or dead-lettered with payload intact. The safer architecture is to keep only a short-lived credential reference or actor-local opaque credential handle in the typed command, resolved at the adapter boundary during the same actor turn, and ensure no bearer value enters proto domain/command payloads.

Architecture checkpoint answers

  • StreamingProxyGAgent owns participant/session facts: yes. Participant join/leave/message/topic/terminal transitions are committed in StreamingProxyGAgentState, and no new session actor type is introduced.
  • Endpoint has no IActorRuntime: yes for endpoint handlers. HandleChatAsync depends on ICommandInteractionService<StreamingProxyRoomChatCommand,...> and admission only.
  • Coordinator shrunk to adapter-only: mostly yes. It resolves Nyx provider candidates and generates one reply through ChatStreamAsync; actor decides join/leave/terminal. It should not receive raw bearer tokens via persisted protobuf payloads.
  • Participant store read-only readmodel: yes for the existing IStreamingProxyParticipantStore; its write methods were removed. The new participant endpoint uses IStreamingProxyRoomParticipantsQueryPort over a room current-state projection.
  • Host has no EventEnvelope construction: yes in StreamingProxyEndpoints.cs. EventEnvelope construction remains in command/envelope factories and existing room command service, not in endpoint handler code.

REVIEW_DONE:874:architect:comment

raw
Reviewed PR #874 against the r2 consensus criteria.

Most structural goals are met: the endpoint no longer performs actor runtime lookup or participant loop orchestration; the coordinator no longer writes participant store state or dispatches actor events; participant list reads from a room readmodel; and `StreamingProxyGAgent` commits room participant/message/terminal facts.

The remaining architecture issue is credential boundary shape. The PR puts the request bearer token into `StreamingProxyRoomChatRequested.access_token`, which is packed into an `EventEnvelope` and sent through the actor command path. Unless this command-envelope path is explicitly non-persistent/non-observable/non-dead-lettered with payload, that violates the r2 "credential input/reference semantics" and "no persisted bearer token" direction. Replace the raw token field with a short-lived credential reference/handle, or otherwise prove and guard that the token cannot be persisted, logged, projected, or replayed.

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 状态 — r1 architect substantive comment(credential safety)→ 派 fix-r2

维度
Reviewer r1 architect: comment(bearer token in proto payload — security)· tests: comment · quality: comment
关键 architect 评价 PR 大部分 r2 consensus 已满足,仅 credential boundary:bearer token 不应进 actor inbox EventEnvelope payload
必修 用 short-lived credential reference / opaque handle,actor turn 内 adapter 边界 resolve
是否需要 maintainer 介入 ❌ 否

🤖 controller status banner

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 review-pr874-architect-r1 (⏳ 进行中; 已跑 11 min)

+<summary>raw</summary>
+
+```text
+Reviewed PR #874 against the r2 consensus criteria.
+
+Most structural goals are met: the endpoint no longer performs actor runtime lookup or participant loop orchestration; the coordinator no longer writes participant store state or dispatches actor events; participant list reads from a room readmodel; and `StreamingProxyGAgent` commits room participant/message/terminal facts.
+
+The remaining architecture issue is credential boundary shape. The PR puts the request bearer token into `StreamingProxyRoomChatRequested.access_token`, which is packed into an `EventEnvelope` and sent through the actor command path. Unless this command-envelope path is explicitly non-persistent/non-observable/non-dead-lettered with payload, that violates the r2 "credential input/reference semantics" and "no persisted bearer token" direction. Replace the raw token field with a short-lived credential reference/handle, or otherwise prove and guard that the token cannot be persisted, logged, projected, or replayed.
+```
+
+</details>
+
+POSTED:https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524507480
+
+⟦AI:AUTO-LOOP⟧

tokens used
129,260
完成。

- Artifact: `.refactor-loop/runs/review-pr874-architect-r1.md`
- Marker: `REVIEW_DONE:874:architect:comment`
- PR comment: https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524507480
- Sentinel included: `⟦AI:AUTO-LOOP⟧`

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 review-pr874-quality-r1 (⏳ 进行中; 已跑 11 min)

+### Raw
+
+- Diff reviewed: `origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration`
+- HEAD verified: `6e756c0ba495ebb8ec7b4fdc87cd2c900bf5de8a`
+- Missing requested input: `.refactor-loop/runs/implement-issue865-streaming-proxy-room-chat-host-orchestration.md` was not present in this worktree.
+- Meta-judge input read: `.refactor-loop/runs/meta-judge-issue865-r2.md`
+- Static checks performed:
+  - `git diff --stat`
+  - `git diff --check`
+  - `git grep` for `StreamingProxyNyxParticipantCoordinator`, `IStreamingProxyParticipantStore`, actor dispatch, inline event handling, process-local participant sets, and new typed room chat/participants names.
+
+POSTED:https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524493420
+
+⟦AI:AUTO-LOOP⟧

tokens used
118,082
已完成 PR #874 quality r1 review。

Artifact: [.refactor-loop/runs/review-pr874-quality-r1.md](/Users/auric/aevatar/.refactor-loop/runs/review-pr874-quality-r1.md)

结果:`REVIEW_DONE:874:quality:comment`

已发布 PR 评论: https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524493420

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 review-pr874-tests-r1 (⏳ 进行中; 已跑 11 min)

+
+- `dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy"`: passed, 65 tests, 0 skipped.
+- `dotnet test test/Aevatar.Tools.Cli.Tests/Aevatar.Tools.Cli.Tests.csproj --nologo --filter "FullyQualifiedName~ActorBackedStoreAdapterTests"`: passed, 68 tests, 0 skipped.
+- `bash tools/ci/test_stability_guards.sh`: passed.
+
+REVIEW_DONE:874:tests:comment
+```
+
+</details>
+
+POSTED:https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524496032
+
+⟦AI:AUTO-LOOP⟧

tokens used
174,509
Done.

Created artifact: [.refactor-loop/runs/review-pr874-tests-r1.md](/Users/auric/aevatar/.refactor-loop/runs/review-pr874-tests-r1.md)

Posted PR comment: https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524496032

Marker: `REVIEW_DONE:874:tests:comment`

Verification run:

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

🤖 fix-codex round 2 — credential safety

PR: #874
Round: 2

Applied

  1. Removed raw bearer material from the StreamingProxy room chat protobuf command path.

    • Replaced StreamingProxyRoomChatRequested.access_token with credential_handle_id.
    • StreamingProxyRoomChatCommand now carries CredentialHandleId, not AccessToken.
    • StreamingProxyRoomChatCommandEnvelopeFactory packs only the opaque handle into EventEnvelope.
  2. Added an opaque short-lived credential handle store.

    • StreamingProxyRoomCredentialHandleStore uses IMemoryCache with a 2-minute TTL.
    • Endpoint extracts the HTTP bearer only at Host boundary and registers it in the handle store.
    • StreamingProxyGAgent consumes the handle inside the actor turn before calling the Nyx adapter.
    • No new actor was introduced.
  3. Kept Nyx provider authentication out of request metadata bags.

    • StreamingProxyNyxParticipantCoordinator now passes the resolved bearer via typed LLMRequestCallerContext.Credentials.NyxIdBearer.
    • It no longer writes LLMRequestMetadataKeys.NyxIdAccessToken into LLMRequest.Metadata.
  4. Added/updated focused tests.

    • Descriptor guard asserts StreamingProxyRoomChatRequested no longer exposes access_token.
    • Envelope test asserts the typed command carries credential_handle_id.
    • Actor-owned chat progression resolves the bearer through the handle store and verifies Nyx receives it only through typed caller credentials, not metadata.
    • Endpoint tests were updated to assert handle propagation rather than token propagation.

Review Notes

  • Architect blocking comment addressed: no bearer value enters the StreamingProxy room chat protobuf command payload or actor EventEnvelope.
  • Tests reviewer comments were non-blocking; this round added credential-safety regression coverage in the existing StreamingProxy test suite.
  • Quality reviewer comments were non-blocking and outside the credential-safety blocker. No extra stale-store/doc rewrite was bundled into this security fix to keep the patch scoped.

Verification

  • dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy": passed, 66 tests, 0 skipped.
  • bash tools/ci/test_stability_guards.sh: passed.
  • git diff --check: passed.

FIX_DONE:874:round-2:applied-4:rejected-0:blocked-0

⟦AI:AUTO-LOOP⟧

…r token 改 opaque handle

按 PR #874 r1 architect substantive comment(security):

- 移除 StreamingProxyRoomChatRequested.access_token proto 字段
- 新 StreamingProxyRoomCredentialHandles.cs: opaque credential handle 模式(actor-local short-lived,non-persisted)
- StreamingProxyRoomChatCommand 改携 credential_handle_id,actor turn 内 adapter 边界 resolve
- StreamingProxyEndpoints/GAgent/RoomInteraction/NyxParticipantCoordinator 适配 handle 流
- ServiceCollectionExtensions 注册 handle registry
- StreamingProxyCoverageTests 更新覆盖 credential boundary(+96 行测试)

确保 bearer token 不再进 proto domain/command payload,避免 EventEnvelope 持久化/日志/replay/dead-letter 风险。

applied=4 rejected=0 blocked=0(per fix-pr874-r2 codex)

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

loning commented May 23, 2026

🤖 Phase 8 Reviewer — quality (round r2)

REVIEW_DONE:874:quality:approve

Verdict

Approve.

The r2 credential-safety fix is quality-clean for the requested scope. StreamingProxyRoomCredentialHandles is a clear enough file name for the new bounded surface, and the actual type names are more explicit: IStreamingProxyRoomCredentialHandleStore / StreamingProxyRoomCredentialHandleStore communicate "room chat credential, opaque handle, store" without pretending the handle is a durable actor fact.

Checks

  • StreamingProxyRoomChatRequested.access_token is gone from streaming_proxy_messages.proto; field 4 is now credential_handle_id.
  • StreamingProxyRoomChatCommand now carries CredentialHandleId, and StreamingProxyRoomChatCommandEnvelopeFactory packs only that handle plus route/model preferences into the EventEnvelope.
  • The HTTP endpoint still extracts the bearer only at the Host boundary, immediately registers it with credentialHandles.Create(...), and sends the returned handle through the command pipeline.
  • The actor consumes the handle inside HandleRoomChatRequested via credentialHandles?.Consume(request.CredentialHandleId) before calling the Nyx adapter.
  • The Nyx participant request no longer writes LLMRequestMetadataKeys.NyxIdAccessToken into LLMRequest.Metadata; it passes the resolved bearer through typed LLMRequestCallerContext.Credentials.
  • The handle registry registration is minimal: services.AddMemoryCache() plus TryAddSingleton<IStreamingProxyRoomCredentialHandleStore, StreamingProxyRoomCredentialHandleStore>().
  • The store itself is small and single-purpose: GUID handle creation, 2-minute memory-cache TTL, remove-on-consume behavior, and whitespace normalization.

Residual Notes

No new quality finding. The r1 quality comments about stale participant-store/doc surfaces remain outside this r2 credential-safety patch and do not block this round.

Verification

  • git rev-parse origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration: 57a7da2e848b17badef8792574ab60f25d5eff75
  • git diff --check origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration: passed.
  • Reviewed r2 commit diff 6e756c0b..57a7da2e and targeted grep for access_token, AccessToken, NyxIdAccessToken, CredentialHandleId, and credential_handle_id across StreamingProxy sources/tests.

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

🤖 Phase 8 Reviewer — architect (round r2)

PR: #874
HEAD: 57a7da2e848b17badef8792574ab60f25d5eff75
Base diff reviewed: origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration

Verdict: comment

Scope checked

  • r1 credential-safety finding from .refactor-loop/runs/review-pr874-architect-r1.md
  • r2 fix report from .refactor-loop/runs/fix-pr874-r2.md
  • StreamingProxy room chat proto, command envelope, endpoint boundary, credential handle store, actor consumption path, and Nyx adapter request construction

Findings

Comment: credential handle is short-lived, but not actor-local/scoped

r2 fixes the raw bearer leak from the protobuf/EventEnvelope path, but the opaque handle is currently a process-wide singleton cache entry keyed only by a random handle id. StreamingProxyRoomCredentialHandleStore.Create stores the bearer under streaming-proxy-room-chat-credential:{handleId}, and Consume returns/removes it for any caller that has that handle id. The handle store is registered as a singleton, and the handle is not bound to the target room actor, session id, scope id, command id, or correlation id before returning the bearer.

Evidence:

  • agents/Aevatar.GAgents.StreamingProxy/ServiceCollectionExtensions.cs:36-38: AddMemoryCache() plus singleton IStreamingProxyRoomCredentialHandleStore.
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyRoomCredentialHandles.cs:27-56: create/consume by opaque id only, with 2 minute TTL and one-time removal.
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyEndpoints.cs:205-216: endpoint creates a handle and sends only the handle through the typed command.
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyGAgent.cs:57-60: room actor resolves the handle during the event handler turn, but without presenting/binding actor/session identity to the store.

That means the handle is short-lived and non-persisted, but it is not yet actor-local in the acceptance sense. If an EventEnvelope payload, log line, dead-letter, test harness, or compromised in-process component exposes the handle id during the TTL window, any code with access to the singleton store can redeem the real bearer. This is much smaller than the r1 raw-token-in-envelope issue, but it is still a credential-boundary gap.

Suggested fix: make the handle capabilities scoped at creation and validated at consume. For example, create a typed handle record { handleId, roomId, sessionId, scopeId, expiresAt } in the in-memory holder, keep only handleId in the protobuf payload, and change actor consumption to Consume(handleId, roomId, sessionId, scopeId) so the bearer is released only to the intended room chat turn. That keeps the EventEnvelope payload opaque while making accidental handle disclosure insufficient to redeem the bearer.

Resolved from r1

  • StreamingProxyRoomChatRequested.access_token is removed. The proto now has credential_handle_id and no access_token field.
  • StreamingProxyRoomChatCommandEnvelopeFactory packs only credential_handle_id, route, model, prompt, session, and scope into the EventEnvelope.
  • The adapter path no longer places the bearer in LLMRequest.Metadata[NyxIdAccessToken]; it uses typed LLMRequestCallerContext.Credentials.
  • The real bearer is resolved inside StreamingProxyGAgent.HandleRoomChatRequested before the Nyx adapter call, not in the HTTP endpoint or command envelope factory.

Architecture checkpoint answers

  • Proto access_token truly deleted: yes.
  • Opaque handle short-lived/non-persisted: yes, via in-memory cache TTL and one-time consume.
  • Opaque handle actor-local: not yet; it is process-local singleton state keyed only by handle id.
  • Adapter boundary resolves handle to real token during actor turn: mostly yes; StreamingProxyGAgent consumes the handle and passes the resolved token into the coordinator/adapter call.
  • Bearer token in EventEnvelope payload: no evidence of raw bearer entering StreamingProxy room chat EventEnvelope payload after r2.

REVIEW_DONE:874:architect:comment

raw
Reviewed PR #874 r2 credential-safety fix at HEAD 57a7da2e.

The raw bearer issue from r1 is materially improved: StreamingProxyRoomChatRequested no longer has access_token; EventEnvelope payload carries credential_handle_id only; Nyx adapter request construction uses typed CallerContext credentials rather than LLMRequest.Metadata[NyxIdAccessToken].

I am leaving this as comment rather than approve because the handle is short-lived and non-persisted, but not actor-local/scoped. The new store is a singleton IMemoryCache holder keyed only by handle id. The actor consumes it inside the handler turn, but Consume does not validate roomId/sessionId/scopeId/commandId. A leaked handle id during the TTL window is therefore sufficient for any in-process component with store access to redeem the real bearer. Bind the handle to the intended room chat identity at Create time and require the actor to present the matching room/session/scope at Consume time.

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

Phase 8 Reviewer - tests - PR #874 round r2

Verdict

REVIEW_DONE:874:tests:comment

Finding

  1. Missing direct credential handle lifecycle tests for single-use and expiry.

    r2 moves bearer material out of StreamingProxyRoomChatRequested and into StreamingProxyRoomCredentialHandleStore, which is the right boundary for the credential-safety fix. The new tests cover the most important non-persistence and adapter-boundary assertions:

    • endpoint command propagation carries CredentialHandleId, not the bearer token (StreamingProxyCoverageTests.HandleChatAsync_ShouldAttachProjectionSession_AndEmitRunFinished);
    • command dispatch packs StreamingProxyRoomChatRequested.CredentialHandleId into the envelope (StreamingProxyRoomInteraction_ShouldBindDispatchEmitFinalizeAndCleanup);
    • proto descriptor no longer exposes access_token (StreamingProxyRoomChatRequested_ShouldNotExposeBearerTokenField);
    • actor-owned room chat resolves the handle and Nyx receives the bearer only via typed caller credentials, not request metadata (GAgent_ShouldOwnRoomChatProgression_ForTypedChatCommand);
    • envelope bytes are checked against a sentinel bearer string (StreamingProxyRoomChatEnvelopeFactory_ShouldBuildTypedChatEnvelope).

    The remaining gap is the handle store contract itself. StreamingProxyRoomCredentialHandleStore creates a 2-minute in-memory entry and removes it on consume, but there is no direct test that:

    • Create stores a normalized token behind an opaque handle;
    • Consume returns the token exactly once and then removes the entry;
    • an expired/missing handle resolves to null without reaching Nyx;
    • blank bearer values do not become usable credentials.

    Suggested coverage: add a focused unit test around StreamingProxyRoomCredentialHandleStore using a controllable cache clock or injectable TTL/options, without Task.Delay. At minimum, cover create/use/single-use and missing/blank handles directly; expiry should be deterministic rather than time-sleep based.

Coverage Assessment

  • Credential not persisted in protobuf command/event envelope: covered for the command payload path by descriptor and sentinel-byte assertions.
  • Credential boundary at adapter call: covered. The actor test verifies CallerContext.Credentials.NyxIdBearer == "token-1" and Metadata does not contain LLMRequestMetadataKeys.NyxIdAccessToken.
  • Endpoint-to-command handle propagation: covered through the endpoint test and command interaction test.
  • Handle store lifecycle: partially covered through integration use, but missing direct single-use and expiry tests.
  • Forbidden test patterns: no new [Skip], Task.Delay, or WaitUntilAsync in the PR diff. bash tools/ci/test_stability_guards.sh passed.

Verification

  • Reviewed full PR diff: origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration.
  • Reviewed r1 artifact: .refactor-loop/runs/review-pr874-tests-r1.md.
  • Reviewed r2 fix report: .refactor-loop/runs/fix-pr874-r2.md.
  • dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy"
    • Passed: 66 passed, 0 failed, 0 skipped.
  • bash tools/ci/test_stability_guards.sh
    • Passed.
  • Diff scan for forbidden patterns in touched test files:
    • no added [Skip], Task.Delay, or WaitUntilAsync.

REVIEW_DONE:874:tests:comment

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 状态 — r2 1 approve + 2 comment(无 reject)→ 派 fix-r3 解 tests narrow comment

维度
Reviewer r2 architect: comment · tests: comment(handle single-use + expiry tests)· quality: ✅ approve
共识 0 reject(可推进),tests narrow comment 易解
是否需要 maintainer 介入 ❌ 否

🤖 controller status banner

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 review-pr874-architect-r2 (⏳ 进行中; 已跑 9 min)

+<details>
+<summary>raw</summary>
+
+```text
+Reviewed PR #874 r2 credential-safety fix at HEAD 57a7da2e.
+
+The raw bearer issue from r1 is materially improved: StreamingProxyRoomChatRequested no longer has access_token; EventEnvelope payload carries credential_handle_id only; Nyx adapter request construction uses typed CallerContext credentials rather than LLMRequest.Metadata[NyxIdAccessToken].
+
+I am leaving this as comment rather than approve because the handle is short-lived and non-persisted, but not actor-local/scoped. The new store is a singleton IMemoryCache holder keyed only by handle id. The actor consumes it inside the handler turn, but Consume does not validate roomId/sessionId/scopeId/commandId. A leaked handle id during the TTL window is therefore sufficient for any in-process component with store access to redeem the real bearer. Bind the handle to the intended room chat identity at Create time and require the actor to present the matching room/session/scope at Consume time.
+```
+
+</details>
+
+⟦AI:AUTO-LOOP⟧

tokens used
128,248
Reviewed PR #874 r2 and posted the architect comment.

Result: `REVIEW_DONE:874:architect:comment`

Artifact: [.refactor-loop/runs/review-pr874-architect-r2.md](/Users/auric/aevatar/.refactor-loop/runs/review-pr874-architect-r2.md)

Posted: https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524598875

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 review-pr874-quality-r2 (⏳ 进行中; 已跑 9 min)

+
+### Residual Notes
+
+No new quality finding. The r1 quality comments about stale participant-store/doc surfaces remain outside this r2 credential-safety patch and do not block this round.
+
+### Verification
+
+- `git rev-parse origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration`: `57a7da2e848b17badef8792574ab60f25d5eff75`
+- `git diff --check origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration`: passed.
+- Reviewed r2 commit diff `6e756c0b..57a7da2e` and targeted grep for `access_token`, `AccessToken`, `NyxIdAccessToken`, `CredentialHandleId`, and `credential_handle_id` across StreamingProxy sources/tests.
+
+POSTED:https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524577713
+
+⟦AI:AUTO-LOOP⟧

tokens used
124,097
Done.

Artifact: [.refactor-loop/runs/review-pr874-quality-r2.md](/Users/auric/aevatar/.refactor-loop/runs/review-pr874-quality-r2.md)

Marker: `REVIEW_DONE:874:quality:approve`

Posted: https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524577713

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 review-pr874-tests-r2 (⏳ 进行中; 已跑 9 min)

+  - Passed: 66 passed, 0 failed, 0 skipped.
+- `bash tools/ci/test_stability_guards.sh`
+  - Passed.
+- Diff scan for forbidden patterns in touched test files:
+  - no added `[Skip]`, `Task.Delay`, or `WaitUntilAsync`.
+
+REVIEW_DONE:874:tests:comment
+
+POSTED:https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524600743
+
+⟦AI:AUTO-LOOP⟧

tokens used
176,471
REVIEW_DONE:874:tests:comment

Posted tests r2 review to PR #874:
https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524600743

Artifact and sentinel written:
- `.refactor-loop/runs/review-pr874-tests-r2.md`
- `.refactor-loop/runs/review-pr874-tests-r2.sentinel`

Verification passed:
- `dotnet test ... --filter "FullyQualifiedName~StreamingProxy"`: 66 passed, 0 skipped

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

🤖 fix-codex round 3 — credential handle lifecycle tests

PR: #874
Round: 3

Applied

  1. Added direct StreamingProxyRoomCredentialHandleStore lifecycle coverage:

    • single-use consume returns the normalized bearer once, then returns null;
    • missing/null/blank handles return null;
    • expiry is deterministic through FakeTimeProvider, without Task.Delay;
    • store contract exposes no enumeration surface and keeps no durable/enumerable store fields.
  2. Addressed the architect r2 scoped-handle advisory:

    • handle creation now binds { roomId, scopeId, sessionId };
    • actor consumption must present the matching room/scope/session scope;
    • wrong room, wrong scope, or wrong session cannot redeem the handle;
    • protobuf/EventEnvelope still carries only credential_handle_id.

Files Changed

  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyRoomCredentialHandles.cs
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyEndpoints.cs
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyGAgent.cs
  • test/Aevatar.AI.Tests/StreamingProxyCoverageTests.cs
  • test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj

Verification

  • dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy"
    • Passed: 71 passed, 0 failed, 0 skipped.
  • bash tools/ci/test_stability_guards.sh
    • Passed.
  • git diff --check
    • Passed.

FIX_DONE:874:round-3:applied-2:rejected-0:blocked-0

⟦AI:AUTO-LOOP⟧

…e + expiry)

按 PR #874 r2 tests narrow comment 补:
- StreamingProxyRoomCredentialHandles: single-use test(handle 用过 invalidate)
- expiry test(TTL 到 expired)

applied=2 rejected=0 blocked=0(per fix-pr874-r3 codex)

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

loning commented May 23, 2026

🤖 Phase 8 Reviewer — quality (round r3)

REVIEW_DONE:874:quality:approve

Verdict

Approve.

r3 resolves the r2 tests comment for credential-handle lifecycle coverage and does not introduce a new quality or architecture-rule violation in the reviewed scope.

Checks

  • r2 quality artifact reviewed: .refactor-loop/runs/review-pr874-quality-r2.md.
  • r3 fix report reviewed: .refactor-loop/runs/fix-pr874-r3.md.
  • HEAD verified: 4463c7e7a3a789cf6ce5af6735fa6f8666e50d02.
  • r3 changed files reviewed:
    • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyRoomCredentialHandles.cs
    • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyEndpoints.cs
    • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyGAgent.cs
    • test/Aevatar.AI.Tests/StreamingProxyCoverageTests.cs
    • test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj

Assessment

  • StreamingProxyRoomCredentialHandleStore now has direct deterministic tests for single-use consume, wrong room/scope/session rejection, blank/missing handle behavior, and expiry via FakeTimeProvider.
  • The handle remains an opaque protobuf/EventEnvelope value; raw bearer material does not return to StreamingProxyRoomChatRequested.
  • r3 also scopes handle creation/consume by { roomId, scopeId, sessionId }, addressing the architect r2 advisory without adding durable credential state or an enumerable store surface.
  • No new [Skip], Task.Delay, WaitUntilAsync, raw access_token, or NyxIdAccessToken regression was found in the r3 diff.

Verification

  • git diff --check origin/auto-refact-dev...4463c7e7: passed.
  • bash tools/ci/test_stability_guards.sh: passed.
  • dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy": passed, 71 passed, 0 failed, 0 skipped.

REVIEW_DONE:874:quality:approve

POSTED:pending

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

Phase 8 Reviewer - tests - PR #874 round r3

Verdict

REVIEW_DONE:874:tests:approve

Assessment

r3 resolves the r2 tests comment. The new focused coverage directly exercises StreamingProxyRoomCredentialHandleStore rather than only proving the endpoint/actor integration path:

  • StreamingProxyRoomCredentialHandleStore_ShouldResolveOnlyOnce_ForMatchingScope verifies token normalization, opaque handle shape, one successful consume, and null on second consume.
  • StreamingProxyRoomCredentialHandleStore_ShouldReturnNullForBlankOrMissingCredentials covers missing/null handles and blank bearer values.
  • StreamingProxyRoomCredentialHandleStore_ShouldExpireHandleDeterministically covers expiry with FakeTimeProvider, without sleeps or polling.
  • StreamingProxyRoomCredentialHandleStore_ShouldNotRedeemHandleForDifferentRoomScopeOrSession also covers the r2 architect scoped-handle advisory by requiring matching { roomId, scopeId, sessionId }.
  • StreamingProxyRoomCredentialHandleStore_ShouldExposeNoEnumerationOrDurableStateSurface guards the store surface from drifting into an enumerable/durable credential registry.

The endpoint and actor changes consistently pass the scoped handle through the existing command path: the endpoint creates the handle with room/scope/session, the protobuf/EventEnvelope still carries only credential_handle_id, and StreamingProxyGAgent consumes with the matching actor id, scope id, and session id.

Verification

  • Reviewed r2 tests artifact: .refactor-loop/runs/review-pr874-tests-r2.md.
  • Reviewed r3 fix report: .refactor-loop/runs/fix-pr874-r3.md.
  • Reviewed r3 commit diff: 57a7da2e..4463c7e7.
  • Reviewed full PR diff: origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration.
  • git diff --check origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration
    • Passed.
  • Diff scan for added forbidden test patterns ([Skip], Task.Delay, WaitUntilAsync, Thread.Sleep, GetAwaiter().GetResult())
    • Passed; no matches.
  • In detached worktree at PR HEAD 4463c7e7:
    • dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy"
    • Passed: 71 passed, 0 failed, 0 skipped.
  • In detached worktree at PR HEAD 4463c7e7:
    • bash tools/ci/test_stability_guards.sh
    • Passed.

No new tests finding.

REVIEW_DONE:874:tests:approve

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

Phase 8 Reviewer - architect (round r3)

PR: #874
HEAD: 4463c7e7a3a789cf6ce5af6735fa6f8666e50d02
Base diff reviewed: origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration

Verdict: approve

Scope checked

  • r2 architect artifact: .refactor-loop/runs/review-pr874-architect-r2.md
  • r3 fix report: .refactor-loop/runs/fix-pr874-r3.md
  • r3 delta from 57a7da2e to 4463c7e7
  • Full PR diff against origin/auto-refact-dev

Evidence

  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyRoomCredentialHandles.cs:45: handle creation now requires StreamingProxyRoomCredentialHandleScope and normalizes { roomId, scopeId, sessionId } before storing the bearer.
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyRoomCredentialHandles.cs:68: handle consumption now requires the same scope, rejects mismatched scope before releasing the bearer, checks expiry with TimeProvider, and removes the cache entry before returning the normalized token.
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyEndpoints.cs:205: endpoint creates the handle with the intended { roomId, scopeId, sessionId } scope and still sends only the opaque handle id through the command.
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyGAgent.cs:59: the room actor consumes the handle with its actor id plus request scope/session, satisfying the r2 actor-local/scoped handle requirement.
  • agents/Aevatar.GAgents.StreamingProxy/StreamingProxyNyxParticipantCoordinator.cs:658: StreamingProxy participant request metadata no longer carries LLMRequestMetadataKeys.NyxIdAccessToken; the bearer is passed through typed LLMRequestCallerContext.Credentials.
  • test/Aevatar.AI.Tests/StreamingProxyCoverageTests.cs:790: r3 adds lifecycle coverage for single-use consumption, wrong room/scope/session rejection, blank/missing credentials, deterministic expiry via FakeTimeProvider, and no enumerable/durable store surface.
  • No new architecture violation found in the r3 delta: no raw bearer in protobuf/EventEnvelope payload, no process-local actor/session fact registry beyond the short-lived opaque credential holder, no new query-time replay/priming, and no new ChatAsync main-chain usage.

Verification

  • dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy" in clean PR worktree: passed, 71 passed, 0 failed, 0 skipped.
  • bash tools/ci/test_stability_guards.sh: passed.
  • git diff --check origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration: passed.

REVIEW_DONE:874:architect:approve

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

✅ 状态 — r3 3/3 unanimous approve → merge

维度
r3 Reviewer architect: ✅ approve · tests: ✅ approve · quality: ✅ approve
共识 3/3 unanimous(r1 reject → r2 comment → r3 unanimous,credential safety 全解)

🤖 controller status banner

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 review-pr874-architect-r3 (⏳ 进行中; 已跑 10 min)

+- `git diff --check origin/auto-refact-dev...origin/refactor/iter43-cluster-043-streaming-proxy-room-chat-host-orchestration`: passed.
+
+REVIEW_DONE:874:architect:approve
+
+POSTED:https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524665628
+
+⟦AI:AUTO-LOOP⟧
diff --git a/.refactor-loop/runs/review-pr874-architect-r3.sentinel b/.refactor-loop/runs/review-pr874-architect-r3.sentinel
new file mode 100644
index 0000000000000000000000000000000000000000..e37f146d8216b5d422b7717a54ec4f000185f8ee
--- /dev/null
+++ b/.refactor-loop/runs/review-pr874-architect-r3.sentinel
@@ -0,0 +1,3 @@
+REVIEW_DONE:874:architect:approve
+POSTED:https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524665628
+⟦AI:AUTO-LOOP⟧

tokens used
180,501
Approved PR #874 r3 as architect.

Artifact: `.refactor-loop/runs/review-pr874-architect-r3.md`  
Sentinel: `.refactor-loop/runs/review-pr874-architect-r3.sentinel`  
Posted: https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524665628

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 review-pr874-quality-r3 (⏳ 进行中; 已跑 10 min)

new file mode 100644
index 0000000000000000000000000000000000000000..662c0c750b44356bc9bda27bc557fb669e820eb0
--- /dev/null
+++ b/.refactor-loop/runs/review-pr874-quality-r3.sentinel
@@ -0,0 +1,2 @@
+REVIEW_DONE:874:quality:approve
+⟦AI:AUTO-LOOP⟧

tokens used
92,645
完成。

Artifact:
- [.refactor-loop/runs/review-pr874-quality-r3.md](/Users/auric/aevatar/.refactor-loop/runs/review-pr874-quality-r3.md)
- [.refactor-loop/runs/review-pr874-quality-r3.sentinel](/Users/auric/aevatar/.refactor-loop/runs/review-pr874-quality-r3.sentinel)

Result:
`REVIEW_DONE:874:quality:approve`

Posted:
https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524660562

Verification:
- `git diff --check origin/auto-refact-dev...4463c7e7`: passed
- `bash tools/ci/test_stability_guards.sh`: passed

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 review-pr874-tests-r3 (⏳ 进行中; 已跑 10 min)

+  - `dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy"`
+  - Passed: 71 passed, 0 failed, 0 skipped.
+- In detached worktree at PR HEAD `4463c7e7`:
+  - `bash tools/ci/test_stability_guards.sh`
+  - Passed.
+
+No new tests finding.
+
+REVIEW_DONE:874:tests:approve
+
+⟦AI:AUTO-LOOP⟧

tokens used
102,007
完成。

Artifact: `.refactor-loop/runs/review-pr874-tests-r3.md`  
Sentinel: `.refactor-loop/runs/review-pr874-tests-r3.sentinel`  
PR comment: https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4524662452

Marker: `REVIEW_DONE:874:tests:approve`

验证通过:
- `dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj --nologo --filter "FullyQualifiedName~StreamingProxy"`: 71 passed, 0 failed, 0 skipped
- `bash tools/ci/test_stability_guards.sh`: passed

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

loning added a commit that referenced this pull request May 23, 2026
… facade + actor-owned compensation (#885)

* iter47 issue-877 chat-endpoints-own-lifecycle-and-compensation: typed lifecycle facades + actor 拥有 compensation/terminal-state

按 issue #877 Phase 9 r1 consensus(meta-judge: structural,no new actor):

1. NyxIdChatLifecycleFacade.cs(新) + StreamingProxyChatLifecycleFacade.cs(新):typed lifecycle command facade
2. NyxIdChatEndpoints: 删 IActorRuntime / IRegistryCommandPort / IChatHistoryStore 注入,改 facade only
3. StreamingProxyEndpoints: 进一步收窄(与 #874 不冲突)
4. NyxIdChatGAgent / StreamingProxyGAgent: 扩展 lifecycle + compensation events(actor-owned)
5. agent_run.proto: typed lifecycle event 字段
6. ServiceCollectionExtensions: 注册 facade + 删 store injection
7. 测试覆盖 endpoint adapter only + facade dispatch + actor compensation events

不引入新 actor / 不 saga actor。

Closes #877

⟦AI:AUTO-LOOP⟧

* fix(pr885 r2): review 反馈 — typed lifecycle facade + actor compensation 完整化

3 reviewer r1: architect/tests reject、quality approve。r2 应用 2 处:
- NyxIdChat / StreamingProxy chat lifecycle facade 与 actor compensation 全链路对齐
- 测试覆盖 facade dispatch / actor compensation / terminal-state readmodel

⟦AI:AUTO-LOOP⟧

* fix(pr885 r3): review r2 4 处 blocking — explicit CreatedLocally + admission gate + room unregister + real-actor compensation tests

1. ARCHITECT: NyxId create resolver 显式 set CreatedLocally(ForwardToGagent=false / fresh=true);actor 不再从 id prefix 推断 → 既有路由 actor 不会被误销毁
2. QUALITY-1: StreamingProxy room delete 补回 IGAgentActorRegistryCommandPort 注销(facade 漏抄);delete-side fail 时返回失败
3. QUALITY-2: NyxId delete resolver non-accepted admission 时拒绝解析 → 403/404 路径不再触发 actor unregister/delete history
4. TESTS: NyxIdChatGAgentTests 加真 envelope test 覆盖 HandleCreationCompensationAsync(createdLocally=true|false 双 branch)+ HandleDeletionCompensationAsync(registration restore);endpoint helper 不再 short-circuit 修后 lifecycle

127 passed + test_stability_guards passed。

⟦AI:AUTO-LOOP⟧

* fix(pr885 r4): await actor outcome — replace pre-dispatch ACK with continuation event

architect+tests r3 同根 BLOCKER:CreateConversationAsync 返 200 基于 pre-dispatch ACK,actor 后续可 compensate/destroy 不抛 → endpoint 报 success 但 actor 没真注册。

修法 A(continuation):
- 新 ICommandDispatchService.DispatchAndAwaitOutcomeAsync<TOutcome> + ActorOutcomeSubscription + CommandOutcomeDispatchResult
- NyxIdChatGAgent 完成 create handler 后发 typed NyxIdChatConversationCreationOutcome continuation event(success/RegistrationUnavailable/Destroyed)
- NyxIdChatLifecycleFacade 改 await continuation,基于真 actor outcome 构 receipt
- endpoint 200 / 503 / 等基于真 outcome
- 删 InlineNyxIdCreateLifecycleDispatchService fake → endpoint test 走真 dispatch + 真 actor + 假 registry 注入失败

加 tests:
- 真 actor compensation test(真 envelope 经真 CommandDispatchService)
- delete admission negative(Denied/ScopeMismatch/Unavailable)— 验证无 actor delete dispatch、无 history delete、无 unregister 副作用

⟦AI:AUTO-LOOP⟧

* fix(pr885 r5): StreamActorOutcomeChannel + subscribe-before-dispatch + real DELETE pipeline

architect r4 3 个 BLOCKER:
1. InMemoryDict 违 actor/distributed 规则 → 改 StreamActorOutcomeChannel(Orleans IStreamProvider)
2. dispatch-before-outcome 顺序倒置 → subscribe-before-dispatch + actor failure 只发 outcome 不 rethrow
3. DELETE endpoint 测试仍 inline fake → 删,走真 dispatch pipeline

修法:
- 删 InMemoryActorOutcomeChannel.cs
- 新 StreamActorOutcomeChannel.cs(IStreamProvider 后端,multi-silo OK)
- DefaultCommandOutcomeDispatchService subscribe 先,然后 dispatch,await outcome(支持 dispatch 异常 fallthrough)
- 约束 outcome dispatch 只接受 protobuf 消息
- NyxIdChatGAgent registration failure 时发 unavailable outcome 后**不 rethrow**(endpoint 基于 outcome map 503)
- 删 DELETE endpoint inline fake dispatch helper;tests 走真 DefaultCommandDispatchService → ActorCommandTargetDispatcher → NyxIdChatGAgent.HandleDeleteConversationAsync
- 加 endpoint 503 assertion(真 outcome 路径)

41/41 + 72/72 PASS,test_stability_guards PASS。

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

loning commented May 23, 2026

PR #874 merge-conflict investigation complete.\n\nResult: escalate, not mechanically resolvable. The conflict is between iter43 actor-owned StreamingProxy room-chat orchestration and newer base changes centered on StreamingProxyChatLifecycleFacade / lifecycle endpoint composition. No business conflict files were resolved; the test merge was aborted. CI checks are still absent (statusCheckRollup is empty).\n\nDetails: https://github.com/aevatarAI/aevatar/blob/4004674cac4018479d01d602926b65751cadb463/.refactor-loop/runs/investigation-pr874-merge-conflict.md

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 状态 — 已收到 maintainer 评论(daemon 识别)

维度
触发评论 id=4525184334 author=loning
评论摘要 PR #874 merge-conflict investigation complete.\n\nResult: escalate, not mechanic
daemon 反应 👀 eyes react 已加
下一步 controller 下次 wakeup(≤25 min)读 daemon log → 派 fresh codex round(maintainer-reply-resets-the-round)→ 更新本卡片
是否需要人介入 ❌ 否(自动响应中)

🤖 comment-monitor.sh daemon

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

Reflection — PR #874 after #885/#893

Date: 2026-05-23

Scope

GitHub State Observed

What #874 Was Trying To Fix

#865 identified that StreamingProxy room chat facts were being advanced outside the room actor:

  • endpoint/runtime code fetched room actors and entered chat callbacks;
  • coordinator code admitted participants, generated replies, removed failed participants, and dispatched room events;
  • participant state was also written through a side-store/read surface;
  • terminal state was assembled outside a single actor-owned command/event progression.

#874's valuable direction was: StreamingProxyGAgent owns participant admission, reply/message events, leave/failure decisions, and terminal-state publication; the coordinator becomes a Nyx external adapter.

What #885 Already Supersedes

#885 supersedes the endpoint/lifecycle part of #874:

  • StreamingProxyEndpoints now goes through StreamingProxyChatLifecycleFacade.RunChatAsync(...) instead of owning HTTP callback orchestration directly.
  • StreamingProxyChatLifecycleFacade submits a typed StreamingProxyRoomChatCommand through the interaction pipeline.
  • StreamingProxyGAgent persists lifecycle accepted and terminal-state events.
  • CQRS outcome plumbing now has StreamActorOutcomeChannel / typed dispatch outcome support.

That means #874's endpoint rewrite and much of its command/interaction scaffolding should not be manually merged. It would regress or duplicate the #885 surface.

What #893 Covers

#893 covers the participant authority / dual-store part of #874:

  • deletes StreamingProxyParticipantGAgent;
  • deletes IStreamingProxyParticipantStore and ActorBackedStreamingProxyParticipantStore;
  • moves participant query through room current-state projection;
  • keeps StreamingProxyGAgent per room as participant authority.

This supersedes #874's side-store/readmodel migration direction, but #893 is not merged yet at inspection time.

Remaining Gap

There is one real #874 idea not cleanly covered by #885 + #893 as currently observed:

That is a legitimate follow-up concern, but it should be redesigned as a small patch on top of #885 and the #893 participant-authority surface. It should not be rescued by manually merging #874.

Decision

Choose C: Drop + redesign.

Rationale:

META_RESOLVED:drop-redesign:#874 is stale after #885 and #893 direction; do not manually merge, redesign only the remaining coordinator-adapter gap on top of the new facade/room-authority surface

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

✅ Closed as superseded (per reflector META_RESOLVED:drop-redesign)

#885 实现了 chat lifecycle facade + StreamActorOutcomeChannel(merged)。#893 实现了 room actor 单 authority + actor-owned idempotent join(待 merge)。两者共同覆盖了 #874 的主要范围。

唯一未覆盖的窄缝:StreamingProxyNyxParticipantCoordinator 改 adapter-only,把 room join/message/leave/terminal event ownership 完全移入 room actor。这是 follow-up 单独 issue,不是 stale conflict 强 merge。

详见 reflection artifact 和 #893

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 23, 2026

📊 codex 进展 reflect-pr874 (⏳ 进行中; 已跑 7 min)

+
+## Decision
+
+Choose C: **Drop + redesign**.
+
+Rationale:
+
+- Not A: #885 clearly supersedes the endpoint lifecycle work, and #893 targets the room actor single-authority work, but #893 is not merged yet and the coordinator-dispatch gap remains observable.
+- Not B: manual conflict resolution would require merging stale #874 abstractions (`StreamingProxyRoomCredentialHandles`, direct actor-owned command path, old tests) with #885's typed facade/outcome pipeline and #893's participant authority rewrite. That is high-risk and likely to reintroduce duplicate lifecycle surfaces.
+- C is the clean path: close #874 as stale/wontfix, let #885 remain the lifecycle base, let #893 own the participant authority cleanup, and open a new narrow issue only for the uncovered coordinator boundary: make `StreamingProxyNyxParticipantCoordinator` adapter-only and have the room actor own join/message/leave/terminal event persistence.
+
+META_RESOLVED:drop-redesign:#874 is stale after #885 and #893 direction; do not manually merge, redesign only the remaining coordinator-adapter gap on top of the new facade/room-authority surface
+
+⟦AI:AUTO-LOOP⟧

tokens used
69,869
Decision: **C. Drop + redesign**.

I wrote the reflection artifact at [.refactor-loop/runs/reflect-pr874.md](/Users/auric/aevatar/.refactor-loop/runs/reflect-pr874.md) and posted it to PR #874:

https://github.com/aevatarAI/aevatar/pull/874#issuecomment-4525192357

Key rationale: #885 supersedes #874’s endpoint/lifecycle rewrite, and #893 covers the participant-authority direction, but #893 is still open in GitHub at inspection time. The only real remaining #874 idea is narrower: make `StreamingProxyNyxParticipantCoordinator` adapter-only and move room join/message/leave/terminal event ownership fully into the room actor. That should be a new targeted issue, not a manual merge of stale #874 conflicts.

自动更新每 10 分钟;edit-in-place 不堆评论;codex 完成后此 comment 自动删除(per Auric "完成后删掉就好了 否则太占空间")。
🤖 controller progress reporter

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