From df136cfb95553cc23dcf826ffdba1b186cfd160e Mon Sep 17 00:00:00 2001 From: george larson Date: Sun, 16 Aug 2026 16:49:02 -0400 Subject: [PATCH 1/7] feat(tools): add per-call llm_profile override to the task tool Co-authored-by: openhands --- .pr/design.md | 221 ++++++++++++++++++ .../conversation/impl/local_conversation.py | 34 +++ .../openhands/sdk/subagent/AGENTS.md | 2 + .../openhands/sdk/tool/builtins/switch_llm.py | 5 +- .../openhands/tools/task/definition.py | 31 ++- openhands-tools/openhands/tools/task/impl.py | 1 + .../openhands/tools/task/manager.py | 84 +++++-- tests/sdk/conversation/test_switch_model.py | 65 ++++++ tests/tools/task/conftest.py | 21 ++ tests/tools/task/test_task_manager.py | 184 +++++++++++++++ tests/tools/task/test_task_tool_set.py | 142 ++++++++++- 11 files changed, 773 insertions(+), 17 deletions(-) create mode 100644 .pr/design.md create mode 100644 tests/tools/task/conftest.py diff --git a/.pr/design.md b/.pr/design.md new file mode 100644 index 0000000000..63ae6e0ba3 --- /dev/null +++ b/.pr/design.md @@ -0,0 +1,221 @@ +# Per-call `llm_profile` on the task tool — design notes + +**Branch:** `feat/task-tool-llm-profile` (off `origin/main` 4fe565663) +**Status:** implemented, post-panel revision applied, tests green, uncommitted +as of 2026-08-16 + +> Line anchors in this doc (`file.py:NNN`) refer to the pre-rebase tree; the +> branch was rebased onto upstream/main 007721b3d on 2026-08-16 and upstream +> churn shifted them. + +> **Post-panel revision (2026-08-16):** a 7-voice review panel returned +> "ship after fixes." Applied: (1) cipher-aware profile resolution via a new +> public `LocalConversation.load_profile_llm` (the per-call path previously +> loaded with no cipher — ciphertext api_key under cipher-at-rest); (2) +> underscore-private cross-package imports eliminated (`_format_profiles` +> promoted to public `format_llm_profiles`; manager no longer touches +> `registry._get_profile_store`); (3) a pinned `model:` now skips per-call +> resolution entirely, so an unknown `llm_profile` under a pin no longer +> errors; (4) `Task.llm_profile` persisted so a bare resume keeps the +> creation-time profile instead of silently reverting to the parent model; +> (5) the tool description omits the profiles section when no profiles are +> saved; (6) the task-test conftest clears the registry's lru_cached profile +> store on setup and teardown. +> +> **Round-2 (7-of-8 SHIP):** the one requested code fix — a pinned definition +> now stores the *effective* profile (`None`) on the task, not the raw ignored +> request, so a bare resume after a later pin→inherit flip can't try to load a +> typo'd name. Remaining items were doc/wording nits; the proposed +> `vision_inspect._format_profiles` dedupe was rejected (its empty-list message +> and ordering differ from `format_llm_profiles`). + +## Problem + +Observed in OpenHands #proj-agent-canvas (2026-08-16, Rajiv Shah): prompting Agent +Canvas with a mixed-model workflow ("GPT-5.6 plans, DeepSeek V4 Pro does the +fixes") makes the main agent call `switch_llm` — switching *itself* to the second +model — instead of delegating to a subagent running that model. Two default +settings steer this: `enable_switch_llm_tool` defaults **on** (and its tool +description advertises every saved profile with "use this when another profile is +better suited for the next step"), while `enable_sub_agents` defaults **off**. + +For sequential plan-then-fix work, self-switching is arguably fine. It breaks +down for the fan-out case: one conversation = one model at a time, so a planner +cannot hold its own model while N workers run on a cheaper one. + +The only pre-existing mixed-model delegation path was authoring a subagent +definition `.md` with `model:` frontmatter — one file per agent-shape × model +combination, and nothing the parent can decide per-call from a prompt. + +## What already existed (and is reused) + +- `model:` frontmatter in subagent definitions resolves through + `LLMProfileStore` (`~/.openhands/profiles/.json`) inside the factory — + `openhands-sdk/openhands/sdk/subagent/registry.py:218-228`. +- Built-in subagents (`code_explorer`, `bash_runner`, `web_researcher`, + `default`) are all `model: inherit` — they take whatever LLM the factory is + handed. +- `switch_llm` already lists saved profiles in its tool description + (`get_llm_profile_names` / `format_llm_profiles` in + `openhands-sdk/openhands/sdk/tool/builtins/switch_llm.py`). +- `LocalConversation` already owns a profile store + optional cipher + (`local_conversation.py:413-414`) and decrypts on its own profile loads + (`switch_profile`, `get_or_create_profile_llm`). + +## The change + +`TaskAction` gains one optional field, `llm_profile: str | None` +(`openhands-tools/openhands/tools/task/definition.py`). When set, the named +profile is loaded **through the parent conversation** +(`LocalConversation.load_profile_llm`, new public accessor — cipher-aware, +honors `definition.profile_store_dir`, no llm_registry registration, no +subscription transforms) and injected into the subagent factory **in place of** +the parent-LLM clone (`openhands-tools/openhands/tools/task/manager.py`, +`_get_sub_agent_from_factory`). Threading: `TaskExecutor.__call__` (impl.py) → +`TaskManager.start_task` → `_create_task` / `_resume_task` → +`_get_sub_agent_from_factory`. + +The tool description gains a section listing saved profiles — only when at +least one profile exists (populated in `TaskToolSet.create` via the public +helpers imported from `switch_llm.py` — imported, not forked). + +## Design decisions (the ones a reviewer should attack) + +1. **Profile name, not model string.** The field can only name a file that + exists in the profile store; it is validated against `store.list()` (inside + `load_profile_llm`, listing from the same store it loads from) before + `store.load()`. No path to arbitrary model strings. Strictly smaller + capability than the default-on `switch_llm`, which moves the whole + conversation. + +2. **Pre-factory injection, not post-factory swap.** The factory derives the + subagent's default condenser LLM from the LLM it is handed. Injecting before + `factory_func` keeps the per-call override semantically identical to the + `model:` pin path (worker condenses on its own model). A post-factory swap + would leave the condenser on the parent model. + +3. **Precedence: definition `model:` pin > per-call `llm_profile` > inherit.** + A pinned definition skips per-call resolution entirely (checked before any + store access), so the field is truly ignored under a pin — even an unknown + name does not error, matching the field description. The *effective* + profile is what gets stored on the task (`None` under a pin), so a bare + resume can never try to load an ignored value if the definition is later + flipped to `inherit`. All built-ins are `inherit`, so the feature works on + built-ins out of the box. + +4. **Unknown profile = loud error, never fallback.** `ValueError` raised before + the task is registered in `self._tasks` (no half-created task leak); + `TaskExecutor.__call__` converts it to an error `TaskObservation` naming the + valid profiles — a retryable message to the parent model, not a crash. A + silent inherit fallback would spend parent-model money while appearing to + honor the override. + +5. **Resume keeps the task's model.** The effective per-call profile is stored + on `Task.llm_profile` at creation. A bare `resume=` rebuilds the worker + on that same profile; an explicit resume-time `llm_profile` switches the + worker and becomes the task's new stored profile. (Pre-panel, a bare resume + silently reverted to the parent model.) + +6. **Field description is prompt engineering.** It disambiguates against + `switch_llm` in the model's own reading ("This affects only the delegated + subagent — your own model is unchanged (use the switch_llm tool to change + your own model)") because the observed bug was self-switch instead of + delegate. + +7. **Profile resolution goes through the parent conversation, not the registry + store.** `LocalConversation.load_profile_llm` decrypts secrets with the + conversation's cipher (parity with `switch_profile`) and — deliberately — + does NOT register the LLM in the parent's `llm_registry` or apply + subscription transforms (that is what `get_or_create_profile_llm` does; + using it would muddle per-task metrics attribution). A definition-level + `profile_store_dir` is honored by constructing a store for that dir; the + conversation cipher is passed at `load()` time regardless of which store is + used (cipher is a deployment property, not a store property). + +## Explicitly out of scope + +- `tool_concurrency_limit` default (1) — parallel fan-out is a product decision. +- Built-in subagents' `model: inherit` — it is what makes them overridable. +- The older Delegate tool — separate schema, separate surface. +- UI surfaces (subagent definitions editor, profile picker at spawn). +- The `enable_switch_llm_tool`-on / `enable_sub_agents`-off default asymmetry — + a settings/rollout conversation, not this diff. +- The definition-pin path's own cipher handling inside + `registry.agent_definition_to_factory` (`store.load` without cipher, + registry.py:228) — pre-existing upstream behavior, unchanged by this diff. + Since this PR introduces the public cipher-aware accessor one floor up + (`LocalConversation.load_profile_llm`), routing the pin path through it is + now a small, obvious follow-up issue. +- Agent-server profile-store dir divergence (`OH_PERSISTENCE_DIR` vs SDK + home-dir default) — pre-existing, affects `model:` pins equally today. + (No agent-server changes: `TaskAction` is deserialized from the same + dynamically-registered tool code server-side, so the new field flows through.) + +## Metrics + +Override LLM is freshly constructed by `load_profile_llm` + `reset_metrics()` +(parity with the clone path) and is never registered in the parent's +`llm_registry`. Parent attribution via `_update_parent_metrics` +(keyed `task:{id}`) is untouched; the parent's LLM object is never mutated. + +## Tests (14 new; tests/tools/task 65 → 76; zero regressions) + +943 passed across the commanded suites (`tests/tools/task` 76 + +`tests/tools/test_tool_name_consistency.py` 3 + `tests/sdk/subagent` + +`tests/sdk/tool/test_switch_llm.py` 125 combined + `tests/sdk/conversation` +739). The 14 new tests: 11 under `tests/tools/task/` (7 manager-level, +4 tool-set-level) + 3 in `tests/sdk/conversation/test_switch_model.py`. + +- `tests/tools/task/test_task_manager.py::TestTaskManagerLLMProfile` — override + applied + `stream is False`; unknown profile raises listing available + profiles with no `_tasks` leak; pin fully ignores override (unknown name does + not error, and the ignored value is not stored on the task); resume keeps + original profile; explicit resume overrides and re-stores; inherit-created + task keeps inheriting on bare resume. +- `tests/tools/task/test_task_tool_set.py` — sequential mixed-model pair + (factories received exactly `["fast-model", "slow-model"]`); unknown profile → + error observation naming the profile; description renders profile list when + profiles exist; description omits the section when the store is empty. +- `tests/sdk/conversation/test_switch_model.py` — `load_profile_llm` decrypts + with the conversation cipher and stays out of `llm_registry`; unknown profile + lists available from the same store; `profile_store_dir` override resolves + from the custom dir only. +- `tests/tools/task/conftest.py` — redirects the profile-store dir to tmp AND + clears the registry's lru_cached store getter on setup/teardown (a cached + store bound to a dead tmp dir must not leak across tests). +- Full `tests/tools`: terminal/tmux failures are pre-existing environment + flakiness (clean tree fails the same directory as a superset). +- `pre-commit run --files `: all hooks pass (incl. pyright). + +## Live verification (2026-08-16, real profiles on laptop: kimi/mimo/minimax) + +Function-level: parent `minimax` (MiniMax-M3); `start_task(llm_profile="mimo")` +→ worker `mimo-v2.5-pro`, completed; no-override → worker == parent model; +unknown profile → clean `ValueError`, no task-state leak. + +Agent-level (the real UX): live parent agent on MiniMax-M3, handed only +`TaskToolSet`, prompted Rajiv-style ("delegate to a general-purpose subagent … +run it on the mimo LLM profile, do not switch your own model"). Event stream +shows the parent **chose** to emit `"llm_profile": "mimo"`; worker ran on +`openai/mimo-v2.5-pro`; parent's model unchanged; synthesis returned to the +parent. Demo scripts: `/tmp/llm-profile-demo.py`, `/tmp/llm-profile-demo-agentic.py`. + +## Known limitations + +- Fan-out still sequential by default (`tool_concurrency_limit=1`). +- Profile list in the tool description is a creation-time snapshot, and it is + built from the DEFAULT profile store; a definition with a custom + `profile_store_dir` executes from that store, so the advertised list and the + resolvable set can differ there. +- Old SDK versions reject new events containing the field (`extra="forbid"` — + standard additive-field version-skew caveat; note in release notes). + +## Maintainer-reception risk (stated honestly) + +In the same Slack thread, Graham Neubig steered toward the conversation-spawn +path (the `agent-canvas-environment` skill, "delegate to a local conversation") +and said "we should make that easier." This PR is the *in-conversation* fix +instead. The answer to "why not the conversation path" is fan-out concurrency +and synthesis-in-parent — but a maintainer may still prefer the +conversation-level direction. This is the chief rejection risk and the main +thing the pre-public panel should attack. diff --git a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py index fb4316046d..f942dd5c70 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py @@ -1720,6 +1720,40 @@ def get_or_create_profile_llm(self, profile_name: str, usage_id: str) -> LLM: self._bind_conversation_context(llm) return llm + def load_profile_llm( + self, profile_name: str, profile_store_dir: str | None = None + ) -> LLM: + """Load a saved profile LLM without registering or activating it. + + Unlike :meth:`get_or_create_profile_llm`, the returned LLM is not added + to ``llm_registry`` and no subscription transforms are applied, so the + caller owns metrics attribution (e.g. per-subagent-task accounting). + Secrets are decrypted with the conversation's cipher, matching + :meth:`switch_profile`. + + Args: + profile_name: Name of a profile previously saved via + LLMProfileStore (a ``.json`` suffix is tolerated). + profile_store_dir: Optional override for the store directory. When + None, the conversation's default store is used. + + Raises: + ValueError: If the profile does not exist in the store. + """ + store = ( + self._profile_store + if profile_store_dir is None + else LLMProfileStore(profile_store_dir) + ) + name = profile_name.removesuffix(".json") + available = [n.removesuffix(".json") for n in store.list()] + if name not in available: + raise ValueError( + f"Profile '{profile_name}' not found in profile store.\n" + f"Available profiles: {available}" + ) + return store.load(name, cipher=self._cipher) + def switch_acp_model(self, model: str) -> None: """Switch the model on an ACP conversation. diff --git a/openhands-sdk/openhands/sdk/subagent/AGENTS.md b/openhands-sdk/openhands/sdk/subagent/AGENTS.md index bd18357299..4789870f4a 100644 --- a/openhands-sdk/openhands/sdk/subagent/AGENTS.md +++ b/openhands-sdk/openhands/sdk/subagent/AGENTS.md @@ -111,6 +111,8 @@ Supported YAML frontmatter keys (see `AgentDefinition.load` in `schema.py`): - `skills` (default: `[]`): a comma-separated string or a list of skill names - `model` (default: `inherit`): `inherit` reuses the parent LLM; another value is loaded as an LLM profile name from `profile_store_dir` or the default profile store + - a pinned `model:` takes precedence over the task tool's per-call + `llm_profile` override (which applies only to `model: inherit` definitions) - `color` (optional) - `max_iteration_per_run` (optional, positive integer) - `max_budget_per_run` (optional, positive number in USD) diff --git a/openhands-sdk/openhands/sdk/tool/builtins/switch_llm.py b/openhands-sdk/openhands/sdk/tool/builtins/switch_llm.py index 99e016f5f0..7921baf783 100644 --- a/openhands-sdk/openhands/sdk/tool/builtins/switch_llm.py +++ b/openhands-sdk/openhands/sdk/tool/builtins/switch_llm.py @@ -88,7 +88,8 @@ def get_llm_profile_names() -> list[str]: return [summary["name"] for summary in LLMProfileStore().list_summaries()] -def _format_profiles(profile_names: Sequence[str]) -> str: +def format_llm_profiles(profile_names: Sequence[str]) -> str: + """Format saved LLM profile names as a bullet list for tool descriptions.""" if not profile_names: return "- No saved LLM profiles are currently available." return "\n".join(f"- {name}" for name in sorted(profile_names)) @@ -164,7 +165,7 @@ def create( return [ cls( description=_DESCRIPTION_TEMPLATE.format( - profiles=_format_profiles(profile_names) + profiles=format_llm_profiles(profile_names) ), action_type=SwitchLLMAction, observation_type=SwitchLLMObservation, diff --git a/openhands-tools/openhands/tools/task/definition.py b/openhands-tools/openhands/tools/task/definition.py index f1a2d335f3..bb70ed8fb2 100644 --- a/openhands-tools/openhands/tools/task/definition.py +++ b/openhands-tools/openhands/tools/task/definition.py @@ -26,6 +26,10 @@ ToolDefinition, register_tool, ) +from openhands.sdk.tool.builtins.switch_llm import ( + format_llm_profiles, + get_llm_profile_names, +) if TYPE_CHECKING: @@ -48,6 +52,19 @@ class TaskAction(Action): default="general-purpose", description="The type of specialized agent to use for this task.", ) + llm_profile: str | None = Field( + default=None, + description=( + "Name of a saved LLM profile to run this subagent on, instead of " + "inheriting your current model. Use this for mixed-model workflows, " + "e.g. delegating implementation to a cheaper or faster profile while " + "you continue planning on your current model. This affects only the " + "delegated subagent — your own model is unchanged (use the switch_llm " + "tool to change your own model). If the chosen subagent_type's " + "definition pins its own model, that pin takes precedence and this " + "field is ignored." + ), + ) resume: str | None = Field( default=None, description="Task ID of the task to resume from.", @@ -117,7 +134,7 @@ def to_llm_content(self) -> Sequence[TextContent | ImageContent]: Available agent types and the tools they have access to: {agent_types_info} -When NOT to use the task tool: +{llm_profiles_section}When NOT to use the task tool: - A single grep, find, or cat command would answer your question — just run it yourself - You are making a file edit (use file_editor directly) - You already have the context needed @@ -235,6 +252,17 @@ def create( agent_types_info = get_factory_info() + profile_names = get_llm_profile_names() + llm_profiles_section = "" + if profile_names: + llm_profiles_section = ( + "To run a subagent on a different model than your own, pass " + "llm_profile with one of these saved LLM profiles:\n" + f"{format_llm_profiles(profile_names)}\n" + "This affects only the delegated subagent; " + "your own model is unchanged.\n\n" + ) + registered = {d.name for d in get_registered_agent_definitions()} task_tool_examples = "\n".join( ex for name, ex in TASK_TOOL_EXAMPLES.items() if name in registered @@ -242,6 +270,7 @@ def create( task_description = TASK_TOOL_DESCRIPTION.format( agent_types_info=agent_types_info, + llm_profiles_section=llm_profiles_section, task_tool_examples=task_tool_examples, ) diff --git a/openhands-tools/openhands/tools/task/impl.py b/openhands-tools/openhands/tools/task/impl.py index eeee58c023..e15bc2cb59 100644 --- a/openhands-tools/openhands/tools/task/impl.py +++ b/openhands-tools/openhands/tools/task/impl.py @@ -35,6 +35,7 @@ def __call__( description=action.description, resume=action.resume, conversation=conversation, + llm_profile=action.llm_profile, ) match task.status: case TaskStatus.COMPLETED: diff --git a/openhands-tools/openhands/tools/task/manager.py b/openhands-tools/openhands/tools/task/manager.py index a8b52a1d60..13adf051ba 100644 --- a/openhands-tools/openhands/tools/task/manager.py +++ b/openhands-tools/openhands/tools/task/manager.py @@ -48,6 +48,13 @@ _SUBAGENTS_DIR: Final[str] = "subagents" +def _has_pinned_model(factory: "AgentFactory") -> bool: + """True when the definition pins its own model (``model:`` set, not + ``inherit``) — the factory then loads its own profile and any per-call + llm_profile override is ignored.""" + return bool(factory.definition.model) and factory.definition.model != "inherit" + + class TaskStatus(StrEnum): """Represents the lifecycle states of a task.""" @@ -73,6 +80,13 @@ class Task(BaseModel): ) result: str | None = Field(default=None, description="Result of the task.") error: str | None = Field(default=None, description="Error if task failed.") + llm_profile: str | None = Field( + default=None, + description="Effective LLM profile the task's worker runs on. Set at " + "creation from the per-call override (None when the definition pins " + "its own model); a bare resume keeps it, an explicit resume-time " + "override replaces it.", + ) conversation: LocalConversation | None = Field( default=None, exclude=True, @@ -168,6 +182,7 @@ def start_task( resume: str | None = None, description: str | None = None, conversation: LocalConversation | None = None, + llm_profile: str | None = None, ) -> Task: """Start a blocking sub-agent task. @@ -177,6 +192,11 @@ def start_task( resume: Task ID to resume (continues existing conversation). description: Short label for the task. conversation: Parent conversation (set on first call). + llm_profile: Optional saved LLM profile to run the sub-agent on + instead of inheriting the parent's model. A ``model:`` pin in + the sub-agent definition takes precedence over this. On resume, + omitting it keeps the profile the task was created with; + passing one explicitly switches the resumed worker to it. Returns: TaskState with the final result. @@ -188,11 +208,13 @@ def start_task( task = self._resume_task( resume=resume, subagent_type=subagent_type, + llm_profile=llm_profile, ) else: task = self._create_task( subagent_type=subagent_type, description=description, + llm_profile=llm_profile, ) return self._run_task( @@ -200,8 +222,14 @@ def start_task( prompt=prompt, ) - def _resume_task(self, resume: str, subagent_type: str) -> Task: - """Resume a sub-agent task.""" + def _resume_task( + self, resume: str, subagent_type: str, llm_profile: str | None = None + ) -> Task: + """Resume a sub-agent task. + + A None llm_profile keeps the profile the task was created with; an + explicit one overrides it (and becomes the task's new profile). + """ with self._tasks_lock: if resume not in self._tasks: raise ValueError( @@ -209,8 +237,14 @@ def _resume_task(self, resume: str, subagent_type: str) -> Task: f"Available tasks: {', '.join(sorted(self._tasks))}" ) + stored = self._tasks[resume] + effective_profile = ( + llm_profile if llm_profile is not None else stored.llm_profile + ) factory = get_agent_factory(subagent_type) - worker_agent = self._get_sub_agent_from_factory(factory) + worker_agent = self._get_sub_agent_from_factory( + factory, llm_profile=effective_profile + ) conversation_id = self._tasks[resume].conversation_id with detached_delegate_context() as link: conversation = LocalConversation( @@ -231,10 +265,11 @@ def _resume_task(self, resume: str, subagent_type: str) -> Task: factory.definition.get_confirmation_policy(), ) - self._tasks[resume] = self._tasks[resume].model_copy( + self._tasks[resume] = stored.model_copy( update={ "conversation": conversation, "status": TaskStatus.RUNNING, + "llm_profile": effective_profile, } ) @@ -244,6 +279,7 @@ def _create_task( self, subagent_type: str, description: str | None, + llm_profile: str | None = None, ) -> Task: """Create a fresh task. @@ -252,7 +288,9 @@ def _create_task( 2. The parent conversation's ``max_iteration_per_run`` """ factory = get_agent_factory(subagent_type) - worker_agent = self._get_sub_agent_from_factory(factory) + worker_agent = self._get_sub_agent_from_factory( + factory, llm_profile=llm_profile + ) effective_max_iter = ( factory.definition.max_iteration_per_run @@ -289,6 +327,10 @@ def _create_task( conversation_id=conversation_id, conversation=sub_conversation, status=TaskStatus.RUNNING, + # Store the effective profile: under a pinned definition the + # override is ignored, so nothing is stored for a later bare + # resume to act on. + llm_profile=None if _has_pinned_model(factory) else llm_profile, ) return self._tasks[task_id] @@ -349,25 +391,41 @@ def _delegate_observability_metadata( **link, } - def _get_sub_agent(self, subagent_type: str) -> Agent: + def _get_sub_agent( + self, subagent_type: str, llm_profile: str | None = None + ) -> Agent: """Return the subagent assigned to the task. Raises: ValueError: If the subagent type is invalid. """ factory = get_agent_factory(subagent_type) - return self._get_sub_agent_from_factory(factory) + return self._get_sub_agent_from_factory(factory, llm_profile=llm_profile) - def _get_sub_agent_from_factory(self, factory: "AgentFactory") -> Agent: + def _get_sub_agent_from_factory( + self, factory: "AgentFactory", llm_profile: str | None = None + ) -> Agent: """Create a sub-agent from an AgentFactory.""" parent = self.parent_conversation parent_llm = parent.agent.llm - llm_updates: dict = {"stream": False} - sub_agent_llm = parent_llm.model_copy(update=llm_updates) - # Reset metrics such that the sub-agent has its own - # Metrics object - sub_agent_llm.reset_metrics() + # A definition with a pinned model: loads its own profile inside + # factory_func, so the per-call override is skipped entirely (even an + # unknown name must not error). + pinned = _has_pinned_model(factory) + if llm_profile is not None and not pinned: + # Injected pre-factory so the factory derives the default condenser + # LLM from it (same semantics as a definition's model: pin). + sub_agent_llm = parent.load_profile_llm( + llm_profile, + profile_store_dir=factory.definition.profile_store_dir, + ) + sub_agent_llm.reset_metrics() + else: + sub_agent_llm = parent_llm.model_copy(update={"stream": False}) + # Reset metrics such that the sub-agent has its own + # Metrics object + sub_agent_llm.reset_metrics() sub_agent = factory.factory_func(sub_agent_llm) diff --git a/tests/sdk/conversation/test_switch_model.py b/tests/sdk/conversation/test_switch_model.py index 75772e2c2f..f076122a35 100644 --- a/tests/sdk/conversation/test_switch_model.py +++ b/tests/sdk/conversation/test_switch_model.py @@ -594,6 +594,71 @@ def test_switch_profile_decrypts_with_cipher(tmp_path, monkeypatch): assert api_key.get_secret_value() == "plaintext-secret" +def test_load_profile_llm_decrypts_with_cipher(tmp_path, monkeypatch): + """load_profile_llm decrypts secrets with the conversation's cipher (like + switch_profile) but does NOT register the LLM in llm_registry — the caller + (e.g. the task tool) owns metrics attribution. + """ + profile_dir = tmp_path / "profiles" + profile_dir.mkdir() + monkeypatch.setattr(llm_profile_store, "_DEFAULT_PROFILE_DIR", profile_dir) + + cipher = Cipher("test-key-for-load-profile-llm") + store = LLMProfileStore(base_dir=profile_dir) + store.save( + "encrypted", + LLM( + model="gpt-4o", + usage_id="encrypted", + api_key=SecretStr("plaintext-secret"), + ), + include_secrets=True, + cipher=cipher, + ) + + conv = LocalConversation( + agent=Agent( + llm=_make_llm("default-model", "test-llm"), + tools=[], + ), + workspace=Path.cwd(), + cipher=cipher, + ) + + llm = conv.load_profile_llm("encrypted") + + assert isinstance(llm.api_key, SecretStr) + assert llm.api_key.get_secret_value() == "plaintext-secret" + with pytest.raises(KeyError): + conv.llm_registry.get(llm.usage_id) + + +def test_load_profile_llm_unknown_lists_available(profile_store): + """Unknown profile raises ValueError listing what the same store has.""" + conv = _make_conversation() + with pytest.raises(ValueError, match="not found") as excinfo: + conv.load_profile_llm("missing") + assert "fast" in str(excinfo.value) + + +def test_load_profile_llm_custom_store_dir(tmp_path, monkeypatch): + """profile_store_dir resolves from that dir instead of the default store.""" + monkeypatch.setattr( + llm_profile_store, "_DEFAULT_PROFILE_DIR", tmp_path / "default-profiles" + ) + custom_dir = tmp_path / "custom-profiles" + store = LLMProfileStore(base_dir=custom_dir) + store.save("custom", _make_llm("custom-model", "custom"), include_secrets=True) + + conv = _make_conversation() + + llm = conv.load_profile_llm("custom", profile_store_dir=str(custom_dir)) + assert llm.model == "custom-model" + + with pytest.raises(ValueError, match="not found"): + conv.load_profile_llm("custom") + + def test_switch_profile_delegates_to_switch_llm(profile_store, monkeypatch): """switch_profile loads from disk and delegates to switch_llm; the LLM handed off carries the canonical ``profile:{name}`` usage_id. diff --git a/tests/tools/task/conftest.py b/tests/tools/task/conftest.py new file mode 100644 index 0000000000..902866a7f9 --- /dev/null +++ b/tests/tools/task/conftest.py @@ -0,0 +1,21 @@ +import pytest + +from openhands.sdk.llm import llm_profile_store +from openhands.sdk.subagent import registry + + +@pytest.fixture(autouse=True) +def _isolate_llm_profile_store(tmp_path, monkeypatch): + """Redirect the default LLM profile dir so tests never touch the real + ~/.openhands/profiles (TaskToolSet.create lists saved profiles). + + The registry's profile-store getter is lru_cached, so it is cleared before + and after each test: a store cached under an earlier test's (now-deleted) + tmp dir must not leak into the next test. + """ + registry._get_profile_store.cache_clear() + monkeypatch.setattr( + llm_profile_store, "_DEFAULT_PROFILE_DIR", tmp_path / "profiles" + ) + yield + registry._get_profile_store.cache_clear() diff --git a/tests/tools/task/test_task_manager.py b/tests/tools/task/test_task_manager.py index e459f42ebe..39cdfda18d 100644 --- a/tests/tools/task/test_task_manager.py +++ b/tests/tools/task/test_task_manager.py @@ -10,6 +10,7 @@ from openhands.sdk.conversation.impl.local_conversation import LocalConversation from openhands.sdk.conversation.state import ConversationExecutionStatus from openhands.sdk.hooks.config import HookConfig, HookDefinition, HookMatcher +from openhands.sdk.llm.llm_profile_store import LLMProfileStore from openhands.sdk.subagent.registry import ( _reset_registry_for_tests, register_agent, @@ -31,6 +32,18 @@ def _make_llm() -> LLM: ) +def _make_profile_store(base_dir: Path, profiles: dict[str, str]) -> LLMProfileStore: + """Save one LLM profile per name -> model mapping into a store.""" + store = LLMProfileStore(base_dir=base_dir / "profiles") + for name, model in profiles.items(): + store.save( + name, + LLM(model=model, api_key=SecretStr(f"{name}-key"), usage_id=name), + include_secrets=True, + ) + return store + + def _make_parent_conversation( tmp_path: Path, persistence_dir: str | Path | None = None, @@ -525,6 +538,177 @@ def test_delegate_metadata_includes_parent_span_link(self, tmp_path): } +class TestTaskManagerLLMProfile: + """Tests for the per-call llm_profile override on task creation/resume. + + The conftest redirects the default profile-store dir to ``tmp_path``, so + ``_make_profile_store(tmp_path, ...)`` writes exactly where the parent + conversation's store (and the registry's cached getter) resolves. + """ + + def setup_method(self): + _reset_registry_for_tests() + + def teardown_method(self): + _reset_registry_for_tests() + + def test_create_task_with_llm_profile(self, tmp_path): + """A saved profile's LLM replaces the parent-inherited one.""" + manager, _ = _manager_with_parent(tmp_path) + register_builtins_agents() + _make_profile_store(tmp_path, {"fast": "fast-model"}) + + task = manager._create_task( + subagent_type="general-purpose", + description=None, + llm_profile="fast", + ) + + assert task.llm_profile == "fast" + assert task.conversation is not None + assert task.conversation.agent.llm.model == "fast-model" + assert task.conversation.agent.llm.stream is False + + def test_create_task_unknown_profile_raises(self, tmp_path): + """Unknown profiles raise before any task state is created.""" + manager, _ = _manager_with_parent(tmp_path) + register_builtins_agents() + _make_profile_store(tmp_path, {"fast": "fast-model"}) + + with pytest.raises(ValueError, match="not found") as excinfo: + manager._create_task( + subagent_type="general-purpose", + description=None, + llm_profile="missing", + ) + + assert "fast" in str(excinfo.value) # available profiles are listed + assert len(manager._tasks) == 0 + + def test_definition_model_pin_beats_llm_profile(self, tmp_path): + """A definition's pinned model: fully ignores the per-call llm_profile — + even a name absent from the store must not raise, and the ignored value + is not stored on the task (a bare resume must never try to load it, + e.g. if the definition is later flipped to inherit).""" + from openhands.sdk.subagent.registry import agent_definition_to_factory + + agent_def = AgentDefinition( + name="pinned_agent", + description="Agent with a pinned model", + model="pinned", + tools=[], + system_prompt="You are pinned.", + ) + register_agent( + name="pinned_agent", + factory_func=agent_definition_to_factory(agent_def), + description=agent_def, + ) + + manager, _ = _manager_with_parent(tmp_path) + _make_profile_store(tmp_path, {"pinned": "pinned-model"}) + + task = manager._create_task( + subagent_type="pinned_agent", + description=None, + llm_profile="typo-does-not-exist", + ) + + assert task.llm_profile is None + assert task.conversation is not None + assert task.conversation.agent.llm.model == "pinned-model" + + def test_resume_with_llm_profile(self, tmp_path): + """Resuming with llm_profile rebuilds the worker on that profile.""" + manager, _ = _manager_with_parent(tmp_path) + register_builtins_agents() + _make_profile_store(tmp_path, {"fast": "fast-model"}) + + task = manager._create_task(subagent_type="general-purpose", description=None) + original_id = task.id + manager._evict_task(task) + + resumed = manager._resume_task( + resume=original_id, + subagent_type="general-purpose", + llm_profile="fast", + ) + + assert resumed.id == original_id + assert resumed.llm_profile == "fast" + assert resumed.conversation is not None + assert resumed.conversation.agent.llm.model == "fast-model" + assert resumed.conversation.agent.llm.stream is False + + def test_resume_without_profile_keeps_original(self, tmp_path): + """A bare resume keeps the profile the task was created with.""" + manager, _ = _manager_with_parent(tmp_path) + register_builtins_agents() + _make_profile_store(tmp_path, {"fast": "fast-model"}) + + task = manager._create_task( + subagent_type="general-purpose", description=None, llm_profile="fast" + ) + original_id = task.id + manager._evict_task(task) + + resumed = manager._resume_task( + resume=original_id, subagent_type="general-purpose" + ) + + assert resumed.llm_profile == "fast" + assert resumed.conversation is not None + assert resumed.conversation.agent.llm.model == "fast-model" + + def test_resume_with_explicit_profile_overrides_stored(self, tmp_path): + """An explicit resume-time profile replaces the stored one for later + bare resumes.""" + manager, _ = _manager_with_parent(tmp_path) + register_builtins_agents() + _make_profile_store(tmp_path, {"fast": "fast-model", "slow": "slow-model"}) + + task = manager._create_task( + subagent_type="general-purpose", description=None, llm_profile="fast" + ) + original_id = task.id + manager._evict_task(manager._tasks[original_id]) + + resumed = manager._resume_task( + resume=original_id, + subagent_type="general-purpose", + llm_profile="slow", + ) + assert resumed.llm_profile == "slow" + assert resumed.conversation is not None + assert resumed.conversation.agent.llm.model == "slow-model" + + manager._evict_task(manager._tasks[original_id]) + resumed_again = manager._resume_task( + resume=original_id, subagent_type="general-purpose" + ) + assert resumed_again.llm_profile == "slow" + assert resumed_again.conversation is not None + assert resumed_again.conversation.agent.llm.model == "slow-model" + + def test_resume_after_inherit_created_still_inherits(self, tmp_path): + """A task created without llm_profile keeps inheriting the parent + model on bare resumes.""" + manager, _ = _manager_with_parent(tmp_path) + register_builtins_agents() + + task = manager._create_task(subagent_type="general-purpose", description=None) + original_id = task.id + manager._evict_task(task) + + resumed = manager._resume_task( + resume=original_id, subagent_type="general-purpose" + ) + + assert resumed.llm_profile is None + assert resumed.conversation is not None + assert resumed.conversation.agent.llm.model == "gpt-4o" + + def _make_task_with_mock_conv(task_id: str, **conv_kwargs) -> Task: """Create a Task with a MagicMock conversation, bypassing Pydantic validation.""" mock_conv = MagicMock(**conv_kwargs) diff --git a/tests/tools/task/test_task_tool_set.py b/tests/tools/task/test_task_tool_set.py index d12e16f397..a4bdb3aa76 100644 --- a/tests/tools/task/test_task_tool_set.py +++ b/tests/tools/task/test_task_tool_set.py @@ -1,9 +1,13 @@ import json +from unittest.mock import patch -from openhands.sdk import Agent, Conversation, LocalConversation, Tool +from pydantic import SecretStr + +from openhands.sdk import LLM, Agent, Conversation, LocalConversation, Tool from openhands.sdk.conversation.state import ConversationExecutionStatus from openhands.sdk.event.llm_convertible.observation import ObservationEvent from openhands.sdk.llm import Message, MessageToolCall, TextContent +from openhands.sdk.llm.llm_profile_store import LLMProfileStore from openhands.sdk.subagent.registry import _reset_registry_for_tests, register_agent from openhands.sdk.testing import TestLLM from openhands.tools.task import TaskToolSet @@ -17,6 +21,7 @@ def _task_tool_call( subagent_type: str = "test_agent", description: str | None = None, resume: str | None = None, + llm_profile: str | None = None, ) -> Message: """Build a Message whose only tool call is the task tool.""" args: dict = { @@ -27,6 +32,8 @@ def _task_tool_call( args["description"] = description if resume is not None: args["resume"] = resume + if llm_profile is not None: + args["llm_profile"] = llm_profile return Message( role="assistant", @@ -152,6 +159,77 @@ def test_two_sequential_tasks(self, tmp_path): assert observations[0].subagent == "agent_a" assert observations[1].subagent == "agent_b" + def test_two_sequential_tasks_with_llm_profiles(self, tmp_path): + """Two sequential tasks can each run on a different saved LLM profile.""" + # The conftest redirects the default profile dir to tmp_path / "profiles", + # which is where the parent conversation's profile store resolves. + store = LLMProfileStore(base_dir=tmp_path / "profiles") + for name, model in (("fast", "fast-model"), ("slow", "slow-model")): + store.save( + name, + LLM(model=model, api_key=SecretStr(f"{name}-key"), usage_id=name), + include_secrets=True, + ) + + factory_llms: list[LLM] = [] + + def make_factory(sub_llm: TestLLM): + def factory(llm: LLM) -> Agent: + factory_llms.append(llm) + return Agent(llm=sub_llm, tools=[]) + + return factory + + register_agent( + name="agent_a", + factory_func=make_factory( + TestLLM.from_messages([_text_message("first result")]) + ), + description="Test agent: agent_a", + ) + register_agent( + name="agent_b", + factory_func=make_factory( + TestLLM.from_messages([_text_message("second result")]) + ), + description="Test agent: agent_b", + ) + + parent_llm = TestLLM.from_messages( + [ + _task_tool_call( + "call_1", + prompt="Task A", + subagent_type="agent_a", + llm_profile="fast", + ), + _task_tool_call( + "call_2", + prompt="Task B", + subagent_type="agent_b", + llm_profile="slow", + ), + _text_message("Both tasks done."), + ] + ) + + agent = Agent(llm=parent_llm, tools=[Tool(name=TaskToolSet.name)]) + conversation = Conversation( + agent=agent, workspace=str(tmp_path), visualizer=None + ) + + conversation.send_message("Run two tasks") + conversation.run() + + assert ( + conversation.state.execution_status == ConversationExecutionStatus.FINISHED + ) + observations = _get_task_observations(conversation) + assert len(observations) == 2 + assert observations[0].text == "first result" + assert observations[1].text == "second result" + assert [llm.model for llm in factory_llms] == ["fast-model", "slow-model"] + def test_task_resume_across_turns(self, tmp_path): """A task can be launched, then resumed by passing the task_id.""" # Sub-agent for the first call @@ -239,6 +317,40 @@ def test_unknown_agent_type_returns_error_observation(self, tmp_path): assert obs.is_error is True assert "nonexistent_agent" in obs.text or "Unknown agent" in obs.text + def test_unknown_llm_profile_returns_error_observation(self, tmp_path): + """An unknown llm_profile yields an error TaskObservation naming it.""" + sub_llm = TestLLM.from_messages([_text_message("never reached")]) + _register_simple_agent("test_agent", sub_llm) + + parent_llm = TestLLM.from_messages( + [ + _task_tool_call( + "call_1", + prompt="Do something", + subagent_type="test_agent", + llm_profile="nonexistent_profile", + ), + _text_message("Oops."), + ] + ) + + agent = Agent(llm=parent_llm, tools=[Tool(name=TaskToolSet.name)]) + conversation = Conversation( + agent=agent, workspace=str(tmp_path), visualizer=None + ) + + conversation.send_message("Do something") + conversation.run() + + assert ( + conversation.state.execution_status == ConversationExecutionStatus.FINISHED + ) + observations = _get_task_observations(conversation) + assert len(observations) == 1 + obs = observations[0] + assert obs.is_error is True + assert "nonexistent_profile" in obs.text + def test_sub_agent_exception_returns_error_observation(self, tmp_path): """When the sub-agent's LLM raises, the task reports an error.""" sub_llm = TestLLM.from_messages( @@ -387,6 +499,34 @@ def test_no_matching_agent_example_excluded(self, tmp_path): for name, example_text in TASK_TOOL_EXAMPLES.items(): assert example_text.strip() not in description + def test_description_lists_llm_profiles(self, tmp_path): + """The tool description lists saved LLM profiles for llm_profile.""" + with patch( + "openhands.tools.task.definition.get_llm_profile_names", + return_value=["fast", "slow"], + ): + tools = TaskToolSet.create( + conv_state=None, # type: ignore[arg-type] + ) + description = tools[0].description + assert "llm_profile" in description + assert "- fast" in description + assert "- slow" in description + + def test_description_without_profiles_omits_section(self, tmp_path): + """With no saved profiles, the llm_profile section is omitted entirely + rather than rendering an incoherent "one of these: none" list.""" + with patch( + "openhands.tools.task.definition.get_llm_profile_names", + return_value=[], + ): + tools = TaskToolSet.create( + conv_state=None, # type: ignore[arg-type] + ) + description = tools[0].description + assert "llm_profile" not in description + assert "saved LLM profiles" not in description + def test_only_registered_examples_included(self, tmp_path): """Only examples for registered agents appear; others are excluded.""" keys = list(TASK_TOOL_EXAMPLES.keys()) From f1c1cb167e18d784d27227513fcfd2ec5d5f9873 Mon Sep 17 00:00:00 2001 From: george larson Date: Wed, 26 Aug 2026 05:26:13 -0400 Subject: [PATCH 2/7] refactor(sdk): share cipher-aware profile loading Co-authored-by: openhands --- .../conversation/impl/local_conversation.py | 25 ++++------- tests/sdk/conversation/test_switch_model.py | 42 +++++++++++++++++-- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py index f942dd5c70..77328907c0 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py @@ -1698,7 +1698,7 @@ def switch_profile(self, profile_name: str) -> None: try: cached = self.llm_registry.get(usage_id) except KeyError: - loaded = self._profile_store.load(profile_name, cipher=self._cipher) + loaded = self.load_profile_llm(profile_name) cached = loaded.model_copy(update={"usage_id": usage_id}) self.switch_llm(cached) @@ -1713,7 +1713,7 @@ def get_or_create_profile_llm(self, profile_name: str, usage_id: str) -> LLM: try: return self.llm_registry.get(usage_id) except KeyError: - loaded = self._profile_store.load(profile_name, cipher=self._cipher) + loaded = self.load_profile_llm(profile_name) llm = loaded.model_copy(update={"usage_id": usage_id}) llm = create_subscription_llm_from_config(llm) self.llm_registry.add(llm) @@ -1725,11 +1725,10 @@ def load_profile_llm( ) -> LLM: """Load a saved profile LLM without registering or activating it. - Unlike :meth:`get_or_create_profile_llm`, the returned LLM is not added - to ``llm_registry`` and no subscription transforms are applied, so the - caller owns metrics attribution (e.g. per-subagent-task accounting). - Secrets are decrypted with the conversation's cipher, matching - :meth:`switch_profile`. + The returned LLM is not added to ``llm_registry`` or made active, so the + caller owns its lifecycle and metrics attribution. Persisted subscription + profiles are restored by :class:`LLMProfileStore` during the load. + Secrets are decrypted with the conversation's cipher. Args: profile_name: Name of a profile previously saved via @@ -1738,21 +1737,15 @@ def load_profile_llm( None, the conversation's default store is used. Raises: - ValueError: If the profile does not exist in the store. + FileNotFoundError: If the profile does not exist. + ValueError: If the profile is corrupted or invalid. """ store = ( self._profile_store if profile_store_dir is None else LLMProfileStore(profile_store_dir) ) - name = profile_name.removesuffix(".json") - available = [n.removesuffix(".json") for n in store.list()] - if name not in available: - raise ValueError( - f"Profile '{profile_name}' not found in profile store.\n" - f"Available profiles: {available}" - ) - return store.load(name, cipher=self._cipher) + return store.load(profile_name, cipher=self._cipher) def switch_acp_model(self, model: str) -> None: """Switch the model on an ACP conversation. diff --git a/tests/sdk/conversation/test_switch_model.py b/tests/sdk/conversation/test_switch_model.py index f076122a35..cf953693e7 100644 --- a/tests/sdk/conversation/test_switch_model.py +++ b/tests/sdk/conversation/test_switch_model.py @@ -629,14 +629,15 @@ def test_load_profile_llm_decrypts_with_cipher(tmp_path, monkeypatch): assert isinstance(llm.api_key, SecretStr) assert llm.api_key.get_secret_value() == "plaintext-secret" + assert conv.agent.llm.model == "default-model" with pytest.raises(KeyError): conv.llm_registry.get(llm.usage_id) def test_load_profile_llm_unknown_lists_available(profile_store): - """Unknown profile raises ValueError listing what the same store has.""" + """Unknown profile uses the store's native error and lists its profiles.""" conv = _make_conversation() - with pytest.raises(ValueError, match="not found") as excinfo: + with pytest.raises(FileNotFoundError, match="not found") as excinfo: conv.load_profile_llm("missing") assert "fast" in str(excinfo.value) @@ -655,10 +656,45 @@ def test_load_profile_llm_custom_store_dir(tmp_path, monkeypatch): llm = conv.load_profile_llm("custom", profile_store_dir=str(custom_dir)) assert llm.model == "custom-model" - with pytest.raises(ValueError, match="not found"): + with pytest.raises(FileNotFoundError, match="not found"): conv.load_profile_llm("custom") +def test_load_profile_llm_restores_subscription_profile(tmp_path, monkeypatch): + """Persisted subscription profiles are restored during the store load.""" + profile_dir = tmp_path / "profiles" + monkeypatch.setattr(llm_profile_store, "_DEFAULT_PROFILE_DIR", profile_dir) + store = LLMProfileStore(base_dir=profile_dir) + store.save( + "subscription", + LLM( + model="gpt-5.2-codex", + usage_id="subscription", + auth_type="subscription", + subscription_vendor="openai", + ), + ) + + restored: list[LLM] = [] + + def _restore_subscription(llm: LLM) -> LLM: + restored.append(llm) + return llm.model_copy(update={"model": "runtime-subscription-model"}) + + monkeypatch.setattr( + "openhands.sdk.llm.auth.openai.create_subscription_llm_from_config", + _restore_subscription, + ) + + conv = _make_conversation() + llm = conv.load_profile_llm("subscription") + + assert len(restored) == 1 + assert restored[0].auth_type == "subscription" + assert llm.model == "runtime-subscription-model" + assert conv.agent.llm.model == "default-model" + + def test_switch_profile_delegates_to_switch_llm(profile_store, monkeypatch): """switch_profile loads from disk and delegates to switch_llm; the LLM handed off carries the canonical ``profile:{name}`` usage_id. From 09b6f76b529d29360836d909dd15af7d1e45fda2 Mon Sep 17 00:00:00 2001 From: george larson Date: Wed, 26 Aug 2026 05:26:13 -0400 Subject: [PATCH 3/7] fix(task): preserve isolated per-call profile behavior Co-authored-by: openhands --- .../openhands/sdk/subagent/AGENTS.md | 5 +- .../openhands/tools/task/definition.py | 11 +-- .../openhands/tools/task/manager.py | 39 ++++---- tests/tools/task/test_task_manager.py | 89 +++++++++++++++++-- tests/tools/task/test_task_tool_set.py | 22 +++++ 5 files changed, 131 insertions(+), 35 deletions(-) diff --git a/openhands-sdk/openhands/sdk/subagent/AGENTS.md b/openhands-sdk/openhands/sdk/subagent/AGENTS.md index 4789870f4a..894e980066 100644 --- a/openhands-sdk/openhands/sdk/subagent/AGENTS.md +++ b/openhands-sdk/openhands/sdk/subagent/AGENTS.md @@ -111,8 +111,9 @@ Supported YAML frontmatter keys (see `AgentDefinition.load` in `schema.py`): - `skills` (default: `[]`): a comma-separated string or a list of skill names - `model` (default: `inherit`): `inherit` reuses the parent LLM; another value is loaded as an LLM profile name from `profile_store_dir` or the default profile store - - a pinned `model:` takes precedence over the task tool's per-call - `llm_profile` override (which applies only to `model: inherit` definitions) + - an agent definition with its own model profile takes precedence over the task + tool's per-call `llm_profile` override; the override applies only to + `model: inherit` definitions - `color` (optional) - `max_iteration_per_run` (optional, positive integer) - `max_budget_per_run` (optional, positive number in USD) diff --git a/openhands-tools/openhands/tools/task/definition.py b/openhands-tools/openhands/tools/task/definition.py index bb70ed8fb2..636fececb4 100644 --- a/openhands-tools/openhands/tools/task/definition.py +++ b/openhands-tools/openhands/tools/task/definition.py @@ -55,14 +55,9 @@ class TaskAction(Action): llm_profile: str | None = Field( default=None, description=( - "Name of a saved LLM profile to run this subagent on, instead of " - "inheriting your current model. Use this for mixed-model workflows, " - "e.g. delegating implementation to a cheaper or faster profile while " - "you continue planning on your current model. This affects only the " - "delegated subagent — your own model is unchanged (use the switch_llm " - "tool to change your own model). If the chosen subagent_type's " - "definition pins its own model, that pin takes precedence and this " - "field is ignored." + "Saved LLM profile for this subagent. If omitted, the subagent " + "inherits the parent model. An agent definition with its own model " + "profile takes precedence." ), ) resume: str | None = Field( diff --git a/openhands-tools/openhands/tools/task/manager.py b/openhands-tools/openhands/tools/task/manager.py index 13adf051ba..e6b2642ca7 100644 --- a/openhands-tools/openhands/tools/task/manager.py +++ b/openhands-tools/openhands/tools/task/manager.py @@ -48,10 +48,8 @@ _SUBAGENTS_DIR: Final[str] = "subagents" -def _has_pinned_model(factory: "AgentFactory") -> bool: - """True when the definition pins its own model (``model:`` set, not - ``inherit``) — the factory then loads its own profile and any per-call - llm_profile override is ignored.""" +def _definition_has_model_profile(factory: "AgentFactory") -> bool: + """Return whether the agent definition selects its own model profile.""" return bool(factory.definition.model) and factory.definition.model != "inherit" @@ -83,8 +81,8 @@ class Task(BaseModel): llm_profile: str | None = Field( default=None, description="Effective LLM profile the task's worker runs on. Set at " - "creation from the per-call override (None when the definition pins " - "its own model); a bare resume keeps it, an explicit resume-time " + "creation from the per-call override (None when the agent definition " + "selects its own model); a bare resume keeps it, an explicit resume-time " "override replaces it.", ) conversation: LocalConversation | None = Field( @@ -238,10 +236,13 @@ def _resume_task( ) stored = self._tasks[resume] - effective_profile = ( + requested_profile = ( llm_profile if llm_profile is not None else stored.llm_profile ) factory = get_agent_factory(subagent_type) + effective_profile = ( + None if _definition_has_model_profile(factory) else requested_profile + ) worker_agent = self._get_sub_agent_from_factory( factory, llm_profile=effective_profile ) @@ -327,10 +328,11 @@ def _create_task( conversation_id=conversation_id, conversation=sub_conversation, status=TaskStatus.RUNNING, - # Store the effective profile: under a pinned definition the - # override is ignored, so nothing is stored for a later bare - # resume to act on. - llm_profile=None if _has_pinned_model(factory) else llm_profile, + # An agent definition's model takes precedence over the + # per-call override, including on a later bare resume. + llm_profile=( + None if _definition_has_model_profile(factory) else llm_profile + ), ) return self._tasks[task_id] @@ -409,13 +411,10 @@ def _get_sub_agent_from_factory( parent = self.parent_conversation parent_llm = parent.agent.llm - # A definition with a pinned model: loads its own profile inside - # factory_func, so the per-call override is skipped entirely (even an - # unknown name must not error). - pinned = _has_pinned_model(factory) - if llm_profile is not None and not pinned: + definition_has_model = _definition_has_model_profile(factory) + if llm_profile is not None and not definition_has_model: # Injected pre-factory so the factory derives the default condenser - # LLM from it (same semantics as a definition's model: pin). + # LLM from the selected profile. sub_agent_llm = parent.load_profile_llm( llm_profile, profile_store_dir=factory.definition.profile_store_dir, @@ -429,6 +428,12 @@ def _get_sub_agent_from_factory( sub_agent = factory.factory_func(sub_agent_llm) + # Subscription-backed LLMs do not support the separate completion + # flow used by an LLM summarizing condenser. This matches top-level + # agent creation and profile switching behavior. + if sub_agent.llm.is_subscription: + sub_agent = sub_agent.model_copy(update={"condenser": None}) + # ensuring that the sub-agent LLM has stream deactivated sub_agent = sub_agent.model_copy( update={"llm": sub_agent.llm.model_copy(update={"stream": False})} diff --git a/tests/tools/task/test_task_manager.py b/tests/tools/task/test_task_manager.py index 39cdfda18d..c5cb134492 100644 --- a/tests/tools/task/test_task_manager.py +++ b/tests/tools/task/test_task_manager.py @@ -554,9 +554,11 @@ def teardown_method(self): def test_create_task_with_llm_profile(self, tmp_path): """A saved profile's LLM replaces the parent-inherited one.""" - manager, _ = _manager_with_parent(tmp_path) + manager, parent = _manager_with_parent(tmp_path) register_builtins_agents() _make_profile_store(tmp_path, {"fast": "fast-model"}) + parent_model = parent.agent.llm.model + parent_metrics = parent.agent.llm.metrics task = manager._create_task( subagent_type="general-purpose", @@ -568,6 +570,9 @@ def test_create_task_with_llm_profile(self, tmp_path): assert task.conversation is not None assert task.conversation.agent.llm.model == "fast-model" assert task.conversation.agent.llm.stream is False + assert task.conversation.agent.llm.metrics is not parent_metrics + assert parent.agent.llm.model == parent_model + assert "profile:fast" not in parent.llm_registry.list_usage_ids() def test_create_task_unknown_profile_raises(self, tmp_path): """Unknown profiles raise before any task state is created.""" @@ -575,7 +580,7 @@ def test_create_task_unknown_profile_raises(self, tmp_path): register_builtins_agents() _make_profile_store(tmp_path, {"fast": "fast-model"}) - with pytest.raises(ValueError, match="not found") as excinfo: + with pytest.raises(FileNotFoundError, match="not found") as excinfo: manager._create_task( subagent_type="general-purpose", description=None, @@ -585,16 +590,17 @@ def test_create_task_unknown_profile_raises(self, tmp_path): assert "fast" in str(excinfo.value) # available profiles are listed assert len(manager._tasks) == 0 - def test_definition_model_pin_beats_llm_profile(self, tmp_path): - """A definition's pinned model: fully ignores the per-call llm_profile — - even a name absent from the store must not raise, and the ignored value - is not stored on the task (a bare resume must never try to load it, - e.g. if the definition is later flipped to inherit).""" + def test_definition_model_beats_llm_profile(self, tmp_path): + """A definition's model fully ignores a per-call ``llm_profile``. + + Even a name absent from the store must not raise, and the ignored value + is not stored for a later bare resume. + """ from openhands.sdk.subagent.registry import agent_definition_to_factory agent_def = AgentDefinition( name="pinned_agent", - description="Agent with a pinned model", + description="Agent with its own model profile", model="pinned", tools=[], system_prompt="You are pinned.", @@ -618,6 +624,24 @@ def test_definition_model_pin_beats_llm_profile(self, tmp_path): assert task.conversation is not None assert task.conversation.agent.llm.model == "pinned-model" + def test_subscription_profile_disables_worker_condenser(self, tmp_path): + """Subscription workers must not receive an LLM-backed condenser.""" + manager, parent = _manager_with_parent(tmp_path) + register_builtins_agents() + subscription_llm = _make_llm() + subscription_llm.is_subscription = True + + with patch.object(parent, "load_profile_llm", return_value=subscription_llm): + task = manager._create_task( + subagent_type="general-purpose", + description=None, + llm_profile="subscription", + ) + + assert task.conversation is not None + assert task.conversation.agent.llm.is_subscription is True + assert task.conversation.agent.condenser is None + def test_resume_with_llm_profile(self, tmp_path): """Resuming with llm_profile rebuilds the worker on that profile.""" manager, _ = _manager_with_parent(tmp_path) @@ -640,6 +664,55 @@ def test_resume_with_llm_profile(self, tmp_path): assert resumed.conversation.agent.llm.model == "fast-model" assert resumed.conversation.agent.llm.stream is False + def test_resume_definition_model_discards_ignored_profile(self, tmp_path): + """A definition-owned model must not preserve an ignored override.""" + from openhands.sdk.subagent.registry import agent_definition_to_factory + + manager, _ = _manager_with_parent(tmp_path) + register_builtins_agents() + _make_profile_store( + tmp_path, + {"fast": "fast-model", "pinned": "pinned-model"}, + ) + task = manager._create_task( + subagent_type="general-purpose", + description=None, + llm_profile="fast", + ) + manager._evict_task(task) + + agent_def = AgentDefinition( + name="pinned_agent", + description="Agent with its own model profile", + model="pinned", + tools=["terminal", "file_editor", "task_tracker"], + system_prompt="You are pinned.", + ) + register_agent( + name="pinned_agent", + factory_func=agent_definition_to_factory(agent_def), + description=agent_def, + ) + + resumed = manager._resume_task( + resume=task.id, + subagent_type="pinned_agent", + llm_profile="ignored-missing-profile", + ) + + assert resumed.llm_profile is None + assert resumed.conversation is not None + assert resumed.conversation.agent.llm.model == "pinned-model" + + manager._evict_task(resumed) + inherited_again = manager._resume_task( + resume=task.id, + subagent_type="general-purpose", + ) + assert inherited_again.llm_profile is None + assert inherited_again.conversation is not None + assert inherited_again.conversation.agent.llm.model == "gpt-4o" + def test_resume_without_profile_keeps_original(self, tmp_path): """A bare resume keeps the profile the task was created with.""" manager, _ = _manager_with_parent(tmp_path) diff --git a/tests/tools/task/test_task_tool_set.py b/tests/tools/task/test_task_tool_set.py index a4bdb3aa76..49c0f6c859 100644 --- a/tests/tools/task/test_task_tool_set.py +++ b/tests/tools/task/test_task_tool_set.py @@ -513,6 +513,28 @@ def test_description_lists_llm_profiles(self, tmp_path): assert "- fast" in description assert "- slow" in description + def test_description_exposes_profile_names_only(self, tmp_path): + """Profile configuration and secrets are excluded from the description.""" + store = LLMProfileStore() + store.save( + "safe-name", + LLM( + model="sentinel-private-model", + base_url="https://sentinel-private.example", + api_key=SecretStr("sentinel-private-key"), + ), + include_secrets=True, + ) + + description = TaskToolSet.create( + conv_state=None, # type: ignore[arg-type] + )[0].description + + assert "- safe-name" in description + assert "sentinel-private-model" not in description + assert "sentinel-private.example" not in description + assert "sentinel-private-key" not in description + def test_description_without_profiles_omits_section(self, tmp_path): """With no saved profiles, the llm_profile section is omitted entirely rather than rendering an incoherent "one of these: none" list.""" From 38a858ecf70a1ccaf5bcd908cc2f63d7fb1317d3 Mon Sep 17 00:00:00 2001 From: george larson Date: Wed, 26 Aug 2026 05:26:13 -0400 Subject: [PATCH 4/7] docs(task): record per-call profile design Co-authored-by: openhands --- .pr/design.md | 359 ++++++++++++++++++++------------------------------ 1 file changed, 141 insertions(+), 218 deletions(-) diff --git a/.pr/design.md b/.pr/design.md index 63ae6e0ba3..88dd0d0a9f 100644 --- a/.pr/design.md +++ b/.pr/design.md @@ -1,221 +1,144 @@ -# Per-call `llm_profile` on the task tool — design notes - -**Branch:** `feat/task-tool-llm-profile` (off `origin/main` 4fe565663) -**Status:** implemented, post-panel revision applied, tests green, uncommitted -as of 2026-08-16 - -> Line anchors in this doc (`file.py:NNN`) refer to the pre-rebase tree; the -> branch was rebased onto upstream/main 007721b3d on 2026-08-16 and upstream -> churn shifted them. - -> **Post-panel revision (2026-08-16):** a 7-voice review panel returned -> "ship after fixes." Applied: (1) cipher-aware profile resolution via a new -> public `LocalConversation.load_profile_llm` (the per-call path previously -> loaded with no cipher — ciphertext api_key under cipher-at-rest); (2) -> underscore-private cross-package imports eliminated (`_format_profiles` -> promoted to public `format_llm_profiles`; manager no longer touches -> `registry._get_profile_store`); (3) a pinned `model:` now skips per-call -> resolution entirely, so an unknown `llm_profile` under a pin no longer -> errors; (4) `Task.llm_profile` persisted so a bare resume keeps the -> creation-time profile instead of silently reverting to the parent model; -> (5) the tool description omits the profiles section when no profiles are -> saved; (6) the task-test conftest clears the registry's lru_cached profile -> store on setup and teardown. -> -> **Round-2 (7-of-8 SHIP):** the one requested code fix — a pinned definition -> now stores the *effective* profile (`None`) on the task, not the raw ignored -> request, so a bare resume after a later pin→inherit flip can't try to load a -> typo'd name. Remaining items were doc/wording nits; the proposed -> `vision_inspect._format_profiles` dedupe was rejected (its empty-list message -> and ordering differ from `format_llm_profiles`). +# Per-call `llm_profile` on the task tool ## Problem -Observed in OpenHands #proj-agent-canvas (2026-08-16, Rajiv Shah): prompting Agent -Canvas with a mixed-model workflow ("GPT-5.6 plans, DeepSeek V4 Pro does the -fixes") makes the main agent call `switch_llm` — switching *itself* to the second -model — instead of delegating to a subagent running that model. Two default -settings steer this: `enable_switch_llm_tool` defaults **on** (and its tool -description advertises every saved profile with "use this when another profile is -better suited for the next step"), while `enable_sub_agents` defaults **off**. - -For sequential plan-then-fix work, self-switching is arguably fine. It breaks -down for the fan-out case: one conversation = one model at a time, so a planner -cannot hold its own model while N workers run on a cheaper one. - -The only pre-existing mixed-model delegation path was authoring a subagent -definition `.md` with `model:` frontmatter — one file per agent-shape × model -combination, and nothing the parent can decide per-call from a prompt. - -## What already existed (and is reused) - -- `model:` frontmatter in subagent definitions resolves through - `LLMProfileStore` (`~/.openhands/profiles/.json`) inside the factory — - `openhands-sdk/openhands/sdk/subagent/registry.py:218-228`. -- Built-in subagents (`code_explorer`, `bash_runner`, `web_researcher`, - `default`) are all `model: inherit` — they take whatever LLM the factory is - handed. -- `switch_llm` already lists saved profiles in its tool description - (`get_llm_profile_names` / `format_llm_profiles` in - `openhands-sdk/openhands/sdk/tool/builtins/switch_llm.py`). -- `LocalConversation` already owns a profile store + optional cipher - (`local_conversation.py:413-414`) and decrypts on its own profile loads - (`switch_profile`, `get_or_create_profile_llm`). - -## The change - -`TaskAction` gains one optional field, `llm_profile: str | None` -(`openhands-tools/openhands/tools/task/definition.py`). When set, the named -profile is loaded **through the parent conversation** -(`LocalConversation.load_profile_llm`, new public accessor — cipher-aware, -honors `definition.profile_store_dir`, no llm_registry registration, no -subscription transforms) and injected into the subagent factory **in place of** -the parent-LLM clone (`openhands-tools/openhands/tools/task/manager.py`, -`_get_sub_agent_from_factory`). Threading: `TaskExecutor.__call__` (impl.py) → -`TaskManager.start_task` → `_create_task` / `_resume_task` → -`_get_sub_agent_from_factory`. - -The tool description gains a section listing saved profiles — only when at -least one profile exists (populated in `TaskToolSet.create` via the public -helpers imported from `switch_llm.py` — imported, not forked). - -## Design decisions (the ones a reviewer should attack) - -1. **Profile name, not model string.** The field can only name a file that - exists in the profile store; it is validated against `store.list()` (inside - `load_profile_llm`, listing from the same store it loads from) before - `store.load()`. No path to arbitrary model strings. Strictly smaller - capability than the default-on `switch_llm`, which moves the whole - conversation. - -2. **Pre-factory injection, not post-factory swap.** The factory derives the - subagent's default condenser LLM from the LLM it is handed. Injecting before - `factory_func` keeps the per-call override semantically identical to the - `model:` pin path (worker condenses on its own model). A post-factory swap - would leave the condenser on the parent model. - -3. **Precedence: definition `model:` pin > per-call `llm_profile` > inherit.** - A pinned definition skips per-call resolution entirely (checked before any - store access), so the field is truly ignored under a pin — even an unknown - name does not error, matching the field description. The *effective* - profile is what gets stored on the task (`None` under a pin), so a bare - resume can never try to load an ignored value if the definition is later - flipped to `inherit`. All built-ins are `inherit`, so the feature works on - built-ins out of the box. - -4. **Unknown profile = loud error, never fallback.** `ValueError` raised before - the task is registered in `self._tasks` (no half-created task leak); - `TaskExecutor.__call__` converts it to an error `TaskObservation` naming the - valid profiles — a retryable message to the parent model, not a crash. A - silent inherit fallback would spend parent-model money while appearing to - honor the override. - -5. **Resume keeps the task's model.** The effective per-call profile is stored - on `Task.llm_profile` at creation. A bare `resume=` rebuilds the worker - on that same profile; an explicit resume-time `llm_profile` switches the - worker and becomes the task's new stored profile. (Pre-panel, a bare resume - silently reverted to the parent model.) - -6. **Field description is prompt engineering.** It disambiguates against - `switch_llm` in the model's own reading ("This affects only the delegated - subagent — your own model is unchanged (use the switch_llm tool to change - your own model)") because the observed bug was self-switch instead of - delegate. - -7. **Profile resolution goes through the parent conversation, not the registry - store.** `LocalConversation.load_profile_llm` decrypts secrets with the - conversation's cipher (parity with `switch_profile`) and — deliberately — - does NOT register the LLM in the parent's `llm_registry` or apply - subscription transforms (that is what `get_or_create_profile_llm` does; - using it would muddle per-task metrics attribution). A definition-level - `profile_store_dir` is honored by constructing a store for that dir; the - conversation cipher is passed at `load()` time regardless of which store is - used (cipher is a deployment property, not a store property). - -## Explicitly out of scope - -- `tool_concurrency_limit` default (1) — parallel fan-out is a product decision. -- Built-in subagents' `model: inherit` — it is what makes them overridable. -- The older Delegate tool — separate schema, separate surface. -- UI surfaces (subagent definitions editor, profile picker at spawn). -- The `enable_switch_llm_tool`-on / `enable_sub_agents`-off default asymmetry — - a settings/rollout conversation, not this diff. -- The definition-pin path's own cipher handling inside - `registry.agent_definition_to_factory` (`store.load` without cipher, - registry.py:228) — pre-existing upstream behavior, unchanged by this diff. - Since this PR introduces the public cipher-aware accessor one floor up - (`LocalConversation.load_profile_llm`), routing the pin path through it is - now a small, obvious follow-up issue. -- Agent-server profile-store dir divergence (`OH_PERSISTENCE_DIR` vs SDK - home-dir default) — pre-existing, affects `model:` pins equally today. - (No agent-server changes: `TaskAction` is deserialized from the same - dynamically-registered tool code server-side, so the new field flows through.) - -## Metrics - -Override LLM is freshly constructed by `load_profile_llm` + `reset_metrics()` -(parity with the clone path) and is never registered in the parent's -`llm_registry`. Parent attribution via `_update_parent_metrics` -(keyed `task:{id}`) is untouched; the parent's LLM object is never mutated. - -## Tests (14 new; tests/tools/task 65 → 76; zero regressions) - -943 passed across the commanded suites (`tests/tools/task` 76 + -`tests/tools/test_tool_name_consistency.py` 3 + `tests/sdk/subagent` + -`tests/sdk/tool/test_switch_llm.py` 125 combined + `tests/sdk/conversation` -739). The 14 new tests: 11 under `tests/tools/task/` (7 manager-level, -4 tool-set-level) + 3 in `tests/sdk/conversation/test_switch_model.py`. - -- `tests/tools/task/test_task_manager.py::TestTaskManagerLLMProfile` — override - applied + `stream is False`; unknown profile raises listing available - profiles with no `_tasks` leak; pin fully ignores override (unknown name does - not error, and the ignored value is not stored on the task); resume keeps - original profile; explicit resume overrides and re-stores; inherit-created - task keeps inheriting on bare resume. -- `tests/tools/task/test_task_tool_set.py` — sequential mixed-model pair - (factories received exactly `["fast-model", "slow-model"]`); unknown profile → - error observation naming the profile; description renders profile list when - profiles exist; description omits the section when the store is empty. -- `tests/sdk/conversation/test_switch_model.py` — `load_profile_llm` decrypts - with the conversation cipher and stays out of `llm_registry`; unknown profile - lists available from the same store; `profile_store_dir` override resolves - from the custom dir only. -- `tests/tools/task/conftest.py` — redirects the profile-store dir to tmp AND - clears the registry's lru_cached store getter on setup/teardown (a cached - store bound to a dead tmp dir must not leak across tests). -- Full `tests/tools`: terminal/tmux failures are pre-existing environment - flakiness (clean tree fails the same directory as a superset). -- `pre-commit run --files `: all hooks pass (incl. pyright). - -## Live verification (2026-08-16, real profiles on laptop: kimi/mimo/minimax) - -Function-level: parent `minimax` (MiniMax-M3); `start_task(llm_profile="mimo")` -→ worker `mimo-v2.5-pro`, completed; no-override → worker == parent model; -unknown profile → clean `ValueError`, no task-state leak. - -Agent-level (the real UX): live parent agent on MiniMax-M3, handed only -`TaskToolSet`, prompted Rajiv-style ("delegate to a general-purpose subagent … -run it on the mimo LLM profile, do not switch your own model"). Event stream -shows the parent **chose** to emit `"llm_profile": "mimo"`; worker ran on -`openai/mimo-v2.5-pro`; parent's model unchanged; synthesis returned to the -parent. Demo scripts: `/tmp/llm-profile-demo.py`, `/tmp/llm-profile-demo-agentic.py`. - -## Known limitations - -- Fan-out still sequential by default (`tool_concurrency_limit=1`). -- Profile list in the tool description is a creation-time snapshot, and it is - built from the DEFAULT profile store; a definition with a custom - `profile_store_dir` executes from that store, so the advertised list and the - resolvable set can differ there. -- Old SDK versions reject new events containing the field (`extra="forbid"` — - standard additive-field version-skew caveat; note in release notes). - -## Maintainer-reception risk (stated honestly) - -In the same Slack thread, Graham Neubig steered toward the conversation-spawn -path (the `agent-canvas-environment` skill, "delegate to a local conversation") -and said "we should make that easier." This PR is the *in-conversation* fix -instead. The answer to "why not the conversation path" is fan-out concurrency -and synthesis-in-parent — but a maintainer may still prefer the -conversation-level direction. This is the chief rejection risk and the main -thing the pre-public panel should attack. +The task tool can delegate work to a subagent, but an inherited subagent always +uses the parent conversation's LLM. A parent that wants a mixed-model workflow +must either switch its own model or rely on a static `model:` value in an agent +definition. Neither supports choosing a worker profile for an individual task +while the parent keeps its current model. + +## User-facing behavior + +`TaskAction` gains an optional `llm_profile` field naming a saved LLM profile. + +The precedence is: + +1. an agent definition with its own model profile; +2. the task call's `llm_profile`; +3. the parent model, inherited by default. + +An agent definition's model is resolved before the per-call override. Therefore +an ignored override is never loaded or validated. For resumable tasks, a bare +resume retains the task's effective per-call profile; an explicit profile on a +resume replaces it. Resuming through an agent definition that supplies its own +model clears any previously stored per-call profile, so that ignored value +cannot affect a later resume. + +Unknown profiles fail loudly and become an error `TaskObservation`. The task is +not partially registered and the worker never silently falls back to the parent +model. + +## Shared profile-loading boundary + +`LocalConversation.load_profile_llm()` is the common cipher-aware loading +primitive for conversation-owned profile operations. It selects the +conversation's default store or a caller-supplied profile directory, then calls +`LLMProfileStore.load(profile_name, cipher=self._cipher)`. + +The operation only loads. It does not activate the returned LLM, add it to the +parent conversation's registry, or bind parent conversation context. + +The existing higher-level methods add their own behavior: + +- `switch_profile()` loads through the primitive, assigns the canonical + `profile:` usage ID, and activates the LLM; +- `get_or_create_profile_llm()` returns a registry hit or loads through the + primitive, assigns the caller's usage ID, registers the LLM, and binds the + conversation context; +- `TaskManager` loads through the primitive, resets metrics, and passes the LLM + into the worker factory. The worker `LocalConversation` then owns registration + and context binding. + +Using `get_or_create_profile_llm()` directly for a worker would register an +otherwise unused template LLM in the parent conversation and give it the wrong +metrics/context owner. Adding a mode flag to that method would also make its +registry-oriented contract ambiguous. + +`LLMProfileStore.load()` already performs persisted subscription restoration +through `LLM.from_persisted()`. The loading primitive therefore returns the +runtime subscription LLM while still avoiding parent registration and +activation. + +The store's native error contract is preserved: a missing profile raises +`FileNotFoundError` with the available profile files, while invalid or corrupted +profiles raise `ValueError`. There is no separate list-before-load operation, so +validation and reading happen through the store's locked load path. + +## Worker construction and metrics + +The selected LLM is injected before `factory_func` runs. This matters because a +factory may derive a default condenser from its input LLM; swapping afterward +would leave the condenser on the parent model. + +Subscription-backed LLMs are the exception: their worker condenser is disabled, +matching top-level agent creation and profile switching, because the separate +LLM completion used for summarization is unsupported on that path. + +The loaded LLM gets a fresh metrics object before factory construction. It is +never registered in the parent's `llm_registry`. The worker conversation tracks +its own agent and condenser usage, and the existing task completion path copies +the worker's combined metrics into the parent under `task:` exactly +once. The parent's active model is unchanged. + +## Profile discovery and confidentiality + +The task tool description lists saved profile names using the same public +`get_llm_profile_names()` and `format_llm_profiles()` helpers as `switch_llm`. +It exposes names only: profile model IDs, provider URLs, API keys, and persisted +JSON are not included. When no profiles exist, the section is omitted. + +The list is a creation-time snapshot of the default profile store. A file-based +agent may specify a custom `profile_store_dir`, so its resolvable profile set can +differ from the advertised default list. This pre-existing multi-store discovery +limitation is not expanded into a new cross-store API in this change. + +## Persistence and compatibility + +`Task.llm_profile` stores the effective per-call profile for resume. An agent +definition that supplies its own model stores `None` because the request override +was ignored. Existing task calls omit the optional field and continue inheriting +the parent model. + +The additive action field has the repository's normal version-skew caveat: an old +SDK that forbids unknown action fields cannot deserialize a new event containing +`llm_profile`. + +## Verification matrix + +Focused tests cover: + +- two sequential tasks choosing different saved profiles; +- parent model unchanged and no worker profile entry in the parent registry; +- independent worker metrics and `task:` merge-back; +- encrypted-secret loading through the conversation cipher; +- persisted subscription restoration; +- subscription workers do not receive an LLM-backed condenser; +- custom profile directories; +- native missing-profile errors with no partial task state; +- definition model precedence, including an invalid ignored override; +- bare-resume retention and explicit resume replacement; +- definition-owned models discard ignored profiles during resume; +- inherited behavior when no override is supplied; +- pre-factory injection and `stream=False` worker behavior; +- profile names present while model IDs, provider URLs, and API keys remain absent + from the tool description; +- omission of the profile section when no profiles exist. + +Current verification on the rebased branch: + +- 235 task, subagent, profile-switch, and conversation-switch tests pass; +- pre-commit passes on every revised file, including Pyright; +- a live encrypted-profile delegation completed with a MiniMax M3 parent and a + MiMo v2.5 Pro worker. The parent remained on MiniMax, its registry contained no + worker profile entry, its only metrics key was `task:task_00000001`, and a bare + resume retained the encrypted worker profile and MiMo model. + +## Out of scope + +- changing the `switch_llm` or subagent defaults; +- enabling parallel task execution by default; +- adding frontend worker-model reporting; +- changing definition-level profile loading; +- reconciling default-store advertising with every custom agent store; +- changing the conversation-spawn workflow. From 7fe2de3feb3ff81e0bf4a899a50c61fccf4c74c5 Mon Sep 17 00:00:00 2001 From: george larson Date: Wed, 26 Aug 2026 11:25:39 -0400 Subject: [PATCH 5/7] fix(task): retain subscription worker condenser Co-authored-by: openhands --- .pr/design.md | 8 ++++---- openhands-tools/openhands/tools/task/manager.py | 6 ------ tests/tools/task/test_task_manager.py | 9 ++++++--- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/.pr/design.md b/.pr/design.md index 88dd0d0a9f..c1418597aa 100644 --- a/.pr/design.md +++ b/.pr/design.md @@ -71,9 +71,9 @@ The selected LLM is injected before `factory_func` runs. This matters because a factory may derive a default condenser from its input LLM; swapping afterward would leave the condenser on the parent model. -Subscription-backed LLMs are the exception: their worker condenser is disabled, -matching top-level agent creation and profile switching, because the separate -LLM completion used for summarization is unsupported on that path. +Subscription-backed LLMs retain the factory-created condenser. Subscription +completion dispatch supports condenser calls, matching top-level agent creation +and profile switching. The loaded LLM gets a fresh metrics object before factory construction. It is never registered in the parent's `llm_registry`. The worker conversation tracks @@ -113,7 +113,7 @@ Focused tests cover: - independent worker metrics and `task:` merge-back; - encrypted-secret loading through the conversation cipher; - persisted subscription restoration; -- subscription workers do not receive an LLM-backed condenser; +- subscription workers retain their supported LLM-backed condenser; - custom profile directories; - native missing-profile errors with no partial task state; - definition model precedence, including an invalid ignored override; diff --git a/openhands-tools/openhands/tools/task/manager.py b/openhands-tools/openhands/tools/task/manager.py index e6b2642ca7..0d001edb99 100644 --- a/openhands-tools/openhands/tools/task/manager.py +++ b/openhands-tools/openhands/tools/task/manager.py @@ -428,12 +428,6 @@ def _get_sub_agent_from_factory( sub_agent = factory.factory_func(sub_agent_llm) - # Subscription-backed LLMs do not support the separate completion - # flow used by an LLM summarizing condenser. This matches top-level - # agent creation and profile switching behavior. - if sub_agent.llm.is_subscription: - sub_agent = sub_agent.model_copy(update={"condenser": None}) - # ensuring that the sub-agent LLM has stream deactivated sub_agent = sub_agent.model_copy( update={"llm": sub_agent.llm.model_copy(update={"stream": False})} diff --git a/tests/tools/task/test_task_manager.py b/tests/tools/task/test_task_manager.py index c5cb134492..622e584ab6 100644 --- a/tests/tools/task/test_task_manager.py +++ b/tests/tools/task/test_task_manager.py @@ -7,6 +7,7 @@ from pydantic import SecretStr from openhands.sdk import LLM, Agent +from openhands.sdk.context.condenser import LLMSummarizingCondenser from openhands.sdk.conversation.impl.local_conversation import LocalConversation from openhands.sdk.conversation.state import ConversationExecutionStatus from openhands.sdk.hooks.config import HookConfig, HookDefinition, HookMatcher @@ -624,8 +625,8 @@ def test_definition_model_beats_llm_profile(self, tmp_path): assert task.conversation is not None assert task.conversation.agent.llm.model == "pinned-model" - def test_subscription_profile_disables_worker_condenser(self, tmp_path): - """Subscription workers must not receive an LLM-backed condenser.""" + def test_subscription_profile_keeps_worker_condenser(self, tmp_path): + """Subscription workers retain the factory's supported condenser.""" manager, parent = _manager_with_parent(tmp_path) register_builtins_agents() subscription_llm = _make_llm() @@ -640,7 +641,9 @@ def test_subscription_profile_disables_worker_condenser(self, tmp_path): assert task.conversation is not None assert task.conversation.agent.llm.is_subscription is True - assert task.conversation.agent.condenser is None + condenser = task.conversation.agent.condenser + assert isinstance(condenser, LLMSummarizingCondenser) + assert condenser.llm.is_subscription is True def test_resume_with_llm_profile(self, tmp_path): """Resuming with llm_profile rebuilds the worker on that profile.""" From 61c3464e4fe30c3f42f08326850c985bb07bd140 Mon Sep 17 00:00:00 2001 From: george larson Date: Wed, 26 Aug 2026 15:23:30 -0400 Subject: [PATCH 6/7] ci: retrigger checks after GitHub Actions outage From d99d9f08b58142b1d341e61a8c97524e41d0cdba Mon Sep 17 00:00:00 2001 From: george larson Date: Wed, 26 Aug 2026 17:40:48 -0400 Subject: [PATCH 7/7] fix(task): make worker profile selection explicit --- .pr/design.md | 39 ++++---- .../openhands/sdk/subagent/AGENTS.md | 5 +- .../openhands/tools/task/definition.py | 12 ++- .../openhands/tools/task/manager.py | 44 ++++----- tests/tools/task/test_task_manager.py | 98 ++++++++++++++----- tests/tools/task/test_task_tool_set.py | 26 +++++ 6 files changed, 147 insertions(+), 77 deletions(-) diff --git a/.pr/design.md b/.pr/design.md index c1418597aa..7c72a272af 100644 --- a/.pr/design.md +++ b/.pr/design.md @@ -12,18 +12,14 @@ while the parent keeps its current model. `TaskAction` gains an optional `llm_profile` field naming a saved LLM profile. -The precedence is: +An inherited agent uses the task call's `llm_profile` when supplied and otherwise +uses the parent model. Supplying `llm_profile` for an agent definition that already +selects a model is a configuration conflict and fails explicitly; neither choice +is silently discarded. -1. an agent definition with its own model profile; -2. the task call's `llm_profile`; -3. the parent model, inherited by default. - -An agent definition's model is resolved before the per-call override. Therefore -an ignored override is never loaded or validated. For resumable tasks, a bare -resume retains the task's effective per-call profile; an explicit profile on a -resume replaces it. Resuming through an agent definition that supplies its own -model clears any previously stored per-call profile, so that ignored value -cannot affect a later resume. +For resumable tasks, a bare resume retains the task's effective per-call profile; +an explicit profile on a resume replaces it. Resume remains scoped to the current +in-memory `TaskManager` lifecycle. Unknown profiles fail loudly and become an error `TaskObservation`. The task is not partially registered and the worker never silently falls back to the parent @@ -88,17 +84,15 @@ The task tool description lists saved profile names using the same public It exposes names only: profile model IDs, provider URLs, API keys, and persisted JSON are not included. When no profiles exist, the section is omitted. -The list is a creation-time snapshot of the default profile store. A file-based -agent may specify a custom `profile_store_dir`, so its resolvable profile set can -differ from the advertised default list. This pre-existing multi-store discovery -limitation is not expanded into a new cross-store API in this change. +The list is a creation-time snapshot of the default profile store. When any +registered agent specifies a custom `profile_store_dir`, the task tool omits the +list rather than advertising names from a store that the selected agent may not +use. ## Persistence and compatibility -`Task.llm_profile` stores the effective per-call profile for resume. An agent -definition that supplies its own model stores `None` because the request override -was ignored. Existing task calls omit the optional field and continue inheriting -the parent model. +`Task.llm_profile` stores the effective per-call profile for resume. Existing task +calls omit the optional field and continue inheriting the parent model. The additive action field has the repository's normal version-skew caveat: an old SDK that forbids unknown action fields cannot deserialize a new event containing @@ -116,14 +110,16 @@ Focused tests cover: - subscription workers retain their supported LLM-backed condenser; - custom profile directories; - native missing-profile errors with no partial task state; -- definition model precedence, including an invalid ignored override; +- explicit conflicts between definition-owned models and per-call profiles; - bare-resume retention and explicit resume replacement; -- definition-owned models discard ignored profiles during resume; +- a clear missing-profile failure when a stored profile is removed before resume; - inherited behavior when no override is supplied; - pre-factory injection and `stream=False` worker behavior; - profile names present while model IDs, provider URLs, and API keys remain absent from the tool description; - omission of the profile section when no profiles exist. +- omission of default-store profile names when a registered agent uses a custom + profile store. Current verification on the rebased branch: @@ -140,5 +136,4 @@ Current verification on the rebased branch: - enabling parallel task execution by default; - adding frontend worker-model reporting; - changing definition-level profile loading; -- reconciling default-store advertising with every custom agent store; - changing the conversation-spawn workflow. diff --git a/openhands-sdk/openhands/sdk/subagent/AGENTS.md b/openhands-sdk/openhands/sdk/subagent/AGENTS.md index 894e980066..6f8e310c2d 100644 --- a/openhands-sdk/openhands/sdk/subagent/AGENTS.md +++ b/openhands-sdk/openhands/sdk/subagent/AGENTS.md @@ -111,9 +111,8 @@ Supported YAML frontmatter keys (see `AgentDefinition.load` in `schema.py`): - `skills` (default: `[]`): a comma-separated string or a list of skill names - `model` (default: `inherit`): `inherit` reuses the parent LLM; another value is loaded as an LLM profile name from `profile_store_dir` or the default profile store - - an agent definition with its own model profile takes precedence over the task - tool's per-call `llm_profile` override; the override applies only to - `model: inherit` definitions + - the task tool's per-call `llm_profile` applies only to `model: inherit` + definitions; supplying it for a definition that selects a model is an error - `color` (optional) - `max_iteration_per_run` (optional, positive integer) - `max_budget_per_run` (optional, positive number in USD) diff --git a/openhands-tools/openhands/tools/task/definition.py b/openhands-tools/openhands/tools/task/definition.py index 636fececb4..f8120c4b3a 100644 --- a/openhands-tools/openhands/tools/task/definition.py +++ b/openhands-tools/openhands/tools/task/definition.py @@ -56,8 +56,8 @@ class TaskAction(Action): default=None, description=( "Saved LLM profile for this subagent. If omitted, the subagent " - "inherits the parent model. An agent definition with its own model " - "profile takes precedence." + "inherits the parent model. Do not supply this when the selected " + "agent definition already specifies a model profile." ), ) resume: str | None = Field( @@ -247,7 +247,11 @@ def create( agent_types_info = get_factory_info() - profile_names = get_llm_profile_names() + agent_definitions = get_registered_agent_definitions() + has_custom_profile_store = any( + definition.profile_store_dir is not None for definition in agent_definitions + ) + profile_names = [] if has_custom_profile_store else get_llm_profile_names() llm_profiles_section = "" if profile_names: llm_profiles_section = ( @@ -258,7 +262,7 @@ def create( "your own model is unchanged.\n\n" ) - registered = {d.name for d in get_registered_agent_definitions()} + registered = {definition.name for definition in agent_definitions} task_tool_examples = "\n".join( ex for name, ex in TASK_TOOL_EXAMPLES.items() if name in registered ) diff --git a/openhands-tools/openhands/tools/task/manager.py b/openhands-tools/openhands/tools/task/manager.py index 0d001edb99..a98c01d81b 100644 --- a/openhands-tools/openhands/tools/task/manager.py +++ b/openhands-tools/openhands/tools/task/manager.py @@ -80,10 +80,8 @@ class Task(BaseModel): error: str | None = Field(default=None, description="Error if task failed.") llm_profile: str | None = Field( default=None, - description="Effective LLM profile the task's worker runs on. Set at " - "creation from the per-call override (None when the agent definition " - "selects its own model); a bare resume keeps it, an explicit resume-time " - "override replaces it.", + description="Effective per-call LLM profile the task's worker runs on. " + "A bare resume keeps it; an explicit resume-time override replaces it.", ) conversation: LocalConversation | None = Field( default=None, @@ -191,8 +189,8 @@ def start_task( description: Short label for the task. conversation: Parent conversation (set on first call). llm_profile: Optional saved LLM profile to run the sub-agent on - instead of inheriting the parent's model. A ``model:`` pin in - the sub-agent definition takes precedence over this. On resume, + instead of inheriting the parent's model. This conflicts with + a ``model:`` pin in the sub-agent definition. On resume, omitting it keeps the profile the task was created with; passing one explicitly switches the resumed worker to it. @@ -225,8 +223,10 @@ def _resume_task( ) -> Task: """Resume a sub-agent task. - A None llm_profile keeps the profile the task was created with; an - explicit one overrides it (and becomes the task's new profile). + A None llm_profile keeps the profile the task was created with when the + selected definition inherits its model. Definitions that pin a model + discard the stored profile. An explicit profile overrides the stored + one, but conflicts with a definition-pinned model. """ with self._tasks_lock: if resume not in self._tasks: @@ -236,15 +236,12 @@ def _resume_task( ) stored = self._tasks[resume] - requested_profile = ( - llm_profile if llm_profile is not None else stored.llm_profile - ) factory = get_agent_factory(subagent_type) - effective_profile = ( - None if _definition_has_model_profile(factory) else requested_profile - ) + requested_profile = llm_profile + if requested_profile is None and not _definition_has_model_profile(factory): + requested_profile = stored.llm_profile worker_agent = self._get_sub_agent_from_factory( - factory, llm_profile=effective_profile + factory, llm_profile=requested_profile ) conversation_id = self._tasks[resume].conversation_id with detached_delegate_context() as link: @@ -270,7 +267,7 @@ def _resume_task( update={ "conversation": conversation, "status": TaskStatus.RUNNING, - "llm_profile": effective_profile, + "llm_profile": requested_profile, } ) @@ -328,11 +325,7 @@ def _create_task( conversation_id=conversation_id, conversation=sub_conversation, status=TaskStatus.RUNNING, - # An agent definition's model takes precedence over the - # per-call override, including on a later bare resume. - llm_profile=( - None if _definition_has_model_profile(factory) else llm_profile - ), + llm_profile=llm_profile, ) return self._tasks[task_id] @@ -412,7 +405,14 @@ def _get_sub_agent_from_factory( parent_llm = parent.agent.llm definition_has_model = _definition_has_model_profile(factory) - if llm_profile is not None and not definition_has_model: + if llm_profile is not None and definition_has_model: + raise ValueError( + f"Agent '{factory.definition.name}' already selects model profile " + f"'{factory.definition.model}'; remove llm_profile or choose an " + "agent definition that inherits the parent model." + ) + + if llm_profile is not None: # Injected pre-factory so the factory derives the default condenser # LLM from the selected profile. sub_agent_llm = parent.load_profile_llm( diff --git a/tests/tools/task/test_task_manager.py b/tests/tools/task/test_task_manager.py index 622e584ab6..66bae18141 100644 --- a/tests/tools/task/test_task_manager.py +++ b/tests/tools/task/test_task_manager.py @@ -591,12 +591,8 @@ def test_create_task_unknown_profile_raises(self, tmp_path): assert "fast" in str(excinfo.value) # available profiles are listed assert len(manager._tasks) == 0 - def test_definition_model_beats_llm_profile(self, tmp_path): - """A definition's model fully ignores a per-call ``llm_profile``. - - Even a name absent from the store must not raise, and the ignored value - is not stored for a later bare resume. - """ + def test_definition_model_conflicts_with_llm_profile(self, tmp_path): + """A definition-owned model must not silently ignore ``llm_profile``.""" from openhands.sdk.subagent.registry import agent_definition_to_factory agent_def = AgentDefinition( @@ -615,15 +611,14 @@ def test_definition_model_beats_llm_profile(self, tmp_path): manager, _ = _manager_with_parent(tmp_path) _make_profile_store(tmp_path, {"pinned": "pinned-model"}) - task = manager._create_task( - subagent_type="pinned_agent", - description=None, - llm_profile="typo-does-not-exist", - ) + with pytest.raises(ValueError, match="already selects model profile 'pinned'"): + manager._create_task( + subagent_type="pinned_agent", + description=None, + llm_profile="typo-does-not-exist", + ) - assert task.llm_profile is None - assert task.conversation is not None - assert task.conversation.agent.llm.model == "pinned-model" + assert len(manager._tasks) == 0 def test_subscription_profile_keeps_worker_condenser(self, tmp_path): """Subscription workers retain the factory's supported condenser.""" @@ -667,8 +662,48 @@ def test_resume_with_llm_profile(self, tmp_path): assert resumed.conversation.agent.llm.model == "fast-model" assert resumed.conversation.agent.llm.stream is False - def test_resume_definition_model_discards_ignored_profile(self, tmp_path): - """A definition-owned model must not preserve an ignored override.""" + def test_resume_definition_model_conflicts_with_llm_profile(self, tmp_path): + """Resume reports a definition/profile conflict without mutating state.""" + from openhands.sdk.subagent.registry import agent_definition_to_factory + + manager, _ = _manager_with_parent(tmp_path) + register_builtins_agents() + _make_profile_store( + tmp_path, + {"fast": "fast-model", "pinned": "pinned-model"}, + ) + task = manager._create_task( + subagent_type="general-purpose", + description=None, + llm_profile="fast", + ) + manager._evict_task(task) + + agent_def = AgentDefinition( + name="pinned_agent", + description="Agent with its own model profile", + model="pinned", + tools=["terminal", "file_editor", "task_tracker"], + system_prompt="You are pinned.", + ) + register_agent( + name="pinned_agent", + factory_func=agent_definition_to_factory(agent_def), + description=agent_def, + ) + + with pytest.raises(ValueError, match="already selects model profile 'pinned'"): + manager._resume_task( + resume=task.id, + subagent_type="pinned_agent", + llm_profile="ignored-missing-profile", + ) + + assert manager._tasks[task.id].llm_profile == "fast" + assert manager._tasks[task.id].conversation is None + + def test_resume_definition_model_discards_stored_profile(self, tmp_path): + """A pinned definition replaces a stored inherited-model profile.""" from openhands.sdk.subagent.registry import agent_definition_to_factory manager, _ = _manager_with_parent(tmp_path) @@ -700,22 +735,12 @@ def test_resume_definition_model_discards_ignored_profile(self, tmp_path): resumed = manager._resume_task( resume=task.id, subagent_type="pinned_agent", - llm_profile="ignored-missing-profile", ) assert resumed.llm_profile is None assert resumed.conversation is not None assert resumed.conversation.agent.llm.model == "pinned-model" - manager._evict_task(resumed) - inherited_again = manager._resume_task( - resume=task.id, - subagent_type="general-purpose", - ) - assert inherited_again.llm_profile is None - assert inherited_again.conversation is not None - assert inherited_again.conversation.agent.llm.model == "gpt-4o" - def test_resume_without_profile_keeps_original(self, tmp_path): """A bare resume keeps the profile the task was created with.""" manager, _ = _manager_with_parent(tmp_path) @@ -736,6 +761,27 @@ def test_resume_without_profile_keeps_original(self, tmp_path): assert resumed.conversation is not None assert resumed.conversation.agent.llm.model == "fast-model" + def test_resume_after_profile_removed_returns_clear_error(self, tmp_path): + """A bare resume fails clearly if its stored profile was removed.""" + manager, _ = _manager_with_parent(tmp_path) + register_builtins_agents() + store = _make_profile_store(tmp_path, {"fast": "fast-model"}) + + task = manager._create_task( + subagent_type="general-purpose", description=None, llm_profile="fast" + ) + manager._evict_task(task) + store.delete("fast") + + with pytest.raises(FileNotFoundError, match="Profile `fast` not found"): + manager._resume_task( + resume=task.id, + subagent_type="general-purpose", + ) + + assert manager._tasks[task.id].llm_profile == "fast" + assert manager._tasks[task.id].conversation is None + def test_resume_with_explicit_profile_overrides_stored(self, tmp_path): """An explicit resume-time profile replaces the stored one for later bare resumes.""" diff --git a/tests/tools/task/test_task_tool_set.py b/tests/tools/task/test_task_tool_set.py index 49c0f6c859..ef38d3001c 100644 --- a/tests/tools/task/test_task_tool_set.py +++ b/tests/tools/task/test_task_tool_set.py @@ -9,6 +9,7 @@ from openhands.sdk.llm import Message, MessageToolCall, TextContent from openhands.sdk.llm.llm_profile_store import LLMProfileStore from openhands.sdk.subagent.registry import _reset_registry_for_tests, register_agent +from openhands.sdk.subagent.schema import AgentDefinition from openhands.sdk.testing import TestLLM from openhands.tools.task import TaskToolSet from openhands.tools.task.definition import TASK_TOOL_EXAMPLES, TaskObservation @@ -549,6 +550,31 @@ def test_description_without_profiles_omits_section(self, tmp_path): assert "llm_profile" not in description assert "saved LLM profiles" not in description + def test_description_omits_profiles_for_custom_store_agent(self, tmp_path): + """Do not advertise default-store names when an agent resolves elsewhere.""" + agent_def = AgentDefinition( + name="custom_store_agent", + description="Agent backed by a custom profile store", + model="inherit", + profile_store_dir=str(tmp_path / "custom-profiles"), + ) + register_agent( + name=agent_def.name, + factory_func=lambda llm: Agent(llm=llm, tools=[]), + description=agent_def, + ) + + with patch( + "openhands.tools.task.definition.get_llm_profile_names", + return_value=["default-only"], + ): + description = TaskToolSet.create( + conv_state=None, # type: ignore[arg-type] + )[0].description + + assert "default-only" not in description + assert "llm_profile" not in description + def test_only_registered_examples_included(self, tmp_path): """Only examples for registered agents appear; others are excluded.""" keys = list(TASK_TOOL_EXAMPLES.keys())