From 08e8797c44b146ba23a9239fca082ce5822e6648 Mon Sep 17 00:00:00 2001 From: Jason Robert Date: Mon, 9 Mar 2026 09:59:28 -0400 Subject: [PATCH] feat(config): expose max_session_seconds in workflow YAML Allow workflow authors to tune the Copilot SDK session wall-clock timeout per-workflow (runtime.max_session_seconds) and per-agent (agent.max_session_seconds), instead of being locked to the hardcoded 1800s default. Per-agent values override the workflow-level default at call time, following the same pattern as agent.model. Closes #28 Co-Authored-By: Claude Opus 4.6 --- src/conductor/config/schema.py | 22 +++++ src/conductor/providers/copilot.py | 21 +++- src/conductor/providers/factory.py | 13 ++- src/conductor/providers/registry.py | 1 + tests/test_config/test_schema.py | 107 ++++++++++++++++++++ tests/test_providers/test_factory.py | 32 ++++++ tests/test_providers/test_idle_recovery.py | 108 +++++++++++++++++++++ tests/test_providers/test_registry.py | 1 + 8 files changed, 302 insertions(+), 3 deletions(-) diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index e906a9a..e56fe92 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -424,6 +424,16 @@ class AgentDef(BaseModel): timeout: int | None = None """Per-script timeout in seconds.""" + max_session_seconds: float | None = Field(None, ge=1.0) + """Maximum wall-clock duration for this agent's Copilot SDK session in seconds. + + Overrides the workflow-level runtime.max_session_seconds for this agent. + Only applies to Copilot provider agents (not script or human_gate). + + Example: A source-gathering agent that should finish in ~60s can set + max_session_seconds: 60 instead of using the default 30-minute timeout. + """ + @field_validator("timeout") @classmethod def validate_timeout(cls, v: int | None) -> int | None: @@ -460,6 +470,8 @@ def validate_agent_type(self) -> AgentDef: raise ValueError("script agents cannot have 'system_prompt'") if self.options: raise ValueError("script agents cannot have 'options'") + if self.max_session_seconds: + raise ValueError("script agents cannot have 'max_session_seconds'") return self @@ -565,6 +577,16 @@ class RuntimeConfig(BaseModel): which limits the total wall-clock time for the entire workflow. """ + max_session_seconds: float | None = Field(None, ge=1.0) + """Maximum wall-clock duration for Copilot SDK sessions in seconds. + + Sets the default max_session_seconds for all agents using the Copilot provider. + Individual agents can override this with their own max_session_seconds field. + + Default is None, which uses the Copilot provider's built-in default (1800s / 30 min). + Set a lower value for workflows where agents should finish quickly. + """ + class WorkflowDef(BaseModel): """Top-level workflow configuration.""" diff --git a/src/conductor/providers/copilot.py b/src/conductor/providers/copilot.py index d09a4df..8af1f7b 100644 --- a/src/conductor/providers/copilot.py +++ b/src/conductor/providers/copilot.py @@ -492,6 +492,11 @@ async def _execute_sdk_call( verbose_enabled = is_verbose() full_enabled = is_full() + # Resolve per-agent max_session_seconds override + effective_max_session = ( + agent.max_session_seconds or self._idle_recovery_config.max_session_seconds + ) + session_destroyed = False try: # Send initial prompt and get response @@ -502,6 +507,7 @@ async def _execute_sdk_call( full_enabled, interrupt_signal=interrupt_signal, event_callback=event_callback, + max_session_seconds=effective_max_session, ) response_content = sdk_response.content @@ -628,6 +634,7 @@ async def _send_and_wait( full_enabled: bool, interrupt_signal: asyncio.Event | None = None, event_callback: EventCallback | None = None, + max_session_seconds: float | None = None, ) -> SDKResponse: """Send a prompt to the session and wait for response. @@ -640,6 +647,8 @@ async def _send_and_wait( When set, the method will attempt to abort the session and return partial content with ``partial=True``. event_callback: Optional callback for streaming SDK events upstream. + max_session_seconds: Per-agent wall-clock session limit override. + If None, uses the provider-level IdleRecoveryConfig default. Returns: SDKResponse with content and usage data. If interrupted, @@ -745,7 +754,12 @@ def on_event(event: Any) -> None: else: # Wait with idle detection and recovery (original path) await self._wait_with_idle_detection( - done, session, verbose_enabled, full_enabled, last_activity_ref + done, + session, + verbose_enabled, + full_enabled, + last_activity_ref, + max_session_seconds=max_session_seconds, ) if error_message: @@ -1301,6 +1315,7 @@ async def _wait_with_idle_detection( verbose_enabled: bool, full_enabled: bool, last_activity_ref: list[Any], + max_session_seconds: float | None = None, ) -> None: """Wait for session completion with idle detection and recovery. @@ -1316,6 +1331,8 @@ async def _wait_with_idle_detection( full_enabled: Whether full logging mode is enabled. last_activity_ref: Mutable [last_event_type, last_tool_call, timestamp] for tracking last activity. + max_session_seconds: Per-agent wall-clock session limit override. + If None, uses the provider-level IdleRecoveryConfig default. Raises: ProviderError: If all recovery attempts are exhausted, or if the @@ -1324,7 +1341,7 @@ async def _wait_with_idle_detection( recovery_attempts = 0 idle_timeout = self._idle_recovery_config.idle_timeout_seconds session_start = time.monotonic() - max_session = self._idle_recovery_config.max_session_seconds + max_session = max_session_seconds or self._idle_recovery_config.max_session_seconds while True: # Check if done was already set (avoids race where session.idle diff --git a/src/conductor/providers/factory.py b/src/conductor/providers/factory.py index 0a6d4e7..26c29be 100644 --- a/src/conductor/providers/factory.py +++ b/src/conductor/providers/factory.py @@ -11,7 +11,7 @@ from conductor.exceptions import ProviderError from conductor.providers.base import AgentProvider from conductor.providers.claude import ANTHROPIC_SDK_AVAILABLE, ClaudeProvider -from conductor.providers.copilot import CopilotProvider +from conductor.providers.copilot import CopilotProvider, IdleRecoveryConfig async def create_provider( @@ -22,6 +22,7 @@ async def create_provider( temperature: float | None = None, max_tokens: int | None = None, timeout: float | None = None, + max_session_seconds: float | None = None, ) -> AgentProvider: """Factory function to create the appropriate provider. @@ -40,6 +41,8 @@ async def create_provider( temperature: Default temperature for generation (0.0-1.0). max_tokens: Maximum output tokens. timeout: Request timeout in seconds. + max_session_seconds: Maximum wall-clock duration for Copilot SDK sessions. + Only applies to the Copilot provider. Returns: Configured AgentProvider instance. @@ -54,10 +57,16 @@ async def create_provider( """ match provider_type: case "copilot": + idle_recovery_config = None + if max_session_seconds is not None: + idle_recovery_config = IdleRecoveryConfig( + max_session_seconds=max_session_seconds, + ) provider = CopilotProvider( mcp_servers=mcp_servers, model=default_model, temperature=temperature, + idle_recovery_config=idle_recovery_config, ) case "openai-agents": raise ProviderError( @@ -125,6 +134,7 @@ async def create_provider( temperature = getattr(runtime_config, "temperature", None) max_tokens = getattr(runtime_config, "max_tokens", None) timeout = getattr(runtime_config, "timeout", None) + max_session_seconds = getattr(runtime_config, "max_session_seconds", None) return await create_provider( provider_type=provider_type, @@ -133,4 +143,5 @@ async def create_provider( temperature=temperature, max_tokens=max_tokens, timeout=timeout, + max_session_seconds=max_session_seconds, ) diff --git a/src/conductor/providers/registry.py b/src/conductor/providers/registry.py index 8c20d55..b33894c 100644 --- a/src/conductor/providers/registry.py +++ b/src/conductor/providers/registry.py @@ -116,6 +116,7 @@ async def _get_or_create_provider(self, provider_type: ProviderType) -> AgentPro temperature=runtime.temperature, max_tokens=runtime.max_tokens, timeout=runtime.timeout, + max_session_seconds=runtime.max_session_seconds, ) # Pass stored resume session IDs to newly created providers diff --git a/tests/test_config/test_schema.py b/tests/test_config/test_schema.py index fba6926..dc8960e 100644 --- a/tests/test_config/test_schema.py +++ b/tests/test_config/test_schema.py @@ -310,6 +310,64 @@ def test_human_gate_without_prompt_raises(self) -> None: assert "prompt" in str(exc_info.value) +class TestAgentDefMaxSessionSeconds: + """Tests for max_session_seconds on AgentDef.""" + + def test_default_is_none(self) -> None: + """Test that max_session_seconds defaults to None.""" + agent = AgentDef(name="a", model="gpt-4", prompt="test") + assert agent.max_session_seconds is None + + def test_accepts_valid_value(self) -> None: + """Test that max_session_seconds accepts valid float values.""" + agent = AgentDef(name="a", model="gpt-4", prompt="test", max_session_seconds=60.0) + assert agent.max_session_seconds == 60.0 + + def test_accepts_integer_value(self) -> None: + """Test that max_session_seconds accepts integer values.""" + agent = AgentDef(name="a", model="gpt-4", prompt="test", max_session_seconds=120) + assert agent.max_session_seconds == 120.0 + + def test_minimum_boundary(self) -> None: + """Test that max_session_seconds accepts the minimum value of 1.0.""" + agent = AgentDef(name="a", model="gpt-4", prompt="test", max_session_seconds=1.0) + assert agent.max_session_seconds == 1.0 + + def test_rejects_zero(self) -> None: + """Test that max_session_seconds rejects zero.""" + with pytest.raises(ValidationError) as exc_info: + AgentDef(name="a", model="gpt-4", prompt="test", max_session_seconds=0) + assert "greater than or equal to 1" in str(exc_info.value) + + def test_rejects_negative(self) -> None: + """Test that max_session_seconds rejects negative values.""" + with pytest.raises(ValidationError) as exc_info: + AgentDef(name="a", model="gpt-4", prompt="test", max_session_seconds=-5.0) + assert "greater than or equal to 1" in str(exc_info.value) + + def test_rejected_on_script_agent(self) -> None: + """Test that script agents cannot have max_session_seconds.""" + with pytest.raises(ValidationError) as exc_info: + AgentDef( + name="s", + type="script", + command="echo hello", + max_session_seconds=60.0, + ) + assert "max_session_seconds" in str(exc_info.value) + + def test_allowed_on_regular_agent(self) -> None: + """Test that regular agents can have max_session_seconds.""" + agent = AgentDef( + name="a", + type="agent", + model="gpt-4", + prompt="test", + max_session_seconds=90.0, + ) + assert agent.max_session_seconds == 90.0 + + class TestRuntimeConfig: """Tests for RuntimeConfig model.""" @@ -472,6 +530,55 @@ def test_round_trip_serialization(self) -> None: assert restored.timeout == original.timeout +class TestRuntimeConfigMaxSessionSeconds: + """Tests for max_session_seconds on RuntimeConfig.""" + + def test_default_is_none(self) -> None: + """Test that max_session_seconds defaults to None.""" + config = RuntimeConfig() + assert config.max_session_seconds is None + + def test_accepts_valid_value(self) -> None: + """Test that max_session_seconds accepts valid float values.""" + config = RuntimeConfig(max_session_seconds=60.0) + assert config.max_session_seconds == 60.0 + + def test_accepts_integer_value(self) -> None: + """Test that max_session_seconds accepts integer values.""" + config = RuntimeConfig(max_session_seconds=120) + assert config.max_session_seconds == 120.0 + + def test_minimum_boundary(self) -> None: + """Test that max_session_seconds accepts the minimum value of 1.0.""" + config = RuntimeConfig(max_session_seconds=1.0) + assert config.max_session_seconds == 1.0 + + def test_rejects_zero(self) -> None: + """Test that max_session_seconds rejects zero.""" + with pytest.raises(ValidationError) as exc_info: + RuntimeConfig(max_session_seconds=0) + assert "greater than or equal to 1" in str(exc_info.value) + + def test_rejects_negative(self) -> None: + """Test that max_session_seconds rejects negative values.""" + with pytest.raises(ValidationError) as exc_info: + RuntimeConfig(max_session_seconds=-10.0) + assert "greater than or equal to 1" in str(exc_info.value) + + def test_serialization_excludes_when_none(self) -> None: + """Test that max_session_seconds is excluded from serialization when None.""" + config = RuntimeConfig() + serialized = config.model_dump(exclude_none=True) + assert "max_session_seconds" not in serialized + + def test_serialization_includes_when_set(self) -> None: + """Test that max_session_seconds is included in serialization when set.""" + config = RuntimeConfig(max_session_seconds=90.0) + serialized = config.model_dump(exclude_none=True) + assert "max_session_seconds" in serialized + assert serialized["max_session_seconds"] == 90.0 + + class TestWorkflowDef: """Tests for WorkflowDef model.""" diff --git a/tests/test_providers/test_factory.py b/tests/test_providers/test_factory.py index 60c8482..530ff0d 100644 --- a/tests/test_providers/test_factory.py +++ b/tests/test_providers/test_factory.py @@ -165,3 +165,35 @@ async def test_validation_can_be_skipped(self) -> None: provider = await create_provider("copilot", validate=False) assert isinstance(provider, CopilotProvider) await provider.close() + + +class TestMaxSessionSeconds: + """Tests for max_session_seconds parameter in create_provider.""" + + @pytest.mark.asyncio + async def test_max_session_seconds_flows_to_copilot_idle_recovery_config(self) -> None: + """Test that max_session_seconds is plumbed into CopilotProvider's IdleRecoveryConfig.""" + provider = await create_provider("copilot", validate=False, max_session_seconds=120.0) + assert isinstance(provider, CopilotProvider) + assert provider._idle_recovery_config.max_session_seconds == 120.0 + await provider.close() + + @pytest.mark.asyncio + async def test_default_max_session_seconds_without_override(self) -> None: + """Test that without max_session_seconds, the default (1800s) is used.""" + provider = await create_provider("copilot", validate=False) + assert isinstance(provider, CopilotProvider) + assert provider._idle_recovery_config.max_session_seconds == 1800.0 + await provider.close() + + @pytest.mark.asyncio + async def test_max_session_seconds_preserves_other_idle_recovery_defaults(self) -> None: + """Test that setting max_session_seconds doesn't change other defaults.""" + provider = await create_provider("copilot", validate=False, max_session_seconds=300.0) + assert isinstance(provider, CopilotProvider) + # max_session_seconds should be overridden + assert provider._idle_recovery_config.max_session_seconds == 300.0 + # Other fields should retain their defaults + assert provider._idle_recovery_config.idle_timeout_seconds == 300.0 + assert provider._idle_recovery_config.max_recovery_attempts == 3 + await provider.close() diff --git a/tests/test_providers/test_idle_recovery.py b/tests/test_providers/test_idle_recovery.py index e91c6da..be5bc50 100644 --- a/tests/test_providers/test_idle_recovery.py +++ b/tests/test_providers/test_idle_recovery.py @@ -741,3 +741,111 @@ def mock_fix_pipe() -> None: await asyncio.gather(*[provider._ensure_client_started() for _ in range(3)]) assert fix_pipe_count == 1 + + +class TestPerAgentMaxSessionSeconds: + """Tests for per-agent max_session_seconds override in _wait_with_idle_detection.""" + + @pytest.mark.asyncio + async def test_override_uses_shorter_timeout(self) -> None: + """Test that per-agent max_session_seconds overrides the provider-level default.""" + config = IdleRecoveryConfig( + idle_timeout_seconds=0.05, + max_recovery_attempts=10, + max_session_seconds=10.0, # Provider default: 10 seconds + ) + provider = CopilotProvider( + mock_handler=stub_handler, + idle_recovery_config=config, + ) + + done = asyncio.Event() # Never set + mock_session = MagicMock() + mock_session.send = AsyncMock() + + last_activity_ref: list[Any] = [None, None, time.monotonic()] + + # Pass a very short per-agent override — should fire quickly + with pytest.raises(ProviderError) as exc_info: + await provider._wait_with_idle_detection( + done=done, + session=mock_session, + verbose_enabled=False, + full_enabled=False, + last_activity_ref=last_activity_ref, + max_session_seconds=0.01, # Per-agent override: 10ms + ) + + assert "exceeded maximum duration" in str(exc_info.value) + + @pytest.mark.asyncio + async def test_none_override_falls_back_to_config(self) -> None: + """Test that None max_session_seconds falls back to IdleRecoveryConfig default.""" + config = IdleRecoveryConfig( + idle_timeout_seconds=0.05, + max_recovery_attempts=10, + max_session_seconds=0.01, # Provider default: 10ms (very short) + ) + provider = CopilotProvider( + mock_handler=stub_handler, + idle_recovery_config=config, + ) + + done = asyncio.Event() # Never set + mock_session = MagicMock() + mock_session.send = AsyncMock() + + last_activity_ref: list[Any] = [None, None, time.monotonic()] + + # Pass None — should use the config default (0.01s) + with pytest.raises(ProviderError) as exc_info: + await provider._wait_with_idle_detection( + done=done, + session=mock_session, + verbose_enabled=False, + full_enabled=False, + last_activity_ref=last_activity_ref, + max_session_seconds=None, + ) + + assert "exceeded maximum duration" in str(exc_info.value) + + @pytest.mark.asyncio + async def test_override_does_not_affect_idle_timeout(self) -> None: + """Test that per-agent max_session_seconds doesn't change idle detection behavior.""" + config = IdleRecoveryConfig( + idle_timeout_seconds=0.1, + max_recovery_attempts=2, + max_session_seconds=100.0, # Provider default: high + ) + provider = CopilotProvider( + mock_handler=stub_handler, + idle_recovery_config=config, + ) + + done = asyncio.Event() + mock_session = MagicMock() + mock_session.send = AsyncMock() + + # Set done after first recovery attempt + async def set_done_after_delay(): + await asyncio.sleep(0.15) + done.set() + + last_activity_ref: list[Any] = ["tool.execution_start", "web_search", 0.0] + + # Per-agent max_session_seconds is high — idle recovery should still work + await asyncio.gather( + provider._wait_with_idle_detection( + done=done, + session=mock_session, + verbose_enabled=False, + full_enabled=False, + last_activity_ref=last_activity_ref, + max_session_seconds=100.0, + ), + set_done_after_delay(), + ) + + # Should have sent at least one recovery message via idle detection + assert mock_session.send.call_count >= 1 diff --git a/tests/test_providers/test_registry.py b/tests/test_providers/test_registry.py index 1725028..8a03fcc 100644 --- a/tests/test_providers/test_registry.py +++ b/tests/test_providers/test_registry.py @@ -329,6 +329,7 @@ async def test_runtime_config_passed_to_provider(self, mock_create: MagicMock) - temperature=0.7, max_tokens=4096, timeout=60.0, + max_session_seconds=None, ) @patch("conductor.providers.registry.create_provider")