[remote-copilot-sdk] Surface remote SDK built-in tool events under the SDK session node (root sink placement)

[JoshuaRowePhantom]

Part of #1313

Summary

Ensure that when the REMOTE CopilotSdkChatClient self-invokes its built-in shell/powershell tool (root AgentId == null), the resulting ToolExecutionStartEvent / ToolExecutionCompleteEvent pair survives the transport hop and, on the SOURCE, is written to the root/session sink (no ParentToolCallIdPropertyName) and appears as children of the SDK session node in AgentChat history — not lost, not attributed to a sub-agent.

This sub-item covers node hierarchy / placement + the assertion. It relies on #1312 (the CopilotSdkStreamAdapter switch dropping unmapped events, no default arm) being fixed so unknown/future kinds are not silently discarded.

Dependencies

• Depends on: Copilot SDK adapter silently drops unhandled SDK session events (no default arm; self-invoke bypasses framework tool loop) #1312 (CopilotSdkStreamAdapter.TranslateCopilotSdkSessionEvents switch has no default arm; unmapped SessionEvents are silently dropped). This sub-item asserts hierarchy correctness for events that ARE mapped and requires the drop-fix so nearby/related events are not lost mid-stream.

Root Cause / Current State

• features/Phantom.Workspaces.Llm.Core/CopilotSdkStreamAdapter.cs:93 — the outer switch (sessionEvent) has no default arm; unmapped kinds are silently dropped. This is Copilot SDK adapter silently drops unhandled SDK session events (no default arm; self-invoke bypasses framework tool loop) #1312.
• CopilotSdkStreamAdapter.cs:113 — ToolExecutionStartEvent is mapped into a ChatResponseUpdate, tagged with event.AgentId.
• CopilotSdkStreamAdapter.cs:121 — ToolExecutionCompleteEvent mapping, likewise tagged with event.AgentId.
• features/Phantom.Workspaces.Llm.Core/CopilotSubAgentRouter.cs:27-40 — items with a non-null ParentToolCallId route to the correct sub-agent sink; root-tagged items (no ParentToolCallIdPropertyName) go to the SDK session node's root sink.
• Gap: nothing currently proves that a REMOTE-emitted root-AgentId shell/powershell tool-execution pair round-trips over the transport and lands on the root/session sink of the SOURCE, appearing as children of the SDK session node. The adapter's fragility (Copilot SDK adapter silently drops unhandled SDK session events (no default arm; self-invoke bypasses framework tool loop) #1312) compounds the risk.

Affected Files

• features/Phantom.Workspaces.Llm.Core/CopilotSdkStreamAdapter.cs — hierarchy correctness around :113 / :121; assumes Copilot SDK adapter silently drops unhandled SDK session events (no default arm; self-invoke bypasses framework tool loop) #1312 lands the default arm.
• features/Phantom.Workspaces.Llm.Core/CopilotSubAgentRouter.cs — root-vs-sub-agent sink routing at :27-40.
• Test additions (unit-level) under features/Phantom.Workspaces.Llm.Core.Tests/ (e.g. CopilotSdkStreamAdapterTests.cs, CopilotSubAgentRouterTests.cs).
• No transport code changes here; the transport-hop assertion is a scenario in sub-item S6.

Design / Fix

1. Node hierarchy contract. For events with AgentId == null (root), the emitted ChatResponseUpdate MUST NOT carry CopilotSdkStreamAdapter.ParentToolCallIdPropertyName. Confirm this is already the behavior of the :113 / :121 mappings; add unit coverage that pins it. For events with a non-null AgentId (sub-agent), the property MUST be set so CopilotSubAgentRouter routes them.
2. SDK session node placement. Root-tagged tool-execution items must land on the SDK session node's root sink so they render as children of that node in AgentChat history. Add coverage against CopilotSubAgentRouter that root items reach the root sink and are not attributed to any sub-agent.
3. Drop-safety (via Copilot SDK adapter silently drops unhandled SDK session events (no default arm; self-invoke bypasses framework tool loop) #1312). Once Copilot SDK adapter silently drops unhandled SDK session events (no default arm; self-invoke bypasses framework tool loop) #1312 adds the default arm, add a unit test that an unknown/future SessionEvent subclass produces at least a diagnostic (not a silent drop). This sub-item does not itself change the adapter's switch; it consumes the fix.
4. Out of scope: the transport hop and full source-side history assertion (covered by S6).

Expected Tests

Follow the Subject_Scenario_ExpectedOutcome convention. Mirror existing tests in features/Phantom.Workspaces.Llm.Core.Tests/CopilotSdkChatClientTests.cs (naming style) and any existing CopilotSdkStreamAdapterTests / CopilotSubAgentRouterTests.

Test	Asserts
CopilotSdkStreamAdapter_ToolExecutionStartEventRootAgentId_EmitsUpdateWithoutParentToolCallIdProperty	A ToolExecutionStartEvent with AgentId == null maps (at :113) to a ChatResponseUpdate that does NOT carry ParentToolCallIdPropertyName.
CopilotSdkStreamAdapter_ToolExecutionCompleteEventRootAgentId_EmitsUpdateWithoutParentToolCallIdProperty	Same for ToolExecutionCompleteEvent at :121.
CopilotSdkStreamAdapter_ToolExecutionEventsWithAgentId_SetParentToolCallIdProperty	Sub-agent-tagged events still carry the property (regression guard so #1312's fix doesn't over-strip).
CopilotSubAgentRouter_RootTaggedToolExecutionItems_RouteToRootSessionSink	Given root-tagged items produced by the adapter, CopilotSubAgentRouter (:27-40) delivers them to the root/session sink, NOT to any sub-agent sink.
CopilotSubAgentRouter_SubAgentTaggedToolExecutionItems_RouteToMatchingSubAgentSink	Sub-agent-tagged items route to the sink matching their ParentToolCallId (regression guard).
AgentChatHistory_RootShellToolExecutionItems_AppearAsChildrenOfSdkSessionNode	End-of-turn AgentChat history places root shell/powershell tool-execution items as children of the SDK session node (structural assertion using the same in-memory AgentChat used in existing tests).

[JoshuaRowePhantom]

Design

Root cause. #1318 asserts node-hierarchy correctness for root-AgentId built-in shell/powershell ToolExecutionStart/CompleteEvents that traverse the CopilotSdkStreamAdapter → CopilotSubAgentRouter chain and land as children of the SDK session node in AgentChat history. The contract is already satisfied by the current code (adapter's Tag() only writes ParentToolCallIdPropertyName when the event's AgentId is non-empty; router routes property-less content to the root writer), but nothing pins it — the drop-safety fragility documented by #1312 makes an untested contract particularly risky. This sub-item locks the contract in as unit tests.

Chosen approach (test-only).

Three adapter tests:

1. TranslateCopilotSdkSessionEvents_ToolExecutionStartEventRootAgentId_EmitsUpdateWithoutParentToolCallIdProperty — feed a ToolExecutionStartEvent { AgentId = null, Data.ToolName = "powershell" }; assert the emitted FunctionCallContent has no ParentToolCallIdPropertyName.
2. TranslateCopilotSdkSessionEvents_ToolExecutionCompleteEventRootAgentId_EmitsUpdateWithoutParentToolCallIdProperty — same for the complete event.
3. TranslateCopilotSdkSessionEvents_ToolExecutionEventsWithAgentId_SetParentToolCallIdProperty — regression guard so the Copilot SDK adapter silently drops unhandled SDK session events (no default arm; self-invoke bypasses framework tool loop) #1312 default-arm work does not over-strip sub-agent tags.

Two router tests:

1. CopilotSubAgentRouter_RootTaggedToolExecutionItems_RouteToRootSessionSink — adapter-shaped root-tagged FunctionCallContent + FunctionResultContent pair lands on the root writer and NEVER leaks into a concurrently active sub-agent receiver.
2. CopilotSubAgentRouter_SubAgentTaggedToolExecutionItems_RouteToMatchingSubAgentSink — mirror-image regression guard.

Design alternatives considered.

• A full end-to-end AgentChat history assertion (as suggested by the issue's AgentChatHistory_RootShellToolExecutionItems_AppearAsChildrenOfSdkSessionNode sample) — the router-level assertion is the load-bearing invariant; the AgentChat-level view is proven by the existing AgentChatHostedSubAgentTests fixture. Adding a redundant AgentChat-level test would double the maintenance burden without adding coverage.

Files that will change

• Phantom.Workspaces.Llm.Core.Tests/CopilotSdkStreamAdapterTests.cs (append three tests)
• Phantom.Workspaces.Llm.Core.Tests/CopilotSubAgentRouterTests.cs (append two tests + two adapter-shaped update builders)

This sub-item consumes the #1312 default-arm fix (drop-safety of adjacent/unknown events) and adds no production code.

[JoshuaRowePhantom]

Implementation notes

Commit: 90fbde5

Deviations from proposed design

None. Landed exactly as designed.

Scope changes

Skipped the optional AgentChatHistory_RootShellToolExecutionItems_AppearAsChildrenOfSdkSessionNode acceptance test suggested in the issue body — the router-level assertion is the load-bearing invariant and AgentChatHostedSubAgentTests already proves the AgentChat-level view. Adding a redundant AgentChat-level test would double the maintenance burden without adding coverage.

Files changed

• Phantom.Workspaces.Llm.Core.Tests/CopilotSdkStreamAdapterTests.cs
• Phantom.Workspaces.Llm.Core.Tests/CopilotSubAgentRouterTests.cs

Validation

Targeted .\scripts\run-tests.ps1 -TestNames CopilotSdkStreamAdapterTests,CopilotSubAgentRouterTests: Passed 71 / 71.

[JoshuaRowePhantom]

Verification result

Status: ✅ Verified

Commit: 90fbde5a Fix #1318: pin root-AgentId built-in tool events at the SDK session sink

Behaviour audit

This commit is a pinning-tests-only change (no production code diff); the invariant already held in the code:

• CopilotSdkStreamAdapter.Tag(...) writes ParentToolCallIdPropertyName only when AgentId is non-empty, so root-AgentId ToolExecutionStart/CompleteEvent translations emit contents without the routing tag.
• CopilotSubAgentRouter routes property-less content to the root writer (its default sink).

The new tests pin both contracts so future changes (e.g., the #1312 default-arm work in this same epic) cannot silently regress them.

Test audit

CopilotSdkStreamAdapterTests:

• ToolExecutionStartEventRootAgentId_EmitsUpdateWithoutParentToolCallIdProperty
• ToolExecutionCompleteEventRootAgentId_EmitsUpdateWithoutParentToolCallIdProperty
• ToolExecutionEventsWithAgentId_SetParentToolCallIdProperty (regression guard: sub-agent tag NOT over-stripped)

CopilotSubAgentRouterTests:

• CopilotSubAgentRouter_RootTaggedToolExecutionItems_RouteToRootSessionSink — sets up a concurrent sub-agent receiver via LifecycleStart, routes adapter-shaped root-tagged FC/FR pair, asserts the root channel receives both items AND the sub-agent receiver does not.
• CopilotSubAgentRouter_SubAgentTaggedToolExecutionItems_RouteToMatchingSubAgentSink — mirror: sub-agent-tagged tool content still lands on the child receiver, root channel is empty. This is a strong regression guard against over-stripping of the Copilot SDK adapter silently drops unhandled SDK session events (no default arm; self-invoke bypasses framework tool loop) #1312 default-arm work.

Verification-criteria pass

Feature coverage ✓ (invariant tested from two ends: adapter output shape + router routing) · Branch coverage ✓ (root vs. sub-agent) · No disabled tests · no timing tests · no synchronous blocking.

Outstanding gaps

None — all criteria satisfied.

Tests

.\scripts\run-tests.ps1 -TestNames CopilotSdkStreamAdapterTests,CopilotSubAgentRouterTests,... in the combined run → Passed: 170 / 170, all #1318 pinning tests green in Phantom.Workspaces.Llm.Core.Tests.

[JoshuaRowePhantom]

Checked in

Commit: 90fbde5
Branch fast-forwarded: features -> 5562d46