Repository navigation
fix(goal-loop): one unreadable loop no longer loses every goal loop (#1248) - #1258
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…loop One loop this build cannot read (a newer build's phase or reason, then a downgrade) moved the whole goal-loop.json aside; the service started empty and its next write made every loop's loss permanent. Valid loops are now read, the original bytes are copied to the existing .corrupt name before any write can drop the unreadable one, and a malformed container still moves aside and throws. Closes #1248 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ive file
Review A / steering q18: the set-aside copy was written inside the parse
try, so a failed copy (an unwritable or directory `.corrupt`) was handled
as an unreadable document. The live file was moved away and start()
persisted `{loops:{}}`, losing every loop and an older quarantine.
Preservation now runs outside that catch, to a digest-named atomic copy
that never replaces `.corrupt`. A failed copy, or a failed whole-file
move, makes the store refuse writes until a read succeeds, so the
service's start-empty persist cannot replace the only copy. The size
limit now counts readable loops, and a partial copy left by a crash is
not trusted.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er guards; name set-aside ids Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… of lasting the process The service reads the store only at start, so "refuse writes until a read succeeds" disabled persistence for the whole process. Each write now first retries the owed preservation (the digest copy, or the whole-file move to .corrupt) and proceeds once it succeeds; it is still refused while it fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… is owed (steering q19) On a per-loop copy failure read() threw, so the service started empty. Once the obstruction cleared, the retried write persisted that empty map and dropped every valid loop. read() now returns the valid loops and owes the copy: every write waits for it (the live file keeps the unreadable loop until its bytes are safe), and the first write after it succeeds carries the valid loops forward. The service-path test follows the whole handoff: obstructed start, refused persist, repair, then a write holding the valid loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
|
Review disposition (2 rounds, plus steering q18/q19; merge on green)
CI quality-gate is green on a head that contains origin/main. Closes #1248. 🤖 Generated with Claude Code |
Juliusolsson05
added a commit
that referenced
this pull request
Sep 25, 2026
…ate rule, #1309 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Juliusolsson05
added a commit
that referenced
this pull request
Sep 25, 2026
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
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.
Closes #1248.
Verified failure
One loop that fails validation (a newer build's phase or reason, after a downgrade) made
GoalLoopStore.read()move the wholegoal-loop.jsonto.corruptand throw.GoalLoopService.start()then ran empty, and its next write made every loop's loss permanent. An existing test pinned that whole-file behaviour. Plan:docs/plans/2026-09-25-goal-loop-store-invalid-loop.md(plan-only commit first).Fix
.corruptname before the next write can drop the unreadable loop. It is not moved, because the file still holds the loops just read.Evidence
testing/fixtures/goal-loop/real-loops-2026-09-25.json.tsc -bis clean;src/main/goalLooppasses (79 tests).🤖 Generated with Claude Code