Skip to content

Split ChannelRuntime megamodule into 8 packages (#263)#451

Open
eanzhao wants to merge 5 commits intodevfrom
feat/2026-04-27_issue-263-channelruntime-split
Open

Split ChannelRuntime megamodule into 8 packages (#263)#451
eanzhao wants to merge 5 commits intodevfrom
feat/2026-04-27_issue-263-channelruntime-split

Conversation

@eanzhao
Copy link
Copy Markdown
Contributor

@eanzhao eanzhao commented Apr 27, 2026

Summary

Physically delete agents/Aevatar.GAgents.ChannelRuntime/ and redistribute its 92 .cs + 1 .proto across 8 packages along clean architectural lines. Closes #263.

Destination map

Package Role
agents/Aevatar.GAgents.Channel.Runtime (extended) ChannelBotRegistration projection + conversation pipeline middleware + tombstone compaction (plugin via ITombstoneCompactionTarget) + generic streaming sink + InboundMessage
agents/channels/Aevatar.GAgents.Channel.NyxIdRelay (extended) Nyx provisioning + scope resolver + callback endpoints + platform reply service + IPlatformAdapter + scope backfill + outbound dispatcher
agents/Aevatar.GAgents.NyxidChat (extended) ChannelLlmReplyInboxRuntime / ChannelConversationTurnRunner / NyxIdConversationReplyGenerator — landed here (not in the relay channel package) to avoid the NyxidChat ↔ NyxIdRelay dep cycle
agents/platforms/Aevatar.GAgents.Platform.Lark (extended) Lark inbox runtime + conversation targets + error codes + proxy response + host defaults
agents/Aevatar.GAgents.Authoring (new) AgentBuilder card flow + templates + FeishuCardHumanInteractionPort + NyxRelayAgentBuilderFlow (Lark-specific single package per RFC §9.4 option a)
agents/Aevatar.GAgents.Scheduled (new) SkillRunnerGAgent / WorkflowAgentGAgent / UserAgentCatalog* (#260) / ChannelScheduleCalculator/Runner / ChannelWorkflowTextRouting
agents/Aevatar.GAgents.Device (new) DeviceRegistration projection + DeviceEventEndpoints
src/Aevatar.AI.ToolProviders.ChannelAdmin (new) ChannelRegistrationTool / Source — operates on ChannelBotRegistration*
src/Aevatar.AI.ToolProviders.AgentCatalog (new) AgentDeliveryTargetTool / Source — operates on UserAgentCatalog*

Architectural decisions

  • Tools follow their data model, not their callerChannelRegistrationTool and AgentDeliveryTargetTool operate on channel bot registration / user agent catalog state respectively. They land in src/Aevatar.AI.ToolProviders.* (matching repo convention) so Channel.Runtime / Scheduled only take a dependency on Aevatar.AI.Abstractions (middleware interface contracts), not on AI.Core or any AI.ToolProviders.* implementation.
  • Authoring package is named honestly (RFC §9.4 option a) — package is Aevatar.GAgents.Authoring.Lark, registration method is AddLarkAgentAuthoring(). Reflects the Lark-specific reality (FeishuCardHumanInteractionPort, AgentBuilderCardFlow hard-coded p2p / card_action semantics, Platform.Lark dep); deferring an Authoring.Abstractions split until a second channel needs authoring.
  • Tombstone compaction is plugin-basedChannelRuntimeTombstoneCompactor iterates IEnumerable<ITombstoneCompactionTarget>; each owning package (Channel.Runtime, Device, Scheduled) registers its own target.
  • Forward-looking proto stubs deleted — removed three messages from conversation_events.proto that were [Channel RFC] Runtime package (grains / middleware / durable inbox integration) #258 architectural scaffolding (transport_binding-shaped ChannelBotRegistrationEntry / UserAgentCatalogEntry / DeviceRegistrationEntry) that conflicted name-for-name with the legacy production schemas now in channel_bot_registration.proto etc. Stubs had no production callers.

Build slice

  • New aevatar.agents.slnf for Authoring / Scheduled / Device (registered in solution_split_guards.sh).
  • ToolProviders.ChannelAdmin / ToolProviders.AgentCatalog join aevatar.ai.slnf.
  • aevatar.channels.slnf and aevatar.platforms.slnf already cover Channel.Runtime / channels/* / platforms/*.

DI

Old 280-line monolithic AddChannelRuntime split into per-package SCEs:

  • AddChannelRuntime(IConfiguration?) (extended)
  • AddDeviceRegistration(IConfiguration?), AddScheduledAgents(IConfiguration?) (new)
  • AddAgentAuthoring() (new)
  • AddNyxIdRelayChannel() (new)
  • AddLarkPlatform() (new), AddTelegramPlatform() (new — Telegram registration was inline in the old SCE; gave it a proper DI surface)
  • AddChannelAdminTools(), AddAgentCatalogTools() (new)

Mainnet host wiring composes them in order.

Test plan

  • dotnet build aevatar.slnx0 errors
  • dotnet test test/Aevatar.GAgents.ChannelRuntime.Tests465/465 pass
  • Full dotnet test aevatar.slnx — only 2 pre-existing failures (MainnetHealthEndpointsTests / MainnetHostCompositionTests BindAsync method found on IStudioMemberService with incorrect format — verified to also fail on origin/dev unchanged; not introduced by this PR)
  • bash tools/ci/architecture_guards.sh — all guards pass; playground_asset_drift_guard skipped (local pnpm/tsc env)
  • bash tools/ci/solution_split_guards.sh — all 9 slnf slices build clean

Deferred follow-ups

  1. Test directory splittest/Aevatar.GAgents.ChannelRuntime.Tests/'s 53 test files currently stay together (referenced by all 8 new packages, runs as integration surface). Issue acceptance asks to split them to per-package *.Tests/ — non-blocking for the architectural split.
  2. IPlatformAdapter cleanup — the legacy interface still has runtime field references in ChannelConversationTurnRunner (no production impl, only test stubs). Pure dead-code removal, separate PR.

References

  • RFC §9.1 — final package shape
  • RFC §9.1.1 — Build slice ownership
  • RFC §9.4 — Authoring's Lark-specific reality

🤖 Generated with Claude Code

Physically delete agents/Aevatar.GAgents.ChannelRuntime/ and redistribute
its 92 .cs files + proto messages across 8 packages along clean
architectural lines:

- Aevatar.GAgents.Channel.Runtime: ChannelBotRegistration projection
  pipeline, conversation pipeline middleware, tombstone compaction with
  pluggable ITombstoneCompactionTarget, channel-agnostic streaming sink
  and inbound-message types.
- agents/channels/Aevatar.GAgents.Channel.NyxIdRelay: NyxId-relay-specific
  files (provisioning services, callback endpoints, scope resolver,
  ownership verifier, scope backfill, platform reply service, IPlatformAdapter,
  outbound dispatcher).
- Aevatar.GAgents.NyxidChat: Nyx-specific runtime impls
  (ChannelLlmReplyInboxRuntime, ChannelConversationTurnRunner,
  NyxIdConversationReplyGenerator) -- avoids the NyxidChat <-> NyxIdRelay
  circular that would form if these landed in the relay channel package.
- agents/platforms/Aevatar.GAgents.Platform.Lark: Lark-specific helpers
  (LarkConversationInboxRuntime, conversation targets, error codes, host
  defaults, proxy response).
- Aevatar.GAgents.Authoring (new): card-driven agent builder flow
  (AgentBuilderCardFlow, NyxRelayAgentBuilderFlow, AgentBuilderTool,
  FeishuCardHumanInteractionPort) -- Lark-specific per RFC §9.4.
- Aevatar.GAgents.Scheduled (new): SkillRunnerGAgent, WorkflowAgentGAgent,
  UserAgentCatalog state/projector/query, channel schedule + workflow text
  routing.
- Aevatar.GAgents.Device (new): Device registration projection pipeline
  + DeviceEventEndpoints.
- src/Aevatar.AI.ToolProviders.{ChannelAdmin,AgentCatalog} (new):
  IAgentTool surfaces (ChannelRegistrationTool, AgentDeliveryTargetTool)
  pulled out of the runtime so Channel.Runtime / Scheduled stay AI-clean.

ServiceCollectionExtensions split along the same lines; Mainnet host
composes per-package AddXxx() calls. Single channel_runtime_messages.proto
split into channel_bot_registration / device_registration /
user_agent_catalog / skill_runner / workflow_agent under their owning
packages. Removed three forward-looking stub messages from
conversation_events.proto that were #258 scaffolding superseded by the
legacy production schemas now in channel_bot_registration.proto.

Build slice: aevatar.agents.slnf created for new agent packages, added
to solution_split_guards. ToolProviders additions go in aevatar.ai.slnf.
CI guards (channel_card_literal_guard, channel_relay_nyx_chat_direct_create_guard,
channel_tombstone_proto_field_guard) updated to new paths.

Tests: ChannelRuntime.Tests stays as the integration test surface and
references all 8 new packages; 465/465 pass. The two pre-existing Mainnet
hosting test failures (BindAsync on IStudioMemberService) reproduce on
origin/dev unchanged.

Closes #263.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 674efdd6e6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

builder.Services.AddScheduledAgents(builder.Configuration);
builder.Services.AddAgentAuthoring();
builder.Services.AddNyxIdRelayChannel();
builder.Services.AddLarkPlatform();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Register Telegram platform services in mainnet composition

After splitting the monolithic AddChannelRuntime, the mainnet host now wires AddLarkPlatform() but never wires AddTelegramPlatform(). NyxIdRelayOutboundPort resolves IMessageComposer/IChannelNativeMessageProducer by platform, so Telegram outbound paths now hit runtime composer_not_found/missing producer behavior for Telegram registrations even though Telegram tools are enabled. Add Telegram platform DI registration in this composition path to preserve the pre-split runtime behavior.

Useful? React with 👍 / 👎.

@@ -0,0 +1,116 @@
syntax = "proto3";

package aevatar.gagents.channel.runtime;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep legacy Any type URLs for channel bot contracts

This package rename changes protobuf full names from the legacy aevatar.gagents.channelruntime.* namespace to aevatar.gagents.channel.runtime.*, which changes Any.TypeUrl values for channel-bot payloads. Existing persisted events/snapshots with old type URLs will no longer unpack unless explicit legacy aliases are provided (the repo already uses this pattern for UserAgentCatalog), so replay/state restoration for pre-split data can silently stop matching handlers after upgrade.

Useful? React with 👍 / 👎.

@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 27, 2026

Architecture review: file split (#451)

Overall the split is well-executed — clean boundaries, correct DI inversion, and proper plugin pattern for tombstone compaction. Below are specific issues found, ordered by severity.

❌ Critical / bug

AddTelegramPlatform() created but never called in MainnetHostBuilderExtensions

TelegramPlatformServiceCollectionExtensions.AddTelegramPlatform() was extracted from the old monolithic DI into a proper surface, but it is not called from MainnetHostBuilderExtensions.AddAevatarMainnetHost(). The old inline registration is gone (deleted ServiceCollectionExtensions.cs, 280 lines), and the production host never wires AddTelegramPlatform().

If Telegram has any production traffic, TelegramMessageComposer / TelegramChannelNativeMessageProducer / TelegramPayloadRedactor are now missing from DI — the IChannelMessageComposerRegistry won’t find Telegram composers, and outbound Telegram messages will fail with "no composer found".

Even if Telegram is not currently active on mainnet, the asymmetry between Lark (called) and Telegram (not called) is a regression risk — a future activation would silently fail without CI catching it. Add AddTelegramPlatform() to the host wiring, or explicitly document why it’s deliberately excluded.

⚠️ Architecture drift

Channel.Runtime depends on Aevatar.AI.Abstractions despite being described as "AI-clean"

The csproj references Aevatar.AI.Abstractions (for ILLMCallMiddleware used by ChannelContextMiddleware). This is legitimate middleware integration, but the PR summary says "Channel.Runtime ... stay AI-clean." The package is not AI-free — it carries AI middleware contracts. Clarify whether:

  • The intent was "no AI.Core / ToolProviders" (which is true — it only depends on AI.Abstractions), or
  • The dependency itself should be inverted so Channel.Runtime defines its own middleware contract.

⚠️ Security hardening mixed with split

TryValidateOwnerIdentity added to ServiceBindingEndpoints.cs is a separate concern

The owner identity validation in src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/ServiceBindingEndpoints.cs (lines +42-49 in the diff) is a security hardening that prevents authenticated bound services from overriding tenantId/appId/namespace to values different from their claims. This is a correctness improvement but unrelated to the file split — it should ideally be a separate PR for audit trail. The change itself looks correct.

📝 Minor observations

  1. LarkConversationHostDefaults kept internal — fine, consumed within the same assembly (Lark platform package). No cross-package access needed.

  2. Test directory remains monolithictest/Aevatar.GAgents.ChannelRuntime.Tests/ now references all 8 packages. Acknowledged as deferred follow-up; acceptable for now as integration surface.

  3. Gate-path tests updatedGovernanceEndpointsTests correctly updated with the new owner identity validation test cases (WhenAuthenticatedOwnerIdentityConflictsWithClaims_ShouldReturnBadRequest, WhenAuthenticatedOwnerIdentityConflictsWithQuery_ShouldReturnBadRequest).

  4. CI guard updateschannel_card_literal_guard.sh correctly rescoped to scan agents/Aevatar.GAgents.Authoring instead of agents/Aevatar.GAgents.ChannelRuntime. solution_split_guards.sh correctly includes aevatar.agents.slnf.

  5. Proto split cleanchannel_bot_registration.proto extracted to Channel.Runtime, device_registration.proto to Device, conversation_events.proto trimmed of forward-looking stubs. ChannelRuntimeProtoTests correctly updated to use ChannelBotRegistrationReflection / UserAgentCatalogReflection.

✅ Verified correct

  • Dependency inversion: IConversationTurnRunner / IConversationReplyGenerator defined in Channel.Runtime, implemented in NyxidChat — clean.
  • NyxidChat ↔ NyxIdRelay cycle avoided: NyxIdRelay does NOT depend on NyxidChat.
  • Tombstone compaction plugin pattern: ITombstoneCompactionTarget defined in Channel.Runtime, each owning package registers its target via TryAddEnumerable — clean.
  • Tool separation: ChannelRegistrationTool / AgentDeliveryTargetTool moved to src/Aevatar.AI.ToolProviders.*, following "tools follow their data model" principle.
  • DI decomposition: Old 280-line monolithic AddChannelRuntime correctly split into per-package extensions with independent ES/InMemory store detection.

- agents/Aevatar.GAgents.ChannelRuntime/ChannelUserConfigScope.cs (added on
  dev for issue #436 / PR #438) → moved to agents/Aevatar.GAgents.Channel.Runtime/
  with namespace updated to Aevatar.GAgents.Channel.Runtime, visibility lifted
  to public so Authoring's AgentBuilderTool / AgentBuilderCardFlow can compose
  per-Lark-user scopes across the package boundary.
- src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/
  ServiceBindingEndpoints.cs: take origin/dev (PR #433 / 9e1737e / 6860b02
  refined the validation/handler-order shape).
- test/Aevatar.GAgentService.Integration.Tests/GovernanceEndpointsTests.cs:
  take origin/dev (matches the refined endpoint shape).

ChannelUserConfigScopeTests: add `using Aevatar.GAgents.Channel.Runtime;`
for the relocated type. 473/473 ChannelRuntime tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor Author

@eanzhao eanzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

架构审查补充(基于最新 head 17262f8e):拆分方向可以,但下面这些边界仍不干净。

{
ArgumentNullException.ThrowIfNull(services);

services.Replace(ServiceDescriptor.Singleton<IHumanInteractionPort, FeishuCardHumanInteractionPort>());
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不能用通用 AddAgentAuthoring() 去全局替换 IHumanInteractionPort 为 Feishu/Lark 实现。Aevatar.GAgents.Authoring 的包名和入口现在看起来是 channel-neutral,但实现与依赖已经是 Lark-specific(FeishuCardHumanInteractionPortPlatform.Lark)。RFC §9.4 也要求要么命名为 Lark-specific,要么拆 Authoring.Abstractions + Authoring.Lark。按现在这样,未来任何非 Lark host 只要组合 Authoring 就会拿到 Feishu port,边界不诚实。建议改成 Aevatar.GAgents.Authoring.Lark/AddLarkAgentAuthoring(),或抽窄通用 authoring contract 后由 Lark 包注册实现。

@@ -0,0 +1,72 @@
syntax = "proto3";

package aevatar.gagents.device;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 package rename 会改变所有 DeviceRegistration Any.TypeUrl/CLR full name(legacy aevatar.gagents.channelruntime.* -> aevatar.gagents.device.*)。仓库里已有持久 actor state/domain events/readmodel payload,升级后没有 LegacyProtoFullName/LegacyClrTypeName alias 就不能从旧事件/快照恢复。UserAgentCatalog 已做 alias,但 Device、SkillRunner、WorkflowAgent 这类拆出来的持久契约没有成套覆盖;ChannelBot 也有同类问题。这里不是单纯文件拆分,必须补兼容 alias + replay/unpack 测试,或保持 proto package/full name 不变。

return;

var actor = await _actorRuntime.GetAsync(actorId);
var actor = await _actorRuntime.GetAsync(target.ActorId);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ChannelRuntimeTombstoneCompactor 不应该解析 actor 实例后直接调用 HandleEventAsync。这次拆分把它变成跨包 compaction plugin,继续绕过 IActorDispatchPort 会把 runtime lookup 和 message delivery 重新揉在同一个服务里,也绕开了标准 inbox/dispatch 契约。ITombstoneCompactionTarget 可以暴露 actor id + command payload,但 compactor 应该通过 runtime 只做必要 lifecycle,再用 IActorDispatchPort.DispatchAsync 投递 envelope(类似 ChannelBotRegistrationStoreCommands 的方向)。

using Aevatar.AI.Abstractions.ToolProviders;
using Aevatar.AI.ToolProviders.NyxId;
using Aevatar.Foundation.Abstractions;
using Aevatar.GAgents.Channel.Runtime;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

拆到 ToolProviders 后,这个类仍然不是薄 adapter:它会启动 projection、读 state version、创建 catalog actor、手写 envelope,并直接 HandleEventAsync。这违反命令骨架/dispatch 边界,也把 projection priming 放进 tool execution 路径。建议把 upsert/delete 收敛到窄的 UserAgentCatalogCommandPort/application service,由该端口用 IActorDispatchPort 返回诚实的 accepted/observed 状态;AI tool 只负责参数映射和结果格式化。

@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 27, 2026

Architecture Review: File Split Quality

Overall the split is well-executed — clean DAG, no circular deps, namespaces consistent with directories, proto namespaces match packages. A few issues worth addressing:


1. ResolveElasticsearchEnabled copy-pasted 3x (Medium)

Identical ResolveElasticsearchEnabled + BuildElasticsearchOptions helper pair is duplicated across:

  • ChannelRuntimeServiceCollectionExtensions.cs
  • DeviceServiceCollectionExtensions.cs
  • ScheduledServiceCollectionExtensions.cs

This should be a shared internal utility (e.g. in Aevatar.CQRS.Projection.Providers.Elasticsearch.DependencyInjection or a shared internal helper). The three copies will silently diverge on the first bugfix.


2. NyxidChat.csproj duplicate Channel.Abstractions reference (Low)

Lines 15 and 23 both reference Aevatar.GAgents.Channel.Abstractions — harmless at build time but should be cleaned up to avoid confusion.


3. ITombstoneCompactionTarget lives in Channel.Runtime — cross-domain coupling (Medium)

Aevatar.GAgents.Device and Aevatar.GAgents.Scheduled both depend on Aevatar.GAgents.Channel.Runtime solely to get ITombstoneCompactionTarget. This creates an awkward coupling: "Device" and "Scheduled" are domain concepts that shouldn't need to know about "Channel" anything.

The interface is purely structural (ActorId, ProjectionKind, TargetName, CreateCommand) — it belongs in a neutral location like Aevatar.CQRS.Projection.Core.Abstractions or a thin Aevatar.GAgents.Abstractions shared package, not in Channel.Runtime.

The ChannelRuntimeTombstoneCompactor itself also lives in Channel.Runtime but processes Device and Scheduled compaction targets — this further confirms the compaction infrastructure is a cross-cutting concern, not channel-specific.


4. ChannelRuntimeTombstoneCompactor still owns all compaction (Medium)

Per above: the compactor iterates IEnumerable<ITombstoneCompactionTarget> which is good plugin design, but the orchestrator itself (ChannelRuntimeTombstoneCompactor + ChannelRuntimeTombstoneCompactionService) is still named "Channel" and still lives in Channel.Runtime. If Device and Scheduled packages are truly independent, they shouldn't rely on Channel.Runtime to drive their tombstone compaction. Either:

  • Rename/relocate to a neutral package, or
  • Give each package its own IHostedService that iterates only its own target.

5. NyxidChat and Authoring reach into Studio layers (Pre-existing, Medium)

  • Aevatar.GAgents.NyxidChatAevatar.Studio.Application + Aevatar.Studio.Infrastructure
  • Aevatar.GAgents.AuthoringAevatar.Studio.Application

This breaks strict layering (agents should depend on Abstractions/Core, not Application/Infrastructure). This is likely pre-existing from before the split, but the split was the right moment to clean it up. At minimum, extract the needed contracts to an abstractions package.


6. IConversationReplyGenerator — interface location vs impl location (Low)

IConversationReplyGenerator is defined in Channel.Runtime but its only production implementation (NyxIdConversationReplyGenerator) lives in NyxidChat. The interface is only consumed via DI replacement in NyxidChat.ServiceCollectionExtensions. This works but the dependency direction is slightly inverted — the runtime package defines an abstraction specifically for a higher-level package to override. Consider whether this belongs in Channel.Abstractions instead, matching the pattern of other strategy interfaces.


What's clean

  • Proto split is well done: each package owns its own .proto with correct csharp_namespace matching package namespace.
  • DI SCE decomposition follows the "per-package extension method" pattern consistently.
  • InternalsVisibleTo properly updated for both new test projects and legacy ChannelRuntime.Tests.
  • CI guard scripts updated to match new paths.
  • No circular dependencies.

…port boundary

Resolves the six inline review comments on PR #451 (Codex P1 + 4 from
@eanzhao + the proto-compat regression they share).

### 1. Re-add AddTelegramPlatform() in Mainnet host (Codex P1)

Original split moved the Telegram inline registration out of the legacy
SCE but the merge with origin/dev dropped the host-side wire-up. Add
AddTelegramPlatform() to MainnetHostBuilderExtensions so
NyxIdRelayOutboundPort can resolve IMessageComposer /
IChannelNativeMessageProducer for Telegram-platform replies.

### 2. Legacy proto / CLR aliases for renamed packages (Codex P1 + @eanzhao MED)

Splitting the proto package + csharp_namespace would break Any.TypeUrl
unpack and CLR full-name resolution against pre-split persisted
events / snapshots. Add [LegacyProtoFullName] / [LegacyClrTypeName]
partial-class aliases (mirroring UserAgentCatalogLegacyAliases from #260)
for every persisted type rolled into a new namespace:

- ChannelBotRegistrationLegacyAliases.cs (12 messages incl.
  ChannelInboundEvent)
- DeviceRegistrationLegacyAliases.cs (9 messages)
- SkillRunnerLegacyAliases.cs (12 messages)
- WorkflowAgentLegacyAliases.cs (11 messages)
- UserAgentCatalogLegacyAliases.cs extended with
  UserAgentCatalogNyxCredentialDocument

### 3. Rename Authoring → Authoring.Lark (@eanzhao HIGH)

Package and SCE method were channel-neutral on the surface but the
implementation (FeishuCardHumanInteractionPort, AgentBuilderCardFlow's
hard-coded Lark p2p / card_action semantics, Platform.Lark dependency)
is Lark-specific. Renamed:

- agents/Aevatar.GAgents.Authoring → agents/Aevatar.GAgents.Authoring.Lark
- Csproj + namespace + slnx + slnf paths
- AddAgentAuthoring() → AddLarkAgentAuthoring()
- channel_card_literal_guard scan_project entry

Reflects RFC §9.4 option (a): "Authoring is currently Lark-only — name
it that way; defer Authoring.Abstractions split until a second
channel needs authoring."

### 4. Tombstone compactor: dispatch via IActorDispatchPort (@eanzhao HIGH)

ChannelRuntimeTombstoneCompactor previously called actor.HandleEventAsync
directly after IActorRuntime.GetAsync, mixing runtime lookup and
delivery and bypassing the standard inbox/dispatch contract. Refactored:

- ITombstoneCompactionTarget gains EnsureActorAsync(IActorRuntime, ct)
  so each plugin owns its own GAgent type lifecycle.
- ChannelRuntimeTombstoneCompactor takes IActorDispatchPort and uses
  EnvelopeRouteSemantics.CreateDirect(publisherId, target.ActorId)
  followed by dispatchPort.DispatchAsync(...). Runtime is only used
  through the target's EnsureActorAsync.
- All three target impls (ChannelBotRegistration, Device, UserAgentCatalog)
  implement EnsureActorAsync.
- Existing tombstone compactor tests updated to assert on
  IActorDispatchPort.DispatchAsync instead of IActor.HandleEventAsync.

### 5. AgentDeliveryTargetTool: thin adapter via IUserAgentCatalogCommandPort
   (@eanzhao HIGH)

Tool was orchestrating projection priming, state-version polling, actor
lifecycle, and direct envelope dispatch — way past the LLM-tool mandate
of "param mapping + result formatting". Extracted:

- IUserAgentCatalogCommandPort with UpsertAsync / TombstoneAsync
  returning honest CatalogCommandOutcome (Accepted / Observed / NotFound).
- UserAgentCatalogCommandPort implementation owns projection priming,
  envelope construction, IActorDispatchPort.DispatchAsync, and the
  state-version polling loop.
- AgentDeliveryTargetTool now: validate args, resolve current owner via
  NyxID, call commandPort, format result. ~70 lines of wait/dispatch
  logic gone from the tool.
- Tests updated to mock IUserAgentCatalogCommandPort.

473/473 ChannelRuntime.Tests pass; full slnx retains only the same two
pre-existing Mainnet hosting BindAsync failures that reproduce on
origin/dev.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 27, 2026

回复审查意见 + 6 条 inline review(commit d1249f93):

对你的 3 条

  1. AddTelegramPlatform() 已在 Mainnet host 加上(line 75)。原本 split 时已加,merge dev 时被冲突解决吃掉了,这次重新落上。
  2. ✅ PR 描述中 "stay AI-clean" 已改为 "only take a dependency on Aevatar.AI.Abstractions (middleware interface contracts), not on AI.Core or any AI.ToolProviders.* implementation" —— 描述意图准确。
  3. ServiceBindingEndpoints / GovernanceEndpointsTests 在 PR diff 与 origin/dev 完全无差异(merge 时取了 --theirs),所以这次 PR 不引入安全加固。git diff origin/dev -- src/platform/.../ServiceBindingEndpoints.cs test/.../GovernanceEndpointsTests.cs 输出为空。

对 inline review 的 6 条

  1. Codex P1: AddTelegramPlatform() — 同上 ✅
  2. Codex P1 + @eanzhao MED: proto 兼容 — 加 [LegacyProtoFullName]/[LegacyClrTypeName] aliases,覆盖 ChannelBotRegistration(含 ChannelInboundEvent)、DeviceRegistration、SkillRunner、WorkflowAgent、UserAgentCatalogNyxCredentialDocument。所有持久 Any.TypeUrl 与 actor state CLR full name 走老路径仍可解。
  3. @eanzhao HIGH: Authoring 命名 — 重命名为 Aevatar.GAgents.Authoring.Lark,方法 → AddLarkAgentAuthoring()。RFC §9.4 option (a)。slnf / slnx / Mainnet host / channel_card_literal_guard 同步更新。
  4. @eanzhao HIGH: 压缩器跨包仍直 HandleEventAsync — 重构 ChannelRuntimeTombstoneCompactorIActorDispatchPort.DispatchAsync,runtime 只做 EnsureActorAsyncITombstoneCompactionTargetEnsureActorAsync(IActorRuntime, ct) 让每个 plugin 拥有自己的 GAgent 类型。
  5. @eanzhao HIGH: AgentDeliveryTargetTool 不是薄 adapter — 抽 IUserAgentCatalogCommandPort (Scheduled package),把 projection priming + envelope 构造 + dispatch + state-version polling 全部内化。Tool 现在只做 param 校验、当前 owner 解析、调 commandPort、formatJSON。返回值通过 CatalogCommandOutcome.{Accepted,Observed,NotFound} 表达诚实状态。

测试:473/473 ChannelRuntime.Tests 通过;全 slnx 仅剩 dev 上预先存在的 2 个 Mainnet hosting BindAsync on IStudioMemberService 失败,本 PR 与之无关。

Origin/dev gained Studio catalog ETag + Daily pipeline doc commits since
the last merge. The only structural conflict was the directory-rename
split warning git emitted because origin/dev still tries to add
`ChannelUserConfigScope.cs` to the deleted `agents/Aevatar.GAgents.ChannelRuntime/`
path; the file already lives at `agents/Aevatar.GAgents.Channel.Runtime/`
with the namespace bumped to `Aevatar.GAgents.Channel.Runtime` and
visibility lifted to `public` (Authoring.Lark needs cross-package
access). Dropped the duplicate in the legacy path and kept the
authoritative copy at the new location.

ChannelUserConfigScopeTests gets the same `using Aevatar.GAgents.Channel.Runtime;`
import retained from the prior merge.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

❌ Patch coverage is 87.23404% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.97%. Comparing base (7c89f22) to head (19aa627).

Files with missing lines Patch % Lines
...lProviders.AgentCatalog/AgentDeliveryTargetTool.cs 78.57% 3 Missing and 3 partials ⚠️
@@            Coverage Diff             @@
##              dev     #451      +/-   ##
==========================================
+ Coverage   70.90%   70.97%   +0.07%     
==========================================
  Files        1209     1215       +6     
  Lines       87045    87735     +690     
  Branches    11411    11504      +93     
==========================================
+ Hits        61722    62273     +551     
- Misses      20876    20961      +85     
- Partials     4447     4501      +54     
Flag Coverage Δ
ci 70.97% <87.23%> (+0.07%) ⬆️

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

Files with missing lines Coverage Δ
...ders.AgentCatalog/AgentDeliveryTargetToolSource.cs 100.00% <ø> (ø)
...viders.AgentCatalog/ServiceCollectionExtensions.cs 100.00% <100.00%> (ø)
...lProviders.ChannelAdmin/ChannelRegistrationTool.cs 83.58% <ø> (ø)
...ders.ChannelAdmin/ChannelRegistrationToolSource.cs 50.00% <ø> (ø)
...viders.ChannelAdmin/ServiceCollectionExtensions.cs 100.00% <100.00%> (ø)
...t.Host.Api/Hosting/MainnetHostBuilderExtensions.cs 94.44% <100.00%> (+0.79%) ⬆️
...o.Projection/Orchestration/StudioProjectionPort.cs 76.47% <ø> (ø)
...lProviders.AgentCatalog/AgentDeliveryTargetTool.cs 73.96% <78.57%> (ø)

... and 1 file 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.

…elper

Resolves three followup architecture-review points on PR #451:

### Channel.Runtime drops AI / Workflow direct deps (review #2)

`Channel.Runtime`'s csproj used to pull in `Aevatar.AI.Abstractions` and
`Aevatar.Workflow.Application.Abstractions` because of two files that
straddled the channel/AI and channel/workflow boundary:

- `ChannelContextMiddleware` (an `ILLMCallMiddleware` impl) — moved to
  `Aevatar.GAgents.NyxidChat`, which is the only package that needs it
  and already references `AI.Abstractions`. NyxidChat SCE registers it
  for the LLM call pipeline; Channel.Runtime SCE no longer touches
  `ILLMCallMiddleware`.
- `ChannelCardActionRouting` (builds `WorkflowResumeCommand`) — moved
  to `Aevatar.GAgents.NyxidChat` for the same reason. Its sole consumer
  (`ChannelConversationTurnRunner`) lives there too.

`Channel.Runtime.csproj` now references only `Channel.Abstractions`,
`Foundation.Abstractions`/`Core`, and the `CQRS.Projection.*` slice —
matching the "channel-agnostic flow + projection infrastructure"
charter from the RFC. Tests (`ChannelCardActionRoutingTests`) get the
extra `using Aevatar.GAgents.NyxidChat;`.

### Extract Elasticsearch projection-store toggle helper (review #4)

The `ResolveElasticsearchEnabled` + `BuildElasticsearchOptions` helper
pair was duplicated three times (Channel.Runtime / Device / Scheduled
SCEs) with slightly different log strings and `Console.Error.WriteLine`
output. Centralized into
`Aevatar.CQRS.Projection.Providers.Elasticsearch.DependencyInjection.ElasticsearchProjectionConfiguration`
with two static helpers:

- `IsEnabled(IConfiguration?, ILogger?, string? storeName)` — explicit
  flag → endpoints presence → false; logs a structured warning via
  `ILogger` (when supplied) instead of `Console.Error.WriteLine`.
- `BindOptions(IConfiguration)` — typed binder for
  `ElasticsearchProjectionDocumentStoreOptions`.

All three SCEs now call into this helper; per-package warning text is
parameterized via `storeName`. Section path
(`Projection:Document:Providers:Elasticsearch`) is exposed as a const
so future call sites stay in sync.

### Followup points acknowledged but deferred

- **Cross-package dep chain `NyxidChat → Authoring.Lark → Scheduled →
  Platform.Lark`** (review #1) — pre-existing arch debt that the split
  surfaced rather than introduced. Cleaner would be to invert via
  `IInboundFlowResolver` plug-ins so `ChannelConversationTurnRunner`
  doesn't reach into `AgentBuilderCardFlow` directly. Out of scope for
  the package split; tracking as a separate follow-up.
- **Tombstone compactor "central coordinator"** (review #3) —
  `Channel.Runtime` defines `ITombstoneCompactionTarget` but does not
  reference `Device` / `Scheduled` at the csproj level; per-package
  targets register themselves through DI. The plug-in pattern is
  intentional and keeps the DAG one-way.
- **`Scheduled` package name vs UserAgentCatalog content** (review #5)
  — `UserAgentCatalog` is the delivery-target registry that Scheduled
  agents read at execution time to route output, so co-locating it
  with `SkillRunnerGAgent` / `WorkflowAgentGAgent` is intentional.
  Renaming to `AgentCatalog` would split actors from their primary
  consumer; deferring.

473/473 ChannelRuntime.Tests pass; full slnx still only fails the same
two pre-existing Mainnet hosting `BindAsync on IStudioMemberService`
tests that reproduce on origin/dev.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 27, 2026

对补充审查的回应(commit `88a0f100`):

采纳并修复

  • Feature/cqrs projection suite #2 Channel.Runtime AI/Workflow 直依赖

    • ChannelContextMiddlewareILLMCallMiddleware 实现)和 ChannelCardActionRouting(构造 WorkflowResumeCommand)从 Channel.Runtime 挪到 NyxidChat(它们的实际使用方)。
    • Channel.Runtime.csproj 砍掉 Aevatar.AI.AbstractionsAevatar.Workflow.Application.Abstractions 引用。现在只引 Channel.Abstractions + Foundation + CQRS.Projection 系列。
    • NyxidChat SCE 接管 ILLMCallMiddleware 注册。
  • Aevatar context database #4 ES 配置 helper 重复

    • ElasticsearchProjectionConfiguration.IsEnabled / BindOptionsAevatar.CQRS.Projection.Providers.Elasticsearch.DependencyInjection
    • 三处 SCE(Channel.Runtime / Device / Scheduled)调用统一 helper,storeName 参数化警告文案。
    • Console.Error.WriteLine 改为 ILogger.LogWarning

保留并说明

  • Refactor/project namespace #1 跨包依赖链 NyxidChat → Authoring.Lark → Scheduled → Platform.Lark — 是 split 揭示的预先架构债,并非本 PR 引入。ChannelConversationTurnRunner 直接 call 具体 builder flow 的解耦(改为 IInboundFlowResolver plug-in)是更大重构,已在跟踪列表里。

  • MAF-Inspired Framework Improvements #3 Tombstone compactor 中心协调 — Channel.Runtime 在 csproj 层 引用 Device/Scheduled,只声明 ITombstoneCompactionTarget 接口;Device/Scheduled 通过 DI 单向注册各自实现。这就是 plug-in 模式,DAG 单向,新增 projection 包确实需要 DI 注册一行 target,但这是契约清晰的代价,比每个包各自跑独立 hosted service 更可调度。

  • Refactor workflow execution lifecycle and align MessageId semantics #5 Scheduled 包名 vs UserAgentCatalog — UserAgentCatalog 本质是"调度执行时读哪个交付目标"的目录,跟 SkillRunnerGAgent/WorkflowAgentGAgent 共生。拆开会让 SkillRunner 跨包引用其状态依赖,得不偿失。Authoring 包名已改 Authoring.Lark 处理诚实化部分。

  • Refactor/core cqrs parallel subsystems #6 Proto 拆分 — 已正确,无 action(你也确认了)。

验证:473/473 ChannelRuntime.Tests 通过;全 slnx 只剩 dev pre-existing 的 2 个 Mainnet hosting BindAsync 失败。

@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 27, 2026

Fix review: all issues resolved ✅

已验证修复

问题 状态
AddTelegramPlatform() 未调用 ✅ 已在 MainnetHostBuilderExtensions:75 处添加
Channel.Runtime 依赖 AI.Abstractions ✅ csproj 已清理——现在仅依赖 Channel.Abstractions / Foundation.* / CQRS.Projection.*
ChannelContextMiddleware 跨越 AI/channel 边界 ✅ 已移至 NyxidChat,通过 ILLMCallMiddleware 注册(路径:ServiceCollectionExtensions.cs:46
ChannelCardActionRouting 跨越 workflow/channel 边界 ✅ 已移至 NyxidChat
ES 开关辅助方法重复三次 ✅ 已集中至 ElasticsearchProjectionConfiguration.IsEnabled() / BindOptions()Console.Error.WriteLine 替换为结构化 ILogger
Authoring 含义模糊 ✅ 已更名为 Authoring.Lark,连同命名空间、csproj、DI 方法(AddLarkAgentAuthoring)及文档一起更新

确认无退化

  • Channel.Runtime.csproj 依赖列表现在与 RFC 完全一致:Channel.AbstractionsFoundation.Abstractions/Core、仅 CQRS.Projection.*
  • AddChannelRuntime SCE 不再导入 Aevatar.AI.Abstractions.Middleware
  • 三个投影 SCE 均使用共享的 ES 辅助方法,并传入不同的 storeName 参数以用于日志上下文
  • NyxidChat SCE 将所有跨领域中间件(ChannelContextMiddlewareChannelConversationTurnRunnerNyxIdConversationReplyGenerator)注册到一个位置

已确认推迟

  • NyxidChat → Authoring.Lark → Scheduled → Platform.Lark 依赖链——拆分前即存在的原有架构债务,不在本次文件拆分的讨论范围内
  • tombstone compactor 的“中心协调器”模式——插件式设计是有意为之;Channel.Runtime 不在 csproj 层面引用 Device / Scheduled

所有审查项均已处理。该 PR 已经可以合并。

@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 27, 2026

Follow-up Review: Fix Commits (d1249f988a0f10)

Fixed ✅

ES helper deduplicationElasticsearchProjectionConfiguration.IsEnabled / BindOptions extracted to CQRS.Projection.Providers.Elasticsearch.DependencyInjection. All three SCEs call the shared helper. Console.Error.WriteLineILogger.LogWarning. Clean.

Channel.Runtime layer tighteningChannelContextMiddleware (ILLMCallMiddleware) and ChannelCardActionRouting (WorkflowResumeCommand builder) moved from Channel.Runtime → NyxidChat. Channel.Runtime.csproj dropped Aevatar.AI.Abstractions and Aevatar.Workflow.Application.Abstractions references. Channel.Runtime is now projection + middleware-infrastructure only. Good.

Still outstanding

NyxidChat.csproj duplicate Channel.Abstractions reference (lines 15 & 23) — minor, but still there. One-line cleanup.

Everything else from the previous review is either properly fixed or acknowledged with valid rationale (tombstone plugin design, Scheduled naming, pre-existing Studio deps). Ship-ready given the remaining item is trivial.

Copy link
Copy Markdown
Contributor Author

@eanzhao eanzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed latest fix commit 88a0f100. Most previous split issues are fixed; one responsibility-boundary issue remains in the Lark authoring package.

using Google.Protobuf.WellKnownTypes;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里把包名改成 Authoring.Lark 是对的,但这个 adapter 里仍然保留了核心编排:AgentBuilderTool 直接从 DI 拿 IActorRuntime,随后在 create/run/disable/enable/delete 路径里直接 CreateAsync + HandleEventAsyncSkillRunnerGAgent / WorkflowAgentGAgent / UserAgentCatalogGAgent,并且自己做 EnsureUserAgentCatalogProjectionAsync + polling。按当前分层,LLM/tool 层应该只做参数映射和 Nyx/Lark adapter 交互;scheduled agent lifecycle、catalog tombstone、projection priming/观察这些应收敛到 Scheduled application command port(类似这次新增的 IUserAgentCatalogCommandPort,或一个专门的 agent lifecycle command port)并通过 IActorDispatchPort 投递。否则文件虽然拆了,核心业务编排仍留在 Lark authoring adapter 里,边界没有真正清掉。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Channel RFC] Package split (Authoring / Scheduled / Device split from ChannelRuntime)

1 participant