Skip to content

Commit 4eea127

Browse files
anticomputerCopilot
andcommitted
doc + refactor: address remaining PR review threads
README.md: Add the per-task backend override as the highest-precedence selection level. Tasks can put 'backend:' in their own model_settings block to override the model-level value, per _resolve_task_model(). doc/GRAMMAR.md: Tighten the 'passed through to the selected SDK backend' claim. openai_agents accepts the standard OpenAI parameter set, anthropic_sdk forwards a curated subset (temperature, top_p, reasoning, max_tokens, stream_thinking), and copilot_sdk consumes only its own exposed keys (e.g. reasoning_effort) and silently ignores the rest. Avoid misleading users about arbitrary key forwarding. mcp_utils.py: Make list_tools_unfiltered idempotent on the prefix. Strip an existing namespace prefix before re-applying so the method is safe to call repeatedly even if the underlying session somehow returns a cached/reused tool object whose name was previously namespaced. Uses str.removeprefix() (no-op when prefix is absent). Regression test added covering the previously-prefixed-input path. 276 tests pass, hatch fmt clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b1b139b commit 4eea127

4 files changed

Lines changed: 43 additions & 10 deletions

File tree

README.md

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,16 @@ The runner can drive three SDKs behind a common interface:
103103
Designed for use with CAPI's Anthropic endpoint; auth uses
104104
`Authorization: Bearer` (not `x-api-key`).
105105

106-
Selection precedence:
107-
108-
1. Per-model `backend:` in `model_settings` (allows mixed backends in a
109-
single taskflow).
110-
2. `backend:` field in the model config document (global default).
111-
3. `SECLAB_TASKFLOW_BACKEND` environment variable.
112-
4. `openai_agents`.
106+
Selection precedence (highest to lowest):
107+
108+
1. Per-task `backend:` in the task's own `model_settings` block (overrides
109+
the model-level value for that one task; see `_resolve_task_model()`).
110+
2. Per-model `backend:` in the model config's `model_settings` (allows
111+
mixed backends in a single taskflow).
112+
3. `backend:` field at the top level of the model config document
113+
(global default).
114+
4. `SECLAB_TASKFLOW_BACKEND` environment variable.
115+
5. `openai_agents`.
113116

114117
```yaml
115118
seclab-taskflow-agent:

doc/GRAMMAR.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -550,5 +550,10 @@ passed to the underlying model provider:
550550
| `endpoint` | API base URL for this model | The global `AI_API_ENDPOINT` env var |
551551
| `token` | Name of an environment variable containing the API key | Uses `AI_API_TOKEN` / `COPILOT_TOKEN` |
552552

553-
All other keys (e.g. `temperature`, `top_p`, `reasoning`) are passed through as
554-
model parameters to the selected SDK backend.
553+
All other keys (e.g. `temperature`, `top_p`, `reasoning`) are forwarded to
554+
the selected SDK backend. Which keys are actually honored depends on the
555+
backend: `openai_agents` accepts the standard OpenAI parameter set;
556+
`anthropic_sdk` forwards a curated subset (currently `temperature`,
557+
`top_p`, `reasoning`, `max_tokens`, `stream_thinking`); `copilot_sdk`
558+
consumes only the keys its SDK exposes (e.g. `reasoning_effort`) and
559+
silently ignores the rest. Consult the backend-specific docs if in doubt.

src/seclab_taskflow_agent/mcp_utils.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,12 @@ async def list_tools_unfiltered(self) -> list[Any]:
106106
run loop -- e.g. when handing tools to a different SDK at build
107107
time).
108108
109+
Prefixing is idempotent: if a tool's name already starts with this
110+
wrapper's namespace (e.g. because the underlying session returned a
111+
previously-namespaced object), the existing prefix is stripped
112+
before re-applying so calling this method multiple times never
113+
yields ``<ns><ns>name``.
114+
109115
Raises ``RuntimeError`` if the underlying server has no active
110116
MCP session yet (caller should ensure the server is connected
111117
before calling this).
@@ -120,7 +126,9 @@ async def list_tools_unfiltered(self) -> list[Any]:
120126
namespaced_tools: list[Any] = []
121127
for tool in result.tools:
122128
tool_copy = tool.copy() if hasattr(tool, "copy") else tool
123-
tool_copy.name = f"{self.namespace}{tool.name}"
129+
# Idempotent: strip existing prefix before re-applying
130+
base_name = tool_copy.name.removeprefix(self.namespace)
131+
tool_copy.name = f"{self.namespace}{base_name}"
124132
namespaced_tools.append(tool_copy)
125133
return namespaced_tools
126134

tests/test_mcp_utils.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,23 @@ def test_list_tools_unfiltered_does_not_share_state_with_caller():
111111
assert original.name == "read_file"
112112

113113

114+
def test_list_tools_unfiltered_idempotent_on_prefixed_input():
115+
"""If the session returns a tool whose name is already namespace-prefixed
116+
(e.g. because of a cached/reused tool object), the prefix must NOT be
117+
applied a second time. Required for safe repeated/reentrant calls."""
118+
ns = compress_name("RepoContext")
119+
pre_prefixed = _FakeTool(f"{ns}read_file", "Read a file")
120+
session = MagicMock()
121+
session.list_tools = AsyncMock(return_value=SimpleNamespace(tools=[pre_prefixed]))
122+
wrapper = _make_wrapper("RepoContext", session=session)
123+
124+
result = asyncio.run(wrapper.list_tools_unfiltered())
125+
126+
# Result must have exactly one prefix, not two
127+
assert result[0].name == f"{ns}read_file"
128+
assert not result[0].name.startswith(f"{ns}{ns}")
129+
130+
114131
# -- list_tools() (regression) --
115132

116133

0 commit comments

Comments
 (0)