fix(installer): replace fs-extra with native fs to prevent non-deterministic file loss#1780
Conversation
🤖 Augment PR SummarySummary: Replaces Changes:
Technical Notes: This avoids 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughRemoves Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (40)
tools/cli/lib/agent-analyzer.js (2)
41-44:⚠️ Potential issue | 🟠 Major
exec.routetype is not validated before string ops.Line 43 calls
.endsWith()onexec.routewhen truthy; non-string values will crash analysis. Gate this withtypeof exec.route === 'string'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/agent-analyzer.js` around lines 41 - 44, The code calls exec.route.endsWith(...) without ensuring exec.route is a string; update the check in the agent-analyzer logic so you first verify typeof exec.route === 'string' before calling .endsWith(), and only then add profile.usedAttributes.add('workflow'); this prevents runtime errors for non-string route values while keeping the existing workflow detection behavior.
21-27:⚠️ Potential issue | 🟠 MajorMissing guard for malformed parsed YAML.
analyzeAgentObjectassumesagentYamlis an object. If parsing yieldsnull/scalar, Line 21 will throw. Add an early shape check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/agent-analyzer.js` around lines 21 - 27, analyzeAgentObject currently assumes agentYaml is an object and will throw if YAML parsing returns null or a scalar; add an early shape check at the start of analyzeAgentObject to verify agentYaml is a non-null object (and that agentYaml.agent is an object) and bail out or set safe defaults (e.g., leave profile flags false and menuItems = []) if the check fails; update code paths that set profile.hasPrompts and compute menuItems (the existing references to agentYaml.agent, profile.hasPrompts, and menuItems) to run only after this guard so malformed parsed YAML cannot cause a runtime exception.tools/cli/installers/lib/core/manifest.js (4)
156-168:⚠️ Potential issue | 🔴 Critical
update()silently ignorescustomModulesupdates.
addCustomModule()andremoveCustomModule()pass{ customModules: ... }, butupdate()never applies that field, so changes are dropped.Proposed fix
if (updates.ides) { manifest.ides = updates.ides; } + if (updates.customModules) { + manifest.customModules = updates.customModules; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest.js` around lines 156 - 168, The update() path in manifest.js currently ignores updates.customModules so calls from addCustomModule() and removeCustomModule() are dropped; modify update() to apply updates.customModules (e.g. set or merge into manifest.customModules) by checking if (updates.customModules) and updating manifest.customModules accordingly so custom module changes from addCustomModule()/removeCustomModule() persist.
941-956:⚠️ Potential issue | 🟠 MajorHTTP fallback has no timeout and no status validation.
This request can hang and blocks update checks. Add a timeout + non-200 handling before parsing payload.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest.js` around lines 941 - 956, The HTTPS lookup Promise around https.get(`https://registry.npmjs.org/${packageName}`) lacks a request timeout and doesn't validate HTTP status before JSON.parse, which can hang or parse error on non-200 responses; update the promise to set a request timeout (e.g., using req.setTimeout or a manual timer that calls req.abort()/resolve(null) after a short interval) and check res.statusCode !== 200 (or not in 2xx) in the response handler to immediately resolve(null) before attempting to accumulate/JSON.parse the body, and also ensure error and timeout paths both resolve(null) and clear any timers to avoid leaks.
22-22:⚠️ Potential issue | 🟠 MajorVersion lookup from
process.cwd()is brittle and context-dependent.Line 22 can read the wrong
package.jsonwhen CLI is invoked from another directory. UsegetProjectRoot()(already imported) for deterministic behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest.js` at line 22, The version lookup uses process.cwd() which can point to the wrong package.json; update the fallback in manifest.js so that bmadVersion uses getProjectRoot() instead of process.cwd() — i.e., replace the require(path.join(process.cwd(), 'package.json')).version reference with one that requires package.json from getProjectRoot() so bmadVersion deterministically reads the project root package.json.
933-939:⚠️ Potential issue | 🟡 MinorUse
execFileSyncwith array arguments instead of template string interpolation for the npm command.The package names passed to this function come from
external-official-modules.yaml(project-controlled configuration) and user-providedmodule.yamlfiles. While npm's strict naming rules restrict package names to lowercase letters, numbers, hyphens, underscores, dots, and forward slashes—preventing injection of shell metacharacters—usingexecFileSync('npm', ['view', packageName, 'version'], {...})follows security best practices and eliminates dependency on npm's validation as a control. Apply this for defense-in-depth.Recommended fix
- const { execSync } = require('node:child_process'); + const { execFileSync } = require('node:child_process'); // Try using npm view first (more reliable) try { - const result = execSync(`npm view ${packageName} version`, { + const result = execFileSync('npm', ['view', packageName, 'version'], { encoding: 'utf8', stdio: 'pipe', timeout: 10_000, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest.js` around lines 933 - 939, Replace the execSync call that interpolates packageName into a shell string with execFileSync invoked with the npm executable and an argument array to avoid shell interpretation; specifically, change the call that currently does execSync(`npm view ${packageName} version`, { encoding, stdio, timeout }) to execFileSync('npm', ['view', packageName, 'version'], { encoding: 'utf8', stdio: 'pipe', timeout: 10_000 }) while preserving the same options and return semantics around the packageName variable and existing try/catch in manifest.js.tools/cli/installers/lib/modules/external-manager.js (1)
29-31:⚠️ Potential issue | 🟠 MajorNull parse result can crash module listing.
If YAML is empty/invalid-but-parseable to
null,this.cachedModules = configstoresnull, then Line 46 accessesconfig.modulesand throws. Normalize parsed output before caching.Proposed fix
- const config = yaml.parse(content); - this.cachedModules = config; - return config; + const parsed = yaml.parse(content); + const config = parsed && typeof parsed === 'object' ? parsed : { modules: {} }; + if (!config.modules || typeof config.modules !== 'object') { + config.modules = {}; + } + this.cachedModules = config; + return config;Also applies to: 46-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/modules/external-manager.js` around lines 29 - 31, yaml.parse(content) can return null, which then sets this.cachedModules to null and later causes an exception when accessing config.modules; change the logic around yaml.parse(content) in external-manager.js to normalize the parsed result before caching (e.g., let config = yaml.parse(content) || {}; ensure config.modules is at least an empty array if code expects it) and assign this.cachedModules = config so subsequent accesses like config.modules are safe.tools/cli/installers/lib/core/manifest-generator.js (3)
213-216:⚠️ Potential issue | 🟡 MinorWorkflow manifest paths can contain
//for top-level files.When
relativePathis empty, Lines 215-216 generate.../workflows//workflow.yaml. Normalize this branch to avoid double separators in manifests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest-generator.js` around lines 213 - 216, The installPath construction sometimes produces a double slash when relativePath is empty (yielding .../workflows//workflow.yaml); update the logic that builds installPath (the expression using moduleName, this.bmadFolderName, relativePath and entry.name) to normalize or omit the extra separator when relativePath is empty—either by using a path-joining helper or by conditional concatenation so the path becomes .../workflows/workflow.yaml for top-level files.
1036-1038:⚠️ Potential issue | 🟠 Major
files-manifest.csvrows are written without escaping.Manual interpolation at Line 1037 breaks CSV when any value contains quotes, commas, or newlines. Reuse the same escaping helper used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest-generator.js` around lines 1036 - 1038, The CSV row construction in the loop over allFiles currently interpolates values directly into csv (csv += `"${file.type}",...`) which breaks when fields contain quotes, commas or newlines; update the loop in manifest-generator.js to call the existing CSV field-escaping helper used elsewhere (e.g., escapeCsvField or the project's CSV escape utility) for each file property (file.type, file.name, file.module, file.path, file.hash) and then join those escaped values with commas and append a newline so rows are written safely.
776-785:⚠️ Potential issue | 🟠 MajorExisting CSV parse is unguarded and can fail the entire manifest write.
If
agent-manifest.csvis malformed,csv.parsethrows and aborts generation. This should degrade gracefully (same issue pattern applies in task/tool manifest reads).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest-generator.js` around lines 776 - 785, The agent-manifest.csv read/parsing is unguarded: wrap the csv.parse call and subsequent record loop in a try/catch so a malformed CSV doesn't throw and abort generation; specifically surround the fs.readFile + csv.parse + for (const record of records) { existingEntries.set(`${record.module}:${record.name}`, record); } sequence with error handling that logs the parse error (including csvPath/agent-manifest.csv) and continues with an empty or previously accumulated existingEntries, and apply the same guarded parse pattern to the task/tool manifest reads that also call csv.parse.tools/cli/lib/config.js (3)
122-130:⚠️ Potential issue | 🟠 Major
validateConfigtrusts inputs and can throw on null/invalid args.If
configorschemais null/non-object, checks likefield in configcan throw. Add a defensive guard and return a structured validation error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/config.js` around lines 122 - 130, The validateConfig function currently assumes config and schema are objects and may throw when called with null/invalid args; add a defensive guard at the start of validateConfig to check that both config and schema are non-null objects (e.g., typeof === 'object' && config !== null), and if not return a structured validation result (errors array containing a clear message like "Invalid or missing config/schema" and empty or appropriate warnings) instead of continuing to run checks like schema.required and using `field in config`; ensure all subsequent code in validateConfig uses the validated local variables.
59-64:⚠️ Potential issue | 🟠 MajorPlaceholder replacement can mutate unintended text.
Line 59 merges raw
replacementskeys intoallReplacements; if caller passes keys likeroot, this loop replaces everyrootoccurrence globally, not just template tokens.Proposed fix
- const allReplacements = { ...standardReplacements, ...replacements }; + const allReplacements = { ...standardReplacements, ...replacements }; for (const [placeholder, value] of Object.entries(allReplacements)) { - if (typeof placeholder === 'string' && typeof value === 'string') { + if ( + typeof placeholder === 'string' && + placeholder.startsWith('{') && + placeholder.endsWith('}') && + typeof value === 'string' + ) { const regex = new RegExp(placeholder.replaceAll(/[.*+?^${}()|[\]\\]/g, String.raw`\$&`), 'g'); content = content.replace(regex, value); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/config.js` around lines 59 - 64, The replacement loop currently treats raw keys (placeholder) as plain text, causing unintended global replacements; update the loop that builds regex (involving allReplacements, standardReplacements, replacements, placeholder, and content.replace) to target only explicit template tokens instead of bare words—e.g., construct the token form you expect (such as `{{${placeholder}}}` or another delimiter used by standardReplacements), escape that token for regex, and replace only that token instance globally; ensure placeholder.replaceAll escaping remains but is applied to the token string rather than the raw placeholder so only template markers are replaced.
30-34:⚠️ Potential issue | 🔴 CriticalChange
yaml.dump()toyaml.stringify()—dumpis not available inyaml@2.7.0.Line 30 uses
yaml.dump(), but the installed version (yaml@2.7.0) provides onlyyaml.stringify(). Additionally,noRefsis not a valid option in eemeli/yaml; it's specific to js-yaml. Remove it since other stringify calls in the codebase don't use it.Proposed fix
- const yamlContent = yaml.dump(config, { + const yamlContent = yaml.stringify(config, { indent: 2, lineWidth: 120, - noRefs: true, });Note:
tools/migrate-custom-module-paths.js(line 66) has the same issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/config.js` around lines 30 - 34, Replace the unsupported yaml.dump call with yaml.stringify and remove the invalid noRefs option: update the assignment that creates yamlContent to call yaml.stringify(config, { indent: 2, lineWidth: 120 }) instead of yaml.dump(..., { indent: 2, lineWidth: 120, noRefs: true }); apply the same change to the other occurrence that uses dump (e.g., the similar assignment near the migrate flow) so all YAML serialization uses yaml.stringify with only valid options.tools/cli/lib/xml-handler.js (2)
130-132:⚠️ Potential issue | 🔴 Critical
injectActivationSimplealso dereferences undefinederror.Same runtime failure pattern here: this method throws before doing any useful work.
Safer fallback
injectActivationSimple(agentContent, metadata = {}) { - console.error('Error in simple injection:', error); + return this.injectActivation(agentContent, metadata); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/xml-handler.js` around lines 130 - 132, The function injectActivationSimple incorrectly references an undefined variable error, causing a runtime crash; update injectActivationSimple(agentContent, metadata = {}) to perform its intended injection logic and remove the stray console.error('Error in simple injection:', error) or replace it with a proper try/catch that logs the caught error (e.g., catch (err) { console.error('injectActivationSimple failed:', err) }); ensure any error variable used comes from the catch block or a passed-in parameter and keep the metadata/default handling intact.
46-48:⚠️ Potential issue | 🔴 Critical
loadActivationTemplateis broken and throws immediately.Line 47 references
errorout of scope, causingReferenceErrorevery time this method runs. This is a hard runtime failure.Minimal safe fix
async loadActivationTemplate() { - console.error('Failed to load activation template:', error); + throw new Error('loadActivationTemplate is not implemented'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/xml-handler.js` around lines 46 - 48, The loadActivationTemplate method currently references an out-of-scope variable `error`, causing a ReferenceError; wrap the template-loading logic in a try/catch inside the loadActivationTemplate function and move the console.error call into the catch block using the caught variable (e.g., catch (error) { console.error('Failed to load activation template:', error); }), or otherwise ensure the logged variable is the caught exception; keep the same function name loadActivationTemplate and preserve existing success behavior (rethrow or return a safe value as appropriate).tools/cli/installers/lib/ide/github-copilot.js (3)
581-585:⚠️ Potential issue | 🟠 MajorYAML tools list can break on unescaped custom tool names.
Custom tool names are injected into single-quoted YAML scalars without escaping
'. One bad entry can corrupt frontmatter output.Proposed fix
getToolsForFile(fileName) { const defaultTools = ['read', 'edit', 'search', 'execute']; const tools = (this.existingToolPermissions && this.existingToolPermissions.get(fileName)) || defaultTools; - return '[' + tools.map((t) => `'${t}'`).join(', ') + ']'; + return '[' + tools.map((t) => `'${this.escapeYamlSingleQuote(String(t))}'`).join(', ') + ']'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` around lines 581 - 585, The getToolsForFile method injects tool names into single-quoted YAML scalars without escaping, so any tool containing a single quote breaks the frontmatter; update getToolsForFile (in tools/cli/installers/lib/ide/github-copilot.js) to escape single quotes in each tool name before wrapping with single quotes (e.g., replace each ' with '' per YAML single-quote escaping) when building the tools list string so the returned value remains valid YAML even for custom tool names.
47-47:⚠️ Potential issue | 🟡 Minor
cleanup()is called without forwarding caller options.Line 47 drops
silent/mode flags fromsetup(), so cleanup logging and behavior can diverge from requested run mode.Proposed fix
- await this.cleanup(projectDir); + await this.cleanup(projectDir, options);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` at line 47, The call to cleanup(projectDir) inside setup() drops caller options (e.g., silent/mode flags), causing cleanup to run with wrong logging/behavior; update the setup implementation to forward the same options object/flags it received into this.cleanup (e.g., pass the options or this.options like silent/mode) so cleanup receives and honors the caller's flags; locate the setup method and the cleanup(projectDir) invocation and change it to call cleanup with the propagated options (matching cleanup's signature) so logging and behavior remain consistent.
206-210:⚠️ Potential issue | 🟠 MajorPrompt filename is not sanitized before path join.
entry.commandis used directly in a filename. Path separators or traversal tokens can escapepromptsDiror create invalid files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` around lines 206 - 210, The code uses entry.command directly to build promptFileName, allowing path separators/traversal; sanitize entry.command before producing promptFileName (e.g., use path.basename to strip dirs, then replace or whitelist characters to allow only [A-Za-z0-9._-] or run a slugify/encode routine), ensure removal of ".." and path.sep, enforce a non-empty safe fallback, then use that sanitized value when calling getToolsForFile, createWorkflowPromptContent, building promptPath with path.join(promptsDir, sanitizedName) and passing to writeFile; update variable names (promptFileName -> sanitizedPromptFileName) and add a validation step that throws or logs on invalid commands.tools/cli/lib/project-root.js (1)
35-36:⚠️ Potential issue | 🟠 MajorFallback to
process.cwd()can write to the wrong project.When discovery fails, silently returning CWD risks installing/manipulating files in an unrelated directory. This should fail fast with a clear error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/project-root.js` around lines 35 - 36, The current fallback in project-root.js that silently returns process.cwd() when project discovery fails should be changed to fail fast: update the function that currently returns process.cwd() (the module's project discovery/get-root function) to throw a clear, descriptive Error (or reject a Promise) indicating the project root could not be found instead of returning CWD; include guidance in the error text about running the command from a project directory or initializing the project, and ensure callers of this function (any code invoking the project-root module) handle the thrown error appropriately.tools/migrate-custom-module-paths.js (1)
44-52:⚠️ Potential issue | 🟡 MinorBug: Logging deleted property on line 52.
Line 50 deletes
customModule.relativePath, but line 52 attempts to log it. This will outputundefinedin the success message.🐛 Proposed fix
if (customModule.relativePath && !customModule.sourcePath) { // Convert relative path to absolute const absolutePath = path.resolve(projectRoot, customModule.relativePath); + const originalRelativePath = customModule.relativePath; customModule.sourcePath = absolutePath; // Remove the old relativePath delete customModule.relativePath; - console.log(chalk.green(` ✓ Updated ${customModule.id}: ${customModule.relativePath} → ${absolutePath}`)); + console.log(chalk.green(` ✓ Updated ${customModule.id}: ${originalRelativePath} → ${absolutePath}`)); updated = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/migrate-custom-module-paths.js` around lines 44 - 52, The success log references customModule.relativePath after it was deleted, causing "undefined"; before deleting or overwriting, capture the original relative path (e.g., store it in a variable like originalRelative = customModule.relativePath) and use that in the console.log message when reporting the update for the customModule.id and absolutePath, then delete customModule.relativePath and set customModule.sourcePath as currently done.tools/cli/commands/uninstall.js (2)
57-60:⚠️ Potential issue | 🟠 MajorGuard
bmadDirbefore callingfs.pathExists.Line 59 assumes
installer.findBmadDir(projectDir)always returns a valid path. If it returnsnull/undefined, this call can throw and bypass the intended “No BMAD installation found” path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/commands/uninstall.js` around lines 57 - 60, Check the result of installer.findBmadDir(projectDir) for null/undefined before calling fs.pathExists: if bmadDir is falsy, call prompts.log.warn('No BMAD installation found.') and return/exit early; otherwise proceed to await fs.pathExists(bmadDir). This prevents calling fs.pathExists with an invalid path and preserves the intended "No BMAD installation found" flow.
121-142:⚠️ Potential issue | 🟠 MajorSpinner cleanup is not exception-safe.
Each phase starts a spinner and stops it only on success. If Line 124/132/140 throws, the spinner is left active and output becomes messy.
Proposed pattern
if (removeIdeConfigs) { const s = await prompts.spinner(); - s.start('Removing IDE integrations...'); - await installer.uninstallIdeConfigs(projectDir, existingInstall, { silent: true }); - s.stop(`Removed IDE integrations (${ides || 'none'})`); + s.start('Removing IDE integrations...'); + try { + await installer.uninstallIdeConfigs(projectDir, existingInstall, { silent: true }); + s.stop(`Removed IDE integrations (${ides || 'none'})`); + } catch (e) { + s.stop('Failed removing IDE integrations'); + throw e; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/commands/uninstall.js` around lines 121 - 142, The spinner started via prompts.spinner() around each phase (see calls to prompts.spinner(), s.start and s.stop) is not exception-safe; if installer.uninstallIdeConfigs, installer.uninstallOutputFolder, or installer.uninstallModules throws the spinner remains active. Fix by creating the spinner before each phase and wrapping the async phase call in try/finally so s.stop(...) is always invoked (optionally with a different message on error in a catch block), e.g., for the blocks around installer.uninstallIdeConfigs, installer.uninstallOutputFolder and installer.uninstallModules ensure s.stop is called in finally so the spinner is cleaned up on success or failure.tools/cli/installers/lib/core/detector.js (2)
93-94:⚠️ Potential issue | 🟠 MajorDetection can crash on unreadable directories.
Both directory scans call
fs.readdir(...)without protectivetry/catch. A permission/transient error will throw and fail detection instead of returning partial/empty results.Also applies to: 159-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/detector.js` around lines 93 - 94, The two fs.readdir calls in detector.js (the one that populates the local variable entries at the bmadDir scan and the other at the later scan around lines 159-160) can throw on unreadable directories; wrap each fs.readdir(...) call in a try/catch and fall back to an empty array on error (optionally log a debug/warn via the same logger used in this module) so detection continues instead of crashing; update the blocks that iterate over entries (the for (const entry of entries) loops) to work with the fallback empty array.
65-67:⚠️ Potential issue | 🟠 MajorValidate
manifestData.modulesas an array before iteration.Current guard checks truthiness and
.length, then iterates. A string/object value can pass and produce garbage module records.Proposed fix
- if (manifestData && manifestData.modules && manifestData.modules.length > 0) { + if (manifestData && Array.isArray(manifestData.modules) && manifestData.modules.length > 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/detector.js` around lines 65 - 67, The loop over manifestData.modules can iterate non-array values; change the guard to explicitly verify Array.isArray(manifestData.modules) (and that length > 0) before iterating so only a real array is used. Locate the block that checks manifestData && manifestData.modules && manifestData.modules.length and replace the truthy check with an Array.isArray check for manifestData.modules to prevent strings/objects from producing invalid module records.tools/cli/installers/lib/ide/_config-driven.js (2)
320-335:⚠️ Potential issue | 🟠 MajorTemplate path inputs are not constrained to the template root.
header_template/body_templateare joined directly into filesystem paths. Values like../...can escapetemplates/splitand read arbitrary files.Proposed containment check
- const headerPath = path.join(__dirname, 'templates', 'split', headerTpl); + const splitRoot = path.join(__dirname, 'templates', 'split'); + const headerPath = path.resolve(splitRoot, headerTpl); + if (!headerPath.startsWith(splitRoot + path.sep)) { + throw new Error(`Invalid header_template path: ${headerTpl}`); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 320 - 335, The header_template/body_template values (headerTpl, bodyTpl) are joined directly to form headerPath/bodyPath allowing path traversal; fix by resolving the candidate path against the templates root (the resolved root for 'templates/split') and validate it does not escape that root (use path.resolve and either path.relative or startsWith on the resolved root) before reading; apply the same containment check for headerPath, defaultHeaderPath (when using templateType) and bodyPath, and throw/skip with a clear error if the resolved path is outside the templates root.
38-40:⚠️ Potential issue | 🟠 MajorAncestor conflict detection ignores multi-target installers.
findAncestorConflict()checks onlyinstallerConfig.target_dir. For configs usingtargets[], conflicts in ancestor directories for those targets are never checked, so duplicate command trees can still be installed.Also applies to: 570-572
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 38 - 40, The ancestor-conflict check only examines installerConfig.target_dir and therefore misses configs that declare multiple targets in installerConfig.targets; update the logic around the call to findAncestorConflict(projectDir) (and the duplicate call at the other location around lines noted) to iterate through installerConfig.targets (or normalize targets into a single array) and call findAncestorConflict(projectDir, targetDir) for each targetDir (or modify findAncestorConflict to accept a targetDir parameter and check ancestor paths for that target); ensure the check short-circuits and returns/confirms a conflict if any target reports an ancestor conflict.tools/cli/installers/lib/ide/codex.js (2)
33-35:⚠️ Potential issue | 🟠 Major
setup()does a full artifact collection twice and reports mixed counters.First pass (
collectClaudeArtifacts) is used forcounts/artifacts; second pass is used for actual writes. This can drift under filesystem changes and overstates work done in logs.Also applies to: 47-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/codex.js` around lines 33 - 35, The setup() function currently calls collectClaudeArtifacts twice which causes mismatched artifacts and counts; change setup() so it calls collectClaudeArtifacts(projectDir, bmadDir, options) only once, keep the returned { artifacts, counts } and reuse that artifacts list for the subsequent write/cleanup steps instead of re-running collection; update any logging to use the single counts value and remove the second invocation so collectClaudeArtifacts, artifacts, and counts remain the single source of truth.
52-52:⚠️ Potential issue | 🟠 MajorTasks are incorrectly module-filtered in IDE install flow.
Tasks are fetched with
selectedModulesat Lines 52 and 147. In this layer, only agents should be module-scoped; workflows/tasks are intended to be global.Based on learnings: "In the BMAD installer, for files under tools/cli/installers/lib/ide/, treat workflows and tasks as global artifacts that are always installed regardless of selected modules. Only agents are module-scoped and should be filtered by selectedModules."Proposed fix
- const tasks = await getTasksFromBmad(bmadDir, options.selectedModules || []); + const tasks = await getTasksFromBmad(bmadDir); ... - const tasks = await getTasksFromBmad(bmadDir, selectedModules); + const tasks = await getTasksFromBmad(bmadDir);Also applies to: 147-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/codex.js` at line 52, The code calls getTasksFromBmad(bmadDir, options.selectedModules || []) (at both occurrences) which incorrectly limits tasks to selected modules; change these calls to fetch tasks/workflows globally (remove the options.selectedModules argument so getTasksFromBmad is invoked without module-filtering) while ensuring only agent-related fetches still receive options.selectedModules for module scoping; update both getTasksFromBmad invocations (the ones passing options.selectedModules) to call without that second parameter so workflows/tasks are always installed.tools/cli/installers/lib/ide/shared/agent-command-generator.js (1)
97-102:⚠️ Potential issue | 🟠 MajorNested agent launchers can overwrite each other.
Lines 100-101 always write
${artifact.name}.mdunder<module>/agents. If two agents sharenamein different nested folders, one overwrites the other.Proposed fix (preserve nested path)
- const launcherPath = path.join(moduleAgentsDir, `${artifact.name}.md`); + const modulePrefix = path.join(artifact.module, 'agents') + path.sep; + const relativeInModule = + artifact.relativePath?.startsWith(modulePrefix) + ? artifact.relativePath.slice(modulePrefix.length) + : `${artifact.name}.md`; + const launcherPath = path.join(moduleAgentsDir, relativeInModule); + await fs.ensureDir(path.dirname(launcherPath)); await fs.writeFile(launcherPath, artifact.content);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/agent-command-generator.js` around lines 97 - 102, The code always writes launcher files to moduleAgentsDir using `${artifact.name}.md`, causing collisions when agents with the same name live in nested folders; change the write path to preserve the agent's nested path (e.g., use artifact.path or artifact.relativePath within the module) by building launcherPath with path.join(moduleAgentsDir, artifact.path, `${artifact.name}.md`) or path.join(moduleAgentsDir, artifact.relativePath) and ensure the parent directory exists (fs.ensureDir on path.dirname(launcherPath)) before fs.writeFile; keep using writtenCount++ after the successful write.tools/cli/installers/lib/custom/handler.js (3)
316-322:⚠️ Potential issue | 🟠 MajorEnsure customize directory exists before writing template file.
Line 321 writes
customizePathwithout creating.../_config/agents. On first-run installs this can fail with ENOENT and skip customization file creation.Proposed fix
if (!(await fs.pathExists(customizePath))) { const { getSourcePath } = require('../../../lib/project-root'); const genericTemplatePath = getSourcePath('utility', 'agent-components', 'agent.customize.template.yaml'); if (await fs.pathExists(genericTemplatePath)) { let templateContent = await fs.readFile(genericTemplatePath, 'utf8'); + await fs.ensureDir(path.dirname(customizePath)); await fs.writeFile(customizePath, templateContent, 'utf8');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/custom/handler.js` around lines 316 - 322, The code writes the customization file at customizePath without guaranteeing its parent directory exists, causing ENOENT on first-run; before calling fs.writeFile(customizePath, ...) ensure the containing directory (e.g., path.dirname(customizePath)) is created (use fs.ensureDir or fs.mkdir with { recursive: true }) after the fs.pathExists check for genericTemplatePath; keep the rest of the logic (getSourcePath/genericTemplatePath/templateContent) the same and then write the file once the directory has been ensured.
47-48:⚠️ Potential issue | 🟠 MajorPath exclusion check is overly broad and can skip valid directories.
fullPath.startsWith(exclude)will also exclude siblings like<root>/src2when<root>/srcis excluded.Proposed boundary-safe check
- if (excludePaths.some((exclude) => fullPath.startsWith(exclude))) { + if (excludePaths.some((exclude) => fullPath === exclude || fullPath.startsWith(exclude + path.sep))) { continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/custom/handler.js` around lines 47 - 48, The exclusion check using fullPath.startsWith(exclude) is too broad and will wrongly exclude siblings (e.g., /root/src2 when /root/src is excluded); update the .some callback that checks excludePaths so it only matches exact directory boundaries: resolve both exclude and fullPath (use path.resolve) and then test either fullPath === exclude OR fullPath.startsWith(exclude + path.sep), or use path.relative(exclude, fullPath) and ensure the result does not start with '..' (and is not equal to '..') to confirm fullPath is inside exclude; apply this change to the callback that references excludePaths and fullPath in handler.js.
89-112:⚠️ Potential issue | 🟠 MajorValidate YAML parse result before reading fields.
After
yaml.parse, code assumes an object and accessesconfig.code,config.name, etc. Non-object YAML (null/string/list) will throw and turn parse issues into generic read errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/custom/handler.js` around lines 89 - 112, After parsing YAML into config, ensure config is an object before reading fields: check that config is not null, is of type 'object', and not an Array (validate in the handler where yaml.parse is called and before accessing config.code, config.name, config.description, config.default_selected). If the validation fails, log a clear warning (include configPath and the type/value found) and return null (same failure path as parse errors) so non-object YAML (null/string/list) does not cause runtime exceptions when building the returned object.tools/cli/installers/lib/ide/rovodev.js (2)
163-170:⚠️ Potential issue | 🟠 MajorCleanup has unhandled directory-read failures.
fs.readdir(workflowsPath)is not wrapped intry/catch. Permission or transient FS errors will abort cleanup instead of best-effort removal.Also applies to: 205-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/rovodev.js` around lines 163 - 170, The directory read can throw and abort cleanup; wrap the fs.readdir(workflowsPath) call in a try/catch so permission/transient errors are caught and do not stop best-effort removal: inside the block where you check this.pathExists(workflowsPath) (and the similar block around the second occurrence), try to await fs.readdir(workflowsPath) in a try/catch, log or ignore the error and return/continue gracefully, and only iterate/remove entries (using fs.remove(path.join(workflowsPath, entry))) when readdir succeeded; this ensures bmad-* entries are still removed when possible without crashing on readdir failures.
99-108:⚠️ Potential issue | 🟠 Major
prompts.ymlcan receive duplicatenameentries.
_collectPromptEntries()pushes flattened names blindly. If two artifacts normalize to the same dash name,prompts.ymlgets duplicate command names and ambiguous behavior.Also applies to: 138-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/rovodev.js` around lines 99 - 108, The code in _collectPromptEntries creates duplicate prompt names by deriving name from toDashPath(artifact.relativePath) without checking uniqueness; update _collectPromptEntries to detect name collisions before pushing to writtenFiles (use a Set or map of existing names) and, when a collision occurs, generate a deterministic unique name (e.g., append a numeric suffix like -1, -2 or include more of the relative path) and use that as the name while keeping description/contentFile unchanged; apply the same deduplication change to the other identical prompt-collection logic referenced in the file so prompts.yml cannot receive duplicate name entries.tools/cli/installers/lib/ide/platform-codes.js (1)
22-24:⚠️ Potential issue | 🟠 MajorValidate parsed YAML shape before dereferencing
config.platforms.Line 23 accepts any YAML value; Lines 46/58/69/80 assume
config.platformsis an object. A malformed or scalar YAML will throw at runtime.Proposed hardening
const content = await fs.readFile(PLATFORM_CODES_PATH, 'utf8'); _cachedPlatformCodes = yaml.parse(content); + if ( + !_cachedPlatformCodes || + typeof _cachedPlatformCodes !== 'object' || + !_cachedPlatformCodes.platforms || + typeof _cachedPlatformCodes.platforms !== 'object' + ) { + throw new Error(`Invalid platform codes configuration at: ${PLATFORM_CODES_PATH}`); + } return _cachedPlatformCodes;Also applies to: 45-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/platform-codes.js` around lines 22 - 24, After parsing PLATFORM_CODES_PATH into _cachedPlatformCodes via yaml.parse, validate that the parse result is a non-null plain object and that config.platforms (or _cachedPlatformCodes.platforms) exists and is an object before any dereference; if the shape is invalid, either throw a clear error or normalize to a safe default (e.g., _cachedPlatformCodes = { platforms: {} }) so downstream code that accesses config.platforms (used around the checks at lines referencing config.platforms) won't throw; implement this check immediately after yaml.parse and before returning _cachedPlatformCodes, referencing the symbols _cachedPlatformCodes, yaml.parse, and config.platforms in the change.tools/cli/installers/lib/modules/manager.js (5)
1009-1027:⚠️ Potential issue | 🔴 Critical
isUpdatecheck occurs after the file is written, so it will always betrue.Line 1024 checks if
targetMdPathexists to determine if this is an update. However, the file was already written at line 1009 withfs.writeFile(targetMdPath, xml, 'utf8'). This meansisUpdatewill always evaluate totrue, defeating the purpose of the update-safe handling logic incopySidecarToMemory.The check should occur before writing the file:
🐛 Proposed fix
+ // Determine if this is an update BEFORE writing (check if agent already exists) + const isUpdate = await fs.pathExists(targetMdPath); + // Compile with customizations if any const { xml } = await compileAgent(yamlContent, answers, agentName, relativePath, { config: this.coreConfig || {} }); // Write the compiled agent await fs.writeFile(targetMdPath, xml, 'utf8'); // Handle sidecar copying if present if (hasSidecar) { // Get the agent's directory to look for sidecar const agentDir = path.dirname(agentFile); const sidecarDirName = `${agentName}-sidecar`; const sourceSidecarPath = path.join(agentDir, sidecarDirName); // Check if sidecar directory exists if (await fs.pathExists(sourceSidecarPath)) { // Memory is always in _bmad/_memory const bmadMemoryPath = path.join(bmadDir, '_memory'); - // Determine if this is an update (by checking if agent already exists) - const isUpdate = await fs.pathExists(targetMdPath); - // Copy sidecar to memory location with update-safe handling const copiedFiles = await this.copySidecarToMemory(sourceSidecarPath, agentName, bmadMemoryPath, isUpdate, bmadDir, installer);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/modules/manager.js` around lines 1009 - 1027, The isUpdate detection is done after writing the agent metadata (fs.writeFile to targetMdPath), so isUpdate will always be true; move the existence check (isUpdate = await fs.pathExists(targetMdPath)) to before the fs.writeFile call, compute isUpdate using the pre-write state, then call copySidecarToMemory with that value and proceed to write the file (or adjust order so copySidecarToMemory receives the correct pre-existing flag); reference variables/functions: targetMdPath, fs.writeFile, isUpdate, copySidecarToMemory, sourceSidecarPath, agentFile, bmadDir, installer.
1095-1113:⚠️ Potential issue | 🟠 MajorDead code:
processAgentFilesfunction body is entirely commented out.The entire implementation of
processAgentFilesis commented out, leaving an empty function. This is still called frominstall()(line 586) andupdate()(line 645). Either remove the dead code and the calls, or restore the functionality if it's still needed.Leaving large blocks of commented code creates maintenance burden and confusion about intended behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/modules/manager.js` around lines 1095 - 1113, The processAgentFiles function currently contains only commented-out code, leaving it inert while install() and update() still call it; restore its intended behavior or remove both the dead code and caller invocations. Either (A) uncomment and re-enable the original logic inside processAgentFiles (use path.join, fs.pathExists, this.findAgentMdFiles, loop over agent files, check for '.md', read/write with fs, and call this.xmlHandler.injectActivationSimple when content contains '<agent' but not '<activation') so callers keep working, or (B) remove the commented block entirely and update install() and update() to stop calling processAgentFiles (and delete the empty method) to avoid no-op calls and confusion. Ensure references to processAgentFiles, install, update, this.findAgentMdFiles, and this.xmlHandler.injectActivationSimple are updated consistently.
570-573:⚠️ Potential issue | 🟠 MajorSilent removal before install could lose user data without warning.
When reinstalling a module, if
targetPathexists, it's silently removed at line 572 before copying. If the user had modified files in that module directory, those modifications are lost without any warning or backup.Consider either:
- Warning the user before removal
- Creating a backup
- Using the same update-safe logic used elsewhere in this file
// Check if already installed if (await fs.pathExists(targetPath)) { await fs.remove(targetPath); // User modifications silently lost }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/modules/manager.js` around lines 570 - 573, The code silently removes an existing installation by calling fs.remove when fs.pathExists(targetPath) is true, which can destroy user changes; change this to either prompt the user before deletion, create a timestamped backup of targetPath (e.g., move/rename to targetPath + '.bak.' + timestamp) before removing, or invoke the existing update-safe logic used elsewhere in this module instead of directly calling fs.remove; update the block that currently checks fs.pathExists(targetPath) to perform one of these safer actions and ensure subsequent code operates on the new path or backup accordingly.
876-880:⚠️ Potential issue | 🟡 MinorSilent fallback masks errors in YAML processing.
When YAML processing fails in
copyWorkflowYamlStripped, the catch block logs a warning and falls back tofs.copy. However, this could mask serious issues:
- The warning message doesn't include the error details
- If
fs.copyalso fails, the error chain is lost- The user has no way to know their
web_bundlesection wasn't stripped} catch { // If anything fails, just copy the file as-is await prompts.log.warn(` Could not process ${path.basename(sourceFile)}, copying as-is`); await fs.copy(sourceFile, targetFile, { overwrite: true }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/modules/manager.js` around lines 876 - 880, The catch block in copyWorkflowYamlStripped currently swallows the original error and silently falls back to fs.copy; change it to catch the error as e, include e.message/stack in the prompts.log.warn so the original YAML processing error is visible, then perform the fs.copy inside its own try/catch so any copy failure is logged and rethrown (or returned) to preserve the error chain, and add a clear log note that the web_bundle section was not stripped when falling back. Ensure you update the catch in copyWorkflowYamlStripped to reference the error variable and handle fs.copy errors explicitly.
1193-1208:⚠️ Potential issue | 🟡 MinorRegex patterns for parsing workflow paths are fragile and may not match all valid formats.
The regex at line 1193
sourceWorkflowPath.match(/\{project-root\}\/(?:_bmad)\/([^/]+)\/workflows\/(.+)/)only matches paths with_bmadprefix, but the comment at line 1191 says it should handle "hardcoded 'bmad'" too. The regex doesn't actually match thebmad/variant.Similarly, line 1204's regex only matches
_bmad, not other folder names.♻️ Proposed fix to match both patterns
- const sourceMatch = sourceWorkflowPath.match(/\{project-root\}\/(?:_bmad)\/([^/]+)\/workflows\/(.+)/); + const sourceMatch = sourceWorkflowPath.match(/\{project-root\}\/(?:_bmad|bmad)\/([^/]+)\/workflows\/(.+)/);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/modules/manager.js` around lines 1193 - 1208, The path-parsing regexes for sourceWorkflowPath and installWorkflowPath are too strict and only match `_bmad` not `bmad`; update both regexes (used where sourceWorkflowPath.match(...) producing sourceMatch and installWorkflowPath.match(...) producing installMatch) to accept either `_bmad` or `bmad` (e.g. use a pattern that allows an optional leading underscore like _?bmad), and ensure the capture groups still yield the same values used later (sourceModule, sourceWorkflowSubPath and the corresponding install captures) so downstream code extracting installMatch[...] continues to work.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
package.jsontest/test-fs-wrapper.jstest/test-installation-components.jstools/cli/commands/status.jstools/cli/commands/uninstall.jstools/cli/installers/lib/core/config-collector.jstools/cli/installers/lib/core/custom-module-cache.jstools/cli/installers/lib/core/dependency-resolver.jstools/cli/installers/lib/core/detector.jstools/cli/installers/lib/core/ide-config-manager.jstools/cli/installers/lib/core/installer.jstools/cli/installers/lib/core/manifest-generator.jstools/cli/installers/lib/core/manifest.jstools/cli/installers/lib/custom/handler.jstools/cli/installers/lib/ide/_base-ide.jstools/cli/installers/lib/ide/_config-driven.jstools/cli/installers/lib/ide/codex.jstools/cli/installers/lib/ide/github-copilot.jstools/cli/installers/lib/ide/kilo.jstools/cli/installers/lib/ide/manager.jstools/cli/installers/lib/ide/platform-codes.jstools/cli/installers/lib/ide/rovodev.jstools/cli/installers/lib/ide/shared/agent-command-generator.jstools/cli/installers/lib/ide/shared/bmad-artifacts.jstools/cli/installers/lib/ide/shared/module-injections.jstools/cli/installers/lib/ide/shared/task-tool-command-generator.jstools/cli/installers/lib/ide/shared/workflow-command-generator.jstools/cli/installers/lib/message-loader.jstools/cli/installers/lib/modules/external-manager.jstools/cli/installers/lib/modules/manager.jstools/cli/lib/activation-builder.jstools/cli/lib/agent-analyzer.jstools/cli/lib/agent-party-generator.jstools/cli/lib/config.jstools/cli/lib/file-ops.jstools/cli/lib/fs.jstools/cli/lib/platform-codes.jstools/cli/lib/project-root.jstools/cli/lib/ui.jstools/cli/lib/xml-handler.jstools/cli/lib/yaml-xml-builder.jstools/migrate-custom-module-paths.js
adambiggs
force-pushed
the
fix/installer-nondeterministic-file-loss
branch
from
f5cf288 to
17f9cdf
Compare
…inistic file loss fs-extra routes all async operations through graceful-fs, whose EMFILE retry queue causes non-deterministic file loss on macOS APFS during bulk copy operations (~500+ files). Approximately 50% of install runs lose 26+ files from _bmad/. Replace fs-extra entirely with a thin native wrapper (tools/cli/lib/fs.js) that provides the same API surface backed by node:fs and node:fs/promises. Copy and remove operations use synchronous native calls to eliminate the race condition. Verified across 8+ consecutive runs with zero file loss. - Add tools/cli/lib/fs.js native wrapper with full fs-extra API compat - Update all 40 files to require the wrapper instead of fs-extra - Remove fs-extra from package.json dependencies - Add 37-test suite including 250-file bulk copy determinism test
adambiggs
force-pushed
the
fix/installer-nondeterministic-file-loss
branch
from
17f9cdf to
373c06b
Compare
…endency - copy() overwrite:false catch now only ignores ENOENT/ENOTDIR, consistent with pathExists(); permission errors propagate correctly - 'copy creates parent directories' test creates its own fixture instead of depending on state from a previous test
|
Thanks for this @adambiggs - we will review soon! |
2 participants
What
Replace
fs-extraentirely with a thin nativenode:fs/node:fs/promiseswrapper to fix non-deterministic file loss during multi-module installation.Why
fs-extraroutes all operations throughgraceful-fs, which globally monkey-patchesnode:fswith an EMFILE retry queue (setTimeout(retry, 0)). Even though the installer code is fully sequential (everyfs.copy()is individuallyawaited infor...ofloops), graceful-fs's deferred retries can fire after anawaitresolves.Each module installation starts with
fs.remove(targetPath)followed by file-by-file copying into the same location. Deferred retry-queued unlink operations from the remove phase can fire after subsequent copies have written files, silently deleting them. Single-module installs (~200 file ops) are unlikely to trigger this, but multi-module installs (~580 file ops for bmm+bmb+tea) hit it frequently — ~50% of runs lost 26+ files in our testing.Fixes #1779
How
tools/cli/lib/fs.js— thin native wrapper providing the same API surface (copy,remove,move,ensureDir,pathExists,readJsonSync, etc.) backed bynode:fsandnode:fs/promisesdirectly, bypassinggraceful-fscopyFileSync,mkdirSync,rmSync) so when anawaitresolves, the operation is definitively complete with no deferred background workrequirethe wrapper instead offs-extrafs-extrafrompackage.jsondependenciesTesting
test/test-fs-wrapper.js) covering all wrapper methods including a 250-file bulk copy determinism test