Skip to content

Allow files in pull-reprint selections - #4582

Open
fredrikekelund wants to merge 20 commits into
trunkfrom
fix/pull-reprint-file-picker
Open

Allow files in pull-reprint selections#4582
fredrikekelund wants to merge 20 commits into
trunkfrom
fix/pull-reprint-file-picker

Conversation

@fredrikekelund

@fredrikekelund fredrikekelund commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Related issues

How AI was used in this PR

This is the tail end of WordPress/reprint#564, where I drove with Codex to plan and implement the whole thing. Same thing here: Codex did the implementation. The diff is small enough (mostly deletions) that it doesn't make any noticeable difference to reviewers.

Proposed Changes

In #4491, I updated the interactive CLI file picker to use a Jetpack API to query the server-side file tree (instead of the previous approach that read Reprint state in a hacky way to achieve something similar). I had to make the file picker display only directories, since Reprint only supported directory paths (not file paths) in the --only flag (see WordPress/reprint#539). WordPress/reprint#564 fixes that problem. Once it's merged and deployed to wpcomsh, we can proceed to land this PR.

I also took the opportunity in this PR to start passing the --mode=mirror flag to the Reprint pull-files command. Background:

I identified an issue a while back where a Reprint pull would not overwrite a locally edited file unless it had been updated on the remote. Adam fixed this over a series of PRs, most notably this one. The behavior I was looking for is currently enabled through an explicit flag: --mode=mirror.

Testing Instructions

  1. Wait for Accept a file path in pull-files --only WordPress/reprint#564 to be merged, included in a Reprint release, and deployed to wpcomsh
  2. npm run cli:build
  3. node apps/cli/dist/cli/main.mjs create to create a new throwaway site
  4. STUDIO_ENABLE_PULL_REPRINT=1 node apps/cli/dist/cli/main.mjs pull-reprint --path PATH_TO_NEW_SITE
  5. In the interactive file selector, select nothing but a single file
  6. Ensure that the sync finishes successfully

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

fredrikekelund and others added 17 commits August 6, 2026 15:05
flat-docroot --force deletes whatever stands where a symlink must go,
so a first pull wiped the plugins, themes and uploads that only the
local site had. --on-flatten-to-conflict=adopt moves those entries into
the fs-root first, so they survive and stay where a later push can read
them, and a plugin or theme the pull also carries is left to the pull
rather than merged with the local copy.

Still first-pull only. A delta re-pull passes no mode at all, keeping
the default that refuses to overwrite a live site.

Reprint is pinned to the committed phar while these changes are
unreleased, so the download step no longer replaces it.
Testing pull-reprint end to end needs all three unreleased Reprint
changes at once, and no single branch carries them:

  #557 preserve-local-content-into-fs-root    02d0ab73
       --on-flatten-to-conflict=error|replace|adopt
  #540 f26d/apply-runtime-db-apply-option     ba329f38
       --target-* options for apply-runtime
  #436 adamziel/expose-import-layout-metadata 6dc0141f
       source layout and artifact status from import-metadata

Built from integration/studio-test-build in the reprint worktree, which
merges the three cleanly; its Import suite passes at 650 tests. Rebuild
by merging those three heads and running composer build:phar.
Reprint's adopt conflict mode now moves the wp-content entries that
only the site directory holds into the fs-root, so Studio no longer
needs its own pass to do it. The two overlapped on every remaining
case: an unselected path, a kept database and a local-only plugin are
all simply absent from the fs-root, which is what adopt keys on.

Reprint's version is also the more careful one. It stops at whole
plugins and themes, where seedEntry() recursed into any directory both
sides had and would merge two versions of the same plugin. And it runs
where the source of each component is already resolved from preflight,
so a detached uploads directory is adopted against the directory it
actually came from.

The first-pull selective-sync test now asserts what Studio is
responsible for — asking for the adopt mode — rather than the file
movement a mocked Reprint never performs.
resolveOnlyPathsToAbsolute() existed to tell the local preservation
pass which remote paths the selection covered. Nothing asks that
question now, so it and its token table go, along with the test that
was its only remaining consumer.

Also drops a stale line claiming a partial first-pull selection has its
unselected local content preserved here; the flatten step keeps it.
#436 and #540 have landed on Reprint's trunk, so the bundled build no
longer needs an integration branch carrying three heads. This one is
trunk plus the merge-wp-content PR, which replaces #557's
--on-flatten-to-conflict with a command of its own:

  merge-wp-content <url> --state-dir=DIR --fs-root=DIR --from=DIR

Built from the merge-wp-content branch in the reprint worktree with
composer build:phar; its Import suite passes at 650 tests. Rebuild by
checking out that branch and running the same command.

The download step in scripts/download-wp-server-files.ts stays disabled
while these changes are unreleased.
A first pull onto a site that already has content kept that content by
asking flat-docroot to adopt it. Reprint has replaced that mode with a
command of its own, so the pull now runs it as a step:

  merge-wp-content <url> --state-dir --fs-root --from=<site path>

It moves the plugins, themes, uploads and anything else only the local
wp-content has into the fs-root, so the symlink the flatten then puts in
their place still reaches them, and files-push, which reads the fs-root
and nothing else, can still see them. Entries the pull already has are
left alone, and a plugin or theme both sides have is never merged:
keeping the files the pulled version dropped would leave a directory
matching no release.

The step runs only on a first pull. Afterwards the site's wp-content is
a symlink into the fs-root, so it has nothing of its own left to move
and the command would find nothing. It has to follow pull-files, whose
local index Reprint reads as the record of a finished file pull; run any
earlier and every unfetched path looks absent, so local copies move in
and the pull writes remote versions over them.

flat-docroot goes back to --force on a first pull, exactly as it was
before the adopt mode existed.

Two consequences worth knowing, both unchanged from adopt. A blank
Studio install's wp-content/db.php and wp-content/mu-plugins/
sqlite-database-integration move into the fs-root, which is the tree
Reprint indexes and a future files-push would send, even though
Studio's own exporter excludes both as local-only. Neither runs: the
generated runtime.php pre-sets $wpdb, so require_wp_db() returns before
it would require db.php, and WordPress does not auto-load mu-plugin
subdirectories. And on a first pull with the database excluded, the live
wp-content/database/.ht.sqlite moves too, resolving again through the
symlink the flatten places, so there is a window between the two steps
where the site's database is not at its path.
Review of the Reprint side turned up two defects, both fixed on the
merge-wp-content branch at 3f8020ca and neither visible in Studio's
invocation, so this is a phar refresh with no code change beside it.

A relative symlink pointing inside the wp-content being merged now keeps
its value instead of being recomputed against where its target stood
before the run. A child theme pointing at its parent broke as soon as
both moved, which a first pull onto a site using one would have hit.

And --from is resolved against the working directory when relative. That
threw "Path must be absolute" on every run with anything to move.
Studio passes the absolute site.path from its own site record, so it
never reached this one.

Built from the merge-wp-content branch in the reprint worktree with
composer build:phar, and checked through the archive itself: a relative
--from resolves, an in-tree sibling link keeps its value and finds its
parent, and a link pointing out of the tree is still rewritten and still
resolves.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reprint's --from took a site directory and appended wp-content to it.
It now takes that directory, so a site which moved WP_CONTENT_DIR merges
like a conventional one, and pointing the flag at a wp-content directory
no longer succeeds having done nothing.

Studio passes metadata.sitePath + wp-content, which is the same path it
was resolving to before, so the pull behaves as it did.

The bundled Reprint is rebuilt from the branch carrying that change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A pulled site's wp-content is a symlink into the pull's raw directory,
and nothing that looks for symlinks descends through one: `find` needs
-L to follow, the Node walk treats a symlinked directory as a leaf, and
chokidar runs with followSymlinks off. So the open_basedir scan of the
site directory stopped at that link and never saw the plugins and themes
behind it, and the watcher, pointed at the same link, saw nothing at all.

A theme symlinked into wp-content before a pull survives the pull now,
and then fails to load:

  Warning: is_dir(): open_basedir restriction in effect.
  File(…/wp-content/themes/koinonia) is not within the allowed path(s)

Resolving the directory before scanning it fixes both. On the test site
that takes the granted set from 4 entries to 12; seven of the new ones
are WP.com's symlinked plugins, which resolve inside the pull directory
and were already covered, and the eighth is the theme, which points at
~/Downloads and was not.

The scan resolves its own argument, so any caller handing it a symlink
is covered. That alone does not reach the reported case: the startup
scan is given the site directory, which is real, and the link is one
level inside it — hence the second scan naming wp-content.

realpath is the native variant for the reason getFullyResolvedTmpDirPath
already uses it: these paths become an INI value, and a Windows 8.3 short
name carries a tilde that PHP's argument scanner rejects.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…opt-flatten-to-content

# Conflicts:
#	apps/cli/commands/pull-reprint.ts
#	apps/cli/commands/tests/pull-reprint.test.ts
#	apps/cli/lib/pull/preserve-local-content.test.ts
#	apps/cli/lib/pull/preserve-local-content.ts
#	apps/cli/lib/pull/reprint-selector.test.ts
#	apps/cli/lib/pull/reprint-selector.ts
#	apps/cli/php-server-child.ts
#	scripts/download-wp-server-files.ts
@fredrikekelund
fredrikekelund requested a review from a team August 18, 2026 08:15
@fredrikekelund fredrikekelund self-assigned this Aug 18, 2026
@fredrikekelund
fredrikekelund requested a review from a team as a code owner August 19, 2026 08:41
@fredrikekelund
fredrikekelund force-pushed the fix/pull-reprint-file-picker branch from 7eb1878 to c80daac Compare August 20, 2026 13:36
Base automatically changed from f26d/pull-reprint-adopt-flatten-to-content to trunk August 20, 2026 14:09
@wpmobilebot

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing 69e281e vs trunk

app-size

Metric trunk 69e281e Diff Change
App Size (Mac) 1436.08 MB 1436.08 MB 0.00 MB ⚪ 0.0%

site-editor

Metric trunk 69e281e Diff Change
load 1241 ms 1327 ms +86 ms 🔴 6.9%

site-startup

Metric trunk 69e281e Diff Change
siteCreation 7540 ms 7518 ms 22 ms ⚪ 0.0%
siteStartup 2875 ms 3364 ms +489 ms 🔴 17.0%

Results are median values from multiple test runs.

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

@fredrikekelund

Copy link
Copy Markdown
Contributor Author

This PR is ready to land, but I'm holding off until we have some clear word on Automattic/jetpack#51430, which is the server component to this change.

If nothing else, we need the wpcomsh Reprint server dependency to upgrade to the latest version, or anyone trying to pull only files with this command will get an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants