feat: multi-agent delegation (FRD 0007 v1) (#102) * docs(frds): add FRD 0006 - Multi-agent delegation (agent-as-tool) Draft FRD for serverless-agents multi-agent delegation (agent-as-tool via MAF BaseAgent.as_tool()). Refs Azure/azure-functions-bucees-planning#1185. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68b4980b-91ee-44c7-a655-e0f08e8d34bb * docs(frds): simplify FRD 0006, drop id/tool_name, fail-fast slug collisions - Drop authoring `id` and `tool_name`; identity is the file-stem slug and the delegated tool is always `delegate_` (Decision #16). - Same-stem agent slug collisions now fail fast app-wide instead of silent auto-suffixing, unifying with skills/workflow-tool handling (Decision #17, breaking change; release-note item). - Simplify for all audiences: plain-language intro, glossary, per-section TL;DRs. - Fix leftover suffix/ambiguity language flagged in final rubber-duck review. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68b4980b-91ee-44c7-a655-e0f08e8d34bb * docs(frds): resolve open questions - subagents field, any agent may declare - Confirm authoring field name `subagents` (Decision #5). - Add Decision #18: any independently runnable agent may declare `subagents`; single-level (#6) still applies when that agent is itself delegated, so it cannot delegate onward. - Convert the two former open questions into resolved product questions; no open questions remain (status stays Draft pending final human sign-off). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68b4980b-91ee-44c7-a655-e0f08e8d34bb * docs(frds): sharpen observability (S4.12) - auto-instrumentation + v1 enrichment - Rewrite S4.12: delegated calls are traced automatically (runtime already enables MAF instrumentation; as_tool()->run() + FunctionTool.invoke() auto-nest under one App Insights OperationId via in-proc OTel contextvars). Verified vs MAF tag python-1.3.0 and the Functions Python worker. - Correct the prior vague "must record correlated logs/traces" wording; note runtime logs go to a system logger (spans are the surface). - Scope into v1: af.delegate.* attributes, delegate metrics, and explicit delegated-error accounting (fix _looks_like_tool_error mis-count). - Document limitations: no cross-boundary token roll-up; SSE black-box at the boundary. Add sampling guidance (parentbased_traceidratio) for fan-out. - Add Decision #19, a Goals bullet, module-map + docs-impact entries, and two observability test-plan items. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68b4980b-91ee-44c7-a655-e0f08e8d34bb * feat(config): add subagents field, slug identity, fail-fast collisions (M1) Implements FRD 0006 Milestone 1 (schema/merge/validation/naming): - schema.py: new SubagentRef model (object-only: agent + optional when; extra="forbid" so string shorthand / id / tool_name are rejected) and AgentSpec.subagents: list[SubagentRef]. ResolvedAgent gains a stable slug field (file-stem identity) used for delegate_ tool naming and catalog keys. - merge.py: normalizes subagents during resolution and derives slug via the same sanitization previously used only for auto-suffixed function names, so slug is a safe drop-in replacement (Decision from FRD S5). - validation.py: validate_subagent_references() rejects unknown, duplicate, and self references; relaxes the trigger-or-builtin_endpoints requirement for agents that are only referenced as internal specialists (no direct entry point needed). - registration/_naming.py: BREAKING CHANGE - replaces the silent auto-suffix behavior for duplicate function names with a fail-fast startup error, unifying duplicate-slug handling for agents with the existing duplicate skill/workflow-tool handling (FRD Decision #17). Same-stem files in different directories (e.g. root vs agents/) or names that sanitize alike now fail fast with an actionable rename message instead of silently colliding. - registration/capabilities.py: existing_tool_names() and validate_subagent_tool_names() catch delegate_ collisions against user/MCP/sandbox/workflow-management/other-specialist tool names. - registration/catalog.py (new): CatalogEntry / AgentCatalog / build_catalog - the immutable agent_id -> (ResolvedAgent, AgentCapabilities) structure that app.py's two-pass composition (M2) freezes and hands to handlers. - _slug.py (new): delegate_tool_name(), _safe_function_name(), and _function_name_from_source() - single source of truth for slug/tool-name sanitization shared by merge.py and _naming.py. Adds a tests/fixtures/config_scenarios/15_multi_agent_delegation/ fixture (coordinator + two specialists) and unit tests for: object-form parsing, extra=forbid, string-shorthand rejection, slug normalization, app-wide duplicate-slug fail-fast (including cross-directory same-stem collisions), unknown/duplicate/self reference rejection, and tool-name collisions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * feat(app): two-pass composition with immutable AgentCatalog (M2) Implements FRD 0006 Milestone 2 (app.py composition root): - app.py: create_function_app now runs a two-pass pipeline instead of a single per-agent loop: 1. Pre-block: _fail_on_duplicate_slugs() fails fast app-wide on any slug collision across all discovered agents (listing every colliding source file), then validate_subagent_references() is run per agent to reject unknown/duplicate/self references and to accumulate the global referenced-subagent slug set. 2. Loop A (pure, no FunctionApp mutation): validate_resolved_agent() (passing is_referenced_as_subagent so specialist-only agents are not required to declare a trigger/builtin endpoint), build_capabilities(), and validate_subagent_tool_names() for every agent; results populate catalog_entries. 3. Freeze: build_catalog() turns catalog_entries into an immutable AgentCatalog (agent_id -> (ResolvedAgent, AgentCapabilities)). 4. Loop B (mutates the FunctionApp): registers triggers/endpoints using the frozen catalog; resolved.slug is now used directly as the function name (allocate_unique_function_name is removed - it is fully superseded by the fail-fast pre-block). - registration/triggers.py, endpoints.py, _handlers.py: thread an optional catalog: AgentCatalog | None parameter from register_agent()/register_builtin_endpoints() down through the dispatch helpers and handler factories into run_agent()/ run_agent_stream(), so handlers operate on immutable catalog data (not live MAF agent objects) - runner-side delegation (M3) will read subagents/catalog from here. - registration/_handlers.py: adds _total_tool_error_count(), combining the existing heuristic tool-error count with AgentResult's new delegate_error_count so delegated-specialist failures are reflected in run summaries/telemetry without relying on the JSON {error}/stderr heuristic (FRD Decision #19). Test fixes: test_registration_triggers.py assertions updated for the new trailing catalog argument now threaded through _register_builtin_agent/_register_http_agent (pre-existing tests that monkeypatch these dispatch points with positional-arg capture). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * feat(runner): M3 - chat-time delegation tool building + observability Implements FRD 0006 Milestone 3: build one delegate_ function tool per subagents reference and expose it on the coordinator's own tool list so specialists run inside the normal gent.run() tool-calling loop (no HandoffBuilder, no HITL - v1 scope only). runner.py: - _build_role_agent replaces _build_agent_session_history's old inline Agent(...) construction with a shared tail used by both execution roles (Decisions #13/#15): direct (coordinator/any directly-triggered agent) passes a real history provider and, when subagents is declared, delegate_tools; delegated (a specialist invoked as a delegate_ tool) passes history_provider=None and delegate_tools=None, so per-request sandbox tools and main-only Dynamic-Workflow tools are naturally absent rather than stripped from a shared list. - _build_delegated_agent builds one specialist "as itself": own instructions/model/static user+MCP+skills tools, reusing the process-wide ClientManager, built fresh per request (MAF Agent.run() self-mutates). Never reads esolved.subagents - the structural enforcement of single-level delegation (Decision #6): a delegated specialist cannot itself gain delegate_* tools, with no runtime depth counter required. - uild_subagent_tools builds one tool per reference via MAF's BaseAgent.as_tool() (name=delegate_, description=when or the specialist's own description, arg_name="task", approval_mode="never_require", propagate_session=False), eagerly for every reference (Decision #14/#4.7) - a specialist only runs if the coordinator's model selects the corresponding tool call. - _delegate_adapter wraps MAF's own _agent_wrapper with the failure/cancellation split (Decision #12): specialist error or specialist-local timeout is recoverable (sanitized string returned to the model, full detail to telemetry, racker.record_error() increments AgentResult.delegate_error_count); CancelledError is a BaseException neither xcept clause catches, so parent/request cancellation propagates and aborts. Effective timeout = min(specialist timeout, coordinator's own remaining budget), recomputed per call from a shared coordinator_deadline. - Concurrency (Decision #14): each delegate tool gets its own syncio.Lock() - calls to the same specialist serialize; different specialists run in parallel. - _check_delegate_tool_name_collisions re-checks tool-name collisions against the actual live tool list (MCP tool names are only known at runtime) right before final assembly, on top of the composition-time check added in M2. - un_agent/ un_agent_stream gain subagents/catalog parameters threaded through to _build_agent_session_history; AgentResult.delegate_error_count surfaces the per-run recoverable delegate-failure count (streaming intentionally does not surface this in the SSE vocabulary - telemetry captures it identically either way; documented in the stream docstring). _observability.py (Decision #19, FRD §4.12): - FaultDomain.DELEGATE for delegate-attributed span errors. - current_span() wraps the already-active span (MAF/FunctionTool's own auto-nested xecute_tool delegate_ span) instead of starting a new one, so delegation enrichment lands on the span the runtime already produces rather than adding a redundant nested span. - ecord_delegate_call(*, error) mirrors ecord_sandbox_execution: zure_functions_agents.delegate.{calls,errors} counters, gated behind the existing enabled/disabled switch. registration/_handlers.py (already present from M2, exercised here): - _total_tool_error_count combines _tool_error_count's JSON {error}/stderr heuristic with AgentResult.delegate_error_count so a specialist's sanitized free-text failure - invisible to that heuristic - is still counted in f.agent.tool_error_count and the response log, without being misclassified by the heuristic itself. Tests: - tests/test_runner_delegation.py (new): pure-helper tests (_DelegateErrorTracker, _sanitize_delegate_failure, _check_delegate_tool_name_collisions); _build_role_agent composition tests proving the delegated role gets only its own tools (no sandbox/workflow tools) while the direct role gets the full superset, plus tool-name collision fail-fast; _build_delegated_agent proves single-level enforcement end-to-end with mutual A<->B subagents references (no recursion); uild_subagent_tools guard-clause tests (missing catalog, unknown reference); adapter tests for success, recoverable exception, specialist timeout, coordinator-remaining-caps-effective-timeout, and cancellation propagation (not recorded as a delegate error); concurrency tests proving same-specialist serialization and different-specialist parallelism; a real-OTel-span-sharing test (local TracerProvider + InMemorySpanExporter) proving nested xecute_tool delegate_ spans share one trace id under concurrent syncio.gather and carry f.delegate.specialist/ f.delegate.outcome. - tests/test_registration_handlers.py: _total_tool_error_count tests - combines heuristic + delegate errors, does not misclassify sanitized delegate text as a heuristic match, handles missing fields/None gracefully. - tests/test_observability.py: FaultDomain.DELEGATE value, current_span() no-op-when-disabled / wraps-active-span-when- enabled, ecord_delegate_call gating and call/error counter behavior, safety smoke test. - tests/test_runner_streaming.py: updated ake_build_agent_session_history fixture for the 3-tuple -> 4-tuple return-shape change (added the delegate-error tracker). Validation: ruff check src tests (clean), mypy src (clean), pytest tests -q (656 passed, up from 627). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs+sample: multi-agent delegation docs and runnable sample (M4) Complete the FRD 0006 v1 documentation and sample milestone: Docs: - README.md: new "Multi-agent delegation (`subagents`)" subsection (trust boundary / Decision #10, self-contained task, single-level / Decision #6, cross-links); fixes a second, previously-missed instance of stale auto-suffix language under "Multiple functions from markdown" and adds a prominent breaking-change callout (no CHANGELOG file exists in this repo, so this is the release-note record for the `_naming.py` fail-fast change); new Samples list row. - docs/architecture.md: full "Multi-agent delegation" section covering the two-pass composition root, immutable AgentCatalog, delegated execution role, and failure/concurrency handling. - docs/front-matter-spec.md: `subagents` field reference (schema, validation rules, delegated-execution semantics, trust boundary, concurrency, failure handling), Example 6, updated Quick Reference / Validation Rules / File Naming Conventions sections, and the breaking-change note for slug collisions. - docs/observability.md: new "Span `execute_tool delegate_`" section (nested span tree, `af.delegate.*` attributes, the `delegate` fault domain, delegate call/error metrics, explicit delegated-error accounting, accepted v1 limitations), a delegation-heavy sampling note, and a KQL query for delegate failures. - docs/triggers.md: new "Endpoint-less internal specialists" subsection explaining the relaxed trigger/`builtin_endpoints` requirement for agents referenced only via `subagents:`. - docs/frds/README.md: index row for FRD 0006 ("In review", matching its own frontmatter status). - eng/scripts/generate_config_reference.py: generator updates for the new field; docs/front-matter-reference.md regenerated (verified byte-identical to the generator's output). - samples/README.md: new sample row. Sample (samples/multi-agent-delegation/): - A coordinator (`main.agent.md`, `builtin_endpoints: true`) that declares two specialists via `subagents:`. - `billing` — a specialist with its own HTTP trigger (independently runnable AND delegatable). - `tech` — an endpoint-less internal specialist (no trigger, no builtin_endpoints; reachable only through delegation), demonstrating the relaxed reachability rule. - Local-run only (matches the `multi-agent-folder` sample's precedent): no `azure.yaml`/`infra/` is included, since this sample demonstrates a chat-time composition feature rather than a new deployable Azure resource footprint. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix review findings B1-B4: absolute deadlines, agent name attribution, delegate-error accounting, cancellation telemetry B1 - Coordinator/turn deadline enforcement - _delegate_adapter: recompute remaining/effective_timeout AFTER acquiring specialist_lock (not before), so lock-wait time counts against the coordinator's absolute deadline. If the budget is already exhausted post-lock, raise TimeoutError immediately without attempting the call. - run_agent_stream: bound each iteration's wait for the next stream update with asyncio.wait_for(stream_iter.__anext__(), timeout=remaining) instead of only checking elapsed time after an update was already yielded, so a stalled generator cannot exceed the deadline. Reuses the existing 'Timeout after {timeout}s' error format/event. B2 - Specialist Agent telemetry attribution - _build_role_agent: pass name=agent_name into the Agent(...) constructor so MAF's AgentTelemetryLayer stamps gen_ai.agent.name instead of a random UUID. - _build_delegated_agent: pass agent_name=resolved.slug (not resolved.name) so a delegated specialist's invoke_agent span is attributed by slug, matching delegate_ and FRD Decision #19. Direct-role call sites (coordinators) are left unchanged - their outer agent.run {name} wrapper span is also named by display name, so keeping the inner Agent's name in sync with that is correct, not the same bug. B3 - Delegate-error accounting on built-in/streaming surfaces - registration/endpoints.py: handle_chat and handle_mcp_agent_chat now open their own agent.run {name} span (nothing upstream did before, since these call run_agent directly) and apply _set_run_result_attributes(), matching the trigger-registered handler path in _handlers.py. - runner.py run_agent_stream: now opens its own agent.run {name} span (necessary because callers like handle_chat_stream construct the generator without driving it) and applies af.agent.tool_error_count from the delegate_error_tracker in a finally block so it is always recorded regardless of success/timeout/error outcome. B4 - Failure/cancellation telemetry - _delegate_adapter: use span.record_exception(exc, fault_domain=...) instead of span.set_error(...) for both the timeout and generic-exception branches, preserving real exception type/traceback in telemetry. - _sanitize_delegate_failure: removed type(exc).__name__ from the model-facing string so it is class-independent; exception detail now only reaches telemetry, never the model. - asyncio.CancelledError is now caught, annotates af.delegate.outcome=cancelled on the span for observability, and is immediately re-raised (never swallowed, never counted as a recoverable delegate error per Decision #12). Tests: added a real-instrumentation test that drives an actual agent_framework.Agent specialist through build_subagent_tools and asserts gen_ai.agent.name == slug on MAF's own invoke_agent span; added a lock-wait regression test proving a second queued call to the same specialist that exhausts the deadline while waiting never invokes the underlying call; added a cancellation-outcome test; added built-in-endpoint and streaming delegate-error-count tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(delegation): strengthen MCP collision + specialist-ownership coverage (S5, S6b) S5 - MCP collision guard scope accuracy: - _check_delegate_tool_name_collisions (runner.py) and existing_tool_names (registration/capabilities.py) only ever inspected each MCP server connection object's own .name; they never saw the individually expanded remote functions/tools MAF loads later via MCPTool.load_tools(). The existing tests used SimpleNamespace fakes that didn't exercise this gap. - Corrected both docstrings to state the guard's actual scope (composition- time / connection-name collisions only) instead of implying it catches every possible tool-name collision. - Added test_check_delegate_tool_name_collisions_misses_expanded_mcp_remote_function_names to prove the gap using a fake MCP server exposing an expanded .functions collection with a colliding name. - Added test_real_maf_agent_run_raises_on_expanded_mcp_function_collision to prove MAF's own Agent.run() independently re-checks tool-name uniqueness once it expands MCPTool.functions, raising ValueError before any model or tool call happens - so the scope gap in this repo's own guard is backstopped by MAF itself, not a silent hole in practice. S6b - specialist-ownership regression coverage: - Added test_build_delegated_agent_uses_specialists_own_model_instructions_tools_and_skills_not_coordinators, which builds a real MAF Agent for a specialist via _build_delegated_agent and a contrasting "coordinator" agent via a direct _build_role_agent call with deliberately different model/instructions/tools/skill-paths, then asserts the specialist's agent.client.model, default_options["instructions"], and wired tool/skill inputs all come from the specialist's OWN resolved config and capabilities - never the coordinator's. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: add end-to-end fixture coverage for endpoint-less specialists and composition regression (S6a, S6c) S6a - endpoint-less internal specialist through the real pipeline: - Added test_create_function_app_accepts_endpoint_less_specialist_referenced_only_via_subagents in test_app.py, using the existing 15_multi_agent_delegation fixture through the real create_function_app() pipeline (not a manually reconstructed catalog). Asserts the coordinator/billing endpoints register normally and that the "shipping" specialist (no trigger, no builtin_endpoints, only reachable via another agent's subagents:) contributes zero functions/ routes of its own while still being usable as a delegate. S6c - pre-FRD-0006 multi-agent regression + tool-assembly scoping: - Added a new fixture tests/fixtures/config_scenarios/16_no_subagent_regression/ (main + nightly_report + resource_summary, none of which declare subagents:) plus a tools/ directory with one real discoverable tool (get_region_status), since no existing fixture had one - needed to make a "tools: false empties this agent's tools" assertion meaningful rather than vacuously true against zero discovered tools everywhere. - Added test_no_subagent_regression_fixture in test_config_fixtures.py: loads/composes/validates all three agents exactly as a single-pass (pre- delegation) pipeline would have, then uses build_capabilities() directly to confirm resource_summary's tools: false yields an empty filtered_user_tools while main/nightly_report (no tools: false) retain the discovered tool - proving tools: false stays scoped to its own agent under two-pass composition. - Added test_create_function_app_regression_pre_existing_multi_agent_fixture_unaffected_by_two_pass_composition in test_app.py: runs the new fixture through the real create_function_app() pipeline end-to-end and asserts the exact 6 registered functions and HTTP routes, proving a collision-free, subagents-free multi-agent fixture registers identically under the new two-pass composition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: correct AgentCatalog immutability claims to its actual shallow guarantee (N7) registration/catalog.py's docstrings described the AgentCatalog/CatalogEntry as "immutable", but ResolvedAgent is a plain (non-frozen) pydantic.BaseModel and AgentCapabilities is a plain (non-frozen) dataclass - only the MappingProxyType wrapper and CatalogEntry's own frozen=True prevent rebinding entries/fields; the resolved/capabilities objects an entry points to (and their internal lists) are still ordinary mutable Python objects. Verified nothing in src/ currently performs a post-construction mutation of a ResolvedAgent, AgentCapabilities, or CatalogEntry (everything is built via constructor kwargs in compose()/build_capabilities()/build_catalog() and never reassigned afterwards), so making the types themselves deeply frozen would be a non-trivial, invasive change for no behavior-changing benefit today. Chose the documentation fix over a structural one per this being a nice-to-have: reworded catalog.py's module/class/comment docstrings to state the guarantee accurately ("read-only mapping of otherwise-ordinary resolved/capability objects") and updated the two "immutable AgentCatalog" references in app.py's pipeline comments to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix review findings M1-M3, S1, S3-S4, N1 (round 3): outer deadline enforcement, MAF stream finalization, streaming tool-error accounting, slug attribution, exception preservation, real-invoke span coverage M1 - Outer coordinator run now honors the absolute coordinator deadline - run_agent: the session-lock wait is now itself bounded by coordinator_deadline via asyncio.wait_for(lock.acquire(), timeout=...). On lock-wait timeout, raises the same RuntimeError("Agent run timed out after {timeout}s") the existing outer-timeout path already raises (no new error shape). After the lock is acquired, remaining_after_lock is recomputed from the same coordinator_deadline and used as the agent.run(...) wait_for bound instead of the stale full timeout, so total wall-clock for one turn can no longer exceed timeout by however long the lock wait took. Switched from `async with lock` to manual acquire()/try-finally-release() so the bounded-acquire call is possible. - run_agent_stream: identical pattern using the streaming loop's existing `deadline` variable - the lock wait is bounded before the per-update loop begins, and on timeout emits the same data: {"type": "error", ...} SSE event shape the existing per-update timeout branch already emits, then returns cleanly. - Both fixes ensure a coordinator-budget timeout aborts the whole run (FRD 0006 Section 4.6) instead of letting the outer run continue on a fresh/unbounded budget while delegate adapters misclassify the same deadline passing as an isolated recoverable specialist timeout. M2 - Cancelled/timed-out MAF streams are now explicitly finalized - Added _finalize_maf_stream(stream, exc): a best-effort helper that probes a MAF ResponseStream's private _run_cleanup_hooks/_stream_error surface (confirmed via direct inspection of the pinned agent_framework package) and invokes it, mirroring what ResponseStream.__anext__ does on a normal Exception - work that never runs when asyncio.wait_for's timeout instead injects asyncio.CancelledError (a BaseException) into whichever task is awaiting __anext__()/the wrapped coroutine. - _build_delegate_tool: wraps specialist_agent.run so the ResponseStream as_tool()'s _agent_wrapper creates internally (and never otherwise exposes) can be captured; both the CancelledError and TimeoutError branches in _delegate_adapter now call _finalize_maf_stream before re-raising/returning. - run_agent_stream: the coordinator's own per-update asyncio.wait_for(stream_iter.__anext__(), timeout=remaining) pull now catches (TimeoutError, asyncio.CancelledError), finalizes the stream via the same helper, and re-raises. M3 - Streaming's af.agent.tool_error_count now includes ordinary tool failures, not just delegate failures - run_agent_stream imports the existing _looks_like_tool_error heuristic from registration._handlers (no duplication of the JSON-error-envelope/ stderr detection logic) and applies it to every function_result content item's result as it is emitted, accumulating ordinary_tool_error_count. - The final af.agent.tool_error_count span attribute is now delegate_error_tracker.count + ordinary_tool_error_count instead of only the delegate half, matching the non-streaming path's _total_tool_error_count contract. S1 - Coordinator/direct-role agents now attributed by resolved.slug - registration/_handlers.py (make_agent_handler, make_http_agent_handler) and registration/endpoints.py (_run_builtin_agent, _run_builtin_agent_stream, _register_http_chat, _register_mcp_endpoint): the agent.run {name} span name and af.agent.name attribute now use resolved.slug, with the previous mutable display name preserved separately as af.agent.display_name. agent_name=resolved.slug is now threaded into the run_agent/run_agent_stream calls in _handlers.py, which previously omitted the parameter entirely. Matches round 2's B2 fix for delegated specialists; FRD 0006 Section 4.3, "Display `name` is never an identity." S3 - Delegate-adapter timeout telemetry preserves the real exception object - _delegate_adapter's `except TimeoutError:` is now `except TimeoutError as exc:`, and span.record_exception(exc, ...) records whichever TimeoutError instance was actually caught - wait_for's own expiry error, the pre-emptive budget-exhausted raise (now carrying a descriptive message instead of a bare TimeoutError), or a TimeoutError the specialist's own code happened to raise internally - instead of constructing a synthetic new TimeoutError(...) for telemetry. The model-facing sanitized string is unchanged. S4 - Added a real end-to-end span-tree test through FunctionTool.invoke() - New test builds a real specialist via _build_delegated_agent/ build_subagent_tools, invokes the delegate tool via `await tool.invoke(...)` (MAF's actual entry point, confirmed via direct inspection of agent_framework._tools.FunctionTool.invoke) under an active coordinator-level span, and asserts a nested `execute_tool delegate_` span and a further-nested `invoke_agent ` span exist, share the coordinator's trace id, and parent correctly - exercising MAF's own instrumentation and parameter-binding path rather than calling the adapter function directly. N1 - Built-in MCP endpoint span session id - registration/endpoints.py _register_mcp_endpoint: when the caller supplies no explicit session id, the span's af.agent.session_id attribute is now refreshed from result.session_id (the id the runner actually resolved/generated) instead of being left unset from the pre-call None. Tests: 12 new tests added (670 -> 682 passing) covering M1 (session-lock contention forcing the coordinator deadline to expire mid-wait, for both run_agent and run_agent_stream), M2 (real MAF stream finalization on both specialist timeout and outer task cancellation, verified via a spy on the underlying stream/generator), M3 (an ordinary tool failure alone, and combined with a delegate failure, correctly summed into af.agent.tool_error_count), S1 (slug vs. display-name attribution across both handler and built-in-endpoint call sites), S3 (exact exception-object identity preserved in span.record_exception for both a wait_for-expiry timeout and a specialist-raised inner TimeoutError), S4 (the real tool.invoke() span-tree test described above), and N1 (session id refresh on the MCP endpoint span). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs: correct FRD 0006 AgentCatalog wording to match code (N2) Round 2's N7 fix already corrected catalog.py/app.py's docstrings from claiming full "immutable" AgentCatalog semantics to accurately describing it as a read-only mapping. The FRD's own prose in Section 4.2 (the register-stage responsibility table and the ordered composition pipeline list) still said "immutable AgentCatalog" / "immutable runtime AgentCatalog" in two spots. Updated both to "read-only" to match the code-side wording exactly. Since PR #102 now delivers the FRD doc itself alongside the implementation (retargeted to main), this wording mismatch was an in-scope PR defect rather than unrelated-file drift. No structural changes to the FRD. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix round-4 rubber-duck review findings (B1, B2a/b/c, S1b, S3b) B1 (blocking): `_build_delegate_tool`/`_delegate_adapter` captured the specialist's in-flight `ResponseStream` in a dict shared across every future invocation of a `delegate_` tool. A call still queued on `specialist_lock`, cancelled before ever acquiring it, would still hit `except asyncio.CancelledError` and finalize whatever *other* call currently held the lock and its own live stream -- out from under it, mid-flight. Fixed by making the capture slot (`my_stream`) a fresh local created on every call to `_delegate_adapter`, and rebinding `specialist_agent.run` to a closure over *that* call's slot only after acquiring `specialist_lock`, immediately before invoking `original_func`. Safe with no additional synchronization: asyncio is single-threaded, and `specialist_lock` already guarantees only one invocation is ever inside that region. `original_run` stays a single tool-build-scope reference (just the real underlying method; safe to reuse). The `asyncio.wait_for(lock.acquire(), ...)` + `try/finally: lock.release()` pattern itself is untouched, as instructed. B2a (must-fix): `run_agent_stream`'s per-update pull loop raised its `remaining <= 0` deadline pre-check *above* the `try` that finalizes the underlying MAF stream on timeout/cancellation, so a deadline already exhausted at the top of a loop iteration (rather than expiring while genuinely awaiting `stream_iter.__anext__()`) reached the outer `except TimeoutError` unfinalized. Moved the pre-check inside the same inner `try` so both cases go through one finalize-and-reraise path. B2b (must-fix): tearing down the streaming generator while suspended AT one of its own `yield f"data: ..."` statements (ASGI disconnect via `aclose()`, or the enclosing task cancelled between yields rather than while awaiting `__anext__()`) delivers `GeneratorExit`/cancellation directly at the suspension point -- a code path none of the existing `except TimeoutError`/`except Exception` clauses can catch. Added a `finally:` guarded by a `stream_settled` flag (set by every other exit path) that finalizes the stream using `sys.exc_info()` (falling back to a generic `CancelledError` when torn down with nothing in flight), without double-finalizing an already-settled stream. B2c (must-fix, source-verified): extended `_finalize_maf_stream` to walk `stream._inner_stream` and finalize every level of the chain, not just the outermost object -- correct and effective for compositions where a further-wrapped stream is reachable via that attribute (e.g. MAF's own `Agent.run(stream=True)` -> `.map()`-transform stream, `agent_framework._agents` / `_types.py`, cited in the updated docstring). Verified against the installed `agent-framework-core==1.3.0` source that this repository's real chat clients (`OpenAIChatClient`/`FoundryChatClient`) stack `FunctionInvocationLayer` above `ChatTelemetryLayer`; with function invocation enabled (the default), the per-turn `chat {model}` span's backing stream is a *purely local variable* inside `FunctionInvocationLayer`'s own internal `_stream()` closure (`_tools.py`), never assigned to `_inner_stream` or exposed on any externally reachable attribute, and that generator has no enclosing `try`/`except`/`finally` of its own. This is a genuine, verified upstream MAF limitation with no discoverable workaround from outside `agent_framework` -- documented as a residual gap directly in `_finalize_maf_stream`'s docstring, with exact source lines cited, rather than papered over. S1b (should-fix): `run_agent_stream` opened its own `agent.run {name}` span with `af.agent.name` but no `af.agent.display_name`, unlike `endpoints.py`'s non-streaming/MCP handler spans which already set both. Added a `display_name: str | None = None` parameter to `run_agent_stream`, threaded it into the span attributes, and updated `endpoints.py`'s streaming call site to pass `display_name=resolved.name`. `run_agent` (non-streaming) opens no span of its own, so no parity change was needed there. S3b (should-fix): `_delegate_adapter`'s single `except TimeoutError` handler classified both a genuine `asyncio.wait_for` deadline expiry and any ordinary `TimeoutError` raised from inside the specialist's own execution identically, as `outcome=timeout` -- even though the latter is just an ordinary recoverable specialist failure that happens to be a `TimeoutError` instance. Added a timing heuristic (`call_start` recorded immediately before `wait_for`, tolerance `_INNER_TIMEOUT_MISCLASSIFICATION_TOLERANCE_SECONDS = 0.05`): when elapsed time is meaningfully less than `effective_timeout`, classify via the same generic `_record_generic_delegate_failure` path used for other exception types (`outcome=error`) instead. The unambiguous pre-dispatch budget-exhausted case (`effective_timeout <= 0` before `original_func` is ever called) always stays `outcome=timeout`, using the same per-call "was `original_func` ever invoked" signal introduced for B1. Tests added (8 new, 682 -> 690 passing; `ruff check src tests` and `mypy src` both clean): - test_runner_delegation.py: `..._queued_cancellation_does_not_finalize_other_calls_stream` (B1 regression: a real MAF never-responding specialist stream held by call A must not be finalized by call B's cancellation while B is still queued on the same specialist's lock). - test_runner_delegation.py: updated `..._preserves_inner_timeout_error_instance_in_telemetry` and strengthened `..._effective_timeout_uses_coordinator_remaining_when_smaller` for S3b's three classification cases (genuine wait_for expiry -> timeout; inner specialist TimeoutError -> error; pre-dispatch budget exhausted -> timeout). - test_runner_streaming.py: `..._finalizes_when_deadline_exhausted_between_updates` (B2a), `..._finalizes_when_cancelled_while_suspended_at_a_yield` (B2b, via `gen.aclose()` mid-stream), four new direct unit tests of `_finalize_maf_stream`'s `_inner_stream`-chain walk in isolation (B2c: multi-level cascade, `_stream_error` non-clobber, hostile-hook resilience, `None`-stream no-op), and `..._reports_display_name_on_span` (S1b). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Simplify delegate execution: hand-written non-streaming tool, no lock Replaces the `as_tool()`-based `delegate_` tool with a hand-written `@tool(schema=...)` function tool (the same pattern this repo already uses for `web_request`/`execute_python`) whose handler builds a FRESH specialist `agent_framework.Agent` on every call and awaits its plain, non-streaming `agent.run(task)` directly. Why: a delegate only ever needs the specialist's final answer as one string back to the coordinator -- it never streams the specialist's tokens anywhere (an explicit FRD 0006 non-goal). `BaseAgent.as_tool()`'s own `_agent_wrapper` always ran the specialist via `Agent.run(stream=True, ...)` internally before `await`-ing `stream.get_final_response()` back into that same one string anyway. That unnecessary streaming was the *only* reason the previous implementation needed to monkeypatch `specialist_agent.run` (`_capturing_run`) to capture the internal `ResponseStream` so it could be force-finalized via `_finalize_maf_stream` on timeout/cancellation -- `ResponseStream.__anext__`'s cleanup hooks only run from its own `except StopAsyncIteration`/`except Exception` branches, never from a `BaseException` such as `asyncio.CancelledError`. And because the specialist `Agent` object was built once and shared across every call to a `delegate_` tool in a turn, concurrent same-specialist calls had to be serialized behind a per-specialist `asyncio.Lock` so the monkeypatch rebind + capture slot stayed race-free (the B1 fix). STEP 1 verification (required before rewriting, per task instructions): confirmed against installed `agent-framework-core==1.3.0` source that non-streaming `Agent.run()` does NOT have the same cleanup gap as the streaming path. `AgentTelemetryLayer._run` and `ChatTelemetryLayer ._get_response` (agent_framework.observability) open their OTel spans with an ordinary `with _get_span(...) as span:` / `with trace.use_span(...)` context manager, whose `__exit__` is guaranteed to run on ANY exception -- `asyncio.CancelledError` included -- via standard Python `with`-statement semantics. This is fundamentally different from the streaming design's manually-invoked, per-branch cleanup-hook protocol. So removing the streaming removes the need for `_finalize_maf_stream` on the delegate path entirely, with no gap and no reintroduced workaround. `_finalize_maf_stream` itself is unchanged and keeps its 4 remaining callers in `run_agent_stream` (the coordinator's own SSE streaming endpoint -- a separate, untouched feature, "Place A"). Because each call now builds its own, unshared specialist `Agent` instance, there is no live agent for two overlapping calls to race on in the first place, so the per-specialist `asyncio.Lock`, the `_capturing_run` rebind, and the per-call `my_stream` capture slot are all removed: same-specialist concurrent calls now simply run in parallel, each on its own instance. Preserved exactly (FRD 0006): - Failure/cancellation split (Decision #12): specialist error or specialist-local timeout -> recoverable, sanitized `tool_end` string; parent/request cancellation -> propagates and aborts the turn. - Timeout math: `effective_timeout = min(specialist timeout, coordinator remaining budget)`, enforced via `asyncio.wait_for`. - Observability (Decision #19): `af.delegate.*` span attributes, the delegate `FaultDomain`, delegate call/error metrics, and delegate-error accounting for `af.agent.tool_error_count` -- now on a cleaner, more debuggable per-call span timeline (each specialist run's span opens/ closes at one well-defined point per call, on every path). - Single-level delegation (Decision #6): a delegated specialist still gets no `delegate_*` tools; `_build_delegated_agent` still never reads `resolved.subagents`. - Schema/merge/validation/`_naming.py` fail-fast, two-pass composition, the read-only catalog, and per-request freshness -- all unchanged. Tests (tests/test_runner_delegation.py, 30 tests, all passing): - Removed `..._queued_cancellation_does_not_finalize_other_calls_stream` (the B1 race test) and `..._serializes_concurrent_calls_to_same_specialist` -- both tested machinery that no longer exists. - Added `..._concurrent_calls_to_same_specialist_run_on_independent_ instances_in_parallel`: two concurrent calls to the same specialist each build their own `_FakeSpecialistAgent` instance and run fully in parallel, both producing correct, independent results. - Simplified `_FakeSpecialistAgent` to a minimal double with an `async def run(self, task, **kwargs)` (no more `BaseAgent` subclassing or `as_tool()`-shaped `run(stream=True)` plumbing); `_RunnableFakeChatClient`/ `_NeverRespondingChatClient` now support `stream=False` (the only mode the rewritten handler ever uses). - Adapted the real-MAF-span finalization tests (renamed off "stream", since there is no stream) to prove the real `invoke_agent` span still closes deterministically on specialist timeout and on outer cancellation, with no `_finalize_maf_stream` spy/call on this path at all. - Adapted the single-level end-to-end test to actually invoke the built `delegate_a` tool (via `_RunnableFakeClientManager`), since the specialist `Agent` is no longer built at tool-assembly time. - All other call sites updated from `tool.func(ctx, task=...)` to `tool.func(task=...)`, since the hand-written tool's `func` takes no `ctx` parameter (only MAF's own `_agent_wrapper` needed one). - tests/test_runner_streaming.py: fixed two docstrings that cited the now- removed `specialist_lock`/`original_func`/"B1 scenario" as example context for `_finalize_maf_stream(None, ...)`; the assertions themselves were unaffected and needed no behavior change. Docs: FRD 0006 updated to match -- Decision #14 revised (no more per- specialist serialization), new Decision #20 logging this simplification and its rationale, and §4.1/4.2/4.4/4.5/4.6/4.7/4.8/4.9/4.11/4.12/4.13 (plus §1 Summary, §3 Goals, §8 Status) updated to describe the hand-written, per-call mechanism instead of `as_tool()` + lock + monkeypatch. Also updated docs/architecture.md, README.md, docs/observability.md, docs/front-matter-spec.md, and samples/multi-agent-delegation/README.md, which all described the now-superseded `as_tool()`-based mechanism. `ruff check src tests`, `mypy src`, and `pytest tests -q` are all green (688 passed). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e29a2a61-a2ca-4c90-bac9-c21261dd1c48 * Fix round-5 rubber-duck review findings (B1, S1, S2, N1, N2) Addresses the gpt-5.6-sol rubber-duck pass on the delegate-execution simplification (commit b1cdffd). The pass confirmed the direction and all the important properties (fresh-per-call construction, non-streaming, no lock/monkeypatch/capture, CancelledError caught-before-Exception + re-raised + not counted as an error, timeout math including the negative-clamp, single-level structural enforcement, `_finalize_maf_stream`/ `run_agent_stream` left untouched, the real-MAF span tests being genuine, and dropping `UserInputRequiredException` handling being fine under Decision #8) but found one blocking issue, two should-fix items, and two nice-to-haves. ## Blocking (fixed) - **B1 — Specialist construction was outside the recoverable-failure boundary.** `_build_delegated_agent(...)` was called BEFORE the `try` in `_build_delegate_tool`'s handler (`runner.py`). A construction failure (e.g. a misconfigured specialist model failing chat-client construction, or tool assembly raising) would bypass sanitization, `_record_generic_delegate_failure`, delegate error accounting, and `af.delegate.outcome` entirely, and propagate as an unhandled exception that aborts the whole coordinator turn — contradicting FRD §4.6's "specialist failure is recoverable" contract (Decision #12). Fixed by moving `_build_delegated_agent(...)` inside the same `try` as the run, with `call_start` initialized to `None` before the `try` and only set once the build succeeds (right before `wait_for`) — so the existing inner-vs-deadline `TimeoutError` classification heuristic still measures run time specifically, and a `TimeoutError` raised by the builder itself (defensively guarded, even though nothing in `_build_delegated_agent` currently raises one) is treated as a generic recoverable error rather than misclassified as a genuine `wait_for` deadline expiry. New test: `test_delegate_adapter_recovers_from_specialist_construction_ failure_with_sanitized_message` — verified it actually fails against the pre-fix code (an unhandled `RuntimeError` propagates with sensitive text intact) before confirming it passes against the fix. ## Should-fix (both addressed) - **S1 — Cancellation wasn't recorded in the delegate call metric.** The `except asyncio.CancelledError` branch set `af.delegate.outcome=cancelled` and re-raised correctly, but never called `record_delegate_call`, so a specialist call that was genuinely dispatched and then cancelled was invisible to the call-volume metric (FRD §4.12 promises delegate call/ error metrics). Added `record_delegate_call(error=False)` before the re-raise — counts it as a call, per `record_delegate_call`'s own documented semantics (`error=False` only suppresses the error counter, not the call counter), while the error tracker/counter stay untouched. Updated the two existing cancellation tests (`test_delegate_adapter_propagates_cancellation_recording_a_call_but_not_ an_error`, renamed from `..._without_recording_a_delegate_error`, and `test_delegate_handler_finalizes_real_maf_agent_span_on_outer_ cancellation`) to assert `calls == [False]` instead of `calls == []`. - **S2 — Docs had drifted from the implementation.** - `docs/architecture.md`: the "parent/request cancellation is not caught by the handler at all" sentence was already imprecise (an explicit `except asyncio.CancelledError` clause has always annotated the span before re-raising) and became more clearly wrong once S1 added a metric call inside that same clause. Rewrote to describe what actually happens: caught, telemetry annotated (span outcome + call metric, not an error), then re-raised unhandled. Also updated the adjacent "specialist failure" bullet and the "Building delegate_ tools" steps to note construction failures are covered by the same recoverable boundary (B1). - `docs/observability.md`: the `af.delegate.outcome` table was missing `cancelled` from its value list; added it, and expanded the cancellation paragraph to mention the call-metric nuance. - `docs/frds/0006-multi-agent-delegation.md`: Decision #19 still described the observability mechanism as `as_tool()`→`run()`, which reads as contradicting Decision #20 (the hand-written, non-streaming, per-call design). Updated #19's mechanism clause to note the tool now calls `run()` directly (same effect: still auto-nested, still no new tracing code) rather than rewriting its historical conclusion. Also fixed two smaller stale `as_tool()`-specific asides in Decisions #3 and #7 (the "no new dependency" and "handoff participant" mentions) the same way — pointer/wording only, conclusions unchanged. §4.6 and §4.12 prose updated to describe the B1/S1 behavior. Left the R1-R3 review-round summary in §8 and the rest of the historical Decisions log untouched. ## Nice-to-have (both done) - **N1** — `test_delegate_adapter_effective_timeout_uses_coordinator_ remaining_when_smaller` now also asserts `_build_delegated_agent` was never called (not just that the specialist's `respond()` body never ran) when the coordinator budget is already exhausted before dispatch. - **N2** — Added `test_delegate_tool_schema_has_a_single_required_string_ task_field`, a direct assertion on the built `FunctionTool.parameters()` output: exactly one required string property named `task`. Did NOT add `ConfigDict(extra="forbid")` to `_DelegateTaskParams` (the reviewer left this as a judgment call, "not a functional risk today") — kept it consistent with this repo's existing `WebRequestParams`/ `ExecutePythonParams`, neither of which sets `extra="forbid"` either. Tests: 690 passed (688 -> 690; +2 new). `ruff check src tests` and `mypy src` both clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e29a2a61-a2ca-4c90-bac9-c21261dd1c48 * Simplicity audit: remove defensive/heuristic code, trim comment burden Addresses a claude-opus-4.8 simplicity audit of the delegation feature (P1-P6). Mechanism (fresh-per-call, non-streaming, cancel-vs-failure split, timeout math, observability) is unchanged; this removes now-unnecessary defensive layers and heavy prose while keeping a short docstring on every function/class. P1 - Remove inner-TimeoutError misclassification heuristic (runner.py): both arms of `except TimeoutError` produced the same recoverable outcome, so the elapsed-vs-effective_timeout branching, `call_start` plumbing, and `_INNER_TIMEOUT_MISCLASSIFICATION_TOLERANCE_SECONDS` only picked a cosmetic span label. Collapsed to one recoverable-timeout path via a new `_record_delegate_timeout()` helper, shared (DRY) by the pre-dispatch budget-exhausted branch and the collapsed except-branch. `except CancelledError` / `except Exception` untouched. P3 - Remove redundant runtime tool-name collision re-check (`_check_delegate_tool_name_collisions`, runner.py): composition-time `existing_tool_names` + `validate_subagent_tool_names` (capabilities.py) already cover user/MCP/workflow/web_request/sandbox names; a delegate-vs-sandbox collision is structurally impossible (`delegate_` vs the fixed `execute_python` name). The only gap (MCP-expanded `.functions`) is backstopped by MAF's own `Agent.run()`, proven by an existing test. Removed the function, its call site, and 4 now-redundant tests. FRD Sec 4.2/4.8 corrected to match. P4 - Investigated whether `_naming.py`'s allocator (`_allocate_unique_name`, `allocate_unique_function_name`, `allocate_unique_builtin_slug`) and endpoints.py's `_builtin_slug_registry`/`_ensure_unique_slug` fallback are dead now that `app.py:_fail_on_duplicate_slugs` is authoritative. Confirmed `create_function_app` already bypasses them in production (passes `slug=`/`function_name=` directly), BUT found real, direct test reliance (5 tests across test_registration_triggers.py and test_registration_endpoints.py exercise the allocator/fallback in isolation). Left that machinery in place per the reviewer's own explicit fallback. Removed the one piece with zero callers anywhere in the repo: `reset_builtin_slug_registry` (endpoints.py + its exports). P5 - Downgraded two "unreachable" guards in `build_subagent_tools` (`catalog is None`, `entry is None`) from multi-line `raise RuntimeError(...)` blocks to one-line `assert`s with a terse comment; updated the two tests to expect `AssertionError`. P2 - Trimmed `_finalize_maf_stream`'s ~88-line docstring (round-3/4 labels, exact MAF source line citations) to ~9 lines covering what/why + the one residual-gap caveat. Function body, its 4 call sites in `run_agent_stream`, and the B1 wait_for-bounded `__anext__` are untouched - this is coordinator-streaming cleanup, not delegation. P6 - Trimmed comment/docstring burden across the delegation path (runner.py, capabilities.py, validation.py, app.py, _naming.py, paths.py): removed changelog/review-round narration (M1-M3, B1-B4, S1-S5, N-labels) and MAF-internal line-number citations from inline comments and docstrings; every function/class keeps a short (1-6 line) docstring stating what it does and, briefly, why when non-obvious. One piece of information that only lived in a trimmed `create_function_app` docstring (app_root resolution precedence) was relocated to `get_app_root()`'s own docstring rather than dropped. Net this round: production code +130/-462 (~366 lines were pure docstring/comment; ~69 were actual removed logic - the collision re-check function+call site, the timeout heuristic/call_start plumbing, the dead reset_builtin_slug_registry, the two verbose guards - offset by ~19 new lines for the DRY timeout helper and the two asserts). Tests +31/-138 (4 tests removed with the code they tested, 2 updated for the new assert/timeout-outcome behavior). No behavior change; 686 tests pass (was 690 pre-round; -4 from P3's redundant tests, matching the removed coverage exactly). Gate: ruff check src tests, mypy src, pytest tests -q all green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e29a2a61-a2ca-4c90-bac9-c21261dd1c48 * Strengthen typing in the delegation footprint, remove Any-laundering Verified against the installed `agent-framework-core==1.3.0` (which ships `py.typed`, so mypy genuinely checks against its real type stubs - confirmed with a throwaway probe before editing) that the delegation code's `Any`-typed values map to concrete MAF types at every point checked. Tightened those annotations so mypy verifies the attribute accesses that were previously permitted by `Any`, and dropped one `str(...)` cast that was laundering an `Any` rather than converting anything real. Root-cause classes (per the audit's own framing): - (a) our own under-annotated code, concrete type known and already used elsewhere in this codebase: `_fail_on_duplicate_slugs`'s and `validate_subagent_references`'s `resolved: Any` -> `ResolvedAgent`; `_record_generic_delegate_failure`'s/`_record_delegate_timeout`'s `span: Any` -> `RuntimeSpan` (already what `current_span()` returns). - (b) MAF returns that are concretely typed: `_build_role_agent`'s `chat_client: Any` -> `SupportsChatGetResponse[Any]` (matches `Agent.__init__`'s own parameter type), `history_provider`/ `context_providers` -> `ContextProvider` (matches `Agent.__init__`'s `context_providers` param), `_build_role_agent`/`_build_delegated_agent` `-> Any` -> `-> Agent[Any]`, `_build_delegate_tool -> Any` -> `-> FunctionTool` and `build_subagent_tools`'s `list[Any]` -> `list[FunctionTool]` (matches this repo's own `@tool(...)` return type). Once `specialist_agent` is `Agent[Any]`, `agent.run(task)` resolves to `Awaitable[AgentResponse[Any]]` and `.text` is a concrete `str` property (verified: never `None`) - so the delegate handler's `result = str(response.text)` cast was laundering an untyped value, not converting anything; simplified to `result = response.text`. - (c) confirmed-and-kept `Any`, with reasons: `workflow_durable_client` (no type stubs for `azure.durable_functions`, and this repo's own `build_workflow_tools` already declares it `Any`); the heterogeneous `tools`/`mcp_tools`/`sandbox_tools`/`web_request_tools` lists (MAF's own `ToolTypes` alias is `FunctionTool | MCPTool | Mapping[str, Any] | object` - the trailing `object` member means even MAF's "concrete" type is maximally permissive by design, so `list[Any]` here is not under-typing, it matches the framework's own contract). Added a `TYPE_CHECKING`-guarded `agent_framework` import for the annotation-only symbols (`Agent`, `ContextProvider`, `SupportsChatGetResponse`) - zero runtime import cost, matching this codebase's existing lazy-import convention for the heavier agent-construction symbols (already used in `sandbox.py`/`_credential.py` for the same reason). `FunctionTool` is imported through this package's own `_function_tool` re-export rather than reaching into `agent_framework` a second way. Scope, per the audit's own instruction: only code this PR added or changed (diffed against `main` per function/file), matching the explicitly named delegation footprint (`runner.py`'s delegate-tool functions, `config/schema.py`, `config/merge.py`, `config/validation.py`, `registration/capabilities.py`, `registration/catalog.py`, `app.py`'s composition additions). `config/schema.py` (`SubagentRef`), `config/merge.py`, and both `registration/` files were already fully typed with no `Any`/getattr in their new code - confirmed by diffing each against `main`, no changes needed. Every other `getattr`/`Any` found in these files was verified pre-existing (predates this PR) and left untouched, per "do not touch pre-existing repo code" - most notably `validate_resolved_agent`'s own `resolved: Any` (only a new keyword argument was added to that pre-existing function) and `_build_agent_session_history`'s pre-existing `tuple[Any, Any, str, ...]` return slots (only its 4th, already-typed, slot is new). Flagged both as natural next candidates for the deliberate later whole-repo typing pass. No behavior change: `agent.run()` is still awaited the same way, the cancel/failure split, timeout math, single-level structural enforcement, and `af.delegate.*` observability are all untouched. Gate: `ruff check src tests` clean; `mypy src` clean (39 files, zero new `# type: ignore`); `mypy --strict` on the three touched files (runner.py, config/validation.py, app.py) also clean; `pytest tests -q` 686 passed (unchanged from before this pass). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e29a2a61-a2ca-4c90-bac9-c21261dd1c48 * Extract bounded session-lock acquire/release into a shared CM The bounded-acquire-then-guaranteed-release pattern (bound the session-lock wait by the coordinator's absolute deadline, then always release) was duplicated ~verbatim in `run_agent` and `run_agent_stream` - the code's own comment even said "see the matching comment in run_agent". Extracted it into one `@contextlib.asynccontextmanager`, `_session_lock_bounded_by(session_id, deadline)`, placed right next to `_get_session_lock` since it wraps that call. Semantics preserved exactly (verified by construction, not just by re-running tests): - The bounded acquire (`wait_for(lock.acquire(), timeout=max(0.0, deadline - now))`) runs before the generator's `yield`, so a `TimeoutError` there propagates out of `__aenter__` before the `async with` body ever starts - `release()` (after `yield`, in `finally`) is structurally unreachable on that path, exactly matching the old "acquire never won, so no release needed" comment. - Once acquired, `release()` always runs in `finally`, on any exit path from the body - success, an ordinary exception, or a BaseException (cancellation) - identical to the old explicit `finally: lock.release()`. - `run_agent`: merged the old two separate `except TimeoutError:` blocks (one around the acquire, one around the post-lock `agent.run()` call) into one, wrapping the `async with`. Both branches already raised the exact same `RuntimeError(f"Agent run timed out after {timeout}s")` - confirmed by reading the pre-refactor code line by line - so merging them changes nothing observable and incidentally removes a second copy of that literal string. - `run_agent_stream`: confirmed the streaming body has its OWN self-contained `except TimeoutError as exc:`/`except Exception as exc:` handlers (they yield their own SSE error frame and do not re-raise), so a body-level `TimeoutError` (e.g. the per-update stream-pull timeout) is fully absorbed *inside* the body and can never reach the new outer `except TimeoutError:` - that clause is reachable only from the CM's own pre-yield acquire bound, exactly matching the previous acquire-only handler. Dropped the old handler's explicit `return` after its `yield`, since falling off the end of the `except`/`finally` is the last code in the function - behaviorally identical for an async generator. - Both call sites still keep their own outer `try`/`except`/`finally` around the `async with` for their respective timeout reactions (`RuntimeError` for `run_agent`; the SSE error frame + span attributes for `run_agent_stream`) - the CM itself only acquires, yields, and releases. Did not touch `_get_session_lock`/`_SESSION_LOCKS` (pre-existing, load-bearing, unrelated to this dedup) or anything in the delegation path. Added 3 focused unit tests directly against the new CM (isolated from the full `run_agent`/`run_agent_stream` machinery): releases after a successful body, releases when the body raises, and does not touch the lock's state when the bounded acquire itself times out (proving no leak and no double-release on the existing holder's side). The two pre-existing "M1" lock-contention tests for `run_agent`/ `run_agent_stream` pass unchanged, confirming the observable behavior through the public functions is unaffected. Gate: `ruff check src tests` clean; `mypy src` clean (40 files); `mypy --strict` on `runner.py` clean; zero new `# type: ignore`; `pytest tests -q` 707 passed, 9 deselected (704 before + 3 new tests). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e29a2a61-a2ca-4c90-bac9-c21261dd1c48 * Type run_agent's response access to match the delegate path `run_agent`'s response-text/messages extraction predates the delegation typing pass and was still going through `getattr(...)` even though `response` is a real MAF `AgentResponse[Any]` (the result of `agent.run(...)` on the same `Agent` type the delegate path already uses). Annotated `response: AgentResponse[Any]` at its assignment (the call site itself can't be re-typed without touching `_build_agent_session_history`'s pre-existing, out-of-scope `agent: Any` return slot, so this narrows `Any` -> `AgentResponse[Any]` at the one place it's actually consumed - a standard, mypy-verified pattern) so `.text`/`.messages` resolve without `getattr`: - `text = str(getattr(response, "text", "") or "")` -> `text = response.text` (same as the delegate path - `.text` is the same concrete `@property -> str`, never `None`). - `for msg in getattr(response, "messages", None) or []:` (both occurrences) -> `for msg in response.messages:` (`AgentResponse.__init__` always assigns a `list[Message]`, never `None`, so the `or []` fallback was already unreachable). Left every dynamic MAF content-item `getattr` untouched per scope: `getattr(msg, "contents", ...)`, `getattr(item, "call_id"/"name"/ "arguments"/"result", ...)` iterate MAF's `Contents` union, where per-type fields genuinely differ and aren't cleanly typeable without isinstance dispatch - that's the later whole-repo pass's job, not this one. The enclosing `try/except Exception` blocks around both extraction loops are also left as-is (they still guard the untouched dynamic content-item access below the now-typed `.messages`/`.text` lines). No behavior change - both forms yield the identical string/list for every input `AgentResponse` can construct. Gate: `ruff check src tests` clean; `mypy src` clean (40 files); `mypy --strict` on `runner.py` clean; zero new `# type: ignore`; `pytest tests -q` 707 passed, 9 deselected (unchanged). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e29a2a61-a2ca-4c90-bac9-c21261dd1c48 * Type validate_resolved_agent's resolved param (last pre-existing spot) Pulled this one pre-existing function in because this PR already added a keyword arg (`is_referenced_as_subagent`) to its signat…

Uh oh!

Add a Version to Global config file #12

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions