Skip to content

Fix EEXIST in importer when wp-content path is blocked by a non-directory file#3553

Merged
ivan-ottinger merged 17 commits into
trunkfrom
fix-import-eexist-symlink-stub
May 21, 2026
Merged

Fix EEXIST in importer when wp-content path is blocked by a non-directory file#3553
ivan-ottinger merged 17 commits into
trunkfrom
fix-import-eexist-symlink-stub

Conversation

@ivan-ottinger
Copy link
Copy Markdown
Contributor

@ivan-ottinger ivan-ottinger commented May 20, 2026

Related issues

How AI was used in this PR

Diagnosis, reproduction, and patch were drafted with Claude Code. The bug was reproduced end-to-end on Windows; the underlying Node fs behavior was verified via probes on Windows, macOS, and the Linux CI runs on this branch's commits.

Proposed Changes

  • Add ensureDir( dir ) helper in apps/cli/lib/import-export/import/importers/importer.ts that recovers from EEXIST / ENOTDIR on mkdir(recursive:true) by moving the non-directory blocker to trash before retrying.
  • Use it in BaseBackupImporter.importWpContent — the call site that throws.
  • Add unit tests for happy path, idempotent path, final-component blocker
    (EEXIST), and ancestor-component blocker (ENOTDIR).

Why this happens

The reporter's Pressable repo server branch tracks wp-content/plugins/akismet as a Git symlink (mode 120000, confirmed via git ls-tree). On their Windows machine with core.symlinks=false (Git default on Windows), git checkout materialized that entry as a 41-byte regular file containing the symlink target text. The next Studio Sync pull failed in the Jetpack importer:

EEXIST: file already exists, mkdir '...wp-content\plugins\akismet'

mkdir(recursive:true) treats EEXIST as a no-op only when the existing path is itself a directory. A regular file at that path makes mkdir throw and the import abort.

A related case the helper also handles: when mkdir is called on a path below the blocker (e.g. …/akismet/_inc while …/akismet is the file), Node raises ENOTDIR instead.

When mkdir fails, the helper recurses upward: it ensures the parent directory exists first, then stats the leaf — if it exists and isn't a directory, move it to trash before retrying mkdir. Each level only has to decide whether its own path is a blocker, because the recursion guarantees the parent is already a directory by the time control returns. Trash (rather than unlink) is used so that — in the unlikely event the blocker is something the user values rather than a checkout artifact — the file is recoverable, matching the precedent set by moveExistingDatabaseToTrash in the same file.

Testing Instructions

1. Unit tests

npm test -- apps/cli/lib/import-export/import/importers/tests/importer.test.ts

Four tests pass.

2. End-to-end Sync pull

  1. Connect a Pressable site to Studio and run an initial Sync pull. Note the local site path, e.g. ~/Studio/my-site.

  2. Replace the akismet plugin directory with a regular file:

    macOS / Linux:

    SITE=~/Studio/my-site   # adjust to your site path
    rm -rf "$SITE/wp-content/plugins/akismet"
    printf '/managed/akismet' > "$SITE/wp-content/plugins/akismet"

    Windows (PowerShell):

    $Site = "$HOME\Studio\my-site"   # adjust to your site path
    Remove-Item -Recurse -Force "$Site\wp-content\plugins\akismet" -ErrorAction Ignore
    '/managed/akismet' | Set-Content -LiteralPath "$Site\wp-content\plugins\akismet" -NoNewline
  3. Trigger another Sync pull in the Studio UI.

Pull should succeed.

Against the unpatched trunk build, the pull fails with EEXIST: file already exists, mkdir '...wp-content/plugins/akismet'.

Pre-merge Checklist

  • No new TypeScript errors in wp-studio workspace; ESLint clean on modified files
  • Unit tests pass (4/4 new, 9/9 in apps/cli/lib/import-export)
  • End-to-end Sync pull verified with the fix applied
  • Have you checked for TypeScript, React or other console errors?

When a Pressable site is checked out via Git on Windows with core.symlinks=false,
tracked symlinks (e.g. wp-content/plugins/akismet) materialize as small regular
files containing the symlink target as text. The Jetpack importer's subsequent
mkdir(...recursive:true) on that path throws EEXIST because the path exists and
is not a directory, aborting the Sync pull.

ensureDir() unlinks a non-directory blocking the target before retrying. Happy
paths are unchanged.
mkdir with recursive:true is documented to throw EEXIST only when the path
exists and is not a directory, so the post-catch lstat was guarding an
impossible case.
mkdir(recursive:true) throws ENOTDIR (not EEXIST) when the blocker is an
intermediate path component, e.g. when the importer copies a deep file
like wp-content/plugins/akismet/_inc/akismet.css before a top-level file
in the same plugin. Catch both codes, require error.path, and warn about
the removed blocker so production logs reflect when the recovery fires.

Adds a regression test for the ancestor-blocker case.
Node populates error.path differently on Linux (the original mkdir target)
versus Windows (the actual non-directory blocker). The previous code
unlinked error.path and worked only on Windows; on Linux the unlink hit
the same ENOTDIR. Replace it with a directed lstat walk-up from dir to
locate the real blocker, then unlink that.
@ivan-ottinger ivan-ottinger requested a review from Copilot May 20, 2026 13:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the CLI import path by handling cases where a non-directory file (often from Windows Git symlink checkouts) blocks creation of directories under wp-content, preventing mkdir({ recursive: true }) from throwing EEXIST/ENOTDIR and aborting the import.

Changes:

  • Add ensureDir(dir) helper that retries mkdir(recursive:true) after locating and unlinking a non-directory path component blocker.
  • Use ensureDir when preparing destination directories during wp-content import.
  • Add Vitest unit coverage for creation, idempotency, and both “target blocker” and “ancestor blocker” scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
apps/cli/lib/import-export/import/importers/importer.ts Introduces ensureDir + blocker discovery and uses it in importWpContent before copying files.
apps/cli/lib/import-export/import/importers/tests/importer.test.ts Adds unit tests validating ensureDir behavior across common blocker cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/cli/lib/import-export/import/importers/importer.ts
Comment thread apps/cli/lib/import-export/import/importers/importer.ts Outdated
Pass the destination wp-content directory to ensureDir and refuse to
unlink any blocker that resolves outside of it. Without this guard, a
malformed archive entry that escapes the destination tree (e.g. via
`..` segments) could cause an unrelated file on disk to be unlinked
before mkdir retries.
The previous bottom-up walk had to special-case ENOTDIR on lstat to
keep climbing through a file blocker on macOS (where lstat below a
file throws ENOTDIR rather than ENOENT). Walking top-down sidesteps
the quirk — we never stat below a file because we stop at the first
non-directory.

Use stat (not lstat) so that symlinks to directories, which mkdir
traverses without complaint, are recognized as directories rather
than flagged as blockers.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread apps/cli/lib/import-export/import/importers/importer.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@ivan-ottinger ivan-ottinger marked this pull request as ready for review May 21, 2026 08:37
@ivan-ottinger ivan-ottinger requested a review from a team May 21, 2026 09:38
@wpmobilebot
Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing 76cc459 vs trunk

app-size

Metric trunk 76cc459 Diff Change
App Size (Mac) 1338.49 MB 1338.49 MB +0.00 MB ⚪ 0.0%

site-editor

Metric trunk 76cc459 Diff Change
load 1652 ms 1635 ms 17 ms ⚪ 0.0%

site-startup

Metric trunk 76cc459 Diff Change
siteCreation 8594 ms 9597 ms +1003 ms 🔴 11.7%
siteStartup 4956 ms 4955 ms 1 ms ⚪ 0.0%

Results are median values from multiple test runs.

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

Copy link
Copy Markdown
Contributor

@gavande1 gavande1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for debugging and fixing this issue! The sync works correctly on this branch while fails on trunk. LGTM 👍

CleanShot 2026-05-21 at 18 44 40@2x

@ivan-ottinger ivan-ottinger merged commit 603c82c into trunk May 21, 2026
10 checks passed
@ivan-ottinger ivan-ottinger deleted the fix-import-eexist-symlink-stub branch May 21, 2026 13:27
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.

4 participants