Skip to content

Skip dangling symlinks when archiving site content#4038

Open
epeicher wants to merge 4 commits into
trunkfrom
fix-archive-broken-symlinks
Open

Skip dangling symlinks when archiving site content#4038
epeicher wants to merge 4 commits into
trunkfrom
fix-archive-broken-symlinks

Conversation

@epeicher

@epeicher epeicher commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Related issues

  • Related to: reprint-pulled (WP Cloud) sites shipping a dangling advanced-cache.php drop-in symlink.

How AI was used in this PR

Claude Code reproduced the failure, traced it to fs.realpathSync on a dangling symlink aborting the archive, wrote the guard and the unit test, and verified the fix end-to-end (a real preview-site creation against an affected site). I reviewed the diff.

Proposed Changes

Creating a preview site — and pushing — from a reprint-pulled WP Cloud site failed with:

Failed to create preview site: ENOENT: no such file or directory, stat '.../wp-content/advanced-cache.php'

Those sites keep the WP Cloud advanced-cache.php cache drop-in as a symlink pointing into wordpress/drop-ins/, which the pull doesn't download — so it dangles. When archiving wp-content, fs.realpathSync on that broken link threw ENOENT and aborted the whole archive.

Now a dangling symlink is skipped (with a warning) instead of failing the whole operation, so preview create/update and push succeed. This makes the archiver robust to any broken symlink — which matters for sites already pulled, since they keep the dangling drop-in until re-pulled.

The reprint side — not creating the dangling symlink in the first place — is fixed separately in WordPress/reprint.

Testing Instructions

  1. On a reprint-pulled WP Cloud site (its wp-content/advanced-cache.php is a broken symlink), rebuild the CLI (npm run cli:build) and run:
    node apps/cli/dist/cli/main.mjs preview create --path ~/Studio/<site>
  2. Before: fails with ENOENT ... advanced-cache.php. After: the preview site is created successfully.
  3. Unit tests: npm test -- apps/cli/lib/tests/archive.test.ts (includes a dangling-symlink case).

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors? (tsc -p apps/cli clean, ESLint clean)

epeicher added 2 commits July 1, 2026 19:13
Reprint-pulled WP Cloud sites keep the advanced-cache.php drop-in as a symlink whose target (wordpress/drop-ins/...) isn't pulled, so it dangles. fs.realpathSync on it threw ENOENT and aborted the whole archive, failing 'preview create'/'update' (and push export) with 'Failed to create preview site: ENOENT ... advanced-cache.php'. Skip entries whose realpath fails, in both archiveSiteContent and the default exporter.
@wpmobilebot

wpmobilebot commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing 4ab9f7a vs trunk

app-size

Metric trunk 4ab9f7a Diff Change
App Size (Mac) 1316.93 MB 1316.83 MB 0.11 MB ⚪ 0.0%

site-editor

Metric trunk 4ab9f7a Diff Change
load 1087 ms 1119 ms +32 ms ⚪ 0.0%

site-startup

Metric trunk 4ab9f7a Diff Change
siteCreation 6508 ms 6513 ms +5 ms ⚪ 0.0%
siteStartup 1862 ms 1865 ms +3 ms ⚪ 0.0%

Results are median values from multiple test runs.

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

@fredrikekelund fredrikekelund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good fix regardless of whether the site is Reprint-pulled or not 👍

One thing worth looking into is the calculateDirectorySizeForArchive() function. I'm getting a warning like this from there:

Error processing /Users/fredrik/Studio/my-true-website/wp-content/advanced-cache.php: Error: ENOENT: no such file or directory, stat '/Users/fredrik/Studio/my-true-website/wp-content/advanced-cache.php'
    at async Object.stat (node:internal/fs/promises:1038:18)
    at async file:///Users/fredrik/Code/studio/apps/cli/dist/cli/rewrite-wp-cli-post-content-wf2YZvuH.mjs:44:22
    at async Promise.all (index 0)
    at async calculateSize (file:///Users/fredrik/Code/studio/apps/cli/dist/cli/rewrite-wp-cli-post-content-wf2YZvuH.mjs:36:5) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'stat',
  path: '/Users/fredrik/Studio/my-true-website/wp-content/advanced-cache.php'

This looks like the exact same underlying problem, so it seems sensible to address in this PR

Comment thread apps/cli/lib/archive.ts Outdated
Comment on lines 60 to 70
let resolvedPath: string;
try {
resolvedPath = fs.realpathSync( path.join( wpContentPath, relativePath ) );
} catch ( error ) {
// Dangling symlink. Skip it rather than aborting the whole archive.
console.warn( `Skipping ${ archiveEntryPath }: ${ error }` );
continue;
}
archiveBuilder.file( resolvedPath, {
name: archiveEntryPath,
} );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
let resolvedPath: string;
try {
resolvedPath = fs.realpathSync( path.join( wpContentPath, relativePath ) );
} catch ( error ) {
// Dangling symlink. Skip it rather than aborting the whole archive.
console.warn( `Skipping ${ archiveEntryPath }: ${ error }` );
continue;
}
archiveBuilder.file( resolvedPath, {
name: archiveEntryPath,
} );
try {
const resolvedPath = fs.realpathSync( path.join( wpContentPath, relativePath ) );
archiveBuilder.file( resolvedPath, { name: archiveEntryPath } );
} catch ( error ) {
// Dangling symlink. Skip it rather than aborting the whole archive.
console.warn( `Skipping ${ archiveEntryPath }: ${ error }` );
}

Optional nit, but it doesn't seem necessary to call fs.realpathSync() in a different scope to archiveBuilder.file()

Comment on lines 274 to 284
let resolvedPath: string;
try {
resolvedPath = fs.realpathSync( fullEntryPathOnDisk );
} catch ( error ) {
// Dangling symlink. Skip it rather than aborting the whole archive.
console.warn( `Skipping ${ entryPathRelativeToArchiveRoot }: ${ error }` );
continue;
}
this.archiveBuilder.file( resolvedPath, {
name: entryPathRelativeToArchiveRoot,
} );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
let resolvedPath: string;
try {
resolvedPath = fs.realpathSync( fullEntryPathOnDisk );
} catch ( error ) {
// Dangling symlink. Skip it rather than aborting the whole archive.
console.warn( `Skipping ${ entryPathRelativeToArchiveRoot }: ${ error }` );
continue;
}
this.archiveBuilder.file( resolvedPath, {
name: entryPathRelativeToArchiveRoot,
} );
try {
const resolvedPath = fs.realpathSync( path.join( wpContentPath, relativePath ) );
archiveBuilder.file( resolvedPath, { name: archiveEntryPath } );
} catch ( error ) {
// Dangling symlink. Skip it rather than aborting the whole archive.
console.warn( `Skipping ${ archiveEntryPath }: ${ error }` );
}

Optional nit. Same thing here

@epeicher

epeicher commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Good fix regardless of whether the site is Reprint-pulled or not 👍

I agree, the user could create dangling symlinks and we don't want to break some flows.

One thing worth looking into is the calculateDirectorySizeForArchive() function. I'm getting a warning like this from there:
This looks like the exact same underlying problem, so it seems sensible to address in this PR

Yes, that's the same error, let me look at it

epeicher added 2 commits July 2, 2026 16:53
The push file-selection UI computes sizes via the getFileSize IPC handler and calculateDirectorySizeForArchive, both of which stat through symlinks. A dangling symlink (e.g. the advanced-cache.php drop-in on reprint-pulled sites) made getFileSize throw — logged twice with full stacks by Electron — and made the directory walk log a noisy stack trace. Since such entries are skipped when archiving, count them as zero and log a single Skipping line instead.
@epeicher

epeicher commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review @fredrikekelund, I have addressed the other functions that were causing noisy warnings. I have turned down the warning so it's still logged that the symlink is skipped, but in a controlled way. Please let me know your view on that, the commit is 4ab9f7a

@epeicher epeicher enabled auto-merge (squash) July 2, 2026 15:14
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.

3 participants