fix(orchestrator): use atomic_write_text in _write_feedback_file and finalize_plan (#452) - #453
Merged
Merged
Conversation
…finalize_plan (#452) Both functions used plain write_text on files that must survive concurrent access or process death without corruption — the same bug class fixed in #446 and #450. _write_feedback_file wrote .map/<branch>/<filename> feedback artifacts with write_text; a torn write leaves a partial markdown file. finalize_plan wrote artifact_manifest.json — a critical state file read by multiple tools — with write_text; a torn write can corrupt the manifest and stall every downstream gate that reads it. Replace both calls with atomic_write_text (already imported from map_utils), which uses tempfile.mkstemp for a process-unique temp and os.replace for an atomic rename, matching the pattern already applied to StepState.save, _write_retry_quarantine (#450), and _append_restored_subtask_to_plan (#446). Add regression tests that record Path.replace call sites and assert: - distinct temp paths per invocation (no shared .tmp collision) - no temp residue after a successful write - correct content written atomically Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnSmZxuMDE9BZCCh2kn8Mm
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe orchestrator now uses ChangesAtomic orchestrator writes
Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. A rabbit checked each file with care Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_write_feedback_fileusedwrite_texton.map/<branch>/<filename>feedback artifacts — a torn write leaves a partial markdown filefinalize_planusedwrite_textonartifact_manifest.json— a critical state file read by multiple tools; a torn write can corrupt the manifest and stall every downstream gateSame bug class as #446, #448, #450. Both functions already import
atomic_write_textfrommap_utils; the fix is a one-line swap in each.Changes
write_textwithatomic_write_textin_write_feedback_fileandfinalize_planinmap_orchestrator.py.jinjamake render-templatestest_write_feedback_file_uses_atomic_write,test_finalize_plan_uses_atomic_write) that recordPath.replacecall sites and assert distinct temp paths per invocation, no temp residue, and correct contentTest plan
uv run pytest tests/test_map_orchestrator.py::test_write_feedback_file_uses_atomic_write tests/test_map_orchestrator.py::test_finalize_plan_uses_atomic_write— 2 passeduv run ruff check src/ tests/— All checks passeduv run python -m pyright src/— 0 errors, 0 warnings, 0 informationsmake check-render— Generated trees match templates_src🤖 Generated with Claude Code
https://claude.ai/code/session_01EnSmZxuMDE9BZCCh2kn8Mm
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
Tests