From afe539e8dceeb121887ab8bfe43ce82bc9ac558a Mon Sep 17 00:00:00 2001 From: malkuthro <166633396+malkuthro@users.noreply.github.com> Date: Wed, 6 May 2026 15:07:14 +0200 Subject: [PATCH] chore(dev-sync): brief output + build-id stamp + chat-report convention (#95) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(dev-sync): brief output + build-id stamp + chat-report convention After a dev-sync the maintainer was running 4 parallel ComfyUI installs with no way to tell which version was just synced and a 50-word "what's new" message that was hard to skim. Two fixes, one PR. Script (`scripts/sync_to_dev.py`): • Default output is now ONE line — the per-file "copied: " lines move behind `-v` / `--verbose`. Useful when something goes wrong; not useful for the routine "did it work" signal. • New summary line includes a build identifier: synced 10 entries @ v -> Both the SHA (via `git rev-parse --short HEAD`) and version (parsed from pyproject.toml) are best-effort — either can be missing and the line still reads cleanly. • Dry-run picks up the same shape: would sync 10 entries @ v -> CLAUDE.md (project instructions): • Tightened the `dev-sync` section to its essentials. • Added a "Chat report format" subsection — the agent must include the build line from script output and a ≤10-word summary naming what to LOOK AT after restart, not a list of changes: dev-sync @ v — <≤10 word summary>. Restart. • Concrete examples included so the convention is unambiguous. No runtime behavior change — pure tooling + docs. Co-Authored-By: Claude Opus 4.7 (1M context) * chore(dev-sync): two-line report format — sha + worktree, then summary Maintainer feedback: the previous format ("dev-sync @ v — . Restart.") cluttered slot 1 with version info and forced the summary onto the same line. Both made the message harder to scan when juggling multiple parallel ComfyUI installs. New format — two lines, build id alone on line 1: - <≤10 word summary of what to look at> Worktree name disambiguates "which checkout fed this sync" when the maintainer is running several in parallel — the version field rarely moves between syncs so it bought less than the worktree name does. Script changes: • Replaced `_build_id()` with `_build_line()` returning the pre-formatted `` - `` header. • Replaced `_get_version()` with `_get_worktree_name()` (REPO_ROOT basename — works for both worktrees and the main checkout). • Output now prints the header line first, then the mechanical "synced N entries -> " line, then the restart hint. Eye lands on the identifier before the details. CLAUDE.md change: matching update to the chat-report convention, including two examples. Co-Authored-By: Claude Opus 4.7 (1M context) * chore(dev-sync): drop "restart ComfyUI" line + user-initiated-only policy Maintainer feedback: • The "restart ComfyUI to pick up changes" line at the end of the script output is noise — they know to restart, the reminder clutters the message. Removed. • Dev-sync was being run by sessions automatically (post-commit, on ship-pr, etc.), silently overwriting code that other parallel sessions were reviewing. Now explicitly user-initiated only: project CLAUDE.md spells out the policy and lists the automated flows that must NOT trigger it; the script docstring carries the same warning so anything that reads it sees the rule. Chat report convention: line 2 is now described as "scope of what to review" (the change in this build) rather than "thing to look at after restart", to match how the maintainer actually thinks about it. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- CLAUDE.md | 62 ++++++++++++++++++++++--------- scripts/sync_to_dev.py | 84 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 121 insertions(+), 25 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f0dfe3b..e896285 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,28 +37,54 @@ ## `dev-sync` — copy runtime files into a live ComfyUI install -For fast iteration on a fix that doesn't deserve its own version bump, -copy runtime files straight into a live ComfyUI custom_nodes folder -instead of cutting a release. +Fast iteration without cutting a release. Copies runtime files to +`KOLOOK_COMFYUI_DEV_PATH` (set in `.env`; see `.env.example`). -- Set `KOLOOK_COMFYUI_DEV_PATH` in `.env` (see `.env.example`). The path - must point at the maintainer's `custom_nodes//`. It is - intentionally kept out of the committed tree. -- Run `python scripts/sync_to_dev.py` (or `--dry-run` to preview). -- Restart ComfyUI to load the new code. +- `python scripts/sync_to_dev.py` — brief two-line summary; use `-v` for per-file output +- `--dry-run` — preview without touching the target -**Trigger phrase: `dev-sync`** (or any clear synonym like "copy those -files", "sync dev", "push to dev"). When the maintainer says it during -a session, the agent runs `scripts/sync_to_dev.py` and reports what was -copied. The script errors out cleanly when the env var is unset; never -guess a path. +**Trigger phrase: `dev-sync`** (or "copy those files", "sync dev", "push to dev"). +The agent runs the script and reports completion. The script errors +cleanly when the env var is unset; never guess a path. -This is a development convenience only — anything worth shipping still -goes through the normal PR + release flow described below. +**User-initiated ONLY — never automatic.** Dev-sync overwrites the +maintainer's live ComfyUI install. Do NOT trigger it as part of any +automated flow: -The broader iteration pattern (trigger phrases, what survives across -re-syncs vs what doesn't, push/publish gates) is captured in -[`docs/maintainers/dev-iteration-loop.md`](docs/maintainers/dev-iteration-loop.md). + • after a commit + • after a PR merge or `/ship-pr` + • at session end / wrap-up + • on hook completion or "task complete" cleanup + • from any agent skill that doesn't explicitly require it + +The maintainer typically runs multiple parallel sessions across worktrees; +an unsolicited sync from one session silently overwrites what another is +reviewing in ComfyUI, and the wrong code gets reviewed. Wait for the +explicit trigger phrase. If unsure whether the user wants a sync, ASK — +don't sync. + +**Chat report format** — two lines, copied verbatim from the script's +own header line: + +``` + - +<≤10 word summary of the PR / change scope to review> +``` + +Line 1 is the script's first output line — never invent it, just relay. +Line 2 is the *scope of what to review* — the change in this build, ≤10 +words, no list of all files. Examples: + +``` +8dfb966 - dreamy-jones-40655e +Quick Save + status dot + recovery section + +a1b2c3d - feat-install-missing +install-missing button on Nodes row +``` + +Broader iteration pattern (push/publish gates, what survives re-syncs) +is in [`docs/maintainers/dev-iteration-loop.md`](docs/maintainers/dev-iteration-loop.md). ## Kforge Labs sidebar — starter preset distribution diff --git a/scripts/sync_to_dev.py b/scripts/sync_to_dev.py index 0ec93dc..c89d75e 100644 --- a/scripts/sync_to_dev.py +++ b/scripts/sync_to_dev.py @@ -3,6 +3,14 @@ Copy runtime-relevant Koolook files into a live ComfyUI custom_nodes folder so a fix can be tested without a tag-and-publish round-trip. +USER-INITIATED ONLY. This script overwrites a live ComfyUI install. +Agents must NEVER run it automatically — not after a commit, not after +a PR merge or ship-pr, not at session end, not from any "task complete" +cleanup. The maintainer typically has multiple parallel sessions across +worktrees, and an unsolicited sync from one silently destroys what +another is reviewing. See project CLAUDE.md `dev-sync` section for the +full policy. Run only on the explicit user trigger phrase. + The target path is read from the KOLOOK_COMFYUI_DEV_PATH environment variable (loaded from `.env` at the repo root if present). The variable is intentionally kept out of the committed tree — see `.env.example`. @@ -37,11 +45,59 @@ from __future__ import annotations import argparse import os import shutil +import subprocess import sys from pathlib import Path REPO_ROOT = Path(__file__).resolve().parent.parent + +def _get_short_sha() -> str | None: + """Best-effort short commit SHA of the source tree being synced. + Returns ``None`` if git isn't reachable or the call fails — the + summary line just omits the SHA in that case. + """ + try: + r = subprocess.run( + ["git", "rev-parse", "--short", "HEAD"], + cwd=str(REPO_ROOT), + capture_output=True, + text=True, + timeout=2, + ) + if r.returncode == 0: + sha = r.stdout.strip() + if sha: + return sha + except (OSError, subprocess.TimeoutExpired): + pass + return None + + +def _get_worktree_name() -> str: + """Returns the basename of the source tree being synced — useful when + the maintainer is running multiple parallel ComfyUI installs and + needs to know which checkout fed the most recent sync. + + For a worktree at ``.../ComfyUI-Koolook/.claude/worktrees/foo`` this + returns ``foo``; for the main repo at ``.../ComfyUI-Koolook`` it + returns ``ComfyUI-Koolook``. Either is informative enough to + disambiguate.""" + return REPO_ROOT.name + + +def _build_line() -> str: + """Composes the two-piece header line consumed by the chat-report + convention defined in project CLAUDE.md: + + - + + SHA falls back to ``unknown`` if git is unreachable (we always need + SOMETHING in slot 1 — the line shape is part of the convention). + Worktree name comes from ``REPO_ROOT.name`` and is always present.""" + sha = _get_short_sha() or "unknown" + return f"{sha} - {_get_worktree_name()}" + # Files / dirs ComfyUI actually loads at runtime. Anything outside this # list (CI, docs, .claude/, .github/, .cursor/, CHANGELOG, LICENSE, # pyproject.toml, README, fork manifest YAML, etc.) does not affect what @@ -87,7 +143,7 @@ def _ignore(_dir: str, names: list[str]) -> list[str]: return [n for n in names if n in DIR_EXCLUDES] -def sync(target: Path, dry_run: bool) -> int: +def sync(target: Path, dry_run: bool, verbose: bool) -> int: copied = 0 for rel in RUNTIME_PATHS: src = REPO_ROOT / rel @@ -95,7 +151,8 @@ def sync(target: Path, dry_run: bool) -> int: continue dst = target / rel if dry_run: - print(f"would copy: {rel}") + if verbose: + print(f"would copy: {rel}") copied += 1 continue if src.is_dir(): @@ -105,7 +162,8 @@ def sync(target: Path, dry_run: bool) -> int: else: dst.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(src, dst) - print(f"copied: {rel}") + if verbose: + print(f"copied: {rel}") copied += 1 return copied @@ -178,6 +236,15 @@ def main() -> int: "in place. Use on first-run only." ), ) + parser.add_argument( + "--verbose", + "-v", + action="store_true", + help=( + "Print each file/dir as it's copied. Default output is just " + "the one-line build summary." + ), + ) args = parser.parse_args() load_dotenv(REPO_ROOT / ".env") @@ -196,11 +263,14 @@ def main() -> int: if err is not None: return err - n = sync(target, dry_run=args.dry_run) + n = sync(target, dry_run=args.dry_run, verbose=args.verbose) verb = "would sync" if args.dry_run else "synced" - print(f"\n{verb} {n} entries -> {target}") - if not args.dry_run: - print("restart ComfyUI to pick up changes.") + # Two-line summary — see project CLAUDE.md `dev-sync` section for the + # chat-report convention that consumes this output. Header first so + # the maintainer's eye lands on the build identifier before the + # mechanical sync details. + print(_build_line()) + print(f"{verb} {n} entries -> {target}") return 0