-
Notifications
You must be signed in to change notification settings - Fork 20
Description
Summary
Multiple issues in the CLI's file handling cause crashes and data corruption when installPlugins is called concurrently (e.g. from adapt-authoring-adaptframework during course import).
Problems
1. readValidateJSON silently swallows errors (lib/util/JSONReadValidate.js)
export async function readValidateJSON (filepath, done) {
try {
const data = await fs.readFile(filepath, 'utf8')
validateJSON(data, filepath)
done?.(null, JSON.parse(data))
return JSON.parse(data)
} catch (err) {
done?.(err.message)
// BUG: returns undefined instead of re-throwing
}
}When the file can't be read (concurrent write, permissions, etc.), this returns undefined instead of throwing, causing downstream TypeError crashes.
2. getManifestDependencies doesn't handle missing data (lib/integration/Project.js:63-66)
async getManifestDependencies () {
const manifest = await readValidateJSON(this.manifestFilePath)
return manifest.dependencies // crashes if manifest is undefined or has no dependencies key
}3. project.add() has an unsafe read-modify-write cycle (lib/integration/Project.js:123-133)
add (plugin) {
let manifest = { version: '0.0.0', dependencies: {} }
if (this.containsManifestFile) {
manifest = readValidateJSONSync(this.manifestFilePath) // READ
}
manifest.dependencies[plugin.packageName] = ... // MODIFY
fs.writeJSONSync(this.manifestFilePath, manifest, ...) // WRITE
}Under concurrency, two add() calls can both read the same state, each add their plugin, and the second write silently overwrites the first plugin's entry. This is a classic lost-update race condition.
Impact
When installPlugins is called concurrently, random plugins are reported as "failed to install" even though they installed successfully — it's only the post-install manifest update that fails. Different plugins fail on each run due to non-deterministic timing.
Suggested Fixes
-
readValidateJSON: re-throw on error} catch (err) { done?.(err.message) throw err }
-
getManifestDependencies: defensive accessasync getManifestDependencies () { const manifest = await readValidateJSON(this.manifestFilePath) return manifest?.dependencies ?? {} }
-
project.add(): use a file lock or queue writes through a single serialized method to prevent lost updates
Metadata
Metadata
Assignees
Labels
Type
Projects
Status