admin/ComfyUI-OpenClaw
1
0
Fork 0
mirror of https://github.com/rookiestar28/ComfyUI-OpenClaw.git synced 2026-08-14 08:52:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(outputs): preserve explicit asset api fallback contract

Browse Source
This commit is contained in:
rookiestar28
2026-04-16 14:56:17 +08:00
parent f4ddb22233
commit d0acea3952
9 changed files with 424 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/r167_asset_api_adoption_decision.md
+49
View File
@@ -0,0 +1,49 @@
# R167 ComfyUI Asset API Adoption Decision (2026-04-16)
## Scope
- Item: `R167` from the active roadmap (`ComfyUI asset API adoption decision and bounded phase-2 interop seam`).
- Goal: decide whether OpenClaw should adopt upstream `/api/assets` semantics as a normal runtime dependency beyond the `R165` bounded `/view` interoperability layer.
## Current baseline
- Current history/output-facing interop already accepts:
- classic ComfyUI output refs (`filename`, `subfolder`, `type`)
- asset-hash-backed refs that still resolve through `/view?filename=blake3:...`
- Current operator/runtime surfaces in scope:
- sidebar `Jobs`
- callback delivery payloads
- history/result consumption paths derived from `services.comfyui_history`
- Current non-goal:
- no gallery/explorer/runtime flow currently requires direct `/api/assets` fetches to stay functional.
## Decision
- **No-go for first-class `/api/assets` runtime adoption in phase 2.**
- OpenClaw keeps `/history` + `/view` as the supported runtime contract for normal output handling.
- Asset-api-only identifiers are now treated as explicit unsupported contracts rather than implicit fetch targets.
## Rationale
1. Current OpenClaw output surfaces still succeed on the existing bounded `/view` contract, including asset-hash-backed refs.
2. Adding `/api/assets` as a normal dependency would widen runtime coupling to upstream host behavior without a demonstrated operator need in current features.
3. A silent fallback from `asset id only` to `/api/assets` would weaken boundary clarity and make host drift harder to reason about.
## Approved phase-2 seam
- Preserve current supported refs exactly:
- classic refs -> `/view?filename=...&type=...`
- asset-hash-backed refs -> `/view?filename=blake3:...`
- For refs that expose only asset-service identifiers and are not representable through `/view`:
- keep them in normalized output payloads
- mark them as `asset_api_required`
- do not auto-fetch `/api/assets`
- surface a bounded operator-facing message where relevant
## Re-open triggers
Revisit this decision only if one of the following becomes true:
1. A current operator-facing surface cannot complete its supported workflow without direct `/api/assets` semantics.
2. Upstream ComfyUI stops providing `/view`-compatible output metadata for supported runtime flows.
3. OpenClaw intentionally adds a new asset-management feature whose documented contract depends on asset-service metadata beyond hash-backed preview resolution.
services/comfyui_history.py
+77 -35
View File
@@ -25,6 +25,78 @@ COMFYUI_URL = (
HISTORY_TIMEOUT = 5
def _pick_string(payload: Dict[str, Any], *keys: str) -> str:
for key in keys:
value = payload.get(key)
if isinstance(value, str) and value.strip():
return value.strip()
return ""
def _pick_asset_hash(image_ref: Dict[str, Any]) -> str:
asset_hash = _pick_string(image_ref, "asset_hash")
if asset_hash:
return asset_hash
nested = image_ref.get("asset")
if isinstance(nested, dict):
return _pick_string(nested, "asset_hash")
return ""
def _pick_asset_api_id(image_ref: Dict[str, Any]) -> str:
asset_api_id = _pick_string(image_ref, "asset_id")
if asset_api_id:
return asset_api_id
nested = image_ref.get("asset")
if isinstance(nested, dict):
return _pick_string(nested, "asset_id", "id")
return ""
def normalize_history_image_ref(image_ref: Dict[str, Any]) -> Optional[Dict[str, Any]]:
if not isinstance(image_ref, dict):
return None
asset_hash = _pick_asset_hash(image_ref)
asset_api_id = _pick_asset_api_id(image_ref)
named_filename = _pick_string(image_ref, "filename", "name")
filename = named_filename or asset_hash or asset_api_id
subfolder = _pick_string(image_ref, "subfolder")
img_type = _pick_string(image_ref, "type") or "output"
if not filename:
return None
asset_api_required = bool(asset_api_id and not asset_hash and not named_filename)
view_url = ""
resolution = "asset_api_required" if asset_api_required else "view"
if not asset_api_required:
# IMPORTANT: keep OpenClaw on the bounded /view contract. Asset-hash refs
# are accepted because they still resolve through /view; do not escalate
# asset-api-only identifiers into implicit /api/assets runtime fetches.
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)}"
return {
"filename": filename,
"subfolder": subfolder,
"type": img_type,
"asset_hash": asset_hash,
"asset_api_id": asset_api_id,
"asset_api_required": asset_api_required,
"resolution": resolution,
"view_url": view_url,
}
def fetch_history(prompt_id: str) -> Optional[Dict[str, Any]]:
"""
Fetch history for a given prompt_id from ComfyUI.
@@ -44,10 +116,10 @@ def fetch_history(prompt_id: str) -> Optional[Dict[str, Any]]:
return None
def extract_images(history_item: Dict[str, Any]) -> List[Dict[str, str]]:
def extract_images(history_item: Dict[str, Any]) -> List[Dict[str, Any]]:
"""
Extract image outputs from a history item.
Returns list of { filename, subfolder, type, view_url }.
Returns list of normalized image refs.
"""
results = []
outputs = history_item.get("outputs", {})
@@ -55,39 +127,9 @@ 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:
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
# 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)}"
results.append(
{
"filename": filename,
"subfolder": subfolder,
"type": img_type,
"asset_hash": asset_hash,
"view_url": view_url,
}
)
normalized = normalize_history_image_ref(img)
if normalized:
results.append(normalized)
return results
tests/e2e/specs/r107_live_backend.spec.js
+79
View File
@@ -123,6 +123,85 @@ test.describe('R107 Live Backend Parity', () => {
await expect(page.locator('img[src*="test_img.png"]')).toBeVisible();
});
test('Job Monitor keeps the phase-2 asset API no-go contract explicit', async ({ page }) => {
const jobId = "job-asset-phase2";
let assetApiCalls = 0;
await page.route(`**/history/${jobId}`, async route => {
await route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({
[jobId]: {
status: { status_str: "success", completed: true },
outputs: {
"9": {
images: [
{
filename: "preview.png",
type: "temp",
asset_hash: "blake3:abc123",
},
{
asset: {
id: "asset-only-42",
},
},
],
},
},
},
}),
});
});
await page.route('**/openclaw/trace/**', async route => {
await route.fulfill({
status: 404,
contentType: 'application/json',
body: JSON.stringify({ error: 'not_found' }),
});
});
await page.route('**/api/assets**', async route => {
assetApiCalls += 1;
await route.fulfill({
status: 500,
contentType: 'application/json',
body: JSON.stringify({ error: 'asset_api_should_not_be_called' }),
});
});
await page.route('**/view**', async route => {
const request = route.request();
const url = new URL(request.url());
if (
request.method() !== 'GET'
|| url.searchParams.get('filename') !== 'blake3:abc123'
|| url.searchParams.has('type')
|| url.searchParams.has('subfolder')
) {
await route.fallback();
return;
}
await route.fulfill({
status: 200,
contentType: 'image/png',
body: TEST_OUTPUT_PNG,
});
});
await clickTab(page, 'Jobs');
await page.locator('input[placeholder="prompt_id"]').fill(jobId);
await page.getByText('Add').click();
await expect(page.locator('.openclaw-kv-val.ok')).toHaveText('completed', { timeout: 10000 });
await expect(page.locator('img[src*="blake3%3Aabc123"]')).toBeVisible();
await expect(page.locator('.openclaw-job-output-fallback')).toContainText('Asset API output requires /api/assets');
expect(assetApiCalls).toBe(0);
});
test('Degraded Adapter / Fail Handling', async ({ page }) => {
// Mock Planner Failure (503 Service Unavailable)
await page.route('**/openclaw/assist/planner', async route => {
tests/test_comfyui_history_parsing.py
+29
View File
@@ -101,6 +101,35 @@ class TestComfyUIHistoryParsing(unittest.TestCase):
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"])
self.assertFalse(images[0]["asset_api_required"])
self.assertEqual(images[0]["resolution"], "view")
def test_extract_images_preserves_asset_api_only_refs_as_explicit_no_go_contract(
self,
):
from services.comfyui_history import extract_images
history_item = {
"outputs": {
"3": {
"images": [
{
"asset": {
"id": "asset-only-42",
}
}
]
}
}
}
images = extract_images(history_item)
self.assertEqual(len(images), 1)
self.assertEqual(images[0]["filename"], "asset-only-42")
self.assertEqual(images[0]["asset_api_id"], "asset-only-42")
self.assertTrue(images[0]["asset_api_required"])
self.assertEqual(images[0]["resolution"], "asset_api_required")
self.assertEqual(images[0]["view_url"], "")
def test_get_job_status(self):
from services.comfyui_history import get_job_status
tests/test_r167_asset_api_contract.py
+66
View File
@@ -0,0 +1,66 @@
import unittest
from unittest.mock import AsyncMock, MagicMock, patch
class TestR167AssetApiContract(unittest.IsolatedAsyncioTestCase):
async def test_callback_delivery_preserves_asset_api_only_refs_without_view_fetch(self):
import services.callback_delivery as callback_delivery
sent_payloads = []
history_item = {
"outputs": {
"3": {
"images": [
{
"asset": {
"id": "asset-only-42",
}
}
]
}
}
}
async def fake_run_io(func, *args, **kwargs):
if func is callback_delivery.fetch_history:
return history_item
if func is callback_delivery.safe_request_json:
sent_payloads.append(args[2])
return {"ok": True}
raise AssertionError(f"unexpected func: {func}")
with (
patch.object(
callback_delivery, "run_io_in_thread", side_effect=fake_run_io
),
patch.object(
callback_delivery.asyncio, "sleep", AsyncMock(return_value=None)
),
patch.object(
callback_delivery,
"get_callback_allow_hosts",
return_value={"example.com"},
),
patch.object(callback_delivery, "get_job_status", return_value="completed"),
patch.object(
callback_delivery, "get_job_event_store", return_value=MagicMock()
),
patch.object(callback_delivery.trace_store, "add_event", return_value=None),
):
await callback_delivery._watch_and_deliver(
"p-r167",
{"url": "https://example.com/hook"},
trace_id="trace-r167",
)
self.assertEqual(len(sent_payloads), 1)
outputs = sent_payloads[0]["outputs"]
self.assertEqual(len(outputs), 1)
self.assertEqual(outputs[0]["asset_api_id"], "asset-only-42")
self.assertTrue(outputs[0]["asset_api_required"])
self.assertEqual(outputs[0]["resolution"], "asset_api_required")
self.assertEqual(outputs[0]["view_url"], "")
if __name__ == "__main__":
unittest.main()
web/openclaw_api.js
+1 -1
View File
@@ -237,7 +237,7 @@ export class OpenClawAPI {
buildViewUrlForRef(imageRef) {
const normalized = normalizeComfyOutputRef(imageRef);
if (!normalized) {
if (!normalized || !normalized.viewParams) {
return "";
}
return apiURL(`/view?${new URLSearchParams(normalized.viewParams).toString()}`);
web/openclaw_asset_refs.js
+47 -11
View File
@@ -13,6 +13,29 @@ function pickAssetHash(imageRef = {}) {
return "";
}
function pickAssetApiId(imageRef = {}) {
if (!imageRef || typeof imageRef !== "object") {
return "";
}
const direct = typeof imageRef.asset_api_id === "string"
? imageRef.asset_api_id.trim()
: (typeof imageRef.asset_id === "string" ? imageRef.asset_id.trim() : "");
if (direct) {
return direct;
}
const nested = imageRef.asset;
if (!nested || typeof nested !== "object") {
return "";
}
if (typeof nested.asset_id === "string" && nested.asset_id.trim()) {
return nested.asset_id.trim();
}
if (typeof nested.id === "string" && nested.id.trim()) {
return nested.id.trim();
}
return "";
}
function pickFilename(imageRef = {}) {
if (!imageRef || typeof imageRef !== "object") {
return "";
@@ -28,7 +51,9 @@ function pickFilename(imageRef = {}) {
export function normalizeComfyOutputRef(imageRef = {}) {
const assetHash = pickAssetHash(imageRef);
const filename = pickFilename(imageRef) || assetHash;
const assetApiId = pickAssetApiId(imageRef);
const namedFilename = pickFilename(imageRef);
const filename = namedFilename || assetHash || assetApiId;
const subfolder = typeof imageRef.subfolder === "string" ? imageRef.subfolder : "";
const type = typeof imageRef.type === "string" && imageRef.type ? imageRef.type : "output";
@@ -36,22 +61,33 @@ export function normalizeComfyOutputRef(imageRef = {}) {
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 } : {}),
};
const explicitAssetApiRequired = imageRef.asset_api_required === true;
const assetApiRequired = Boolean(explicitAssetApiRequired || (assetApiId && !assetHash && !namedFilename));
// IMPORTANT: asset-backed refs still resolve through /view when possible; do
// not promote asset-api-only identifiers into implicit /api/assets fetches.
const viewParams = assetApiRequired
? null
: (
assetHash
? { filename: assetHash }
: {
filename,
type,
...(subfolder ? { subfolder } : {}),
}
);
return {
filename,
subfolder,
type,
asset_hash: assetHash || "",
is_asset_backed: Boolean(assetHash),
asset_api_id: assetApiId || "",
asset_api_required: assetApiRequired,
resolution: assetApiRequired ? "asset_api_required" : "view",
unsupported_reason: assetApiRequired ? "asset_api_required" : "",
is_asset_backed: Boolean(assetHash || assetApiId),
viewParams,
};
}
web/tabs/job_monitor_tab.js
+35 -9
View File
@@ -183,15 +183,38 @@ export const jobMonitorTab = {
outputGrid.style.marginTop = "8px";
job.outputs.forEach((out) => {
const img = document.createElement("img");
img.src = out.view_url;
img.style.maxWidth = "80px";
img.style.maxHeight = "80px";
img.style.objectFit = "cover";
img.style.cursor = "pointer";
img.title = out.filename;
img.onclick = () => window.open(out.view_url, "_blank");
outputGrid.appendChild(img);
if (out.view_url) {
const img = document.createElement("img");
img.src = out.view_url;
img.style.maxWidth = "80px";
img.style.maxHeight = "80px";
img.style.objectFit = "cover";
img.style.cursor = "pointer";
img.title = out.filename;
img.onclick = () => window.open(out.view_url, "_blank");
outputGrid.appendChild(img);
return;
}
if (out.asset_api_required) {
const fallback = document.createElement("div");
fallback.className = "openclaw-job-output-fallback";
fallback.style.width = "80px";
fallback.style.minHeight = "80px";
fallback.style.padding = "6px";
fallback.style.display = "flex";
fallback.style.alignItems = "center";
fallback.style.justifyContent = "center";
fallback.style.textAlign = "center";
fallback.style.fontSize = "10px";
fallback.style.lineHeight = "1.3";
fallback.style.border = "1px dashed var(--border-color)";
fallback.style.borderRadius = "6px";
fallback.style.background = "var(--comfy-menu-bg, rgba(255,255,255,0.04))";
fallback.title = out.asset_api_id || out.filename || "Asset API output";
fallback.textContent = "Asset API output requires /api/assets. Preview disabled.";
outputGrid.appendChild(fallback);
}
});
row.appendChild(outputGrid);
@@ -270,6 +293,9 @@ function extractImages(historyItem) {
subfolder: img.subfolder,
type: img.type,
asset_hash: img.asset_hash,
asset_api_id: img.asset_api_id,
asset_api_required: img.asset_api_required,
resolution: img.resolution,
view_url: openclawApi.buildViewUrlForRef(img),
}));
}
web/tests/unit/openclaw_asset_refs.test.js
+41
View File
@@ -15,6 +15,10 @@ describe("openclaw asset refs", () => {
subfolder: "session-a",
type: "temp",
asset_hash: "",
asset_api_id: "",
asset_api_required: false,
resolution: "view",
unsupported_reason: "",
is_asset_backed: false,
viewParams: {
filename: "result.png",
@@ -36,6 +40,10 @@ describe("openclaw asset refs", () => {
subfolder: "",
type: "output",
asset_hash: "blake3:abc123",
asset_api_id: "",
asset_api_required: false,
resolution: "view",
unsupported_reason: "",
is_asset_backed: true,
viewParams: {
filename: "blake3:abc123",
@@ -56,6 +64,10 @@ describe("openclaw asset refs", () => {
subfolder: "",
type: "output",
asset_hash: "blake3:def456",
asset_api_id: "",
asset_api_required: false,
resolution: "view",
unsupported_reason: "",
is_asset_backed: true,
viewParams: {
filename: "blake3:def456",
@@ -63,6 +75,27 @@ describe("openclaw asset refs", () => {
});
});
it("keeps asset-api-only refs explicit instead of silently turning them into /api/assets fetches", () => {
expect(
normalizeComfyOutputRef({
asset: {
id: "asset-only-42",
},
})
).toEqual({
filename: "asset-only-42",
subfolder: "",
type: "output",
asset_hash: "",
asset_api_id: "asset-only-42",
asset_api_required: true,
resolution: "asset_api_required",
unsupported_reason: "asset_api_required",
is_asset_backed: true,
viewParams: null,
});
});
it("extracts mixed history outputs without dropping temp classifications", () => {
expect(
extractHistoryImageRefs({
@@ -90,6 +123,10 @@ describe("openclaw asset refs", () => {
subfolder: "",
type: "output",
asset_hash: "",
asset_api_id: "",
asset_api_required: false,
resolution: "view",
unsupported_reason: "",
is_asset_backed: false,
viewParams: {
filename: "classic.png",
@@ -101,6 +138,10 @@ describe("openclaw asset refs", () => {
subfolder: "preview",
type: "temp",
asset_hash: "blake3:temp123",
asset_api_id: "",
asset_api_required: false,
resolution: "view",
unsupported_reason: "",
is_asset_backed: true,
viewParams: {
filename: "blake3:temp123",