fix: handle concurrent file edits and symlink loops

[carlos-alm]

Summary

• Add readFileSafe helper in builder.js that retries on transient OS errors (EBUSY/EACCES/EPERM) caused by editors performing non-atomic saves — replaces bare readFileSync in 3 builder sites and 1 watcher site
• Add symlink loop detection to collectFiles via realpathSync visited-set tracking, preventing infinite recursion from circular symlinks
• Update generated/architecture.md sections Bump tree-sitter-python from 0.23.6 to 0.25.0 #7, fix: rewrite v1.4.0 changelog and add license scan allowlist #15, and summary table to reflect these fixes

Test plan

• All 463 tests pass (npm test)
• Lint clean (npm run lint)
• Self-build succeeds (node src/cli.js build .)

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review Complete

Review Checklist:

• Read repository context and guidelines
• Analyze changed files and implementation
• Phase 1: Root cause analysis
• Phase 2: Configuration change detection
• Phase 3: Test integrity verification
• Phase 4: Code quality assessment
• Provide final rating and recommendation

---

Root Cause Analysis

Problem Being Solved: Two legitimate operational issues: (1) Transient OS errors (EBUSY/EACCES/EPERM) when editors perform non-atomic saves causing file reads to fail intermittently, and (2) Infinite recursion when collectFiles encounters circular symlinks.

Why This Approach: The solutions address real root causes, not symptoms:

• readFileSafe retry logic handles genuine OS-level race conditions during editor saves (documented issue)
• Symlink loop detection via realpathSync visited-set tracking prevents infinite recursion - a classic traversal solution
• Both fixes are surgical additions to existing code paths without changing behavior for normal cases

Risk Assessment: Low risk. These are defensive fixes that only activate under error conditions that currently cause failures.

Critical Concerns

None. This is exactly the kind of defensive programming needed for robust file system interaction.

Configuration Change Detection

No configuration changes detected. No linting rules, test thresholds, or build configs modified.

Test Integrity

Excellent test hygiene. No tests were modified or weakened. All 463 existing tests pass, demonstrating the changes don't break existing functionality. The fixes are additive defensive measures.

Code Quality Assessment

readFileSafe Implementation (builder.js:155-167)

Strengths:

• Properly scoped retry logic (only EBUSY/EACCES/EPERM errors)
• Configurable retry count (default 2)
• Uses Atomics.wait for precise delay timing
• Clean fallback - re-throws non-transient errors immediately
• Shared utility pattern - exported for reuse by watcher.js

Technical soundness: This is a textbook solution for editor race conditions. The retry mechanism is conservative and the error codes are exactly what you'd expect from file system contention.

Symlink Loop Detection (builder.js:55-66)

Strengths:

• Uses realpathSync to resolve to canonical paths for accurate loop detection
• Proper visited-set tracking with _visited parameter threading
• Graceful error handling if realpathSync fails
• Informative warning message when loops detected
• Early return prevents infinite recursion

Technical soundness: This is the standard algorithm for cycle detection in file system traversal. The implementation is clean and correct.

Integration Quality

• readFileSafe properly replaces bare readFileSync calls in 4 strategic locations
• Symlink fix is integrated into the existing collectFiles recursive function cleanly
• No API changes or breaking modifications
• Documentation updates in architecture.md accurately reflect the changes

Minor Observations

• The 50ms retry delay is reasonable for editor contention scenarios
• The default of 2 retries gives 3 total attempts (initial + 2 retries) which is appropriate
• The TRANSIENT_CODES set could potentially include additional codes like 'EMFILE' for file descriptor exhaustion, but the current set covers the documented use case

Historical Context

The architecture document confirms these exact issues were identified in the "cold analysis":

"two concrete edge cases — concurrent file edits causing EBUSY/EACCES during read, and symlink loops causing infinite recursion in collectFiles — have been fixed"

This PR delivers precisely the fixes that were planned.

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE
• Reasoning: This is high-quality defensive programming that fixes two real operational issues without changing behavior for normal cases. The implementation is technically sound, properly tested, and follows good engineering practices. The only reason it's not 5 stars is that these are bug fixes rather than innovative features, but they're exceptionally well-executed bug fixes.

Key Strengths:

• Addresses genuine root causes, not symptoms
• Minimal, surgical changes with proper error handling
• Excellent code reuse (readFileSafe shared between modules)
• No configuration workarounds or test weakening
• All existing tests pass, demonstrating safety
• Clear documentation updates

This is exactly the kind of reliability improvement that makes a production system more robust.

---

[greptile-apps]

Greptile Summary

This PR introduces two defensive fixes for edge cases in file handling: transient OS error retries and symlink loop detection.

Key changes:

• Added readFileSafe helper that retries on EBUSY/EACCES/EPERM errors (common with non-atomic editor saves) using a busy-wait sleep implementation
• Enhanced collectFiles with realpathSync-based visited-set tracking to detect and break symlink loops
• Shared readFileSafe between builder.js and watcher.js, replacing 4 bare readFileSync calls
• Updated generated/architecture.md to document these improvements as partial progress toward unified builder/watcher code paths

The implementation is clean and handles edge cases gracefully (e.g., realpathSync failures return early). The busy-wait sleep was corrected from an invalid Atomics.wait approach per previous review feedback.

Confidence Score: 5/5

• Safe to merge — defensive fixes with graceful error handling and no breaking changes
• The changes are well-contained, fix real edge cases (concurrent file edits and symlink loops), and follow the existing code patterns. The busy-wait sleep issue identified in previous review was already fixed. All tests pass according to the PR description, and the implementation uses standard error handling patterns.
• No files require special attention

Important Files Changed

Filename	Overview
src/builder.js	Added readFileSafe with retry logic for transient OS errors and symlink loop detection in collectFiles using realpathSync visited-set tracking
src/watcher.js	Replaced bare readFileSync with imported readFileSafe for consistent error handling

_{Last reviewed commit: 24f8ab1}

[greptile-apps]

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Atomics.wait cannot be used on the main thread in Node.js — it will throw "Atomics.wait cannot be called in this context". Use a simple synchronous sleep implementation instead:

Suggested change

	Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, RETRY_DELAY_MS);
	const end = Date.now() + RETRY_DELAY_MS;
	while (Date.now() < end) {}

[carlos-alm]

Fixed in 24f8ab1 — replaced with Date.now() busy-wait loop. Good catch.

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review Complete

Review Checklist:

• Read repository context and guidelines
• Analyze changed files and implementation
• Phase 1: Root cause analysis
• Phase 2: Configuration change detection
• Phase 3: Test integrity verification
• Phase 4: Code quality assessment
• Provide final rating and recommendation

---

Root Cause Analysis

Problem Being Solved: Two legitimate operational issues: (1) Transient OS errors (EBUSY/EACCES/EPERM) when editors perform non-atomic saves causing file reads to fail intermittently, and (2) Infinite recursion when collectFiles encounters circular symlinks.

Why This Approach: The solutions address real root causes, not symptoms:

• readFileSafe retry logic handles genuine OS-level race conditions during editor saves (documented issue)
• Symlink loop detection via realpathSync visited-set tracking prevents infinite recursion - a classic traversal solution
• Both fixes are surgical additions to existing code paths without changing behavior for normal cases

Risk Assessment: Low risk. These are defensive fixes that only activate under error conditions that currently cause failures.

Critical Concerns

None. This is exactly the kind of defensive programming needed for robust file system interaction.

Configuration Change Detection

No configuration changes detected. No linting rules, test thresholds, or build configs modified.

Test Integrity

Excellent test hygiene. No tests were modified or weakened. All 463 existing tests pass, demonstrating the changes don't break existing functionality. The fixes are additive defensive measures.

Code Quality Assessment

readFileSafe Implementation (builder.js:155-167)

Strengths:

• Properly scoped retry logic (only EBUSY/EACCES/EPERM errors)
• Configurable retry count (default 2)
• Uses safe busy-wait loop instead of Atomics.wait (fixes critical Node.js main thread issue)
• Clean fallback - re-throws non-transient errors immediately
• Shared utility pattern - exported for reuse by watcher.js

Technical soundness: This is a textbook solution for editor race conditions. The retry mechanism is conservative and the error codes are exactly what you'd expect from file system contention.

Symlink Loop Detection (builder.js:55-66)

Strengths:

• Uses realpathSync to resolve to canonical paths for accurate loop detection
• Proper visited-set tracking with _visited parameter threading
• Graceful error handling if realpathSync fails
• Informative warning message when loops detected
• Early return prevents infinite recursion

Technical soundness: This is the standard algorithm for cycle detection in file system traversal. The implementation is clean and correct.

Integration Quality

• readFileSafe properly replaces bare readFileSync calls in 4 strategic locations
• Symlink fix is integrated into the existing collectFiles recursive function cleanly
• No API changes or breaking modifications
• Documentation updates in architecture.md accurately reflect the changes

Minor Observations

• The 50ms retry delay is reasonable for editor contention scenarios
• The default of 2 retries gives 3 total attempts (initial + 2 retries) which is appropriate
• The TRANSIENT_CODES set could potentially include additional codes like 'EMFILE' for file descriptor exhaustion, but the current set covers the documented use case

Historical Context

The architecture document confirms these exact issues were identified in the "cold analysis":

"two concrete edge cases — concurrent file edits causing EBUSY/EACCES during read, and symlink loops causing infinite recursion in collectFiles — have been fixed"

This PR delivers precisely the fixes that were planned.

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE
• Reasoning: This is high-quality defensive programming that fixes two real operational issues without changing behavior for normal cases. The implementation is technically sound, properly tested, and follows good engineering practices. The only reason it's not 5 stars is that these are bug fixes rather than innovative features, but they're exceptionally well-executed bug fixes.

Key Strengths:

• Addresses genuine root causes, not symptoms
• Minimal, surgical changes with proper error handling
• Excellent code reuse (readFileSafe shared between modules)
• No configuration workarounds or test weakening
• All existing tests pass, demonstrating safety
• Clear documentation updates

This is exactly the kind of reliability improvement that makes a production system more robust.

---

[carlos-alm]

@greptileai

[greptile-apps]

_{4 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[carlos-alm]

@greptileai

[greptile-apps]

_{4 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}