Skip to content

Commit eb6d17f

Browse files
annehe9claude
andauthored
feat(knowledge): fetch the bundle from the cloud knowledge channel by default (BE-8952) (#813)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 626552e commit eb6d17f

9 files changed

Lines changed: 201 additions & 35 deletions

File tree

‎README.md‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -631,10 +631,11 @@ custom_nodes:
631631

632632
Discovery commands can attach a `knowledge` block to their `--json` output: which
633633
model a name refers to, whether it is deprecated, ranked picks per capability,
634-
and known pitfalls. It is **off unless you point the CLI at a bundle**, so a
635-
default install never emits the block.
634+
and known pitfalls. A signed-in install (`comfy cloud login`) fetches the bundle
635+
from the cloud knowledge channel on its own. Signed out, the fetch fails quietly
636+
and no block is emitted unless a bundle is already cached.
636637

637-
Turn it on with either of:
638+
Point the CLI at a different bundle with either of:
638639

639640
```
640641
export COMFY_KNOWLEDGE_URL=https://.../knowledge.json # fetched and cached
@@ -655,8 +656,8 @@ Turn it off again:
655656
export COMFY_KNOWLEDGE_DISABLE=1
656657
```
657658

658-
Clearing `COMFY_KNOWLEDGE_URL` is *not* an off switch. Once a bundle is cached it
659-
keeps being served, stale or not. `COMFY_KNOWLEDGE_DISABLE` suppresses envelope
659+
Signing out is *not* an off switch. Once a bundle is cached it keeps being
660+
served, stale or not. `COMFY_KNOWLEDGE_DISABLE` suppresses envelope
660661
enrichment outright; the `comfy knowledge` verbs keep working under it, since
661662
those are you asking for the bundle directly.
662663

@@ -683,9 +684,9 @@ comfy tracking disable
683684
When tracking is on, we use Mixpanel to understand usage patterns and know where
684685
to prioritize our efforts.
685686

686-
**One event carries text you typed.** If a curated knowledge bundle is configured
687-
— it is not by default, and needs `COMFY_KNOWLEDGE_URL` or `COMFY_KNOWLEDGE_FILE`
688-
— then `comfy nodes search`, `comfy templates ls`, `comfy generate schema`,
687+
**One event carries text you typed.** If a curated knowledge bundle is loaded
688+
(a signed-in install fetches one on its own), then `comfy nodes search`,
689+
`comfy templates ls`, `comfy generate schema`,
689690
`comfy generate list` and `comfy models search` send the search terms you typed
690691
and which curated entries they matched. This tells us what people look for and fail
691692
to find. It needs both the bundle and your tracking consent, so with either one

‎comfy_cli/cloud/command.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import typer
2626
from rich import markup as rich_markup
2727

28-
from comfy_cli import tracking
28+
from comfy_cli import knowledge, tracking
2929
from comfy_cli.auth import store
3030
from comfy_cli.cloud import CLIENT_ID, CLIENT_NAME, CONFIG_KEY_BASE_URL, get_base_url, get_resource_url, get_scopes
3131
from comfy_cli.cloud.oauth import OAuthError, run_login
@@ -134,6 +134,10 @@ def _on_url(url: str) -> None:
134134
expires_at=result.tokens.expires_at,
135135
)
136136

137+
# Fill the knowledge cache while the user is online and freshly signed
138+
# in, so the cache-only attach path has a bundle on a brand-new install.
139+
knowledge.refresh_if_stale()
140+
137141
if renderer.is_pretty():
138142
from comfy_cli.output.branding import welcome_banner
139143

‎comfy_cli/command/knowledge.py‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,10 @@
2424

2525

2626
def _env_context() -> dict[str, Any]:
27-
url = os.environ.get(knowledge.ENV_URL, "").strip()
2827
return {
2928
"env_file": os.environ.get(knowledge.ENV_FILE, "").strip() or None,
3029
# Userinfo, query and fragment can carry a token; the path still shows which bundle is configured.
31-
"url": tracking._scrub_value(url) if url else None,
30+
"url": tracking._scrub_value(knowledge.bundle_url()),
3231
"ttl_seconds": knowledge.ttl_seconds(),
3332
"cache_path": str(knowledge.cache_paths()[0]),
3433
}
@@ -42,13 +41,22 @@ def _text(value: Any) -> str | None:
4241
return value if isinstance(value, str) else None
4342

4443

44+
_UNAVAILABLE_HINTS = {
45+
knowledge.REASON_SIGNED_OUT: "sign in with `comfy cloud login` so the bundle can be fetched, or set COMFY_KNOWLEDGE_FILE to a knowledge.json",
46+
knowledge.REASON_FETCH_FAILED: "check COMFY_KNOWLEDGE_URL, or set COMFY_KNOWLEDGE_FILE to a knowledge.json",
47+
knowledge.REASON_ENV_FILE: "check the COMFY_KNOWLEDGE_FILE path",
48+
}
49+
50+
4551
def _require_bundle(renderer) -> knowledge.Bundle:
4652
bundle = knowledge.load_bundle()
4753
if bundle is None:
54+
reason = knowledge.last_reason()
55+
hint = _UNAVAILABLE_HINTS.get(reason, "set COMFY_KNOWLEDGE_FILE to a knowledge.json")
4856
renderer.error(
4957
code="knowledge_unavailable",
50-
message="no knowledge bundle is loaded",
51-
hint="set COMFY_KNOWLEDGE_FILE to a knowledge.json, or COMFY_KNOWLEDGE_URL to fetch one; see `comfy knowledge status`",
58+
message=f"no knowledge bundle is loaded: {reason}",
59+
hint=f"{hint}; see `comfy knowledge status`",
5260
)
5361
raise typer.Exit(code=1)
5462
return bundle
@@ -59,7 +67,10 @@ def _require_bundle(renderer) -> knowledge.Bundle:
5967
def status_cmd(
6068
refresh: Annotated[
6169
bool,
62-
typer.Option("--refresh", help="Re-fetch from COMFY_KNOWLEDGE_URL, ignoring the cache TTL."),
70+
typer.Option(
71+
"--refresh",
72+
help="Reload the bundle, ignoring the cache TTL (re-reads COMFY_KNOWLEDGE_FILE when set, else re-fetches).",
73+
),
6374
] = False,
6475
):
6576
renderer = get_renderer()

‎comfy_cli/knowledge.py‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
The bundle (``knowledge.json`` + optional ``manifest.json``) is produced by
44
the curated knowledge bundle repo. It reaches this process by one of three routes,
55
tried in order: an explicit ``COMFY_KNOWLEDGE_FILE``, the per-user cache, or a
6-
fetch from ``COMFY_KNOWLEDGE_URL``. A missing or broken bundle is a normal
6+
fetch from ``COMFY_KNOWLEDGE_URL``, which defaults to the knowledge channel
7+
under the cloud base URL. A missing or broken bundle is a normal
78
state: every entry point here returns ``None`` rather than raising, and nothing
89
is written to stdout or stderr. :func:`attach` is how discovery commands add a
910
capped ``knowledge`` block to their payload; it is fail-open for the same reason.
@@ -39,6 +40,7 @@
3940
ENV_URL = "COMFY_KNOWLEDGE_URL"
4041
ENV_TTL = "COMFY_KNOWLEDGE_TTL"
4142
ENV_DISABLE = "COMFY_KNOWLEDGE_DISABLE"
43+
DEFAULT_URL_PATH = "/api/knowledge/knowledge.json"
4244
DEFAULT_TTL_SECONDS = 24 * 60 * 60
4345
FETCH_TIMEOUT_SECONDS = 10.0
4446
MAX_BUNDLE_BYTES = 16 * 1024 * 1024
@@ -77,8 +79,11 @@
7779
MIN_SINGLE_TOKEN_CHARS = 4
7880

7981
REASON_ENV_FILE = "COMFY_KNOWLEDGE_FILE is set but could not be loaded"
80-
REASON_NO_URL = "no cache and COMFY_KNOWLEDGE_URL is not set"
81-
REASON_FETCH_FAILED = "fetch failed and no cached bundle exists"
82+
REASON_SIGNED_OUT = (
83+
"fetch from the cloud knowledge channel failed and no cached bundle exists; "
84+
"the usual cause is being signed out (run `comfy cloud login`)"
85+
)
86+
REASON_FETCH_FAILED = "fetch from COMFY_KNOWLEDGE_URL failed and no cached bundle exists"
8287

8388

8489
@dataclass(frozen=True)
@@ -137,11 +142,27 @@ def ttl_seconds() -> float:
137142
return max(ttl, 0.0) if math.isfinite(ttl) else DEFAULT_TTL_SECONDS
138143

139144

145+
def default_url() -> str:
146+
"""``knowledge.json`` under the cloud base URL, derived on every call.
147+
148+
Never stored: :func:`_http_get` attaches credentials only under
149+
``get_base_url()``, which resolves per invocation, so a remembered
150+
production URL would silently fetch unauthenticated for anyone pointed at
151+
another environment.
152+
"""
153+
return get_base_url().rstrip("/") + DEFAULT_URL_PATH
154+
155+
156+
def bundle_url() -> str:
157+
"""``COMFY_KNOWLEDGE_URL`` when set, else :func:`default_url`."""
158+
return os.environ.get(ENV_URL, "").strip() or default_url()
159+
160+
140161
def load_bundle(*, force_fetch: bool = False, cache_only: bool = False) -> Bundle | None:
141162
"""Return the indexed bundle, or ``None`` when no usable bundle exists.
142163
143164
Memoized per process; ``force_fetch=True`` re-runs the load and skips the
144-
cache TTL gate so a fetch happens whenever ``COMFY_KNOWLEDGE_URL`` is set.
165+
cache TTL gate so a fetch happens unless ``COMFY_KNOWLEDGE_FILE`` is set.
145166
``cache_only=True`` never touches the network: env file, then any cache
146167
(fresh or stale), else ``None``. The memo is shared, except that a
147168
cache-only miss is not memoized so a later full load can still fetch.
@@ -315,17 +336,17 @@ def _load(*, force_fetch: bool, cache_only: bool = False) -> tuple[Bundle | None
315336
if bundle is not None:
316337
return bundle, None
317338

318-
url = os.environ.get(ENV_URL, "").strip()
319-
if url and not cache_only:
320-
bundle = _fetch(url, knowledge_path, manifest_path)
339+
explicit_url = os.environ.get(ENV_URL, "").strip()
340+
if not cache_only:
341+
bundle = _fetch(explicit_url or default_url(), knowledge_path, manifest_path)
321342
if bundle is not None:
322343
return bundle, None
323344

324345
source = "cache" if fresh else "stale-cache"
325346
bundle = _load_file(knowledge_path, manifest_path, source=source, stale=not fresh)
326347
if bundle is not None:
327348
return bundle, None
328-
return None, (REASON_FETCH_FAILED if url else REASON_NO_URL)
349+
return None, (REASON_FETCH_FAILED if explicit_url else REASON_SIGNED_OUT)
329350

330351

331352
def _cache_is_fresh(path: Path) -> bool:
@@ -390,7 +411,9 @@ def _http_get(url: str) -> bytes:
390411
if url.startswith(get_base_url().rstrip("/") + "/"):
391412
from comfy_cli.target import resolve_target
392413

393-
opened = authed_urlopen(url, resolve_target(where="cloud"), timeout=FETCH_TIMEOUT_SECONDS)
414+
# Best-effort background fetch: a spurious refresh failure must not log the user out.
415+
target = resolve_target(where="cloud", allow_clear=False)
416+
opened = authed_urlopen(url, target, timeout=FETCH_TIMEOUT_SECONDS)
394417
else:
395418
req = urllib.request.Request(url, headers={"User-Agent": "comfy-cli"})
396419
opened = plain_urlopen(req, timeout=FETCH_TIMEOUT_SECONDS)
@@ -922,9 +945,9 @@ def attach(
922945
exactly as it was.
923946
924947
``COMFY_KNOWLEDGE_DISABLE`` suppresses the block entirely. A cached bundle
925-
keeps being read once it exists, stale or not, so clearing
926-
``COMFY_KNOWLEDGE_URL`` is not an off switch and this is. Following
927-
``DO_NOT_TRACK``, any value but empty or ``"0"`` disables.
948+
keeps being read once it exists, stale or not, and the default URL means a
949+
signed-in install fetches one without being asked, so this is the only off
950+
switch. Following ``DO_NOT_TRACK``, any value but empty or ``"0"`` disables.
928951
"""
929952
try:
930953
disable = os.environ.get(ENV_DISABLE, "")

‎comfy_cli/schemas/knowledge.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
"as_of": { "type": ["string", "null"], "description": "ISO-8601 UTC timestamp: file mtime (status) or the capability's as_of, null when the capability has none (pick)." },
1414
"path": { "type": "string", "description": "File the bundle bytes were read from (status)." },
1515
"env_file": { "type": ["string", "null"], "description": "Value of COMFY_KNOWLEDGE_FILE (status)." },
16-
"url": { "type": ["string", "null"], "description": "Value of COMFY_KNOWLEDGE_URL with userinfo, query and fragment removed (status)." },
16+
"url": { "type": ["string", "null"], "description": "URL a fetch uses: COMFY_KNOWLEDGE_URL, else knowledge.json under the cloud base URL; userinfo, query and fragment removed (status)." },
1717
"ttl_seconds": { "type": "number", "description": "Cache TTL in effect (status)." },
1818
"cache_path": { "type": "string", "description": "Where a fetched bundle is cached (status)." },
1919
"counts": {

‎comfy_cli/target.py‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,12 @@ def url(self, *parts: str) -> str:
5151

5252

5353
def resolve_target(
54-
*, where: str | None, host: str | None = None, port: int | None = None, config: Any = None
54+
*,
55+
where: str | None,
56+
host: str | None = None,
57+
port: int | None = None,
58+
config: Any = None,
59+
allow_clear: bool = True,
5560
) -> Target:
5661
"""Pick a Target based on ``--where`` plus host/port (for local).
5762
@@ -60,6 +65,10 @@ def resolve_target(
6065
auto-detect (``cloud`` if credentials exist, else ``local``).
6166
The ``config`` arg is honored if passed; otherwise
6267
we instantiate a ``ConfigManager`` to read the persisted key.
68+
69+
``allow_clear=False`` is forwarded to :func:`resolve_cloud_credential` so a
70+
best-effort background caller cannot wipe the stored session on a failed
71+
token refresh.
6372
"""
6473
from comfy_cli import where as where_module
6574

@@ -92,7 +101,7 @@ def resolve_target(
92101
# when the token isn't near expiry (no network), and the client/loader
93102
# still force-refresh *reactively* on a server 401 as a backstop.
94103
base_url = get_base_url()
95-
cred = resolve_cloud_credential(purpose="cloud", base_url=base_url, refresh=True)
104+
cred = resolve_cloud_credential(purpose="cloud", base_url=base_url, refresh=True, allow_clear=allow_clear)
96105
token = cred.value if cred is not None and cred.kind == "oauth" else None
97106
api_key = cred.value if cred is not None and cred.kind == "api_key" else None
98107

‎tests/comfy_cli/cloud/test_login_command.py‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,52 @@ def _no_tracking(monkeypatch):
5757
monkeypatch.setattr("comfy_cli.tracking.track_event", lambda *a, **k: None)
5858

5959

60+
@pytest.fixture(autouse=True)
61+
def _knowledge_warm(monkeypatch):
62+
"""Record the post-login knowledge warm instead of fetching a bundle."""
63+
calls: list[None] = []
64+
monkeypatch.setattr(command.knowledge, "refresh_if_stale", lambda: calls.append(None))
65+
return calls
66+
67+
68+
def test_login_warms_the_knowledge_cache_after_saving_the_session(monkeypatch, capsys, _knowledge_warm):
69+
order: list[str] = []
70+
71+
def fake_run_login(**kwargs):
72+
kwargs["on_url_ready"](_AUTHORIZE_URL)
73+
return _login_result()
74+
75+
real_save = command.store.save_cloud_session
76+
77+
def save_then_record(**kwargs):
78+
order.append("save")
79+
return real_save(**kwargs)
80+
81+
monkeypatch.setattr(command, "run_login", fake_run_login)
82+
monkeypatch.setattr(command.store, "save_cloud_session", save_then_record)
83+
monkeypatch.setattr(command.knowledge, "refresh_if_stale", lambda: order.append("warm"))
84+
set_renderer(Renderer(mode=OutputMode.JSON, command="cloud login", version="test"))
85+
86+
command.login_cmd(no_browser=True, timeout=300)
87+
88+
assert order == ["save", "warm"]
89+
assert _parse_lines(capsys.readouterr().out)[-1]["ok"] is True
90+
91+
92+
def test_login_timeout_does_not_warm_the_knowledge_cache(monkeypatch, _knowledge_warm):
93+
def fake_run_login(**kwargs):
94+
kwargs["on_url_ready"](_AUTHORIZE_URL)
95+
raise oauth.OAuthError("timed out")
96+
97+
monkeypatch.setattr(command, "run_login", fake_run_login)
98+
set_renderer(Renderer(mode=OutputMode.JSON, command="cloud login", version="test"))
99+
100+
with pytest.raises(typer.Exit):
101+
command.login_cmd(no_browser=True, timeout=1)
102+
103+
assert _knowledge_warm == []
104+
105+
60106
def test_json_login_emits_login_url_event_before_envelope(monkeypatch, capsys):
61107
"""`comfy --json cloud login --no-browser` streams a `login_url` event
62108
(carrying the authorize URL) ahead of the final, session-redacted envelope."""

0 commit comments

Comments
 (0)