Skip to content

fix(firehose): seed firehose-apps.json with empty data; atomic write …#709

Merged
castrojo merged 1 commit into
projectbluefin:mainfrom
castrojo:fix/p0-seed-and-atomic-write
Apr 1, 2026
Merged

fix(firehose): seed firehose-apps.json with empty data; atomic write …#709
castrojo merged 1 commit into
projectbluefin:mainfrom
castrojo:fix/p0-seed-and-atomic-write

Conversation

@castrojo
Copy link
Copy Markdown
Contributor

@castrojo castrojo commented Apr 1, 2026

…for images.json

  • Replace 1.5MB populated firehose-apps.json with a minimal empty seed {"apps":[],"metadata":{"schemaVersion":"1.0.0","generatedAt":null}} so a clean git clone builds successfully without running fetch-firehose.js Closes docs(tips): my recommended extensions #48
  • Write images.json atomically via a .tmp file + fs.renameSync to prevent build-time reads from seeing a partially-written file on concurrent runs Closes feat: move to Docusaurus! #49

Assisted-by: Claude Sonnet 4.6 via GitHub Copilot

…for images.json

- Replace 1.5MB populated firehose-apps.json with a minimal empty seed
  {"apps":[],"metadata":{"schemaVersion":"1.0.0","generatedAt":null}}
  so a clean git clone builds successfully without running fetch-firehose.js
  Closes #48
- Write images.json atomically via a .tmp file + fs.renameSync to prevent
  build-time reads from seeing a partially-written file on concurrent runs
  Closes #49

Assisted-by: Claude Sonnet 4.6 via GitHub Copilot
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@castrojo castrojo merged commit 93466da into projectbluefin:main Apr 1, 2026
1 check passed
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies scripts/fetch-github-images.js to implement atomic file writes by using a temporary file before renaming it to the final destination. A review comment identified a potential race condition where concurrent executions of the script would collide on the same static temporary filename and suggested incorporating the process ID into the filename to ensure uniqueness.

Comment on lines +756 to +758
const TMP_FILE = OUTPUT_FILE + ".tmp";
fs.writeFileSync(TMP_FILE, JSON.stringify(output, null, 2), "utf-8");
fs.renameSync(TMP_FILE, OUTPUT_FILE);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using a static filename for the temporary file (OUTPUT_FILE + ".tmp") introduces a race condition if multiple instances of this script run concurrently. While the atomic rename protects readers, concurrent writers will collide on the same temporary file, potentially leading to data corruption or process crashes (e.g., when one instance renames the file while another is still writing to it). To safely handle concurrent runs as intended, use a unique temporary filename by incorporating the process ID.

Suggested change
const TMP_FILE = OUTPUT_FILE + ".tmp";
fs.writeFileSync(TMP_FILE, JSON.stringify(output, null, 2), "utf-8");
fs.renameSync(TMP_FILE, OUTPUT_FILE);
const TMP_FILE = OUTPUT_FILE + "." + process.pid + ".tmp";
fs.writeFileSync(TMP_FILE, JSON.stringify(output, null, 2), "utf-8");
fs.renameSync(TMP_FILE, OUTPUT_FILE);

@castrojo castrojo deleted the fix/p0-seed-and-atomic-write branch May 9, 2026 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant