diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index 5896f72141..0ae0c73032 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -66,23 +66,45 @@ def _assemble_instructions( def _assemble_compaction_provider( *, disable_compaction: bool, - max_context_window_tokens: int, - max_output_tokens: int, + max_context_window_tokens: int | None, + max_output_tokens: int | None, history_source_id: str, before_compaction_strategy: CompactionStrategy | None, after_compaction_strategy: CompactionStrategy | None, tokenizer: TokenizerProtocol | None, ) -> CompactionProvider | None: - """Build the compaction provider from parameters or defaults.""" + """Build the compaction provider from parameters or defaults. + + The token-budget defaults (``ContextWindowCompactionStrategy`` for the before phase and + ``ToolResultCompactionStrategy`` for the after phase) are only applied when the token + params are provided. Caller-supplied strategies are always honored. Either phase may end + up ``None``, which ``CompactionProvider`` interprets as "skip that phase". + + Returns None when compaction is explicitly disabled, or when neither phase has a strategy + (no custom strategies and no token budget to build the defaults). + """ if disable_compaction: return None - before_strategy = before_compaction_strategy or ContextWindowCompactionStrategy( - max_context_window_tokens=max_context_window_tokens, - max_output_tokens=max_output_tokens, - tokenizer=tokenizer, - ) - after_strategy = after_compaction_strategy or ToolResultCompactionStrategy(keep_last_tool_call_groups=2) + # Resolve the before-strategy: custom strategy wins; otherwise fall back to the + # token-budget-aware default when token params are available. + before_strategy = before_compaction_strategy + if before_strategy is None and max_context_window_tokens is not None and max_output_tokens is not None: + before_strategy = ContextWindowCompactionStrategy( + max_context_window_tokens=max_context_window_tokens, + max_output_tokens=max_output_tokens, + tokenizer=tokenizer, + ) + + # Resolve the after-strategy: custom strategy wins; otherwise fall back to the default + # when token params are available. + after_strategy = after_compaction_strategy + if after_strategy is None and max_context_window_tokens is not None and max_output_tokens is not None: + after_strategy = ToolResultCompactionStrategy(keep_last_tool_call_groups=2) + + # Nothing to compact in either phase: skip the provider entirely. + if before_strategy is None and after_strategy is None: + return None return CompactionProvider( before_strategy=before_strategy, @@ -157,8 +179,8 @@ def create_harness_agent( harness_instructions: str | None = None, agent_instructions: str | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, - max_context_window_tokens: int, - max_output_tokens: int, + max_context_window_tokens: int | None = None, + max_output_tokens: int | None = None, history_provider: HistoryProvider | None = None, disable_compaction: bool = False, before_compaction_strategy: CompactionStrategy | None = None, @@ -206,8 +228,6 @@ def create_harness_agent( agent = create_harness_agent( OpenAIChatClient(model="gpt-4o"), - max_context_window_tokens=128_000, - max_output_tokens=16_384, ) session = agent.create_session() response = await agent.run("Plan a weekend trip to Seattle", session=session) @@ -243,13 +263,21 @@ def create_harness_agent( (e.g., "You are a research assistant focused on academic sources."). tools: Additional tools to include in the agent's toolset. max_context_window_tokens: Maximum tokens the model's context window supports. + Used to construct the default token-budget-aware compaction strategies. When None + (default) and no custom ``before_compaction_strategy`` / ``after_compaction_strategy`` + is provided, compaction is automatically disabled. max_output_tokens: Maximum output tokens per response. + Used to construct the default compaction strategies and sets a default max_tokens + chat option. When None (default), no default max_tokens option is set, and unless a + custom compaction strategy is provided, compaction is automatically disabled. history_provider: Custom history provider. When None, an InMemoryHistoryProvider is used. disable_compaction: When True, skip compaction provider setup. - before_compaction_strategy: Custom before-run compaction strategy. - Defaults to ContextWindowCompactionStrategy (token-budget aware). - after_compaction_strategy: Custom after-run compaction strategy. - Defaults to ToolResultCompactionStrategy. + before_compaction_strategy: Custom before-run compaction strategy. When provided, + compaction runs even if token params are omitted. Defaults to + ContextWindowCompactionStrategy (token-budget aware) when token params are provided. + after_compaction_strategy: Custom after-run compaction strategy. When provided, + compaction runs even if token params are omitted. Defaults to + ToolResultCompactionStrategy when token params are provided. tokenizer: Custom tokenizer for compaction strategies. disable_todo: When True, skip the TodoProvider. todo_provider: Custom TodoProvider instance. Ignored when disable_todo is True. @@ -283,14 +311,19 @@ def create_harness_agent( A fully configured :class:`~agent_framework.Agent` instance. Raises: - ValueError: If max_context_window_tokens <= 0 or max_output_tokens < 0 - or max_output_tokens >= max_context_window_tokens. + ValueError: If max_context_window_tokens is provided and <= 0, or + max_output_tokens is provided and <= 0, or max_output_tokens >= + max_context_window_tokens when both are provided. """ - if max_context_window_tokens <= 0: + if max_context_window_tokens is not None and max_context_window_tokens <= 0: raise ValueError("max_context_window_tokens must be positive.") - if max_output_tokens < 0: - raise ValueError("max_output_tokens must be non-negative.") - if max_output_tokens >= max_context_window_tokens: + if max_output_tokens is not None and max_output_tokens <= 0: + raise ValueError("max_output_tokens must be positive.") + if ( + max_context_window_tokens is not None + and max_output_tokens is not None + and max_output_tokens >= max_context_window_tokens + ): raise ValueError("max_output_tokens must be less than max_context_window_tokens.") # Build history provider. @@ -347,7 +380,8 @@ def create_harness_agent( # Build default options dict. default_opts: dict[str, Any] = dict(default_options) if default_options else {} - default_opts.setdefault("max_tokens", max_output_tokens) + if max_output_tokens is not None: + default_opts.setdefault("max_tokens", max_output_tokens) agent = Agent( client, diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 97afe66cea..91bdb61914 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -3516,9 +3516,7 @@ async def get_content(self) -> str: result = await self._client.read_resource(_mcp_any_url(self._skill_md_uri)) text = _mcp_join_text(result) if not text: - raise ValueError( - f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'." - ) + raise ValueError(f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'.") self._content = text return text @@ -3572,11 +3570,7 @@ def _validate_resource_name(name: str) -> str | None: or ``None`` if the name is unsafe. """ normalized = name.replace("\\", "/") - if ( - normalized.startswith("/") - or "://" in normalized - or any(seg == ".." for seg in normalized.split("/")) - ): + if normalized.startswith("/") or "://" in normalized or any(seg == ".." for seg in normalized.split("/")): logger.debug("Rejecting resource name with unsafe path components: %r", name) return None return normalized diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index 58ef3f5f2d..7da1bdbf36 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -194,6 +194,63 @@ def test_create_harness_agent_returns_full_agent() -> None: assert isinstance(agent, FullAgent) +def test_create_harness_agent_no_token_params_disables_compaction() -> None: + """When token params are omitted, compaction is automatically disabled.""" + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + ) + provider_types = [type(p) for p in agent.context_providers] + assert CompactionProvider not in provider_types + + +def test_create_harness_agent_no_token_params_skips_max_tokens_option() -> None: + """When max_output_tokens is omitted, max_tokens should not be set in default options.""" + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + ) + assert agent.default_options.get("max_tokens") is None + + +def test_create_harness_agent_custom_before_strategy_enables_compaction_without_tokens() -> None: + """A custom before_compaction_strategy enables compaction even when token params are omitted.""" + from agent_framework import ToolResultCompactionStrategy + + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + before_compaction_strategy=ToolResultCompactionStrategy(), + ) + provider_types = [type(p) for p in agent.context_providers] + assert CompactionProvider in provider_types + + +def test_create_harness_agent_disable_compaction_overrides_custom_before_strategy() -> None: + """disable_compaction=True wins even when a custom before strategy is provided.""" + from agent_framework import ToolResultCompactionStrategy + + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + before_compaction_strategy=ToolResultCompactionStrategy(), + disable_compaction=True, + ) + provider_types = [type(p) for p in agent.context_providers] + assert CompactionProvider not in provider_types + + +def test_create_harness_agent_custom_after_strategy_enables_compaction_without_tokens() -> None: + """A custom after_compaction_strategy enables compaction even when token params are omitted.""" + from agent_framework import ToolResultCompactionStrategy + + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + after_compaction_strategy=ToolResultCompactionStrategy(), + ) + compaction_providers = [p for p in agent.context_providers if isinstance(p, CompactionProvider)] + assert len(compaction_providers) == 1 + # Before phase is skipped (no token budget, no custom before strategy), after phase is set. + assert compaction_providers[0].before_strategy is None + assert compaction_providers[0].after_strategy is not None + + # --- Validation Tests --- @@ -207,14 +264,15 @@ def test_create_harness_agent_rejects_invalid_context_tokens() -> None: ) -def test_create_harness_agent_rejects_negative_output_tokens() -> None: - """max_output_tokens must be non-negative.""" - with pytest.raises(ValueError, match="max_output_tokens must be non-negative"): - create_harness_agent( - client=_FakeChatClient(), # type: ignore[arg-type] - max_context_window_tokens=1000, - max_output_tokens=-1, - ) +def test_create_harness_agent_rejects_non_positive_output_tokens() -> None: + """max_output_tokens must be positive when provided.""" + for invalid_value in (0, -1): + with pytest.raises(ValueError, match="max_output_tokens must be positive"): + create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + max_context_window_tokens=1000, + max_output_tokens=invalid_value, + ) def test_create_harness_agent_rejects_output_gte_context() -> None: diff --git a/python/packages/core/tests/core/test_mcp_observability.py b/python/packages/core/tests/core/test_mcp_observability.py index 226e976120..8b17f3dc88 100644 --- a/python/packages/core/tests/core/test_mcp_observability.py +++ b/python/packages/core/tests/core/test_mcp_observability.py @@ -281,9 +281,7 @@ async def test_mcp_prompts_get_creates_client_span(span_exporter: InMemorySpanEx async def test_mcp_prompts_get_mcp_error_sets_error_type(span_exporter: InMemorySpanExporter): """When session.get_prompt() raises McpError, the span should have error.type and ERROR status.""" tool = _make_connected_mcp_tool() - tool.session.get_prompt = AsyncMock( - side_effect=McpError(ErrorData(code=-32602, message="prompt not found")) - ) + tool.session.get_prompt = AsyncMock(side_effect=McpError(ErrorData(code=-32602, message="prompt not found"))) span_exporter.clear() with pytest.raises(ToolExecutionException): diff --git a/python/packages/core/tests/core/test_mcp_skills.py b/python/packages/core/tests/core/test_mcp_skills.py index 3e7c67662a..74993997d0 100644 --- a/python/packages/core/tests/core/test_mcp_skills.py +++ b/python/packages/core/tests/core/test_mcp_skills.py @@ -35,26 +35,22 @@ Body content here. """ -SAMPLE_SKILL_INDEX = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "unit-converter", - "type": "skill-md", - "description": "Convert between common units.", - "url": "skill://unit-converter/SKILL.md", - } - ], - } -) +SAMPLE_SKILL_INDEX = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "unit-converter", + "type": "skill-md", + "description": "Convert between common units.", + "url": "skill://unit-converter/SKILL.md", + } + ], +}) def _make_text_result(text: str, uri: str = "skill://test") -> ReadResourceResult: """Create a ReadResourceResult with a single TextResourceContents.""" - return ReadResourceResult( - contents=[TextResourceContents(uri=AnyUrl(uri), text=text, mimeType="text/markdown")] - ) + return ReadResourceResult(contents=[TextResourceContents(uri=AnyUrl(uri), text=text, mimeType="text/markdown")]) def _make_blob_result( @@ -230,12 +226,10 @@ async def test_get_content_raises_on_empty(self) -> None: @pytest.mark.asyncio async def test_get_resource_text(self) -> None: - client = _make_client( - **{ - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), - } - ) + client = _make_client(**{ + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), + }) from agent_framework import SkillFrontmatter fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.") @@ -249,12 +243,10 @@ async def test_get_resource_text(self) -> None: @pytest.mark.asyncio async def test_get_resource_binary(self) -> None: data = bytes([0x01, 0x02, 0x03, 0x04]) - client = _make_client( - **{ - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/assets/icon.bin": _make_blob_result(data), - } - ) + client = _make_client(**{ + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/assets/icon.bin": _make_blob_result(data), + }) from agent_framework import SkillFrontmatter fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.") @@ -345,12 +337,10 @@ class TestMCPSkillsSource: @pytest.mark.asyncio async def test_index_based_discovery_returns_skill(self) -> None: - client = _make_client( - **{ - "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - } - ) + client = _make_client(**{ + "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + }) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -373,9 +363,7 @@ async def test_no_index_returns_empty(self) -> None: async def test_does_not_read_skill_md_during_discovery(self) -> None: # Index points to a skill, but SKILL.md is not registered on the server. # Discovery should succeed because it only reads the index. - client = _make_client( - **{"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json")} - ) + client = _make_client(**{"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -384,19 +372,17 @@ async def test_does_not_read_skill_md_during_discovery(self) -> None: @pytest.mark.asyncio async def test_invalid_name_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "UnitConverter", # Invalid: uppercase - "type": "skill-md", - "description": "Convert between common units.", - "url": "skill://UnitConverter/SKILL.md", - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "UnitConverter", # Invalid: uppercase + "type": "skill-md", + "description": "Convert between common units.", + "url": "skill://UnitConverter/SKILL.md", + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -404,18 +390,16 @@ async def test_invalid_name_is_skipped(self) -> None: @pytest.mark.asyncio async def test_missing_required_fields_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "unit-converter", - "type": "skill-md", - # Missing description and url - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "unit-converter", + "type": "skill-md", + # Missing description and url + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -423,19 +407,17 @@ async def test_missing_required_fields_is_skipped(self) -> None: @pytest.mark.asyncio async def test_unsupported_type_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "some-skill", - "type": "archive", - "description": "Packaged skill.", - "url": "skill://some-skill.tar.gz", - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "some-skill", + "type": "archive", + "description": "Packaged skill.", + "url": "skill://some-skill.tar.gz", + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -443,18 +425,16 @@ async def test_unsupported_type_is_skipped(self) -> None: @pytest.mark.asyncio async def test_template_type_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "type": "mcp-resource-template", - "description": "Per-product documentation skill", - "url": "skill://docs/{product}/SKILL.md", - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "type": "mcp-resource-template", + "description": "Per-product documentation skill", + "url": "skill://docs/{product}/SKILL.md", + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -462,31 +442,25 @@ async def test_template_type_is_skipped(self) -> None: @pytest.mark.asyncio async def test_empty_index_returns_empty(self) -> None: - client = _make_client( - **{"skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json")} - ) + client = _make_client(**{"skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() assert skills == [] @pytest.mark.asyncio async def test_malformed_index_json_returns_empty(self) -> None: - client = _make_client( - **{"skill://index.json": _make_text_result("not valid json", uri="skill://index.json")} - ) + client = _make_client(**{"skill://index.json": _make_text_result("not valid json", uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() assert skills == [] @pytest.mark.asyncio async def test_sibling_text_resource(self) -> None: - client = _make_client( - **{ - "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), - } - ) + client = _make_client(**{ + "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), + }) source = MCPSkillsSource(client=client) skill = (await source.get_skills())[0] resource = await skill.get_resource("references/checklist.md") @@ -497,13 +471,11 @@ async def test_sibling_text_resource(self) -> None: @pytest.mark.asyncio async def test_sibling_binary_resource(self) -> None: data = bytes([0x01, 0x02, 0x03, 0x04]) - client = _make_client( - **{ - "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/assets/icon.bin": _make_blob_result(data), - } - ) + client = _make_client(**{ + "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/assets/icon.bin": _make_blob_result(data), + }) source = MCPSkillsSource(client=client) skill = (await source.get_skills())[0] resource = await skill.get_resource("assets/icon.bin") @@ -649,9 +621,7 @@ async def test_get_resource_generic_mcp_error_propagates(self) -> None: from agent_framework import SkillFrontmatter client = AsyncMock() - client.read_resource = AsyncMock( - side_effect=McpError(error=ErrorData(code=0, message="Handler error")) - ) + client.read_resource = AsyncMock(side_effect=McpError(error=ErrorData(code=0, message="Handler error"))) fm = SkillFrontmatter(name="test-skill", description="Test.") skill = MCPSkill(frontmatter=fm, skill_md_uri="skill://test/SKILL.md", client=client) with pytest.raises(McpError): diff --git a/python/samples/02-agents/harness/README.md b/python/samples/02-agents/harness/README.md index 3bf0f09110..15424e1422 100644 --- a/python/samples/02-agents/harness/README.md +++ b/python/samples/02-agents/harness/README.md @@ -45,13 +45,23 @@ python samples/02-agents/harness/harness_research.py ### Minimal Setup -`create_harness_agent` requires only a chat client and token budget parameters: +`create_harness_agent` requires only a chat client: ```python from agent_framework import create_harness_agent from agent_framework.foundry import FoundryChatClient from azure.identity import AzureCliCredential +agent = create_harness_agent( + client=FoundryChatClient(credential=AzureCliCredential()), +) +``` + +### With Compaction + +Provide token budget parameters to enable automatic context-window compaction: + +```python agent = create_harness_agent( client=FoundryChatClient(credential=AzureCliCredential()), max_context_window_tokens=128_000, @@ -59,7 +69,7 @@ agent = create_harness_agent( ) ``` -### Customization +### Further Customization Disable or customize any feature: