feat(risk): add GitLab support for risk assessment tier scripts - #981
feat(risk): add GitLab support for risk assessment tier scripts#981maruiz93 wants to merge 18 commits into
Conversation
|
🤖 Finished Review · ✅ Success · Started 8:03 AM UTC · Completed 8:40 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.40 |
PR Summary by QodoAdd GitLab support and pipeline integration for PR risk assessment
AI Description
Diagram
High-Level Assessment
Files changed (23)
|
Deterministic bash script computing PR metadata signals (blast radius, path sensitivity, CI impact, dependency risk, test coverage, author context) for the risk assessment sub-agent. KEY=VALUE output format. Refs: fullsend-ai/fullsend#4698 Signed-off-by: Marta Anon <manon@redhat.com>
SKILL.md defines the three-tier scoring model (metadata 50%, git history 30%, linked issue 20%) with anchoring examples and output format. Sub-agent definition dispatches as a synchronous pre-pass using sonnet. Refs: fullsend-ai/fullsend#4698 Signed-off-by: Marta Anon <manon@redhat.com>
Add risk_assessment to review-result.schema.json (optional field). Add FULLSEND_RISK_ASSESSMENT_ENABLED to env/review.env (default true). Update orchestrator: roster table, generalized skill-loading table (replaces docs-currency special case), and new step 3c-2 for the risk assessment pre-pass. Refs: fullsend-ai/fullsend#4698 Signed-off-by: Marta Anon <manon@redhat.com>
Post-review.sh reads risk_assessment from the result JSON, applies a risk/* label with color coding, and posts a sticky comment with the score breakdown. Labels are idempotent — stale risk/* labels are removed before applying the new one. Refs: fullsend-ai/fullsend#4698 Signed-off-by: Marta Anon <manon@redhat.com>
Two review eval cases: a low-risk typo fix (expected risk/low label) and a high-risk auth refactor (expected risk/high or risk/critical). Exercises the full pipeline including risk sub-agent and post-review label application. Refs: fullsend-ai/fullsend#4698 Signed-off-by: Marta Anon <manon@redhat.com>
…rding Replace word-splitting array assignment with mapfile to handle filenames containing spaces. Fix 'Both' to 'All three' in non-standard dispatch type documentation. Refs: fullsend-ai/fullsend#4698 Signed-off-by: Marta Anon <manon@redhat.com>
- Add skills/pr-risk-assessment to harness skills array (sandbox access) - Rename FULLSEND_RISK_ASSESSMENT_ENABLED to REVIEW_RISK_ASSESSMENT_ENABLED - Add --paginate to PR files API call for PRs with >100 files - Add RISK_SCORE integer 1-5 validation with warning fallback - URL-encode AUTHOR in search query for defense-in-depth - Read REVIEW_PROTECTED_PATHS env var with hardcoded fallback - Add -e omission comment to risk-tier1.sh shell opts - Add Glob to risk-assessment sub-agent tools list - Add score/level consistency description to schema - Document risk labels and REVIEW_RISK_ASSESSMENT_ENABLED in docs - Add test coverage: invalid level/score, stale label removal Signed-off-by: Marta Anon <maruiz93@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
.github/* matched all files under .github/ (CODEOWNERS, ISSUE_TEMPLATE, etc.) as CI changes. Restrict to .github/workflows/* and .github/actions/*, and add .gitlab-ci.yml, Jenkinsfile, azure-pipelines.yml patterns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
Replace undocumented TARGET_REPO_DIR with the standard REPO_DIR/GITHUB_WORKSPACE/target-repo fallback chain used by other scripts. Add a warning when the target directory is not found instead of silently skipping. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
Add if/then constraints so {score: 1, level: "critical"} is rejected
by schema validation. Each score value is tied to its canonical level.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Marta Anon <manon@redhat.com>
The requirements*.txt glob already covers the requirements.txt literal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
Replace double jq-parse for author signals with single @TSV projection. Add e2e test for multi-page pagination (jq -s 'add' merge). Signed-off-by: Marta Anon <maruiz93@users.noreply.github.com> Signed-off-by: Marta Anon <manon@redhat.com>
… fixes - Fix compute_test_ratio to match on basename, so nested test_* files (e.g. tests/test_foo.py) are correctly detected - Add risk_label_present eval judge to catch a broken risk pipeline (no-op assessment silently passes without it) - Fix invalid risk level fallthrough: run stale-label cleanup when level fails validation instead of silently skipping - Move rationale truncation into jq to avoid SIGPIPE under pipefail - Add frontmatter qualifier to SKILL.md model instruction for consistency - Document REVIEW_GIT_FETCH_DEPTH in docs/review.md Variables table Signed-off-by: Marta Anon <maruiz93@users.noreply.github.com> Signed-off-by: Marta Anon <manon@redhat.com>
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
ReviewFindingsCritical
Medium
Low
Next steps:
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
|
fullsend-ai-review
Bot
added
the
requires-manual-review
Move the risk assessment label section before forge_post_review so risk/* labels land even when the review submission fails — e.g. when GitHub returns 422 for a self-review (same token creates and reviews the PR, as happens in eval environments). Previously, a 422 on REQUEST_CHANGES caused post-review.sh to exit before reaching the risk label block, leaving cases 001 and 002 without risk/* labels and failing the risk_label_present eval judge. Signed-off-by: Marta Anon <marta@redhat.com> Signed-off-by: Marta Anon <manon@redhat.com>
|
Re: api-contract (hardcoded host allowlist) — the duplication is intentional. |
|
Addressed in baf841c:
|
|
🤖 Review · Commit: |
Extract remove_stale_risk_labels() helper to deduplicate risk-label removal loops, fix return→exit 0 in pagination test stub, and add array-type guard for gh API edge case in risk-tier1.sh. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
baf841c to
b19d023
Compare
|
🤖 Review · Commit: |
b19d023 to
6f112b6
Compare
|
🤖 Finished Review · ✅ Success · Started 10:24 AM UTC · Completed 10:43 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.03 |
- Add skills/pr-risk-assessment to harness skills array (sandbox access) - Rename FULLSEND_RISK_ASSESSMENT_ENABLED to REVIEW_RISK_ASSESSMENT_ENABLED - Add --paginate to PR files API call for PRs with >100 files - Add RISK_SCORE integer 1-5 validation with warning fallback - URL-encode AUTHOR in search query for defense-in-depth - Read REVIEW_PROTECTED_PATHS env var with hardcoded fallback - Add -e omission comment to risk-tier1.sh shell opts - Add Glob to risk-assessment sub-agent tools list - Add score/level consistency description to schema - Document risk labels and REVIEW_RISK_ASSESSMENT_ENABLED in docs - Add test coverage: invalid level/score, stale label removal Signed-off-by: Marta Anon <maruiz93@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
Make risk-tier1.sh forge-aware with _fetch_pr_files_json() and _fetch_pr_meta() that dispatch on FULLSEND_FORGE. GitLab path uses curl against the MR API with diff-text parsing for line counts and first_contribution field for author signals. Add GitLab clone deepening in pre-review using oauth2 token auth, with host derived from PR_URL when GITLAB_HOST is unset. Replace deprecated fullsend post-comment with fullsend issues post-comment --tracker for forge-aware sticky risk comments. Closes fullsend-ai#922 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…lback, add pagination - Sanitize ::warning:: interpolations with _gha_sanitize (pre-review.src.sh, risk-tier1.sh) - Fall back to GITLAB_TOKEN when REVIEW_TOKEN is unset in sandbox (risk-tier1.sh) - Add REVIEW_TOKEN to GitLab sandbox env in harness for consistency - Paginate GitLab /diffs endpoint with max 10 pages (risk-tier1.sh) - Validate JSON response is an array before processing diffs - Enforce host allowlist on clone-deepening credentialed URL (pre-review.src.sh) - Fix shellcheck SC2005 and git identity in CI test (pre-review-test.sh) - Add GitLab token fallback and pagination-aware test stubs (risk-tier1-test.sh) - Add GitLab forge tests for risk label and tracker-aware comment (post-review-test.sh) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
6f112b6 to
f00ffcb
Compare
|
🤖 Finished Review · ✅ Success · Started 10:46 AM UTC · Completed 11:05 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $10.46 |
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
scripts/post-review-test.sh(file-level): Line 478 · [low] naming-convention
Several test names are truncated with ellipsis (risk-a..., risk-l..., risk-g..., risk-i...). Four distinct tests all print risk-i..., making failures impossible to triage from CI logs. All other tests use full descriptive kebab-case names.
Suggested fix: Use full descriptive names matching existing patterns.
docs/review.md(file-level): Line 95 · [low] docs-currency
The paragraph below the Variables table says 'Override either variable' but the table now contains six variables. The PR widens a pre-existing staleness gap by adding two more variables.
Suggested fix: Change 'Override either variable' to 'Override any of these variables'.
| if [[ -z "${_gl_host}" && -n "${PR_URL:-}" ]]; then | ||
| _gl_host=$(echo "${PR_URL}" | sed -E 's|^https://([^/]+)/.*|\1|') | ||
| fi | ||
| if [[ -n "${_gl_token}" && -n "${_gl_host}" ]]; then |
There was a problem hiding this comment.
[low] host-validation-inconsistency
Clone-deepening and _gitlab_api_call() use a hardcoded host allowlist while other GitLab API code uses _validate_gitlab_host() which validates against CI_SERVER_HOST dynamically. Fail-closed (safe) but creates maintenance risk when onboarding new GitLab instances.
Suggested fix: Use _validate_gitlab_host where available, or document the deliberate redundancy.
fullsend-ai-review
Bot
removed
the
requires-manual-review
1 participant
Summary
Adds GitLab support for the risk assessment tier scripts.
_fetch_pr_files_json()and_fetch_pr_meta()with GitLab API support via_gitlab_api_call(). Uses the paginated/diffsendpoint, includes host allowlist validation and overflow handling.oauth2/gitlab-ci-tokenauth depending on token type.fullsend post-commentwithfullsend issues post-comment --trackerfor forge-aware sticky comments.Closes #922
Test plan
risk-tier1-test.sh— GitLab e2e test with curl stub, fallback testpre-review-test.sh— GitLab clone deepening test with git stubpost-review-test.sh— Updated expected pattern for tracker-aware commandFULLSEND_FORGE=gitlaband verify risk signals on a GitLab MR🤖 Generated with Claude Code