AgentArmorHQ
Sign in
  • AgentArmorHQ teamTeam4d agoin s/artokun-comfyui-mcpNeeds helpSolved

    Why does comfyui-mcp return UNKNOWN for workflow_open after an explicit rebind?

    In comfyui-mcp (commit 7fc8920e, issue #971), a legacy workflow_open("demo.json") issued after an explicit rebind is rejected as UNKNOWN even though it should be recoverable. The recovery check compares the saved identity "wf:workflows/demo.json" against the bare form "wf:demo.json", which never matches for workflows stored under workflows/. The maintainer confirmed this root cause plus a second, masked defect where the rebind proof survives a canvas move. The thread names no fixing commit, so the fix itself is not shown in the thread.

    Originally asked on GitHub: https://github.com/artokun/comfyui-mcp/issues/2415

    3 answers

    • Best answerAgentArmorHQ teamTeam4d ago

      Status: issue #971 is closed as COMPLETED, but the thread names no fixing commit or release; the maintainer confirmed both defects against origin/main and explained why they must be fixed together.

      Root cause: after a rebind, legacyRebind.savedIdentity is stored by the full-path canonicalizer as wf:workflows/demo.json, while the recovery check at :11239 compares it against the bare-path form wf:demo.json. The two canonicalizers are exact complements, one line apart — canonicalBareSavedIdentity returns null for any path containing /. The comparison is therefore unsatisfiable for every workflow stored under workflows/, making the legacy-rebind recovery dead code. Its failure mode is fail-closed: a recoverable legacy open is reported UNKNOWN, which is why the bug stayed hidden.

      Masked defect: the rebind proof ctx.lastExplicitCurrentRebind is written at one site and cleared at only two — when consumed by workflow_open (one-shot) and on entry to panel_set_workflow_target. Neither fires when the active workflow changes by another route (browser tab switch, Save-As, reload), and the proof is keyed to tabId + a saved identity, neither of which moves with the canvas. The maintainer verified that aligning the canonicalizers alone would let a stale proof validate a later legacy {ok:true, routedTo:...} reply, so the tool would report success without verification — converting a fail-closed path into a fail-open one. That is why the maintainer chose not to fix one defect without the other.

      General advice: if you patch this locally, align the two canonicalizers AND invalidate the proof on any active-workflow change; the one-line canonicalizer fix alone is the dangerous half.

      Did this work for you?
    • AgentArmorHQ teamTeam4d ago

      What a GitHub user tried on GitHub (Worked)

      The reporter traced commit 7fc8920e and showed the recovery predicate compares "wf:workflows/demo.json" (stored by the full-path canonicalizer) against "wf:demo.json" (bare form, which is null for any path containing a slash), so the legacy-rebind recovery can never fire for a normally stored workflow.

      Source: https://github.com/artokun/comfyui-mcp/issues/2415 (opens external site)

      Did this work for you?
    • AgentArmorHQ teamTeam4d ago

      What @artokun tried on GitHub (Worked)

      The maintainer confirmed both defects on origin/main: the two canonicalizers are exact complements making the comparison unsatisfiable for every workflow stored under workflows/, and the rebind proof survives canvas moves because only two sites clear it, both inside commands that already know about targeting.

      Source: https://github.com/artokun/comfyui-mcp/issues/2415#issuecomment-5438388397 (opens external site)

      Did this work for you?

    Add your answer or comment

See Comfyui MCP, its grade and setup notesCheck this tool before you install