fix(install): recover from non-directory entries blocking .bit_roots links#10355
Open
zkochan wants to merge 5 commits intoteambit:masterfrom
Open
fix(install): recover from non-directory entries blocking .bit_roots links#10355zkochan wants to merge 5 commits intoteambit:masterfrom
zkochan wants to merge 5 commits intoteambit:masterfrom
Conversation
…links
`hardLinkDirectory` is invoked during post-install linking into
`node_modules/.bit_roots/<env>/...`. If a previous install was interrupted
or the env layout drifted across versions, an ancestor directory in the
target path can be left behind as a regular file or a dangling symlink.
That made `mkdir(... { recursive: true })` throw `ENOTDIR` (or `ENOENT`
through a broken symlink) and aborted the whole install with no clear
remediation other than `rm -rf node_modules/.bit_roots`.
Detect this case, remove the offending non-directory entry, retry the
mkdir, and surface a warning. The destination tree under `.bit_roots` is
owned by bit and rebuilt on every install, so deleting a stray entry is
safe.
Use printWarning + logger.warn from @teambit/legacy.logger as the default onWarn for hardLinkDirectory, so the recovery message both surfaces in the CLI (yellow "Warning: …", honoring no_warnings config) and lands in debug.log alongside the install context. Tests still inject their own collector, so this stays unit-testable without touching the global logger.
Always go through bit's logger (logger.warn + printWarning). The option existed only to keep tests from depending on the global logger, but the recovery contract is sufficiently verified by asserting the file gets linked through — the warning is a side effect, not the behavior under test.
Centralize the cast to NodeJS.ErrnoException in a tiny helper so each catch site stays an unknown without sprinkling `as any` around.
The blocking entry could be high up the path (a stray file at @scope, or even at node_modules itself in a weird state) and we don't want to discard the user's data on a heuristic. Rename it to <offender>.bit-stray-<ts> alongside, surface that path in the warning, and let the user inspect or remove it themselves. The retry mkdir then succeeds because the original name is free.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens hardLinkDirectory() against corrupted destination paths (e.g., where an expected directory in the destination tree is instead a regular file or a dangling symlink), by quarantining the blocking entry and retrying directory creation so installs/linking can proceed without manual cleanup.
Changes:
- Introduce
ensureDir()+ helpers to recover frommkdir(..., { recursive: true })failures caused by non-directory path entries, by renaming the blocking entry aside and retrying. - Update
hardLinkDirectory()/linkFile()to route directory creation throughensureDir()and normalize errno handling viaerrnoCode(). - Add unit tests covering recovery when destination ancestors/targets are regular files or dangling symlinks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scopes/toolbox/fs/hard-link-directory/hard-link-directory.ts | Adds self-healing directory creation (ensureDir) and wires it into linking flow; adds warnings via legacy logger. |
| scopes/toolbox/fs/hard-link-directory/hard-link-directory.spec.ts | Adds regression tests for recovery/quarantine behavior under corrupted destination layouts. |
Comment on lines
+103
to
+125
| try { | ||
| await fs.mkdir(dir, { recursive: true }); | ||
| return; | ||
| } catch (err) { | ||
| // ENOTDIR: a regular file blocks the path. EEXIST: leaf already exists as a non-directory | ||
| // (rare with recursive: true). ENOENT: a dangling symlink in the path can't be traversed. | ||
| const code = errnoCode(err); | ||
| if (code !== 'ENOTDIR' && code !== 'EEXIST' && code !== 'ENOENT') throw err; | ||
| const offender = await findNonDirectoryAncestor(dir); | ||
| if (offender == null) { | ||
| // EEXIST with a directory already at `dir` is benign — recursive mkdir normally | ||
| // swallows it, but be defensive against races. | ||
| if (code === 'EEXIST') return; | ||
| throw err; | ||
| } | ||
| const quarantined = await quarantineStrayEntry(offender); | ||
| const msg = | ||
| `non-directory entry at ${offender} blocked link target ${dir}; ` + | ||
| `moved aside to ${quarantined} so the install could continue. inspect or delete it manually if it isn't expected.`; | ||
| logger.warn(msg); | ||
| printWarning(msg); | ||
| await fs.mkdir(dir, { recursive: true }); | ||
| } |
| import fs from 'fs-extra'; | ||
| import symlinkDir from 'symlink-dir'; | ||
| import resolveLinkTarget from 'resolve-link-target'; | ||
| import { logger, printWarning } from '@teambit/legacy.logger'; |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hardLinkDirectoryis invoked during post-install linking intonode_modules/.bit_roots/<env>/.... If a previous install was interrupted or the env layout drifted across versions (e.g. across the env-root-dir naming changes), an ancestor directory in the target path can be left behind as a regular file or a dangling symlink. That makesfs.mkdir(... { recursive: true })throwENOTDIR(orENOENTthrough a broken symlink) and aborts the whole install with no clear remediation other thanrm -rf node_modules/.bit_roots.A user just hit this on
bit install:This PR makes the linker self-heal in that situation:
ensureDirhelper. OnENOTDIR/EEXIST/ENOENTfromfs.mkdir, walk up the path withlstat, find the deepest existing ancestor that is not a directory (regular file or dangling symlink), and move it aside rather than delete it — the offender could be anywhere up the tree, so we don't want to discard the user's data on a heuristic. It gets renamed to<offender>.bit-stray-<timestamp>(with collision-bumping) andmkdiris retried. Bothmkdircall sites inhardLinkDirectory(the directory-recursion branch andlinkFile's recovery branch) go through it.logger.warn(debug.log) +printWarning(yellowWarning: …to console, honoringno_warningsconfig). The message includes both the original path and the quarantine path so the user can inspect and clean up.While in there, replaced
catch (err: any)withcatch (err)+ a smallerrnoCode(err: unknown)helper that does the singleas NodeJS.ErrnoExceptioncast.Test plan
bd test teambit.toolbox/fs/hard-link-directory— 7/7 passing, including 3 new cases that also assert the stray data is preserved at the quarantined path:bit installshape)ENOENTfrommkdir)bd compile teambit.toolbox/fs/hard-link-directory— cleannpm run lint— same pre-existing TS errors as master (all in unrelated@pnpm/*imports), no new errors in the changed files