fix(agent): give generate_image a first-class native tool schema (validated args, model enum, owner-scoped)

[nsgds]

Summary

generate_image predates native tool-calling and was never promoted: it has a prose
guide section, a fenced-text parser, an MCP execution bridge and a retrieval-index
entry, but no entry in FUNCTION_TOOL_SCHEMAS — and the bundled image server is
not a registered MCP server, so mcp_schemas never carries it either. edit_image
(added later) has a schema. Since native advertisement is FUNCTION_TOOL_SCHEMAS ∩ relevant_tools, a native-tool model can be offered edit_image but never
generate_image, even when selection put both on the table:

[agent-intent] selected_tools=[... 'edit_image', 'generate_image' ...]
[agent-debug]  round=1 tools_sent=7 tool_names=[... 'edit_image' ...]   <- generate_image stripped

Two failure classes follow. Models that only act on advertised schemas can't generate
images at all — the tool is never in the tools array they're sent. Loose models call
it from the prose guide with hallucinated parameters (nothing ever validated the
arguments); observed live on dev:

Tool executed: generate_image: {"toolbench_rapidapi_key": "myapikey123", "image_type": "realistic", ...}

(ToolBench training-data residue; it "worked" only because the MCP server ignores
unknown keys.)

What this PR does:

1. Adds a first-class generate_image schema to FUNCTION_TOOL_SCHEMAS
   (prompt / model / size / quality) — advertisable through the existing filter exactly
   like edit_image; models compose arguments against the real parameter shape.
2. Enum-constrains the model parameter to the actually-installed image models,
   resolved lazily (once per turn, only when generate_image is being sent). Fails open
   if discovery fails.
3. Owner-scopes resolution and the gallery row on the agent path: the trusted _owner
   is injected server-side by the tool-execution bridge — never a model-controlled
   schema field — and threads through _resolve_model(..., owner=...) so a request can
   never resolve against another user's private image endpoint or spend their API key.
   The contract is centralised in _apply_trusted_owner, so a model-issued fully
   qualified mcp__image_gen__generate_image call takes the identical scrub-and-inject
   path (addresses the P2 from review).

Since the last review round: rebased onto current dev; adopted upstream's
non-blocking resolution (await asyncio.to_thread(_resolve_model, cand, owner=owner) —
upstream's _resolve_model accepts owner natively since the rebase); owner-scope
tests pin their settings reads hermetically (a dirty process-wide settings cache could
short-circuit call_tool before resolution, which also let one test pass vacuously —
it now asserts the resolver was reached; supersedes the inline pins from 4650994).

Companion PR #3606 fixes the classifier half of the linked issue (image requests never
deterministically select the tool); this PR makes the call first-class once selection
happens.

Target branch

• This PR targets dev, not main.

Linked Issue

Part of #3605 (defect 2 of 2 — see the issue for the full pipeline analysis)

Type of Change

• Bug fix (non-breaking — fixes a confirmed issue)

Checklist

• I searched open issues and open PRs — this is not a duplicate.
• This PR targets dev
• My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
• I actually ran the app (docker compose up) and verified the change works end-to-end. Type-checks and unit tests are not enough.

How to Test

1. Unit: python -m pytest tests/test_generate_image_native.py tests/test_generate_image_model_enum.py tests/test_generate_image_owner_scope.py -q
   (includes qualified-name and builtin-name _owner spoof regressions on both
   dispatch paths).
2. Behavioral, on dev without this PR (native-tool endpoint): in an existing
   conversation — or with web search on; both avoid the unrelated classifier gate
   (fix(agent): add image domain to tool-RAG classifier #3606) — ask the agent to generate an image and watch the
   [agent-debug] round=1 ... tool_names= log line: generate_image is absent even
   when relevant_tools contains it; edit_image is present.
3. With this PR: generate_image appears in tool_names, the call arrives as a real
   native tool_calls object with schema-shaped arguments, and the model argument is
   constrained to installed image models (observed live:
   {"model": "Qwen-Image-NVFP4", "prompt": ..., "size": ...}).
4. Owner isolation: create a private image endpoint owned by user A; as user B, an
   agent image request must not resolve to it — covered by
   test_resolve_model_isolates_private_image_endpoint_by_owner, which reproduces the
   pre-fix leak against a real in-memory DB.

Visual / UI changes — REQUIRED if you touched anything that renders

Not applicable — schema/advertisement/execution layer only; no rendering code touched.

[RaresKeY]

Findings

issue (security): Native image calls lose the caller owner before resolving endpoints

• Problem: The new native schema makes generate_image callable by tool-supporting models, and the schema enum is built with the caller owner, but execution still goes through the built-in image MCP server. execute_tool_block() routes generate_image to the MCP bridge without passing owner context; the MCP server then calls _resolve_model(cand) with no owner and writes the gallery row without an owner.

• Impact: In a multi-user deployment, a user-visible enum does not guarantee the generation uses that user's endpoint. A request can resolve against another user's private image endpoint/API key, send the prompt to the wrong service, spend the wrong quota, and save the generated row as ownerless compatibility data.

• Ask: Keep native generate_image execution on the existing owner-aware in-process path, or pass trusted server-side owner context through the MCP bridge and use it for both _resolve_model(..., owner=owner) and GalleryImage.owner. Do not expose owner as a model-controlled schema field.

• Location: mcp_servers/image_gen_server.py:83

issue (performance): Image model discovery runs before knowing the image tool will be sent

• Problem: Every agent turn computes the installed image-model enum before checking whether the selected endpoint uses native schemas, before relevant-tool filtering, before disabled-tool filtering, and before confirming generate_image is part of the schemas sent this round. The helper then walks enabled image endpoints and can issue a live /models request with a 3s timeout per endpoint.

• Impact: Normal agent requests that have nothing to do with images can stall once per owner/cache TTL and can generate avoidable traffic to down, slow, manual, or cloud image endpoints.

• Ask: Defer image-model discovery until after schema filtering proves generate_image will actually be sent this round, and prefer cached endpoint model lists or existing refresh policy before issuing live probes.

• Location: src/agent_loop.py:2118

Validation

Ran focused parser/schema/tool-policy/image privilege tests and checked the current PR checks. I did not run a live image endpoint.

[nsgds]

Thanks @RaresKeY — both findings were spot on. Addressed in d67a712 (fix) + 46451ea (tests).

P1 (owner-scoping). Took your second option — thread a trusted server-side owner through the MCP bridge, rather than re-routing to the in-process do_generate_image — since it's the smaller change and keeps the candidates-fallback / raise-on-error hardening this PR already added. Specifically:

• _call_mcp_tool injects _owner only for generate_image, after arg parsing — it's never a schema field, so the model can't control or spoof it (_parse_generate_image drops any model-supplied owner, and the enum schema doesn't expose one).
• The image MCP server uses it for both _resolve_model(cand, owner=owner) and GalleryImage(owner=...).

The existing test_ai_interaction_owner_scope pins do_generate_image (the UI path), but the agent runs the MCP server, so I added coverage for that path: source-inspection guards on the bridge + server, plus a runtime isolation test — a private image endpoint owned by alice on an in-memory DB is invisible to bob through _resolve_model (real owner_filter). bob is blocked with zero probes to alice's endpoint (prompt + key never sent), while the ownerless path (pre-fix behaviour) resolves it and sends alice's key — so the leak is reproduced and closed.

P2 (eager discovery). Made the installed-model lookup lazy — it now runs only once generate_image is actually in the schemas sent that round (sentinel-guarded, at most once per turn, still cached + off the event loop), so image-free turns no longer probe image endpoints.

[RaresKeY]

Thanks for the update. The wrapper path is improved, but I found one remaining owner-boundary gap through the qualified MCP tool name.

Findings

issue: Qualified image MCP calls bypass trusted owner injection

• Problem: The wrapper path now injects _owner for generate_image, but the generic qualified-MCP branch still passes JSON args directly to MCP for any mcp__... tool name. A direct mcp__image_gen__generate_image call can therefore carry a model-controlled _owner instead of the caller owner.

• Impact: In a tool-allowed execution path, the image tool can resolve another user's private image endpoint/API key and persist the gallery row under the spoofed owner.

• Ask: Either reject the built-in image tool when it arrives through the generic qualified-MCP branch, or scrub/overwrite _owner there with the trusted caller owner just like the wrapper path does. Please add a regression for the qualified tool name as well as the wrapper name.

• Location: src/tool_execution.py:886

Validation

The focused image-owner/native/model-enum tests pass on the current head. A direct qualified-MCP probe still reaches the MCP layer with the _owner supplied in the tool-call JSON.

[nsgds]

Thanks — fixed in 4650994, and rebased onto current dev.

P2 (qualified mcp__image_gen__generate_image bypasses owner injection). Centralised the owner contract in _apply_trusted_owner(name, args, owner): it drops any model-supplied _owner (a server-side-only field) and re-injects the trusted caller owner for owner-scoped tools — keyed by both the builtin name and the qualified mcp__ name. Both dispatch paths now call it (_call_mcp_tool and the generic mcp__ branch in _execute_tool_block_impl), so a direct qualified call can no longer carry a model-controlled _owner to resolve another user's endpoint/key or mis-tag the gallery row. Regressions cover the qualified name and the builtin name, a strip on a non-owner-scoped mcp__ tool, the no-owner case, and a guard that both paths call the helper.

Good timing on #4335 — it landed the same owner-injection for mcp__email__ in this exact branch while I was on it, so I slotted the image scrub right beside the email injection rather than duplicating the pattern.

One small heads-up from being in there: _apply_trusted_owner strips _owner unconditionally (a model can never supply it, even when the caller owner is None), whereas the email path only overwrites _odysseus_owner if … and owner:. So a model-supplied _odysseus_owner would pass through when no owner is threaded. It's a narrow window — only an ownerless/single-user call, where there's no boundary to cross — but if it's useful I'm happy to fold _odysseus_owner into the same unconditional scrub so both built-in tools are symmetric. Didn't want to touch #4335's arg handling in this PR without your nod.

One extra from the rebase: it surfaced a pre-existing CI-only isolation flake in the existing test_server_uses_trusted_underscore_owner_not_model_owner. It reads image_gen_enabled from a process-wide cache another test can leave dirty; when that reads falsy, call_tool short-circuits before _resolve_model, so the captured owner is empty and the assert becomes a false negative (passes locally + in isolation, fails under full-suite ordering). Pinned the setting in the two server tests so they deterministically reach resolution.

[RaresKeY]

Findings

issue (agent): Resolved text models still stop image-model fallback

• Problem: The fallback loop accepts the first candidate that _resolve_model() can find, but _resolve_model() searches enabled chat/text endpoints too. If a native call supplies a real text model name such as gpt-4o-mini, the loop breaks before trying the configured image model, then posts to that text endpoint's /images/generations URL and raises on the provider error.

• Impact: Image generation can still fail for native-tool models that emit a known text model, providers that ignore the injected enum, or turns where enum discovery failed open, even when a valid configured image model is available.

• Ask: Treat a candidate as selected only after it resolves to an image-capable endpoint/model, or continue to the next candidate when the resolved endpoint rejects /images/generations as non-image. Please add a focused regression where a requested text model resolves first and the configured image endpoint is tried next.

• Location: mcp_servers/image_gen_server.py:85

Validation

Focused native/model-enum/owner tests pass on the current head, and a fake text-endpoint/configured-image-endpoint probe reproduces the fallback gap. I did not run a live image endpoint.

[nsgds]

Sorry for going quiet for a couple of weeks! Rebased onto current dev and re-verified. Changes since the last review round:

• Adopted upstream's non-blocking resolution: the owner-scoped call is now
  await asyncio.to_thread(_resolve_model, cand, owner=owner) (upstream's
  _resolve_model accepts owner natively since the rebase).
• The P2 fix (qualified mcp__image_gen__generate_image bypassing owner injection)
  is carried unchanged — owner contract stays centralized in the bridge.
• Owner-scope tests hardened: settings reads are pinned hermetically (a dirty
  process-wide settings cache could short-circuit call_tool before resolution,
  which also let the no-owner test pass vacuously — it now asserts the resolver was
  reached). Supersedes the inline pins from 4650994.

Also rewrote the body: testing on current dev showed loose models can already
invoke the tool from the prose guide — with hallucinated parameters (real example in
the body: toolbench_rapidapi_key). So the accurate claim is validated native
advertisement + model-name enum + owner scoping, not bare invocability. Repro guidance
in the body now routes around the classifier bug (#3606) so this PR can be tested in
isolation.

[RaresKeY]

I checked the latest head against current dev and found one additional issue in the native image-model enum path.

Findings

issue (model enum): Native model-list responses bypass the validation enum

• Problem: list_image_model_ids() calls build_models_url(), which selects native Ollama's /api/tags endpoint, but only reads OpenAI-style data[].id. It also discards top-level model lists, even though _resolve_model() handles both those forms and native models[].name.
• Impact: On a supported native endpoint, a tool-calling model can again choose an unavailable model name; with no configured default, image generation fails even though a custom installed model exists because discovery never constrains or retries that model.
• Ask: Normalize top-level-list, data[].id, and models[].name/model responses in list_image_model_ids(), then add focused coverage for an Ollama /api/tags payload and a bare-list payload that verifies the emitted native schema contains the expected enum.
• Location: src/ai_interaction.py:207-216, src/endpoint_resolver.py:263-264

Validation

• Focused native-schema, enum, and owner-scope tests pass, but they do not cover the /api/tags or bare-list response shapes.
• A hermetic enabled-Ollama-endpoint probe reproduces the empty discovery result; live provider validation was not run.