Gate distributions UI behind distributions_enabled PostHog flag - #1396
Draft
claude[bot] wants to merge 20 commits into
Draft
Gate distributions UI behind distributions_enabled PostHog flag#1396claude[bot] wants to merge 20 commits into
claude[bot] wants to merge 20 commits into
Conversation
…, stage models, launch) Main-process comfy-builder library: client, artifact install (download + sha256 verify + extract), model staging into ComfyUI/models, host/artifact selection, and launch spec.
Main-process cloud auth library: PKCE OAuth via system browser + loopback callback, encrypted token store, single-flight refresh, JWT claims (display-only), and workspace listing.
Main-process comfybuilder distribution feature: CloudSession-backed IPC bridge (sign-in, workspaces, catalog, install kickoff), the install/launch SourcePlugin (download + sha verify + extract, model staging, host-pinned accel args), plus version-update support - update-available detection gated on a host-compatible newer artifact, and updateDistribution that re-points the existing install in place (clean venv on re-extract, staged models preserved).
* feat(desktop): builder distribution UI Combined distribution UX on the merged comfybuilder functionality: account chip + workspace switcher (real IPC), avatar, two-line install tiles, kebab menu on distribution cards, one footer grammar, install action pill, and the ChooserView integration. Includes a quality pass: error/retry states for the grid and the switcher, workspace name/avatar-colour fix, account-menu scroll cap, double-install guard, right-click parity on cards, and dead-code cleanup. * feat(chooser): workspace shelf and distribution card states Splits the chooser into a bare Your installs shelf plus a Workspace shelf (distribution-backed installs, then still-available distributions). One box glyph per distribution whether installed or a card; anything not on this machine recedes by one weight; a blocked build names the OS it targets. Folds Willie's #1323 onto the dist-ux base. * feat(devplatform): a real manage view for distribution installs Gives a distribution install real Status / Update / Settings tabs (was two tabs by accident). Update reuses the local-install version table (VersionStatPanel extracted from ChannelPicker), and moving between versions re-points the install and re-runs the shared install flow, rolling back on failure and keeping the working venv until the new one lands. Storage and Snapshots stay off by design. Folds Willie's #1325 onto the dist-ux base. * chore(devplatform): review follow-ups on the distribution UI fold - type installedVersion as a number, matching how the row builder sets it - cover resolveHostArtifactForVersion: the named-version artifact selection the update and rollback path relies on (was mocked out in every existing test) - cover the check-update action warming and guarding the version cache - tighten a cold-cache assertion and drop a vacuous rowIds check - collapse a dead blank-line block in ChannelPicker's style block * fix(chooser): keep a distribution-backed install visible when signed out A distribution-backed install was excluded from Your installs by identity and rendered only inside the workspace shelf, which was gated on being signed in. So an installed distribution vanished from the launcher whenever the user was signed out (or before sign-in at launch) and could not be launched. Show the shelf for those installs regardless of session; only the still-available distributions to add need a workspace. Caught by the signed-out Windows e2e. * chore(devplatform): final-review polish - cover the version-stats update button and the no-doubled-footer invariant - pin the signed-out workspace-shelf invariant (count, and no phantom cards) - assert the distribution install-type icon like every sibling case - correct the chooserGridEntry module doc to describe the entry type
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
The chip face and the workspace rows below it were sized independently: 30px vs 26px avatar, caption vs body text, 6px 10px vs 8px padding. A row is the face restated, so both now derive from one spec. The face didn't satisfy "text block the same height as the avatar" either — two lines at line-height 1.3 plus a 1px gap came to ~32px against a 30px avatar, so the text overhung. Dropping the gap and tightening to 1.2 makes the pair exactly 2.4em inside a 2.5em box. Sizing the avatar off the type scale rather than a hard 30px keeps that true across the fluid range (30px at 1024, 35px at 1920) and lands on today's value at the anchor width. Every menu row picked up the face's 10px inset so avatars and icons share one left edge, and its caption size so no row outweighs a workspace name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Flip the compiled defaults from staging to platformapi.comfy.org/builder and cloud.comfy.org (via cloud/config.ts's existing prod default). The staging-issuer side-effect seed is gone, so the import-order note in session.ts is dropped along with it. COMFY_BUILDER_BASE_URL and COMFY_CLOUD_ISSUER env overrides still win for local staging work.
Windows archives now stage the interpreter one level below the venv root, at venv/base/python.exe. That placement is what keeps the venv relocatable: CPython resolves a venv's sys.prefix as dirname(dirname(executable)), so an interpreter sitting AT the venv root resolves to the venv's PARENT - uv then installs outside the venv and every entry point it writes bakes an absolute build path. POSIX already had this shape via venv/bin/. Builder side: Comfy-Org/cloud#6138 (merged). Measured there on a real windows archive: 137 of 143 .exe trampolines embedded C:\comfy\workspace\venv\python.exe, and the fix was verified on the windows build image in both directions. Falls back to the old root path when venv/base is absent, so archives cut before that change still launch.
…portions fix(devplatform): size switcher rows off the account chip
… broadcast Two changes the file-menu login needs, both in the dev-platform bridge. The login call sat inside `ipcMain.handle(signIn)` with a closure-local `signOutGeneration`. The file menu starts sign-ins from main with no renderer in the loop, so a second call site would have silently forked that counter and a sign-out could no longer kill a browser flow the menu had started. The counter is now module-scope behind one `signInToCloud()`; the handler delegates. `broadcastAuthChanged` also never reached the dashboard. A host window loads no page of its own — the chooser renderer lives in a child `panelView` WebContentsView — so `BrowserWindow.getAllWindows()` alone delivered to an empty webContents. That was unobservable while the chip was the only sign-in trigger, because it set its store from `signIn()`'s return value rather than the push; driving sign-in from main makes the broadcast the only path back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "Log in" button rendered on the chooser for every user. It now lives in the title-bar file menu at the head of the settings group, shown only while signed out, and the chooser's account chip renders nothing until there is an account to name. Deliberately not behind a rollout flag: Comfy Builder availability is decided per account, so the decision cannot be made until the user has logged in — gating login on it would put the flag's own precondition behind the flag. Adds `DevPlatformAccountChip.test.ts`; the component had no test file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add a distributions_enabled ops flag (same key the platform website's Distributions UI already reads) so the Comfy Builder distribution shelf on desktop can be hidden without a release. Wired the same way as the existing desktop-cloud-capacity switch: boot-time fetch via getOpsFlag (bypasses telemetry consent, since this is a kill-switch not analytics), cached in main, exposed to the renderer over IPC, defaulting to enabled since the feature previously shipped unflagged. ChooserView's distribution-listing fetch and the chooserDistributions/ distributionNote computeds now also check the flag before showing anything, and registerDevPlatformHandlers gates listDistributions/installDistribution/ updateDistribution on it too, on top of (not instead of) the existing session.isSignedIn() check.
Prevent an older browser login or workspace switch from persisting after a newer auth action. Repeated login requests now share one browser flow, and the IPC bridge delegates race handling to CloudSession. Amp-Thread-ID: https://ampcode.com/threads/T-019ffd3c-3859-70e1-84be-8abdd7ca515d Co-authored-by: Amp <amp@ampcode.com>
…he-sign-in-button-into-the-file-menu feat(titlebar): move Log in into the file menu
# Conflicts: # src/main/lib/ipc/registerDevPlatformHandlers.ts
Amp-Thread-ID: https://ampcode.com/threads/T-019ffd3c-3859-70e1-84be-8abdd7ca515d Co-authored-by: Amp <amp@ampcode.com>
…butions-enabled-flag # Conflicts: # src/main/index.ts # src/main/lib/ipc/registerAppHandlers.ts # src/preload/api.ts # src/renderer/src/views/ChooserView.vue # src/types/ipc.ts
Run prettier --write on the three files this PR added/modified (distributionsEnabled.ts, useDistributionsEnabled.ts, ChooserView.vue) to fix formatting violations introduced by this branch's changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
<!-- ccr-slack-attribution -->
Requested by James Kwon · Slack thread
What
Adds a
distributions_enabledPostHog flag gate on top of the existing sign-in gate, so the Comfy Builder "distributions" UI can be turned off remotely without a release — exactly the ask from James: "we should usedistributions_enabledto hide all the distributions specific stuff that shows up on desktop, e.g. listing distributions after logging in."Flag key:
distributions_enabled— the same key the platform website already uses to gate its own Distributions UI (constants/posthog.ts/middleware/distributions-enabled.ts/composables/useDistributionsEnabled.tsinComfy-Org/platform). Both apps now read one shared PostHog flag.What hides when the flag is off
All of it lives in
ChooserView.vue(there was only one call site):authStore.fetchDistributions()call in the sign-in/workspace watcher) no longer fires.chooserDistributions(and everything downstream of it:visibleDistributions, the workspace-available shelf tiles, the "nothing published to this workspace" note) returns empty/blank.listDistributions,installDistribution,updateDistributioninregisterDevPlatformHandlers.ts) also refuse when the flag is off, as defense-in-depth on top of the renderer gate.Left untouched, deliberately: already-installed Comfy Builder distributions (
allBuilderInstalls) keep showing in the chooser — hiding software the user already installed on their machine behind a remote kill-switch would be surprising and harmful, and that's not what the flag is for.session.isSignedIn()gating is also untouched — the flag is an additional gate, not a replacement.How it's wired
Mirrors the existing
desktop-cloud-capacitypattern (src/main/lib/cloudCapacity.ts) exactly, just with a plain boolean instead of a 3-state status:src/main/lib/distributionsEnabled.ts(new): definesDISTRIBUTIONS_ENABLED_FLAG_KEY = 'distributions_enabled', fetches it once at boot viatelemetry.getOpsFlag(bypasses telemetry consent — this is server config pushed to the client, not analytics collected from the user), caches it for the process lifetime.src/main/index.ts: boots the switch alongsideinitCloudCapacity.src/main/lib/ipc/registerAppHandlers.ts: newget-distributions-enabledIPC handler.src/preload/api.ts/src/types/ipc.ts: exposegetDistributionsEnabled(): Promise<boolean>onwindow.api.src/renderer/src/composables/useDistributionsEnabled.ts(new): renderer composable mirroringuseCloudCapacity, fails OPEN totrue(the distributions UI shipped unflagged before this switch existed, so a fetch miss must never accidentally hide it for everyone).src/renderer/src/views/ChooserView.vue: consumes the composable to gate the fetch trigger and the two computeds noted above.src/main/lib/ipc/registerDevPlatformHandlers.ts: same flag check added to the three IPC handlers, on top of the existingsession.isSignedIn()check.Considered reusing the generic A/B-flag infra in
experiments.ts(getFlag/getFlagAsync) instead, but that path is consent-gated and built for randomized-variant A/B tests with exposure tracking — wrong shape for a simple visibility kill-switch. ThecloudCapacity.tsops-flag pattern is the correct precedent already in the codebase.Verification
pnpm run typecheck(node, web, e2e, integration projects) — clean.pnpm run lint(viaeslint) on all touched/added files — clean.pnpm run test(vitest) — full suite: 2782 passed, 2 skipped. The only failing suites (10, all pre-existing and unrelated to this change) fail withElectron failed to install correctly— a sandbox limitation where the Electron binary itself couldn't be downloaded, not a regression from this PR. All tests touching the changed files (ChooserView.test.ts,registerDevPlatformHandlers.test.ts,useCloudCapacity.test.ts,authStore.test.ts) pass.Generated by Claude Code