fix(safety): E2E sequence-gap message reports the wrong "last" counter - #57
Merged
Conversation
safety/e2e.go's gap-detection branch set r.lastSeq to the just-received (gapped) sequence number *before* using it to build the diagnostic message, so "last=%d" in the E2EError.Message always printed recv-1 instead of the actual previously-accepted counter. The gap-detection condition and rejection behavior were unaffected — only the forensic message text was wrong, which misleads anyone debugging a sequence gap from logs (a safety-relevant condition per REQ-SAFETY-009). Capture the previous counter before overwriting r.lastSeq, and assert on the exact message text in TestUnwrap_sequenceGap so this can't silently regress again. Closes #49 Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
6 tasks
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.
Summary
safety/e2e.go's gap-detection branch mutatedr.lastSeqto the just-received (gapped) sequence number before using it to build the diagnostic message, soMessagealways reportedrecv-1instead of the actual previously-accepted counter.*E2EError{Kind: ErrSequenceGap}) were correct and unaffected — only the human-readable forensic text was wrong.r.lastSeq, and lock in the exact message text with an assertion inTestUnwrap_sequenceGap.Why it matters
This message is forensic/diagnostic text for investigating sequence gaps — a safety-relevant condition per REQ-SAFETY-009. A misleading "last" value undermines root-cause analysis after a real fault.
Closes #49
Test plan
go build ./...,go vet ./...go test -race -count=1 ./...— all green, including the updatedTestUnwrap_sequenceGapgofusa check/trace/verify(v0.36.0, this repo's current CI pin) — all green