Repository navigation
Harden forge posting: transient failures no longer discard a completed review - #30
Merged
Merged
Conversation
…d review A forge posting failure (rate limit, transient 5xx, network blip) on the success path previously propagated through run_remote/finish and exited 2, discarding the already-computed envelope/SARIF output even though the review itself succeeded. finish() now logs post_review and complete_checks failures to stderr instead of propagating them; the exit code always derives from envelope.gate.failing, matching the existing error-path handling and the documented exit-code contract. GitLab's per-finding comment loop aborted on the first failed post, unlike Bitbucket and Azure, which post everything they can and report failures together; GitLab now does the same via a post_finding helper. Azure reconstructs diffs by fetching full old/new file content with no per-file size limit, so one huge file was fetched and diffed in full before any global cap applied. Files over a 2 MiB per-file cap are now marked skipped the same way binary files are, instead of being diffed. Adds a unit test pinning that the ProviderError marker used by the fail-open/fail-closed classifier survives additional anyhow context wrapping, and a regression test covering the forge-posting-failure exit code.
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.
A forge posting failure (rate limit, transient 5xx, network blip) on the success path used to propagate all the way out and exit 2, discarding the already-computed envelope/SARIF output even though the review itself succeeded; finish() now logs post_review and complete_checks failures to stderr and always derives the exit code from the gate, matching the existing error-path handling. GitLab's per-finding comment loop aborted on the first failed post instead of posting everything it can and reporting failures together, unlike Bitbucket and Azure; it now follows the same pattern. Azure reconstructed diffs by fetching full old/new file content with no per-file size limit, so one huge file was fetched and diffed in full before any global cap applied; files over a 2 MiB per-file cap are now marked skipped the same way binary files are. Also adds a unit test pinning that the ProviderError marker survives additional anyhow context wrapping and a regression test covering the forge-posting exit-code behavior; cargo fmt, clippy (zero warnings), and the full test suite (76 unit + 33 integration tests) pass.