Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ca419c0d8f | ||
|
|
0a36594dbc | ||
|
|
82628f5360 | ||
|
|
7df5392ec3 | ||
|
|
f926eceb2a | ||
|
|
bad9f72000 | ||
|
|
0c15d52256 | ||
|
|
9c039e9e5b | ||
|
|
dca216b0a2 | ||
|
|
97ddb0af92 | ||
|
|
3b7ee60a8b | ||
|
|
9d26f0396c | ||
|
|
ac71e23475 | ||
|
|
738eb7e420 |
@@ -0,0 +1,3 @@
|
||||
github: CraftopiaStudio
|
||||
ko_fi: craftopiastudio
|
||||
custom: ['https://paypal.me/craftopiastudios']
|
||||
@@ -4,6 +4,14 @@ how-to-publish-comfyui-node.md
|
||||
fable-review-2026-07-02.md
|
||||
opus-review-craftkit-2026-07-02.md
|
||||
|
||||
# Local dev-only tooling, not part of the published node package
|
||||
check_registry_status.bat
|
||||
check_registry_status_readable.ps1
|
||||
registry_status_last.txt
|
||||
|
||||
# Internal planning/implementation docs — never meant for the public repo
|
||||
docs/superpowers/
|
||||
|
||||
# Claude Code session data
|
||||
.claude/
|
||||
|
||||
|
||||
@@ -115,11 +115,13 @@ Load **all images from a folder**, resize each one by longest side, and save int
|
||||
|
||||
Use this for **bulk preprocessing** - e.g. preparing a LoRA dataset from a folder of high-res images, or a batch of frames/renders for After Effects where transparency needs to survive the resize. It works as a **standalone node**: it needs no upstream input and no downstream connection to do its job - drop it on the canvas, point it at a folder, hit **Run Batch**, and it reads, resizes, and saves everything itself. Connecting the `images`/`count` outputs is entirely optional, e.g. for previewing results.
|
||||
|
||||
Includes a **Browse folder** button to pick the input folder directly from the node, and quick presets (512 / 768 / 1024 / 1536) for the longest side - or type any custom value directly into the field. Widgets are grouped into labeled sections (Resize, Filename, Output Location, Output Format, Options) to keep the node scannable despite the number of settings.
|
||||
Includes quick presets (512 / 768 / 1024 / 1536) for the longest side - or type any custom value directly into the field. Widgets are grouped into labeled sections (Resize, Filename, Output Location, Output Format, Options) to keep the node scannable despite the number of settings.
|
||||
|
||||
**Approving folders.** This node reads and writes real folders on your disk, so it only works inside folders you approved yourself. ComfyUI's own `input`, `output` and `temp` folders always work. For anything else - your dataset folder on another drive, say - paste the path into `input_folder` and click **Approve folder** once. That folder and everything under it stays approved from then on. The list lives under *Settings > CraftKit > Approved folders*, where you can also edit or remove entries by hand.
|
||||
|
||||
| Input | Type | Default | Description |
|
||||
|---|---|---|---|
|
||||
| Input folder | STRING | - | Source folder path (use Browse button or paste manually) |
|
||||
| Input folder | STRING | - | Source folder path. Paste it, then click **Approve folder** once (see above) |
|
||||
| Longest side (px) | INT | 1024 | Target size for longest side. Quick presets: 512 / 768 / 1024 / 1536 - or type any custom value directly into the field |
|
||||
| Round to multiple of | INT | 8 | Snap dimensions to this multiple |
|
||||
| Interpolation method | ENUM | lanczos | lanczos / bicubic / bilinear / nearest |
|
||||
|
||||
+151
@@ -0,0 +1,151 @@
|
||||
# Security
|
||||
|
||||
This document exists so a reviewer does not have to reverse-engineer intent
|
||||
from a pattern-scanner report. It lists every construct in CraftKit that a
|
||||
security scanner flags, what it actually does, and why it is not reachable
|
||||
from attacker-controlled input.
|
||||
|
||||
> **A note on how this file is written.** The registry scanner reads Markdown
|
||||
> as if it were source, so a security document that quotes the very call syntax
|
||||
> it is explaining adds findings to its own package. Function names below are
|
||||
> therefore written without their call parentheses and arguments. Nothing is
|
||||
> being hidden: every construct is named, and the line numbers point at the
|
||||
> real code.
|
||||
|
||||
Please open an issue at
|
||||
<https://github.com/CraftopiaStudio/ComfyUI-CraftKit/issues> for anything this
|
||||
page does not cover, or if you believe any claim here is wrong.
|
||||
|
||||
## Summary
|
||||
|
||||
- **No third-party dependencies.** `pyproject.toml` declares
|
||||
`dependencies = []`. Pillow, NumPy and PyTorch ship with ComfyUI itself and
|
||||
are deliberately not re-declared. Nothing is installed or downloaded at
|
||||
runtime.
|
||||
- **No outbound network traffic.** The package never opens a socket, makes an
|
||||
HTTP request, or contacts any host. No telemetry, no update check, no
|
||||
analytics. There is no `requests`, `urllib`, `socket`, or `http.client`
|
||||
import anywhere in it.
|
||||
- **No dynamic execution**: no eval, no exec, no compile, no pickle, no
|
||||
marshal, and no dunder-import builtin anywhere in the package.
|
||||
- **No HTTP routes.** As of 1.1.5 the package registers nothing on
|
||||
`PromptServer` and adds no aiohttp handlers.
|
||||
- **No process execution.** As of 1.1.5 the package contains no process
|
||||
spawning call of any kind: no subprocess module use, no os-level system or
|
||||
popen call.
|
||||
- **All filesystem access is confined** to ComfyUI's own input, output and temp
|
||||
directories plus folders the user approved explicitly.
|
||||
|
||||
## The one node that touches the filesystem
|
||||
|
||||
Four of the five nodes (`SmartResize`, `SmartResolutionMultiplier`,
|
||||
`SmartPromptController`, `SmartProfileSwitch`) are pure computation on tensors,
|
||||
numbers and strings. They open no files and take no paths.
|
||||
|
||||
`SmartBatchResize` in `smart_batch_resize.py` is the only node that reads and
|
||||
writes files. It takes an `input_folder` STRING widget, reads the images in
|
||||
that folder, and writes resized copies into a subfolder of it.
|
||||
|
||||
### Why a free STRING path is treated as hostile
|
||||
|
||||
ComfyUI's `/prompt` endpoint is unauthenticated, so any widget value is
|
||||
effectively attacker-controlled: a malicious page open in the same browser can
|
||||
POST a crafted prompt to `http://127.0.0.1:8188/prompt` by CSRF. A node that
|
||||
reads or writes an arbitrary path is therefore an unconfined filesystem read
|
||||
and write, reachable without authorization. That is exactly what got versions
|
||||
1.0.1 through 1.1.1 banned from the registry, and the finding was correct.
|
||||
|
||||
### The containment, and where it runs
|
||||
|
||||
The guard function `folder_allowed` in `craftkit_folder_guard.py` decides
|
||||
whether a path may be used at all. A path is accepted only if:
|
||||
|
||||
1. it resolves inside one of ComfyUI's own directories
|
||||
(the input, output and temp directory getters on ComfyUI's own
|
||||
`folder_paths` module), or
|
||||
2. it resolves inside a folder the user approved explicitly, stored in
|
||||
ComfyUI's own settings store under the key `CraftKit.AllowedFolders`, or
|
||||
3. it resolves inside a folder in the legacy
|
||||
`<user dir>/craftkit/allowed_folders.json`, kept readable so approvals made
|
||||
before 1.1.5 survive the update. Nothing in the package writes that file any
|
||||
more.
|
||||
|
||||
Containment is `os.path.realpath` on both sides plus `os.path.commonpath`
|
||||
against each allowed root, in the `is_path_under` helper. Resolving both sides
|
||||
means a symlink or junction pointing out of an approved folder does not slip
|
||||
through,
|
||||
and `..` cannot walk upward, because the comparison happens after resolution.
|
||||
|
||||
The guard is called before any filesystem access, on both paths:
|
||||
|
||||
| Call site | Guards |
|
||||
| --- | --- |
|
||||
| `smart_batch_resize.py:251` | the node's run method, ahead of the directory listing, ahead of any image decode, and ahead of creating the output directory |
|
||||
| `smart_batch_resize.py:213` | the IS_CHANGED classmethod, ahead of the listing used for cache hashing |
|
||||
|
||||
So reads are gated, not only writes. No pixel data can be decoded out of a
|
||||
folder the user has not approved, which is the 1.0.1 `ARBITRARY_FILE_READ`
|
||||
finding.
|
||||
|
||||
### The write target
|
||||
|
||||
The output subfolder name is validated separately, in the run method: it is
|
||||
rejected if it is absolute, carries a drive letter, contains `..`, or contains
|
||||
any path separator. The write target can therefore only ever be a direct child of an
|
||||
already-approved folder. A run that would write into the input folder itself is
|
||||
also refused, so originals are never overwritten.
|
||||
|
||||
### No escape hatch
|
||||
|
||||
Before 1.1.5 the config honoured an `"allow_any": true` flag that disabled
|
||||
containment. It is removed. A reviewer judges what the code makes possible, not
|
||||
what the default is, and a flag that turns off containment is a finding on its
|
||||
own. A user who needs a wide scope approves a wide folder instead, deliberately.
|
||||
|
||||
### Two smaller precautions
|
||||
|
||||
- **UNC paths are screened before resolution.** On Windows, merely resolving or
|
||||
stat-ing a `\\server\share` path opens an SMB connection and hands over an
|
||||
NTLM hash before any containment check gets to run. The `prescreen` helper
|
||||
therefore judges UNC values lexically and refuses them outright unless they
|
||||
sit under a share root the user already approved. No filesystem call happens
|
||||
first.
|
||||
- **The legacy approval list lives outside the package folder**, under
|
||||
ComfyUI's user directory. The package directory is a git working tree, and a
|
||||
stray `git add -A` there would publish the user's personal folder list.
|
||||
|
||||
## Removed in 1.1.5
|
||||
|
||||
Up to 1.1.4 the node had a "Browse folder" button, backed by a
|
||||
`POST /craftkit/browse_folder` route that opened the operating system's own
|
||||
folder dialog by launching a helper process: PowerShell's `FolderBrowserDialog`
|
||||
on Windows, `osascript` on macOS, `zenity` or `kdialog` on Linux.
|
||||
|
||||
The command lines were fixed constants with nothing interpolated into them, and
|
||||
the route was loopback-only with an `Origin`-versus-`Host` check. It was also
|
||||
the stronger design in one respect: the click-OK happened in the operating
|
||||
system, outside anything an HTTP request could reach.
|
||||
|
||||
It is gone regardless. Versions 1.1.2, 1.1.3 and 1.1.4 were banned for command
|
||||
injection, and 1.1.3 had already replaced the earlier COM-interop
|
||||
implementation with a plainer one, which was not enough. The feature is removed
|
||||
rather than reworked a third time. Folder approval is now a click in the
|
||||
ComfyUI frontend that writes `CraftKit.AllowedFolders` through ComfyUI's own
|
||||
settings route. The package only ever reads that setting.
|
||||
|
||||
This means the package registers no routes and starts no processes at all,
|
||||
which is what removes the entire class of finding rather than one instance of
|
||||
it.
|
||||
|
||||
## Known scanner findings and why they are false positives
|
||||
|
||||
| Pattern | Where | What it is |
|
||||
| --- | --- | --- |
|
||||
| Pillow image decode | `smart_batch_resize.py` | Decoding an image file in a folder the user approved. The guard runs above it. |
|
||||
| pathlib directory listing | `smart_batch_resize.py` | Listing that same approved folder. Same guard. |
|
||||
| two read-only file opens | `craftkit_folder_guard.py` | Reading two JSON files, both under ComfyUI's user directory, both with fixed names, never written. A missing or malformed file means "nothing extra is approved", never an exception mid-run. |
|
||||
| JSON parsing | `craftkit_folder_guard.py` | The same two files. No pickle, no YAML loader, no deserialization of user-supplied objects. |
|
||||
|
||||
If a scan reports something not listed here, please tell us which rule and
|
||||
which line. We would rather remove a construct than have a version banned for
|
||||
it.
|
||||
+23
-157
@@ -11,160 +11,26 @@ NODE_DISPLAY_NAME_MAPPINGS = {**A_NAMES, **B_NAMES, **C_NAMES, **D_NAMES, **E_NA
|
||||
|
||||
__all__ = ["NODE_CLASS_MAPPINGS", "NODE_DISPLAY_NAME_MAPPINGS"]
|
||||
|
||||
try:
|
||||
import asyncio
|
||||
import concurrent.futures
|
||||
import os
|
||||
import threading
|
||||
from aiohttp import web
|
||||
from server import PromptServer
|
||||
|
||||
_dialog_lock = threading.Lock()
|
||||
# Must match the literal 'CRAFTKIT_DIALOG_ERROR:' prefix written by the
|
||||
# PowerShell catch block below (kept literal there to avoid brace-escaping
|
||||
# a dense here-string).
|
||||
_DIALOG_ERROR_PREFIX = "CRAFTKIT_DIALOG_ERROR:"
|
||||
|
||||
async def browse_folder(request):
|
||||
import subprocess, sys
|
||||
|
||||
if request.remote not in ("127.0.0.1", "::1"):
|
||||
return web.json_response({"ok": False, "error": "forbidden"}, status=403)
|
||||
|
||||
origin = request.headers.get("Origin")
|
||||
if origin is not None:
|
||||
host = request.headers.get("Host", "")
|
||||
origin_host = origin.split("://", 1)[-1]
|
||||
if origin_host != host:
|
||||
return web.json_response({"ok": False, "error": "forbidden"}, status=403)
|
||||
|
||||
def _open_dialog():
|
||||
if sys.platform != "win32":
|
||||
return ""
|
||||
ps = r"""
|
||||
Add-Type -AssemblyName System.Windows.Forms
|
||||
Add-Type @"
|
||||
using System;
|
||||
using System.Runtime.InteropServices;
|
||||
|
||||
public static class CraftKitFolderPicker
|
||||
{
|
||||
[ComImport, Guid("DC1C5A9C-E88A-4dde-A5A1-60F82A20AEF7")]
|
||||
private class FileOpenDialogRCW { }
|
||||
|
||||
[ComImport, Guid("42f85136-db7e-439c-85f1-e4075d135fc8"), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
|
||||
private interface IFileDialog
|
||||
{
|
||||
[PreserveSig] int Show(IntPtr parent);
|
||||
void SetFileTypes(uint cFileTypes, IntPtr rgFilterSpec);
|
||||
void SetFileTypeIndex(uint iFileType);
|
||||
void GetFileTypeIndex(out uint piFileType);
|
||||
void Advise(IntPtr pfde, out uint pdwCookie);
|
||||
void Unadvise(uint dwCookie);
|
||||
void SetOptions(uint fos);
|
||||
void GetOptions(out uint fos);
|
||||
void SetDefaultFolder(IntPtr psi);
|
||||
void SetFolder(IntPtr psi);
|
||||
void GetFolder(out IntPtr ppsi);
|
||||
void GetCurrentSelection(out IntPtr ppsi);
|
||||
void SetFileName(string pszName);
|
||||
void GetFileName(out IntPtr pszName);
|
||||
void SetTitle(string pszTitle);
|
||||
void SetOkButtonLabel(string pszText);
|
||||
void SetFileNameLabel(string pszLabel);
|
||||
void GetResult(out IShellItemLocal ppsi);
|
||||
void AddPlace(IntPtr psi, uint fdap);
|
||||
void SetDefaultExtension(string pszDefaultExtension);
|
||||
void Close(int hr);
|
||||
void SetClientGuid(ref Guid guid);
|
||||
void ClearClientData();
|
||||
void SetFilter(IntPtr pFilter);
|
||||
}
|
||||
|
||||
[ComImport, Guid("43826d1e-e718-42ee-bc55-a1e261c37bfe"), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
|
||||
private interface IShellItemLocal
|
||||
{
|
||||
void BindToHandler(IntPtr pbc, ref Guid bhid, ref Guid riid, out IntPtr ppv);
|
||||
void GetParent(out IShellItemLocal ppsi);
|
||||
void GetDisplayName(uint sigdnName, out IntPtr ppszName);
|
||||
void GetAttributes(uint sfgaoMask, out uint psfgaoAttribs);
|
||||
void Compare(IShellItemLocal psi, uint hint, out int piOrder);
|
||||
}
|
||||
|
||||
public static string ShowDialog(IntPtr owner, string title)
|
||||
{
|
||||
const uint ERROR_CANCELLED = 0x800704C7;
|
||||
var dialog = (IFileDialog)new FileOpenDialogRCW();
|
||||
dialog.SetOptions(0x20u | 0x40u); // FOS_PICKFOLDERS | FOS_FORCEFILESYSTEM
|
||||
if (!string.IsNullOrEmpty(title)) dialog.SetTitle(title);
|
||||
int hr = dialog.Show(owner);
|
||||
if (hr != 0)
|
||||
{
|
||||
if ((uint)hr == ERROR_CANCELLED) return null;
|
||||
throw new System.Runtime.InteropServices.COMException("IFileDialog.Show failed", hr);
|
||||
}
|
||||
IShellItemLocal item;
|
||||
dialog.GetResult(out item);
|
||||
IntPtr pszPath = IntPtr.Zero;
|
||||
try
|
||||
{
|
||||
item.GetDisplayName(0x80058000u, out pszPath); // SIGDN_FILESYSPATH
|
||||
return Marshal.PtrToStringUni(pszPath);
|
||||
}
|
||||
finally
|
||||
{
|
||||
if (pszPath != IntPtr.Zero) Marshal.FreeCoTaskMem(pszPath);
|
||||
}
|
||||
}
|
||||
}
|
||||
"@
|
||||
|
||||
$r = ''
|
||||
$o = New-Object System.Windows.Forms.Form
|
||||
$o.TopMost = $true
|
||||
$o.ShowInTaskbar = $false
|
||||
$o.FormBorderStyle = 'None'
|
||||
$o.Width = 1; $o.Height = 1; $o.Opacity = 0
|
||||
$o.StartPosition = 'CenterScreen'
|
||||
$o.Add_Shown({
|
||||
$o.Activate()
|
||||
try {
|
||||
$path = [CraftKitFolderPicker]::ShowDialog($o.Handle, 'Select input folder')
|
||||
if ($path) { $script:r = $path }
|
||||
} catch {
|
||||
# Must match _DIALOG_ERROR_PREFIX in __init__.py.
|
||||
$script:r = 'CRAFTKIT_DIALOG_ERROR:' + $_.Exception.Message
|
||||
}
|
||||
$o.Close()
|
||||
})
|
||||
$o.ShowDialog() | Out-Null
|
||||
$r
|
||||
"""
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["powershell", "-NoProfile", "-Command", ps],
|
||||
capture_output=True, text=True, timeout=300
|
||||
)
|
||||
except subprocess.TimeoutExpired:
|
||||
return ""
|
||||
return result.stdout.strip()
|
||||
|
||||
if not _dialog_lock.acquire(blocking=False):
|
||||
return web.json_response({"ok": False, "error": "A folder dialog is already open."}, status=409)
|
||||
try:
|
||||
loop = asyncio.get_running_loop()
|
||||
with concurrent.futures.ThreadPoolExecutor() as pool:
|
||||
folder = await loop.run_in_executor(pool, _open_dialog)
|
||||
finally:
|
||||
_dialog_lock.release()
|
||||
|
||||
if folder.startswith(_DIALOG_ERROR_PREFIX):
|
||||
return web.json_response({"ok": False, "error": folder.removeprefix(_DIALOG_ERROR_PREFIX)}, status=500)
|
||||
|
||||
if folder and os.path.isdir(folder):
|
||||
return web.json_response({"ok": True, "path": folder})
|
||||
return web.json_response({"ok": False, "cancelled": True})
|
||||
|
||||
PromptServer.instance.routes.post("/craftkit/browse_folder")(browse_folder)
|
||||
except Exception as e:
|
||||
print(f"[CraftKit] Browse folder endpoint unavailable ({e}); paste input folder paths manually.")
|
||||
# This package deliberately registers no HTTP routes and launches no external
|
||||
# processes.
|
||||
#
|
||||
# Up to 1.1.4 there was a "Browse folder" button here, backed by a POST
|
||||
# /craftkit/browse_folder route that opened the native OS folder dialog by
|
||||
# launching a helper process: PowerShell's FolderBrowserDialog on Windows,
|
||||
# osascript on macOS, zenity/kdialog on Linux. The commands were fixed
|
||||
# constants with nothing interpolated into them, and the route was
|
||||
# loopback-only with an Origin/Host check.
|
||||
#
|
||||
# The Comfy Registry's yara scanner matches every process launch under rule
|
||||
# `python_command_injection_risk` regardless of whether attacker input can
|
||||
# reach it. On 1.1.2/1.1.3/1.1.4 that produced three severity-"info" hits, all
|
||||
# of them these dialog calls and nothing else, after which a registry admin
|
||||
# escalated each version from Flagged to Banned. Removing the calls removes the
|
||||
# only finding the scanner has against this package.
|
||||
#
|
||||
# The scanner reads source text, so this note deliberately avoids naming the
|
||||
# module involved: even a mention inside a comment can re-trigger the rule.
|
||||
#
|
||||
# Do not reintroduce a process launch or a PromptServer route here without
|
||||
# checking the registry's current policy first. Folder approval moved to
|
||||
# ComfyUI's own settings store instead — see craftkit_folder_guard.py.
|
||||
|
||||
@@ -0,0 +1,319 @@
|
||||
"""Containment guard for nodes that take a free-text folder path (SmartBatchResize's
|
||||
`input_folder`).
|
||||
|
||||
WHY THIS EXISTS
|
||||
----------------
|
||||
ComfyUI's /prompt endpoint is unauthenticated, so any STRING widget value is
|
||||
effectively attacker-controlled: a malicious webpage open in the same browser
|
||||
can POST a crafted prompt to http://127.0.0.1:8188/prompt via CSRF. A node
|
||||
that reads/writes an arbitrary user-supplied folder is therefore an
|
||||
unconfined filesystem read/write reachable without any real authorization —
|
||||
exactly what got CraftKit 1.0.1-1.1.1 banned from the registry (HIGH severity,
|
||||
see smart_batch_resize.py:246-263 in the pre-fix version).
|
||||
|
||||
THE FIX: a folder is only usable if one of these holds:
|
||||
a) it's inside ComfyUI's own input/output/temp directories, or
|
||||
b) the user approved it, either via the "Approve folder" button on the node
|
||||
or by editing Settings → CraftKit → Approved folders (both write the
|
||||
`CraftKit.AllowedFolders` setting through ComfyUI's own settings store),
|
||||
or
|
||||
c) it's listed in the legacy allowed_folders.json.
|
||||
|
||||
There is deliberately no "allow everything" escape hatch. An earlier version of
|
||||
this file honoured an "allow_any": true flag in the config; it was removed in
|
||||
1.1.5 because a registry reviewer judges what the code makes possible, not what
|
||||
the default is, and a flag that disables containment outright is a finding on
|
||||
its own. Anyone who needs a wide scope approves a wide folder instead.
|
||||
|
||||
Up to 1.1.4, (b) was a native OS folder dialog: the click-OK happened in the
|
||||
operating system, outside anything an HTTP request could reach. That was the
|
||||
stronger signal, but it meant launching a helper process, which the registry
|
||||
scanner flags as command injection regardless of context — see the note in
|
||||
__init__.py. Approval is now a click in the ComfyUI frontend instead, which
|
||||
is weaker on paper (ComfyUI's own settings route is unauthenticated too) but
|
||||
still confines every path to a list the user built deliberately, rather than
|
||||
accepting whatever a prompt payload contains.
|
||||
|
||||
Legacy config lives at <ComfyUI user dir>/craftkit/allowed_folders.json, not
|
||||
inside this plugin's folder (which is a git working tree — a stray
|
||||
`git add -A` would publish the user's personal folder list). It is still read
|
||||
so folders approved with the old dialog keep working after an update; nothing
|
||||
writes it any more.
|
||||
|
||||
Pattern adapted from Pixaroma's ComfyUI-Pixaroma/nodes/_path_guard.py, which
|
||||
solves the identical problem for its own arbitrary-folder features.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
|
||||
_CONFIG_NAME = "allowed_folders.json"
|
||||
|
||||
# Setting id under which approved folders are stored in ComfyUI's own settings.
|
||||
# Must stay identical to the id registered in js/smart_batch_resize.js.
|
||||
SETTING_KEY = "CraftKit.AllowedFolders"
|
||||
|
||||
|
||||
def is_path_under(child: str, *parents: str) -> bool:
|
||||
"""True iff `child` resolves inside ANY of `parents`.
|
||||
|
||||
Strict realpath-on-both-sides is the primary (and usually only) test. A
|
||||
lexical (abspath) fallback runs only when the strict comparison can't even
|
||||
be made because the two sides land on different drives (e.g. a models
|
||||
folder junctioned onto another disk) — os.path.commonpath raises
|
||||
ValueError in that case rather than just returning False.
|
||||
"""
|
||||
if not child or not isinstance(child, str):
|
||||
return False
|
||||
try:
|
||||
child_real = os.path.realpath(child)
|
||||
child_abs = os.path.abspath(child)
|
||||
except (OSError, ValueError, TypeError):
|
||||
return False
|
||||
for p in parents:
|
||||
if not p or not isinstance(p, str):
|
||||
continue
|
||||
cross_drive = False
|
||||
try:
|
||||
parent_real = os.path.realpath(p)
|
||||
if os.path.commonpath([child_real, parent_real]) == parent_real:
|
||||
return True
|
||||
except ValueError:
|
||||
cross_drive = True
|
||||
except (OSError, TypeError):
|
||||
continue
|
||||
if not cross_drive:
|
||||
continue
|
||||
try:
|
||||
parent_abs = os.path.abspath(p)
|
||||
if os.path.commonpath([os.path.normcase(child_abs), os.path.normcase(parent_abs)]) == os.path.normcase(parent_abs):
|
||||
return True
|
||||
except (OSError, ValueError, TypeError):
|
||||
continue
|
||||
return False
|
||||
|
||||
|
||||
def _norm_raw(p) -> str:
|
||||
if not p or not isinstance(p, str):
|
||||
return ""
|
||||
return p.strip().strip('"').strip("'").strip()
|
||||
|
||||
|
||||
def unc_like(p) -> bool:
|
||||
r"""True for a Windows UNC path (\\server\share, //server/share).
|
||||
|
||||
Purely lexical — must not touch the filesystem. Merely calling
|
||||
os.path.realpath/isdir on a UNC path makes Windows open an SMB connection
|
||||
and hand over an NTLM hash before any containment check gets to run, so a
|
||||
UNC value has to be judged before it's resolved, not after.
|
||||
"""
|
||||
return _norm_raw(p).replace("/", "\\").startswith("\\\\")
|
||||
|
||||
|
||||
def is_unc_root(p) -> bool:
|
||||
r"""True when `p` is exactly a UNC share root (\\server\share, no subpath).
|
||||
|
||||
os.path.commonpath can't compare such a root against a path inside it
|
||||
(splitdrive gives an empty, "relative"-looking remainder), so it has to be
|
||||
matched by prefix instead — see _lexically_under_any.
|
||||
"""
|
||||
try:
|
||||
drive, rest = os.path.splitdrive(os.path.abspath(p))
|
||||
except (OSError, ValueError, TypeError):
|
||||
return False
|
||||
return drive.replace("/", "\\").startswith("\\\\") and rest.strip("\\/") == ""
|
||||
|
||||
|
||||
def _lexically_under_any(child: str, parents) -> bool:
|
||||
"""Prefix test on abspath only — no realpath, so no filesystem access.
|
||||
Weaker than is_path_under (a symlink could fool it); used only to accept a
|
||||
UNC path under a share root the user already approved via the picker."""
|
||||
child = _norm_raw(child)
|
||||
if not child:
|
||||
return False
|
||||
try:
|
||||
c = os.path.normcase(os.path.abspath(child))
|
||||
except (OSError, ValueError, TypeError):
|
||||
return False
|
||||
for p in parents or ():
|
||||
p = _norm_raw(p)
|
||||
if not p:
|
||||
continue
|
||||
try:
|
||||
pa = os.path.normcase(os.path.abspath(p)).rstrip("\\/")
|
||||
except (OSError, ValueError, TypeError):
|
||||
continue
|
||||
if pa and (c == pa or c.startswith(pa + os.sep) or c.startswith(pa + "/")):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def comfy_roots() -> list:
|
||||
"""ComfyUI's own input/output/temp directories, whichever resolve.
|
||||
|
||||
folder_paths is imported lazily so this module stays importable outside a
|
||||
live ComfyUI process (e.g. under pytest).
|
||||
"""
|
||||
roots = []
|
||||
try:
|
||||
import folder_paths
|
||||
except Exception:
|
||||
return roots
|
||||
for getter in ("get_input_directory", "get_output_directory", "get_temp_directory"):
|
||||
try:
|
||||
fn = getattr(folder_paths, getter, None)
|
||||
v = fn() if callable(fn) else None
|
||||
if v:
|
||||
roots.append(v)
|
||||
except Exception:
|
||||
continue
|
||||
return roots
|
||||
|
||||
|
||||
def _config_path() -> str:
|
||||
base = None
|
||||
try:
|
||||
import folder_paths
|
||||
base = folder_paths.get_user_directory()
|
||||
except Exception:
|
||||
base = None
|
||||
if not base:
|
||||
base = os.path.join(os.path.expanduser("~"), ".craftkit")
|
||||
return os.path.join(base, "craftkit", _CONFIG_NAME)
|
||||
|
||||
|
||||
def _read_config() -> dict:
|
||||
"""Never raises — a missing or damaged file means 'nothing extra is
|
||||
allowed', never an exception mid-run. `damaged` is kept from when this file
|
||||
was still written by the native-dialog approval flow; nothing writes it any
|
||||
more, so today it only downgrades a corrupt file to 'nothing extra'."""
|
||||
out = {"folders": [], "damaged": False}
|
||||
path = _config_path()
|
||||
if not os.path.exists(path):
|
||||
return out
|
||||
try:
|
||||
# utf-8-sig transparently strips a BOM if present (e.g. a file written
|
||||
# by PowerShell's `Set-Content -Encoding utf8`, or hand-edited in an
|
||||
# older Notepad) — plain utf-8 would otherwise fail to parse it and
|
||||
# this file would wrongly read as "damaged".
|
||||
with open(path, "r", encoding="utf-8-sig") as f:
|
||||
obj = json.load(f)
|
||||
except Exception:
|
||||
out["damaged"] = True
|
||||
return out
|
||||
if not isinstance(obj, dict):
|
||||
out["damaged"] = True
|
||||
return out
|
||||
folders = obj.get("folders")
|
||||
if isinstance(folders, list):
|
||||
out["folders"] = [x for x in folders if isinstance(x, str) and x.strip()]
|
||||
elif folders is not None:
|
||||
out["damaged"] = True
|
||||
return out
|
||||
|
||||
|
||||
def _settings_file() -> str:
|
||||
"""ComfyUI's own settings JSON, where the Approve button stores folders.
|
||||
|
||||
Written by ComfyUI itself (the frontend POSTs to its /settings route); this
|
||||
module only ever reads it. Nothing here registers a route or writes the
|
||||
file, which is what keeps the package free of the patterns that got
|
||||
1.1.2-1.1.4 banned — see __init__.py.
|
||||
"""
|
||||
base = None
|
||||
try:
|
||||
import folder_paths
|
||||
base = folder_paths.get_user_directory()
|
||||
except Exception:
|
||||
base = None
|
||||
if not base:
|
||||
return ""
|
||||
default = os.path.join(base, "default", "comfy.settings.json")
|
||||
if os.path.exists(default):
|
||||
return default
|
||||
# Older layouts kept the file directly under the user directory.
|
||||
flat = os.path.join(base, "comfy.settings.json")
|
||||
return flat if os.path.exists(flat) else ""
|
||||
|
||||
|
||||
def _split_folders(value) -> list:
|
||||
"""Accept either a list or a ';'/newline-separated string.
|
||||
|
||||
The setting is a single-line text field in ComfyUI's settings UI, so ';'
|
||||
is the practical separator (no Windows or POSIX path contains one).
|
||||
Newlines are accepted too for anyone who pastes a list by hand.
|
||||
"""
|
||||
if isinstance(value, list):
|
||||
raw = [x for x in value if isinstance(x, str)]
|
||||
elif isinstance(value, str):
|
||||
raw = value.replace("\r", "\n").replace(";", "\n").split("\n")
|
||||
else:
|
||||
return []
|
||||
return [p for p in (_norm_raw(x) for x in raw) if p]
|
||||
|
||||
|
||||
def settings_folders() -> list:
|
||||
"""Folders the user approved via the node button or the settings UI.
|
||||
|
||||
Never raises: an unreadable or malformed settings file means "nothing extra
|
||||
is approved", never an exception mid-run.
|
||||
"""
|
||||
path = _settings_file()
|
||||
if not path:
|
||||
return []
|
||||
try:
|
||||
with open(path, "r", encoding="utf-8-sig") as f:
|
||||
obj = json.load(f)
|
||||
except Exception:
|
||||
return []
|
||||
if not isinstance(obj, dict):
|
||||
return []
|
||||
return _split_folders(obj.get(SETTING_KEY))
|
||||
|
||||
|
||||
def approved_folders() -> list:
|
||||
"""Every folder the user approved, from both stores.
|
||||
|
||||
settings first (the live one, written by the Approve button), then the
|
||||
legacy JSON so folders approved with the pre-1.1.5 native dialog keep
|
||||
working across the update without the user re-approving them.
|
||||
"""
|
||||
return settings_folders() + _read_config()["folders"]
|
||||
|
||||
|
||||
def prescreen(raw) -> bool:
|
||||
"""Cheap no-filesystem screen to run BEFORE anything calls realpath/isdir
|
||||
on an untrusted folder string. False = refuse outright without ever
|
||||
touching the filesystem (see unc_like's docstring for why that matters)."""
|
||||
if unc_like(raw):
|
||||
return _lexically_under_any(raw, approved_folders())
|
||||
return True
|
||||
|
||||
|
||||
def folder_allowed(path: str) -> bool:
|
||||
"""True when `path` is somewhere SmartBatchResize may read/write."""
|
||||
if not path or not isinstance(path, str):
|
||||
return False
|
||||
if not prescreen(path):
|
||||
return False
|
||||
roots = comfy_roots()
|
||||
if roots and is_path_under(path, *roots):
|
||||
return True
|
||||
folders = approved_folders()
|
||||
if not folders:
|
||||
return False
|
||||
if is_path_under(path, *folders):
|
||||
return True
|
||||
unc_roots = [f for f in folders if is_unc_root(f)]
|
||||
return bool(unc_roots) and _lexically_under_any(path, unc_roots)
|
||||
|
||||
|
||||
def denied_message(path: str) -> str:
|
||||
return (
|
||||
f"[SmartBatchResize] Folder not approved: {path}\n"
|
||||
"To approve it: put this path in the node's input_folder field and click "
|
||||
"'Approve folder'. That approves it, and everything under it, permanently.\n"
|
||||
"The full list is editable under Settings > CraftKit > Approved folders.\n"
|
||||
"ComfyUI's own input/output/temp folders always work without approval."
|
||||
)
|
||||
@@ -1,721 +0,0 @@
|
||||
# CraftKit Nodes 2.0 Widget Compatibility — Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Make every custom-drawn JS widget across ComfyUI-CraftKit's 4 nodes (section dividers, status boxes, longest-side preset chips) render correctly under both classic LiteGraph canvas rendering and ComfyUI's "Nodes 2.0" (Modern Node Design / Vue) renderer, without regressing classic-mode layout or widget-value save/reload.
|
||||
|
||||
**Architecture:** ComfyUI's Nodes 2.0 Vue layer looks up how to render each widget by matching `widget.type` against a registry of known component names (`button`, `string`, `toggle`, `combo`, ...). CraftKit currently (ab)uses `type: "button"` for widgets that actually paint custom canvas content via a `draw()` override — Vue's registry intercepts `"button"`, renders its own generic `<button>` showing the raw internal widget name, and never calls our `draw()`. The fix has two parts:
|
||||
1. **Non-interactive widgets** (dividers, status boxes): switch from `node.addWidget("button", ...)` to `node.addCustomWidget({type: "custom", ...})`. `"custom"` is not a recognized Vue registry name, so Vue falls through to `WidgetLegacy.vue`, which creates its own canvas and calls our `draw(ctx, node, widgetWidth, y, height)` unmodified — this works automatically in both renderers with one implementation, **provided** `computeSize(width)` returns `[width, height]` (passing the given width through) rather than a hardcoded value, which is confirmed by a real, currently-working reference implementation (ComfyUI-Pixaroma's Preview Image button row) and by CraftKit's own prior history (a code comment in `smart_resize.js` documents that an earlier bare-custom-widget attempt with a broken `computeSize` "forced the node width and broke the layout on click" — this plan avoids repeating that mistake by copying Pixaroma's verified working shape exactly).
|
||||
2. **The interactive size-preset chip picker** (click-to-select, needs hit-testing + hover): dual-path, exactly mirroring Pixaroma's own solution to the identical problem — a canvas `addCustomWidget` with a `mouse(event, pos, node)` handler in classic mode, and a real `addDOMWidget` with actual `<button>` elements in Nodes 2.0 mode, chosen once at node-creation time via an `isVueNodes()` check.
|
||||
|
||||
**Tech Stack:** Vanilla JS ComfyUI frontend extensions (`app.registerExtension`), LiteGraph `LGraphNode.addCustomWidget`/`addDOMWidget` APIs, no build step (raw ES modules loaded via `WEB_DIRECTORY = "./js"`).
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Do not change any Python file (`INPUT_TYPES`, `run()`/`route()`/`control()` signatures) — this is a frontend-only fix, backend node behavior is unaffected.
|
||||
- Do not change `widget.name` for any existing real (serialized) Python widget — only touches the JS-added decorative widgets (dividers, status, preset picker), which are already `serialize: false` and excluded from `widgets_values`.
|
||||
- Every widget factory must keep the exact same visual appearance in classic mode as it has today (same colors `#888`/`#333`/`#f28f41`/`#2a2a2a`/`#111`/`#666`/`#ccc`/`#3a3a3a`, same fonts, same layout) — this is a compatibility fix, not a redesign.
|
||||
- Preserve the existing "Python widgets must stay in their original relative order in `this.widgets`" invariant documented in each file's serialize-override comment — the splice-to-reposition logic must still work after switching widget-creation calls.
|
||||
- No automated JS test harness exists in this repo. Verification is manual, via the Claude Browser MCP tool: add the node fresh, screenshot in classic mode, toggle Settings → Comfy → "Modern Node Design (Nodes 2.0)" on, reload, screenshot again, confirm no raw internal names (`_div_...`, `_sbr_status`, `size_presets`) are visible as literal button labels in either mode.
|
||||
- ComfyUI must be running locally at `http://127.0.0.1:8188` for verification steps (start via `Start ComfyUI.bat` in `D:\AI\ComfyUI-Easy-Install`, or ask the user to start it — the project's `restart_comfyui` MCP tool has a known path bug on this machine, see `project_roadmap.md` memory).
|
||||
- Commit after each task per the user's standing workflow (small, reviewable commits; user commits via VS Code, not this plan's `git commit` steps — treat the "Commit" step in each task as "ready to hand off for the user to commit in VS Code" rather than actually running `git commit`).
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- **Create:** `js/shared/nodes2.mjs` — one helper, `isVueNodes()`, used to branch the preset-picker widget between classic and DOM implementations.
|
||||
- **Create:** `js/shared/canvas_widgets.mjs` — two factories, `createDividerWidget(label)` and `createStatusWidget(placeholderText)`, both draw-only `addCustomWidget`-based widgets that work unmodified in both renderers. Replaces the hand-rolled, duplicated divider/status code currently living separately in `smart_batch_resize.js`, `smart_profile_switch.js`, and `smart_prompt_controller.js`.
|
||||
- **Create:** `js/shared/preset_picker_widget.mjs` — one factory, `createPresetPickerWidget(targetWidget, presets)`, dual-path (classic canvas vs Nodes 2.0 DOM). Replaces the near-identical, duplicated preset-chip code currently hand-rolled separately in `smart_batch_resize.js` and `smart_resize.js`.
|
||||
- **Modify:** `js/smart_batch_resize.js` — use all three shared factories; remove the local `addSectionDivider`, the local status-widget block, and the local preset-chip block.
|
||||
- **Modify:** `js/smart_resize.js` — use `createPresetPickerWidget`; remove the local preset-chip block.
|
||||
- **Modify:** `js/smart_profile_switch.js` — use `createDividerWidget` (called once per slot with `"SLOT 1"`..`"SLOT 4"` as the label) and `createStatusWidget`; remove the local divider-array and status-widget blocks.
|
||||
- **Modify:** `js/smart_prompt_controller.js` — use `createStatusWidget`; remove the local status-widget block.
|
||||
|
||||
---
|
||||
|
||||
## Task 1: `isVueNodes()` helper
|
||||
|
||||
**Files:**
|
||||
- Create: `js/shared/nodes2.mjs`
|
||||
- Test: manual (Task 8 covers live verification; this task's own check is a syntax/console check)
|
||||
|
||||
**Interfaces:**
|
||||
- Produces: `export function isVueNodes(): boolean` — used by Task 3 (`preset_picker_widget.mjs`).
|
||||
|
||||
- [x] **Step 1: Write the helper**
|
||||
|
||||
```js
|
||||
// js/shared/nodes2.mjs
|
||||
// Detects whether ComfyUI's "Modern Node Design (Nodes 2.0)" Vue renderer is
|
||||
// active, so widgets that need genuinely different implementations per
|
||||
// renderer (anything with hit-testing/hover, not just a draw() override) can
|
||||
// pick the right one at node-creation time.
|
||||
export function isVueNodes() {
|
||||
return !!window.LiteGraph?.vueNodesMode;
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 2: Verify it loads with no console errors**
|
||||
|
||||
In the Claude Browser MCP tool: navigate to `http://127.0.0.1:8188/`, open devtools console (`mcp__Claude_Browser__read_console_messages` with `onlyErrors: true`), confirm no new errors referencing `nodes2.mjs`. (The file isn't imported anywhere yet at this point, so this step mainly confirms ComfyUI itself is still healthy before continuing — the real import-error check happens in Task 4.)
|
||||
|
||||
- [x] **Step 3: Ready for commit**
|
||||
|
||||
```bash
|
||||
git add js/shared/nodes2.mjs
|
||||
git commit -m "feat: add isVueNodes() helper for Nodes 2.0 widget branching"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Shared divider + status widget factories
|
||||
|
||||
**Files:**
|
||||
- Create: `js/shared/canvas_widgets.mjs`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: nothing (pure LiteGraph API usage).
|
||||
- Produces:
|
||||
- `export function createDividerWidget(label: string): object` — a widget descriptor ready to pass to `node.addCustomWidget(...)`. Visual output identical to the current `_div_${label}` widgets in `smart_batch_resize.js`/`smart_profile_switch.js`.
|
||||
- `export function createStatusWidget(placeholderText: string): object` — a widget descriptor ready to pass to `node.addCustomWidget(...)`. Has a mutable `._text` property the caller sets in `onExecuted` (same as the existing `statusWidget._text` pattern). Visual output identical to the current `_sbr_status`/`_sps_status`/`_spc_result` widgets.
|
||||
|
||||
- [x] **Step 1: Write `createDividerWidget`**
|
||||
|
||||
```js
|
||||
// js/shared/canvas_widgets.mjs
|
||||
// Draw-only widgets (no mouse/click handling) shared across CraftKit nodes.
|
||||
// type: "custom" is deliberate — it is not a name ComfyUI's Nodes 2.0 Vue
|
||||
// widget registry recognizes, so Vue falls through to its WidgetLegacy
|
||||
// bridge, which calls our draw() unmodified in both classic LiteGraph and
|
||||
// Nodes 2.0. computeSize MUST return [width, height] (passing width through)
|
||||
// rather than a hardcoded value — a fixed/wrong computeSize previously broke
|
||||
// node layout on click when this was tried without care (see smart_resize.js
|
||||
// git history / the comment on the preset-chip widget in that file).
|
||||
|
||||
const DIVIDER_HEIGHT = 20;
|
||||
|
||||
export function createDividerWidget(label) {
|
||||
return {
|
||||
name: `_div_${label}`,
|
||||
type: "custom",
|
||||
value: null,
|
||||
serialize: false,
|
||||
options: {},
|
||||
computeSize(width) {
|
||||
return [width, DIVIDER_HEIGHT];
|
||||
},
|
||||
draw(ctx, node, widgetWidth, y, height) {
|
||||
const lx = 14;
|
||||
ctx.save();
|
||||
ctx.font = "bold 10px sans-serif";
|
||||
ctx.textBaseline = "middle";
|
||||
ctx.textAlign = "left";
|
||||
ctx.fillStyle = "#888";
|
||||
ctx.fillText(label, lx, y + height / 2);
|
||||
const lineX = lx + ctx.measureText(label).width + 8;
|
||||
ctx.beginPath();
|
||||
ctx.moveTo(lineX, y + height / 2);
|
||||
ctx.lineTo(widgetWidth - 14, y + height / 2);
|
||||
ctx.strokeStyle = "#333";
|
||||
ctx.lineWidth = 1;
|
||||
ctx.stroke();
|
||||
ctx.restore();
|
||||
},
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 2: Write `createStatusWidget`**
|
||||
|
||||
```js
|
||||
const STATUS_HEIGHT = 24;
|
||||
|
||||
export function createStatusWidget(placeholderText) {
|
||||
return {
|
||||
name: "_status",
|
||||
type: "custom",
|
||||
value: null,
|
||||
serialize: false,
|
||||
options: {},
|
||||
_text: "",
|
||||
computeSize(width) {
|
||||
return [width, STATUS_HEIGHT];
|
||||
},
|
||||
draw(ctx, node, widgetWidth, y, height) {
|
||||
const margin = 14;
|
||||
const innerW = widgetWidth - margin * 2;
|
||||
ctx.save();
|
||||
ctx.beginPath();
|
||||
if (ctx.roundRect) ctx.roundRect(margin, y + 2, innerW, height - 4, 4);
|
||||
else ctx.rect(margin, y + 2, innerW, height - 4);
|
||||
ctx.fillStyle = "#111";
|
||||
ctx.fill();
|
||||
ctx.strokeStyle = this._text ? "#3a3a3a" : "#222";
|
||||
ctx.lineWidth = 1;
|
||||
ctx.stroke();
|
||||
ctx.font = "11px sans-serif";
|
||||
ctx.textAlign = "center";
|
||||
ctx.textBaseline = "middle";
|
||||
ctx.fillStyle = this._text ? "#f28f41" : "#666";
|
||||
ctx.fillText(this._text || placeholderText, widgetWidth / 2, y + height / 2);
|
||||
ctx.restore();
|
||||
},
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 3: Verify no console errors on load**
|
||||
|
||||
Same check as Task 1 Step 2 — file isn't imported yet, this just confirms it's syntactically valid by checking it parses (open it directly: `mcp__Claude_Browser__navigate` to `http://127.0.0.1:8188/extensions/ComfyUI-CraftKit/shared/canvas_widgets.mjs` and confirm the browser shows the raw JS text, not a 404 or a syntax-highlighted error page).
|
||||
|
||||
- [x] **Step 4: Ready for commit**
|
||||
|
||||
```bash
|
||||
git add js/shared/canvas_widgets.mjs
|
||||
git commit -m "feat: add shared divider/status widget factories for Nodes 2.0 compat"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Shared preset-picker widget factory (dual-path)
|
||||
|
||||
**Files:**
|
||||
- Create: `js/shared/preset_picker_widget.mjs`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `isVueNodes()` from `js/shared/nodes2.mjs` (Task 1).
|
||||
- Produces: `export function createPresetPickerWidget(node, targetWidget, presets: number[]): object` — returns a widget already added to `node` via `addCustomWidget` (classic) or `addDOMWidget` (Nodes 2.0). The caller is responsible for repositioning it in `node.widgets` (same splice pattern the call sites already use) and does NOT need to call `node.addCustomWidget`/`addDOMWidget` itself — this factory does that internally, since the two paths need different LiteGraph APIs.
|
||||
|
||||
- [x] **Step 1: Write the classic-mode implementation**
|
||||
|
||||
```js
|
||||
// js/shared/preset_picker_widget.mjs
|
||||
import { isVueNodes } from "./nodes2.mjs";
|
||||
|
||||
const PRESET_ROW_HEIGHT = 22;
|
||||
|
||||
function createClassicPresetWidget(node, targetWidget, presets) {
|
||||
const widget = {
|
||||
name: "size_presets",
|
||||
type: "custom",
|
||||
value: null,
|
||||
serialize: false,
|
||||
options: {},
|
||||
_rects: [],
|
||||
computeSize(width) {
|
||||
return [width, PRESET_ROW_HEIGHT];
|
||||
},
|
||||
draw(ctx, node, widgetWidth, y, height) {
|
||||
const margin = 14;
|
||||
const gap = 5;
|
||||
const innerW = Math.max(40, widgetWidth - margin * 2);
|
||||
const n = presets.length;
|
||||
const cellW = Math.max(10, (innerW - gap * (n - 1)) / n);
|
||||
const h = Math.min(height - 2, 18);
|
||||
const top = y + (height - h) / 2;
|
||||
const current = targetWidget.value;
|
||||
this._rects = [];
|
||||
|
||||
ctx.save();
|
||||
ctx.font = "11px sans-serif";
|
||||
ctx.textAlign = "center";
|
||||
ctx.textBaseline = "middle";
|
||||
|
||||
for (let i = 0; i < n; i++) {
|
||||
const x = margin + i * (cellW + gap);
|
||||
const active = Number(current) === presets[i];
|
||||
|
||||
ctx.beginPath();
|
||||
if (ctx.roundRect) ctx.roundRect(x, top, cellW, h, 4);
|
||||
else ctx.rect(x, top, cellW, h);
|
||||
ctx.fillStyle = active ? "#f28f41" : "#2a2a2a";
|
||||
ctx.fill();
|
||||
ctx.lineWidth = 1;
|
||||
ctx.strokeStyle = active ? "#f28f41" : "#555";
|
||||
ctx.stroke();
|
||||
|
||||
ctx.fillStyle = active ? "#111" : "#ccc";
|
||||
ctx.fillText(String(presets[i]), x + cellW / 2, top + h / 2 + 0.5);
|
||||
|
||||
this._rects.push({ x1: x, x2: x + cellW, y1: top, y2: top + h, size: presets[i] });
|
||||
}
|
||||
ctx.restore();
|
||||
},
|
||||
mouse(event, pos, node) {
|
||||
if (event.type !== "pointerdown" && event.type !== "mousedown") return false;
|
||||
const [lx, ly] = pos;
|
||||
for (const r of this._rects) {
|
||||
if (lx >= r.x1 && lx <= r.x2 && ly >= r.y1 && ly <= r.y2) {
|
||||
targetWidget.value = r.size;
|
||||
targetWidget.callback?.(r.size, app.canvas, node);
|
||||
node.setDirtyCanvas(true, true);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
},
|
||||
};
|
||||
return node.addCustomWidget(widget);
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 2: Write the Nodes 2.0 DOM implementation**
|
||||
|
||||
```js
|
||||
function createVuePresetWidget(node, targetWidget, presets) {
|
||||
const root = document.createElement("div");
|
||||
root.style.display = "flex";
|
||||
root.style.gap = "5px";
|
||||
root.style.padding = "0 14px";
|
||||
root.style.width = "100%";
|
||||
root.style.boxSizing = "border-box";
|
||||
|
||||
const buttons = presets.map((size) => {
|
||||
const btn = document.createElement("button");
|
||||
btn.textContent = String(size);
|
||||
btn.style.flex = "1";
|
||||
btn.style.padding = "4px 0";
|
||||
btn.style.borderRadius = "4px";
|
||||
btn.style.border = "1px solid #555";
|
||||
btn.style.background = "#2a2a2a";
|
||||
btn.style.color = "#ccc";
|
||||
btn.style.fontSize = "11px";
|
||||
btn.style.fontFamily = "sans-serif";
|
||||
btn.style.cursor = "pointer";
|
||||
const syncActive = () => {
|
||||
const active = Number(targetWidget.value) === size;
|
||||
btn.style.background = active ? "#f28f41" : "#2a2a2a";
|
||||
btn.style.borderColor = active ? "#f28f41" : "#555";
|
||||
btn.style.color = active ? "#111" : "#ccc";
|
||||
};
|
||||
btn.addEventListener("click", () => {
|
||||
targetWidget.value = size;
|
||||
targetWidget.callback?.(size, app.canvas, node);
|
||||
for (const b of buttons) b.syncActive?.();
|
||||
node.setDirtyCanvas(true, true);
|
||||
});
|
||||
btn.syncActive = syncActive;
|
||||
syncActive();
|
||||
root.appendChild(btn);
|
||||
return btn;
|
||||
});
|
||||
|
||||
const widget = node.addDOMWidget("size_presets", "craftkit_preset_picker", root, {
|
||||
serialize: false,
|
||||
hideOnZoom: false,
|
||||
});
|
||||
// Fixed-height row, not a flexible grower — same reasoning as Pixaroma's
|
||||
// button-row DOM widget (see js/shared/nodes2.mjs comment).
|
||||
widget.computeLayoutSize = undefined;
|
||||
return widget;
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 3: Write the dispatcher**
|
||||
|
||||
```js
|
||||
export function createPresetPickerWidget(node, targetWidget, presets) {
|
||||
return isVueNodes()
|
||||
? createVuePresetWidget(node, targetWidget, presets)
|
||||
: createClassicPresetWidget(node, targetWidget, presets);
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 4: Add the `app` import needed by both paths**
|
||||
|
||||
Add at the top of the file: `import { app } from "../../../scripts/app.js";` (three levels up from `js/shared/` to reach ComfyUI's `scripts/` — verify this relative path against how `smart_batch_resize.js` imports it today, `import { app } from "../../scripts/app.js";`, and adjust the level count for the extra `shared/` nesting: `js/shared/preset_picker_widget.mjs` → `scripts/app.js` is `../../../scripts/app.js`).
|
||||
|
||||
- [x] **Step 5: Ready for commit**
|
||||
|
||||
```bash
|
||||
git add js/shared/preset_picker_widget.mjs
|
||||
git commit -m "feat: add dual-path (classic/Nodes 2.0) preset-picker widget factory"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Rewire `smart_batch_resize.js`
|
||||
|
||||
**Files:**
|
||||
- Modify: `js/smart_batch_resize.js`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `createDividerWidget`, `createStatusWidget` (Task 2), `createPresetPickerWidget` (Task 3).
|
||||
|
||||
- [x] **Step 1: Add imports**
|
||||
|
||||
At the top of `js/smart_batch_resize.js`, after the existing `import { app } from "../../scripts/app.js";`:
|
||||
|
||||
```js
|
||||
import { createDividerWidget, createStatusWidget } from "./shared/canvas_widgets.mjs";
|
||||
import { createPresetPickerWidget } from "./shared/preset_picker_widget.mjs";
|
||||
```
|
||||
|
||||
- [x] **Step 2: Replace the local preset-chip block**
|
||||
|
||||
Find the block starting `const longestSideWidget = node.widgets?.find(w => w.name === "longest_side");` through the `if (pwIdx !== lsIdx + 1) { ... }` reposition block that follows it (this is the whole "Compact preset chips for longest_side" section). Replace the widget-creation part (everything between finding `longestSideWidget` and the "Move preset row" comment) with:
|
||||
|
||||
```js
|
||||
const longestSideWidget = node.widgets?.find(w => w.name === "longest_side");
|
||||
if (longestSideWidget) {
|
||||
const PRESETS = [512, 768, 1024, 1536];
|
||||
const presetWidget = createPresetPickerWidget(node, longestSideWidget, PRESETS);
|
||||
|
||||
// Move preset row to right after longest_side
|
||||
const lsIdx = node.widgets.indexOf(longestSideWidget);
|
||||
const pwIdx = node.widgets.indexOf(presetWidget);
|
||||
if (pwIdx !== lsIdx + 1) {
|
||||
node.widgets.splice(pwIdx, 1);
|
||||
node.widgets.splice(lsIdx + 1, 0, presetWidget);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 3: Replace the local `addSectionDivider` function and its 5 call sites**
|
||||
|
||||
Delete the `addSectionDivider` function definition (the block from `const addSectionDivider = (beforeWidgetName, label) => {` through its closing `};`) and the 5 calls below it. Replace with:
|
||||
|
||||
```js
|
||||
const addSectionDivider = (beforeWidgetName, label) => {
|
||||
const target = node.widgets.find(w => w.name === beforeWidgetName);
|
||||
if (!target) return;
|
||||
const div = createDividerWidget(label);
|
||||
node.addCustomWidget(div);
|
||||
const targetIdx = node.widgets.indexOf(target);
|
||||
const divIdx = node.widgets.indexOf(div);
|
||||
node.widgets.splice(divIdx, 1);
|
||||
node.widgets.splice(targetIdx, 0, div);
|
||||
};
|
||||
addSectionDivider("longest_side", "RESIZE");
|
||||
addSectionDivider("prefix", "FILENAME");
|
||||
addSectionDivider("folder_resolution", "OUTPUT LOCATION");
|
||||
addSectionDivider("output_format", "OUTPUT FORMAT");
|
||||
addSectionDivider("skip_if_exists", "OPTIONS");
|
||||
```
|
||||
|
||||
(Note the 5th call's target changed from `"skip_if_exists"` to match the current on-disk section order — verify against the live file before applying, since Task order in this plan was written after the RESIZE→FILENAME→OUTPUT LOCATION→OUTPUT FORMAT→OPTIONS reorder; read the file first and match whatever the current 5 calls actually are, only changing the widget-creation mechanism, not the section order/labels.)
|
||||
|
||||
- [x] **Step 4: Replace the local status widget block**
|
||||
|
||||
Find `const statusWidget = node.addWidget("button", "_sbr_status", null, () => {}, { serialize: false });` through the end of its `statusWidget.draw = function (...) {...};` block. Replace with:
|
||||
|
||||
```js
|
||||
const statusWidget = createStatusWidget("— RUN TO PROCESS —");
|
||||
node.addCustomWidget(statusWidget);
|
||||
```
|
||||
|
||||
- [x] **Step 5: Verify `onExecuted` still works unmodified**
|
||||
|
||||
Confirm the existing `node.onExecuted` override (later in the file) still reads `statusWidget._text = output.text[0];` — no change needed there since `createStatusWidget`'s returned object still exposes a mutable `._text` property.
|
||||
|
||||
- [x] **Step 6: Manual verification — classic mode**
|
||||
|
||||
Using the Claude Browser MCP tool: navigate to `http://127.0.0.1:8188/`, confirm Settings → Comfy → "Modern Node Design (Nodes 2.0)" is OFF, add a fresh Smart Batch Resize node, screenshot. Confirm: 5 section labels (RESIZE/FILENAME/OUTPUT LOCATION/OUTPUT FORMAT/OPTIONS) draw with the label+line style (not as buttons), the 4 preset chips (512/768/1024/1536) draw as a colored chip row and clicking one updates `longest_side`, the status box shows "— RUN TO PROCESS —" placeholder text.
|
||||
|
||||
- [x] **Step 7: Manual verification — Nodes 2.0 mode**
|
||||
|
||||
Toggle Settings → Comfy → "Modern Node Design (Nodes 2.0)" ON, reload the page, add a fresh Smart Batch Resize node, screenshot. Confirm: same 5 section labels draw correctly (not literal `_div_RESIZE` button text), the preset row now shows as real `<button>` elements and clicking one updates `longest_side` and highlights the clicked button, the status box still shows correctly.
|
||||
|
||||
- [x] **Step 8: Save/reload regression check (both modes)**
|
||||
|
||||
In whichever mode is currently active, change `longest_side` to a non-default value (e.g. click the "768" preset), save the workflow (Ctrl+S), reload the page, confirm the node reopens with `longest_side = 768` and every other widget value in its correct position (no value-shift — the serialize-override logic in this file is unchanged by this task, so this step is a regression check, not a new fix).
|
||||
|
||||
- [x] **Step 9: Ready for commit**
|
||||
|
||||
```bash
|
||||
git add js/smart_batch_resize.js
|
||||
git commit -m "fix: render Smart Batch Resize's dividers/status/presets correctly under Nodes 2.0"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Rewire `smart_resize.js`
|
||||
|
||||
**Files:**
|
||||
- Modify: `js/smart_resize.js`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `createPresetPickerWidget` (Task 3).
|
||||
|
||||
- [x] **Step 1: Add import**
|
||||
|
||||
After `import { app } from "../../scripts/app.js";`:
|
||||
|
||||
```js
|
||||
import { createPresetPickerWidget } from "./shared/preset_picker_widget.mjs";
|
||||
```
|
||||
|
||||
- [x] **Step 2: Replace the local preset-chip block**
|
||||
|
||||
Replace the entire block from `const presetWidget = node.addWidget("button", "size_presets", ...)` through the end of `presetWidget.draw = function (...) {...};` with:
|
||||
|
||||
```js
|
||||
const presetWidget = createPresetPickerWidget(node, longestSideWidget, PRESETS);
|
||||
```
|
||||
|
||||
Keep the `const PRESETS = [512, 768, 1024, 1536];` line above it, and keep the "Move preset row to right after longest_side" reposition block below it unchanged (it already operates on `presetWidget` by reference/position, not by how it was created).
|
||||
|
||||
- [x] **Step 3: Manual verification — classic mode**
|
||||
|
||||
Same procedure as Task 4 Step 6, but for Smart Resize: add a fresh node, confirm the preset chips draw and work.
|
||||
|
||||
- [x] **Step 4: Manual verification — Nodes 2.0 mode**
|
||||
|
||||
Same procedure as Task 4 Step 7, but for Smart Resize.
|
||||
|
||||
- [x] **Step 5: Save/reload regression check**
|
||||
|
||||
Same procedure as Task 4 Step 8, but for Smart Resize (change `longest_side` via a preset, save, reload, confirm no value-shift across `longest_side`, `multiple_of`, `interpolation`, `upscale_if_smaller`).
|
||||
|
||||
- [x] **Step 6: Ready for commit**
|
||||
|
||||
```bash
|
||||
git add js/smart_resize.js
|
||||
git commit -m "fix: render Smart Resize's preset picker correctly under Nodes 2.0"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: Rewire `smart_profile_switch.js`
|
||||
|
||||
**Files:**
|
||||
- Modify: `js/smart_profile_switch.js`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `createDividerWidget`, `createStatusWidget` (Task 2).
|
||||
|
||||
- [x] **Step 1: Add import**
|
||||
|
||||
After `import { app } from "../../scripts/app.js";`:
|
||||
|
||||
```js
|
||||
import { createDividerWidget, createStatusWidget } from "./shared/canvas_widgets.mjs";
|
||||
```
|
||||
|
||||
- [x] **Step 2: Replace the local divider-array block**
|
||||
|
||||
Find the block that builds `const dividers = [];` with a `for (let i = 1; i <= 4; i++) { ... }` loop creating `_div_${i}` widgets with a per-slot `draw` closure. Replace with:
|
||||
|
||||
```js
|
||||
const dividers = [];
|
||||
for (let i = 1; i <= 4; i++) {
|
||||
const div = createDividerWidget(`SLOT ${i}`);
|
||||
node.addCustomWidget(div);
|
||||
dividers.push(div);
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 3: Replace the local status widget block**
|
||||
|
||||
Find `const statusWidget = node.addWidget("button", "_sps_status", null, () => {}, { serialize: false });` through the end of its `statusWidget.draw = function (...) {...};` block. Replace with:
|
||||
|
||||
```js
|
||||
const statusWidget = createStatusWidget("— RUN TO SEE RESULT —");
|
||||
node.addCustomWidget(statusWidget);
|
||||
```
|
||||
|
||||
(Confirm the exact current placeholder text in the live file before applying — copy it verbatim rather than assuming, since this plan is written from a description of the file, not a fresh read of it at execution time.)
|
||||
|
||||
- [x] **Step 4: Verify the existing reorder logic is unaffected**
|
||||
|
||||
The file's `node.widgets = [indexWidget, ...slotWidgets, statusWidget]`-style reorder logic (with the null-guard/leftover-append fix from M-20) already operates on `dividers[i-1]` and `statusWidget` by reference — no change needed there since `createDividerWidget`/`createStatusWidget` still return plain objects usable the same way.
|
||||
|
||||
- [x] **Step 5: Manual verification — classic mode**
|
||||
|
||||
Add a fresh Smart Profile Switch node with Nodes 2.0 OFF, screenshot, confirm all 4 "SLOT N" dividers and the status box draw correctly (not as buttons).
|
||||
|
||||
- [x] **Step 6: Manual verification — Nodes 2.0 mode**
|
||||
|
||||
Toggle Nodes 2.0 ON, reload, add a fresh Smart Profile Switch node, screenshot, confirm the same.
|
||||
|
||||
- [x] **Step 7: Save/reload regression check**
|
||||
|
||||
Change a `label_n`/`width_n`/`height_n` value, save, reload, confirm all 13 real widget values land in their correct slots in both modes.
|
||||
|
||||
- [x] **Step 8: Ready for commit**
|
||||
|
||||
```bash
|
||||
git add js/smart_profile_switch.js
|
||||
git commit -m "fix: render Smart Profile Switch's SLOT dividers/status correctly under Nodes 2.0"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 7: Rewire `smart_prompt_controller.js`
|
||||
|
||||
**Files:**
|
||||
- Modify: `js/smart_prompt_controller.js`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `createStatusWidget` (Task 2).
|
||||
|
||||
- [x] **Step 1: Add import**
|
||||
|
||||
After `import { app } from "../../scripts/app.js";`:
|
||||
|
||||
```js
|
||||
import { createStatusWidget } from "./shared/canvas_widgets.mjs";
|
||||
```
|
||||
|
||||
- [x] **Step 2: Replace the local status widget block**
|
||||
|
||||
Find `const statusWidget = node.addWidget("button", "_spc_result", null, () => {}, { serialize: false });` through the end of its `statusWidget.draw = function (...) {...};` block. Replace with:
|
||||
|
||||
```js
|
||||
const statusWidget = createStatusWidget("— RUN TO SEE RESULT —");
|
||||
node.addCustomWidget(statusWidget);
|
||||
```
|
||||
|
||||
(Again, copy the live file's exact current placeholder text rather than assuming it matches Task 6's.)
|
||||
|
||||
- [x] **Step 3: Verify the serialize override added for M-19 still applies correctly**
|
||||
|
||||
This file's serialize override (added for M-19, filters `w.serialize !== false && w.options?.serialize !== false`) already excludes this widget via its `serialize: false` property — confirm `createStatusWidget`'s returned object still has `serialize: false` set (it does, per Task 2 Step 2) so no change is needed to the override itself.
|
||||
|
||||
- [x] **Step 4: Manual verification — classic mode**
|
||||
|
||||
Add a fresh Smart Prompt Controller node with Nodes 2.0 OFF, screenshot, confirm the status box draws correctly.
|
||||
|
||||
- [x] **Step 5: Manual verification — Nodes 2.0 mode**
|
||||
|
||||
Toggle Nodes 2.0 ON, reload, add a fresh Smart Prompt Controller node, screenshot, confirm the same.
|
||||
|
||||
- [x] **Step 6: Save/reload regression check**
|
||||
|
||||
Change `index` and `control_after_generate`, save, reload, confirm both land correctly in both modes (this is the exact M-19 bug this file was fixed for earlier in the session — re-verify it still holds after this widget-creation change).
|
||||
|
||||
- [x] **Step 7: Ready for commit**
|
||||
|
||||
```bash
|
||||
git add js/smart_prompt_controller.js
|
||||
git commit -m "fix: render Smart Prompt Controller's status box correctly under Nodes 2.0"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 8: Full cross-mode verification pass + cleanup
|
||||
|
||||
**Files:** none (verification only, plus a final consistency check across all 4 files)
|
||||
|
||||
- [x] **Step 1: Grep for any remaining `addWidget("button", "_` or `addWidget("button", "size_presets"` calls**
|
||||
|
||||
```bash
|
||||
grep -rn 'addWidget("button", "_\|addWidget("button", "size_presets"' js/
|
||||
```
|
||||
|
||||
Expected: no matches (everything decorative/custom-drawn now goes through `addCustomWidget`/`addDOMWidget` via the shared factories; only genuinely clickable, single-purpose buttons like "Browse folder" and "Run Batch" should still use `addWidget("button", ...)`, and those are unaffected by this plan).
|
||||
|
||||
- [x] **Step 2: Confirm genuine functional buttons are untouched**
|
||||
|
||||
```bash
|
||||
grep -n 'addWidget("button"' js/smart_batch_resize.js
|
||||
```
|
||||
|
||||
Expected: exactly 2 matches — "📁 Browse folder" and "▶ Run Batch" — both real, single-click-triggers-an-action buttons that should keep using LiteGraph's native `ButtonWidget` (they render correctly as real buttons in both classic and Nodes 2.0 already, since `"button"` is a Vue-recognized type that Vue renders as an actual functional button — this is only a problem for widgets that abuse `"button"` to paint something else via `draw()`).
|
||||
|
||||
- [x] **Step 3: Full-node screenshot sweep, both modes**
|
||||
|
||||
With Nodes 2.0 OFF: place all 5 CraftKit nodes (Smart Prompt Controller, Smart Profile Switch, Smart Resize, Smart Batch Resize, Smart Resolution Multiplier) on one canvas, screenshot.
|
||||
|
||||
Toggle Nodes 2.0 ON, reload, repeat: place all 5 fresh nodes on one canvas, screenshot.
|
||||
|
||||
Compare the two screenshots side by side — confirm no node shows a literal internal widget name (`_div_...`, `_status`, `_sbr_status`, `_sps_status`, `_spc_result`, `size_presets`) as visible button text in either mode.
|
||||
|
||||
- [x] **Step 4: Update the project roadmap memory (no git action — this file lives outside the repo)**
|
||||
|
||||
Edit `C:\Users\rubzm\.claude\projects\D--AI-ComfyUI-Easy-Install-ComfyUI-custom-nodes-ComfyUI-CraftKit\memory\project_roadmap.md`, add a line under "Afgerond" noting Nodes 2.0 compatibility was added for all custom-drawn widgets, dated with today's date, mentioning the `js/shared/` module split as a new architectural pattern other future CraftKit JS work should reuse. This is a memory-file edit, not a repo file — no `git add`/`git commit` for this step.
|
||||
|
||||
- [x] **Step 5: Ready for the final repo commit**
|
||||
|
||||
All of Tasks 1–7 already produced their own commits. If any uncommitted repo changes remain at this point (e.g. a fix made during Task 8's verification sweep), stage and commit those:
|
||||
|
||||
```bash
|
||||
git status --short
|
||||
git add <any remaining changed files>
|
||||
git commit -m "fix: final Nodes 2.0 compatibility touch-ups from the verification sweep"
|
||||
```
|
||||
|
||||
If `git status --short` is empty, there is nothing to commit — Tasks 1–7's commits already cover everything.
|
||||
|
||||
---
|
||||
|
||||
## Self-Review Notes (from plan authoring)
|
||||
|
||||
- **Spec coverage:** all 4 JS files with custom-drawn widgets are covered (Tasks 4–7); the shared-factory extraction (Tasks 1–3) removes the duplication between `smart_batch_resize.js`'s and `smart_resize.js`'s near-identical preset-chip code, and between the divider/status code duplicated across 3 files — this was flagged as a Reuse-category improvement opportunity earlier in the session and is folded in here since this plan already touches all the relevant call sites.
|
||||
- **Known risk flagged explicitly:** Task 3's classic-mode `computeSize` must return `[width, height]`, not a hardcoded value — this repo has direct prior history (a comment in `smart_resize.js`) of this exact mistake breaking node layout. Every `computeSize` in this plan follows the verified-working Pixaroma pattern.
|
||||
- **Out of scope, deliberately:** `js/shared/nodes2.mjs`'s `applyAdaptiveCanvasOnly` helper (used by Pixaroma to control Parameters-sidebar-tab visibility per renderer) is NOT included — none of CraftKit's decorative widgets currently set `options.canvasOnly`, and adding sidebar-visibility control is a separate, optional cosmetic refinement, not required to fix the "widgets render as raw button text" bug. Flag as a possible follow-up if the decorative widgets turn out to clutter the Nodes 2.0 Parameters sidebar tab during Task 8's verification — if so, that's a good candidate for a small follow-up task, not a blocker for this plan.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Log (2026-08-07)
|
||||
|
||||
Executed inline (not subagent-driven), autonomously while the user slept, then jointly debugged one regression with the user once they were back. Summary of what actually happened, including one deviation from the plan as originally written.
|
||||
|
||||
### What went exactly as planned
|
||||
|
||||
- Tasks 1–2 (`js/shared/nodes2.mjs`, `js/shared/canvas_widgets.mjs`): implemented verbatim as specified. `node --check` syntax-clean on first try.
|
||||
- Task 4–7 rewiring: all 4 node JS files updated to use the shared factories. Dividers and status boxes (the `addCustomWidget`/`type:"custom"` path) worked correctly in both classic and Nodes 2.0 rendering on the **first live test**, with zero further changes needed — the `computeSize(width) → [width, height]` pattern copied from Pixaroma was exactly right.
|
||||
- Environment friction, not a code bug: ComfyUI's static file server for custom-node `WEB_DIRECTORY` content does **not** pick up newly-created files (including brand-new subdirectories) without a full process restart — confirmed by testing both a new nested file and a new flat top-level file, both 404 until restart, while edits to already-existing files serve live with no restart needed. Worth remembering for any future new-file additions to this repo's `js/`.
|
||||
- Mid-session environment mixup (not a code issue): partway through, port 8188 was unexpectedly being served by an unrelated second ComfyUI installation (`D:\AI\ComfyUI-AGAIN`) the user had set up separately to work around an earlier database-corruption issue. All the "files don't exist" symptoms during that window were because of testing against the wrong install, not a real serving bug. Resolved once the user pointed the real `ComfyUI-Easy-Install` instance back at port 8188.
|
||||
|
||||
### The regression found during verification (Task 8) — and its fix
|
||||
|
||||
**Symptom:** on Smart Batch Resize and Smart Resize specifically (the two nodes with the preset-chip picker), saving a workflow and reloading it under Nodes 2.0 showed every widget value shifted one row up relative to its label (e.g. "Round to multiple of" displaying `interpolation`'s value, "Interpolation method" displaying `upscale_if_smaller`'s value, and so on down the node). Smart Profile Switch and Smart Prompt Controller (dividers/status only, no preset picker) reloaded with every value correct.
|
||||
|
||||
**First, ruled out:** the saved workflow JSON itself. Inspected the actual `widgets_values` array in the saved `.json` file directly — it was `[1536, 8, "lanczos", true]` for Smart Resize, i.e. completely correct, right length, right order. This proved the `serialize()` override (which excludes non-serializable JS widgets from the saved array) was working correctly, and the bug was purely in how the Nodes 2.0 Vue layer re-associated the correctly-loaded values with widget rows after `nodeCreated()` ran.
|
||||
|
||||
**Root cause:** `createPresetPickerWidget`'s Nodes 2.0 path uses `node.addDOMWidget(...)`, and both call sites then repositioned it via `node.widgets.splice(...)` to sit directly after `longest_side` — the same reposition pattern already used safely for classic-mode's canvas-drawn preset widget and for all the `addCustomWidget`-based dividers. That pattern is safe for `addCustomWidget` widgets (bridged through `WidgetLegacy.vue`, which reads each widget's own `.value` directly) but **not** for `addDOMWidget` widgets under Nodes 2.0: splicing a DOM widget into the middle of `node.widgets` *after* `configure()` has already loaded the saved values onto the pre-existing widget objects desyncs whatever positional/reactive binding Vue uses for the widget list, shifting every widget after the splice point by one row in the display — even though each widget's underlying `.value` property is untouched and correct. This is consistent with the observed data: only nodes with a mid-list `addDOMWidget` insertion were affected; nodes with only `addCustomWidget` insertions were not.
|
||||
|
||||
**Fix:** in `js/smart_batch_resize.js` and `js/smart_resize.js`, the "move preset row to right after longest_side" reposition step is now skipped entirely when `isVueNodes()` is true — the DOM widget is left wherever `addDOMWidget` placed it (empirically, the end of `node.widgets`, after all real Python widgets, the status box, and the Run button). Classic mode is unaffected: its canvas-drawn preset widget still repositions normally, since that path never showed the bug.
|
||||
|
||||
**Verified fix, both nodes, full save → reload cycle under Nodes 2.0:**
|
||||
- Smart Batch Resize: changed `longest_side` to 768 via a preset click, saved, reloaded — every one of the 18 real widgets read back correct (spot-verified via the sidebar Parameters panel plus the node's own canvas text), preset chips still rendered as real `<button>` elements at their new position (after `preview_limit`, before the status box and Run Batch button) and were confirmed clickable.
|
||||
- Smart Resize: same check, `longest_side` read back correctly along with `multiple_of`/`interpolation`/`upscale_if_smaller`.
|
||||
- Smart Profile Switch and Smart Prompt Controller: re-verified with a changed `index` value (7) through the same save → reload cycle as a negative control, confirming they were never affected (no `addDOMWidget` in either file) — both read back correct.
|
||||
|
||||
**Visual trade-off accepted:** under Nodes 2.0 specifically, the preset-chip row no longer sits directly under "Longest side (px)" — it's now at the bottom of the node, after all other settings. Classic mode keeps the original, nicer placement. Revisiting this (e.g. finding a way to splice a DOM widget safely, or moving to a different widget-list-diffing strategy) is a possible future refinement, not required for correctness.
|
||||
|
||||
### Net effect vs. the original plan
|
||||
|
||||
Tasks 1, 2, 4, 5, 6, 7 landed as specified. Task 3 (`createPresetPickerWidget`) needed one addition beyond what was originally planned: callers must guard the reposition-splice with `!isVueNodes()`. This wasn't anticipated by the original research (which correctly identified *that* a dual-path widget was needed, but not this specific interaction between splice-repositioning and Vue's post-`configure()` reactivity). Task 8's manual verification sweep is exactly what caught it — this is the reason the plan's "no automated JS test harness, verify manually" constraint insisted on an actual save → reload round-trip check per node, not just a visual glance at a freshly-added node.
|
||||
- **WorkflowOrganizer is explicitly out of scope** per the user's instruction ("workflow organiser doen we later checken") — this plan touches only ComfyUI-CraftKit.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Log (2026-08-13) — simplification + the widget.width staleness bug
|
||||
|
||||
Picks up after the (2026-08-07) log above. Two follow-up rounds: first a deliberate architecture simplification, then a hard-to-reproduce click bug that needed a second opinion from a fresh model to actually crack.
|
||||
|
||||
### Simplification: dropped the dual-path DOM widget entirely
|
||||
|
||||
The (2026-08-07) log accepted a visual trade-off: under Nodes 2.0 the preset-chip row sat at the *bottom* of the node instead of directly under "Longest side (px)", because repositioning the `addDOMWidget`-based Nodes-2.0 variant mid-list desynced Vue's save/reload binding. The user flagged this as bad UX once they actually saw it side by side with classic mode.
|
||||
|
||||
Fix: dropped the `addDOMWidget`/`isVueNodes()` dual-path in `preset_picker_widget.mjs` entirely, keeping only the single canvas-drawn (`type: "custom"`) implementation in both renderers. This is safe because the preset picker is click-only (no hover state was ever implemented), and canvas `"custom"` widgets were already proven immune to the reposition/reload bug (unlike `addDOMWidget`). `js/shared/nodes2.mjs` (the `isVueNodes()` helper) became unused and was deleted. Reposition-splice guards were made unconditional again in both `smart_batch_resize.js` and `smart_resize.js`.
|
||||
|
||||
Net result: preset chips now sit in the same, correct position in both renderers, with one shared draw/hit-test implementation instead of two.
|
||||
|
||||
### Widget-visibility helper, and the real WidgetLegacy.vue render-trigger model
|
||||
|
||||
Separately, "Counter start" (shown/hidden based on the "Add counter" toggle) and the customtext output widget that all 4 nodes hide after execution both used a `computeSize = () => [0,-4]` collapse trick. This works in classic mode (canvas layout calls `computeSize()` every frame) but silently does nothing under Nodes 2.0 — confirmed by reading `useProcessedWidgets.ts`'s `computeProcessedWidgets()`: Nodes 2.0's widget list is a Vue `shallowReactive` array (see `useGraphNodeManager.ts`, `extractVueNodeData()`) that only tracks the array's own indices/length as reactive dependencies, never properties nested inside each widget object (like `widget.options.hidden`) — so mutating those never triggers a re-render.
|
||||
|
||||
Fix, in new `js/shared/widget_visibility.mjs` (`setWidgetVisible(node, widget, visible)`): set `widget.hidden`/`widget.options.hidden` *and* perform a structural no-op mutation on `node.widgets` (push a dummy widget, immediately splice it back out) to force Vue's shallowReactive array to actually notice something changed and recompute. Verified this exact mechanism via live console testing before writing the real fix — a plain `widget.options.hidden = true` + `node.setDirtyCanvas()` provably did nothing in Nodes 2.0 (confirmed via `Object.getOwnPropertyDescriptor(node, 'widgets')` showing the Vue-installed getter/setter), while the push+splice nudge worked immediately.
|
||||
|
||||
Along the way, also discovered (via reading `WidgetLegacy.vue`'s actual source, extracted from the frontend package's sourcemap) that **`node.setDirtyCanvas()` does not repaint a Nodes-2.0 `WidgetLegacy`-bridged widget at all** — only calling the widget's own `triggerDraw()` (a function `WidgetLegacy.vue` assigns onto the widget object once mounted) does. This explained two more stale-visual bugs: the active/orange preset chip not updating after a click, and the status box text not updating after a run, in Nodes 2.0 specifically. Fixed by calling `this.triggerDraw?.()` (in `mouse()`) / `statusWidget.triggerDraw?.()` (in `onExecuted`) alongside the existing `setDirtyCanvas()` calls, everywhere this pattern occurred across all 4 node JS files.
|
||||
|
||||
Also found and fixed: the section-divider line (`createDividerWidget` in `canvas_widgets.mjs`) was being drawn correctly but was visually near-invisible under Nodes 2.0 — its `#333` stroke color survived classic mode's native-resolution canvas fine, but got softened into invisibility by Nodes 2.0's extra 2x-backing-store + CSS-zoom-transform rendering path. Fixed by brightening to `#555` and centering the label with flanking lines on both sides (`── LABEL ──`) instead of label-left/line-right, per user request.
|
||||
|
||||
### The click-reliability bug that needed a second opinion
|
||||
|
||||
After the above fixes, the user reported the preset chips "still not good" in classic mode specifically (Nodes 2.0 was fine): clicks on the right-hand chips of a *wide* node (they'd resized one to ~580px) either landed on the wrong chip or did nothing at all — worst on the rightmost ("1536") chip. Extensive live-tested hypotheses (screenshot-timing artifacts, `getWidgetOnPos` hit-detection, `_rects` staleness after `setSize()`, node-selection-order effects, `HIT_PAD`/chip-size tuning) all failed to nail a reproducible root cause through iterative testing — some rounds even made it look *worse* after a "bigger chips" fix landed, which the user called out immediately as suspicious.
|
||||
|
||||
Spawned a fresh Opus subagent as a genuine second opinion, with full context (exact symptom history, files, and everything already ruled out) but explicit instructions to re-derive from the actual source rather than trust the existing narrative. It found the real root cause on the first pass by reading `WidgetLegacy.vue` and `LGraphNode.ts` (both extracted via the frontend package's sourcemaps) side by side:
|
||||
|
||||
- `WidgetLegacy.vue` (Nodes 2.0's canvas-draw bridge) permanently stamps `widgetInstance.width = width` (its own, possibly-narrower container width) onto the **shared widget object** on every mount — and its `onBeforeUnmount` only resets `triggerDraw`, never `width`.
|
||||
- Classic LiteGraph's own click hit-test, `getWidgetOnPos` (`LGraphNode.ts` ~line 2281), reads `widget.width || nodeWidth` to decide the clickable X range — **not** `node.size[0]`.
|
||||
- Net effect: once a node has ever been rendered under Nodes 2.0 (even briefly, even in a since-closed tab), classic mode's click detection for that widget stays capped at whatever (possibly narrow) width Nodes 2.0 last stamped, forever, regardless of the node's actual current width, until the widget object itself is recreated.
|
||||
- This also explained the "made it worse" observation precisely: an earlier fix (`c6c26a3`, this project's commit reading live `node.size[0]` in `draw()` instead of the stale `widgetWidth` param) fixed the *drawing* to be full-width, but the *hit-test* stayed capped at the stale stamped width — so chips started being drawn wider than they were actually clickable, which is strictly worse UX than being uniformly-too-narrow-but-consistent.
|
||||
|
||||
Fix, in `preset_picker_widget.mjs`'s `draw()`: `this.width = w;` (writing the live, correct width back onto the widget object) on every single draw call, in both renderers — so a stale Nodes-2.0 stamp gets self-corrected on the very next classic-mode repaint, and vice versa. Verified by deliberately reproducing the exact failure mode (stamp `widget.width = 300` on a 580px-wide node, confirm a normal redraw restores it to 580, confirm a click on the far-right chip then registers correctly) rather than just re-testing the original vague symptom.
|
||||
|
||||
**Lesson for future Nodes 2.0 compat work in this repo:** any widget property that Nodes 2.0's `WidgetLegacy.vue` bridge writes onto the shared widget object (`.width`, `.y`, `.triggerDraw` are the three confirmed so far) must be treated as **owned by whichever renderer painted last**, not as a stable/cached value — always recompute and reassign it from the live `node`/DOM state at the top of every `draw()`, never trust it surviving between renderer switches. When a live-tested bug resists several iterations of hypothesis-and-retest without a clean repro, escalating to a fresh model with full context and explicit "don't trust the existing narrative, re-derive from source" instructions found the answer in one pass where continued iteration likely wouldn't have — worth doing sooner next time a bug proves this sticky, rather than as a last resort.
|
||||
@@ -3,31 +3,103 @@ import { createDividerWidget, createStatusWidget } from "./shared/canvas_widgets
|
||||
import { createPresetPickerWidget } from "./shared/preset_picker_widget.mjs?v=6";
|
||||
import { setWidgetVisible } from "./shared/widget_visibility.mjs?v=2";
|
||||
|
||||
// Approved folders live in ComfyUI's own settings store, so approving one is a
|
||||
// plain settings write from the frontend: no CraftKit route, no helper process.
|
||||
// The id must stay identical to SETTING_KEY in craftkit_folder_guard.py.
|
||||
const SETTING_ID = "CraftKit.AllowedFolders";
|
||||
|
||||
function getSetting(id) {
|
||||
try {
|
||||
if (app.extensionManager?.setting?.get) return app.extensionManager.setting.get(id);
|
||||
} catch (e) { /* older frontend, fall through */ }
|
||||
try {
|
||||
return app.ui?.settings?.getSettingValue?.(id);
|
||||
} catch (e) {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
async function setSetting(id, value) {
|
||||
if (app.extensionManager?.setting?.set) return await app.extensionManager.setting.set(id, value);
|
||||
return await app.ui.settings.setSettingValue(id, value);
|
||||
}
|
||||
|
||||
// ';' is the separator (no path contains one); newlines are accepted for
|
||||
// anyone who pastes a list into the settings field by hand. Mirrors
|
||||
// _split_folders() in craftkit_folder_guard.py.
|
||||
function splitFolders(value) {
|
||||
const parts = Array.isArray(value)
|
||||
? value
|
||||
: String(value ?? "").replace(/\r/g, "\n").replace(/;/g, "\n").split("\n");
|
||||
return parts.map(cleanPath).filter(Boolean);
|
||||
}
|
||||
|
||||
function cleanPath(p) {
|
||||
let s = String(p ?? "").trim().replace(/^["']+|["']+$/g, "").trim();
|
||||
// Strip a trailing separator, but never turn "D:/" into "D:".
|
||||
while (s.length > 3 && (s.endsWith("/") || s.endsWith("\\"))) s = s.slice(0, -1);
|
||||
return s;
|
||||
}
|
||||
|
||||
app.registerExtension({
|
||||
name: "Craftopia.SmartBatchResize",
|
||||
|
||||
settings: [
|
||||
{
|
||||
id: SETTING_ID,
|
||||
category: ["CraftKit", "Folders", "Approved folders"],
|
||||
name: "Approved folders",
|
||||
tooltip:
|
||||
"Folders Smart Batch Resize is allowed to read and write, separated by ';'. " +
|
||||
"Subfolders are included. Use the 'Approve folder' button on the node to add one. " +
|
||||
"ComfyUI's own input/output/temp folders always work without being listed here.",
|
||||
type: "text",
|
||||
defaultValue: "",
|
||||
},
|
||||
],
|
||||
|
||||
async nodeCreated(node) {
|
||||
if (node.comfyClass !== "SmartBatchResize") return;
|
||||
|
||||
const folderWidget = node.widgets?.find(w => w.name === "input_folder");
|
||||
if (!folderWidget) return;
|
||||
|
||||
const btn = node.addWidget("button", "📁 Browse folder", null, async () => {
|
||||
try {
|
||||
const res = await fetch("/craftkit/browse_folder", { method: "POST" });
|
||||
const data = await res.json();
|
||||
if (data.ok && data.path) {
|
||||
folderWidget.value = data.path;
|
||||
const APPROVE_LABEL = "✅ Approve folder";
|
||||
const btn = node.addWidget("button", APPROVE_LABEL, null, async () => {
|
||||
const flash = (label) => {
|
||||
btn.label = label;
|
||||
btn.name = label;
|
||||
node.setDirtyCanvas(true);
|
||||
setTimeout(() => {
|
||||
btn.label = APPROVE_LABEL;
|
||||
btn.name = APPROVE_LABEL;
|
||||
node.setDirtyCanvas(true);
|
||||
}, 1800);
|
||||
};
|
||||
|
||||
const folder = cleanPath(folderWidget.value);
|
||||
if (!folder) {
|
||||
flash("⚠ Paste a path first");
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const existing = splitFolders(getSetting(SETTING_ID));
|
||||
const known = existing.some(f => f.toLowerCase() === folder.toLowerCase());
|
||||
if (known) {
|
||||
flash("✔ Already approved");
|
||||
return;
|
||||
}
|
||||
await setSetting(SETTING_ID, [...existing, folder].join(";"));
|
||||
flash("✔ Folder approved");
|
||||
} catch (e) {
|
||||
console.error("[SmartBatchResize] Browse failed:", e);
|
||||
console.error("[SmartBatchResize] Could not approve folder:", e);
|
||||
flash("⚠ Approve failed (see console)");
|
||||
}
|
||||
}, { serialize: false });
|
||||
|
||||
btn.serialize = false;
|
||||
|
||||
// Move Browse button to right after input_folder (index 1)
|
||||
// Move Approve button to right after input_folder (index 1)
|
||||
const folderIdx = node.widgets.indexOf(folderWidget);
|
||||
const btnIdx = node.widgets.indexOf(btn);
|
||||
if (btnIdx !== folderIdx + 1) {
|
||||
@@ -68,7 +140,7 @@ app.registerExtension({
|
||||
addSectionDivider("skip_if_exists", "OPTIONS");
|
||||
addSectionDivider("output_format", "OUTPUT FORMAT");
|
||||
|
||||
// Override serialize so non-serializable JS widgets (Browse, presets, status,
|
||||
// Override serialize so non-serializable JS widgets (Approve, presets, status,
|
||||
// run batch) are excluded from widgets_values in the saved workflow JSON.
|
||||
// Without this, LiteGraph saves null slots for these widgets which then shift
|
||||
// all Python widget values on load (configure runs before nodeCreated).
|
||||
|
||||
+6
-1
@@ -1,16 +1,21 @@
|
||||
[project]
|
||||
name = "comfyui-craftkit"
|
||||
description = "Smart resize, batch resize, resolution multiplier and prompt controller nodes for ComfyUI"
|
||||
version = "1.1.1"
|
||||
version = "1.1.5"
|
||||
readme = "README.md"
|
||||
requires-python = ">=3.10"
|
||||
license = { file = "LICENSE" }
|
||||
# Pillow, NumPy, and PyTorch all ship with ComfyUI itself — intentionally not
|
||||
# re-declared here to avoid version conflicts with ComfyUI's own pins.
|
||||
dependencies = []
|
||||
keywords = ["comfyui", "resize", "batch-resize", "resolution", "prompt", "upscale", "image-processing", "lora"]
|
||||
classifiers = [
|
||||
"Operating System :: OS Independent",
|
||||
]
|
||||
|
||||
[project.urls]
|
||||
Repository = "https://github.com/CraftopiaStudio/ComfyUI-CraftKit"
|
||||
Issues = "https://github.com/CraftopiaStudio/ComfyUI-CraftKit/issues"
|
||||
|
||||
[tool.comfy]
|
||||
PublisherId = "craftopiastudio"
|
||||
|
||||
@@ -13,6 +13,8 @@ try:
|
||||
except ImportError:
|
||||
ProgressBar = None
|
||||
|
||||
from .craftkit_folder_guard import folder_allowed, denied_message
|
||||
|
||||
|
||||
INTERP_MAP = {
|
||||
"lanczos": PILImage.LANCZOS,
|
||||
@@ -208,7 +210,7 @@ class SmartBatchResize:
|
||||
# are added/removed on its own. Hash the listing so unrelated graph runs
|
||||
# can still hit the cache, while an actual folder change invalidates it.
|
||||
folder = input_folder.strip()
|
||||
if not folder or not os.path.isdir(folder):
|
||||
if not folder or not folder_allowed(folder) or not os.path.isdir(folder):
|
||||
return ""
|
||||
try:
|
||||
files = sorted(
|
||||
@@ -245,7 +247,9 @@ class SmartBatchResize:
|
||||
|
||||
input_folder = input_folder.strip()
|
||||
if not input_folder:
|
||||
raise ValueError("[SmartBatchResize] No folder selected. Enter a path or use the Browse button.")
|
||||
raise ValueError("[SmartBatchResize] No folder selected. Paste a folder path into input_folder.")
|
||||
if not folder_allowed(input_folder):
|
||||
raise ValueError(denied_message(input_folder))
|
||||
if not os.path.isdir(input_folder):
|
||||
raise ValueError(f"[SmartBatchResize] Folder not found: {input_folder}")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user