From 8318b3da0b2275bd10625824b3a77dfd9f695f81 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:56:17 +0300 Subject: [PATCH] core, app: install, update and remove node packs Third off the port list. Users will want to add the one pack a workflow needs without being sent to another UI to do it, and a pack that ships experimental work needs the same for itself. custom_nodes.py comes across as core/nodes_manager.py, changed in two places: where FunPack's root is, and one guard that the port made necessary. THE GUARD. custom_nodes_root() asks ComfyUI first and falls back to "FunPack lives inside custom_nodes" -- a reliable anchor in an install, and a wrong one in a development checkout, where the parent is somebody's home directory. Every rule protecting the delete measures against that answer, so with ComfyUI's paths unavailable, "remove Foo" resolved against ~/Documents and all four guards happily agreed. It now checks the assumption it is named after and refuses with where it thinks it is. Found by running the function, not by reading it. Two faults of my own, both in the window: * Update and Remove were a button GROUP -- radios -- for two actions that are not a choice between two states. It also left Remove looking selected after a click, which is an alarming thing to look at. * A failure was drawn and then wiped by the redraw in the same finally block: the message appeared and vanished in one frame. What went wrong is state now, not an argument to one draw call. --- app/boot.js | 2 + app/shell/layout.js | 1 + app/shell/menubar.js | 4 +- app/shell/packs.js | 158 +++++++++++++++++ app/tests-layout/packs.spec.js | 98 +++++++++++ core/nodes_manager.py | 289 +++++++++++++++++++++++++++++++ core/routes.py | 46 ++++- core/tests/test_routes_update.py | 57 ++++++ 8 files changed, 653 insertions(+), 2 deletions(-) create mode 100644 app/shell/packs.js create mode 100644 app/tests-layout/packs.spec.js create mode 100644 core/nodes_manager.py diff --git a/app/boot.js b/app/boot.js index c61a1a72..5184b68e 100644 --- a/app/boot.js +++ b/app/boot.js @@ -22,6 +22,7 @@ import { createWheel } from "./shell/wheel.js"; import { offerAction } from "./shell/actions.js"; import { open as openWizard } from "./shell/wizard.js"; import { open as openUpdates } from "./shell/updates.js"; +import { open as openPacks } from "./shell/packs.js"; const root = document.querySelector("#app"); @@ -108,6 +109,7 @@ async function start() { onCancel: () => run.cancel(), onConstructor: () => page.constructor.open(), onUpdates: () => openUpdates(), + onPacks: () => openPacks(), // What the Edit menu offers, and what the keyboard reaches. One list, so a // menu item and its shortcut cannot drift apart. edits: { diff --git a/app/shell/layout.js b/app/shell/layout.js index de75b2dc..31377e31 100644 --- a/app/shell/layout.js +++ b/app/shell/layout.js @@ -188,6 +188,7 @@ export function build(root, handlers = {}) { workspace, onPipeline: handlers.onPipeline, onUpdates: handlers.onUpdates, + onPacks: handlers.onPacks, onProject: handlers.onProject, edits: handlers.edits, projects: handlers.projects, diff --git a/app/shell/menubar.js b/app/shell/menubar.js index 23a2b225..2f515798 100644 --- a/app/shell/menubar.js +++ b/app/shell/menubar.js @@ -70,7 +70,7 @@ function menu(label, itemsOf, onPick) { } export function createMenubar({ workspace, onPipeline, onProject, projects = () => [], - current = () => null, edits = null, onUpdates, + current = () => null, edits = null, onUpdates, onPacks, theme = services.theme } = {}) { const connection = createConnection(); @@ -119,9 +119,11 @@ export function createMenubar({ workspace, onPipeline, onProject, projects = () { id: "pipeline", label: "Models and pipeline…" }, { separator: true }, { id: "updates", label: "Updates…" }, + { id: "packs", label: "Node packs…" }, ], (id) => { if (id === "pipeline" && onPipeline) onPipeline(); else if (id === "updates" && onUpdates) onUpdates(); + else if (id === "packs" && onPacks) onPacks(); }); const bar = composer.toolbar.default({ diff --git a/app/shell/packs.js b/app/shell/packs.js new file mode 100644 index 00000000..8e918001 --- /dev/null +++ b/app/shell/packs.js @@ -0,0 +1,158 @@ +// Node packs: install, update, remove. +// +// A stand-in for ComfyUI-Manager's three operations, so somebody adding the one +// pack a workflow needs is not sent to another UI to do it. There is no +// catalogue and no curation -- the URL is theirs -- which keeps this honest +// about what it is: git, in a directory. +// +// Everything here can take minutes. Nothing pretends otherwise: the row that is +// working says so, and the rest of the list stays usable. + +import { composer } from "../composer/composer.js"; + +const BASE = "/funpack/api/packs"; + +async function ask(method, path = "", body) { + const res = await fetch(`${BASE}${path}`, { + method, + headers: body === undefined ? {} : { "Content-Type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + const payload = await res.json().catch(() => ({})); + if (!res.ok) throw new Error(payload.detail || `HTTP ${res.status}`); + return payload; +} + +/** What a pack's git state says, in words rather than two numbers. */ +function saySince(pack, checked) { + if (!pack.git) return "not a git checkout — update and remove are not offered"; + const news = checked && checked[pack.name]; + if (!news) return `${pack.branch || "?"} · ${pack.commit || ""}`.trim(); + if (!news.checked) return `${pack.branch || "?"} · could not check: ${news.reason}`; + if (news.behind) return `${news.behind} update${news.behind === 1 ? "" : "s"} available`; + return `${pack.branch || "?"} · up to date`; +} + +export function open() { + let window_ = null; + let listing = null; + let checked = null; + let busy = null; // the pack currently being worked on, by name + // What went wrong, held rather than passed: every redraw would otherwise wipe + // it, and the one after a failure arrives immediately -- the message appeared + // and vanished in the same frame. + let problem = null; + + const body = composer.region.stack({ gap: "sm", label: "Node packs" }); + + const run = async (name, work) => { + busy = name; + problem = null; + draw(); + try { + await work(); + await load(); + } catch (err) { + problem = err.message; + } finally { + busy = null; + draw(); + } + }; + + function rows() { + const out = []; + if (problem) out.push(composer.banner.danger({ text: problem })); + + const url = composer.input.md({ label: "Git URL", + placeholder: "https://github.com/owner/pack" }); + out.push(composer.settingsRow.default({ + label: "Install a pack", + hint: "Anything git can clone. ComfyUI restarts itself afterwards if it must.", + control: composer.button.md({ + label: busy === "*" ? "Installing…" : "Install", + disabled: Boolean(busy), + onClick: () => { + const value = String(url.value || "").trim(); + // Said here rather than sent: an empty URL is not a server's problem. + if (!value) { problem = "Paste a git URL first."; draw(); return; } + run("*", () => ask("POST", "/install", { url: value })); + }, + }), + })); + out.push(composer.field.default({ label: "Git URL", control: url })); + + if (!listing) { + out.push(composer.hint.default({ text: "Reading custom_nodes…" })); + return out; + } + + out.push(composer.label.section({ text: `${listing.nodes.length} installed` })); + for (const pack of listing.nodes) { + const working = busy === pack.name; + out.push(composer.settingsRow.default({ + label: pack.name + (pack.is_funpack ? " — this is FunPack" : ""), + hint: working ? "Working…" : saySince(pack, checked), + // Buttons, not a button GROUP: these are two actions, not a choice + // between two states. A radio group also leaves one of them looking + // chosen afterwards, which for "Remove" is an alarming thing to look at. + control: composer.toolbar.default({ + label: pack.name, + items: [ + ...(pack.git && !pack.is_funpack ? [composer.button.sm({ + label: "Update", disabled: Boolean(busy), + onClick: () => run(pack.name, () => ask("POST", "/update", { name: pack.name })), + })] : []), + ...(pack.is_funpack ? [] : [composer.button.sm({ + label: "Remove", tone: "danger", disabled: Boolean(busy), + onClick: () => run(pack.name, () => ask("POST", "/remove", { name: pack.name })), + })]), + ], + }), + })); + } + return out; + } + + function draw() { + body.set(rows()); + if (window_) { + window_.setFooter({ + note: listing ? listing.root : "", + actions: [ + composer.button.md({ label: "Check for updates", disabled: Boolean(busy), + onClick: () => check() }), + composer.button.md({ label: "Close", tone: "primary", + onClick: () => window_.close("done") }), + ], + }); + } + } + + async function load() { + try { listing = await ask("GET"); } + catch (err) { listing = null; problem = err.message; } + draw(); + } + + async function check() { + // Fetching every pack is network-bound and slow; the button says what it is + // doing rather than looking broken for twenty seconds. + busy = "*check*"; + draw(); + problem = null; + try { checked = (await ask("POST", "/check")).nodes || null; } + catch (err) { problem = err.message; } + finally { busy = null; draw(); } + } + + window_ = composer.modal.generic({ + title: "Node packs", + subtitle: "Install, update and remove ComfyUI custom nodes.", + size: "lg", + body, + onClose: () => { window_ = null; }, + }); + load(); + return window_; +} diff --git a/app/tests-layout/packs.spec.js b/app/tests-layout/packs.spec.js new file mode 100644 index 00000000..0d167e72 --- /dev/null +++ b/app/tests-layout/packs.spec.js @@ -0,0 +1,98 @@ +// Node packs, against the real routes. +// +// The dev server puts ComfyUI on the path, so the listing is the real +// custom_nodes directory. Nothing here installs, updates or removes anything: +// those change the machine the tests run on, so each is stubbed at the fetch, +// and the stub is what proves the right request went out. + +import { test, expect } from "@playwright/test"; + +const openPacks = async (page) => { + await page.getByRole("button", { name: "Settings" }).click(); + await page.getByRole("menuitem", { name: /Node packs/ }).click(); + await expect(page.locator(".cx-modal")).toBeVisible(); +}; + +test("the list is the real custom_nodes directory, and says so", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await openPacks(page); + + const real = await page.evaluate(async () => { + const r = await fetch("/funpack/api/packs"); + return r.json(); + }); + await expect(page.locator(".cx-modal-foot")).toContainText(real.root); + await expect(page.locator(".cx-modal")).toContainText(`${real.nodes.length} installed`); +}); + +test("FunPack itself is listed without a way to delete it", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await page.route("**/api/packs", (route) => route.fulfill({ + json: { root: "/x/custom_nodes", nodes: [ + { name: "ComfyUI-FunPack", is_funpack: true, git: true, branch: "dev", commit: "abc1234" }, + { name: "ComfyUI-Other", is_funpack: false, git: true, branch: "main", commit: "def5678" }, + ] }, + })); + await openPacks(page); + + const mine = page.locator(".cx-settings-row", { hasText: "ComfyUI-FunPack" }); + await expect(mine).toContainText("this is FunPack"); + await expect(mine.getByRole("button", { name: "Remove" })).toHaveCount(0); + await expect(page.locator(".cx-settings-row", { hasText: "ComfyUI-Other" }) + .getByRole("button", { name: "Remove" })).toHaveCount(1); +}); + +test("a pack that is not a git checkout is not offered an update", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await page.route("**/api/packs", (route) => route.fulfill({ + json: { root: "/x/custom_nodes", nodes: [{ name: "Hand-Copied", is_funpack: false, git: false }] }, + })); + await openPacks(page); + + const row = page.locator(".cx-settings-row", { hasText: "Hand-Copied" }); + await expect(row).toContainText("not a git checkout"); + await expect(row.getByRole("button", { name: "Update" })).toHaveCount(0); +}); + +test("installing sends the URL that was typed, and nothing when it is empty", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await page.route("**/api/packs", (route) => route.fulfill({ + json: { root: "/x/custom_nodes", nodes: [] }, + })); + const sent = []; + await page.route("**/api/packs/install", async (route) => { + sent.push(route.request().postDataJSON()); + return route.fulfill({ json: { name: "pack" } }); + }); + await openPacks(page); + + // Empty: refused here, not sent to a server to be refused there. + await page.getByRole("button", { name: "Install" }).click(); + await expect(page.locator(".cx-modal")).toContainText("Paste a git URL first."); + expect(sent).toEqual([]); + + await page.locator(".cx-modal input[type='text']").fill("https://github.com/o/p"); + await page.getByRole("button", { name: "Install" }).click(); + await expect.poll(() => sent).toEqual([{ url: "https://github.com/o/p" }]); +}); + +test("a refusal from the server is shown in the window", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await page.route("**/api/packs", (route) => route.fulfill({ + json: { root: "/x/custom_nodes", nodes: [ + { name: "ComfyUI-Other", is_funpack: false, git: true, branch: "main" }] }, + })); + await page.route("**/api/packs/remove", (route) => route.fulfill({ + status: 400, json: { detail: "That is FunPack itself." }, + })); + await openPacks(page); + + await page.locator(".cx-settings-row", { hasText: "ComfyUI-Other" }) + .getByRole("button", { name: "Remove" }).click(); + await expect(page.locator(".cx-modal")).toContainText("That is FunPack itself."); +}); diff --git a/core/nodes_manager.py b/core/nodes_manager.py new file mode 100644 index 00000000..3cfa1f68 --- /dev/null +++ b/core/nodes_manager.py @@ -0,0 +1,289 @@ +"""Install, update and remove ComfyUI custom node packs. + +A small stand-in for ComfyUI-Manager's three operations, so FunPack users are not sent to +another UI to add the one pack a workflow needs: clone from a git URL, pull, delete. + +There is no registry and no curation here — the user supplies the URL. That keeps this +honest about what it is (git, in a directory) rather than implying a vetted catalogue. + +Carried across from v4 with one change -- where FunPack's root is -- because the +delete is the whole risk in it and every rule below was written against a real +way to get it wrong. + +**The delete is the whole risk in this file.** Everything that resolves a pack name goes +through `node_dir`, which refuses anything that is not a direct child directory of +`custom_nodes` — resolved, so a symlink cannot point the deletion somewhere else — and +refuses FunPack itself. Nothing here interpolates into a shell. +""" +from __future__ import annotations + +import re +import shutil +import subprocess +import sys +from pathlib import Path + +FUNPACK_ROOT = Path(__file__).resolve().parents[1] + + +class CustomNodeError(RuntimeError): + pass + + +def custom_nodes_root() -> Path: + """The custom_nodes directory, or a refusal saying why it cannot be found. + + ComfyUI's own paths are asked first. The fallback -- "FunPack lives inside + custom_nodes" -- is a reliable anchor in an INSTALL and a wrong one in a + development checkout, where the parent is somebody's home directory. v4 + could rely on it because v4 only ever ran from inside custom_nodes. + + So the fallback checks the assumption it is named after. Without that, a + delete in a worktree resolves against ~/Documents and every guard below + happily agrees, because they all measure against THIS answer. + """ + try: + import folder_paths + base = Path(folder_paths.base_path) / "custom_nodes" + if base.is_dir(): + return base.resolve() + except Exception: # noqa: BLE001 + pass + parent = FUNPACK_ROOT.parent.resolve() + if parent.name != "custom_nodes": + raise CustomNodeError( + f"Cannot tell where node packs live: ComfyUI's paths are not available and " + f"FunPack is at {FUNPACK_ROOT}, which is not inside a custom_nodes directory. " + f"Start FunPack from a ComfyUI install to manage node packs.") + return parent + + +def node_dir(name: str) -> Path: + """Resolve a pack name to its directory, or refuse. + + Every rule here exists to keep a delete inside custom_nodes: + * one path segment, so "../.." cannot climb out; + * resolved, so a symlink cannot redirect it; + * the resolved parent must BE custom_nodes, not merely start with it; + * never FunPack itself, which would delete the code doing the deleting. + """ + raw = str(name or "").strip() + if not raw or raw in (".", ".."): + raise CustomNodeError("A node pack name is required.") + if "/" in raw or "\\" in raw or raw.startswith("."): + raise CustomNodeError(f"{raw!r} is not a node pack name.") + root = custom_nodes_root() + path = (root / raw) + if not path.exists(): + raise CustomNodeError(f"No custom node named {raw!r} is installed.") + resolved = path.resolve() + if resolved.parent != root: + raise CustomNodeError( + f"{raw!r} does not resolve to a directory inside custom_nodes — refusing to touch it.") + if not resolved.is_dir(): + raise CustomNodeError(f"{raw!r} is not a directory.") + if resolved == FUNPACK_ROOT: + raise CustomNodeError( + "That is FunPack itself. Use Settings ▸ Update FunPack, or remove it by hand.") + return resolved + + +# ── git helpers ─────────────────────────────────────────────────────────────── + +def _git(cwd: Path, *args: str, timeout: int = 300) -> subprocess.CompletedProcess[str]: + if not shutil.which("git"): + raise CustomNodeError("git not found on PATH.") + return subprocess.run(["git", *args], cwd=str(cwd), capture_output=True, + text=True, timeout=timeout) + + +def _git_info(path: Path) -> dict: + if not (path / ".git").exists(): + return {"git": False, "branch": "", "commit": "", "remote": ""} + def one(*args): + p = _git(path, *args, timeout=30) + return (p.stdout or "").strip() if p.returncode == 0 else "" + return { + "git": True, + "branch": one("rev-parse", "--abbrev-ref", "HEAD"), + "commit": one("rev-parse", "--short", "HEAD"), + "remote": one("config", "--get", "remote.origin.url"), + } + + +# ── listing ─────────────────────────────────────────────────────────────────── + +def _is_pack(p: Path) -> bool: + if not p.is_dir() or p.name.startswith(".") or p.name == "__pycache__": + return False + # ComfyUI's own disabled-pack convention, and its example directory. + return not p.name.endswith(".disabled") + + +def list_nodes() -> dict: + root = custom_nodes_root() + out = [] + if root.is_dir(): + for entry in sorted(root.iterdir(), key=lambda e: e.name.lower()): + if not _is_pack(entry): + continue + info = {"name": entry.name, "is_funpack": entry.resolve() == FUNPACK_ROOT} + info.update(_git_info(entry)) + out.append(info) + return {"root": str(root), "nodes": out} + + +# ── install ─────────────────────────────────────────────────────────────────── + +# Deliberately narrow: an https/ssh git URL, nothing that could name a local path. Cloning +# is not a shell call, so this is not injection defence — it is refusing to fetch code from +# somewhere the user did not mean. +_URL_RE = re.compile(r"^(https://|git@)[A-Za-z0-9._~:/?#\[\]@!$&'()*+,;=%-]+$") + + +def repo_name(url: str) -> str: + """Directory a clone of `url` would land in — git's own rule: last segment, no .git.""" + tail = str(url or "").rstrip("/").rsplit("/", 1)[-1] + if tail.endswith(".git"): + tail = tail[:-4] + return tail.strip() + + +def install(url: str) -> dict: + """Clone a pack into custom_nodes, then install its requirements if it has any.""" + url = str(url or "").strip() + if not _URL_RE.match(url): + raise CustomNodeError( + "Enter a git URL, e.g. https://github.com/owner/repo (https:// or git@ only).") + name = repo_name(url) + if not name or "/" in name or "\\" in name or name.startswith("."): + raise CustomNodeError(f"Could not work out a directory name from {url!r}.") + root = custom_nodes_root() + target = root / name + if target.exists(): + raise CustomNodeError( + f"{name!r} is already in custom_nodes. Update it instead, or remove it first.") + proc = _git(root, "clone", "--depth", "1", url, name, timeout=900) + if proc.returncode != 0: + raise CustomNodeError((proc.stderr or proc.stdout or "git clone failed").strip()[-800:]) + reqs = install_requirements(target) + return {"name": name, "requirements": reqs, + "detail": (proc.stderr or proc.stdout or "").strip()[-400:]} + + +def install_requirements(path: Path, timeout: int = 900) -> dict | None: + """`pip install -r requirements.txt` for one pack, into ComfyUI's interpreter. + + None when the pack declares none. Never raises: the pack is on disk either way, and a + failure the user can act on beats an exception that hides what already happened. + """ + req = path / "requirements.txt" + if not req.is_file(): + return None + cmd = [sys.executable, "-m", "pip", "install", "--disable-pip-version-check", + "-r", str(req)] + try: + proc = subprocess.run(cmd, cwd=str(path), capture_output=True, text=True, + timeout=timeout) + except subprocess.TimeoutExpired: + return {"ran": True, "ok": False, + "detail": f"pip did not finish within {timeout}s — run it yourself: " + f"{sys.executable} -m pip install -r {req}"} + except OSError as e: + return {"ran": True, "ok": False, "detail": f"could not run pip: {e}"} + if proc.returncode != 0: + tail = ((proc.stderr or proc.stdout or "").strip() or "pip failed")[-800:] + return {"ran": True, "ok": False, + "detail": f"pip install failed — run it yourself:\n" + f" {sys.executable} -m pip install -r {req}\n\n{tail}"} + return {"ran": True, "ok": True, "detail": (proc.stdout or "").strip()[-400:]} + + +# ── update / remove ─────────────────────────────────────────────────────────── + +def update(name: str) -> dict: + path = node_dir(name) + if not (path / ".git").exists(): + raise CustomNodeError(f"{name!r} is not a git checkout — nothing to pull.") + dirty = _git(path, "status", "--porcelain", timeout=60) + if (dirty.stdout or "").strip(): + raise CustomNodeError( + f"{name!r} has local changes. Commit or discard them before updating.") + before = _git_info(path).get("commit", "") + proc = _git(path, "pull", "--ff-only", timeout=600) + if proc.returncode != 0: + raise CustomNodeError((proc.stderr or proc.stdout or "git pull failed").strip()[-800:]) + after = _git_info(path).get("commit", "") + reqs = install_requirements(path) if before != after else None + return {"name": name, "before": before, "after": after, "updated": before != after, + "requirements": reqs, "detail": (proc.stdout or "").strip()[-400:]} + + +def remove(name: str) -> dict: + """Delete a pack's directory. Irreversible, so `node_dir` is the whole safety story.""" + path = node_dir(name) + shutil.rmtree(path) + return {"name": name, "removed": True, "path": str(path)} + + +# ── checking for updates ────────────────────────────────────────────────────── + +def _behind_ahead(path: Path) -> dict: + """How far one pack is from its upstream, after fetching it. + + Costs a network round trip per pack, which is why it is a button rather than part of + the listing: on a tunnelled instance with a dozen packs, doing this on open would make + the panel feel broken. + """ + if not (path / ".git").exists(): + return {"checked": False, "reason": "not a git checkout"} + br = _git(path, "rev-parse", "--abbrev-ref", "HEAD", timeout=30) + branch = (br.stdout or "").strip() + if br.returncode != 0 or not branch or branch == "HEAD": + return {"checked": False, "reason": "not on a branch (detached HEAD)"} + remote = _git(path, "config", "--get", "remote.origin.url", timeout=30) + if remote.returncode != 0 or not (remote.stdout or "").strip(): + return {"checked": False, "reason": "no origin remote"} + fetched = _git(path, "fetch", "--quiet", "origin", branch, timeout=180) + if fetched.returncode != 0: + return {"checked": False, + "reason": (fetched.stderr or "could not reach origin").strip()[-200:]} + counts = _git(path, "rev-list", "--left-right", "--count", + f"HEAD...origin/{branch}", timeout=60) + if counts.returncode != 0: + return {"checked": False, "reason": "no upstream branch to compare against"} + parts = (counts.stdout or "").split() + if len(parts) != 2: + return {"checked": False, "reason": "could not compare with origin"} + ahead, behind = int(parts[0]), int(parts[1]) + return {"checked": True, "branch": branch, "ahead": ahead, "behind": behind} + + +def check_updates() -> dict: + """Fetch every git pack and report how far behind each one is. + + Fetches run concurrently and bounded: they are network-bound, so serial would be as slow + as the sum of them, and unbounded would open a connection per pack at once. + """ + from concurrent.futures import ThreadPoolExecutor + + packs = [n for n in list_nodes()["nodes"]] + targets = [p for p in packs if p.get("git")] + results: dict[str, dict] = {} + if targets: + with ThreadPoolExecutor(max_workers=4) as pool: + futures = {} + for p in targets: + try: + futures[pool.submit(_behind_ahead, node_dir(p["name"]))] = p["name"] + except CustomNodeError as e: + results[p["name"]] = {"checked": False, "reason": str(e)} + for fut, name in futures.items(): + try: + results[name] = fut.result() + except Exception as e: # noqa: BLE001 — one bad pack must not sink the sweep + results[name] = {"checked": False, "reason": str(e)[-200:]} + for p in packs: + if p["name"] not in results: + results[p["name"]] = {"checked": False, "reason": "not a git checkout"} + return {"checked": results} diff --git a/core/routes.py b/core/routes.py index 7bcd19ea..e639f696 100644 --- a/core/routes.py +++ b/core/routes.py @@ -7,7 +7,8 @@ thin adapters over pure functions in `serve`. import json -from . import (config, graph as graph_mod, log, projects, update as update_mod, +from . import (config, graph as graph_mod, log, nodes_manager, projects, + update as update_mod, registry as registry_mod, serve as static, widgets) from .contract import CONTRACT_VERSION from .relations import order @@ -295,6 +296,49 @@ def register(routes, prefix=None): async def _git_rollback(_req): return await _git(update_mod.rollback) + # --- node packs ----------------------------------------------------------- + # + # A stand-in for ComfyUI-Manager's three operations, so a user is not sent to + # another UI to add the one pack a workflow needs. There is no catalogue: the + # URL is theirs, which keeps this honest about being git in a directory. + # + # Every one of these can take minutes (clone, pip), so they run off the loop. + + async def _pack(action, *args): + import asyncio + try: + return web.json_response(await asyncio.to_thread(action, *args)) + except nodes_manager.CustomNodeError as exc: + # Refusals name the pack and what to do: not a name, not installed, + # not inside custom_nodes, or FunPack itself. + return web.json_response({"detail": str(exc)}, status=400) + except Exception as exc: # noqa: BLE001 + log.broke("node packs", exc, doing=getattr(action, "__name__", "pack")) + return web.json_response({"detail": f"{type(exc).__name__}: {exc}"}, status=500) + + @routes.get(P + "/api/packs") + async def _packs(_req): + return await _pack(nodes_manager.list_nodes) + + @routes.post(P + "/api/packs/check") + async def _packs_check(_req): + return await _pack(nodes_manager.check_updates) + + @routes.post(P + "/api/packs/install") + async def _packs_install(req): + body = await req.json() if req.can_read_body else {} + return await _pack(nodes_manager.install, str((body or {}).get("url") or "")) + + @routes.post(P + "/api/packs/update") + async def _packs_update(req): + body = await req.json() if req.can_read_body else {} + return await _pack(nodes_manager.update, str((body or {}).get("name") or "")) + + @routes.post(P + "/api/packs/remove") + async def _packs_remove(req): + body = await req.json() if req.can_read_body else {} + return await _pack(nodes_manager.remove, str((body or {}).get("name") or "")) + @routes.get(P + "/api/nodes") async def _nodes(req): """What the nodes in a pipeline look like to someone editing them. diff --git a/core/tests/test_routes_update.py b/core/tests/test_routes_update.py index 0afa4a4b..aa57d246 100644 --- a/core/tests/test_routes_update.py +++ b/core/tests/test_routes_update.py @@ -162,3 +162,60 @@ def test_a_remote_that_cannot_be_reached_is_said_rather_than_guessed(server, mon assert body["fetch_ok"] is False assert body["checked_remote"] is True assert body["behind"] == 0, "a failed fetch was read as 'up to date'" + + +# --- node packs -------------------------------------------------------------- + +def test_the_pack_list_says_where_it_is_looking(server): + status, body = _request(server["port"], "GET", "/funpack/api/packs") + assert status == 200, body + assert body["root"].endswith("custom_nodes"), body["root"] + assert isinstance(body["nodes"], list) + + +def test_removing_something_that_is_not_a_pack_name_is_refused(server): + """The delete is the whole risk here: one segment, resolved, inside + custom_nodes, and never FunPack itself.""" + for name in ["..", "../..", "a/b", ".git", ""]: + status, body = _request(server["port"], "POST", "/funpack/api/packs/remove", + {"name": name}) + assert status == 400, (name, status, body) + assert body["detail"], name + + +def test_removing_funpack_itself_is_refused_by_name(server): + from core import nodes_manager + me = nodes_manager.FUNPACK_ROOT.name + status, body = _request(server["port"], "POST", "/funpack/api/packs/remove", {"name": me}) + assert status == 400 + # It has to say WHICH thing it refused to delete, or the user tries again. + assert "FunPack" in body["detail"] + + +def test_a_pack_that_is_not_installed_is_refused_rather_than_created(server): + status, body = _request(server["port"], "POST", "/funpack/api/packs/update", + {"name": "ComfyUI-DoesNotExist"}) + assert status == 400 + assert "ComfyUI-DoesNotExist" in body["detail"] + + +def test_installing_from_nothing_is_refused(server): + status, body = _request(server["port"], "POST", "/funpack/api/packs/install", {"url": ""}) + assert status == 400 + assert body["detail"] + + +def test_an_install_outside_comfyui_says_it_cannot_tell_where_packs_live(server, monkeypatch): + """A development checkout is not inside custom_nodes. v4 could assume it was, + because v4 only ever ran from an install; here the assumption is checked and + the answer is a refusal rather than somebody's home directory.""" + from core import nodes_manager + import sys as _sys + monkeypatch.setitem(_sys.modules, "folder_paths", None) + monkeypatch.setattr(nodes_manager, "FUNPACK_ROOT", + nodes_manager.Path("/tmp/somewhere/ComfyUI-FunPack-v5")) + + status, body = _request(server["port"], "GET", "/funpack/api/packs") + assert status == 400, body + assert "custom_nodes" in body["detail"] + assert "/tmp/somewhere" in body["detail"], "it did not say where it thinks it is"