mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
feature/python-hosting
36
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  
|
5ac864dfd9 |
Updating versions for release 1.6.2 (#6019)
Co-authored-by: alliscode <bentho@microsoft.com> |
||
 
|
b559545fa4 |
.NET: Fix declarative workflow regressions for hosted agents (#5905)
* Fix declarative workflow regressions for hosted agents
Three regressions surfaced when running a declarative workflow as a
Foundry hosted agent. Together they caused every condition group to fall
through to elseActions and the raw agent JSON to leak to the caller.
1. AgentProviderExtensions.InvokeAgentAsync forced autoSend to true
whenever the agent ran on the workflow conversation, which overrode
the explicit autoSend: false declared in workflow.yaml and streamed
the raw structured-output JSON straight to the user. Honor the
caller-supplied autoSend instead.
2. IWorkflowContextExtensions.ReadState / QueueStateUpdateAsync /
QueueStateResetAsync took the variable name and namespace alias
directly from PropertyPath.VariableName / NamespaceAlias. Against
Microsoft.Agents.ObjectModel 2026.2.4.1 those properties return null
for a dotted reference such as `Local.Triage` even when
SegmentCount == 2 and IsValid == true, so every assignment threw
ArgumentNullException via Throw.IfNull. Fall back to Segments() to
reconstruct the name and alias when the parser returns null.
3. The same ObjectModel version no longer recognizes the user-facing
`Local` scope alias: VariableScopeNames.IsValidName(`Local`)
returns false and GetNamespaceFromName(`Local`) returns Unknown, so
the declarative interpreter's IsManagedScope check fails and the
State.Set call is silently skipped. Translate the `Local` alias to
its canonical `Topic` form before forwarding to
QueueStateUpdateAsync; WorkflowFormulaState.Bind continues to expose
it as `Local` to PowerFx.
Verified end-to-end against a deployed Foundry hosted agent: the
declarative triage workflow now routes Technical / Billing / General
inputs correctly and only the autoSend-eligible messages reach the
caller.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Hosted-agent HITL: persist session across previous_response_id chains; run approved local AIFunctions
Two regressions hit declarative workflows that use require_approval=true when
the client chains turns via previous_response_id (no conversation_id):
1. AgentFrameworkResponseHandler keyed the AgentSession store solely on
conversation_id, so when only previous_response_id was present the
StateBag (which holds ToolApprovalIdMap) was discarded after each turn.
The next turn then threw 'No approval mapping recorded for wire id ...'
in InputConverter.ConvertMcpApprovalResponse.
Fix: fall back to previous_response_id on load and to context.ResponseId
on save so the response-id chain becomes a valid session key. Conversation
id remains preferred when present.
2. InvokeFunctionToolExecutor.CaptureResponseAsync only acted on
FunctionResultContent. In the hosted Foundry path the approval response
arrives as a ToolApprovalResponseContent with no FunctionResultContent,
so the local AIFunction never ran and downstream PropertyPath/SendActivity
consumers (e.g. {Local.RefundResult}) saw empty values.
Fix: when no FunctionResultContent matches but an approved
ToolApprovalResponseContent does, look up the registered AIFunction by
name on agentProvider.Functions and invoke it with the evaluated
arguments, surfacing the result through the existing assignment path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Apply PropertyPath workaround to initialization path; share + tidy helpers
Address PR #5905 review feedback:
* Move the PropertyPath VariableName/NamespaceAlias fallback and 'Local'
-> 'Topic' scope remap into a shared internal PropertyPathExtensions
helper. Materializes Segments() once, names the magic 'Local' alias
as a const, and carries a TODO referencing the tracking issue.
* Apply the same helper in WorkflowDiagnostics.InitializeDefaults so a
declared default for a dotted variable like 'Local.Triage' is no
longer silently skipped at workflow startup (closes the gap flagged
by the reviewer: runtime ReadState/QueueStateUpdateAsync worked but
state.Initialize did not).
* Restore the previous strict failure mode on namespace alias by
wrapping GetNamespaceAlias() in Throw.IfNull at call sites so a
malformed single-segment path keeps failing fast rather than
silently passing null to State.Get/Set.
All 821 unit tests pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add tests for AgentProviderExtensions.InvokeAgentAsync autoSend behavior
Covers the autoSend regression fix: when the agent runs on the workflow conversation with autoSend=false, no AgentResponseUpdateEvent or AgentResponseEvent is added to the context. Also covers autoSend=true (events emitted) and autoSend=false on a non-workflow conversation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Surface SendActivity output via AgentResponseUpdateEvent
SendActivityExecutor previously only emitted the activity text via YieldOutputAsync, which the runtime converts to an AgentResponseEvent. WorkflowSession gates AgentResponseEvent behind includeWorkflowOutputsInResponse, so when a host opts out of summary outputs (the default for AsAIAgent) the SendActivity reply is silently dropped.
Mirror the pattern used by AgentProviderExtensions for autoSend agent invocations: also emit an AgentResponseUpdateEvent, which WorkflowSession yields unconditionally. This makes SendActivity reliably reach chat-protocol clients without requiring includeWorkflowOutputsInResponse = true (which would also duplicate autoSend agent output).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Revert previous_response_id session-key fallback
The fallback let a session be keyed by an unbroken previous_response_id chain,
but conversation_id is the right way to thread state across turns: it survives
shared/branched chains (e.g. when another agent generates a response in between)
and is the documented model for stateful clients. Restore conversation_id as the
sole session key and rely on the client to thread it. The InvokeFunctionTool
approval/local-function half of
|
||
|
|
8e54f0b0e7 |
Python: Shell tool with support for local and Docker (#5664)
* feat(tools): add cross-OS LocalShellTool in new agent-framework-tools package Introduces a safe, cross-OS local shell tool as the first citizen of a new agent-framework-tools workspace package. Supports persistent (default) and stateless modes across pwsh/powershell.exe/bash/sh, with policy denylist, allowlist, approval gating, process-tree kill on timeout, output truncation, and audit hooks. Integrates with existing provider get_shell_tool(func=...) factories via FunctionTool kind='shell'. See docs/decisions/0026-builtin-tools-local-shell.md for the full design. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(tools): security hardening for LocalShellTool Codifies what LocalShellTool does and does not defend against, and delegates the security-relevant lifecycle primitive to a battle-tested library instead of hand-rolled per-OS code. Changes: - Adopt psutil for cross-OS process-tree termination (executor + session). Replaces hand-rolled taskkill/killpg with one canonical implementation. - Resolve taskkill.exe to absolute %SystemRoot%\System32 path so PATH poisoning cannot redirect us to an attacker-supplied binary. - Reframe ShellPolicy docstring + ADR + README: denylist is a guardrail, not a security boundary. - Require acknowledge_unsafe=True to set approval_mode='never_require', making the unsafe path explicitly opt-in with a self-documenting name. - Add tests/test_security.py codifying named CVE-style cases. Defenses we DO claim are asserted; non-defenses (denylist bypasses via backslash insertion, variable expansion, interpreter escape, base64, alternative tools, PowerShell-native verbs) are documented as expected-to-pass tests so residual risk stays visible. - Add Threat Model + Confidence Strategy sections to ADR 0026. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(tools): add DockerShellTool sandboxed shell tier Adds a container-backed shell executor as the recommended pattern for untrusted-input shell workflows. The container provides the security boundary (--network none, non-root user, --read-only, --cap-drop ALL, no-new-privileges, memory/pids limits, tmpfs /tmp), so approval gating is optional unlike LocalShellTool. Also introduces a ShellExecutor Protocol so callers can plug in custom backends (Firecracker, SSH, WASI) without forking the framework. Removes the planned HyperlightShellExecutor follow-up from ADR 0026: Hyperlight is a WASM code sandbox with no kernel/userland/shell binary, so a Hyperlight-backed shell is not viable. Docker is the realistic sandbox tier for shell. Tests: 11 unit tests for argv builders + lifecycle (no Docker daemon required); 3 integration tests gated on is_docker_available(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(tools): backport shell-tool fixes from .NET parity review Applies the applicable subset of bug fixes accumulated during the .NET shell-tool PR review (microsoft/agent-framework#5604) to the Python shell tool. A1 - Quote workdir safely in _maybe_reanchor Previously _tool.py used double-quote interpolation when emitting the cd/Set-Location prefix, which expanded $VAR, $(), and backticks in the workdir path. A workdir containing shell metacharacters could trigger arbitrary command execution before the user command ran. Replaced with single-quote escaping helpers _quote_posix and _quote_powershell that emit literal-string forms safe for both hosts. A5/A6 - Consolidate truncation to a single byte-aware helper Extracted a shared truncate_head_tail / truncate_text_head_tail helper in _truncate.py. The new implementation distributes odd caps so head receives floor(cap/2) and tail receives ceil(cap/2) bytes, matching the .NET round-9 fix and ensuring no input bytes are silently dropped on the boundary. _session.py previously truncated by Python str length while the caller passed _max_output_bytes - the unit mismatch is now gone: raw byte buffers go through truncate_head_tail and decoded text goes through truncate_text_head_tail. Unit tests added for the truncate and quote helpers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(tools): tone down narrative and overconfident comments in shell tool The shell tool's docstrings and comments contained two patterns that the .NET review pushed back on: - Narrative framing about implementation history ("hard-won", "we sidestep", "design inspiration: ...", competitor framework name-drops in module docstrings). - Overstated security guarantees ("battle-tested", "reasonable for untrusted input", "recommended executor for any agent that runs commands from untrusted input", "destructive commands are blocked", "safe local shell tool", "blocks shell injection"). Rewrites the affected docstrings and comments to describe what the code does in neutral terms. Behaviour is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(tools): add ShellEnvironmentProvider for the Python shell tool Ports the .NET ShellEnvironmentProvider as a Python ContextProvider so agents using LocalShellTool or DockerShellTool can be primed with an accurate description of the shell they're talking to (family, version, OS, working directory, and which CLIs are available). The provider runs probes through any ShellExecutor, caches the resulting snapshot, and on every before_run extends the session instructions with a markdown block describing the shell idiom to use. A failed first probe leaves the cache empty so the next call retries (no permanent poisoning). Probe failures from a narrow set of expected error types (ShellCommandError, ShellExecutionError, ShellTimeoutError, and asyncio.TimeoutError from the per-probe timeout) are recorded as None fields in the snapshot. Other exceptions propagate. Tool names are validated against ^[A-Za-z0-9._-]+$ before being interpolated into a probe command. Includes 12 unit tests covering happy path, stderr fallback, timeout handling, expected/unexpected exception paths, malicious tool name rejection, case-insensitive deduplication, retry after failure, concurrent first-callers sharing one probe, and the default and custom formatter paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(tools): document ShellEnvironmentProvider and finish comment cleanup Add a README section introducing ShellEnvironmentProvider, soften two remaining overconfident security-boundary comments in _executor_base.py and the DockerShellTool class docstring, and add a sample (shell_with_environment_provider.py) that demonstrates the provider in stateless and persistent modes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(tools): move shell samples to python/samples/02-agents/tools The repository convention is to host samples under python/samples/ rather than inside the package directory. Move the two net-new shell samples (allow-list and environment-provider) to python/samples/02-agents/tools/ and drop the in-package samples/ directory; the existing top-level providers/openai/client_with_local_shell.py already covers the basic LocalShellTool walkthrough. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(tools): cover confine_workdir default and ShellResult.format_for_model Two new tests in test_local_shell_tool.py exercise the default confine_workdir=True behaviour on POSIX and PowerShell, asserting that 'cd' inside one persistent-mode call does not leak into the next. A new test_shell_result.py module provides direct unit coverage for every conditional branch of ShellResult.format_for_model (stdout, truncated, stderr, timed_out, exit_code) so regressions in the LLM-facing format are caught immediately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(tools): address PR #5664 review feedback - _tool.py: detect PowerShell via is_powershell() helper instead of basename string match - _environment.py: use public ContextProvider import (no private _ prefix) - _session.py: trim _stdout_buf/_stderr_buf after copying to avoid unbounded retention across calls - _docker.py: short-circuit start()/close() in stateless mode; add configurable shell kwarg (default bash, e.g. 'sh' for alpine) - tests: parenthesized multi-line assert; alpine integration tests now pass shell='sh' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(tools): satisfy CI quality gates - pyupgrade: drop quoted self-class refs in __aenter__/method annotations - ruff format: reflow long lines per workspace style - pyright: assert psutil non-None in optional-import branch; lowercase mutable module globals; annotate _approval_mode as Literal so tool() Literal-typed kwarg is accepted; add ... body to ShellExecutor.run protocol; remove unused deprecated _kill_tree wrapper - tests: skip docker integration tests on win32 (Windows containers don't support --read-only / alpine images) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove DEFAULT_DENYLIST; document single-session ownership; fix bandit findings Mirrors the .NET PR #5604 cleanup: - Remove DEFAULT_DENYLIST from ShellPolicy. ShellPolicy() now ships with an empty deny-list; operators opt into site-specific patterns explicitly. No major agent framework uses regex matching as a primary security control; AutoGen v2 removed theirs. Approval gating + sandbox tier remain the real boundaries. - Rewrite module / class docstrings to frame ShellPolicy as a UX pre-filter, not a security control. - Add Single-session ownership paragraphs to ShellExecutor, ShellSession, LocalShellTool, and DockerShellTool: a persistent-mode tool is owned by exactly one conversation / agent session; do not share across users or concurrent conversations. - Tests now supply explicit deny patterns instead of relying on a default. - Address Pre-commit Hooks (bandit) CI failures: convert internal-invariant asserts to explicit RuntimeError, annotate intentional subprocess/shell usage with # nosec, document container-internal /tmp paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5664 round-2 review feedback Deny-list documentation drift: - README and the OpenAI/local-shell sample no longer claim a built-in deny-list of destructive commands. ShellPolicy is described as an optional, operator-supplied UX pre-filter; the real boundaries remain approval gating and the sandbox tier. Behavioural fixes called out in review: - ShellPolicy.evaluate() now denies empty / whitespace-only commands explicitly instead of returning allow with no rationale. - truncate_head_tail() raises ValueError for cap <= 0 instead of silently returning the full input with truncated=False, which previously could defeat output-capping in callers that mis-configured the budget. - LocalShellTool.as_function() / DockerShellTool.as_function() return the ShellCommandError text directly so the model sees a single, non-redundant 'Command rejected by policy: …' message instead of the prior duplicated 'Command blocked by policy: Command rejected …' wrapping. - ShellSession POSIX sentinel trailer now snapshots and restores the prior errexit (set -e) state around the trailer, so a user 'set -e' in the persistent shell is no longer permanently disabled by the next run(). Tests: - New test_shell_parse_rc.py covers the full _parse_rc() edge-case surface (zero, positive, negative, CRLF, no newline, missing prefix, empty input, non-digits, trailing garbage, partial digits). - test_policy.py asserts the new empty-command deny. - test_shell_truncate_and_quote.py asserts ValueError for cap=0 and cap<0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review feedback for shell tool - _resolve.py: reject empty/whitespace shell override string - _tool.py / _docker.py: mode-aware default tool description (persistent vs stateless) - _tool.py: fix misleading workdir docstring (re-anchor, not blocking) - _types.py: emit stream-agnostic [output truncated] marker - _policy.py: declare _denies/_allows as dataclass fields - _environment.py: use $(pwd) instead of $PWD in POSIX probe Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review feedback: shell override flag + probe timeout safety - _resolve.py: in stateless mode, ensure shell overrides end with -c/-Command so commands aren't misinterpreted as script-file paths. - ShellExecutor.run / LocalShellTool.run / DockerShellTool.run now accept an optional imeout kwarg; ShellEnvironmentProvider drops the outer asyncio.wait_for and lets the executor enforce the probe timeout internally, so cancellation no longer risks leaving a hung subprocess or corrupted session. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback: docker isolation + lifecycle robustness - pyproject.toml: bump agent-framework-core minimum from 1.2.0 to 1.2.2 to align with the rest of the workspace. - _docker.py: validate extra_run_args at construction time and reject flags that would dismantle the isolation defaults (--privileged, --cap-add, --security-opt, --network/--net, -v/--volume/--mount, --device, --pid, --ipc, --userns, --user, --read-only, --tmpfs, --add-host, --gpus, --cgroupns, --device-cgroup-rule); also documented the warning on the docstring. - _docker._stop_container: retry docker rm -f once and log a warning/error when it does not succeed, so operators can audit leaked containers instead of getting a silent success. - _docker._run_stateless timeout path: fall back to docker rm -f when docker kill fails or times out (--rm only reaps on clean exit), and log instead of silently swallowing communicate() errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: alliscode <25218250+alliscode@users.noreply.github.com> |
||
|
|
741259476f |
Fix CA1873 in DevUI by using LoggerMessage source generator (#5831)
Replaces two ILogger.LogWarning(string, params object?[]) calls in DevUIAuthFilter and DevUIExtensions with allocation-free [LoggerMessage] partial methods on a new internal DevUILog class. Preserves original message templates and structured property names ({RemoteIp}, {EnvVar}).
Co-authored-by: alliscode <25218250+alliscode@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
||
|
|
574631671d |
Update version for release. (#5789)
Co-authored-by: alliscode <25218250+alliscode@users.noreply.github.com> |
||
|
|
981726cc15 |
.NET: feat(evals): add ground_truth/expected_output support for workflow evaluation (#5755)
* .NET: feat(evals): add ground_truth/expected_output support for workflow eval Brings .NET to parity with Python PR #5234 for issue #5135: - Add expectedOutput parameter to Run.EvaluateAsync (workflow) and stamp on the overall EvalItem.ExpectedOutput. - Map EvalItem.ExpectedOutput -> ground_truth in the Foundry JSONL payload, item_schema, and data_mapping for similarity. - Add GroundTruthEvaluators set (currently builtin.similarity) and a FindMissingGroundTruthEvaluators helper. - Fail fast with InvalidOperationException when a ground-truth evaluator is selected but no item provides an ExpectedOutput, instead of surfacing a remote provider error. - Add tests in FoundryEvalConverterTests and WorkflowEvaluationTests. - Add Evaluation_WorkflowExpectedOutputs sample (workflow + Foundry similarity). Fixes microsoft/agent-framework#5135 (.NET side). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: relax BuildOverallItem events to IReadOnlyList<WorkflowEvent> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Sample: disable per-agent breakdown when using reference-based evaluator Per-agent EvalItems are intentionally left without ExpectedOutput, so the new fail-fast validation in FoundryEvals would throw when Similarity is invoked for per-agent items. Pass includePerAgent: false in the workflow + similarity sample, and document this gotcha in the EvaluateAsync XML doc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix BuildOverallItem: fall back to last ExecutorCompletedEvent AgentResponseEvent is only emitted when AIAgentHostOptions.EmitAgentResponseEvents is enabled, which is not the default for WorkflowBuilder(agent).AddEdge(...). When it is absent, fall back to the last non-internal ExecutorCompletedEvent whose Data is an AgentResponse / ChatMessage / string so the overall EvalItem (and any expectedOutput) is produced. Without this, samples wired up the standard way returned 0 evaluation items. Update test to cover the fallback path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Sample: enable EmitAgentResponseEvents; eval throws clear error when no overall response found Root cause of '0 results': AIAgentHostExecutor only emits AgentResponseEvent when AIAgentHostOptions.EmitAgentResponseEvents is true (default false). For ordinary AIAgent executors the runtime's ExecutorCompletedEvent.Data is null, so the prior fallback couldn't find a final response either. Sample now builds executors with EmitAgentResponseEvents=true via BindAsExecutor(hostOptions). EvaluateAsync now throws InvalidOperationException with a remediation hint when the user supplies expectedOutput but no overall final response can be located, instead of silently returning 0/0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Guard against null sample/error/usage/datasource_item in ParseDetailedItem Foundry eval responses can have these properties present with JSON null or non-object values, which caused JsonElement.TryGetProperty to throw 'requires Object, has Null'. Check ValueKind == Object before drilling in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review: reorder expectedOutput, tighten ground-truth check, add fail-fast test * WorkflowEvaluationExtensions.EvaluateAsync: move 'expectedOutput' to after 'splitter' so the original positional contract of (splitter, cancellationToken) is preserved for existing callers. * FoundryEvals: require ALL items to carry ExpectedOutput when a ground-truth evaluator is selected (e.g. similarity), not just any. Reference-based evaluators score per-item, so a single missing GT would still surface as a provider-side validation error. Updated fail-fast message accordingly. * WorkflowEvaluationTests: add EvaluateAsync_WithExpectedOutputButNoFinalResponse_ThrowsAsync to verify the InvalidOperationException is thrown (and that the message mentions EmitAgentResponseEvents). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fail-fast on missing overall item regardless of expectedOutput; harden BuildOverallItem default * EvaluateAsync now throws InvalidOperationException whenever 'includeOverall' is requested but BuildOverallItem cannot produce an item, instead of only when 'expectedOutput' is supplied. Same misconfiguration (agents not bound with EmitAgentResponseEvents) used to silently return empty results — now it surfaces a clear, actionable error in both cases. * BuildOverallItem switch default now throws instead of returning null. The preceding for-loop already constrains Data to AgentResponse/ChatMessage/ string, so reaching default would indicate a contract drift; throw to make the bug visible. * Test renamed and broadened to verify the throw fires without expectedOutput. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <25218250+alliscode@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
bd0d6070f1 |
Fixing FoundryToolboxMcp sample to use created toolbox. (#5786)
Co-authored-by: alliscode <bentho@microsoft.com> |
||
|
|
4409b00b86 |
.NET: Feat/dotnet shell tool (#5604)
* feat(dotnet): add Microsoft.Agents.AI.Tools.Shell with LocalShellTool Ports Python LocalShellTool to .NET as a new package (net8/9/10). - Microsoft.Agents.AI.Tools.Shell: LocalShellTool, ShellPolicy (deny-list guardrail), ShellResolver (cross-OS pwsh/powershell/cmd vs bash/sh), ShellResult with head+tail truncation, timeout + process-tree kill, AsAIFunction with required-by-default human approval gate. - Persistent mode via ShellSession (sentinel protocol over pwsh/bash). - acknowledgeUnsafe parity gate matches the Python implementation. - Auto-injected platform context in the AIFunction description so the LLM sees the active OS and shell at tool-discovery time. - 17 xunit.v3 tests cover policy allow/deny, echo roundtrip, exit codes, timeout/kill, AsAIFunction shape + approval wrapping, persistent cwd/env carry-over, head+tail truncation, sentinel race. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(shell): close Python parity gaps for LocalShellTool Closes the .NET vs Python parity gaps identified in the competitive eval: - Default mode flipped to ShellMode.Persistent (matches Python). Every call now reuses a long-lived shell so cd/exports/functions persist; pass mode: ShellMode.Stateless to opt out. - New IShellExecutor interface — pluggable backend so future DockerShellTool / Hyperlight / SSH executors don't fork the framework. LocalShellTool implements it. - Workdir confinement: confineWorkingDirectory (default true) re-anchors every persistent-mode command back to workingDirectory so a wandering cd in one call doesn't leak to the next. Mirrors Python _maybe_reanchor. - Graceful interrupt on timeout: ShellSession sends SIGINT (POSIX) or Ctrl+C-on-stdin (Windows) before falling back to a hard close+respawn. Successfully-interrupted commands return exit 124 + TimedOut=true while preserving session state for the next call. - cleanEnvironment opt-in: when true, only PATH/HOME/USER/USERNAME/ USERPROFILE/SystemRoot/TEMP/TMP plus user-supplied vars are visible. - shellArgv: IReadOnlyList<string> override accepted alongside the string shell binary param (mutually exclusive). Lets advanced callers inject flags like --rcfile or --login. - Typed exceptions ShellTimeoutException and ShellExecutionException replace InvalidOperationException for launch / liveness failures. Tests: 17 -> 23. New cases cover persistent-default ctor, mutually- exclusive shell/shellArgv, confined re-anchor, confine-disabled leak, clean-env strip, and IShellExecutor implementation. All green on net10.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(shell): add DockerShellTool sandboxed shell tier Ports the Python DockerShellTool to .NET. Mirrors the public surface of LocalShellTool but executes commands inside an isolated container, where the container is the security boundary. Stateless and persistent modes both supported; persistent mode reuses ShellSession by launching 'docker exec -i <ctr> bash --noprofile --norc' as the long-lived REPL, so the sentinel protocol works unchanged. Defaults chosen for safety: - --network none, --user 65534:65534 (nobody), --read-only root - --cap-drop=ALL, --security-opt=no-new-privileges - 512m memory cap, pids-limit 256, --tmpfs /tmp - Optional host workdir mount, ro by default Public surface: - DockerShellTool ctor with image/container_name/mode/host_workdir/ workdir/network/memory/pids_limit/user/read_only_root/extra_run_args/ environment/policy/timeout/max_output_bytes/on_command/docker_binary - StartAsync, CloseAsync, RunAsync, AsAIFunction, IShellExecutor impl - IsAvailableAsync(binary) probe - Static argv builders (BuildRunArgv, BuildExecArgv) — pure, side- effect free, so unit tests don't need a Docker daemon AsAIFunction defaults to requireApproval: false (the container IS the boundary). LocalShellTool keeps the opposite default. Tests: 23 -> 35. 12 new tests cover argv builders, env/extra-args/host- workdir flags, exec interactive vs stateless, container name uniqueness, IShellExecutor implementation, AsAIFunction approval defaults, and IsAvailableAsync false-path. None require Docker. Multi-TFM build (net8/9/10) green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(shell): add DockerShellTool integration tests Adds 9 end-to-end tests that exercise DockerShellTool against a live Docker (or Podman) daemon. Tests are tagged [Trait("Category", "Integration")] and auto-skip via Assert.Skip when no daemon is available, so they are CI-safe. Coverage: - IsAvailableAsync probe - Persistent mode basic command + state preservation across calls - --network none blocks outbound DNS - --read-only root prevents writes outside /tmp; /tmp tmpfs is writable - --user 65534:65534 (nobody) is in effect - Stateless mode: env vars do not leak across calls - HostWorkdir bind-mount + read-only enforcement - Environment variables passed via -e Tests use debian:stable-slim (alpine ships only busybox sh, which ShellSession persistent bash REPL cannot drive). Run locally: dotnet test --filter "Category=Integration" or filter by class on the test exe directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style(shell): apply dotnet format pass - Whitespace and code-style fixes from `dotnet format` across both projects - Convert all new files to UTF-8 with BOM and LF line endings (repo convention) - Rename ShellSession statics to s_ prefix (IDE1006) - Add Async suffix to async test methods (IDE1006) No behavioral changes. All 44 tests still pass on net10.0; multi-TFM build (net8/net9/net10) green. `dotnet format --verify-no-changes` now reports clean for both projects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(shell): add DockerShellTool walkthrough with sequence diagrams Explains the mental model (we shell out to the docker CLI; we never speak the engine API), the hardened docker run argv, persistent vs stateless lifecycles with mermaid sequence diagrams, the full agent-to-bash call ladder, and the failure modes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * PR 5604 review fixes (group a): libc DllImport, namespace cleanup, policy-msg dedup Three quick-win review comments on PR #5604: 1. ShellSession: the libc `killpg` P/Invoke was annotated with `DllImportSearchPath.System32`, a Windows-only loader hint that does nothing for libc.so on POSIX. Switched to `SafeDirectories` (CA5392 /CA5393 clean) and added a comment noting the call site is gated to non-Windows. 2. DockerShellToolTests: replaced the fully-qualified `Extensions.AI.ApprovalRequiredAIFunction` with a `using Microsoft.Extensions.AI;` import and the bare type name, matching `LocalShellToolTests`. 3. LocalShellTool / DockerShellTool: `AsAIFunction`'s catch block was producing a doubled "Command blocked by policy: Command rejected by policy: ..." prefix because the `ShellPolicyException` message already starts with "Command rejected by policy". Now we return `ex.Message` directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * PR 5604 review fix (group b): add ShellKind.Sh for /bin/sh fallback Review comment (#3): when /bin/bash is missing the resolver fell back to /bin/sh but tagged it as ShellKind.Bash, so the launcher passed bash-only flags --noprofile --norc to dash/ash/busybox, which interpret them as positional script names. Fix: * Added ShellKind.Sh for minimal POSIX shells (sh, dash, ash, busybox). * /bin/sh fallback is now tagged Sh. * ClassifyKind maps "SH" / "DASH" / "ASH" / "BUSYBOX" binary names to Sh. * StatelessArgvForCommand emits just `-c <command>` for Sh (no bash-only flags); PersistentArgv emits no flags at all. * LocalShellTool's system-prompt builder describes Sh distinctly and warns the model away from bash-only constructs. Tests: ShellResolverTests covers Sh/Bash classification through the observable argv output (14 new theory cases). Total: 58/58. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * PR 5604 review fix (group d): honor timeout=null, add DefaultTimeout Review comment (#5): both LocalShellTool and DockerShellTool documented `timeout: null` as "disables timeouts" but the constructor coerced null to 30 seconds, making the documented disable mechanism unreachable through the public API. Fix: * Drop the `?? TimeSpan.FromSeconds(30)` coercion in both ctors. `_timeout` now faithfully reflects what the caller passed (null = disabled). The downstream CTS-construction sites already short-circuit on null, so no other code changes are required. * Add `public static readonly TimeSpan DefaultTimeout` (30 s) on both tools so callers who want a bounded timeout can opt in explicitly. Tests: * New `RunAsync_NullTimeout_DoesNotTimeOutAsync` confirms a quick command runs to completion when the caller passes `timeout: null`. * New `DefaultTimeout_IsThirtySeconds` documents the constant. Behavioral note: this is a deliberate change-of-default. Callers that previously omitted `timeout` and relied on the implicit 30 s now get "no timeout". They should pass `LocalShellTool.DefaultTimeout` or `DockerShellTool.DefaultTimeout` explicitly to preserve the prior behavior. Tests: 60/60 (44 baseline + 14 resolver + 2 new timeout tests). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * PR 5604 review fix (group e): smart requireApproval default for DockerShellTool Review comment (#6, design): requireApproval: false baked in a safety decision the type cannot prove on its own. Callers can weaken any isolation knob (network, user, readOnlyRoot, mount, extraRunArgs) and still get an unapproved tool by default. Fix: * New public IsHardenedConfiguration property returns true iff the effective config matches the safe defaults: network=="none", non-root user, read-only root, host mount (if any) read-only, no extra run args. * AsAIFunction's requireApproval parameter is now bool? defaulting to null. When null, approval is enabled iff IsHardenedConfiguration is false. Pass false explicitly to opt out, or true to force. * docker-shell-tool.md updated with the new approval matrix. Tests: 4 new theory cases + 2 facts cover hardened-default, relaxed-network, root-user, writable-root, extraRunArgs, and explicit-opt-out branches. Total: 66/66. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * PR 5604 review fix (group c): wrap POSIX shell in setsid for correct killpg Review comment (#1): killpg(proc.Id, SIGINT) only behaves like a process-group signal when proc.Id IS a process group id. Since the .NET launcher does not call setsid() / setpgid() itself, the spawned shell inherits the agent host's process group — so killpg targeted the wrong group and the cancel signal could leak to the agent. Fix: * On non-Windows, EnsureStartedAsync probes for setsid (well-known paths first, then PATH). When found it wraps the shell launch as `setsid <shell> <args...>` so the spawned shell becomes a session leader (PID == PGID). * A new _isSessionLeader flag tracks whether the wrap succeeded. * InterruptCurrentCommandAsync only calls killpg when _isSessionLeader is true. Without setsid, killpg on an unsuited PID could signal the agent itself, so we skip the fast path and let the caller's hard close-and-respawn handle the timeout. * Windows behaviour is unchanged (Ctrl+C-via-stdin to pwsh). No public-API changes; existing tests cover the interrupt path and all 66/66 still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * .Net: DockerShellTool design + caller-cancel container leak fixes (PR #5604) Addresses three Copilot review findings on PR #5604. Design (group f): * StartAsync: change inner ResolvedShell from ShellKind.Bash to ShellKind.Sh. BuildExecArgv() already includes `--noprofile --norc` in ExtraArgv; Bash's PersistentArgv() was appending those flags a second time, yielding `bash --noprofile --norc --noprofile --norc`. Sh's PersistentArgv() returns Array.Empty so ExtraArgv is forwarded unchanged. * BuildExecArgv: remove the dead `interactive: false` branch and the `interactive` parameter. The `false` path produced an unusable argv ending in `-c` with no command and was never invoked internally (stateless mode uses BuildRunArgvStateless). Updated tests and docs/docker-shell-tool.md sequence diagram. Reliability (group g): * RunStatelessAsync: add a second `catch (OperationCanceledException)` guarded on `cancellationToken.IsCancellationRequested` that issues `docker kill --signal KILL <perCallName>` before rethrowing. Previously, caller-driven cancellation bypassed the timeout-only catch and propagated without killing the container; because `--rm` only fires when PID 1 exits, the container ran indefinitely. Extracted the kill-by-name logic into a `BestEffortKillContainerAsync` helper shared by both the timeout and caller-cancel paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * .Net: Fill PR #5604 test coverage gaps for Shell tools Addresses the test-coverage findings in the latest Copilot review. * ShellResultTests (new): direct branch coverage for ShellResult.FormatForModel() — empty stdout, non-empty stderr, truncated, timed-out, success, and the truncated-with-empty-stdout edge where the marker is intentionally suppressed. This method's string is what the language model sees, so it benefits from explicit unit-level coverage independent of integration tests. * ShellSessionTests (new): direct unit tests for the internal TruncateHeadTail head-tail truncation utility — under-cap (no truncation), exactly at cap (no truncation), over-cap (truncated with marker, both head and tail preserved), and empty-string. Reachable via InternalsVisibleTo. * LocalShellToolTests: Theory test exercising 8 representative patterns from ShellPolicy.DefaultDenyList (rm -rf /, mkfs.ext4, curl|sh, wget|sh, Remove-Item /, shutdown, reboot, Format-Volume) to catch deny-list regex regressions; previously only 1/16 was tested. * LocalShellToolTests: explicit stderr-capture assertion (echo to stderr → result.Stderr contains the message). Stderr capture was not directly asserted anywhere in the suite. * DockerShellToolTests: RunAsync_RejectedCommand throws ShellCommandRejectedException. The Docker-side policy check is a pure-logic path that runs before any docker invocation, so this test covers the rejection branch without needing a Docker daemon. Total: 66 -> 85 tests, all passing on net10.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(dotnet/shell): add ShellEnvironmentProvider for OS-aware shell instructions Pairs LocalShellTool/DockerShellTool with an AIContextProvider that probes the live shell once per session (OS, family, version, CWD, configurable CLI versions) and injects authoritative instructions so the agent uses platform-native idioms (PowerShell vs POSIX). Fixes the class of bugs where the model emits 'VAR=value' / '/tmp' / '$VAR' on a Windows PowerShell session. - ShellEnvironmentProvider/Snapshot/Options public surface in the existing Microsoft.Agents.AI.Tools.Shell package (one new project reference to Microsoft.Agents.AI.Abstractions). - Probes go through the same IShellExecutor that runs agent commands, so they respect the configured policy and (for DockerShellTool) the container boundary. - 8 unit tests covering snapshot capture, default formatter idioms, missing-tool handling, custom formatter override, and refresh. - Agent_Step21_ShellWithEnvironment sample replays the DEMO_TOKEN cross-call scenario using a persistent local shell. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(dotnet/shell): address PR review feedback round 3 - ShellEnvironmentProvider.cs split into one-type-per-file (ShellFamily, ShellEnvironmentSnapshot, ShellEnvironmentProviderOptions, plus the provider class) to match FoundryMemoryProvider/AgentSkillsProvider layout. - csproj: drop IsPackable=false (package will publish on merge), add IsReleased=true and disable package validation baseline (first release), use TargetFrameworksCore, add InjectSharedDiagnosticIds and InjectExperimentalAttributeOnLegacy to align with shipping packages. - Sample: refactor to demonstrate stateless mode first (independent read-only commands), then persistent mode (state carried across calls, e.g. DEMO_TOKEN). Strip narrative/historical comments. - Move docker-shell-tool.md out of the package — that doc lives in the docs repo (semantic-kernel-pr/agent-framework, branch feat/dotnet-shell-tool). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5604 round 4 review feedback - Sample (Agent_Step21_ShellWithEnvironment): add prominent WARNING block noting LocalShellTool runs real commands on the host. Restructure sample to demonstrate stateless mode first (cd does not carry across calls) then persistent mode (cd and env vars persist), motivating when to pick each. - DockerShellTool class XML doc: reframe as a best-effort baseline rather than a security guarantee; list mitigations users should still apply. - DockerShellTool ShellKind.Sh comment: rephrase as forward-looking design rationale (avoid duplicate --noprofile/--norc if Bash is reintroduced) instead of bug-history narrative. - DockerShellTool.IsHardenedConfiguration / AsAIFunction XML docs: clarify these are configuration-shape checks and convenience defaults, not security guarantees. - Drop IDisposable from LocalShellTool and DockerShellTool. The previous sync Dispose() blocked on DisposeAsync().GetAwaiter().GetResult() with a VSTHRD002 suppression, which is fragile under sync contexts. Both tools now expose IAsyncDisposable only; tests updated to await using. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add Async suffix to async test methods to satisfy IDE1006 Fixes check-format CI failure on PR #5604. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix CPU busy-spin in WaitForSentinelAsync When new bytes arrived in the stdout read loop, the producer called TrySetResult on _stdoutSignal but did not replace it with a fresh TCS. A consumer looping inside WaitForSentinelAsync would then re-read the same already-completed TCS, causing WaitAsync(100ms) to return synchronously every iteration — a tight busy-spin that pinned a core until the sentinel arrived or the timeout fired. Swap the signal before completing the old one so the next consumer iteration observes a fresh (uncompleted) TCS, matching the pattern already used in ReadExitCodeAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove unused onCommand audit hook from shell tools The Action<string> onCommand callback was a redundant audit-logging seam: no production callers, no Python parity, and the framework already provides function-invocation middleware for cross-cutting concerns at the AIFunction layer. Removing the parameter from LocalShellTool and DockerShellTool keeps the public surface lean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Align Shell csproj with Foundry.Hosting preview-package conventions - Add RootNamespace - Move Title/Description into the primary PropertyGroup with TargetFrameworks/VersionSuffix to match the Foundry.Hosting layout - Drop IsReleased (preview packages do not set it) - Drop UTF-8 BOM Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Document why ShellEnvironmentProvider uses Instructions, not Messages Expand the class XML doc to record the design rationale: the shell environment is stable runtime metadata, not per-turn retrieval, so it belongs in AIContext.Instructions (matching AgentSkillsProvider). Messages is reserved for retrieval payloads (TextSearchProvider, ChatHistoryMemoryProvider). System-role placement also has higher steering weight and benefits from prompt caching in major providers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify which probe failures ShellEnvironmentProvider swallows Name the four exception types explicitly (timeout, policy rejection, spawn failure, cancellation) and note that all other exceptions propagate normally. Avoids the misleading impression that the provider is a blanket try/catch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strip cross-language and bug-history narrative from shell tool comments Remove "hard-won" framing and explicit "Mirrors the Python ..." cross references from class XML docs and inline comments in ShellSession, DockerShellTool, and ShellResolver. Comments now describe current behavior without commentary on prior implementations or development history. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5604 round 5 review feedback - ShellResolver: classify only `bash` as ShellKind.Bash; sh/zsh/dash/ash/ksh/busybox now route through ShellKind.Sh so bash-only --noprofile/--norc flags are not emitted to shells that reject them. Update enum doc and tests. - ShellEnvironmentProvider.ProbeToolVersionAsync: validate the tool name against ^[A-Za-z0-9._-]+$ before interpolating into a shell command (prevents injection if ProbeTools is sourced from untrusted config). Fall back to stderr when stdout is empty so CLIs like java/older gcc still report a version. Drop misleading 'quoted' comment. - ShellSession.TruncateHeadTail: truncate by UTF-8 byte count on rune boundaries, honouring the documented maxOutputBytes contract for non-ASCII output. - ShellEnvironmentProviderTests: drop reflection on private _options; assert against the options instance the test already owns. Rename misnamed RefreshAsync test to reflect re-probing semantics. Add coverage for invalid tool names and stderr-only version output. - ShellSessionTests: add multi-byte UTF-8 truncation tests (byte-budget honoured, no rune split, no U+FFFD). - Move DockerShellToolIntegrationTests.cs from the unit test project into a new Microsoft.Agents.AI.Tools.Shell.IntegrationTests project so 'dotnet test' on the unit suite no longer requires a Docker daemon. Wire the new project into agent-framework-dotnet.slnx. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5604 round 6 review feedback - ShellSession.MaybeReanchor: switch from double-quoted to single-quoted literal-quoting per shell. Double quotes still expand $VAR, ``, and backticks in both PowerShell and POSIX, so a working directory containing shell metacharacters could trigger command substitution. Add QuotePowerShell (escape ' as '') and QuotePosix (close-and-reopen around ') helpers and route MaybeReanchor through them. Add tests covering ``, $VAR, backticks, and embedded single quotes. - ShellEnvironmentProvider.RunProbeAsync: narrow the OperationCanceledException filter to `when (!cancellationToken.IsCancellationRequested)` so caller-driven cancellation propagates instead of being silently converted to a null snapshot. Update the class XML doc to call out the distinction. Add tests for both paths (caller cancellation throws, probe-timeout returns null fields). - DockerShellTool.RunStatelessAsync / RunDockerCommandAsync: replace unbounded StringBuilder accumulators with a shared HeadTailBuffer (extracted from LocalShellTool into its own internal type). Caps memory at roughly maxOutputBytes regardless of how much output a command emits; drops the now-redundant trailing TruncateHeadTail call. RunDockerCommandAsync caps helper-command output at 1 MiB (defends against chatty docker pull progress streams). Add HeadTailBufferTests covering bounded behaviour over 10 MiB of streamed input. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5604 round 7 review feedback - HeadTailBuffer: switch to UTF-8 byte-aware truncation. The class previously capped on UTF-16 char count while callers pass _maxOutputBytes, so multi-byte output could exceed the budget and head/tail boundaries could split surrogate pairs into orphaned halves. Now tracks UTF-8 byte counts and treats each rune as an indivisible unit (encode -> bytes -> head/tail), guaranteeing the final string round-trips through UTF-8 and never contains an unpaired surrogate. The truncation marker now reads `bytes` instead of `chars` to match. - ShellEnvironmentProvider: clear cached _snapshotTask on failure. Previously a faulted/cancelled first probe permanently poisoned the provider — every later ProvideAIContextAsync await replayed the same exception. Now the failed task is cleared via a CompareExchange so the next caller starts a fresh probe. Tests: added rune-boundary coverage for HeadTailBuffer, plus two regression tests for poison-recovery (executor-throw and caller-cancellation paths). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5604 round 8 review feedback - HeadTailBuffer odd-cap data loss: previously _halfCap = cap / 2 was used as both the head fill bound and the tail eviction threshold, so an odd cap (e.g. cap=5 -> halfCap=2) would silently drop a byte while ToFinalString still reported truncated == false. Split into _headCap = cap / 2 and _tailCap = cap - _headCap so head + tail budgets always sum to exactly cap; any input whose UTF-8 size is <= cap now round-trips losslessly. - ShellSession.TakePrefixByBytes unpaired-high-surrogate: the prefix walker advanced 2 chars whenever it saw a high surrogate, without verifying that the next char was actually a low surrogate. Mirrored the pair check from TakeSuffixByBytes so unpaired surrogates are treated as a single (invalid) BMP char and the encoder substitutes U+FFFD as it would anywhere else. - Centralize clean-environment preserved-vars list. The {PATH, HOME, USER, USERNAME, USERPROFILE, SystemRoot, TEMP, TMP} allowlist was duplicated in LocalShellTool (stateless launch) and ShellSession (persistent startup), so adding a new variable required touching both. Extracted into CleanEnvironmentHelper.PreservedVariables / ApplyPreserved; both call sites collapse to a single line. Tests: HeadTailBuffer round-trip-at-odd-cap regression, ShellSession unpaired- surrogate test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5604 round 9 review feedback - ShellSession.TruncateHeadTail odd-cap budget: same fix applied to HeadTailBuffer last round but missed here. Use headCap = cap/2 + tailCap = cap - headCap so the head/tail budgets sum to exactly cap. - Replace TakePrefixByBytes / TakeSuffixByBytes Encoder.Convert loops with rune iteration. The old code ignored Encoder.charsUsed and trusted the caller's hand-rolled surrogate-pair detection, which made the byte count fragile around unpaired surrogates. EnumerateRunes + Utf8SequenceLength is stateless and self-evidently correct. - ShellEnvironmentProvider.ProbeAsync now skips case-insensitive duplicates in the user-supplied ProbeTools list. Previously {\"git\",\"GIT\"} would probe twice and rely on insertion order to determine the kept value. - DockerShellToolTests.AsAIFunction_RelaxedConfig_DefaultsToApprovalGated: removed unused trailing ool _ parameter and matching InlineData column. Tests: added duplicate-ProbeTools regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5604 round 10 review feedback * ShellSession.ReadLoopAsync: replace per-byte buf.Add(chunk[i]) loop with a single buf.AddRange(new ArraySegment<byte>(chunk, 0, n)) bulk copy on the read hot path. * ShellPolicy: compile allow-list patterns with RegexOptions.IgnoreCase, matching the deny-list and avoiding case-mismatch surprises. * LocalShellToolTests.RunAsync_NonZeroExit: drop the redundant ternary that selected between two identical 'exit 7' literals. * DockerShellToolIntegrationTests.NetworkNone: fix the comment to reference 'getent' (matching the actual command) instead of the stale 'wget' phrasing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(dotnet): address PR #5604 round-3 review feedback - Rename LocalShellTool/DockerShellTool -> LocalShellExecutor/DockerShellExecutor - Rename IShellExecutor.StartAsync/CloseAsync -> InitializeAsync/ShutdownAsync - Rename ShellDecision -> ShellPolicyOutcome - Rename CleanEnvironmentHelper.ApplyPreserved -> EnvironmentSanitizer.RemoveNonPreserved - Convert ShellRequest/ShellPolicyOutcome from record struct to plain readonly struct (with IEquatable<T>) - Split ShellMode, ShellTimeoutException, ShellExecutionException into their own files - Add DockerNetworkMode static class with None/Bridge/Host constants - Convert DockerShellExecutor memory parameter from string to long? memoryBytes - Use Throw.IfNull(image) in DockerShellExecutor ctor - Make ShellResolver.EnvVarName public const - Inline-comment each DefaultDenyList regex; document allow-precedence-over-deny on ShellPolicy.Evaluate Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(dotnet): address PR #5604 round-3 follow-up nits - DockerShellExecutor / LocalShellExecutor: drop redundant IAsyncDisposable from class declarations (IShellExecutor : IAsyncDisposable already covers it) - DockerShellExecutor: scope DefaultImage / DefaultContainerUser / DefaultNetwork / DefaultMemoryBytes / DefaultPidsLimit / DefaultContainerWorkdir to internal (only used as parameter defaults; tests have InternalsVisibleTo) - DockerShellExecutor.RunAsync: blank line after the null-guard block (style consistency) - csproj: move <Title>/<Description> below the nuget-package.props import so they are not overwritten by the shared defaults; refresh wording to match new executor names Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Refactor shell tool: abstract ShellExecutor, options classes, ContainerUser record Round-3 review responses for PR #5604: * Replace IShellExecutor interface with abstract ShellExecutor base class so the surface can be extended without breaking implementers (review feedback from @westey-m). * Drop ShutdownAsync from the executor surface; DisposeAsync is the canonical teardown (review feedback from @SergeyMenshykh). * Replace the long parameter lists on Local/DockerShellExecutor constructors with LocalShellExecutorOptions and DockerShellExecutorOptions classes so adding new knobs is no longer a breaking change (review feedback from @SergeyMenshykh). * Introduce ContainerUser(Uid, Gid) record in place of a 'uid:gid' string for the Docker user, with Default and Root statics (review feedback from @lokitoth). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove IsHardenedConfiguration; AsAIFunction defaults to approval-gated Addresses PR #5604 review thread AZpMj. The IsHardenedConfiguration property was a configuration-shape check, not a security guarantee, and using it to auto-disable approval gating gave false confidence. - Delete IsHardenedConfiguration property. - AsAIFunction(requireApproval: null) now always wraps in ApprovalRequiredAIFunction; callers must explicitly pass false to opt out. - Update class- and method-level XML docs to drop hardened-attestation language and call out approval gating as the primary safety control. - Drop two hardening-assertion tests and the relaxed-config theory; add one test asserting null requireApproval is approval-gated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Replace ShellExecutionException/ShellTimeoutException with standard exceptions Addresses PR #5604 review threads AaqVP and Aasod. The custom exception types added no behavior beyond the base type — only a different name — so callers gain nothing from them. - Delete ShellExecutionException.cs and ShellTimeoutException.cs. - Process spawn failures (LocalShellExecutor, DockerShellExecutor) and broken-pipe to a long-lived shell (ShellSession) now throw IOException, which is the natural .NET shape for these failures. - ShellTimeoutException was declared but never thrown; the only in-process timeout path uses the OperationCanceledException raised by the linked CancellationTokenSource. The catch-and-swallow in ShellEnvironmentProvider now matches IOException + TimeoutException. - Update XML doc comments accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove ShellPolicy.DefaultDenyList; default policy is empty Addresses PR #5604 review thread AY7Ba. A regex deny-list is bypassed in seconds by hex escapes ($(echo -e "\x72\x6D")), command substitution ($(base64 -d <<<...)), and envvar splicing ($(A=r B=m; echo $A$B)). No major agent framework uses regex matching as a primary control; AutoGen explicitly removed theirs in v2. The real defenses are approval gating (default) and the Docker sandbox tier. - Delete DefaultDenyList property from ShellPolicy. - ShellPolicy(denyList: null) now means an empty deny-list. - Rewrite ShellPolicy class XML docs to frame as a UX pre-filter for operator-supplied patterns, not as a security control. - Update LocalShellExecutorOptions/DockerShellExecutorOptions Policy docs to match. - Tests that exercise the deny-list mechanism now supply patterns explicitly, mirroring real operator usage. - Add Policy_DefaultConstruction_AllowsAnyNonEmptyCommand test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Document single-session ownership for persistent shell mode Several PR #5604 review threads (notably AaQh2) raised that the persistent shell experience has no concurrency story. The framework's actual design is "one executor per conversation" — there is no per-caller isolation — but that contract was only stated briefly on ShellExecutor and not at all on the types and properties developers reach for first. Strengthen the docs in the places a user is most likely to land: - ShellMode.Persistent: explicit single-session-ownership paragraph (state visible across calls, single pipe, no isolation, one per session). - ShellExecutor: rewrite the Concurrency paragraph to enumerate what leaks (cwd, env, history, background jobs) and call out DI scoping. - LocalShellExecutor: new Single-session-ownership paragraph mirroring the executor-level contract and pointing at Stateless mode as the escape hatch. - DockerShellExecutor: same, framed around the container + bash REPL the persistent-mode executor owns end-to-end. - ShellSession: add a Single-owner paragraph on the type docs and a comment on _runLock clarifying that it serializes the owner's calls, not multiple tenants. - LocalShellExecutorOptions.Mode / DockerShellExecutorOptions.Mode: per-property note pointing at the executor remarks. Docs-only; src builds clean with zero warnings, zero errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
9199c84d42 |
.NET: Remove Foundry Toolbox server-side tools support (#5753)
* .NET: Remove Foundry Toolbox server-side tools support Mirrors the Python cleanup in microsoft/agent-framework#5671. Passing toolbox tools as server-side Responses tools is not the experience we want to support; the hosted-agent MCP toolbox path (HostedMcpToolboxAITool + FoundryToolboxService) remains the supported way to consume Foundry Toolboxes. Removed: - FoundryToolbox static class (GetToolboxVersionAsync / GetToolsAsync / ToAITools / SanitizeAndConvert) - AIProjectClient.GetToolboxToolsAsync extension - Agent_Step25_ToolboxServerSideTools sample (+ slnx entry) - FoundryToolboxTests, TestDataUtil, HttpHandlerAssert, and the toolbox JSON fixtures only those tests referenced - ToolboxHostedAgentTests and ToolboxHostedAgentFixture; the "toolbox" switch arm + CreateToolboxAgent helper in TestContainer; matching README scenario row and bootstrap script entry Kept (MCP path, unchanged): - HostedMcpToolboxAITool, FoundryAITool.CreateHostedMcpToolbox, FoundryAIToolExtensions.CreateHostedMcpToolbox(ToolboxRecord/Version) - FoundryToolboxService, AddFoundryToolboxes, marker injection in AgentFrameworkResponseHandler, InputConverter.ReadMcpToolboxMarkers - Hosted-Toolbox sample, McpToolbox* tests, FoundryToolboxServiceTests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * .NET: Add Foundry Toolbox MCP sample (Agent_Step25_FoundryToolboxMcp) Adds a non-hosted-agent equivalent of the Python foundry_chat_client_with_toolbox.py sample. The agent connects to a Foundry Toolbox's MCP endpoint via Streamable HTTP, injects a fresh Azure AI bearer token on every request, and discovers the toolbox's tools at runtime via McpClient.ListToolsAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * .NET: Tighten Agent_Step25_FoundryToolboxMcp README/Program comments Drop 'non-hosted agent' framing from README (this sample isn't related to hosted agents) and remove narrative comparison to server-side tools from the Program.cs header comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Drop python sample reference from Agent_Step25 README Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Drop incorrect .NET 10 prereq from Agent_Step25 README Toolboxes don't require .NET 10 (Microsoft.Agents.AI.Foundry targets net8.0+); the parent AgentsWithFoundry README already lists the sample SDK prereq. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix Toolsets api-version in Agent_Step25 example endpoint Use 2025-05-01-preview to match FoundryToolboxOptions.ApiVersion. The placeholder 'v1' is not accepted by the Toolsets endpoint. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
0bbedc4fa2 |
.NET: Fix/per service input persistence on stream error (#5744)
* .NET: Persist input messages on streaming errors in PerServiceCallChatHistoryPersistingChatClient When the underlying chat service emits an in-stream error (for example a `response.error` SSE event from the OpenAI Responses API on rate limit), the OpenAI client surfaces it as an `ErrorContent` update and ends the stream without throwing. Previously, `PerServiceCallChatHistoryPersistingChatClient` only persisted history when the streaming loop completed successfully and `NotifyProvidersOfNewMessagesAsync` was called at the end. On the in-stream-error path, the input messages handed to that iteration - typically `FunctionResultContent` produced by `FunctionInvokingChatClient` in the previous iteration - were never persisted. The next run would replay session history with a dangling `FunctionCallContent` and the service would reject the request with `No tool output found for function call <id>`. This change: - Adds a `PersistInputOnErrorAsync` helper that persists the input messages (with no response messages) so function-call/function-result pairings are not split across failures. - Calls the helper from every error path: pre-loop enumerator creation, the first `MoveNextAsync`, the in-loop `MoveNextAsync`, and a new `finally` that handles abnormal iterator disposal. - After the streaming loop, scans the assembled response for any `ErrorContent` and, if present, persists the input, notifies providers of failure, and throws `InvalidOperationException` so the error is surfaced to the caller instead of silently corrupting history. - Hardens `InMemoryChatHistoryProvider.StoreChatHistoryAsync` to treat a null `RequestMessages` as empty, since the new error path can invoke it with no response messages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix dropped FunctionResultContent on streaming pipeline early-disposal When a consumer of ChatClientAgent.RunStreamingAsync stops iterating early (e.g. ToolApprovalAgent yields the approval request and then `yield break`), the framework cascades DisposeAsync down the stream. C# async iterators do not auto-dispose IAsyncDisposable locals, so the inner enumerator returned by IChatClient.GetStreamingResponseAsync(...).GetAsyncEnumerator(ct) was left suspended. That suspended FunctionInvokingChatClient downstream, which suspended PerServiceCallChatHistoryPersistingChatClient at its `yield return`, so its finally block never ran and the in-flight FunctionResultContent for the just-completed tool call was not persisted to chat history. The next turn then loaded a session that contained a FunctionCallContent with no matching FunctionResultContent and the model returned HTTP 400 `No tool output found for function call`. Fixes: * ChatClientAgent.RunStreamingAsync: wrap the iteration in try/finally that disposes the inner enumerator. Disposal now cascades through the pipeline and PerService's finally runs on early exit. * PerServiceCallChatHistoryPersistingChatClient: in the streaming path, snapshot input messages with `messages.ToList()` (the caller, FICC, reuses a single mutable buffer across iterations and may mutate it before our finally / error path persists), wrap GetAsyncEnumerator, the first MoveNextAsync, and in-loop MoveNextAsync in try/catch each calling PersistInputOnErrorAsync + NotifyProvidersOfFailureAsync, and add a finally that calls PersistInputOnErrorAsync when the loop did not exit normally so per-iteration FRCs are persisted on early disposal as well as on errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * .NET: Add tests for PerService streaming error/dispose persistence paths Adds five regression tests covering the new error-path persistence in PerServiceCallChatHistoryPersistingChatClient.GetStreamingResponseInnerAsync: - Persists input messages when GetStreamingResponseAsync throws synchronously. - Persists input messages when the first MoveNextAsync throws. - Persists input messages when a mid-stream MoveNextAsync throws. - Persists input messages when the consumer abandons enumeration early (the ToolApprovalAgent yield-break / disposal-cascade case). - Throws and persists input when the stream emits an in-band ErrorContent. All 66 tests in the class pass on net10.0 and net472. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * .NET: Address PR feedback on PerService streaming error persistence Two follow-ups from PR #5744 review: 1. Prevent duplicate persistence on the in-loop MoveNextAsync catch path. The inner catch persists input messages, then rethrows, which propagates through the surrounding try/finally where loopExitedNormally is still false, causing the finally to persist again. Introduced an inputPersisted flag that the inner catch sets after persisting; the finally now skips when inputPersisted is true. 2. Use the caller's CancellationToken in the abnormal-exit finally instead of CancellationToken.None, so cleanup remains responsive to cancellation. Fall back to CancellationToken.None only when the caller's token is already canceled (otherwise the persist call would observe the cancellation, throw, and mask the original early-exit reason). Tightened all five new streaming-error tests from Times.AtLeastOnce to Times.Once on the input-persistence matcher to regression-guard against duplicate persistence. All 66 tests in the class still pass (net10.0 + net472). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * .NET: Scope PerService streaming changes to cooperative early-exit only Per discussion on PR #5744, scope this PR back to fix only the original ToolApprovalAgent dropped-FunctionResultContent bug and address the enumerator-disposal review comment. Specifically: - Remove input-message persistence from the GetAsyncEnumerator and MoveNextAsync error paths. Routing failed service calls through the success notification channel was breaking the provider contract; we will instead rely on inner-agent retries for transient errors. Failure paths still call NotifyProvidersOfFailureAsync as before. - Remove the in-stream ErrorContent detection block (same rationale). - Keep the try/finally that calls the (now narrower) early-exit input notification on cooperative disposal (e.g. ToolApprovalAgent yield break). A new serviceErrorOccurred flag ensures we do NOT renotify on exception paths. - Always DisposeAsync the underlying enumerator on every exit path, addressing the copilot-reviewer comment about leaked HTTP/streams. - Rename PersistInputOnErrorAsync -> NotifyProvidersOfEarlyExitInputAsync to better reflect what it does and when it runs (rogerbarreto nit). - Apply rogerbarreto nit on InMemoryChatHistoryProvider null-coalescing. - Drop the four tests that covered the removed error-path behavior; keep RunStreamingAsync_PersistsInputMessages_WhenConsumerAbandons EnumerationAsync (regression guard for the cooperative-pause path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
76772ffc19 |
.NET: Fix function_call_output.output to be a JSON string on the wire (#5705)
* Fix function_call_output.output to be a JSON string on the wire
OutputConverter was passing the JSON serialization of complex tool results (e.g. List<TodoItem>) directly into OutputItemFunctionToolCallOutput via BinaryData.FromString. The Responses SDK treats that BinaryData as the *raw JSON value* for the field, so non-string results landed on the wire as an unquoted JSON array (e.g. `"output":[{...}]`) instead of a JSON string.
The OpenAI Responses spec requires `function_call_output.output` to be a JSON string. The strict-parsing OpenAI .NET client (FunctionCallOutputResponseItem) consequently failed when threading a follow-up turn that replayed such an item, with: `The JSON value could not be converted... requires an element of type 'String', but the target element has type 'Array'`.
Always wrap the payload as a JSON string literal:
- string s -> JSON-encode s (quoted, with escapes)
- object o -> JSON-serialize o, then JSON-encode the resulting text
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR feedback: JsonElement special-case, symmetric inbound unwrap, tests
OutputConverter: extract EncodeFunctionResultAsJsonStringPayload helper
that special-cases JsonElement / JsonDocument so a string-kind element
does not get double-encoded into "\"value\"". Other JsonElement kinds
(object/array/number/bool) round-trip via GetRawText() and are then
JSON-string-wrapped, matching the spec.
InputConverter: symmetric DecodeFunctionResultPayload added to
ConvertFunctionCallOutput and ConvertFunctionToolCallOutput so
previously-stored function_call_output items replayed via
previous_response_id unwrap back to the original tool result text
instead of leaking the JSON-encoded form into FunctionResultContent.Result.
Legacy non-conforming raw-JSON-value payloads pass through unchanged.
Tests:
- Replace ConvertUpdatesToEventsAsync_FunctionResultStringPayload_EmittedAsRawTextAsync
with EmittedAsJsonStringAsync asserting the new wire contract ("sunny" -> "\"sunny\"").
- Add coverage for object payloads, JsonElement string kind (no double-encoding),
and JsonElement array kind (JSON-stringified).
- Add InputConverter round-trip tests for spec-compliant JSON-string payloads
and legacy raw-JSON-array payloads.
All 663 tests pass on net8/net9/net10. Verified end-to-end against the local
hosted-harness sample: T1-T4 (incl. TodoList tool replay across turns) all
succeed with no SDK parse errors.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: alliscode <bentho@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
||
|
|
cdd80c61ac |
.NET: Issue 5662 (#5668)
* Fix dangling function_call on approval response in Foundry hosting (#5662) Make the wire<->AF approval translation in Microsoft.Agents.AI.Foundry.Hosting lossless so the resume turn pairs function_call/function_call_output correctly. Root cause: InputConverter.ConvertMcpApprovalResponse rebuilt FunctionCallContent with CallId set to the FICC-composed AF request id (ficc_<callId>) and Name hardcoded to 'mcp_approval'. This (a) broke Azure Conversations pairing because the persisted function_call had CallId <callId> without prefix, and (b) made FICC unable to invoke the original tool by name on resume. Fix: ToolApprovalIdMap now records the original FunctionCallContent (CallId, Name, Arguments) keyed by wire id at outbound time. InputConverter reconstructs the original FCC on inbound, falling back to the legacy placeholder when no mapping exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Suppress orphan function_call items at the wire (#5662) Foundry-Hosting's OutputConverter was emitting FunctionCallContent as wire `function_call` items while dropping the paired FunctionResultContent. The result: every auto-invoked tool call left an orphan `function_call` in the response store. The next turn (chained via previous_response_id or via a workflow that yields after one turn under externalLoop) reloaded that history and submitted it to Azure Conversations, which rejected it with HTTP 400 `No tool output found for function call ...`. Function call/result pairs are entirely internal to the agent's tool-calling loop and have no place on the wire. Approval-required calls already surface separately via ToolApprovalRequestContent → mcp_approval_request, so dropping FCC is safe. FCC's message-close behavior is preserved so pre-tool text doesn't accidentally concatenate with post-tool text under the same MessageId. Existing OutputConverter tests asserting FCC wire emission are updated to assert suppression. Verified end-to-end against the declarative-workflow-menu external_loop bench: three-turn previous_response_id chain (menu → carbonara price → EXIT) now completes, where it previously failed at turn 2 with HTTP 400. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fail fast when no approval mapping is recorded (#5662) The previous best-effort placeholder fallback in InputConverter.ConvertMcpApprovalResponse couldn't actually round-trip — it just delayed and obscured the failure as an HTTP 400 deep inside the agent loop. Throw InvalidOperationException with the wire id and a clear cause hint instead so the failure is local and actionable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Trim narrative comments and exception message (#5662) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Defer FunctionCallContent emission until matched FunctionResultContent (#5662) Replace blanket FCC suppression with deferred emission. FunctionCallContent is buffered (name + serialized arguments) keyed by CallId; the function_call and function_call_output wire items are only flushed once the matching FunctionResultContent arrives. - Auto-invoked FCC/FRC pairs surface as paired wire items so Azure's stored conversation has matched call+output and previous_response_id resume works (closes the orphan-function_call symptom from #5662). - Orphan FCCs (e.g. workflow paused at a checkpoint mid-tool-loop) are dropped so they never poison the response store. - Approval flows are unchanged: TARC still emits mcp_approval_request and the post-approval FRC has no buffered FCC to pair with so it is dropped; the approval round-trip handles its own pairing via mcp_approval_*. - Leaves the door open for future client-side function calling: that pattern would surface an FCC without an FRC, would need to opt out of buffering, but the wire shape is already correct. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Emit FunctionCallContent and FunctionResultContent directly (option B) Replace the deferred-emission/buffer-and-drop strategy with direct emission of both function_call and function_call_output wire items. Rationale: a lone FunctionCallContent in OutputConverter's input can mean two semantically different things, and only the caller knows which: - Auto-invoke (FICC response surface): always paired with a matching FRC; both halves should appear on the wire as historical record. - HITL / port-pause request (typed RequestPort<FunctionCallContent,...> or workflow synthesizing a request): a lone FCC IS the wire signal that the caller must resume by supplying a function_call_output. Buffering+dropping orphans silently swallows the second case. Emitting both directly is the only correct shape for OpenAI Responses semantics. The InputConverter already accepts function_call_output and mcp_approval_response on resume, so the round-trip works for both kinds. The approval-flow round-trip fixes (ToolApprovalIdMap rich ApprovalEntry, fail-fast on missing mapping in ConvertMcpApprovalResponse) remain intact. Tests: updated 7 OutputConverter tests + 1 OutputConverterWorkflow test that asserted the old buffer/drop semantics; all 227 tests pass. Refs #5662 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5668 review feedback on TryLoadMap Stop swallowing JsonException in ToolApprovalIdMap.TryLoadMap. The catch block recovered to an empty map and a stale comment claimed the caller would gracefully degrade via a 'wire-id fallback path' — but that path no longer exists: InputConverter.ConvertMcpApprovalResponse fails fast when no entry is found. Letting the JsonException propagate produces an error message that points at the actual cause (a state-bag format incompatibility), instead of converting it into a confusing 'no approval mapping recorded' InvalidOperationException one stack frame later. Refs #5662, PR #5668 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5668 review feedback round 2 - OutputConverter FRC: emit string results as raw text (no JSON-quoting), matching the wire contract for function_call_output.output. - OutputConverter FCC: validate non-empty CallId before closing the in-flight text message, so a skipped FCC no longer breaks output-item boundaries. - ToolApprovalIdMap.Record: take pre-serialized arguments JSON (string) and primitive callId/name. Drops [RequiresUnreferencedCode]/[RequiresDynamicCode] so trim/AOT warnings stop propagating to call sites. - ToolApprovalIdMap.Record: no-op when callId or name is empty. - Tests: dedup duplicate ConvertItemsToMessages_McpApprovalResponse no-mapping test; add coverage for empty-CallId boundary, raw-string FRC payload, and Record empty-key no-op. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
4b5a8478de |
.NET: Hosting updates to declarative workflows (#5589)
* Make DeclarativeWorkflowExecutor ChatProtocol-compatible for AsAIAgent hosting Extends the existing DeclarativeWorkflowExecutor<TInput> root executor with additional ChatProtocol-compatible input routes (string, ChatMessage, IEnumerable<ChatMessage>, ChatMessage[], TurnToken) so that workflows built via DeclarativeWorkflowBuilder.Build<TInput>(...) work both for direct invocation and when hosted via Workflow.AsAIAgent(...). - Each input message advances the declarative graph immediately; the TurnToken that the host sends after the message batch is treated as a no-op since the message has already been processed. - Conversation id resolution now prefers persisted workflow system state, then DeclarativeWorkflowOptions.ConversationId, then a newly created conversation. This makes multi-turn invocations reuse the prior conversation rather than creating a fresh one each turn. - The separate DeclarativeChatProtocolStartExecutor and DeclarativeWorkflowBuilder.BuildChatProtocol overloads introduced earlier are removed; callers continue to use Build<TInput>(...). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: use DeclarativeWorkflowContext when reading workflow conversation id GetWorkflowConversation() requires a DeclarativeWorkflowContext (it calls ReadState which dynamic-casts via the DeclarativeContext helper). The chat-protocol auxiliary handlers receive a BoundWorkflowContext, so calling the extension on the raw IWorkflowContext throws `Invalid workflow context: BoundWorkflowContext`. Use the wrapped declarativeContext that we already constructed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: surface ExecutorFailedEvent as ErrorContent in AsAIAgent response WorkflowSession.InvokeStageAsync only converted WorkflowErrorEvent into an ErrorContent payload. ExecutorFailedEvent fell through to the default branch which emits an empty AgentResponseUpdate carrying the event in RawRepresentation. OutputConverter then mapped that to a workflow_action item with status=failed and dropped the exception entirely, so callers got status=completed and error=null even when an executor threw. - WorkflowSession.cs: add ExecutorFailedEvent case mirroring WorkflowErrorEvent. Honors _includeExceptionDetails. - OutputConverter.cs: when an update carries both a WorkflowEvent in RawRepresentation and non-empty Contents, fall through to content processing so the unwrapped error (or any future content payload from a workflow event) is actually emitted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * improve: walk inner exceptions when surfacing ExecutorFailedEvent DeclarativeActionExecutor wraps inner exceptions in DeclarativeActionException with a generic `Unhandled workflow failure` message, hiding the real cause. Walk InnerException so the response shows the full chain (e.g. the underlying HTTP 400 / auth error). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Surface declarative SendActivity output as chat content SendActivityExecutor now emits AgentResponseEvent in addition to MessageActivityEvent so chat protocols (e.g. AsAIAgent) receive the formatted activity text. The existing MessageActivityEvent is preserved for DevUI/observability. Also extend WorkflowSession.WorkflowOutputEvent handling to accept AgentResponse payloads, mapping them to their constituent ChatMessages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Persist hosted-agent sessions to disk; fix System.LastMessageText Adds FileSystemAgentSessionStore that writes the serialized AgentSession JSON (which already embeds the workflow's in-memory checkpoint manager) to a per- conversation file under /.checkpoints when running in a Foundry hosted env or {cwd}/.checkpoints locally. Mirrors the python foundry_hosting._responses FileCheckpointStorage pattern so multi-turn workflow state survives process restarts without requiring callers to wire up storage themselves. AddFoundryResponses now defaults to FileSystemAgentSessionStore.CreateDefault() instead of InMemoryAgentSessionStore; callers can still override via DI. Also fixes {System.LastMessageText} resolving empty: DeclarativeWorkflowExecutor .AdvanceAsync was passing the message rehydrated from CreateMessageAsync to SetLastMessageAsync, but ResponseItem -> ChatMessage round-trip drops the .Text extension content. Use the original input ChatMessage (which still has the user-supplied text) and copy the server-assigned MessageId across when present. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Close multi-modal input parity gaps with python foundry_hosting InputConverter now mirrors the python _responses.py content handling: - ComputerScreenshotContent maps to UriContent/HostedFileContent (was dropped). - Plain TextContent and SummaryTextContent map to MEAI TextContent. - MessageContentReasoningTextContent maps to MEAI TextReasoningContent. - input_file with text/* file_data data URIs is decoded inline into TextContent with a [File: name] prefix, matching python _convert_file_data so {System.LastMessageText} surfaces the file body. Non-text data URIs and hosted/url file references preserve filename as AdditionalProperties. Image/file extraction logic is extracted into shared AppendImageContent and AppendFileContent helpers used by both the fresh-input and history-replay switches. Existing 37 InputConverter tests still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Foundry hosting: round-trip tool-approval (HITL) content as mcp_approval_request/response Closes the gap where Microsoft.Agents.AI.Foundry.Hosting silently dropped MEAI ToolApprovalRequestContent/ToolApprovalResponseContent in both directions. We now serialize them onto the wire as the standard Responses API mcp_approval_request/mcp_approval_response items with server_label='agent_framework', and parse the symmetric inbound shapes back into MEAI content. Wire format: - The Responses API only standardizes mcp_approval_* as the approval primitive. We declare AF as a virtual MCP server via the server_label field, which is honest for AF's server-side tool-call holding pattern. - The SDK enforces a strict {prefix}_{50hex} wire-id format, so we hash the AF RequestId and persist a wireId<->afRequestId mapping in AgentSession.StateBag so a later mcp_approval_response can be matched back to the originating workflow request. Coexists with the existing ConsentAwareMcpClientAIFunction flow (AgentFrameworkResponseHandler.cs) which emits mcp_approval_request from a side-channel, not via OutputConverter's content switch. Known follow-up: python (foundry_hosting/_responses.py) has the same output-side gap (ToolApprovalRequestContent emission). Out of scope here. Tests: +9 unit tests covering both fresh-input and history-replay shapes, StateBag mapping resolution, and the non-FunctionCallContent skip path. Existing 108 converter tests still pass; full suite 370/370. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review feedback for hosted-declarative-dotnet FileSystemAgentSessionStore reliability/scoping: - Bound Sanitize() stackalloc at 256 chars, fall back to ArrayPool for longer ids so a long conversationId can no longer crash the hosting process with StackOverflowException. - Use a Guid-suffixed temp file (\{path}.{guid}.tmp\) so concurrent SaveSessionAsync calls on the same conversation can no longer race on the same temp file. Best-effort temp cleanup on failure. - Bucket session files by agent.Name when set so two keyed agents that happen to share a conversationId no longer overwrite each other's persisted state. Single-agent / unnamed-agent cases keep the original flat layout (Python parity). DeclarativeWorkflowExecutor chat-protocol routing: - ConfigureChatProtocolRoutes uses IsAssignableFrom rather than exact type equality so a broader TInput (object, base interfaces) does not have its inherited inputTransform shadowed by handlers we register here. - HandleChatMessagesAsync / HandleChatMessageArrayAsync now advance through every message in the batch instead of keeping only the trailing one, so multi-message turns and replayed history are no longer silently truncated. AdvanceAsync gains a finalizeTurn flag so only the last message in the batch sends the result. Tests: - New FileSystemAgentSessionStoreTests covering constructor, fresh-session fallback for missing/empty files, root-directory creation, save/get round-trip, agent-Name scoping isolation, long conversationId, invalid-character sanitization, and concurrent-save behavior. - New InputConverterTests covering AppendFileContent: text/* data URI decode (with and without filename prefix), non-text data URI passthrough, malformed data URI fallback, and filename propagation onto UriContent / HostedFileContent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add tests for remaining PR review feedback (C2, D1, E1) C2: InputConverter — add 9 tests covering SDK content types that previously had no coverage: - SdkTextContent → TextContent (input + output paths) - SummaryTextContent → TextContent (input + output paths) - MessageContentReasoningTextContent → TextReasoningContent (input + output) - ComputerScreenshotContent (HTTP URL → UriContent, data: URI → DataContent, output path → UriContent) D1: OutputConverter — add 2 tests for the WorkflowEvent + Contents fall-through: - WorkflowEvent in RawRepresentation with text Contents must flow through the content-processing path (text-delta event emitted). - WorkflowEvent + ErrorContent must produce a failed event rather than be swallowed by the workflow branch. E1: SendActivityExecutor — extend CaptureActivityAsync to assert that the executor emits an AgentResponseEvent carrying the activity text with the correct ExecutorId and ChatRole.Assistant role. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Defense-in-depth: neutralize dot-segments in Sanitize and cap TryDecodeTextDataUri input size Addresses claude-opus-4.6 security review on PR #5589: - FileSystemAgentSessionStore.Sanitize now replaces all-dot segments (., .., ...) with underscores so a developer-controlled agent.Name cannot escape the root directory on Linux (where Path.GetInvalidFileNameChars only contains NUL and '/'). - InputConverter.TryDecodeTextDataUri rejects encoded payloads larger than 16 MiB before calling Convert.FromBase64String, preventing a single oversized data URI from triggering a multi-megabyte allocation. - Adds unit tests covering both fixes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix Linux-only failure in SaveSessionAsync_SanitizesInvalidPathCharactersAsync '?' is in Path.GetInvalidFileNameChars only on Windows, not on Linux/macOS, so the test failed on Ubuntu in CI. Use Path.GetInvalidFileNameChars()[0] (skipping NUL) to pick a guaranteed-invalid character for the running OS, and assert the result no longer contains it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address claude-opus-4.6 security/reliability review feedback WorkflowSession.cs: - ExecutorFailedEvent handler no longer leaks the internal executor ID in error messages. Mirror the WorkflowErrorEvent pattern: surface the exception's Message when _includeExceptionDetails is true, fall back to the generic 'An error occurred while executing the workflow.' otherwise. This also resolves the failing WorkflowHostSmokeTests assertions. FileSystemAgentSessionStore.cs: - GetSessionPath no longer has a write side effect. Directory.CreateDirectory for the per-agent bucket is now performed only on the SaveSessionAsync path, so a read miss on GetSessionAsync no longer leaves an empty directory on disk. - Adds GetSessionAsync_NoExistingFile_DoesNotCreateAgentDirectoryAsync to lock in the no-side-effect-on-read contract. OutputConverterTests.cs: - Strengthen ConvertUpdatesToEventsAsync_ToolApprovalRequest_NonFunctionToolCall_SkippedAsync to assert exactly one event (the terminal ResponseCompletedEvent) so a spurious output-item-added/-done leak would now fail the test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review: clean up comments and rename TryParseArguments - Remove Python-codebase references from C# XML docs and inline comments. - Drop fix-history comments referring to previously-resolved issues. - Drop `Defense-in-depth:` prefixes; keep the concrete `what & why`. - Drop `previously we kept only the trailing message` comment in DeclarativeWorkflowExecutor; just describe current loop behavior. - Rename InputConverter.TryParseArguments to ParseFunctionArgumentsObject to make the intent obvious at the call site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review: collision-free Sanitize, MAF-style refactors - FileSystemAgentSessionStore.Sanitize now percent-encodes invalid chars (and `%` itself) instead of replacing them with `_`, eliminating collisions like `foo/bar` vs `foo_bar` mapping to the same bucket. All-dot segments encode every dot so Windows trailing-dot trimming cannot reintroduce a navigable name. - AddFoundryResponses XML doc updated to accurately describe the default store root (/.checkpoints when hosted, {cwd}/.checkpoints locally). - DeclarativeWorkflowExecutor.ConfigureChatProtocolRoutes now uses exact type equality instead of IsAssignableFrom so a broad TInput (e.g. object) does not skip registering IEnumerable<ChatMessage>, which ChatProtocolExtensions.IsChatProtocol requires verbatim. - SendActivityExecutor uses context.YieldOutputAsync(response) instead of manually constructing AgentResponseEvent, so the activity will participate in any future OutputFilter coverage. - WorkflowSession handles AgentResponseEvent in its own switch case, avoiding the second typecheck against output.Data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): bridge declarative HITL through Foundry hosting via IExternalRequestEnvelope Introduce a new public interface IExternalRequestEnvelope in Microsoft.Agents.AI.Workflows that lets the runtime peek through a declarative-layer envelope without taking a circular reference back into the declarative package. ExternalInputRequest (declarative) implements it; ExternalInputResponse is constructed via the request's CreateResponse factory. WorkflowSession unwraps inner AIContent on the request side and rewraps the client's ChatMessage reply into an ExternalInputResponse on the response side. PortableValue cannot deserialize directly into an interface, so TryGetRequestEnvelope resolves the concrete type via RequestPortInfo.RequestType (TypeId -> Type.GetType) before casting. Public WorkflowHarness contract preserved: InvokeFunctionToolExecutor and WorkflowActionVisitor are unchanged from upstream, so public InvokeToolWorkflowTest scenarios continue to drive ExternalInputRequest / ExternalInputResponse directly through the harness. AgentFrameworkResponseHandler: skip prior conversation history replay when an existing session is being resumed (workflow checkpoint already holds the prior messages). WorkflowSession: when includeExceptionDetails is opted in, also unwrap DeclarativeActionException so HITL failures are debuggable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
6cd81286a9 |
.NET: dotnet: Add hosted-agent User-Agent supplement to outgoing requests (#5453)
* dotnet: Add hosted-agent User-Agent supplement to outgoing requests
When an agent runs inside a Foundry Hosted Agent, the outgoing
User-Agent header now includes 'agent-framework-hosted/{version}'
alongside the existing 'MEAI/{version}' segment.
- Add HostedAgentContext with AsyncLocal<string?> property
- MeaiUserAgentPolicy reads the supplement per-call
- AgentFrameworkResponseHandler sets/restores the context
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* chore: update hosted UA format to foundry-hosting/agent-framework-dotnet/{version}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Trying to get UA flowing, no luck yet.
* .NET: Polyfill MEAI OpenAIResponsesChatClient to add hosted-agent User-Agent supplement
When AgentFrameworkResponseHandler resolves an agent (i.e. we are running in a
hosted context), TryApplyUserAgent walks the agent's IChatClient decorator chain
to find MEAI's internal OpenAIResponsesChatClient and reflectively swaps its
inner _responseClient field with a DelegatingResponsesClient wrapper. The
wrapper overrides the public-virtual protocol methods to add a per-call
HostedAgentUserAgentPolicy to the RequestOptions and delegate to the inner
ResponsesClient. The OpenAI SDK's internal streaming overloads bottom out in
calls to the public-virtual non-streaming overloads via virtual dispatch on
this, so streaming is covered without overriding any non-virtual member.
The wrapper accepts any ResponsesClient-derived inner — both the Foundry
ProjectResponsesClient and the native OpenAI ResponsesClient — and preserves
the inner client's full pipeline (Transport, RetryPolicy, NetworkTimeout,
OrganizationId / ProjectId / UserAgentApplicationId, custom policies).
- Add DelegatingResponsesClient + HostedAgentUserAgentPolicy in Microsoft.Agents.AI.Foundry.Hosting.
- Add TryApplyUserAgent next to ApplyOpenTelemetry in FoundryHostingExtensions; wire it into AgentFrameworkResponseHandler.GetAgent for both keyed and default-agent paths.
- Drop earlier-iteration dead code: AddHostedAgentTelemetry extension, HostedUserAgentPolicy class, HostedAgentContext.cs, and the never-called ToRequestOptions helper.
- Revert RequestOptionsExtensions.MeaiUserAgentPolicy to MEAI-only (the supplement is now injected by the polyfill).
- Revert unrelated whitespace change in Agent_Step25_ToolboxServerSideTools sample.
- Tests cover streaming AND non-streaming, retry policy preservation, OrganizationId/ProjectId/UserAgentApplicationId pass-through, idempotency, native OpenAI ResponsesClient, and reflection guards for MEAI/OpenAI shape drift.
* .NET: Address review feedback on hosted-agent User-Agent polyfill
- TryApplyUserAgent: replace silent null-return with ArgumentNullException to match the codebase's convention.
- Add idempotency test (TryApplyUserAgent_CalledTwiceOnSameAgent_DoesNotDoubleWrap) — runs the polyfill twice on the same agent and asserts the wire UA contains exactly one foundry-hosting segment, proving the 'current is DelegatingResponsesClient' guard prevents nested wrapping.
- Add retry-double-append test (Polyfill_RetryWithinCall_DoesNotDuplicateSupplementInUserAgent) — exercises the HostedAgentUserAgentPolicy Contains-guard via a custom retry policy that re-runs the inner pipeline on the same message.
- Replace TryApplyUserAgent_NullAgent_ReturnsNullWithoutThrowing with TryApplyUserAgent_NullAgent_ThrowsArgumentNullException to match the new contract.
* .NET: Drop null check from TryApplyUserAgent and its now-redundant test
The two call sites in AgentFrameworkResponseHandler.GetAgent already null-check the agent before invoking TryApplyUserAgent, so the defensive ArgumentNullException is unreachable. Remove it and the corresponding test.
* .NET: Remove unused Microsoft.Shared.Diagnostics import in ServiceCollectionExtensions
The Throw.IfNull helper from this namespace was used by the now-removed null check in TryApplyUserAgent. Drop the unused import to satisfy IDE0005 in CI's full-project dotnet format run.
---------
Co-authored-by: alliscode <bentho@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com>
|
||
|
8b71f9459a |
Python: Feature/hosted dwf (#5531)
* Fix declarative Workflow.as_agent() by accepting list[Message] in start executor The declarative start executor (JoinExecutor) only advertised dict and str in its input_types, so WorkflowAgent.__init__ rejected it with 'Workflow's start executor cannot handle list[Message]'. Add list[Message] to the JoinExecutor handler annotation and add a matching branch in DeclarativeActionExecutor._ensure_state_initialized that extracts the last user-message text and falls through to the string-input initialization path, so =System.LastMessageText works end-to-end via as_agent(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Populate Conversation.messages from list[Message] trigger When Workflow.as_agent() is invoked with a list[Message], the start executor now populates Conversation.messages / Conversation.history / System.conversations.{id}.messages with prior turns only (excluding the latest user message), and surfaces the latest user message via Inputs.input and System.LastMessage*. This matches InvokeAzureAgent's contract that the messages binding holds prior turns and the executor itself appends the new user input before invoking, avoiding double-append of the trailing user turn while preserving full history (incl. assistant/system/tool roles and multi-modal content) for downstream actions. * Coerce Enum values when serializing PowerFx symbols MessageRole and other str-subclass Enums passed isinstance(v, str) and were forwarded to pythonnet unchanged. pythonnet then raised 'MessageRole value cannot be converted to System.String' for every PowerFx primitive when ConditionGroup/Expr eval walked the symbol table containing Conversation.messages. Reduce Enum members to their underlying value before the primitive check so eval sees plain strings/ints. * Foundry hosting: pass full conversation history to workflow agents _handle_inner_workflow only forwarded the latest user turn to WorkflowAgent.run, even though _handle_inner_agent already prepends history fetched from Foundry storage to the messages it sends a regular agent. Declarative workflows reset Conversation.messages on every run (state.initialize), so checkpoint replay alone does not give them prior turns - the host has to pass them in, the same way it does for non-workflow agents. Mirror that contract: fetch context.get_history() and pass [*history, *input_messages] to the workflow agent. * feat(workflows): support combined message + checkpoint_id for multi-turn continuation Allow Workflow.run(message=..., checkpoint_id=...) so callers can restore prior workflow state from a checkpoint AND deliver a new message to the start executor in a single call. The existing reset_context logic already preserves shared state when checkpoint_id is set, so this gives us 'fresh start executor invocation with prior state intact' - exactly what hosted multi-turn declarative workflows need. - _workflow.py: drop the message+checkpoint_id mutual exclusion and update _execute_with_message_or_checkpoint to do both (restore then execute) when both are provided. - _agent.py: in _run_core's checkpoint branch, also forward input_messages so WorkflowAgent.run(messages, checkpoint_id=...) works end-to-end. Falls back to the legacy 'restore only' behavior when messages are absent. - _declarative_base.py: detect continuation in _ensure_state_initialized by checking whether DECLARATIVE_STATE_KEY already exists in shared state; if so, refresh inputs/LastMessage* and append non-user trigger messages instead of calling state.initialize() (which would wipe Conversation/Local/System). - foundry_hosting/_responses.py: collapse the host's two-call pattern (restore-only, then fresh run) into a single combined call now that the underlying APIs support it. - tests: drop the assertion that combined message+checkpoint_id raises. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Pivot: preserve workflow state across run() calls Replace the prior 'combined message + checkpoint_id in one run()' approach with a cleaner default: Workflow.run no longer wipes shared state or runner- context messages between calls. Iteration counting and per-run kwargs still reset on a fresh-message run; checkpoint and responses runs are continuations that preserve everything. This lets a WorkflowAgent be invoked repeatedly on the same instance and maintain multi-turn context (e.g. accumulated Conversation.messages) without asking developers to opt in. Hosted-agent multi-turn pattern becomes two explicit calls: restore-from-checkpoint (drive to idle), then run-with-message. Key changes: - _workflow.py: drop _state.clear() and reset_for_new_run() from run(). Reset iteration count and run kwargs on fresh-message runs only. Restore 'Cannot provide both message and checkpoint_id' validation. Add async guard: fresh-message run with un-drained pending executor messages from a prior run is invalid. - _runner.py: clear _state before import_state in restore_from_checkpoint so restore is authoritative (import_state merges, not replaces). - _agent.py: revert checkpoint branch to restore-only (no message forward). - _responses.py (foundry_hosting): two-call host pattern - restore checkpoint silently, then run with new user input. - tests: state-preservation is the new default; rebuild Workflow for clean slate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix CI lint and mypy issues from prior pivot commit - _workflow.py: collapse nested if (SIM102), drop redundant assignment (RET504) - _declarative_base.py: remove unused last_user_msg = tail assignment whose Message | None type clashed with the prior Message-typed branch Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review: fix Inputs.input update and checkpoint storage path - _declarative_base.py: continuation branch was writing 'Inputs.input' via state.set, which routes to the Custom namespace and never updates the PowerFx-visible Workflow.Inputs.input. Update state_data['Inputs'] in place via get_state_data / set_state_data so =Workflow.Inputs.input and =inputs.input see the new turn's user text on continuation. - _declarative_base.py: refresh docstring to clarify that on a list[Message] trigger, Conversation.messages excludes the current user message at the start of the turn (agent executors append it before invoking the inner agent). - _responses.py: when previous_response_id is supplied (no conversation_id), the prior checkpoint lives under <storage>/<previous_response_id> but new checkpoints must land under <storage>/<current_response_id> for the next turn to find them. Hold onto restore_storage from the get_latest lookup and pass it to the restore-only run; pass write_storage (current id) to the message-delivery run and to checkpoint cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix pyright errors in _declarative_base.py for CI - Replace state._state.get(...) protected access with new public is_initialized() method on DeclarativeWorkflowState (also clearer intent for the continuation detection use case). - Add narrow pyright ignores for the Any-typed trigger paths that pyright cannot fully narrow (the list[Message] isinstance loop and the fallback-DefaultTransform branch). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review batch: tests + Workflow.reset escape hatch * Add Workflow.reset() public method as recovery escape hatch when an in-flight run aborted (e.g. WorkflowConvergenceException) and the workflow is not checkpointed. Update the in-flight messages guard's error message to point callers at it. * Add test_workflow_run_inflight_messages_guard exercising both the guard (sync + streaming) and the reset() recovery path. * Add test_workflow_reset_rejects_concurrent_runs to lock down the in-progress guard on reset. * Add test_as_agent_continuation_preserves_prior_state covering the is_continuation branch in _ensure_state_initialized: stamps a marker between calls and asserts it survives, while Inputs.input and System.LastMessageText refresh to the new turn. * Add test_powerfx_safe.py regression tests for the Enum branch in _make_powerfx_safe (str-subclass, int-subclass, plain Enum, and Enums nested in dict/list). * Drop redundant @pytest.mark.asyncio on test_as_agent_round_trip_with_last_message_text (asyncio_mode='auto'). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skip restore-only pre-pass when checkpoint has pending request_info Address Copilot review on _responses.py: the restore-only checkpoint replay populates self._agent.pending_requests for any request_info events captured in the checkpoint. The follow-up run(input_messages) call would then route through WorkflowAgent._process_pending_requests, which expects function-response content and rejects plain text input as 'unexpected content while awaiting request info responses'. Workflows resumed from a checkpoint that was idle-with-pending-requests would therefore fail every subsequent plain-text user turn. Inspect the loaded checkpoint and skip the pre-pass when its pending_request_info_events dict is non-empty. Workflows that don't use request_info (the current sample set) are unaffected; workflows that do will fall through to a fresh-message run rather than silently corrupting the routing state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Loosen azure-ai-agentserver-* pins to major version The exact-version pins on azure-ai-agentserver-{core,responses,invocations} forced foundry-hosting consumers to upgrade in lockstep with every beta bump from upstream. Switch to '>=current,<next-major' so we pick up patch and feature updates within the same major series without a coordinated release. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Drop Workflow.reset(); checkpointing is the recovery path The in-flight-messages guard prevented silent misbehavior, but the companion Workflow.reset() escape hatch only cleared _messages while leaving iteration count, executor-local state, and shared State mutations in an indeterminate condition after a mid-run failure. That gave a false sense of recovery. Recovery from a mid-run failure is supported only via checkpoint restoration. Keep the guard and reframe its error message accordingly; remove reset() and its tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Tao's review on PR 5531 - Rename Workflow._run_workflow_with_tracing parameter is_fresh_message_run -> is_continuation (default False, inverted). Fresh-message turns reset per-run accounting; continuations (checkpoint restores, responses replays) preserve it. - Simplify the in-flight-messages guard: _validate_run_params already enforces that 'message' is mutually exclusive with 'checkpoint_id' and 'responses', so the additional checks were dead code. - foundry_hosting _responses: move the restore-only pre-pass above emit_created/emit_in_progress; restore is preparation, not run progress. Drop the skip-restore gate (state preservation requires unconditional restore) and instead clear agent.pending_requests after the restore-only call. Collapse over-conditioned check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Don't clear pending_requests after restore-only pre-pass Pending requests in the restored checkpoint represent genuinely outstanding HITL requests. The next user input may carry function responses (Responses API `function_call_output` items become FunctionResultContent / FunctionApprovalResponseContent), which `WorkflowAgent._process_pending_requests` correctly extracts and matches against the populated `pending_requests`. Clearing them after restore would silently drop that state and force the next turn to be treated as a fresh input even when the caller is responding to the outstanding requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Evan Mattson <35585003+moonbox3@users.noreply.github.com> |
||
 
|
5fe8941ff9 |
.NET: dotnet: Add server-side Foundry Toolbox support and fix SDK beta.4 br… (#5450)
* dotnet: Add server-side Foundry Toolbox support and fix SDK beta.4 breaking changes Add FoundryToolbox and AIProjectClient extensions to Microsoft.Agents.AI.Foundry.Hosting for server-side toolbox tool integration matching Python's FoundryChatClient.get_toolbox() pattern. Tools are fetched from the Foundry project SDK and passed as server-side tools in the Responses API request. New files: - FoundryToolbox.cs: Core implementation using AgentAdministrationClient SDK - AIProjectClientToolboxExtensions.cs: Extension methods on AIProjectClient - Agent_Step25_ToolboxServerSideTools sample with create helper and combine flow - 19 unit tests covering param validation, conversion, sanitization, and extensions SDK breaking changes (Azure.AI.AgentServer.Responses beta.3 -> beta.4): - FunctionToolCallOutputResource renamed to OutputItemFunctionToolCallOutput - AzureAIAgentServerResponsesModelFactory made internal, replaced with direct constructors - ResponseUsage constructor now requires non-null token details parameters Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: reuse endpoint variable in CreateSampleToolboxAsync Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: pass endpoint through static local functions to avoid capture Static local functions cannot capture top-level variables. Thread the endpoint parameter through Main, CombineToolboxes, and CreateSampleToolboxAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: remove unused projectClient param from CreateSampleToolboxAsync Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step25_ToolboxServerSideTools/README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step25_ToolboxServerSideTools/Program.cs Co-authored-by: westey <164392973+westey-m@users.noreply.github.com> * Update dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step25_ToolboxServerSideTools/Program.cs Co-authored-by: westey <164392973+westey-m@users.noreply.github.com> * Removing GetToolbocVersion. * Removing tests for GetToolboxVersion * fix: map cached/reasoning token counts in ConvertUsage instead of hardcoding zeros Extract InputTokenDetails.CachedTokenCount and OutputTokenDetails.ReasoningTokenCount from UsageDetails.AdditionalCounts, matching the pattern in AgentResponseExtensions. Also accumulate detail counts when merging with existing usage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: westey <164392973+westey-m@users.noreply.github.com> |
||
|
|
aee1acbf8b |
.NET: Foundry Evals integration for .NET (#4914)
* Foundry Evals integration for .NET - Core evaluation framework: EvalItem, LocalEvaluator, FunctionEvaluator, EvalChecks - IAgentEvaluator interface with MeaiEvaluatorAdapter bridge - AgentEvaluationExtensions for agent.EvaluateAsync() overloads - FoundryEvals wrapping MEAI quality/safety evaluators - ConversationSplitters (LastTurn, Full) and IConversationSplitter - EvalItem.PerTurnItems() for multi-turn decomposition - HasImageContent for multimodal content detection - WorkflowEvaluationExtensions for per-agent workflow evaluation - 7 eval samples mirroring Python parity: 02-agents/Evaluation: SimpleEval, ExpectedOutputs, Multimodal 03-workflows/Evaluation: WorkflowEval 05-end-to-end/Evaluation: FoundryQuality, MixedProviders, ConversationSplits - Comprehensive unit tests (1958 passing) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rewrite FoundryEvals to use real Foundry Evals API Replace MEAI evaluator shim with actual OpenAI EvaluationClient protocol methods. FoundryEvals now creates eval definitions, submits runs, polls for completion, and fetches per-item results server-side. - New constructor: FoundryEvals(AIProjectClient, model, evaluators) - Add FoundryEvalConverter for MEAI ChatMessage -> Foundry JSON format - Add EvalId, RunId, ReportUrl to AgentEvaluationResults - All 20 built-in evaluator constants now work (agent, tool, quality, safety) - Remove Microsoft.Extensions.AI.Evaluation.Quality/Safety dependencies - Update all samples for new constructor (no more ChatConfiguration) - Replace BuildEvaluators tests with ResolveEvaluator tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add response output to CustomEvals and ExpectedOutputs samples Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: pagination, validation, error handling, tests FoundryEvals fixes: - Add pagination for output items (has_more/after cursor) - Add guard clauses for pollIntervalSeconds/timeoutSeconds <= 0 - Fix double TryGetProperty for passed field parsing - Throw on all-tool-evaluators with no tool definitions - Fix XML doc (default 300s, not 180s) New tests (30 added, 1989 total): - EvalChecks: NonEmpty, ContainsExpected (pass/fail/skip/case), HasImageContent, ToolCallsPresent - FoundryEvalConverter: ConvertMessage (text, image, function call, function results fan-out, empty fallback, mixed content), ConvertEvalItem, BuildTestingCriteria (quality/agent/tool/groundedness data mappings), BuildItemSchema Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix review: null-refs, Data.ToString() bug, ContainsExpected, add tests - Fix NullReferenceException in sample Response display (pattern matching) - Fix WorkflowEvaluationExtensions Data?.ToString() producing type names instead of message text (pattern-match ChatMessage/AgentResponse/list) - Change EvalChecks.ContainsExpected to return Passed=false when no ExpectedOutput (was silently passing, masking misconfiguration) - Add EvalItem constructor tests with LastTurn/Full/null splitters - Add FoundryEvalConverter.ConvertMessage DataContent (base64 image) test - Add ExtractAgentData tests with ChatMessage, list, and AgentResponse data Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix review: conversation fidelity, eval caching, fallback tests - WorkflowEvaluationExtensions: preserve full response messages (tool calls, intermediate) instead of synthetic 2-message conversation. Cast completed Data to AgentResponse and use Messages when available, fallback to text. - FoundryEvals: cache evalId per schema shape (hasContext, hasTools) so subsequent EvaluateAsync calls create runs under the same eval definition. - MeaiEvaluatorAdapter: code already correctly passes queryMessages (not full conversation) to IEvaluator — no change needed, verified by inspection. - Add tests: AgentResponse full messages preservation, unknown object ToString() fallback for ExtractAgentData. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rename AzureAI→Foundry: move eval files, update references - Move FoundryEvals.cs and FoundryEvalConverter.cs from Microsoft.Agents.AI.AzureAI to Microsoft.Agents.AI.Foundry - Update namespace from AzureAI to Foundry in both files - Add explicit usings required by Foundry project (no implicit usings) - Move FoundryEvalConverter tests to Foundry.UnitTests project (avoids ReplacingRedactor type conflict from dual project refs) - Update all sample csproj references and using statements - Remove Foundry project reference from AI UnitTests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * PR review round 4: wire up tool extraction, remove eval cache, fix null safety - BuildEvalItem: extract tools from agent via GetService<ChatOptions>() into EvalItem.Tools (Python parity) - FoundryEvals: remove eval ID cache - each call creates fresh definition (matches Python behavior) - FoundryEvals: replace null-forgiving operators with descriptive InvalidOperationException - MixedProviders sample: remove unnecessary explicit PackageReferences (transitively provided) - FoundryEvalConverter: document that tool results take precedence over text content - Add LocalEvaluator zero-checks test documenting 0 metrics = failed behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Python-dotnet parity: 9 feature gaps filled New checks: - ToolCallArgsMatch() — verify tool call names + argument subset match - ToolCalledCheck(ToolCalledMode.Any, ...) — match any of the specified tools - ToolCalledMode enum (All/Any) FoundryEvals enhancements: - Default evaluators now [Relevance, Coherence, TaskAdherence] (was Relevance, Coherence) - Auto-add ToolCallAccuracy when items have tool definitions - EvaluateTracesAsync — evaluate by response_ids, trace_ids, or agent_id - EvaluateFoundryTargetAsync — evaluate deployed Foundry targets Result type enrichment: - AgentEvaluationResults: added Status, Error, PerEvaluator, DetailedItems - New EvalItemResult/EvalScoreResult/PerEvaluatorResult types - FoundryEvals populates all new fields from API responses Workflow fix: - Skip internal executors (_*, input-conversation, end-conversation, end) Tests: 8 new tests covering ToolCallArgsMatch, ToolCalledMode.Any, internal executor filtering Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add MeaiEvaluatorAdapter and PerTurnItems edge case tests - 3 tests for MeaiEvaluatorAdapter: query message forwarding, synthetic response fallback, multiple items aggregation - 3 tests for EvalItem.PerTurnItems: empty conversation, no user messages, system+assistant only - StubEvaluator and StubChatClient test helpers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Blocking link check for outdated package in DevUI. * Replace Dictionary<string, object> payloads with typed wire models Introduce internal FoundryEvalWireModels.cs with compile-time-safe types for the OpenAI Evals API wire format. The OpenAI .NET SDK (2.9.1) only provides protocol-level methods with BinaryContent/ClientResult — no typed request models. These internal models replace scattered dictionary literals with [JsonPropertyName]-annotated classes, giving: - Compile-time safety (typos become build errors) - Single point of change when the API evolves - IntelliSense discoverability - Cleaner serialization via JsonPolymorphic for content items Models: WireContentItem hierarchy (text, image, tool_call, tool_result), WireMessage, WireEvalItemPayload, WireTestingCriterion, WireItemSchema, WireCreateEvalRequest, WireCreateRunRequest, and data source variants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skip metric when Foundry returns neither score nor passed When an evaluator returns no score and no passed value, the previous code created BooleanMetric(name, false), which falsely failed items via ItemPassed. Now we skip the MEAI metric entirely for indeterminate results — the raw data remains available in DetailedItems for diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #4914 review comments: fix tool evaluator bug and add tests - Fix duplicate ToolCallAccuracy: resolve evaluator names before checking against ToolEvaluators set (Comment 2) - Make FilterToolEvaluators internal for testability; add tests for the ArgumentException edge case when all evaluators are tool-type (Comment 3) - Add CancellationToken test for LocalEvaluator (Comment 4) - Add EvaluateAsync integration test on Run with sequential workflow and per-agent SubResults verification (Comment 5) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Peter's review comments on PR #4914 - Add trailing newline to Evaluation_FoundryQuality.csproj (Comment 6) - Make evaluator name lookups case-insensitive: switch BuiltinEvaluators, ToolEvaluators, AgentEvaluators, and ResolveEvaluator's StartsWith check from Ordinal to OrdinalIgnoreCase (Comment 7) - Add Trace.TraceWarning when Foundry returns fewer results than submitted items, indicating expected vs actual count before padding (Comment 8) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add Microsoft.Extensions.AI.Evaluation packages to Directory.Packages.props These were removed in #5269 as unused, but are needed by the Foundry and core evaluation integration added in this PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
35adfdb318 |
Python: Foundry Evals integration for Python (#4750)
* Foundry Evals integration for Python Merged and refactored eval module per Eduard's PR review: - Merge _eval.py + _local_eval.py into single _evaluation.py - Convert EvalItem from dataclass to regular class - Rename to_dict() to to_eval_data() - Convert _AgentEvalData to TypedDict - Simplify check system: unified async pattern with isawaitable - Parallelize checks and evaluators with asyncio.gather - Add all/any mode to tool_called_check - Fix bool(passed) truthy bug in _coerce_result - Remove deprecated function_evaluator/async_function_evaluator aliases - Remove _MinimalAgent, tighten evaluate_agent signature - Set self.name in __init__ (LocalEvaluator, FoundryEvals) - Limit FoundryEvals to AsyncOpenAI only - Type project_client as AIProjectClient - Remove NotImplementedError continuous eval code - Add evaluation samples in 02-agents/ and 03-workflows/ - Update all imports and tests (167 passing) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: resolve mypy redundant-cast errors while keeping pyright happy Use cast(list[Any], x) with type: ignore[redundant-cast] comments to satisfy both mypy (which considers casting Any redundant) and pyright strict mode (which needs explicit casts to narrow Unknown types). Also fix evaluator decorator check_name type annotation to be explicitly str, resolving mypy str|Any|None mismatch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: CI failures — pyupgrade, evaluator overloads, sample API, reset attr - Apply pyupgrade: Sequence from collections.abc, remove forward-ref quotes - Add @overload signatures to evaluator() for proper @evaluator usage - Fix evaluate_workflow sample to use WorkflowBuilder(start_executor=) API - Fix _workflow.py executor.reset() to use getattr pattern for pyright - Remove unused EvalResults forward-ref string in default_factory lambda Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: skip gRPC-dependent observability test The test_configure_otel_providers_with_env_file_and_vs_code_port test triggers gRPC OTLP exporter creation, but the grpc dependency is optional and not installed by default. Add skipif decorator matching the pattern used by all other gRPC exporter tests in the same file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: add nosec B101 for bandit assert check Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: align eval samples with repo conventions - Move module docstrings before imports (after copyright header) - Add -> None return type to all main() and helper functions - Fix line-too-long in multiturn sample conversation data - Add Workflow import for typed return in all_patterns_sample Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review feedback: async fixes, sample bugs, deprecation warnings - Simplify _ensure_async_result to direct await (async-only clients) - Replace get_event_loop() with get_running_loop() - Narrow _fetch_output_items exception handling to specific types - Add warning log when _filter_tool_evaluators falls back to defaults - Add DeprecationWarning to options alias in Agent.__init__ - Add DeprecationWarning to evaluate_response() - Rename raw key to _raw_arguments in convert_message fallback - Fix evaluate_agent_sample.py: replace evals.select() with FoundryEvals() - Fix evaluate_multiturn_sample.py: use Message/Content/FunctionTool types - Fix evaluate_workflow_sample.py: replace evals.select() with FoundryEvals() - Update test mocks to use AsyncMock for awaited API calls Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add test coverage for review feedback items - Add num_repetitions=2 positive test verifying 2×items and 4 agent calls - Add _poll_eval_run tests: timeout, failed, and canceled paths - Add evaluate_traces tests: validation error, response_ids path, trace_ids path - Add evaluate_foundry_target happy-path test with target/query verification Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix ruff ISC004 lint error and apply formatter - Wrap implicit string concatenation in parens in evaluate_multiturn_sample.py - Apply ruff formatter to 6 other files with minor formatting drift Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove core type changes (extracted to fix/workflow-stale-session branch) Reverts changes to _agents.py, _agent_executor.py, and _workflow.py back to upstream/main. These fixes are now in a separate PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review round 2: bugs, tests, and architecture Code fixes: - Fix _normalize_queries inverted condition (single query now replicates to match expected_count) - Fix substring match bug: 'end' in 'backend' matched; use exact set lookup for executor ID filtering - Fix used_available_tools sample: tool_definitions→tools param, use FunctionTool attribute access instead of dict .get() - Add None-check in _resolve_openai_client for misconfigured project - Add Returns section to evaluate_workflow docstring - Cache inspect.signature in @evaluator wrapper (avoid per-item reflection) Architecture: - Extract _evaluate_via_responses as module-level helper; evaluate_traces now calls it directly instead of creating a FoundryEvals instance - Move Foundry-specific typed-content conversion out of core to_eval_data; core now returns plain role/content dicts, FoundryEvals applies AgentEvalConverter in _evaluate_via_dataset Tests: - evaluate_response() deprecation warning emission and delegation - num_repetitions > 1 with expected_output and expected_tool_calls - Mock output_items.list in test_evaluate_calls_evals_api - Update to_eval_data assertions for plain-dict format - Unknown param error now raised at @evaluator decoration time Skipped (separate PR): executor reset loop, xfail removal, options alias Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix CI: revert test_full_conversation, fix pyright errors - Revert test_full_conversation.py to upstream/main (the session preservation test was incorrectly changed to assert clearing) - Fix pyright reportUnnecessaryComparison on get_openai_client() None check by adding ignore comment - Fix pyright reportPrivateUsage: add public EvalItem.split_messages() method and use it in FoundryEvals._evaluate_via_dataset instead of accessing private _split_conversation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review round 3: reliability, test gaps, cleanup - Add try/except guard for non-numeric score in _coerce_result - Add poll_interval minimum bound (0.1s) to prevent tight loops - Add runtime async client check in _resolve_openai_client - Remove _ensure_async_result wrapper (10 call sites → direct await) - Better error message when queries provided without agent - Import-time asserts for evaluator set consistency - Remove 28 redundant @pytest.mark.asyncio decorators - Add doc note about _raw_arguments sensitive data - Tests: tool_called_check mode=any, _normalize_queries branches, _extract_result_counts paths, _extract_per_evaluator, bare check via evaluate_agent, output_items assertion, modulo wrapping, async client check, queries-without-agent error Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix CI: ruff S101 assert, pyright and mypy arg-type errors - Replace module-level assert with if/raise for evaluator set consistency checks (ruff S101 disallows bare assert) - Add type: ignore[arg-type] and pyright: ignore[reportArgumentType] on OpenAI SDK evals API calls that pass dicts where typed params are expected (SDK accepts dicts at runtime) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review round 4: bugs, reliability, test fixes - Fix all_passed ignoring parent result_counts when sub_results present - Fix _extract_tool_calls: parse string arguments via json.loads before falling back to None (real LLM responses use string arguments) - Sanitize _raw_arguments to '[unparseable]' to avoid leaking sensitive tool-call data to external evaluation services - Add NOTE comment on to_eval_data message serialization dropping non-text content (tool calls, results) - Eliminate double conversation split in _evaluate_via_dataset: build JSONL dicts directly from split_messages + AgentEvalConverter - Raise poll_interval floor from 0.1s to 1.0s to prevent rate-limit exhaustion - Fix MagicMock(name=...) bug in test: sets display name not .name attr - Fix mock_output_item.sample: use MagicMock object instead of dict so _fetch_output_items exercises error/usage/input/output extraction Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review round 5: reliability, docs, test coverage Code fixes: - Move import-time RuntimeError checks to unit tests (avoids breaking imports for all users on developer set-drift mistake) - _filter_tool_evaluators now raises ValueError when all evaluators require tools but no items have tools (was silently substituting) - Add poll_interval upper bound (60s) to prevent single-iteration sleep - Log exc_info=True in _fetch_output_items for debugging API changes - Fix evaluate() docstring: remove claim about Responses API optimization - Validate target dict has 'type' key in evaluate_foundry_target - Document to_eval_data() limitation: non-text content is omitted Tests: - TestEvaluatorSetConsistency: verify _AGENT/_TOOL subsets of _BUILTIN - TestEvaluateTracesAgentId: agent_id-only path with lookback_hours - TestFilterToolEvaluatorsRaises: ValueError on all-tool no-items - TestEvaluateFoundryTargetValidation: target without 'type' key - Assert items==[] on failed/canceled poll results - Mock output_items.list in response_ids test for full flow - TestAllPassedSubResults: result_counts=None + sub_results delegation and parent failures override sub_results - TestBuildOverallItemEmpty: empty workflow outputs returns None Skipped r5-07 (_raw_arguments length hint): marginal debugging value, could leak content size information. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix error message: evaluate_responses() → evaluate_traces(response_ids=...) The referenced function doesn't exist; the correct API is evaluate_traces(response_ids=...) from the azure-ai package. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove dead to_eval_data() method, fix docstring claims - Remove to_eval_data() from EvalItem (dead code after r4-05 JSONL refactor) - Migrate 15 tests from to_eval_data() to split_messages() - Update sample to use split_messages() + Message properties - Remove unimplemented Responses API optimization docstring claim - Update split_messages() docstring to not reference removed method Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Reduce default eval timeout from 600s to 180s (3 minutes) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove dead _evaluate_via_responses method from FoundryEvals The method was never called — evaluate() uses _evaluate_via_dataset, and evaluate_traces() calls _evaluate_via_responses_impl directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert unrelated formatting changes to get-started samples Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix pyright: remove phantom FoundryMemoryProvider import, apply ruff format - Remove import of non-existent _foundry_memory_provider module (incorrectly kept during rebase conflict resolution) - Apply ruff formatter to test_local_eval.py and get-started samples Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix eval samples: use FoundryChatClient for Agent() The upstream provider-leading client refactor (#4818) made client= a required parameter on Agent(). Update the three getting-started eval samples to use FoundryChatClient with FOUNDRY_PROJECT_ENDPOINT, matching the standard pattern from 01-get-started samples. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Simplify self-reflection sample using FoundryEvals Replace ~80 lines of manual OpenAI evals API code (create_eval, run_eval, manual polling, raw JSONL params) with FoundryEvals: - evaluate_groundedness() uses FoundryEvals.evaluate() with EvalItem - Remove create_openai_client(), create_eval(), run_eval() functions - Remove openai SDK type imports (DataSourceConfigCustom, etc.) - run_self_reflection_batch creates FoundryEvals instance once, reuses it for all iterations across all prompts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update eval samples to FoundryChatClient and FOUNDRY_PROJECT_ENDPOINT - Migrate all foundry_evals samples from AzureOpenAIResponsesClient to FoundryChatClient - Update env var from AZURE_AI_PROJECT_ENDPOINT to FOUNDRY_PROJECT_ENDPOINT - Use AzureCliCredential consistently across all samples - Fix README.md: correct function names (evaluate_dataset -> FoundryEvals.evaluate, evaluate_responses -> evaluate_traces) - Update self_reflection .env.example and README.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix lint errors in eval samples (E501, ASYNC240, formatting) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove evaluate_all_patterns_sample.py (redundant with focused samples) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix async credential mismatch: use azure.identity.aio for async AIProjectClient AIProjectClient from azure.ai.projects.aio requires an async credential. Switch all foundry_evals samples from azure.identity.AzureCliCredential to azure.identity.aio.AzureCliCredential. Also pass project_client to FoundryChatClient instead of duplicating endpoint+credential. Close credential in self_reflection sample to avoid resource leak. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert test_observability.py to upstream/main (not our test) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address moonbox3 review: sphinx docstrings, pagination, isinstance check - Convert all Example:: / Typical usage:: code blocks to .. code-block:: python format matching codebase convention (both _evaluation.py and _foundry_evals.py) - Add async pagination in _fetch_output_items via async for (handles large result sets) - Replace hasattr(__aenter__) with isinstance(client, AsyncOpenAI) in _resolve_openai_client - Move AsyncOpenAI import from TYPE_CHECKING to runtime (needed for isinstance) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix test failures and address remaining moonbox3 review comments - Fix tests: use MagicMock(spec=AsyncOpenAI) for project_client mocks (isinstance check now requires proper type, not duck-typing) - Fix tests: replace mock_page.__iter__ with _AsyncPage helper for async for - Fix evaluate_response: auto-extract queries from response messages when query is not provided (previously always raised ValueError) - Add debug logging when skipping internal _-prefixed executor IDs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Tao's PR review comments on Foundry Evals - T1: Add comment explaining builtin.* pass-through in _resolve_evaluator - T2: Add comment referencing OpenAI evals API for testing_criteria dict - T3: Document Mustache-style {{item.*}} template placeholders - T4: Document poll loop 60s sleep upper bound rationale - T5: Narrow run type to RunRetrieveResponse, use typed field access instead of vars()/getattr dance in _extract_result_counts and _extract_per_evaluator; use run.error and run.report_url directly - T6: Clarify openai_client docstring re: Azure Foundry endpoint - T8: Remove misleading empty expected_tool_calls from sample - Update tests to match real SDK PerTestingCriteriaResult shape Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove unnecessary Any union from run type annotations RunRetrieveResponse is the correct type — no backward compat needed for a brand new feature. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Accept FoundryChatClient instead of raw AsyncOpenAI FoundryEvals now takes client: FoundryChatClient as its primary parameter instead of openai_client: AsyncOpenAI. The builtin.* evaluators require a Foundry endpoint, so the type should reflect that. - FoundryEvals.__init__: client: FoundryChatClient replaces openai_client - evaluate_traces / evaluate_foundry_target: same change - _resolve_openai_client: extracts .client from FoundryChatClient - project_client fallback retained for standalone functions - All samples updated to construct FoundryChatClient and pass as client= - Tests updated (openai_client= → client=) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove implicit 60s upper bound on poll interval If a developer sets a higher poll_interval, respect it. Only clamp to remaining time and enforce a 1s minimum for rate-limit protection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove 1s floor on poll interval — let the developer control it Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update python/samples/05-end-to-end/evaluation/foundry_evals/.env.example Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com> * Update python/samples/02-agents/evaluation/evaluate_agent.py Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com> * Address eavanvalkenburg review (round 2) on Python eval PR - Rename model_deployment -> model across FoundryEvals and all samples - Make model param optional, resolves from client.model - Convert EvalResults from dataclass to regular class - Remove deprecated evaluate_response() function - Refactor splitters: BUILT_IN_SPLITTERS dict + standalone functions - Change per_turn_items from classmethod to staticmethod - Simplify EvalCheck type alias to use Awaitable[CheckResult] - Remove errored property from EvalResults - Remove default value from Evaluator protocol eval_name - Rename assert_passed -> raise_for_status, add EvalNotPassedError - Type agent param as SupportsAgentRun | None - Fix Arguments docstring - Update __init__.py exports - Update all tests and samples Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move FoundryEvals to foundry package, split tool eval sample - Move _foundry_evals.py from azure-ai to foundry package - Move test_foundry_evals.py to foundry/tests/ - Update lazy re-exports in agent_framework.foundry namespace - Update .pyi type stubs - All samples now import from agent_framework.foundry - Split tool-call evaluation into evaluate_tool_calls_sample.py - Fix all_passed to check errored count from result_counts - Fix raise_for_status to include errored item details Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Auto-create FoundryChatClient from env vars when no client provided FoundryEvals() now works zero-config when FOUNDRY_PROJECT_ENDPOINT and FOUNDRY_MODEL environment variables are set. Auto-creates a FoundryChatClient under the hood, matching the established env var pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix pyright errors: remove dead _normalize_queries, suppress EvalAPIError check - Remove unused _normalize_queries function and its tests - Add pyright ignore for EvalAPIError None check (defensive guard) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Support multimodal image content in eval pipeline Add image (data/uri) content handling to AgentEvalConverter.convert_message() so that Content.from_data() and Content.from_uri() image payloads are preserved as input_image parts in the Foundry evaluator format. - Handle Content type='data' and type='uri' → emit input_image parts - Add 6 unit tests for image content through convert_message/convert_messages - Add integration test verifying images flow through EvalItem → JSONL path - Add evaluate_multimodal.py sample demonstrating local image eval Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address remaining review comments - Fix project_client docstring to say async-only (not sync/async) - Add builtin evaluator name validation warning in _resolve_evaluator - Replace getattr with typed attribute access in _poll_eval_run, _extract_result_counts, _extract_per_evaluator, _fetch_output_items - Remove cast import from _foundry_evals (no longer needed) - Tighten _coerce_result: honour explicit 'passed' when both 'score' and 'passed' are present; remove performative cast - Fix self_reflection sample: add env file existence check - Fix traces sample: correct Pattern 2 section label - Update all Foundry eval samples to FoundryChatClient + FOUNDRY_MODEL (remove AIProjectClient + AZURE_AI_MODEL_DEPLOYMENT_NAME pattern) - Add eval_name and OpenAI client docs to FoundryEvals docstring - Update test mocks to match typed SDK objects (_MockResultCounts) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix ruff lint errors (E501, SIM108, SIM102) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix pyright errors: type-narrow dict to dict[str, Any], add ignore comments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Replace ConversationSplitter type alias with Protocol ConversationSplitter is now a runtime-checkable Protocol with a named 'conversation' parameter, making the expected signature self-documenting. ConversationSplit enum members gain a __call__ method so they satisfy the protocol directly -- ConversationSplit.LAST_TURN(conversation) works. This simplifies _split_conversation from an isinstance dispatch to a single split(conversation) call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Standardize on AZURE_AI_MODEL_DEPLOYMENT_NAME and fix Unicode in samples - Replace FOUNDRY_MODEL with AZURE_AI_MODEL_DEPLOYMENT_NAME in all eval samples to match repo convention - Replace Unicode symbols with ASCII equivalents in all eval sample print statements to avoid cp1252 encoding errors on Windows Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update python/samples/03-workflows/evaluation/evaluate_workflow.py Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com> * Rename ADR 0020 to 0023 (foundry evals integration) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com> |
||
|
|
3f964c4cdb |
Removing old code-gen docs from dotnet root (#4997)
Co-authored-by: alliscode <bentho@microsoft.com> |
||
|
|
9dfe7c40ca |
Add ADR-0020: Foundry Evals integration (#4731)
* Add ADR-0020: Foundry Evals integration design Captures the design for integrating Azure AI Foundry Evaluations with agent-framework. Key decisions: - EvalItem with conversation (list[Message]) as single source of truth - query/response derived from configurable conversation split strategies - Tools as list[FunctionTool] (including auto-extracted MCP tools) - FoundryEvals provider with auto-detection of evaluator capabilities - LocalEvaluator with @function_evaluator decorator for local checks - Consistent Python/C# APIs: evaluate_agent, evaluate_workflow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Mark ADR 0020 Foundry Evals as accepted Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
db48f8ce09 |
.NET: Fixing issue with invalid node Ids when visualizing dotnet workflows. (#4269)
* Fix Mermaid rendering errors in WorkflowVisualizer.ToMermaidString Fix two bugs in the Mermaid diagram output: 1. Use safe node aliases (node_0, node_1, ...) instead of raw executor IDs as Mermaid node identifiers. Raw IDs containing spaces, dots, or non-ASCII characters (e.g. Japanese) caused Mermaid parse errors. 2. Fix conditional edge arrow syntax from '.--> ' (invalid) to '.-> ' (valid Mermaid dotted arrow syntax). Fixes #1406 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Use recognizable sanitized IDs for Mermaid node identifiers\n\nReplace generic node_0/node_1 aliases with IDs derived from the original\nexecutor names. ASCII letters, digits, and underscores are preserved;\nother characters become underscores (collapsed, trimmed). Leading digits\nget an n_ prefix. Collisions are resolved with a numeric suffix.\n\nThis keeps node IDs readable in the Mermaid source while the display\nlabels continue to show the full original names." * Remove issue number references from test names and comments" * Address PR review feedback from Copilot\n\n- Add Throw.IfNull(id) guard to SanitizeMermaidNodeId\n- Add safety limit (10,000) to collision resolution loop\n- Restore missing edge assertions (middle1/middle2 --> end)\n- Fix comment to show actual sanitized ID (n_1_User_input)\n- Use stricter regex in Unicode test (must start with letter/underscore)" * Address second round of PR review feedback\n\n- Escape node display labels via EscapeMermaidLabel to handle quotes,\n brackets, and newlines in executor IDs\n- Fix XML doc on SanitizeMermaidNodeId to accurately describe that\n existing consecutive underscores in input are preserved\n- Restore specific edge assertion (mid --> end) in conditional edge test\n- Restore fan-in routing assertions (s1/s2 through intermediate node,\n no direct edges to t) in fan-in test" --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
425f27f989 |
.NET: Fixing issue where OpenTelemetry span is never exported in .NET in-process workflow execution (#4196)
* 1. Add reproduction test for issue #4155: workflow.run Activity never stopped in streaming OffThread path The WorkflowRunActivity_IsStopped_Streaming_OffThread test demonstrates that the workflow.run OpenTelemetry Activity created in StreamingRunEventStream.RunLoopAsync is started but never stopped when using the OffThread/Default streaming execution. The background run loop keeps running after event consumption completes, so the using Activity? declaration never disposes until explicit StopAsync() is called. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> 2. Fix workflow.run Activity never stopped in streaming OffThread execution (#4155) The workflow.run OpenTelemetry Activity in StreamingRunEventStream.RunLoopAsync was scoped to the method lifetime via 'using'. Since the run loop only exits on cancellation, the Activity was never stopped/exported until explicit disposal. Fix: Remove 'using' and explicitly dispose the Activity when the workflow reaches Idle status (all supersteps complete). A safety-net disposal in the finally block handles cancellation and error paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add root-level workflow.session activity spanning run loop lifetime\n\nImplements two-level telemetry hierarchy per PR feedback from lokitoth:\n- workflow.session: spans the entire run loop / stream lifetime\n- workflow_invoke: per input-to-halt cycle, nested within the session\n\nThis ensures the session activity stays open across multiple turns,\nwhile individual run activities are created and disposed per cycle.\n\nAlso fixes linkedSource CancellationTokenSource disposal leak in\nStreamingRunEventStream (added using declaration)." * Address Copilot review: fix Activity/CTS disposal, rename activity, add error tag\n\n1. LockstepRunEventStream: Remove 'using' from Activity in async iterator\n and manually dispose in finally block (fixes #4155 pattern). Also dispose\n linkedSource CTS in finally to prevent leak.\n2. Tags.cs: Add ErrorMessage (\"error.message\") tag for runtime errors,\n distinct from BuildErrorMessage (\"build.error.message\").\n3. ActivityNames: Rename WorkflowRun from \"workflow_invoke\" to \"workflow.run\"\n for cross-language consistency.\n4. WorkflowTelemetryContext: Fix XML doc to say \"outer/parent span\" instead\n of \"root-level span\".\n5. ObservabilityTests: Assert WorkflowSession absence when DisableWorkflowRun\n is true.\n6. WorkflowRunActivityStopTests: Fix streaming test race by disposing\n StreamingRun before asserting activities are stopped.\n7. StreamingRunEventStream/LockstepRunEventStream: Use Tags.ErrorMessage\n instead of Tags.BuildErrorMessage for runtime error events." * Review fixes: revert workflow_invoke rename, use 'using' for linkedSource, move SessionStarted earlier\n\n- Revert ActivityNames.WorkflowRun back to \"workflow_invoke\" (OTEL semantic convention contract)\n- Use 'using' declaration for linkedSource CTS in LockstepRunEventStream (no timing sensitivity)\n- Move SessionStarted event before WaitForInputAsync in StreamingRunEventStream to match Lockstep behavior" * Improve naming and comments in WorkflowRunActivityStopTests" * Prevent session Activity.Current leak in lockstep mode, add nesting test Save and restore Activity.Current in LockstepRunEventStream.Start() so the session activity doesn't leak into caller code via AsyncLocal. Re-establish Activity.Current = sessionActivity before creating the run activity in TakeEventStreamAsync to preserve parent-child nesting. Add test verifying app activities after RunAsync are not parented under the session, and that the workflow_invoke activity nests under the session." * Fix stale XML doc: WorkflowRun -> WorkflowInvoke in ObservabilityTests --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
23fe2c16b3 |
Python: Fixing issue #1366 - Thread corruption when max_iterations is reached. (#4234)
* Fix thread corruption when max_iterations exhausted (#1366) When the function invocation loop exhausts max_iterations while the model keeps requesting tools, the failsafe code path (calling the model with tool_choice='none' and prepending fcc_messages) was unreachable because 'if response is not None: return response' short-circuited before it. The fix removes the premature return so the failsafe always runs after loop exhaustion, making a final model call with tool_choice='none' to produce a clean text answer and prepending accumulated fcc_messages from prior iterations. This matches the existing pattern used by the error threshold and max_function_calls paths. Also unskips test_max_iterations_limit and test_streaming_max_iterations_limit which were previously skipped with 'needs investigation in unified API'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add fix report for issue #1366 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix ruff formatting in _tools.py and test_issue_1366_thread_corruption.py Apply ruff format to fix multi-line string concatenation and function call formatting issues flagged by the linter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add quality review for issue #1366 fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove temporary investigation docs. * Address PR review: explicit enabled check in log condition, clarify mock behavior in test - Add explicit function_invocation_configuration['enabled'] check to the 'Maximum iterations reached' log condition in both non-streaming and streaming paths, making intent clearer when function invocation is disabled. - Add comment in test_thread_safe_after_max_iterations_with_agent explaining that the failsafe response (tool_choice='none') is provided automatically by the mock client, not from run_responses. * Blend fix and tests into project without issue-specific callouts - Remove issue #1366 references from _tools.py comments - Move regression tests from standalone test_issue_1366_thread_corruption.py into test_function_invocation_logic.py alongside existing max_iterations tests - Clean up test docstrings to describe behavior generically - Delete the standalone issue-specific test file --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
c609b14f63 |
Python: Fix GroupChat orchestrator message cleanup issue (#3712)
* Fix GroupChat orchestrator message cleanup issue Apply clean_conversation_for_handoff to GroupChatOrchestrator and AgentBasedGroupChatOrchestrator _handle_response methods to remove tool-related content that causes API errors from empty messages. Fixes #3705 * Move orchestration related files to orchestrations package. * Fix imports --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Evan Mattson <evan.mattson@microsoft.com> |
||
 
|
907654a489 |
[BREAKING] Obsoleting ReflectingExecutor in favor of source gen (#3380)
* Initial working version with tests.
* Updates to validate class data once instead of for each handler method. Also updated Diagnostics Ids to format of MAFGENWF{NUM}
* Formatting and trying to fix generation project pack.
* Another atempt at getting the genrators project to build.
* More attempts to fix generator build and pack.
* Fixing file encodings.
* Initail round of cleanup.
* Trying to fix packing.
* Still trying to fix pipeline pack.
* Remove obsolescence markers, sample updates, and docs from generator branch.
This commit separates the generator core functionality from the
deprecation of ReflectingExecutor. The removed changes will be
re-added in a dependent branch (wf-obsolete-reflector).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Mark ReflectingExecutor and IMessageHandler as obsolete.
This commit deprecates the reflection-based handler discovery approach
in favor of the new [MessageHandler] attribute with source generation.
Changes:
- Add [Obsolete] to ReflectingExecutor<T>, IMessageHandler<T>, IMessageHandler<T,R>
- Add #pragma to suppress warnings in internal reflection code
- Update Concurrent sample to use new [MessageHandler] pattern
- Add Directory.Build.props for samples to include generator
- Add documentation files explaining the migration
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Obsoleteing Reflector-based workflow code generation in favor of Source Generators and updating some samples to use new pattern.
This commit deprecates the reflection-based handler discovery approach
in favor of the new [MessageHandler] attribute with source generation.
Changes:
- Add [Obsolete] to ReflectingExecutor<T>, IMessageHandler<T>, IMessageHandler<T,R>
- Add #pragma to suppress warnings in internal reflection code
- Update Concurrent sample to use new [MessageHandler] pattern
- Add Directory.Build.props for samples to include generator
- Add documentation files explaining the migration
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Cleaning up temporary design and progress files.
---------
Co-authored-by: alliscode <bentho@microsoft.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Chris <66376200+crickman@users.noreply.github.com>
|
||
|
|
ea7818d390 |
Python: .NET: Executor source gen for workflow executor routing (#3131)
* Roslyn Source Generators for Workflow Executor Routing. * Update dotnet/src/Microsoft.Agents.AI.Workflows.Generators/ExecutorRouteGenerator.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * WIP. * All fixed up except dangling sends/yields attriutes, working on that next. * Add protocol-only generation for SendsMessage/YieldsOutput attributes * Ensuring collections that can change order are sorted to enable pipeline caching. * Improvents per PR feedback. --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
|
a4039134de |
.NET: Remove reflection samples (#1460)
* Removing usage of ReflectingExecutor<T> from workflow samples. * Removing uneeded changes. * Clean up. * Update dotnet/samples/GettingStarted/Workflows/_Foundational/01_ExecutorsAndEdges/Program.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update dotnet/samples/GettingStarted/Workflows/_Foundational/01_ExecutorsAndEdges/Program.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update dotnet/samples/GettingStarted/Workflows/_Foundational/02_Streaming/Program.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update dotnet/samples/GettingStarted/Workflows/_Foundational/02_Streaming/Program.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Updates per PR feedback. * Undo changes to generated file. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
64b57639b8 |
.NET: Mitigation for issue 1315 (#1432)
* Adding mitigation for issue 1315, will follow up with long-term plan. * Apply suggestion from @lokitoth Co-authored-by: Jacob Alber <jaalber@microsoft.com> * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Extracting common updates to a helper method. * Fixing spelling mistake. * Update per PR feedback. --------- Co-authored-by: Jacob Alber <jaalber@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
|
54b35e76ed |
Python: Updating python DEV_SETUP to add brew-based uv installation. (#1173)
* Updating python DEV_SETUP to add brew-based uv installation. * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Ben ThomasandGitHub
|
df3e12b2ed |
.Net: Visualizing dotnet workflows (#882)
* Adding more test for workflow vizualization. * Improving README for sample. * More cleanup. * Removing rendering of workflow visualization and adding basic support for mermaid format. * Adding basic mermaid tests. * Improving sample. Switching to another branch. * About to merge from main. * Formatting. * More fromatting. * Removng uneeded call to ToUpper. * Adding README. * Moving samples under workflow to workflows. * Removing uneeded README for map-reduce sample. |
||
|
Ben ThomasandGitHub
|
647db9635a |
.NET: Rename workflows projects (#975)
* Renaming Microsoft.Agent.Workflows to Microsoft.Agents.AI.Workflows * Removing local settings. * Removing remining old files from merge. |
||
|
Ben ThomasandGitHub
|
e1e5100afe | Removing fallback to DefaultAzureCredential outside of testing. (#755) | ||
|
Ben ThomasandGitHub
|
c7a1a1e991 | Fix link to Declarative Workflow demo (#674) | ||
|
Ben ThomasandGitHub
|
b4d8ad1bd2 |
Adding workflows label to PRs when appropriate. (#643)
* Adding workflows label to issues when appropriate. * Adding declarative workflows path. |
||
 
|
32c26c3c25 |
Adding draft version of workflows design doc. (#271)
* Adding draft version of workflows design doc. * Adding message flow diagrams and fixing group chat sample. * Added sequence diagram for fan_in with WhenAny * Updating workflows design * clean up * Clean up request & response contents * Add more future enhacements * message_handler -> handler * remove mention of visual designer * Minor updates and rename * Name reformat --------- Co-authored-by: Chris <66376200+crickman@users.noreply.github.com> Co-authored-by: Tao Chen <taochen@microsoft.com> Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com> |
||
|
|
b0b3fd151c |
Python: Adding support for nested workflows (#460)
* Adding design documents and data flow descriptions for sub-workflows * Updating docs. * Sub-workflow implementation #1. Stuck because of singleton RequestInfoExecutor, going to make a change to remove that restrivtion. * Removed the singleton restriction on RequestInfoExecutor so enable sub-workflows. * Scenarios seem to be working. * Sample improved. * going to have intern add generic response wrappers. * Wrapped responses working. * Non-hardcoded routing is working. * Sample showing external approved and not approved. * Cleaning up. * Updating some samples and user guide. * Removing old design doc. * Cleaning up. * Adding python-package-setup.md back. * Update python/packages/workflow/agent_framework_workflow/_executor.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update python/packages/workflow/agent_framework_workflow/_validation.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Removing prints. * Fixing lint and type issues. * Fixing lint and type issues. * Update python/packages/workflow/agent_framework_workflow/_executor.py Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com> * Adding type hints to intercepts decorator. * Removing unused files. * Fixing issue with sample 5 groupchat with hil. * Removing redundent samples. * Updates to ensure no conflicting request interceptors and to support a subflow with multiple requests in a single super step. * Fixing pypi errors. * clean up samples * update samples to make it more clear * warning for unhandled request info from sub workflow * add logger info --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com> |