Local text-to-image fallback violates the return_url contract #10388

Activity

1. LIghtJUNction commented on Oct 5, 2026

   

   LIghtJUNction

   on Oct 5, 2026 – with ChatGPT Codex Connector

   Contributor

   More actions

   Confirmed on master@42972e9. The important implementation boundary is that changing LocalRenderStrategy or HtmlRenderer globally would break a supported core path; the public Star wrapper is the right place to enforce the URL contract.

   Code evidence

   • astrbot/core/star/base.py::Star.text_to_image() documents return_url=True as returning an image URL and forwards the flag to HtmlRenderer.render_t2i() without checking the returned kind.
   • astrbot/core/utils/t2i/renderer.py::HtmlRenderer.render_t2i() forwards return_url to NetworkRenderStrategy.render(), but on an ordinary remote failure it calls self.local_strategy.render(text) without the flag.
   • More fundamentally, astrbot/core/utils/t2i/local_strategy.py::LocalRenderStrategy.render() explicitly deletes return_url and always returns save_temp_img(image), a local path. Passing the flag through the fallback alone therefore would not satisfy the contract.
   • astrbot/core/pipeline/result_decorate/stage.py directly calls html_renderer.render_t2i(..., return_url=True) and intentionally accepts either kind: HTTP results become Image.fromURL, while local paths are optionally registered with the file service or become Image.fromFileSystem. That built-in caller relies on best-effort renderer semantics.
   • tests/unit/test_star_base.py only covers renderer argument forwarding and an already-remote URL; it has no local-fallback contract assertion.

   The confirmed chain is:

   Star.text_to_image(return_url=True) → remote render fails (or local-only rendering is selected below the wrapper) → local strategy saves a file → wrapper returns that path unchanged → plugin receives a value that contradicts the documented result kind.

   Suggested change

   Keep HtmlRenderer's mixed URL/path behavior for internal callers, and enforce the public contract in Star.text_to_image() after rendering:

   result = await self.context.html_renderer.render_t2i(...)
   if not return_url or result.startswith(("http://", "https://")):
       return result
   
   callback_base = (config or {}).get("callback_api_base", "").rstrip("/")
   if not callback_base:
       raise RuntimeError("return_url=True requires callback_api_base for local T2I output")
   
   token = await file_token_service.register_file(result)
   return f"{callback_base}/api/file/{token}"

   The exact URL check should use a parser or the project's existing URL helper if available rather than treating every string beginning with http as valid. The registration should happen only at this public boundary; doing it inside LocalRenderStrategy would couple the renderer to session/global configuration and would force URL semantics on the result-decoration path that already handles local files correctly.

   Also decide/document token lifetime and single-use behavior. FileTokenService.register_file() currently defaults to a five-minute, single-use token. If plugin callers are expected to reuse the returned URL, pass an appropriate lifetime/single_use=False; otherwise document the current ephemeral contract. If there is no configured externally reachable callback_api_base, raising is preferable to returning a disguised local path.

   Observable result and compatibility

   Plugin code requesting a URL will receive an actual HTTP(S) file-service URL after local fallback, or a clear exception explaining the missing prerequisite. return_url=False continues returning a path. Core automatic T2I keeps its existing remote/local fallback, including Image.fromFileSystem when the file service is disabled. The extra token registration happens only after a remote failure/local result, so successful remote rendering has no additional I/O.

   Tests

   Extend tests/unit/test_star_base.py and the T2I tests with:

   1. remote URL + return_url=True is returned unchanged and not registered;
   2. local path + return_url=False remains a path;
   3. local path + return_url=True + callback base registers the exact file and returns the file-service URL;
   4. the same request without callback base raises a stable, actionable error;
   5. missing local file propagates FileNotFoundError rather than returning a broken URL;
   6. query/trailing-slash normalization for callback_api_base;
   7. ResultDecorateStage still accepts a local fallback path and emits a filesystem image when its file-service option is off.

   This is separate from #10389/#10394: that PR correctly fixes cancellation propagation, but deliberately preserves the ordinary-error local fallback and therefore does not resolve this return-kind mismatch.
2. ZXLyyds100 commented on Oct 17, 2026

   

   ZXLyyds100

   on Oct 17, 2026

   More actions

   I'd like to address this. I checked the current Star.text_to_image() wrapper and the issue timeline/comments, including the related open T2I cancellation and image-download PRs; those do not fix this wrapper's URL-vs-local-path contract.

   I'll keep the lower-level renderer and automatic core T2I behavior unchanged. At the public Star boundary, successful remote URLs will pass through; local output requested as a URL will use the existing tokenized file service when a valid callback base is configured, otherwise raise an actionable error directing callers to configure that base or request a local path. Local-path requests will remain unchanged. I'll document the existing file-token lifetime and test repeated access, missing files, session-specific configuration, and the no-callback case.

   Validation will include a real local HTTP renderer failure and actual local image rendering, not only a mocked renderer return value. Disclosure: investigation, implementation, tests, self-review, and this comment are Codex-assisted; no independent human review or maintainer approval is implied.
3. ZXLyyds100 commented on Oct 17, 2026

   

   ZXLyyds100

   on Oct 17, 2026

   Last edited by ZXLyyds100

   More actions

   Implemented in #10597. The fix stays at the public Star.text_to_image() boundary; lower-level/core fallback behavior is unchanged.

   After addressing automated review in 1874e38, the PR includes 30 new cases (final full suite: 4031 passed / 4 original skips) and refreshed executable before/after evidence. The screenshot is labelled as a measured service report, not a new WebUI; HTTP failure/local rendering and production ASGI file downloads are real, while expiry is controlled rather than a five-minute wall-clock soak.

   Compatibility impact and the temporary URL's access boundary are explicit in the PR and Chinese/English guides. Earlier transient DNS failures and unchanged-base controls are retained alongside the final green run. Combined with #10596, 4065 tests passed / 4 original skips. Implementation, tests and self-review are Codex-assisted; upstream human review/CI remain separate.