Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 398f7c734a
ℹ️ 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".
| channelAgentKey = await agentKeyProvisioning.ProvisionAsync( | ||
| bot.Platform, | ||
| accessToken, | ||
| relayCallbackUrl, | ||
| scopeResolution.ScopeId!, |
There was a problem hiding this comment.
Move owner resolution, API-key provisioning, grant mutation, route selection/rebinding, rollback, and command construction into an application-layer facade. Implementing the saga directly in HandleRegisterAsync makes this API endpoint the owner of core business orchestration and allows other hosts to bypass or duplicate its partial-failure semantics, contrary to the repository's highest-priority layering rule.
AGENTS.md reference: AGENTS.md:L4-L4
Useful? React with 👍 / 👎.
| var existing = (await queryPort.QueryAllSnapshotsAsync(ct)) | ||
| .Select(static snapshot => snapshot.Registration) | ||
| .FirstOrDefault(registration => !registration.Tombstoned && | ||
| string.Equals(registration.NyxChannelBotId, nyxChannelBotId, StringComparison.Ordinal)); |
There was a problem hiding this comment.
The global read-model lookup runs before GetChannelBotAsync verifies that the caller's token can access this Nyx bot, and the conflict response exposes the owning registration ID. Consequently, a caller who guesses another tenant's bot ID can distinguish whether it is bound and learn its local registration ID; perform the caller-scoped Nyx lookup first and only return binding details after ownership is established.
Useful? React with 👍 / 👎.
| if (channelAgentKey is not null) | ||
| { | ||
| using var cleanup = new CancellationTokenSource(TimeSpan.FromSeconds(10)); | ||
| await agentKeyProvisioning.CleanupAsync(accessToken, channelAgentKey, registrationId, cleanup.Token); | ||
| } |
There was a problem hiding this comment.
If BindConversationRouteAsync creates a route or replaces an existing route's agent key and any later operation fails, this cleanup deletes only the newly provisioned API key. The Nyx bot is then left with a route pointing at a deleted key, taking an existing bot offline; rollback must delete a newly created route or restore the prior route binding before removing the key.
AGENTS.md reference: AGENTS.md:L78-L78
Useful? React with 👍 / 👎.
| nyx_conversation_route_id = routeId, | ||
| relay_callback_url = relayCallbackUrl, | ||
| webhook_url = bot.WebhookUrl, | ||
| workflow_result_delivery_status = "enabled", |
There was a problem hiding this comment.
The receipt only means the command was accepted for dispatch, yet the response claims workflow delivery is already enabled. For example, a caller-supplied registration_id can collide with an existing active registration—the actor rejects that command asynchronously, while this endpoint still returns 202 and leaves the adopted bot route bound to an untracked key. Return only dispatch-acceptance semantics here, or observe a committed/rejected registration outcome before asserting enablement.
AGENTS.md reference: AGENTS.md:L66-L66
Useful? React with 👍 / 👎.
| updatedAgentKey = await UpdateAgentKeyGrantForSelectionAsync( | ||
| nyxClient, | ||
| accessToken, | ||
| registration.ChannelAgentKey, | ||
| effectiveServiceSelection, |
There was a problem hiding this comment.
When service_ids is specified, this call mutates the live Nyx API-key grant before dispatching the actor update. If dispatch throws, is rejected, or is accepted but never committed, the remote key immediately has the new permissions while the authoritative registration and read model retain the old authorization, with no rollback or reconciliation path; make this a compensating saga or defer the external mutation until the committed update is observed.
Useful? React with 👍 / 👎.
| var result = bots.Select(bot => | ||
| { | ||
| var e = snapshot.Registration; | ||
| var capabilityStatus = ChannelWorkflowResultDeliveryCapability.Resolve(e); | ||
| var repairFailed = capabilityStatus == | ||
| ChannelWorkflowResultDeliveryCapabilityStatus.RepairFailed; | ||
| return new | ||
| { | ||
| id = e.Id, | ||
| platform = e.Platform, | ||
| registration_mode = "nyx_relay_webhook", | ||
| authorization_mode = MapAuthorizationMode(e), | ||
| service_ids = MapRegistrationServiceIds(e), | ||
| state_version = snapshot.StateVersion, | ||
| nyx_provider_slug = e.NyxProviderSlug, | ||
| scope_id = e.ScopeId, | ||
| callback_url = string.Empty, | ||
| webhook_url = e.WebhookUrl, | ||
| nyx_channel_bot_id = e.NyxChannelBotId, | ||
| nyx_agent_api_key_id = e.NyxAgentApiKeyId, | ||
| nyx_conversation_route_id = e.NyxConversationRouteId, | ||
| default_skill_name = e.DefaultSkillName, | ||
| default_skill = MapDefaultSkill(e.RuntimeConfig, e.DefaultSkillName), | ||
| has_instructions = !string.IsNullOrWhiteSpace(e.RuntimeConfig?.Instructions), | ||
| has_tool_set_refs = e.RuntimeConfig?.ToolSetRefs.Count > 0, | ||
| has_extra_tool_names = e.RuntimeConfig?.ExtraToolNames.Count > 0, | ||
| nyxid_service_selectors = MapNyxIdServiceSelectors(e.RuntimeConfig), | ||
| agent_key = MapAgentKeyStatus(e), | ||
| workflow_result_delivery_status = MapCapabilityStatus(e, capabilityStatus), | ||
| workflow_result_delivery_failure_phase = repairFailed | ||
| ? MapRepairPhase(e.WorkflowResultDeliveryRepair?.FailurePhase ?? | ||
| ChannelWorkflowResultDeliveryRepairPhase.Unspecified) | ||
| : null, | ||
| workflow_result_delivery_failure_reason = repairFailed | ||
| ? MapRepairFailureReason(e.WorkflowResultDeliveryRepair?.FailureReason ?? | ||
| ChannelWorkflowResultDeliveryRepairFailureReason.Unspecified) | ||
| : null, | ||
| // Whether this bot belongs to the caller's account. Cross-account bots | ||
| // (admin all-view) cannot have their live status read from NyxID. | ||
| owned = string.Equals(e.ScopeId, callerScope, StringComparison.Ordinal), | ||
| }; | ||
| localByBotId.TryGetValue(bot.Id, out var snapshot); | ||
| return MapRegistrationListRow(bot, snapshot, callerScope); |
There was a problem hiding this comment.
In a real multi-account scope=all request, ListChannelBotsAsync returns only bots visible to the caller's Nyx token, while foreign registrations can only be represented by the local snapshots. Because the response iterates bots rather than the visible snapshots, those foreign registrations are never emitted even though the method promises an all-account admin view; merge unmatched local snapshots into the result with degraded/unavailable live status.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Issue
Closes #3652
Scope
/api/channels/registrationssemantics so listing is sourced from NyxID channel bots and overlaid with local registration state.Verification
dotnet test test/Aevatar.GAgents.ChannelRuntime.Tests/Aevatar.GAgents.ChannelRuntime.Tests.csproj --nologo --filter ChannelCallbackEndpointsTests --no-restore /clp:ErrorsOnlypassed, 62 tests.dotnet test test/Aevatar.GAgents.ChannelRuntime.Tests/Aevatar.GAgents.ChannelRuntime.Tests.csproj --nologo --filter ChannelBotRegistration --no-restore /clp:ErrorsOnlypassed, 51 tests.dotnet test test/Aevatar.GAgents.ChannelRuntime.Tests/Aevatar.GAgents.ChannelRuntime.Tests.csproj --nologo --filter ChannelRegistrationTool --no-restore /clp:ErrorsOnlypassed, 35 tests.bash tools/ci/architecture_guards.shpassed.bash tools/ci/test_stability_guards.shpassed.dotnet test aevatar.slnx --nologo --filter ChannelBotRegistration --no-restore /clp:ErrorsOnlyblocked bysrc/Aevatar.Mainnet.Host.Api/Program.cs(2,7): error CS0246: The type or namespace name 'Serilog' could not be found; the focused ChannelRuntime filtered tests passed before the solution build reached this unrelated project error.Notes
.claude/worktrees/and{{pkgetc}}/were left untouched and not included.ChannelRegistrationToolremains on the existing Lark/Telegram provisioning tool surface; this PR keeps the backend API change scoped to the existing/api/channels/registrationsendpoints.Generated with Claude Code.