Repository navigation
Guard ExecutionJournal#record against JSON::GeneratorError - #77
Open
minerva-sky wants to merge 1 commit into
Open
minerva-sky wants to merge 1 commit into
minerva-sky wants to merge 1 commit into
Conversation
A journal write runs from after_task_success, so a payload the json gem refuses to serialize (a BINARY-encoded non-ASCII string, NaN, and on newer json versions a payload with both :k and "k") raised after the task had already succeeded. Rescue GeneratorError and write a fallback line that keeps event, at, task_id and description so replay still counts the task, plus payload_error and a 500-char scrubbed inspect of what could not be written. Follows the lean in codenamev#76: parsing stays strict, the writer alone must never fail a finished task. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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 the one unguarded spot called out in #76.
What
ExecutionJournal#recordruns fromafter_task_success. If the json gem refuses the payload, the task that already succeeded blew up on its own logging. Concretely: a BINARY-encoded non-ASCII string,Float::NAN, and on json versions that reject it, a hash carrying both:kand"k".The writer now rescues
JSON::GeneratorErrorand lands a fallback line instead:event,at,task_id,description(the String fields replay keys on), so the task still counts as completed andmode: :strictreplay does not trip on a missingtask_idpayload_error(the exception class) andpayload_inspect(first 500 chars ofpayload.inspect, scrubbed) so a recovery tool can see what was lostParsing stays strict, per the #76 lean. Only the writer changes.
Specs
Two new examples in
spec/agentic/execution_journal_spec.rb: a BINARY"caf\xE9"output lands, carriespayload_error, and replays strict as completed; a{:k => 1, "k" => 2}output lands and replays. The second one is deliberately version-agnostic: on the bundled json 2.7.1 it writes the ambiguous line as before, on newer json it takes the rescue path, and either way the task shows up incompleted_task_ids.bundle exec rspec spec/agentic/execution_journal_spec.rb→ 11 examples, 0 failures. standardrb clean on both files.Not in this PR
A fallback line means
state.outputs[task_id]isnilfor that task on replay. A resuming plan would treat it as done with no output. If that should instead surface onstate.damage, that is a one-line follow-up inreplay; I left it out to keep this to the writer.🤖 Generated with Claude Code