feat: 🎸 implement Node-compatible cp module

[streamich]

No description provided.

[Copilot]

🟡 Changes recommended

Multiple unresolved critical and moderate correctness and API issues remain in cp.ts and FsPromises.ts.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Implements a shared Node-compatible cp and copyFile engine for synchronous, callback, and promise APIs.

Changes:

• Adds recursive copying, filtering, symlink handling, validation, timestamps, and error mapping.
• Integrates the implementation with Volume and FsPromises.
• Updates copy-related tests and edge-case coverage.

File summaries

File	Summary and final findings
packages/fs-node/src/volume.ts	Integrates the new copy APIs.
packages/fs-node/src/FsPromises.ts	Line 167, moderate (3 votes): make cp options optional. Line 172, moderate (3 votes): make copyFile flags optional.
packages/fs-node/src/cp.ts	Unresolved findings: line 230, critical (3 votes), incorrect dereference preflight; line 213, moderate (1 vote), identity checks ignore dereference mode; line 348, moderate (1 vote), synchronous recursion lacks child preflight; lines 372 and 552, moderate (3 votes) and critical (3 votes), symlink replacement does not unlink existing destinations; line 324, moderate (1 vote), synchronous errorOnExist is ignored; line 566, moderate (1 vote), incorrect symlink-to-subdirectory check; line 236, moderate (2 votes), non-ENOENT destination errors are swallowed; line 267, moderate (1 vote), parent-creation errors are swallowed; line 138, critical (1 vote), forced clone validation can delete an existing destination; line 258, critical (1 vote), root-source recursion is allowed; line 96, moderate (1 vote), explicit undefined options overwrite defaults; line 408, nit (1 vote), async filter promise support is missing from the public type.
packages/fs-node/src/__tests__/volume/promises-cp.test.ts	Updates promise API copy coverage.
packages/fs-node/src/__tests__/volume/cpSync.test.ts	Updates synchronous copy coverage.
packages/fs-node/src/__tests__/volume/cp.test.ts	Updates callback copy coverage.
packages/fs-node/src/__tests__/volume/cp-edge-cases.test.ts	Adds and updates symlink and edge-case coverage.

Review details

Suppressed comments (7)

packages/fs-node/src/cp.ts:217

• identicalPaths always uses statSync, even when cp is preserving symlinks. checkPathsSync uses it both for the src/dest identity check and for detecting a destination parent under the source, so distinct symlinks to the same target (or a symlink copied into its target directory) are incorrectly rejected with EINVAL when dereference is false. Make this helper honor the selected dereference mode or use the already-selected stats.

const identicalPaths = (vol: Volume, a: string, b: string): boolean => {
  try {
    const statA = vol.statSync(a, { throwIfNoEntry: false }) as Stats | undefined;
    if (!statA) return false;
    const statB = vol.statSync(b, { throwIfNoEntry: false }) as Stats | undefined;

packages/fs-node/src/cp.ts:348

• The synchronous recursive path calls getStatsSync directly for children, unlike the async path's checkPaths. Consequently, a nested file/directory type conflict takes the wrong operation (for example, tries to unlink a directory), and nested FIFO/socket/unknown entries are silently skipped instead of producing the cp error. Run the same path/type preflight for each child before copying it.

      getStatsSync(vol, srcItem, destItem, options);

packages/fs-node/src/cp.ts:325

• Unlike the async onDir, this branch never checks errorOnExist when the destination directory already exists. Therefore cpSync(src, dest, { recursive: true, force: false, errorOnExist: true }) silently merges into dest instead of throwing ERR_FS_CP_EEXIST, including for existing nested directories. Mirror the async destination check before recursing.

  if (!destStat) copyDirSync(vol, src, dest, options, true, Number(srcStat.mode));
  else copyDirSync(vol, src, dest, options, false, undefined);

packages/fs-node/src/cp.ts:566

• This check is testing the source target, but the ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY condition is about whether the existing destination is a directory. Besides rejecting the wrong topology, statOf(vol, src, true) makes copying a dangling source symlink over an existing symlink fail with ENOENT even though dereference defaults to false. Use the destination directory check, matching onLinkSync.

  if ((statOf(vol, src, true) as Stats).isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc))

packages/fs-node/src/cp.ts:269

• This catch discards failures while creating the destination parent. Permission and ENOTDIR errors are therefore hidden and the later copy can report a misleading error (or mutate a partial tree) instead of the parent-creation failure. Let mkdirSync propagate non-ENOENT errors.

  try {
    if (!destStat || !vol.existsSync(destParent)) vol.mkdirSync(destParent, { recursive: true });
  } catch {}

packages/fs-node/src/cp.ts:96

• Spreading options over the defaults lets an explicitly supplied undefined overwrite each boolean default, after which the validators reject calls such as { recursive: undefined }. Node treats an undefined option property as omitted, and the previous implementation used nullish defaults; normalize undefined values before validating.

  const resolved: ICpOptionsResolved = { ...CP_DEFAULTS, ...options } as ICpOptionsResolved;

packages/fs-node/src/cp.ts:408

• The async implementation awaits filter, so it supports a Promise-returning filter at runtime, but the public ICpOptions.filter type still only returns boolean (fs-node-utils/src/types/options.ts:193). TypeScript consumers cannot use this new async behavior without a cast; widen the async-facing option type or provide separate sync and async option types.

  if (filter && !(await filter(src, dest))) return undefined;

• Files reviewed: 7/7 changed files
• Comments generated: 8
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved correctness issues remain in source validation, symlink handling, error handling, and recursive copy safeguards.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (10)

Previously missed (1) — in code that hasn't changed since the last review.

packages/fs-node/src/cp.ts:209

• Relative symlink targets are resolved with posix.resolve's host cwd. Volume operations resolve relative paths against the instance's Superblock.process.cwd(), so a volume with a custom cwd can create an absolute link target outside the virtual filesystem when copying a relative source link.

packages/fs-node/src/cp.ts:230

• !dereference reverses the source stat mode: with dereference: true this still lstat's the symlink. The subsequent type checks can classify a symlink-to-file as a directory and reject or overwrite the wrong destination; the async path passes dereference directly.

    srcStat = statOf(vol, src, !dereference);

packages/fs-node/src/cp.ts:244

• identicalPaths always follows symlinks, so sync cp treats distinct symlinks (or a source file and a destination symlink to it) as the same path even when dereference is false. Valid symlink copies then fail; compare the already selected srcStat and destStat as the async path does.

    if (identicalPaths(vol, srcPath, destPath))
      throw cpCodeError('ERR_FS_CP_EINVAL', 'src and dest cannot be the same ' + destPath);

packages/fs-node/src/cp.ts:238

• Unlike statOrNull used by async cp, this catch suppresses every destination stat error, including EACCES and ENOTDIR. The sync path then proceeds as if the destination were absent, potentially creating parents or reporting a later unrelated error; only ENOENT should be treated as missing.

  let destStat: Stats | undefined;
  try {
    destStat = statOf(vol, dest, dereference);
  } catch {}

packages/fs-node/src/cp.ts:348

• Recursive synchronous copies skip checkPathsSync for children, unlike the async loop. Child type conflicts, self-equivalence, and special files are therefore not validated and can produce ENOTDIR or silently skip entries instead of the corresponding cp errors; validate each child before copying it.

      getStatsSync(vol, srcItem, destItem, options);

packages/fs-node/src/cp.ts:325

• When the destination directory already exists, the sync path merges into it without checking errorOnExist/force, while onDir throws when errorOnExist && !force. This makes cpSync disagree with the async APIs and ignore errorOnExist for directory destinations.

  if (!destStat) copyDirSync(vol, src, dest, options, true, Number(srcStat.mode));
  else copyDirSync(vol, src, dest, options, false, undefined);

packages/fs-node/src/cp.ts:373

• For an existing non-symlink destination, readlinkSync returns EINVAL and this branch calls symlinkSync without removing the file, so the default force: true behavior fails with EEXIST instead of replacing it. This path also does not apply the force/errorOnExist policy used for regular files.

    if ((error as ErrnoException).code === 'EINVAL') {
      vol.symlinkSync(resolvedSrc, dest);
      return;

packages/fs-node/src/cp.ts:553

• The async symlink path has the same overwrite bug: an existing regular destination reaches this EINVAL branch and symlinkSync fails with EEXIST even though force defaults to true. The link replacement must apply the same destination policy as file copies.

  } catch (error) {
    if ((error as ErrnoException).code === 'EINVAL') {
      vol.symlinkSync(resolvedSrc, dest);
      return;

packages/fs-node/src/cp.ts:408

• checkPaths awaits filter, but the shared ICpOptions.filter type still returns only boolean. Promise-based filters supported by promises.cp therefore cannot be expressed by TypeScript callers; widen the public option type to boolean | Promise<boolean> while retaining the sync rejection for promise results.

  const filter = options.filter;
  if (filter && !(await filter(src, dest))) return undefined;

packages/fs-node/src/cp.ts:566

• This unconditional statOf(..., true) makes copying a dangling source symlink fail with ENOENT whenever the destination is an existing symlink, even though dangling links are valid to copy with dereference: false. Use the existing non-throwing directory probe so the self-subdirectory check is skipped when the target does not exist.

  if ((statOf(vol, src, true) as Stats).isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc))

• Files reviewed: 7/7 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Multiple moderate correctness issues remain in path resolution, overwrite handling, recursion, symlink behavior, and timestamp preservation.

Review details

Suppressed comments (17)

Previously missed (3) — in code that hasn't changed since the last review.

packages/fs-node/src/cp.ts:15

• posix.resolve uses the host process cwd for relative paths, while Volume/Superblock resolves relative virtual paths against its injected process.cwd(). With a custom virtual cwd, relative cp paths can miss the self-subdirectory check, and relative symlink targets can be rewritten to a host path. Thread the volume's cwd through these path calculations instead of using the global resolver base.
  packages/fs-node/src/cp.ts:103
• Spreading options over the defaults lets an explicitly undefined property overwrite its default, after which validateBoolean rejects it. For example, { recursive: undefined } used to behave like omitted options but now throws; resolve optional fields as omitted when their value is undefined before validating.
  packages/fs-node/src/cp.ts:404
• These lines unconditionally unlink and recreate an existing symlink, ignoring force and errorOnExist. With { force: false }, cp should leave an existing destination unchanged, and with errorOnExist: true it should report the existing-target error; this path overwrites it instead.

This issue also appears on line 591 of the same file.

packages/fs-node/src/tests/volume/cp-edge-cases.test.ts:23

• Adding recursive: true to this file-symlink case weakens the regression test: dereferencing a symlink to a regular file must not require the directory-only recursive option. Keep this call without recursive so it continues to exercise the normal file-copy path.

      vol.cpSync('/link.txt', '/copy.txt', { dereference: true, recursive: true });

packages/fs-node/src/cp.ts:251

• Passing !dereference reverses the intended stat/lstat choice: with the default dereference: false, a symlink is followed here, so a dangling link fails and a link to a directory is treated as a directory; with dereference: true, the source is lstated instead. This makes synchronous cp diverge from the async path and Node behavior. Pass dereference directly.

    srcStat = statOf(vol, src, !dereference);

packages/fs-node/src/cp.ts:369

• Recursive synchronous copies call getStatsSync directly for each child, bypassing checkPathsSync. Nested type conflicts and self/alias checks are therefore skipped; for example, copying a source file over an existing destination directory with force: false silently leaves the wrong destination instead of raising Node's type error. Run the same path checks for each entry, as the async traversal does.

      if (filter && !filterSync(filter, srcItem, destItem)) continue;
      getStatsSync(vol, srcItem, destItem, options);

packages/fs-node/src/cp.ts:314

• After directory traversal reaches a child, this function has no terminal branch for FIFO, socket, or other unknown types, so the synchronous implementation silently skips them. checkPathsSync only validates the top-level source, while the async implementation rejects these types for every item. Add the corresponding terminal errors here.

  if (srcStat.isSymbolicLink()) return onLinkSync(destStat, vol, src, dest, options.verbatimSymlinks);
};

packages/fs-node/src/cp.ts:394

• When dest is an existing regular file, readlinkSync throws EINVAL and this branch attempts to create the replacement symlink without removing that file. symlinkSync then fails with EEXIST even though force defaults to true, so a symlink cannot overwrite a regular destination. Remove the existing non-symlink destination after applying the force/errorOnExist policy.

    if ((error as ErrnoException).code === 'EINVAL') {
      vol.symlinkSync(resolvedSrc, dest);
      return;

packages/fs-node/src/cp.ts:152

• The cleanup path unlinks dest whenever the copy fails, but dest may have existed before core.open because the open uses O_CREAT for both new and existing destinations. A COPYFILE_FICLONE_FORCE/ENOSYS or source-directory failure can therefore delete a pre-existing destination instead of preserving it. Track whether the destination was newly created and only remove newly created outputs.

      if (!copied)
        try {
          core.unlink(dest);
        } catch {}

packages/fs-node/src/cp.ts:570

• The async symlink-copy path has the same overwrite bug: for an existing regular destination, readlinkSync returns EINVAL and the code calls symlinkSync without unlinking the destination, so the default force: true operation fails with EEXIST. Remove the existing non-symlink destination after applying the overwrite policy.

    if ((error as ErrnoException).code === 'EINVAL') {
      vol.symlinkSync(resolvedSrc, dest);
      return;

packages/fs-node/src/cp.ts:592

• The async path unconditionally unlinks and recreates an existing symlink, so it ignores options.force and options.errorOnExist. { force: false } should leave an existing destination unchanged, while errorOnExist should reject it; this branch overwrites it instead.

  vol.unlinkSync(dest);
  vol.symlinkSync(resolvedSrc, dest);

packages/fs-node/src/cp.ts:133

• When the source and destination resolve to the same inode, copyFile should fail with EINVAL (including for hard-link aliases), but this branch reports success. Throw the same-file error while keeping copied = true so the cleanup path does not unlink the source.

      if (destNode === srcNode) {
        copied = true;
        return;

packages/fs-node/src/cp.ts:139

• These failure checks run after the destination has been opened and, for an existing file, truncated. A direct copyFileSync of a directory or with COPYFILE_FICLONE_FORCE can therefore destroy an existing destination before reporting the error, and the finally block then unlinks it as well. Validate these conditions before opening the destination or only clean up a destination created by this call.

    if (flags & COPYFILE_FICLONE_FORCE) throw createError(ERROR_CODE.ENOSYS, 'copyfile', src, dest);
    if (srcNode.isDirectory()) throw createError(ERROR_CODE.EISDIR, 'copyfile', src, dest);

packages/fs-node/src/cp.ts:264

• identicalPaths follows both paths even when dereference is false. This makes copying a regular file over a symlink to that file, or one same-target symlink over another, fail as ERR_FS_CP_EINVAL instead of treating the destination link as a separate entry; compare srcStat and destStat for the non-dereferencing case, as the async path does.

    if (identicalPaths(vol, srcPath, destPath))

packages/fs-node/src/cp.ts:583

• The second self-subdirectory guard stats src with statOf(..., true) instead of checking the resolved destination target, unlike the sync handler. Besides missing the destination-directory case, it throws for a dangling source symlink because the follow-stat fails; use the destination's resolved directory check here.

  if ((statOf(vol, src, true) as Stats).isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc))

packages/fs-node/src/cp.ts:334

• copyFileCore calls srcNode.getBuffer() while copying, and Node.getBuffer() updates the source atime. Re-statting src here therefore captures the post-copy access time, so preserveTimestamps does not preserve the original source timestamps; use the srcStat already captured before the copy.

  if (options.preserveTimestamps) setDestTimestamps(vol, src, dest);

packages/fs-node/src/cp.ts:519

• As in the sync path, copyFileCore updates the source atime before this helper re-stats it. The async preserveTimestamps option consequently copies the new access time rather than the pre-copy value held by srcStat; apply the timestamps from that captured stat.

  if (options.preserveTimestamps) setDestTimestamps(vol, src, dest);

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Lite