From f9d68de2bd05b0c0caa8eebb9467dc9d936502e1 Mon Sep 17 00:00:00 2001 From: rookiestar28 Date: Fri, 27 Mar 2026 13:39:16 +0800 Subject: [PATCH] refactor: add bounded asset surface interop --- services/comfyui_history.py | 21 +++- tests/test_comfyui_history_parsing.py | 27 +++++ tests/test_s50_durable_idempotency.py | 21 ++-- web/openclaw_api.js | 9 ++ web/openclaw_asset_refs.js | 74 ++++++++++++++ web/tabs/job_monitor_tab.js | 30 ++---- web/tests/unit/openclaw_asset_refs.test.js | 111 +++++++++++++++++++++ 7 files changed, 258 insertions(+), 35 deletions(-) create mode 100644 web/openclaw_asset_refs.js create mode 100644 web/tests/unit/openclaw_asset_refs.test.js diff --git a/services/comfyui_history.py b/services/comfyui_history.py index 1d592d3..b47d059 100644 --- a/services/comfyui_history.py +++ b/services/comfyui_history.py @@ -55,17 +55,27 @@ def extract_images(history_item: Dict[str, Any]) -> List[Dict[str, str]]: for node_id, node_output in outputs.items(): images = node_output.get("images", []) for img in images: - filename = img.get("filename", "") + asset_hash = "" + if isinstance(img.get("asset_hash"), str): + asset_hash = img.get("asset_hash", "").strip() + elif isinstance(img.get("asset"), dict): + asset_hash = str(img.get("asset", {}).get("asset_hash", "")).strip() + + filename = img.get("filename") or img.get("name") or asset_hash subfolder = img.get("subfolder", "") img_type = img.get("type", "output") if not filename: continue - # Build /view URL - params = {"filename": filename, "type": img_type} - if subfolder: - params["subfolder"] = subfolder + # IMPORTANT: asset-backed refs must still resolve through /view so + # callback consumers stay compatible with classic history behavior. + if asset_hash: + params = {"filename": asset_hash} + else: + params = {"filename": filename, "type": img_type} + if subfolder: + params["subfolder"] = subfolder view_url = f"{COMFYUI_URL}/view?{urlencode(params)}" @@ -74,6 +84,7 @@ def extract_images(history_item: Dict[str, Any]) -> List[Dict[str, str]]: "filename": filename, "subfolder": subfolder, "type": img_type, + "asset_hash": asset_hash, "view_url": view_url, } ) diff --git a/tests/test_comfyui_history_parsing.py b/tests/test_comfyui_history_parsing.py index 5061791..155d5ec 100644 --- a/tests/test_comfyui_history_parsing.py +++ b/tests/test_comfyui_history_parsing.py @@ -75,6 +75,33 @@ class TestComfyUIHistoryParsing(unittest.TestCase): images = extract_images(history_item) self.assertEqual(len(images), 0) + def test_extract_images_prefers_asset_hash_view_url(self): + from services.comfyui_history import extract_images + + history_item = { + "outputs": { + "2": { + "images": [ + { + "filename": "preview.png", + "subfolder": "nested", + "type": "temp", + "asset_hash": "blake3:abc123", + } + ] + } + } + } + + images = extract_images(history_item) + self.assertEqual(len(images), 1) + self.assertEqual(images[0]["filename"], "preview.png") + self.assertEqual(images[0]["type"], "temp") + self.assertEqual(images[0]["asset_hash"], "blake3:abc123") + self.assertIn("filename=blake3%3Aabc123", images[0]["view_url"]) + self.assertNotIn("subfolder=nested", images[0]["view_url"]) + self.assertNotIn("type=temp", images[0]["view_url"]) + def test_get_job_status(self): from services.comfyui_history import get_job_status diff --git a/tests/test_s50_durable_idempotency.py b/tests/test_s50_durable_idempotency.py index 752ccd4..bbf037a 100644 --- a/tests/test_s50_durable_idempotency.py +++ b/tests/test_s50_durable_idempotency.py @@ -61,16 +61,21 @@ class TestSQLiteDurableBackend(unittest.TestCase): def test_ttl_expiry(self): """Test TTL expiry.""" - # Insert with short TTL - self.backend.check_and_record("key_ttl", 1) - time.sleep(2) # Wait for expiry + # IMPORTANT: drive the backend clock explicitly so this regression stays + # deterministic under the full suite instead of depending on wall time. + with patch( + "services.idempotency_store.time.time", + side_effect=[100.0, 102.0, 102.0], + ): + # Insert with short TTL + self.backend.check_and_record("key_ttl", 1) - # Cleanup should remove it - self.backend.cleanup() + # Cleanup should remove it + self.backend.cleanup() - # Should be fresh again - # Impl: if fresh, returns (False, None) -> is_dup=False - is_dup, pid = self.backend.check_and_record("key_ttl", 3600) + # Should be fresh again + # Impl: if fresh, returns (False, None) -> is_dup=False + is_dup, pid = self.backend.check_and_record("key_ttl", 3600) self.assertFalse(is_dup) diff --git a/web/openclaw_api.js b/web/openclaw_api.js index f9033f6..709ae66 100644 --- a/web/openclaw_api.js +++ b/web/openclaw_api.js @@ -6,6 +6,7 @@ import { OpenClawSession } from "./openclaw_session.js"; import { fetchApi, apiURL, fileURL } from "./openclaw_comfy_api.js"; import { API_PREFIXES, buildAdminTokenHeaders, getApiPathCandidates } from "./openclaw_compat.js"; import { isAbortError, linkAbortSignal, parseJsonSafe } from "./openclaw_utils.js"; +import { normalizeComfyOutputRef } from "./openclaw_asset_refs.js"; import { composeFetchWrappersOnce, withAbortPassthrough, @@ -234,6 +235,14 @@ export class OpenClawAPI { return apiURL(`/view?${params.toString()}`); } + buildViewUrlForRef(imageRef) { + const normalized = normalizeComfyOutputRef(imageRef); + if (!normalized) { + return ""; + } + return apiURL(`/view?${new URLSearchParams(normalized.viewParams).toString()}`); + } + // R21/F20: Get config async getConfig() { return this.fetch(this._path("/config")); diff --git a/web/openclaw_asset_refs.js b/web/openclaw_asset_refs.js new file mode 100644 index 0000000..e465dbc --- /dev/null +++ b/web/openclaw_asset_refs.js @@ -0,0 +1,74 @@ +function pickAssetHash(imageRef = {}) { + if (!imageRef || typeof imageRef !== "object") { + return ""; + } + const direct = typeof imageRef.asset_hash === "string" ? imageRef.asset_hash.trim() : ""; + if (direct) { + return direct; + } + const nested = imageRef.asset; + if (nested && typeof nested === "object" && typeof nested.asset_hash === "string") { + return nested.asset_hash.trim(); + } + return ""; +} + +function pickFilename(imageRef = {}) { + if (!imageRef || typeof imageRef !== "object") { + return ""; + } + if (typeof imageRef.filename === "string" && imageRef.filename.trim()) { + return imageRef.filename.trim(); + } + if (typeof imageRef.name === "string" && imageRef.name.trim()) { + return imageRef.name.trim(); + } + return ""; +} + +export function normalizeComfyOutputRef(imageRef = {}) { + const assetHash = pickAssetHash(imageRef); + const filename = pickFilename(imageRef) || assetHash; + const subfolder = typeof imageRef.subfolder === "string" ? imageRef.subfolder : ""; + const type = typeof imageRef.type === "string" && imageRef.type ? imageRef.type : "output"; + + if (!filename) { + return null; + } + + // IMPORTANT: asset-backed refs still resolve through /view; do not turn this + // helper into a direct /api/assets dependency or classic history parity breaks. + const viewParams = assetHash + ? { filename: assetHash } + : { + filename, + type, + ...(subfolder ? { subfolder } : {}), + }; + + return { + filename, + subfolder, + type, + asset_hash: assetHash || "", + is_asset_backed: Boolean(assetHash), + viewParams, + }; +} + +export function extractHistoryImageRefs(historyItem = {}) { + const results = []; + const outputs = historyItem && typeof historyItem === "object" ? (historyItem.outputs || {}) : {}; + + for (const nodeOutput of Object.values(outputs)) { + const images = Array.isArray(nodeOutput?.images) ? nodeOutput.images : []; + for (const imageRef of images) { + const normalized = normalizeComfyOutputRef(imageRef); + if (normalized) { + results.push(normalized); + } + } + } + + return results; +} diff --git a/web/tabs/job_monitor_tab.js b/web/tabs/job_monitor_tab.js index 9f42520..0353bd6 100644 --- a/web/tabs/job_monitor_tab.js +++ b/web/tabs/job_monitor_tab.js @@ -3,6 +3,7 @@ * Tracks prompt execution and displays outputs. */ import { openclawApi } from "../openclaw_api.js"; +import { extractHistoryImageRefs } from "../openclaw_asset_refs.js"; import { parseJsonSafe } from "../openclaw_utils.js"; const POLL_INTERVAL_MS = 2000; @@ -264,26 +265,11 @@ async function startPolling(promptId, onUpdate) { } function extractImages(historyItem) { - const results = []; - const outputs = historyItem.outputs || {}; - - for (const nodeId in outputs) { - const images = outputs[nodeId].images || []; - for (const img of images) { - if (!img.filename) continue; - const params = new URLSearchParams({ - filename: img.filename, - type: img.type || "output", - }); - if (img.subfolder) params.set("subfolder", img.subfolder); - - results.push({ - filename: img.filename, - subfolder: img.subfolder || "", - type: img.type || "output", - view_url: openclawApi.buildViewUrl(img.filename, img.subfolder || "", img.type || "output"), - }); - } - } - return results; + return extractHistoryImageRefs(historyItem).map((img) => ({ + filename: img.filename, + subfolder: img.subfolder, + type: img.type, + asset_hash: img.asset_hash, + view_url: openclawApi.buildViewUrlForRef(img), + })); } diff --git a/web/tests/unit/openclaw_asset_refs.test.js b/web/tests/unit/openclaw_asset_refs.test.js new file mode 100644 index 0000000..14ab96d --- /dev/null +++ b/web/tests/unit/openclaw_asset_refs.test.js @@ -0,0 +1,111 @@ +import { describe, expect, it } from "vitest"; + +import { extractHistoryImageRefs, normalizeComfyOutputRef } from "../../openclaw_asset_refs.js"; + +describe("openclaw asset refs", () => { + it("keeps classic history refs on the /view filename+type contract", () => { + expect( + normalizeComfyOutputRef({ + filename: "result.png", + subfolder: "session-a", + type: "temp", + }) + ).toEqual({ + filename: "result.png", + subfolder: "session-a", + type: "temp", + asset_hash: "", + is_asset_backed: false, + viewParams: { + filename: "result.png", + subfolder: "session-a", + type: "temp", + }, + }); + }); + + it("prefers asset hashes while keeping display filename metadata", () => { + expect( + normalizeComfyOutputRef({ + filename: "preview.png", + type: "output", + asset_hash: "blake3:abc123", + }) + ).toEqual({ + filename: "preview.png", + subfolder: "", + type: "output", + asset_hash: "blake3:abc123", + is_asset_backed: true, + viewParams: { + filename: "blake3:abc123", + }, + }); + }); + + it("accepts upload-style nested asset metadata", () => { + expect( + normalizeComfyOutputRef({ + name: "uploaded.png", + asset: { + asset_hash: "blake3:def456", + }, + }) + ).toEqual({ + filename: "uploaded.png", + subfolder: "", + type: "output", + asset_hash: "blake3:def456", + is_asset_backed: true, + viewParams: { + filename: "blake3:def456", + }, + }); + }); + + it("extracts mixed history outputs without dropping temp classifications", () => { + expect( + extractHistoryImageRefs({ + outputs: { + "1": { + images: [ + { + filename: "classic.png", + subfolder: "", + type: "output", + }, + { + filename: "temp-preview.png", + subfolder: "preview", + type: "temp", + asset_hash: "blake3:temp123", + }, + ], + }, + }, + }) + ).toEqual([ + { + filename: "classic.png", + subfolder: "", + type: "output", + asset_hash: "", + is_asset_backed: false, + viewParams: { + filename: "classic.png", + type: "output", + }, + }, + { + filename: "temp-preview.png", + subfolder: "preview", + type: "temp", + asset_hash: "blake3:temp123", + is_asset_backed: true, + viewParams: { + filename: "blake3:temp123", + }, + }, + ]); + }); +});