Address remaining TODOs in the repo

packages/agents-hosting-dialogs/src/choices/recognizeChoices.ts

@@ -66,9 +66,7 @@ export function recognizeChoices (
   // Normalize choices
   const list: Choice[] = (choices || [])
     .map((choice) => (typeof choice === 'string' ? { value: choice } : choice))
-    .filter(
-      (choice: Choice) => choice // TODO: does this do anything?
-    )
+    .filter((choice): choice is Choice => choice !== undefined && choice !== null)
 
   // Try finding choices by text search first
   // - We only want to use a single strategy for returning results to avoid issues where utterances

packages/agents-hosting-dialogs/test/choiceRecognizer.test.ts

@@ -143,7 +143,6 @@ describe('Choices Recognizers Tests', function () {
   })
 
   describe('recognizeChoices()', function () {
-    // TODO: add recognizer test with ordinal and number options
     it('should find a choice in an utterance by name.', function () {
       const found = recognizeChoices('the red one please.', colorChoices)
       assert(found.length === 1, `Invalid token count of '${found.length}' returned.`)
@@ -212,5 +211,11 @@ describe('Choices Recognizers Tests', function () {
       })
       assert(found.length === 0, `Invalid token count of '${found.length}' returned.`)
     })
+
+    it('should ignore nullish choices from JavaScript callers.', function () {
+      const found = recognizeChoices('the green one please.', ['red', null, 'green', undefined] as any)
+      assert(found.length === 1, `Invalid token count of '${found.length}' returned.`)
+      assertChoice(found[0], 'green', 1, 1, 'green')
+    })
   })
 })

packages/agents-hosting/src/app/agentApplication.ts

@@ -672,7 +672,7 @@ export class AgentApplication<TState extends TurnState> {
 
     if (!authorized) {
       const managed = trace(AgentApplicationTraceDefinitions.run)
-      managed.record({ authorized, activity: context.activity })
+      managed.record({ authorized, activity: Activity.fromObject(context.activity) })
       managed.end()
       // We don't log a message here because it is handled by the authorization manager and could cause confusion during mid sign-in operations.
       return false
@@ -713,75 +713,81 @@ export class AgentApplication<TState extends TurnState> {
    * Executes the turn processing logic for the given context, including routing and handler execution.
    */
   private async runTurn (context: TurnContext): Promise<boolean> {
-    return trace(AgentApplicationTraceDefinitions.run, async ({ record }) => {
-      record({ authorized: true, activity: context.activity })
+    const managed = trace(AgentApplicationTraceDefinitions.run)
+    managed.record({ authorized: true, activity: Activity.fromObject(context.activity) })
 
-      try {
-        if (this._options.startTypingTimer) {
-          this.startTypingTimer(context)
-        }
+    try {
+      if (this._options.startTypingTimer) {
+        this.startTypingTimer(context)
+      }
 
-        if (this._options.removeRecipientMention && context.activity.type === ActivityTypes.Message) {
-          context.activity.removeRecipientMention()
-        }
+      if (this._options.removeRecipientMention && context.activity.type === ActivityTypes.Message) {
+        context.activity.removeRecipientMention()
+      }
 
-        if (this._options.normalizeMentions && context.activity.type === ActivityTypes.Message) {
-          context.activity.normalizeMentions()
-        }
+      if (this._options.normalizeMentions && context.activity.type === ActivityTypes.Message) {
+        context.activity.normalizeMentions()
+      }
 
-        const { storage, turnStateFactory } = this._options
-        const state = turnStateFactory()
-        await state.load(context, storage)
+      const { storage, turnStateFactory } = this._options
+      const state = turnStateFactory()
+      await state.load(context, storage)
 
-        const route = await this.getRoute(context)
+      const route = await this.getRoute(context)
 
-        record({ routeMatched: route !== undefined })
+      managed.record({ routeMatched: route !== undefined })
 
-        if (!route) {
-          logger.debug('No matching route found for activity:', context.activity)
-          return false
-        }
+      if (!route) {
+        logger.debug('No matching route found for activity:', context.activity)
+        return false
+      }
 
-        const fileDownloaders = this._options.fileDownloaders
-        if (Array.isArray(fileDownloaders) && fileDownloaders.length > 0) {
-          await trace(AgentApplicationTraceDefinitions.downloadFiles, async ({ record }) => {
-            record({ attachmentsCount: context.activity.attachments?.length })
-            for (let i = 0; i < fileDownloaders.length; i++) {
-              await fileDownloaders[i].downloadAndStoreFiles(context, state)
-            }
-          })
-        }
+      const fileDownloaders = this._options.fileDownloaders
+      if (Array.isArray(fileDownloaders) && fileDownloaders.length > 0) {
+        await managed.child(AgentApplicationTraceDefinitions.downloadFiles, async ({ record }) => {
+          record({ attachmentsCount: context.activity.attachments?.length })
+          for (let i = 0; i < fileDownloaders.length; i++) {
+            await fileDownloaders[i].downloadAndStoreFiles(context, state)
+          }
+        })
+      }
 
-        if (this._beforeTurn.length > 0) {
-          await trace(AgentApplicationTraceDefinitions.beforeTurn, async () => {
-            if (!(await this.callEventHandlers(context, state, this._beforeTurn))) {
-              await state.save(context, storage)
-              return false
-            }
-          })
-        }
+      if (this._beforeTurn.length > 0) {
+        const continueTurn = await managed.child(AgentApplicationTraceDefinitions.beforeTurn, async () => {
+          if (!(await this.callEventHandlers(context, state, this._beforeTurn))) {
+            await state.save(context, storage)
+            return false
+          }
 
-        await trace(AgentApplicationTraceDefinitions.routeHandler, async ({ record }) => {
-          record({ isInvoke: route.isInvokeRoute, isAgentic: route.isAgenticRoute })
-          await route.handler(context, state)
+          return true
         })
 
-        if (this._afterTurn.length > 0) {
-          await trace(AgentApplicationTraceDefinitions.afterTurn, async () => {
-            if (await this.callEventHandlers(context, state, this._afterTurn)) {
-              await state.save(context, storage)
-            }
-          })
+        if (!continueTurn) {
+          return false
         }
+      }
 
-        return true
-      } catch (err: any) {
-        logger.error(err)
-        throw err
-      } finally {
-        this.stopTypingTimer(context)
+      await managed.child(AgentApplicationTraceDefinitions.routeHandler, async ({ record }) => {
+        record({ isInvoke: route.isInvokeRoute, isAgentic: route.isAgenticRoute })
+        await route.handler(context, state)
+      })
+
+      if (this._afterTurn.length > 0) {
+        await managed.child(AgentApplicationTraceDefinitions.afterTurn, async () => {
+          if (await this.callEventHandlers(context, state, this._afterTurn)) {
+            await state.save(context, storage)
+          }
+        })
       }
-    })
+
+      return true
+    } catch (err: any) {
+      logger.error(err)
+      throw managed.fail(err)
+    } finally {
+      this.stopTypingTimer(context)
+      managed.end()
+    }
   }
 
   /**

packages/agents-hosting/test/hosting/app/agentApplication.test.ts

@@ -194,6 +194,21 @@ describe('Application', () => {
     assert.equal(handled, true)
   })
 
+  it('should stop processing when a beforeTurn handler returns false', async () => {
+    let routeCalled = false
+
+    app.onTurn('beforeTurn', async (_context, _state) => false)
+    app.onMessage('/yo', async (_context, _state) => {
+      routeCalled = true
+    })
+
+    const context = new TurnContext(testAdapter, testActivity)
+    const handled = await app.runInternal(context)
+
+    assert.equal(routeCalled, false)
+    assert.equal(handled, false)
+  })
+
   it('should route to a act handler with regex', async () => {
     let called = false
 

packages/agents-telemetry/README.md

@@ -183,7 +183,7 @@ For long-lived operations where you need manual control over the span lifecycle:
 ```ts
 import { SpanNames, trace } from '@microsoft/agents-telemetry'
 
-const { record, actions, end, fail } = trace({
+const { record, actions, child, end, fail } = trace({
   name: SpanNames.ADAPTER_PROCESS,
   record: { status: 'pending' },
   end: ({ span, record, duration }) => {
@@ -193,7 +193,20 @@ const { record, actions, end, fail } = trace({
 
 try {
   record({ status: 'processing' })
-  await doWork()
+  const keys = ['conversation']
+  await child(
+    {
+      name: SpanNames.STORAGE_READ,
+      record: { keys: 0 },
+      end: ({ span, record }) => {
+        span.setAttribute('storage.keys', record.keys)
+      },
+    },
+    async ({ record }) => {
+      record({ keys: 5 })
+      await storage.read(keys)
+    }
+  )
   record({ status: 'done' })
   end()
 } catch (error) {
@@ -230,6 +243,8 @@ trace(storageReadTrace, ({ record }) => {
 - Non-Error thrown values are recorded with their type name and string representation.
 - Unrecognized span names throw immediately.
 - Disabled span categories run the callback with a noop context — no telemetry is emitted.
+- Managed spans can start parented spans with `child(definition)` or `child(definition, callback)`.
+- Record updates recursively merge plain objects; arrays are copied and replaced.
 - The `end` hook receives `{ span, record, duration, error? }`.
 
 ### Using debug
@@ -268,6 +283,7 @@ When `@opentelemetry/api` is not available, the instruments are safe noops.
 - **Error recording**: Exceptions are recorded on the span with `ERROR` status
 - **Span name validation**: Unknown span names are rejected early
 - **Record tracking**: Accumulate structured data during a trace and access it in the `end` hook
+- **Managed child spans**: Start child spans from a managed trace context when a manual parent span stays open
 - **Custom actions**: Define span-scoped helper functions via the `actions` factory
 - **Category-based disabling**: Disable groups of spans via environment configuration
 - **Noop fallback**: All instrumentation calls are safe no-ops when OpenTelemetry APIs are not installed
@@ -278,13 +294,17 @@ When `@opentelemetry/api` is not available, the instruments are safe noops.
 
 ### `trace(definition)` — Managed mode
 
-Starts a span and returns a managed context with `record`, `actions`, `end`, and `fail` methods.
+Starts a span and returns a managed context with `record`, `actions`, `child`, `end`, and `fail` methods.
 
 | Parameter | Type | Description |
 |-----------|------|-------------|
 | `definition` | `TraceDefinition` | Trace definition object (see below). |
 
-Returns: `{ record, actions, end, fail }`
+Returns: `{ record, actions, child, end, fail }`
+
+### `managed.child(definition[, callback])`
+
+Starts a span parented to an open managed span. Without a callback, it returns another managed context. With a callback, the child span follows callback-mode lifecycle behavior.
 
 ### `trace(definition, callback)` — Callback mode
 

packages/agents-telemetry/src/observability/trace.ts

@@ -9,12 +9,15 @@ import type {
   TraceDefinition,
   TraceFunction,
   TraceRecord,
+  TraceChildFunction,
+  TraceCallback,
 } from '../types.js'
 import { ExceptionHelper } from '@microsoft/agents-activity'
 import { attempt } from '../utils/attempt.js'
 import { isSpanDisabled } from './category.js'
 import { SpanNames } from './constants.js'
 import { noopContext } from '../utils/noop.js'
+import { cloneRecordValue, mergeRecordValues } from '../utils/record.js'
 import { Errors } from '../errorHelper.js'
 
 /**
@@ -29,6 +32,14 @@ export function traceFactory (otel: OTel) {
   const tracer = otel.trace.getTracer('@microsoft/agents-telemetry')
 
   const trace = function trace (target, callback) {
+    return runTrace(target, callback)
+  } as TraceFunction
+
+  function runTrace (
+    target: TraceDefinition<any, any> | undefined,
+    callback?: TraceCallback<any, any, unknown>,
+    parentSpan?: Span
+  ) {
     if (!target) {
       throw ExceptionHelper.generateException(Error, Errors.TraceDefinitionRequired)
     }
@@ -38,33 +49,44 @@ export function traceFactory (otel: OTel) {
     }
 
     if (isSpanDisabled(target.name)) {
-      return noopContext(callback)
+      return callback ? noopContext(callback) : noopContext()
     }
 
-    const record = createRecord(target)
+    const execute = () => {
+      const record = createRecord(target)
+
+      if (!callback) {
+        const span = tracer.startSpan(target.name)
+        const flow = start(otel, target, span, record)
+        const child = ((definition, childCallback) =>
+          runTrace(definition, childCallback, span)) as TraceChildFunction
+
+        return {
+          record: flow.record,
+          actions: flow.actions,
+          child,
+          fail: flow.fail,
+          end: flow.end
+        }
+      }
 
-    if (!callback) {
-      // TODO: if parent/child span connections are needed, we could expose a 'child' function that internally connects them.
-      const span = tracer.startSpan(target.name)
-      const flow = start(otel, target, span, record)
+      return tracer.startActiveSpan(target.name, span => {
+        const flow = start(otel, target, span, record)
+        return attempt({
+          try: () => callback({ record: flow.record, actions: flow.actions }),
+          catch: (error) => { throw flow.fail(error) },
+          finally: flow.end
+        })
+      })
+    }
 
-      return {
-        record: flow.record,
-        actions: flow.actions,
-        fail: flow.fail,
-        end: flow.end
-      }
+    if (!parentSpan) {
+      return execute()
     }
 
-    return tracer.startActiveSpan(target.name, span => {
-      const flow = start(otel, target, span, record)
-      return attempt({
-        try: () => callback({ record: flow.record, actions: flow.actions }),
-        catch: (error) => { throw flow.fail(error) },
-        finally: flow.end
-      })
-    })
-  } as TraceFunction
+    const parentContext = otel.trace.setSpan(otel.context.active(), parentSpan)
+    return otel.context.with(parentContext, execute)
+  }
 
   trace.define = definition => definition
 
@@ -75,15 +97,15 @@ export function traceFactory (otel: OTel) {
  * Creates the mutable record stored for a trace execution.
  *
  * @remarks
- * - Updates use `Object.assign`, so nested objects are replaced rather than deeply merged.
+ * - Plain objects are recursively merged.
+ * - Arrays are copied and replaced.
  */
 function createRecord<TRecord extends object, TActions extends object> (target: TraceDefinition<TRecord, TActions>): TraceRecord<TRecord> {
-  const state: Partial<TRecord> = target.record ? { ...target.record } : {}
+  const state: Partial<TRecord> = target.record ? cloneRecordValue(target.record) : {}
 
   return {
     set (values) {
-      // TODO: use deep-merge strategy for Object and Array
-      Object.assign(state, values)
+      mergeRecordValues(state as Record<string, unknown>, values as Record<string, unknown>)
     },
     get () {
       return state as Readonly<TRecord>