Skip to content

sync.js wipes skill output directories before downloading, leaving them partially emptied if a download fails #501

Description

@graysoncooper

run() cleans every output directory up front, then downloads each skill in a loop that catches per-file failures and continues without restoring anything:

ai/scripts/sync.js

Lines 105 to 138 in bad904b

console.log(`Found ${skills.length} skills`);
for (const dir of ALL_OUTPUT_DIRS) {
await fs.mkdir(dir, { recursive: true });
await cleanDirectory(dir);
}
let errors = 0;
for (const skill of skills) {
console.log(`Syncing skill: ${skill.name}`);
const skillFiles = skill.files.filter(fileName => !OMIT_FILES.has(fileName));
for (const file of skillFiles) {
const url = `${BASE_URL}/${skill.name}/${file}`;
let content;
try {
content = fetchText(url);
} catch (err) {
console.error(` Error: ${err.message}`);
errors++;
continue;
}
for (const dir of ALL_OUTPUT_DIRS) {
const outputPath = path.join(dir, skill.name, file);
await fs.mkdir(path.dirname(outputPath), { recursive: true });
await fs.writeFile(outputPath, content, "utf8");
console.log(` Written: ${outputPath}`);
}
}
}
if (errors > 0) {
throw new Error(`Sync completed with ${errors} error(s)`);

cleanDirectory removes everything except README.md/.gitkeep before any network call, and the aggregate error is only thrown after all downloads are attempted, so a mid-run fetch failure leaves the directories holding only the files that happened to download before the failure.

Reproduction

  1. Populate the canonical skills dir and all provider dirs with a complete revision (e.g. skills alpha and beta).
  2. Re-run sync with the same manifest but force the beta fetch to fail.
  3. Result: cleanup already removed the old copies, alpha is rewritten, beta is missing from all trees, and the process exits 1 with the folders half-populated.

Effect
A transient network failure can drop the previously-good skill set and leave consumers unable to load affected skills until a later sync succeeds.

Suggested fix
Download into a temp/staging revision directory and swap it into place only after all fetches succeed, or retain the old directories until the new set is fully written.

Found while testing Ito, an automated code-review tool, against recently-merged PRs. It's free for open source. Sharing this because it looked like a real bug worth fixing, not to sell anything: https://app.ito.ai/share/5aad00cc-c98d-4233-9c27-b0a43719f315?tab=details

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions