feat(spec): 002-manager-install plan.md + ADR-003

plan.md covers: - requirements.txt fix (jinja2 only, hand-authored) - aiohttp→production deps in pyproject.toml - @description metadata update - custom-node-list.json PR scope - Docker Compose CPU-only test harness (US4) ADR-003: hand-authored requirements.txt as curated subset of pyproject.toml — cross-cutting policy for all future dep additions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-28 16:17:13 +01:00
parent 6d21a6c58e
commit b3ed126cd3
4 changed files with 273 additions and 3 deletions
@@ -0,0 +1,93 @@
+# ADR-003: Hand-authored requirements.txt as a curated subset of pyproject.toml
+
+## Status
+
+> Accepted
+
+_Date:_ 2026-06-28
+_Deciders:_ darth-veitcher
+
+---
+
+## Context
+
+comfydv has two distribution paths with different dependency environments:
+
+**Path A — git-clone (ComfyUI Manager)**: Manager clones the repo into
+`ComfyUI/custom_nodes/comfydv/` and runs `pip install -r requirements.txt`.
+ComfyUI's own environment already provides `aiohttp` and other packages.
+
+**Path B — pip install**: `pip install comfydv` in a standalone environment
+with no ComfyUI present. All production deps must be declared in
+`pyproject.toml [project.dependencies]`.
+
+The original `requirements.txt` was generated by `uv export --no-editable --no-hashes`
+and contained a bare `.` (local editable install), `colorama==0.4.6`, and
+`termcolor==2.5.0` — all wrong. The actual runtime dep (`jinja2`) was absent.
+Running Manager's post-clone `pip install -r requirements.txt` on this file would fail
+or produce a broken environment.
+
+## Decision
+
+`requirements.txt` is a **hand-authored**, **human-readable** file committed to git.
+It lists only the runtime dependencies that are **not** already provided by ComfyUI's
+own environment. It is a curated subset of `pyproject.toml [project.dependencies]`.
+
+`pyproject.toml [project.dependencies]` remains the **source of truth** for all runtime
+dependencies. It includes deps that ComfyUI already provides (e.g. `aiohttp`) for
+correctness when the package is installed outside a ComfyUI environment.
+
+**Rules**:
+1. `requirements.txt` is never auto-generated (no `uv export`, no Makefile target).
+2. When a new runtime dep is added to `pyproject.toml [project.dependencies]`, the
+   developer must decide: "Is this already provided by ComfyUI?" If no → add to
+   `requirements.txt` too. If yes → leave `requirements.txt` unchanged.
+3. A test in `tests/test_packaging.py` asserts that `requirements.txt` contains no
+   packages absent from `pyproject.toml [project.dependencies]` (no phantom deps).
+
+## Consequences
+
+**Easier:**
+- ComfyUI Manager install succeeds with a clean `pip install -r requirements.txt`.
+- `requirements.txt` is readable and reviewable by humans.
+- The policy is explicit: new contributors know what to do when adding a dep.
+
+**Harder / constrained:**
+- Adding a new runtime dep requires updating two files and making a judgment call about
+  which is ComfyUI-provided. This is a one-minute decision but must not be forgotten.
+- The test in `test_packaging.py` catches only phantom deps (things in `requirements.txt`
+  not in `pyproject.toml`); it does not catch missing deps (things in `pyproject.toml`
+  that should be in `requirements.txt` but aren't), because that requires knowing which
+  packages ComfyUI provides.
+
+**Debt introduced:**
+- None beyond the manual sync obligation above.
+
+## Considered Alternatives
+
+### Alternative A: Auto-generate `requirements.txt` from `pyproject.toml` via `uv export`
+
+**Why rejected:** `uv export` produces locked, pinned versions (`colorama==0.4.6`) —
+the wrong format for a plugin that must coexist with whatever ComfyUI has installed.
+It also includes the local package itself (`.`) and transitive deps of removed packages,
+as demonstrated by the original broken file.
+
+### Alternative B: Remove `requirements.txt` entirely; rely on `pyproject.toml`
+
+**Why rejected:** ComfyUI Manager's git-clone path explicitly looks for and runs
+`pip install -r requirements.txt`. Without it, runtime deps (e.g. `jinja2`) would not
+be installed for git-clone users, and FormatString would fail on first use.
+
+### Alternative C: List all deps (including ComfyUI-provided) in `requirements.txt`
+
+**Why rejected:** If Manager's `pip install -r requirements.txt` installs a different
+version of `aiohttp` than ComfyUI expects, it can silently downgrade ComfyUI's web
+server. The minimal-subset approach avoids this conflict entirely.
+
+---
+
+## Links
+
+- Related spec: `specs/002-manager-compatible-install-requirements-txt-and-custom-node-list-registration/`
+- Parent epic: `project-management/Roadmap/epics/ux-and-install.md`
+- Related ADRs: [ADR-001](ADR-001-stdlib-logging-over-console-libraries.md)
@@ -27,9 +27,7 @@ _None — this epic is self-contained and can start immediately._
 - specs/005-metadata-cleanup-pyproject-description-scripts-dead-code-removal-tooltips-category-consistency/
 ## ADRs

-_To be recorded as cross-cutting decisions are made during DESIGN._
-
-<!-- - project-management/ADRs/ADR-NNN-decision-title.md -->
+- [ADR-003](../../ADRs/ADR-003-requirements-txt-authoring-policy.md) — hand-authored requirements.txt as curated subset of pyproject.toml

 ## Success criteria

@@ -0,0 +1,155 @@
+# Implementation Plan: Manager-Compatible Install
+
+**Branch**: `002-manager-install` | **Date**: 2026-06-28 | **Spec**: [spec.md](./spec.md)
+
+**Input**: Feature specification from `specs/002-manager-compatible-install-requirements-txt-and-custom-node-list-registration/spec.md`
+
+## Summary
+
+Fix comfydv's packaging so ComfyUI Manager can install it correctly: replace the broken
+auto-generated `requirements.txt` with a hand-authored one listing only `jinja2>=3.1.6`,
+promote `aiohttp` from dev-only to production dependencies in `pyproject.toml`, correct the
+stale `@description` metadata in `__init__.py`, and submit a PR to
+`ltdrdata/ComfyUI-Manager` to list the package in Manager's search UI.
+
+## Technical Context
+
+**Language/Version**: Python 3.11 (`pyproject.toml` target), plain text (`requirements.txt`), JSON (`custom-node-list.json` PR)
+
+**Primary Dependencies**:
+- `jinja2>=3.1.6` — sole runtime dep not bundled by ComfyUI
+- `aiohttp>=3.9.0` — provided by ComfyUI's own environment; must be listed in `pyproject.toml [project.dependencies]` for pip-install correctness but **not** in `requirements.txt` (ComfyUI already satisfies it for git-clone users)
+
+**Storage**: N/A — no data storage changes
+
+**Testing**: `uv run pytest` — new test asserting `requirements.txt` correctness and `@description` accuracy
+
+**Target Platform**: Any environment where ComfyUI is installed; CI (GitHub Actions / local)
+
+**Project Type**: ComfyUI custom node plugin; packaging/configuration spec
+
+**Performance Goals**: N/A — install is a one-time operation
+
+**Constraints**: `requirements.txt` must be parseable by `pip install -r`; the ComfyUI Manager PR must follow the exact JSON schema of `custom-node-list.json`
+
+**Scale/Scope**: 3 files changed, 1 external PR opened; no new source files
+
+## Constitution Check
+
+*GATE: Must pass before Phase 0 research. Re-checked after Phase 1 design.*
+
+| Principle | Status | Notes |
+|---|---|---|
+| I. ComfyUI Contract First | ✅ Pass | No changes to node registration or `NODE_CLASS_MAPPINGS` |
+| II. Sandbox All User-Supplied Code | ✅ Pass | N/A — no user-input evaluation changes |
+| III. Test-First (NON-NEGOTIABLE) | ✅ Pass | Tests written before implementation (TDD pairs in tasks.md) |
+| IV. Graceful Degradation Outside ComfyUI | ✅ Pass | Fix improves this — correct deps mean clean import outside ComfyUI |
+| V. Simplicity — Function Before Class | ✅ Pass | Pure configuration changes; no new abstractions |
+| VI. Fixed Output Positions | ✅ Pass | N/A — no output schema changes |
+
+No violations. No Complexity Tracking table needed.
+
+## Project Structure
+
+### Documentation (this feature)
+
+```text
+specs/002-manager-compatible-install-requirements-txt-and-custom-node-list-registration/
+├── spec.md         ✅ done
+├── plan.md         ✅ this file
+├── research.md     ✅ inlined below (no open questions)
+└── tasks.md        ← /beacon:tasks next
+```
+
+### Source Code (repository root)
+
+```text
+requirements.txt          ← rewrite (FR-001, FR-002)
+pyproject.toml            ← move aiohttp to [project.dependencies] (FR-003)
+__init__.py               ← update @description (FR-005)
+tests/test_packaging.py   ← new test file (constitution III)
+```
+
+External artefact (not in this repo):
+```
+ltdrdata/ComfyUI-Manager / custom-node-list.json  ← PR (FR-004)
+```
+
+## Research
+
+### Decision 1: What belongs in `requirements.txt` vs `pyproject.toml`
+
+**Context**: Two separate distribution paths exist: git-clone (ComfyUI Manager) and pip install. They have different dependency environments.
+
+**Decision**: `requirements.txt` lists only deps **not already provided by ComfyUI's own environment**. ComfyUI depends on `aiohttp` itself, so it is always present for git-clone users. Only `jinja2>=3.1.6` goes in `requirements.txt`.
+
+`pyproject.toml [project.dependencies]` lists **all** production runtime deps including `aiohttp>=3.9.0`, for correctness when the package is pip-installed outside a ComfyUI environment.
+
+**Rationale**: Avoids version conflicts from double-installing `aiohttp` in Manager's pip step; keeps `requirements.txt` minimal and correct.
+
+**Alternatives considered**:
+- List `aiohttp` in `requirements.txt` too → redundant, and if versions conflict with ComfyUI's pinned version, Manager install silently downgrades ComfyUI's server
+- Remove `aiohttp` from `pyproject.toml` entirely → correct for git-clone but breaks `pip install comfydv` in standalone environments
+
+### Decision 2: `aiohttp` version constraint
+
+**Decision**: `aiohttp>=3.9.0` — matches the floor ComfyUI itself uses. No upper bound (semver convention for libraries).
+
+**Alternatives considered**: Pinning to `==3.9.x` would prevent upgrades when ComfyUI upgrades its own aiohttp.
+
+### Decision 3: `custom-node-list.json` entry format
+
+**Decision**: Follow the exact schema used by existing entries. Required fields:
+```json
+{
+  "author": "Darth Veitcher",
+  "title": "Comfy DV Nodes",
+  "reference": "https://github.com/darth-veitcher/comfydv",
+  "files": ["https://github.com/darth-veitcher/comfydv"],
+  "install_type": "git-clone",
+  "description": "...",
+  "nodename": ["Format String (Python f-strings)", "Random Choice", "Circuit Breaker"]
+}
+```
+
+**Rationale**: Manager parses this exact structure. `install_type: "git-clone"` is the standard path; `files` contains the repo URL for Manager to clone.
+
+### Decision 4: `@description` replacement text
+
+**Decision**: Replace the stale description with one that names only the three existing nodes and their purpose, removing the "model memory management" / "Model Unloader" reference.
+
+**New text**: `"Quality of life ComfyUI nodes: dynamic string formatting with Python f-strings or Jinja2 templates, seed-controlled random input selection, and workflow circuit-breaker for conditional queue interruption."`
+
+## Contracts
+
+No external API contracts to define — this spec touches only packaging metadata and a
+third-party JSON registry. The `requirements.txt` format is dictated by pip (no new
+contract to author).
+
+## Docker Compose Test Harness
+
+A CPU-only ComfyUI environment is needed to verify SC-001 (deps install cleanly)
+and SC-003 (nodes appear in the menu after install) without a GPU. This is deliverable
+alongside the packaging fixes in this spec.
+
+**Approach**: `docker-compose.yml` at the repo root with a single `comfyui` service:
+- Base image: `python:3.11-slim` (or a community ComfyUI CPU image if one exists and is stable)
+- Startup: clone / install ComfyUI CPU-only, then symlink/copy `comfydv` into
+  `custom_nodes/`, run `pip install -r requirements.txt`, start ComfyUI server
+- No GPU required: `--cpu-only` flag or `torch` CPU wheel
+- Health check: `curl http://localhost:8188/` returns 200 → nodes loaded successfully
+- Used by: developers testing install changes locally; future CI smoke test
+
+**Constraints**: The harness must start in under 3 minutes on a developer laptop
+(no model downloads; only the ComfyUI server + custom node registration). Model weights
+are intentionally excluded — the goal is testing node registration and import, not
+inference.
+
+This is tracked as a dedicated task pair (T050-T / T050-I) in tasks.md.
+
+## ADR Candidates
+
+The following decision is cross-cutting (applies to all future specs, not just this one)
+and warrants an ADR in the parent epic:
+
+- **`requirements.txt` authoring policy**: hand-authored subset of `pyproject.toml` deps excluding ComfyUI-provided packages. This policy applies to every future PR that adds a runtime dependency. Recommend adding as `ADR-003` before closing this spec.
@@ -72,6 +72,30 @@ nodes.

 ---

+### User Story 4 — Local test harness for install verification (Priority: P2)
+
+A developer working on packaging or node registration can spin up a CPU-only ComfyUI
+environment locally in one command to verify that dependencies install cleanly and nodes
+appear in the menu — without needing a GPU or a full ComfyUI installation on their
+machine.
+
+**Why this priority**: Without a local harness, SC-001 and SC-003 can only be verified
+by manually installing ComfyUI on a development machine, which is slow and environment-
+dependent. A Compose harness makes these criteria continuously verifiable and sets the
+foundation for future CI smoke tests.
+
+**Independent Test**: `docker compose up --build` from the repo root starts a ComfyUI
+server, installs comfydv via `requirements.txt`, and the server health endpoint returns
+200 with no import errors in the logs.
+
+**Acceptance Scenarios**:
+
+1. **Given** a machine with Docker and no GPU, **when** `docker compose up --build` is run from the repo root, **then** the ComfyUI server starts and `curl http://localhost:8188/` returns 200.
+2. **Given** the running container, **when** the ComfyUI startup logs are inspected, **then** all three comfydv nodes (FormatString, RandomChoice, CircuitBreaker) appear as loaded with no import errors.
+3. **Given** the running container, **when** `docker compose down` is run, **then** it exits cleanly with no lingering processes.
+
+---
+
 ### Edge Cases

 - What if `jinja2` is already installed? `pip install -r requirements.txt` is idempotent — it satisfies the constraint without downgrading or reinstalling.