Share connected WP.com sites between Studio app and CLI by youknowriad · Pull Request #3227 · Automattic/studio

youknowriad · 2026-04-24T09:19:26Z

Related issues

Related to #

How AI was used in this PR

Opus 4.7 scoped the refactor, wrote the new shared helpers, the migration, and the Desktop/CLI wiring. I (the author) steered the data-model decision (cli.json sites property, per-user keying preserved) and reviewed each hunk. The diff is roughly 520 lines added / 135 removed across 12 files — mostly mechanical once the data layout was settled.

Proposed Changes

Connected WP.com sites used to live in app.json as a top-level connectedWpcomSites: { [userId]: SyncSite[] } — Desktop-only state. This moves the list into per-site entries in cli.json so both Desktop and the CLI share a single source of truth, and wires up the CLI push/pull flows to auto-connect on success.

Shared data layer (tools/common/)
- syncSiteSchema zod schema added next to the SyncSite type, which is now inferred from it.
- New tools/common/lib/connected-sites.ts with read/write helpers against cli.json: getConnectedWpcomSitesForLocalSite, getAllConnectedWpcomSitesForCurrentUser, addConnectedWpcomSite, removeConnectedWpcomSite, updateConnectedWpcomSites, markConnectedWpcomSiteSynced. Uses the existing cli.json.lock lockfile and a permissive passthrough schema so Desktop and CLI can evolve their cli.json site entries independently without corrupting each other's fields.
Storage schemas
- CLI siteSchema now carries an optional connectedWpcomSites: { [userId]: SyncSite[] } map per site.
- connectedWpcomSites removed from the app.json UserData type.
Migration 04 copies app.json.connectedWpcomSites[userId][].{localSiteId} into cli.json sites[].connectedWpcomSites[userId], deduping by remote id. It stamps connectedWpcomSitesMigratedToCli: true on app.json and leaves the legacy field in place for this release so older Studio versions keep working — a follow-up migration will strip it.
Desktop IPC handlers now delegate through the shared helpers instead of reading/writing app.json:
- connectWpcomSites, disconnectWpcomSites, updateConnectedWpcomSites, getConnectedWpcomSites in apps/studio/src/modules/sync/lib/ipc-handlers.ts
- reconcileSessionEnvironmentBeforeRun and setSessionEnvironment in apps/studio/src/ipc-handlers.ts
CLI auto-connect: apps/cli/commands/push.ts and apps/cli/commands/pull.ts call addConnectedWpcomSite + markConnectedWpcomSiteSynced after a successful run.
AI agent push workflow: the studio code agent gained a site_connected_remote_sites MCP tool plus a new system-prompt section instructing it to resolve the target before pushing — 1 attached site → confirm, many → AskUserQuestion list, 0 → open-ended question.

Testing Instructions

Back up ~/.studio/app.json and ~/.studio/cli.json.
Migration: with app.json.connectedWpcomSites populated, launch the Desktop app. Expect cli.json sites[].connectedWpcomSites[userId] to be populated, and app.json.connectedWpcomSitesMigratedToCli: true to be added. The legacy app.json.connectedWpcomSites should still be present.
Desktop: open a site with a connection — publish picker + site dropdown + session environment switcher should show the connection as before.
Desktop connect/disconnect: from the sync modal, connect and disconnect a WordPress.com site — the entry should appear/disappear in cli.json without further app.json writes.
CLI auto-connect: run studio push or studio pull for a site with no connections yet — after success, verify the remote site shows up under cli.json sites[].connectedWpcomSites and that Desktop's site dropdown reflects it.
CLI AI agent: run studio code, ask it to "push my site". It should call site_connected_remote_sites, then either confirm (single attached), show a picker (many), or ask open-ended (none) before calling site_push.

Pre-merge Checklist

Have you checked for TypeScript, React or other console errors? (npm run typecheck clean across workspaces; npm test -- apps/cli apps/studio — 1042 passing)
Manual Desktop smoke test of connect/disconnect UI
Manual CLI push/pull to verify auto-connect

🤖 Generated with Claude Code

…member them Connected sites used to live in app.json as a top-level `connectedWpcomSites: { [userId]: SyncSite[] }` — Desktop-only state that the CLI could not read or write. This moves the list into per-site entries in cli.json so both Desktop and the CLI share a single source of truth, and wires up the CLI push/pull flows to auto-connect on success. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

wpmobilebot · 2026-04-24T09:40:51Z

📊 Performance Test Results

Comparing dd14d49 vs trunk

app-size

Metric	trunk	`dd14d49`	Diff	Change
App Size (Mac)	1454.55 MB	1957.43 MB	+502.88 MB	🔴 34.6%

site-editor

Metric	trunk	`dd14d49`	Diff	Change
load	1824 ms	1780 ms	44 ms	⚪ 0.0%

site-startup

Metric	trunk	`dd14d49`	Diff	Change
siteCreation	8085 ms	8094 ms	+9 ms	⚪ 0.0%
siteStartup	4948 ms	4950 ms	+2 ms	⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

Treat the move of connected sites out of app.json as a breaking change. `loadUserData` now throws `AppConfigVersionMismatchError` if it finds a version higher than this build supports, and the main process surfaces a blocking "Please upgrade Studio" dialog before boot continues. Mirrors the `SharedConfigVersionMismatchError` pattern used for shared.json. Migration 04 now stamps version 2 on app.json and strips the legacy top-level `connectedWpcomSites` field, so once it runs, older Studio builds reading the same `~/.studio/` directory are forced to upgrade before they can clobber the migrated data. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

bcotrim

Great concept and the data model decision makes sense. Left a couple of suggestions, mainly questioning whether shared.json might be a cleaner home for this data than cli.json, and a tweak to the migration strategy.
Using the shared.json file also brings the solution closer to your suggestion of keeping the data shared between Studio app and cli.
Happy to discuss if any of it needs more context!

bcotrim · 2026-04-24T14:05:47Z

+import { syncSiteSchema, type SyncSite } from '../types/sync';
+import { lockFileAsync, unlockFileAsync } from './lockfile';
+import { getCurrentUserId } from './shared-config';
+import { getCliConfigPath, getConfigDirectory } from './well-known-paths';


Did you consider using shared.json instead for storing this data?
The purpose of that file is exactly cases like this one, where both CLI and App want to manage some data. It would give us the existing lockSharedConfig/saveSharedConfig/unlockSharedConfig making this lib a lot cleaner.

I agree it's shared but it's also a site property and "sites" is in cli.json so unless we move sites to shared, for me it makes more sense for this to be under "sites" than being a separate property.

And to be honest, for me everything is shared almost at this point. There's very little arguments to hold onto the split files.

Fair point on sites being a CLI concept, but I think connectedWpcomSites is more of a sync/UI concern than a site property, the App needs it for the publish picker, the CLI needs it for push/pull targeting. Neither process really "owns" it.

There's also a precedent that cuts against the "site property → cli.json" argument: sortOrder and themeDetails are site data too, but they live in app.json because the CLI doesn't need, or should be aware of them.

When we introduced the CLI as the mechanism of running site commands in Studio app, one of the side effects is to make the CLI owner of that implementation. I believe Studio shouldn't be aware or using parts of the CLI implementation. Today the CLI stores its data in a file, tomorrow it can be a database, remote server, etc... the app shouldn't have to adapt to it.

Fair point on sites being a CLI concept, but I think connectedWpcomSites is more of a sync/UI concern than a site property, the App needs it for the publish picker, the CLI needs it for push/pull targeting. Neither process really "owns" it.

Same for the whole "sites" object, both need them. It' still a site property for me, which sites are connected to my local site.

There's also a precedent that cuts against the "site property → cli.json" argument: sortOrder and themeDetails are site data too, but they live in app.json because the CLI doesn't need, or should be aware of them.

sort can be useful to sort the sites in CLI too. themeDetails is a cache that probably doesn't make sense in our config.

bcotrim · 2026-04-24T14:12:30Z

@@ -0,0 +1,139 @@
+/**
+ * Moves `connectedWpcomSites` from `app.json` into per-site entries in


I would suggest doing this migration in 2 steps.

CLI –> Moves data over to shared.json/cli.json if necessary, and continues, no data cleanup

App –> Moves data over to shared.json/cli.json if necessary, does the data cleanup afterwards.
The existing file locking mechanism will prevent these migrations to collide with each other. I think this gives the process a good safety in case the app isn't updated.

I don't understand the current behavior enough to make this change, it's so confusing to me to be honest. I think we need a big simplification but feel free to push changes.

fredrikekelund · 2026-04-29T09:10:52Z

I'm taking a look at this PR now

fredrikekelund · 2026-04-29T10:57:02Z

I implemented a new strategy here. Here's what the code does now:

The goal of my implementation is to move connectedWpcomSites to shared.json, without changing the structure of that object. This achieves the fundamental goal here, without getting lost in questions about ownership.
It accepts the drawback that there's slim risk of some data loss (since connectedWpcomSites is non-critical data). In return, we can run the migration from either the CLI or the app without users needing to think about it. In contrast, the old solution implemented a breaking change in the app.json schema, which is much more disruptive.
In concrete terms, we move the connectedWpcomSites schema to tools/common and add functions for reading/writing the data from/to shared.json. @youknowriad had already done this. I've just refactored some parts of it.
The CLI gets a data migration that runs if app.json contains connectedWpcomSites and shared.json does not. It copies the connectedWpcomSites object to shared.json.
The app gets a data migration that runs if app.json contains connectedWpcomSites. If shared.json doesn't already contain a connectedWpcomSites object, it copies that data. It always deletes connectedWpcomSites from app.json.
In short, either migration can run first, and they're near idempotent. The risk for data loss is if the CLI runs the migration first, before the app is updated, and then the user changes which site is connected

fredrikekelund · 2026-04-29T11:51:00Z

I've asked @bcotrim to review my changes to this PR

bcotrim · 2026-04-29T14:05:06Z

+		}
+
+		const { connectedWpcomSites: _legacy, ...rest } = parsed;
+		await writeFile( getAppConfigPath(), JSON.stringify( rest, null, 2 ) + '\n', {


Should we use the lock/unlock pattern here?

bcotrim · 2026-04-29T14:43:57Z


+			// Remember this connection so future push/pull runs (and the Desktop UI)
+			// can surface it without re-selecting from the full site list.
+			await addConnectedWpcomSite( site.id, { ...remoteSite, localSiteId: site.id } );


Should we try...catch here? The pull was still successful and the user will see the reflected site changes.

bcotrim · 2026-04-29T14:44:44Z


+		// Remember this connection so future push/pull runs (and the Desktop UI)
+		// can surface it without re-selecting from the full site list.
+		await addConnectedWpcomSite( site.id, { ...remoteSite, localSiteId: site.id } );


Similar to https://github.com/Automattic/studio/pull/3227/changes#r3161883090 we could add a try...catch here

bcotrim

Added a couple of suggestions that I think we could address before merging.
Code LGTM and works as described 👍

youknowriad · 2026-04-29T14:47:20Z

In short, either migration can run first, and they're near idempotent. The risk for data loss is if the CLI runs the migration first, before the app is updated, and then the user changes which site is connected

Yes, I think the solution for this is to have a single version of config files that both app and cli can read, and if it's higher than the supported one, it asks you to upgrade.

wojtekn assigned youknowriad Apr 24, 2026

bcotrim reviewed Apr 24, 2026

View reviewed changes

fredrikekelund added 4 commits April 29, 2026 12:15

Move connectedWpcomSites to shared.json

7a93c7f

Simplify logic for updating connectedWpcomSites

d622fae

Minor tweaks

9a98fa6

Fix

dd14d49

bcotrim reviewed Apr 29, 2026

View reviewed changes

bcotrim approved these changes Apr 29, 2026

View reviewed changes

		@@ -0,0 +1,139 @@
		/**
		* Moves `connectedWpcomSites` from `app.json` into per-site entries in

Conversation

youknowriad commented Apr 24, 2026

Related issues

How AI was used in this PR

Proposed Changes

Testing Instructions

Pre-merge Checklist

Uh oh!

wpmobilebot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Performance Test Results

app-size

site-editor

site-startup

Uh oh!

bcotrim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bcotrim Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredrikekelund commented Apr 29, 2026

Uh oh!

fredrikekelund commented Apr 29, 2026

Uh oh!

fredrikekelund commented Apr 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bcotrim left a comment

Choose a reason for hiding this comment

Uh oh!

youknowriad commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wpmobilebot commented Apr 24, 2026 •

edited

Loading

bcotrim Apr 24, 2026 •

edited

Loading

youknowriad Apr 24, 2026 •

edited

Loading