Skip to content

Commit 907c9ca

Browse files
setoelkahficlaude
andcommitted
fix(deploy): use try_exists for prune, add cycle/root symlink tests
Review follow-ups on the dangling-symlink prune: - Swap exists() for try_exists()? in both branches so a stat error on a symlink target propagates instead of being misread as "dangling" and deleting a link we merely failed to stat. - Note in the docstring that the walk depends on pnpm's layout, where every package dir is a real directory reachable directly (so nested dangling links are always visited). - Print the success symbol on the prune line to match the other step output. - Add a cycle-safety test (a dir symlink pointing back at the root must not be followed) and a root-is-dangling-symlink test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e894c6a commit 907c9ca

1 file changed

Lines changed: 57 additions & 5 deletions

File tree

crates/cli/src/deploy/process_deploy_nextjs_ssr.rs

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,22 @@ use {
3535
/// the link is safe. See the `smbcloud-deploy-nextjs` skill for the write-up.
3636
///
3737
/// Real (non-symlink) directories are traversed; a symlinked directory is
38-
/// treated as a leaf and never followed, which also avoids symlink cycles.
38+
/// treated as a leaf and never followed, which also avoids symlink cycles. This
39+
/// relies on pnpm's layout, where every package directory is a real directory
40+
/// reachable directly in the walk (symlinks only ever point *into* it): a
41+
/// dangling link nested under a real dir is always visited and pruned. A
42+
/// dangling link reachable *only* through a valid directory symlink would be
43+
/// skipped, but rsync would follow the same valid link and hit it — that shape
44+
/// does not occur in standalone output.
3945
fn prune_dangling_symlinks(root: &std::path::Path) -> std::io::Result<usize> {
4046
let meta = std::fs::symlink_metadata(root)?;
4147

4248
if meta.file_type().is_symlink() {
4349
// A symlink at the walk root: drop it only if it dangles, never follow.
44-
if !root.exists() {
50+
// `try_exists` follows the link and reports Ok(false) only when the
51+
// target is genuinely absent; a real IO error propagates instead of
52+
// being misread as "dangling".
53+
if !root.try_exists()? {
4554
std::fs::remove_file(root)?;
4655
return Ok(1);
4756
}
@@ -61,8 +70,10 @@ fn prune_dangling_symlinks(root: &std::path::Path) -> std::io::Result<usize> {
6170
let path = entry.path();
6271

6372
if file_type.is_symlink() {
64-
// `exists()` follows the link; `false` means the target is missing.
65-
if !path.exists() {
73+
// `try_exists` follows the link; Ok(false) means the target is
74+
// missing (dangling). An IO error propagates rather than deleting a
75+
// link we merely failed to stat.
76+
if !path.try_exists()? {
6677
std::fs::remove_file(&path)?;
6778
removed += 1;
6879
}
@@ -222,7 +233,8 @@ pub async fn process_deploy_nextjs_ssr(env: Environment, config: Config) -> Resu
222233
match prune_dangling_symlinks(standalone_path) {
223234
Ok(0) => {}
224235
Ok(n) => println!(
225-
"{}",
236+
"{} {}",
237+
succeed_symbol(),
226238
succeed_message(&format!(
227239
"Pruned {} dangling symlink{} from .next/standalone before upload.",
228240
n,
@@ -826,4 +838,44 @@ mod tests {
826838
std::fs::write(root.path().join("a/b/f.txt"), "x").unwrap();
827839
assert_eq!(prune_dangling_symlinks(root.path()).unwrap(), 0);
828840
}
841+
842+
/// A valid symlink *to a directory* is treated as a leaf and never followed.
843+
/// The link here points back at the walk root, so following it would recurse
844+
/// forever; terminating (and leaving the link in place) proves it is not
845+
/// followed — the same property that keeps pnpm's symlink graph cycle-free.
846+
#[test]
847+
fn does_not_follow_valid_directory_symlink() {
848+
let root = tempdir().unwrap();
849+
let base = root.path();
850+
851+
std::fs::write(base.join("server.js"), "// server").unwrap();
852+
// A valid directory symlink that points back at the root: following it
853+
// would loop. `read_dir` reports it as a symlink, so the leaf branch
854+
// keeps it without descending.
855+
let dir_link = base.join("cycle");
856+
symlink(".", &dir_link).unwrap();
857+
858+
let removed = prune_dangling_symlinks(base).unwrap();
859+
860+
assert_eq!(removed, 0, "no dangling links, nothing removed");
861+
assert!(
862+
std::fs::symlink_metadata(&dir_link).is_ok(),
863+
"the valid directory symlink itself is preserved"
864+
);
865+
}
866+
867+
/// The walk root being a dangling symlink is handled by the top-level branch:
868+
/// it is removed and counted.
869+
#[test]
870+
fn dangling_symlink_at_root_is_removed() {
871+
let root = tempdir().unwrap();
872+
let link = root.path().join("root_link");
873+
symlink("./missing_target", &link).unwrap();
874+
875+
assert_eq!(prune_dangling_symlinks(&link).unwrap(), 1);
876+
assert!(
877+
std::fs::symlink_metadata(&link).is_err(),
878+
"the dangling root symlink is gone"
879+
);
880+
}
829881
}

0 commit comments

Comments
 (0)