Repository navigation
chore(#583): inherit upstream REVIEW_FINDING_SEVERITY_THRESHOLD - #584
Open
fullsend-ai-coder[bot] wants to merge 1 commit into
Open
fullsend-ai-coder[bot] wants to merge 1 commit into
fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
The per-repo review harness override previously pinned REVIEW_FINDING_SEVERITY_THRESHOLD to "high" for both runner and sandbox. On PR #582 (a security-sensitive change to .github/workflows/preview.yaml) that override caused the fullsend review to post zero correctness or security findings while a competing bot caught a fork-origin guard regression on the `prepare` job. Verified behavior of scripts/post-review.sh in the pinned base harness fullsend-ai/agents@ce2eedd: * The severity filter (jq pass around line 726) is absolute: it drops every finding below the threshold with no category-based exemption. A `high`-only threshold therefore discards all `low` and `medium` findings from the correctness, security, intent-coherence and style-conventions sub-agents. * The `[protected-path]` medium that reached the comment on PR #582 does not evidence a category exemption. It is appended by the post-script itself after severity filtering (around line 875), so it survives any threshold. A per-category exemption (issue #583 option 3) is therefore not available at the pinned harness commit. This change drops the override entirely (issue #583 option 1) so the upstream default `low` applies via layered-config deep merge — the previous partial override, which set only REVIEW_FINDING_SEVERITY_THRESHOLD and relied on the upstream REVIEW_PROTECTED_PATHS, already proved the merge is deep. readonly_repo: true is retained explicitly for the bool-merge reason already documented in the file. If signal noise becomes too high after one week of observation, step up to "medium" for both runner and sandbox (option 2) by adding env.runner and env.sandbox back with that value; a comment in the file points at that follow-up. Closes #583 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Contributor
|
🚀 Preview is available at https://dd897497.enterprise-contract.pages.dev |
|
Risk Assessment: low (1/5) DetailsSingle config file change by a bot removing a YAML override, with no protected paths, no security sensitivity, no CI workflow impact, and direct 1-to-1 alignment with the linked bug issue — all signals uniformly low. |
|
Looks good to me |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
the
ready-for-merge
This branch has not been deployed
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.
Summary
Drop the per-repo
REVIEW_FINDING_SEVERITY_THRESHOLD: "high"override from.fullsend/customized/harness/review.yamlso the upstream default (low) applies via layered-config deep merge. This is option (1) from issue #583.Why
On PR #582 (a security-sensitive change to
.github/workflows/preview.yaml), the fullsend review posted zero correctness or security findings while CodeRabbit caught a fork-origin guard regression on thepreparejob. Thehigh-only threshold was the mechanism: it was discarding every sub-agent finding belowhigh.Verification of the pinned base harness
I read
scripts/post-review.shatfullsend-ai/agents@ce2eedd097dcccf17e29f4a7cd337ec4d95d7194to answer the triage's open question about whether the[protected-path]medium that posted on PR #582 bypassed the filter via a category exemption:jqpass keeps only findings whose severity rank is>=the configured threshold. There is no exception for any category.Consequence: option (3) in the issue (per-category exemption) is not available at the pinned harness commit. Options (1) and (2) are the only choices, and this PR implements (1).
Change
The upstream harness sets
REVIEW_FINDING_SEVERITY_THRESHOLD: "low"for bothrunnerandsandbox. Layered-config deep merge is already relied on in this file: the previous partial override set only this one key and leftREVIEW_PROTECTED_PATHSandREPO_FULL_NAMEto the upstreamenv.runner/env.sandboxblocks. The protected-path check continued to work on PR #582, confirming the merge is deep rather than a whole-block replace.readonly_repo: trueis retained explicitly as before (its inline comment explains why).Testing
python3 -I -c "import yaml; yaml.safe_load(open('.fullsend/customized/harness/review.yaml'))"parses the file;baseandreadonly_reporemain the only top-level keys andenvis correctly absent.scan-secretson the changed file and on the staged diff passed (no secret material).Follow-up
If signal noise is too high after one week of observation, the file contains an inline note on how to step up to option (2) by reintroducing
env.runner/env.sandboxwith"medium".Closes #583
Post-script verification
agent/583-inherit-review-severity-threshold)c7ab8183da33a46616bfd099ec792f9b551d3039..HEAD)