diff --git a/dare_framework/mcp/tool_provider.py b/dare_framework/mcp/tool_provider.py index 32eaf8c1..c6e4d4d3 100644 --- a/dare_framework/mcp/tool_provider.py +++ b/dare_framework/mcp/tool_provider.py @@ -104,7 +104,13 @@ def requires_approval(self) -> bool: @property def timeout_seconds(self) -> int: value = _tool_field(self._tool_def, "timeout_seconds", 30) - return int(value) if isinstance(value, (int, float, str)) else 30 + if isinstance(value, bool): + return 30 + try: + parsed = int(value) + except (OverflowError, TypeError, ValueError): + return 30 + return parsed if parsed > 0 else 30 @property def is_work_unit(self) -> bool: diff --git a/dare_framework/tool/_internal/tools/edit_line.py b/dare_framework/tool/_internal/tools/edit_line.py index 3f7b7209..bf1ef74b 100644 --- a/dare_framework/tool/_internal/tools/edit_line.py +++ b/dare_framework/tool/_internal/tools/edit_line.py @@ -104,7 +104,11 @@ def _execute_edit(input: dict[str, Any], context: RunContext[Any]) -> ToolResult if mode not in {"insert", "delete"}: raise ToolError(code="INVALID_MODE", message="mode must be insert or delete", retryable=False) - line_number = _parse_line_number(input.get("line_number")) + # Missing line_number should use schema default (1), but explicit null remains invalid. + if "line_number" in input: + line_number = _parse_line_number(input.get("line_number")) + else: + line_number = 1 text = input.get("text", "") strict_match = bool(input.get("strict_match", True)) diff --git a/openspec/changes/archive/2026-02-10-fix-tool-capability-listing-contract/proposal.md b/openspec/changes/archive/2026-02-10-fix-tool-capability-listing-contract/proposal.md new file mode 100644 index 00000000..fdb0c1c3 --- /dev/null +++ b/openspec/changes/archive/2026-02-10-fix-tool-capability-listing-contract/proposal.md @@ -0,0 +1,18 @@ +# Change: Fix tool capability listing contract + +## Why +A P0 regression occurred because the tool capability listing contract drifted between sync and async call sites. Some runtime paths awaited `list_capabilities()` while the implementation returned a plain list, and context assembly temporarily returned tool-definition dicts instead of `CapabilityDescriptor` objects, causing model adapter crashes. + +## What Changes +- Align the gateway contract so `IToolGateway.list_capabilities()` is awaitable and returns `list[CapabilityDescriptor]`. +- Keep synchronous runtime assembly paths supported via a manager-side synchronous capability snapshot helper. +- Require context tool assembly to provide trusted `CapabilityDescriptor` entries to `ModelInput.tools`. +- Add regression coverage to prevent descriptor-to-dict type drift in context assembly. + +## Impact +- Affected specs: `interface-layer` +- Affected code: + - `dare_framework/tool/kernel.py` + - `dare_framework/tool/tool_manager.py` + - `dare_framework/context/context.py` + - `tests/unit/test_context_implementation.py` diff --git a/openspec/changes/archive/2026-02-10-fix-tool-capability-listing-contract/specs/interface-layer/spec.md b/openspec/changes/archive/2026-02-10-fix-tool-capability-listing-contract/specs/interface-layer/spec.md new file mode 100644 index 00000000..cff2fed8 --- /dev/null +++ b/openspec/changes/archive/2026-02-10-fix-tool-capability-listing-contract/specs/interface-layer/spec.md @@ -0,0 +1,27 @@ +## MODIFIED Requirements +### Requirement: Tool manager contract +The tool domain SHALL define an `IToolManager` contract in `dare_framework/tool/kernel.py` that extends `IToolGateway`. The contract SHALL support trusted tool registration, provider aggregation, and prompt tool definition export without executing tools. At minimum it MUST include: +- `register_tool(...)`, `unregister_tool(...)`, `update_tool(...)` +- `register_provider(...)`, `unregister_provider(...)` +- awaitable `list_capabilities(...)`, `refresh(...)` +- `list_tool_defs(...)`, `get_capability(...)` +- `health_check(...)` + +Implementations MAY expose a synchronous capability snapshot helper for synchronous runtime assembly paths, but this helper MUST return the same trusted `CapabilityDescriptor` model as `list_capabilities(...)`. + +#### Scenario: Awaitable capability listing is stable for gateway callers +- **GIVEN** the default `ToolManager` implementation is used as `IToolGateway` +- **WHEN** callers execute `await list_capabilities()` +- **THEN** the result is `list[CapabilityDescriptor]` +- **AND** no sync/async type mismatch error is raised by awaiting callers + +### Requirement: Trusted tool listings for model prompts +Context and runtime model-input assembly SHALL source tool availability from the trusted capability registry exposed by `IToolGateway.list_capabilities()`; tool listings MUST NOT originate from untrusted sources (planner/model output). + +`ModelInput.tools` MUST carry `CapabilityDescriptor` entries. Model adapters are responsible for converting these trusted descriptors into provider-specific tool-definition payloads. + +#### Scenario: Context assembles capability descriptors for model adapters +- **GIVEN** context is wired with the default `ToolManager` +- **WHEN** context assembles tool listings for a model request +- **THEN** each item is a `CapabilityDescriptor` +- **AND** adapters can access descriptor fields (`name`, `description`, `input_schema`) without dict coercion diff --git a/openspec/changes/archive/2026-02-10-fix-tool-capability-listing-contract/tasks.md b/openspec/changes/archive/2026-02-10-fix-tool-capability-listing-contract/tasks.md new file mode 100644 index 00000000..1c2c26d4 --- /dev/null +++ b/openspec/changes/archive/2026-02-10-fix-tool-capability-listing-contract/tasks.md @@ -0,0 +1,12 @@ +## 1. Contract alignment +- [x] 1.1 Make `IToolGateway.list_capabilities()` awaitable in the interface contract. +- [x] 1.2 Update `ToolManager` to implement awaitable `list_capabilities()` and keep a sync capability snapshot helper for sync call sites. + +## 2. Context/model-input type safety +- [x] 2.1 Ensure `Context.list_tools()` returns trusted `CapabilityDescriptor` entries (not tool-definition dicts). +- [x] 2.2 Preserve compatibility for legacy synchronous gateways. + +## 3. Regression coverage and validation +- [x] 3.1 Add a regression test proving context tool assembly returns `CapabilityDescriptor` from `ToolManager`. +- [x] 3.2 Run targeted pytest for tool/context contract paths. +- [x] 3.3 Run `openspec validate fix-tool-capability-listing-contract --strict`. diff --git a/openspec/changes/refactor-skill-store-builder/design.md b/openspec/changes/archive/2026-02-10-refactor-skill-store-builder/design.md similarity index 100% rename from openspec/changes/refactor-skill-store-builder/design.md rename to openspec/changes/archive/2026-02-10-refactor-skill-store-builder/design.md diff --git a/openspec/changes/refactor-skill-store-builder/proposal.md b/openspec/changes/archive/2026-02-10-refactor-skill-store-builder/proposal.md similarity index 100% rename from openspec/changes/refactor-skill-store-builder/proposal.md rename to openspec/changes/archive/2026-02-10-refactor-skill-store-builder/proposal.md diff --git a/openspec/changes/refactor-skill-store-builder/specs/component-management/spec.md b/openspec/changes/archive/2026-02-10-refactor-skill-store-builder/specs/component-management/spec.md similarity index 100% rename from openspec/changes/refactor-skill-store-builder/specs/component-management/spec.md rename to openspec/changes/archive/2026-02-10-refactor-skill-store-builder/specs/component-management/spec.md diff --git a/openspec/changes/refactor-skill-store-builder/specs/configuration-management/spec.md b/openspec/changes/archive/2026-02-10-refactor-skill-store-builder/specs/configuration-management/spec.md similarity index 100% rename from openspec/changes/refactor-skill-store-builder/specs/configuration-management/spec.md rename to openspec/changes/archive/2026-02-10-refactor-skill-store-builder/specs/configuration-management/spec.md diff --git a/openspec/changes/refactor-skill-store-builder/specs/interface-layer/spec.md b/openspec/changes/archive/2026-02-10-refactor-skill-store-builder/specs/interface-layer/spec.md similarity index 100% rename from openspec/changes/refactor-skill-store-builder/specs/interface-layer/spec.md rename to openspec/changes/archive/2026-02-10-refactor-skill-store-builder/specs/interface-layer/spec.md diff --git a/openspec/changes/refactor-skill-store-builder/tasks.md b/openspec/changes/archive/2026-02-10-refactor-skill-store-builder/tasks.md similarity index 100% rename from openspec/changes/refactor-skill-store-builder/tasks.md rename to openspec/changes/archive/2026-02-10-refactor-skill-store-builder/tasks.md diff --git a/openspec/changes/fix-p1-tool-input-defaults/proposal.md b/openspec/changes/fix-p1-tool-input-defaults/proposal.md new file mode 100644 index 00000000..6fc6aa29 --- /dev/null +++ b/openspec/changes/fix-p1-tool-input-defaults/proposal.md @@ -0,0 +1,19 @@ +# Change: Fix P1 tool input defaults and metadata coercion + +## Why +Two P1 issues caused avoidable tool failures: +- `edit_line` rejected insert calls when `line_number` was omitted even though the schema declares a default. +- MCP tool metadata with non-numeric `timeout_seconds` raised conversion errors and could break capability registration paths. + +## What Changes +- Ensure `edit_line` treats missing `line_number` as line 1 for insert/delete indexing. +- Ensure MCP tool timeout parsing is defensive: invalid or non-positive timeout values fall back to a safe default. +- Add regression tests for both behaviors. + +## Impact +- Affected specs: `workspace-file-tools`, `component-management` +- Affected code: + - `dare_framework/tool/_internal/tools/edit_line.py` + - `dare_framework/mcp/tool_provider.py` + - `tests/unit/test_v4_file_tools.py` + - `tests/unit/test_mcp_tool_provider.py` diff --git a/openspec/changes/fix-p1-tool-input-defaults/specs/component-management/spec.md b/openspec/changes/fix-p1-tool-input-defaults/specs/component-management/spec.md new file mode 100644 index 00000000..70fca892 --- /dev/null +++ b/openspec/changes/fix-p1-tool-input-defaults/specs/component-management/spec.md @@ -0,0 +1,21 @@ +## MODIFIED Requirements +### Requirement: Tool manager aggregates providers and exports tool defs +ToolManager SHALL aggregate `IToolProvider` instances and refresh their capabilities into the registry. It MUST provide prompt tool definitions derived from the registry and MUST NOT execute tool side-effects; invocation is owned by `IToolGateway` (implemented by ToolManager). + +Provider-derived metadata coercion MUST be robust: malformed provider metadata values MUST NOT crash registry aggregation. + +#### Scenario: Provider capabilities are visible in the registry +- **GIVEN** a provider is registered with the ToolManager +- **WHEN** `refresh()` is called +- **THEN** its capabilities are available via `list_capabilities()` + +#### Scenario: Invalid provider timeout metadata falls back safely +- **GIVEN** a provider exposes a tool with non-numeric `timeout_seconds` +- **WHEN** ToolManager aggregates provider capabilities +- **THEN** capability registration does not fail +- **AND** the effective timeout metadata falls back to the default value + +#### Scenario: Tool manager does not invoke tools +- **GIVEN** a tool capability is registered +- **WHEN** an invocation is needed +- **THEN** the system routes the call through `IToolGateway.invoke(...)` (ToolManager implements the gateway) diff --git a/openspec/changes/fix-p1-tool-input-defaults/specs/workspace-file-tools/spec.md b/openspec/changes/fix-p1-tool-input-defaults/specs/workspace-file-tools/spec.md new file mode 100644 index 00000000..4dc21e04 --- /dev/null +++ b/openspec/changes/fix-p1-tool-input-defaults/specs/workspace-file-tools/spec.md @@ -0,0 +1,20 @@ +## MODIFIED Requirements +### Requirement: Edit line tool behavior +The `edit_line` tool SHALL insert or delete a line in a text file using a 1-indexed `line_number`, enforce `edit_line.max_bytes`, preserve newline style, and default `strict_match` to true for deletions. + +When `line_number` is omitted, the tool MUST default it to `1`. + +#### Scenario: Insert adds a line at the target index +- **WHEN** `edit_line` is invoked with `mode: insert` +- **THEN** the line is inserted at the requested position and the tool returns the affected line number + +#### Scenario: Insert defaults to first line when line_number is omitted +- **GIVEN** an existing text file under workspace roots +- **WHEN** `edit_line` is invoked with `mode: insert` and no `line_number` +- **THEN** the line is inserted at line 1 +- **AND** the tool returns `line_number: 1` + +#### Scenario: Delete fails on strict mismatch +- **GIVEN** `strict_match` is true and the target line differs from the provided text +- **WHEN** `edit_line` is invoked with `mode: delete` +- **THEN** the tool returns a failure result indicating a mismatch diff --git a/openspec/changes/fix-p1-tool-input-defaults/tasks.md b/openspec/changes/fix-p1-tool-input-defaults/tasks.md new file mode 100644 index 00000000..98b370d8 --- /dev/null +++ b/openspec/changes/fix-p1-tool-input-defaults/tasks.md @@ -0,0 +1,11 @@ +## 1. Edit-line default behavior +- [x] 1.1 Add a failing test proving `edit_line` insert succeeds when `line_number` is omitted. +- [x] 1.2 Update `edit_line` execution to default missing `line_number` to 1. + +## 2. MCP timeout coercion hardening +- [x] 2.1 Add a failing test proving invalid MCP `timeout_seconds` does not crash and falls back. +- [x] 2.2 Update MCP timeout parsing to safely coerce and fallback on invalid/non-positive values. + +## 3. Validation +- [x] 3.1 Run targeted pytest for affected modules. +- [x] 3.2 Run `openspec validate fix-p1-tool-input-defaults --strict`. diff --git a/openspec/specs/component-management/spec.md b/openspec/specs/component-management/spec.md index 253e9995..1d5e66a0 100644 --- a/openspec/specs/component-management/spec.md +++ b/openspec/specs/component-management/spec.md @@ -81,3 +81,24 @@ ToolManager SHALL aggregate `IToolProvider` instances and refresh their capabili - **WHEN** an invocation is needed - **THEN** the system routes the call through `IToolGateway.invoke(...)` (ToolManager implements the gateway) +### Requirement: Skill store builder composes loaders deterministically +The skill domain SHALL provide a `SkillStoreBuilder` that deterministically composes skill loaders and filtering rules before constructing an `ISkillStore`. + +- `SkillStoreBuilder.config(config)` MUST derive filesystem skill loading roots from `Config.workspace_dir` and `Config.user_dir`. +- The builder MUST allow callers to append external skill loaders. +- The builder MUST support disabling skills by `skill_id` before final store exposure. +- The resulting `ISkillStore` MUST provide deterministic `list_skills()` and `get_skill(skill_id)` behavior after composition. + +#### Scenario: Config-derived loader and external loader are combined +- **GIVEN** a `Config` with workspace and user directories +- **AND** an external loader is attached to `SkillStoreBuilder` +- **WHEN** `build()` is called +- **THEN** the resulting store contains skills from both config-derived filesystem loading and the external loader + +#### Scenario: Disabled skill ids are filtered out +- **GIVEN** a composed skill store contains skill ids `a`, `b`, and `c` +- **AND** `disable_skill("b")` is configured +- **WHEN** `build()` is called +- **THEN** `list_skills()` excludes `b` +- **AND** `get_skill("b")` returns `None` + diff --git a/openspec/specs/configuration-management/spec.md b/openspec/specs/configuration-management/spec.md index e1d48713..621edd6b 100644 --- a/openspec/specs/configuration-management/spec.md +++ b/openspec/specs/configuration-management/spec.md @@ -38,14 +38,15 @@ The allowtools and allowmcps settings SHALL be treated as normal configuration k ### Requirement: Config Model Structure The Config model SHALL include at least the following top-level fields: llm, mcp, tools, allowtools, allowmcps, components, workspace_dir, and user_dir. The llm field MUST support connectivity configuration such as adapter, endpoint, api_key, model, and optional proxy settings (http, https, no_proxy, use_system_proxy, disabled). When proxy.disabled is true, proxy settings MUST be treated as disabled. Explicit proxy configuration and system proxy selection MUST be mutually exclusive in the effective model. The mcp and tools fields MUST support component-scoped configuration objects keyed by component name. The components field MUST support enable/disable flags and per-component configuration for entry point components by type and name (including validator, hook, skill, memory, model_adapter, tool, mcp, and prompt). -#### Scenario: LLM connectivity fields available -- **WHEN** the effective Config is produced -- **THEN** the llm field can provide adapter, endpoint, api_key, model, and optional proxy values +The Config model MUST NOT require top-level skill mode/path fields such as `skill_mode`, `skill_paths`, or `initial_skill_path`. -#### Scenario: Proxy disabled takes precedence -- **GIVEN** llm.proxy.disabled is true -- **WHEN** the effective Config is produced -- **THEN** the model adapter treats proxy as disabled regardless of other proxy fields +Skill filesystem discovery MUST be derived from `workspace_dir` and `user_dir` by the skill loading layer. + +#### Scenario: Skill loading relies on workspace/user dirs only +- **GIVEN** an effective Config containing `workspace_dir` and `user_dir` +- **WHEN** the runtime initializes skill loading +- **THEN** skill loading derives filesystem roots from those directories +- **AND** no `skill_mode`, `skill_paths`, or `initial_skill_path` fields are required in Config ### Requirement: Component Enablement Configuration The system SHALL support a uniform enable/disable configuration for all entry point components using a type-scoped structure. The components..disabled list identifies disabled component names. The default behavior MUST be enabled when a name is absent from components..disabled. diff --git a/openspec/specs/interface-layer/spec.md b/openspec/specs/interface-layer/spec.md index 16c562ae..172dd0d5 100644 --- a/openspec/specs/interface-layer/spec.md +++ b/openspec/specs/interface-layer/spec.md @@ -36,26 +36,28 @@ The developer-facing agent API SHALL support composing agents via typed builders - When a required component is not explicitly provided, builders SHALL attempt to resolve it via the corresponding domain manager using the effective `Config`. - For multi-load component categories (e.g., tools/hooks/validators), builders SHALL merge explicit components with manager-loaded components (extend semantics) while preserving injection order. - Config enable/disable filtering MUST apply only to the manager-loaded subset and MUST NOT remove explicitly injected components. +- Builders SHALL keep `assemble_context` externally injectable so callers can customize `AssembledContext` generation. +- Builders SHALL expose a boolean skill-tool toggle (`_enable_skill_tool` via public builder API) as the only built-in skill mode switch. +- When skill-tool toggle is enabled, builders MUST auto-register `search_skill` and default context assembly MUST ignore `sys_skill`. +- When skill-tool toggle is disabled, builders MUST NOT auto-register `search_skill` and explicit `sys_skill` injection remains effective. -#### Scenario: Resolve model adapter via manager -- **GIVEN** a builder with no explicit model adapter -- **AND** a provided `IModelAdapterManager` -- **AND** an effective `Config` +#### Scenario: Skill tool mode auto-registers search tool +- **GIVEN** a builder with skill-tool toggle enabled - **WHEN** `build()` is called -- **THEN** the builder MUST call `IModelAdapterManager.load_model_adapter(config=Config)` and use the returned adapter as the agent model +- **THEN** the built context exposes `search_skill` in tool capabilities +- **AND** default assemble logic does not merge `sys_skill` -#### Scenario: Explicit model overrides manager -- **GIVEN** a builder with an explicitly injected model adapter via builder API -- **AND** a provided `IModelAdapterManager` that would otherwise return a different adapter -- **WHEN** `build()` is called -- **THEN** the explicitly injected adapter MUST be used +#### Scenario: Non skill tool mode preserves explicit sys_skill +- **GIVEN** a builder with skill-tool toggle disabled +- **AND** an explicit `sys_skill` is set on context +- **WHEN** default assemble logic runs +- **THEN** `search_skill` is not auto-registered +- **AND** the assembled system prompt includes the explicit `sys_skill` -#### Scenario: Multi-load extend with config boundary -- **GIVEN** a builder with an explicitly injected tool `tool_x` -- **AND** a provided `IToolManager` that loads `tool_y` and `tool_z` -- **AND** config disables `tool_z` -- **WHEN** `build()` is called -- **THEN** `tool_x` and `tool_y` MUST be included and `tool_z` MUST be omitted, and `tool_x` MUST NOT be filtered out by config +#### Scenario: Custom assemble context remains pluggable +- **GIVEN** a caller injects a custom `assemble_context` +- **WHEN** the agent assembles context for model input +- **THEN** the custom strategy is used instead of the default strategy ### Requirement: Optional MCP Integration Surface The interface layer SHALL NOT define protocol adapter interfaces. Protocol adapter surfaces are deferred until a dedicated integration is specified. @@ -92,19 +94,17 @@ The system SHALL define component manager interfaces alongside their owning doma The tool domain SHALL define an `IToolManager` contract in `dare_framework/tool/kernel.py` that extends `IToolGateway`. The contract SHALL support trusted tool registration, provider aggregation, and prompt tool definition export without executing tools. At minimum it MUST include: - `register_tool(...)`, `unregister_tool(...)`, `update_tool(...)` - `register_provider(...)`, `unregister_provider(...)` -- `list_capabilities(...)`, `refresh(...)` +- awaitable `list_capabilities(...)`, `refresh(...)` - `list_tool_defs(...)`, `get_capability(...)` - `health_check(...)` -#### Scenario: Register tool returns a capability descriptor -- **GIVEN** a valid `ITool` implementation -- **WHEN** `register_tool(...)` is called -- **THEN** the manager returns a `CapabilityDescriptor` with a stable `capability_id` and trusted metadata +Implementations MAY expose a synchronous capability snapshot helper for synchronous runtime assembly paths, but this helper MUST return the same trusted `CapabilityDescriptor` model as `list_capabilities(...)`. -#### Scenario: Tool definitions are derived from registry -- **GIVEN** a tool has been registered into the manager -- **WHEN** `list_tool_defs()` is called -- **THEN** the result is derived from the manager registry and not from model output +#### Scenario: Awaitable capability listing is stable for gateway callers +- **GIVEN** the default `ToolManager` implementation is used as `IToolGateway` +- **WHEN** callers execute `await list_capabilities()` +- **THEN** the result is `list[CapabilityDescriptor]` +- **AND** no sync/async type mismatch error is raised by awaiting callers ### Requirement: Builder facade for variant selection The system SHALL provide a stable facade for selecting which builder variant to use via `BaseAgent`, such as: @@ -133,11 +133,15 @@ The system SHALL use the tool's `name` as the canonical identity for capabilitie - **THEN** the tool definition name equals the tool's `name` and tool calls route by that value ### Requirement: Trusted tool listings for model prompts -`IToolProvider.list_tools()` SHALL derive tool definitions from the trusted capability registry exposed by `IToolGateway.list_capabilities()`; tool listings MUST NOT originate from untrusted sources (planner/model output). +Context and runtime model-input assembly SHALL source tool availability from the trusted capability registry exposed by `IToolGateway.list_capabilities()`; tool listings MUST NOT originate from untrusted sources (planner/model output). + +`ModelInput.tools` MUST carry `CapabilityDescriptor` entries. Model adapters are responsible for converting these trusted descriptors into provider-specific tool-definition payloads. -#### Scenario: Tool provider uses the gateway registry -- **WHEN** the context assembles tool definitions for a model prompt -- **THEN** the tool provider queries the tool gateway capability registry and converts those capabilities into tool definitions for the prompt +#### Scenario: Context assembles capability descriptors for model adapters +- **GIVEN** context is wired with the default `ToolManager` +- **WHEN** context assembles tool listings for a model request +- **THEN** each item is a `CapabilityDescriptor` +- **AND** adapters can access descriptor fields (`name`, `description`, `input_schema`) without dict coercion ### Requirement: Default model adapter manager fallback When no explicit model adapter and no model adapter manager are provided, builders SHALL fall back to a default model adapter manager created by the model domain factory and resolve the adapter using the effective `Config`. diff --git a/tests/unit/test_mcp_tool_provider.py b/tests/unit/test_mcp_tool_provider.py index 83b81234..218349b9 100644 --- a/tests/unit/test_mcp_tool_provider.py +++ b/tests/unit/test_mcp_tool_provider.py @@ -2,11 +2,13 @@ from __future__ import annotations +from math import inf from typing import Any import pytest from dare_framework.mcp.tool_provider import MCPToolProvider +from dare_framework.tool.tool_manager import ToolManager from dare_framework.tool.types import ToolResult @@ -110,3 +112,55 @@ async def test_mcp_tool_execute_forwards_arguments() -> None: assert result.success is True assert client.calls == [("multiply", {"a": 6, "b": 7})] + + +@pytest.mark.asyncio +async def test_mcp_tool_invalid_timeout_falls_back_and_registers() -> None: + client = _FakeMCPClient( + "local_math", + [ + { + "name": "divide", + "description": "Divide two numbers.", + "timeout_seconds": "invalid-timeout", + } + ], + ) + provider = MCPToolProvider([client]) + await provider.initialize() + tool = provider.list_tools()[0] + + assert tool.timeout_seconds == 30 + + manager = ToolManager(load_entrypoints=False) + manager.register_provider(provider) + caps = await manager.list_capabilities() + assert caps + metadata = caps[0].metadata or {} + assert metadata.get("timeout_seconds") == 30 + + +@pytest.mark.asyncio +async def test_mcp_tool_infinite_timeout_falls_back_and_registers() -> None: + client = _FakeMCPClient( + "local_math", + [ + { + "name": "divide", + "description": "Divide two numbers.", + "timeout_seconds": inf, + } + ], + ) + provider = MCPToolProvider([client]) + await provider.initialize() + tool = provider.list_tools()[0] + + assert tool.timeout_seconds == 30 + + manager = ToolManager(load_entrypoints=False) + manager.register_provider(provider) + caps = await manager.list_capabilities() + assert caps + metadata = caps[0].metadata or {} + assert metadata.get("timeout_seconds") == 30 diff --git a/tests/unit/test_v4_file_tools.py b/tests/unit/test_v4_file_tools.py index 7c5dbb85..14b9ac43 100644 --- a/tests/unit/test_v4_file_tools.py +++ b/tests/unit/test_v4_file_tools.py @@ -108,6 +108,46 @@ async def test_edit_line_strict_match_mismatch(tmp_path): assert result.output.get("code") == "LINE_MISMATCH" +@pytest.mark.asyncio +async def test_edit_line_insert_defaults_line_number_to_first_line(tmp_path): + root = tmp_path / "root" + root.mkdir() + target = root / "sample.txt" + target.write_text("one\ntwo\n") + + ctx = RunContext(deps=None, run_id="run", config={"workspace_roots": [str(root)]}) + + tool = EditLineTool() + result = await tool.execute( + {"path": "sample.txt", "mode": "insert", "text": "zero"}, + ctx, + ) + + assert result.success is True + assert result.output["line_number"] == 1 + assert target.read_text() == "zero\none\ntwo\n" + + +@pytest.mark.asyncio +async def test_edit_line_rejects_explicit_null_line_number(tmp_path): + root = tmp_path / "root" + root.mkdir() + target = root / "sample.txt" + target.write_text("one\ntwo\n") + + ctx = RunContext(deps=None, run_id="run", config={"workspace_roots": [str(root)]}) + + tool = EditLineTool() + result = await tool.execute( + {"path": "sample.txt", "mode": "delete", "line_number": None}, + ctx, + ) + + assert result.success is False + assert result.output.get("code") == "INVALID_LINE" + assert target.read_text() == "one\ntwo\n" + + @pytest.mark.asyncio async def test_read_file_line_range_truncates(tmp_path): root = tmp_path / "root"