fix(image): gate image tools on the models' declared abilities

[bluefish-08]

Closes #1295. Sub-issue of #1289.

Background

A production ad-creative task called edit_image 8 times in a row, each failing
instantly with No available image models with edit capabilities, then worked
around it by editing images through execute_python_code 16 more times. That
deployment has a single image model configured with abilities = ["generate"].

The framework problem is that edit_image is registered unconditionally, with no
regard for whether any configured model can serve it. Worth recording: the
description already said so — _edit_model_info_text falls back to the literal
string No image models with edit capabilities available and that text is
formatted into the tool description. Production shows the model ignored that
passive note and blind-retried anyway, so rewording is not a fix; the tool has to
leave the schema.

Those ~24 doomed and detour calls each cost a full LLM round trip (the whole
history plus the tool schemas re-sent), which is where the bulk of that task's
token spend and its 28-minute wall time came from.

Changes

1. Capability gating (vibe/image_tool.py get_tools) — no model with
   generate means generate_image is not registered; no model with edit
   means edit_image is not registered; neither means an empty tool list. When
   editing is unavailable, generate_image gets a leading prohibition in its
   description, because generate_image(images=...) delegates to the edit path
   and is the side door into the same failure.
2. Self-correctable errors (core/image_tool.py) — both "no usable model"
   errors now enumerate the configured models with their abilities and state what
   to do next. The original wording is kept as the prefix so existing log
   searches still match.
3. Ability check on the default-model branches of _get_model and
   _get_edit_model — the explicit model_id branch verified has_ability, but
   the _default_*_model branch trusted its input unconditionally. A
   generate-only model configured as default_edit_model therefore bypassed the
   check, reached the provider, and raised RuntimeError there instead of
   returning the friendly error. The check is deliberately lenient: it rejects
   only a model that explicitly reports lacking the ability, and still trusts one
   without a has_ability method.

Why the long description template is left alone

GENERATE_IMAGE_DESCRIPTION teaches the images parameter in three separate
places. Trimming each would mean adding placeholders to the template for a much
wider diff, and existing tests assert against the class constant directly.
Instead, editing-unavailable deployments get one leading prohibition, backed by
edit_image being absent from the schema and by the self-correctable error. When
editing is available the description is byte-identical to before.

Out of scope

• Deployment-side follow-up: grant the image model the edit ability, or
  register an edit-capable model. This change only guarantees that a tool nothing
  can serve is no longer offered to the model.
• Other items from the parent issue (retry predicate never passed to
  create_retry_wrapper, redundant fields in the tool-result observation,
  serial media tool execution, planner missing the deliverable type) are not in
  this PR.

Verification

• New TestImageToolCapabilityGating with 7 cases: edit_image absent and the
  prohibition present when nothing can edit; both tools present and the
  description unchanged when a model can edit; generate_image absent when only
  editing is available; empty list when neither is available; a
  default_edit_model that denies edit is not trusted; both error strings
  enumerate abilities.
• Updated 3 existing cases that asserted the old contract (tool count with empty
  models; two exact error-string comparisons relaxed to substring).
• tests/core/tools/ passes (exit 0). test_command_path_guard_bash.py and
  test_command_executor.py have pre-existing failures, reproduced on
  origin/main with these changes stashed to confirm they are unrelated.
• ruff check, ruff format, mypy, and the full pre-commit hook set pass.

[gemini-code-assist]

Code Review

This pull request introduces capability gating for image tools, dynamically registering the generate_image and edit_image tools only when configured models support those capabilities. It also refines error messages and default model resolution to respect these abilities. The review feedback correctly identifies that the description for list_image_models contains a note referencing generate_image, which could be misleading if generate_image is omitted due to the new gating logic, and suggests removing it.

[rogercloud]

Summary

This PR moves image-tool availability from prose to schema. ImageGenerationTool.get_tools() now consults has_generate_capable_model() / has_edit_capable_model() and registers only the tools that can actually succeed: no generate-capable model means no generate_image, no edit-capable model means no edit_image, and neither means an empty tool list. It also adds an explicit ability check to the previously-unchecked default_generate_model / default_edit_model branches of _get_model() / _get_edit_model() (via _denies_ability), and replaces the two flat "no available image models" strings with self-correctable errors that enumerate the configured models and their abilities.

Approach verdict

Acceptable with reservations — gating at the schema rather than in description text is the right fix for the observed failure (8 blind edit_image retries followed by 16 execute_python_code fallbacks), and reusing _get_model() / _get_edit_model() as both the runtime resolver and the registration gate is a genuine single source of truth: the adapter and the core cannot disagree about what is available.

The reservation is that it closes the front door and leaves a side door guarded only by prose — the exact pattern the issue's own reasoning says does not work:

• The images= path on generate_image is still reachable when editing is unavailable. generate_image delegates to edit_image() as soon as images is not None, before any of its own model checks. In the common can_generate=True, can_edit=False deployment, generate_image is registered, and a model that passes images= lands in the edit path. The only guard is the prepended "IMAGE EDITING IS UNAVAILABLE" prose in the description — the same class of mitigation this PR argues is insufficient. To be fair, this degrades gracefully (a structured _no_edit_model_error() tool result, not a crash or a silent wrong answer), so it is not a repeat of the original bug — but the recommendation text it returns is wrong in this state (see the first line-level finding), and this whole path is untested for the generate-only case.

• src/xagent/web/api/tools.py:120-133 becomes unreachable in exactly the deployments this PR targets. _create_tool_info() has an elif tool_name == "edit_image": branch that independently re-derives "does any model have the edit ability" and, when not, marks the tool status: "missing_capability" with a remediation message for the admin. That branch only fires if a tool object literally named edit_image is present in what ToolFactory.create_all_tools() returns — which this PR now guarantees it is not, whenever the gate trips. Net effect: the admin tool listing loses the "add an image model with editing support" explanation entirely, and edit_image just silently disappears from the list. That file is untouched here and no test covers the interaction. Either the branch should be updated to render a synthetic disabled row, or (cleaner) the adapter should expose the reason so the web layer stops re-deriving the same predicate. At minimum this deserves a follow-up issue rather than being left as dead code.

• Two opposite leniency defaults for the same edge case live in the same file. _denies_ability (used on the default-model branches) treats a model with no has_ability method as trusted; the explicit-model_id branches treat the same missing method as not having the ability. Verified this is not a live risk — every real model class (DashScopeImageModel, XinferenceImageModel, OpenAIImageModel, GeminiImageModel, ImageModelAdapter) implements has_ability concretely, so only a non-conforming test double can hit it. Hygiene/consistency point, not a blocker, but pick one default and document it.

• Open design question (not a defect): dropping list_image_models along with the others when neither ability exists is deliberate and tested, per the PR description. But the stated rationale — "models blind-retry a doomed tool" — only justifies removing tools that fail. list_image_models is read-only and cannot fail; it is the one tool that could let the agent (or a human reading the trace) answer "why is there nothing here". Was keeping it considered and rejected for a specific reason?

Line-level findings

Moderate

1. src/xagent/core/tools/core/image_tool.py:393-394 — the remediation text is circular in its most likely trigger state. _no_edit_model_error() unconditionally says "render what you need from a text prompt with generate_image instead of retrying edit_image". The realistic trigger is not "both abilities missing" (there, edit_image is unreachable via the schema anyway) — it is can_generate=True, can_edit=False, where generate_image(images=...) delegates into edit_image(). The model then receives "use generate_image instead" as the result of a generate_image call. The message should branch on has_generate_capable_model(): when generate is available, tell the model to retry generate_image without images; otherwise omit the suggestion. No test covers generate_image(..., images=...) in the generate-only state (the existing test that exercises the delegation configures a model with both abilities).

2. src/xagent/core/tools/core/image_tool.py:187-199 — the broad except Exception around create_image_tool() now swallows a much more consequential failure. create_image_tools_from_config wraps the whole construction — which now includes get_tools() and therefore every model.has_ability(...) call — in except Exception as e: logger.warning(...); return []. A model whose has_ability raises now silently removes all image tools, including the always-safe list_image_models, leaving only a warning line. Meanwhile the direct create_image_tool() path has no exception handling at all and propagates the same error raw. Same failure, two opposite outcomes depending on entry point, and neither is tested. (Out of the diff's line range, so no inline comment — but it is a direct consequence of moving ability checks into the registration path.)

Minor

3. src/xagent/core/tools/core/image_tool.py:399 — "No available image models configured." is factually wrong when models are configured but lack the generate ability. The PR's own new test test_generate_error_lists_abilities (tests/core/tools/test_image_tool.py:887-897) configures a real model1 with abilities=["edit"] and produces exactly this misleading lead sentence, immediately contradicted by the enumeration that follows it.

4. src/xagent/core/tools/core/image_tool.py:385 — an explicitly empty abilities list renders as "unknown". ", ".join(...) or "unknown" collapses "this model declares zero abilities" (a known fact) into "we could not determine its abilities" (an unknown). Render "none" for abilities == []. Untested — the neither-ability test only asserts the tool list is [], never the error string.

5. src/xagent/core/tools/core/image_tool.py:383-384 — non-list abilities is silently coerced to []. A malformed model class then reports "unknown" instead of surfacing the misconfiguration. Currently unreachable in production (the BaseImageModel.abilities contract returns a real sequence), so purely defensive-code hygiene — but a logger.warning would be cheap.

6. src/xagent/core/tools/core/image_tool.py:366-369 — _denies_ability's "missing method ⇒ trusted" default contradicts the explicit-model_id branches (lines ~320, ~334, ~345, ~360), which treat the same case as "lacks the ability". See the body note above; consistency nit only.

Test quality

7. tests/core/tools/test_image_tool.py:471 — assert "No image models" in image_tool._model_info_text pins dead internal state. On this path _model_info_text is never read: it is only consumed inside the if can_generate: branch, which this scenario by construction never enters. The behavioral assertion on the previous line (get_tools() == []) is the real test; this one would keep passing even if the text became meaningless.

8. tests/core/tools/test_image_tool.py:848 — assert "generate_image" in result["error"] is tautological. That string is a hardcoded literal in _no_edit_model_error() regardless of whether generate_image is actually registered; the assertion passes identically in the state where the recommendation is invalid. Combined with finding 1, this test gives false confidence that the recommendation is validated.

9. tests/core/tools/core/test_image_tool_core.py:226 and tests/core/tools/test_image_tool.py:241 — loosening == to in drops regression coverage of the new tail. The enumerated-models list and the remediation hint are now completely unasserted on these two paths. Adding a second assertion (e.g. assert "Configured image models:" in result["error"]) restores it without re-pinning the exact string.

10. Missing symmetric coverage for the generate side. test_default_edit_model_that_denies_edit_is_not_trusted (tests/core/tools/test_image_tool.py:850-861) covers a generate-only model configured as default_edit_model, but there is no counterpart for an edit-only model configured as default_generate_model — has_generate_capable_model() combined with a denying default_generate_model has zero references in either test file, even though _get_model and _get_edit_model were changed identically.

11. Style: the new tests use tool.name (tests/core/tools/test_image_tool.py:816, 831, 871) while the rest of the file uses tool.metadata.name. Both are correct; worth matching the surrounding file.

Out of scope, but worth a follow-up issue

src/xagent/skills/builtin/static-visual-design/SKILL.md (~lines 190-193 and ~326) and references/static-ad-art-direction.md (~line 296) still instruct the agent unconditionally to call edit_image or to pass reference images via images=. That skill text is injected into context verbatim with no awareness of which tools were registered, so on a generate-only deployment the agent is now told to use a tool that is not in its schema. Different subsystem, correctly not touched here — but it undercuts part of the win, and the PR description's "Out of scope" section does not mention it. Suggest filing a follow-up.

Simplification opportunities

• src/xagent/core/tools/core/image_tool.py L371-377: shrink — has_generate_capable_model() / has_edit_capable_model() are one-line is not None wrappers with exactly one caller each (src/xagent/core/tools/adapters/vibe/image_tool.py:72-73); inline them at the call site. (Do not collapse them into a single _has_capable_model(ability) — the dispatch costs more than it saves.)
• Considered and rejected: merging _no_edit_model_error / _no_generate_model_error (L389-403). Their opening sentences differ structurally, not by an ability-name substitution, so a template needs a third parameter and just relocates the duplication — and both exact strings are asserted verbatim in tests.

net: -6 lines possible

Prior review

gemini-code-assist[bot]'s inline comment on src/xagent/core/tools/adapters/vibe/image_tool.py:113 is still open: list_image_models's description retains "(Note: model information is already provided in the generate_image tool description)", which becomes misleading precisely in the new no-generate-capable-model state where generate_image is not registered. Not re-filing it inline — flagging it here as carried forward and unaddressed in the current head.

Note on verification: CI is the source of truth for test execution; nothing above is based on a local test run.

[bluefish-08]

Thanks for the depth here — this caught a class of problem the original PR reasoned about in one direction only. All 13 inline threads are addressed and resolved; below is what changed, what I found while fixing, and the two things I deliberately left alone.

Addressed

Error messages — _no_edit_model_error / _no_generate_model_error now branch instead of emitting one fixed string, because each state needs a different way out:

State	What the model is told
model_id given, another model can serve it	retry without model_id, or pick one from the enumerated list
Editing unavailable, generation works	retry generate_image without images — never "use generate_image" as the answer to a generate_image call
Neither ability configured	stop retrying image tools and report it

_no_generate_model_error got the same treatment: its old tail (Retry without model_id to use the default) was a no-op when no model_id was passed. The lead sentence is now "No available image models with generate capabilities", so it no longer contradicts the enumeration that follows it. Empty abilities renders as none rather than unknown, and a non-sequence abilities logs a warning instead of silently reporting unknown.

_denies_ability → _has_ability — one policy, fail closed. A model that does not declare the ability cannot serve it, whether it arrives as an explicit model_id, a configured default, or from the fallback scan. Verified no real model class is affected: every BaseImageModel subclass implements has_ability concretely, and create_retry_wrapper forwards it.

list_image_models is kept — your reasoning holds and is stronger than the PR's original rule. The "unusable tools must leave the schema" argument only covers tools that fail; a read-only listing cannot. Dropping it left silent absence as the agent's only signal. Note this makes the toolset length 1 rather than [] in the neither-ability state, so the acceptance line in #1295 ("no image tool is offered at all") no longer matches the implementation — I will update that issue.

Admin listing — _withheld_edit_image_row() in web/api/tools.py synthesizes the disabled row when capability gating keeps edit_image out of create_all_tools(), so the "add an image model with editing support" remediation survives. It only ever returns a missing_capability row, never an available one.

Simplification — has_generate_capable_model / has_edit_capable_model inlined and deleted, as suggested.

Tests — dead _model_info_text assertion dropped; the tautological "generate_image" in error replaced with an assertion on the actual remediation text; the Configured image models: tail re-covered on both paths loosened from == to in; symmetric coverage added for a denying default_generate_model; tool.name → tool.metadata.name.

Found while fixing

Your first finding had a second trigger the fix initially missed. _get_edit_model(model_id) returns None when the requested model cannot edit even if another configured model can, and the error did not look at model_id at all. So edit_image(model_id="generate_only") produced:

No available image models with edit capabilities. Configured image models: gen_only (abilities: generate); editor (abilities: generate, edit). Image editing is unavailable in this deployment…

— self-contradicting, and reachable in exactly the deployments where edit_image is registered. That is what the model_id branch in the table above fixes. Also added a test for a model object that never declares has_ability, which is what the _has_ability unification is for and had no coverage.

Not changed

1. The broad except Exception around create_image_tool() (adapters/vibe/image_tool.py). You are right that a has_ability that raises now takes down every image tool including list_image_models, and that the direct create_image_tool() path propagates the same error raw. But that handler and the asymmetry both predate this PR, and reaching it requires a model class that violates the BaseImageModel contract. Widening this PR to restructure error handling in the tool-construction path would mix two concerns; happy to file it separately.

2. The generate side of the admin listing. For symmetry, generate_image disappearing from the listing has the same gap when no model declares generate. I left it because that state means image models are configured but none can generate an image, which no real provider produces — adding a second synthetic row and a generate_image branch to _create_tool_info would be code for a state that cannot occur. Reconsidering if you disagree.

Follow-up

Agreed on static-visual-design/SKILL.md and references/static-ad-art-direction.md still instructing edit_image / images= unconditionally — that text is injected verbatim with no view of the registered toolset, so a generate-only deployment now gets told to use a tool absent from its schema. Different subsystem; filing separately rather than expanding this PR.